diff --git a/npc/analyser.go b/npc/analyser.go index 0e98d8d6..376ffa6b 100644 --- a/npc/analyser.go +++ b/npc/analyser.go @@ -46,13 +46,13 @@ func (ns *ns) analysePolicyLegacy(policy *extnapi.NetworkPolicy) ( // From is not provided, this rule matches all sources (traffic not restricted by source). if ingressRule.Ports == nil { // Ports is not provided, this rule matches all ports (traffic not restricted by port). - rule := newRuleSpec(ingress, nil, nil, dstSelector, nil) + rule := newRuleSpec(ingressPolicy, nil, nil, dstSelector, nil) rules[rule.key] = rule } else { // Ports is present and contains at least one item, then this rule allows traffic // only if the traffic matches at least one port in the ports list. withNormalisedProtoAndPortLegacy(ingressRule.Ports, func(proto, port string) { - rule := newRuleSpec(ingress, &proto, nil, dstSelector, &port) + rule := newRuleSpec(ingressPolicy, &proto, nil, dstSelector, &port) rules[rule.key] = rule }) } @@ -78,13 +78,13 @@ func (ns *ns) analysePolicyLegacy(policy *extnapi.NetworkPolicy) ( if ingressRule.Ports == nil { // Ports is not provided, this rule matches all ports (traffic not restricted by port). - rule := newRuleSpec(ingress, nil, srcSelector, dstSelector, nil) + rule := newRuleSpec(ingressPolicy, nil, srcSelector, dstSelector, nil) rules[rule.key] = rule } else { // Ports is present and contains at least one item, then this rule allows traffic // only if the traffic matches at least one port in the ports list. withNormalisedProtoAndPortLegacy(ingressRule.Ports, func(proto, port string) { - rule := newRuleSpec(ingress, &proto, srcSelector, dstSelector, &port) + rule := newRuleSpec(ingressPolicy, &proto, srcSelector, dstSelector, &port) rules[rule.key] = rule }) } @@ -138,11 +138,11 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) ( if allSources { if allPorts { - rule := newRuleSpec(ingress, nil, nil, dstSelector, nil) + rule := newRuleSpec(ingressPolicy, nil, nil, dstSelector, nil) rules[rule.key] = rule } else { withNormalisedProtoAndPort(ingressRule.Ports, func(proto, port string) { - rule := newRuleSpec(ingress, &proto, nil, dstSelector, &port) + rule := newRuleSpec(ingressPolicy, &proto, nil, dstSelector, &port) rules[rule.key] = rule }) } @@ -168,11 +168,11 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) ( } if allPorts { - rule := newRuleSpec(ingress, nil, srcSelector, dstSelector, nil) + rule := newRuleSpec(ingressPolicy, nil, srcSelector, dstSelector, nil) rules[rule.key] = rule } else { withNormalisedProtoAndPort(ingressRule.Ports, func(proto, port string) { - rule := newRuleSpec(ingress, &proto, srcSelector, dstSelector, &port) + rule := newRuleSpec(ingressPolicy, &proto, srcSelector, dstSelector, &port) rules[rule.key] = rule }) } diff --git a/npc/rule.go b/npc/rule.go index f84884da..b34ac11a 100644 --- a/npc/rule.go +++ b/npc/rule.go @@ -10,20 +10,13 @@ import ( "github.com/weaveworks/weave/npc/iptables" ) -type ruleType byte - -const ( - ingress ruleType = iota - egress -) - type ruleSpec struct { - key string - args []string - ruleType ruleType + key string + args []string + policyType policyType } -func newRuleSpec(ruleType ruleType, proto *string, srcHost *selectorSpec, dstHost *selectorSpec, dstPort *string) *ruleSpec { +func newRuleSpec(policyType policyType, proto *string, srcHost *selectorSpec, dstHost *selectorSpec, dstPort *string) *ruleSpec { args := []string{} if proto != nil { args = append(args, "-p", *proto) @@ -47,9 +40,10 @@ func newRuleSpec(ruleType ruleType, proto *string, srcHost *selectorSpec, dstHos } args = append(args, "-j", "ACCEPT") args = append(args, "-m", "comment", "--comment", fmt.Sprintf("%s -> %s", srcComment, dstComment)) + // TODO(brb) embed policyType into key key := strings.Join(args, " ") - return &ruleSpec{key, args, ruleType} + return &ruleSpec{key, args, policyType} } type ruleSet struct { @@ -66,10 +60,7 @@ func (rs *ruleSet) deprovision(user types.UID, current, desired map[string]*rule if _, found := desired[key]; !found { delete(rs.users[key], user) if len(rs.users[key]) == 0 { - chain := IngressChain - if spec.ruleType == egress { - chain = EgressCustomChain - } + chain := rulesChainByPolicyType(spec.policyType) common.Log.Infof("deleting rule %v from %q chain", spec.args, chain) if err := rs.ipt.Delete(TableFilter, chain, spec.args...); err != nil { return err @@ -86,10 +77,7 @@ func (rs *ruleSet) provision(user types.UID, current, desired map[string]*ruleSp for key, spec := range desired { if _, found := current[key]; !found { if _, found := rs.users[key]; !found { - chain := IngressChain - if spec.ruleType == egress { - chain = EgressCustomChain - } + chain := rulesChainByPolicyType(spec.policyType) common.Log.Infof("adding rule %v to %q chain", spec.args, chain) if err := rs.ipt.Append(TableFilter, chain, spec.args...); err != nil { return err @@ -102,3 +90,10 @@ func (rs *ruleSet) provision(user types.UID, current, desired map[string]*ruleSp return nil } + +func rulesChainByPolicyType(policyType policyType) string { + if policyType == egressPolicy { + return EgressCustomChain + } + return IngressChain +} diff --git a/npc/rule_test.go b/npc/rule_test.go index ab5850de..5fac10c7 100644 --- a/npc/rule_test.go +++ b/npc/rule_test.go @@ -9,7 +9,7 @@ func TestRegressionHandleNil3095(t *testing.T) { // Test for handling nil values // https://github.com/weaveworks/weave/issues/3095 - rs := newRuleSpec(ingress, nil, nil, nil, nil) + rs := newRuleSpec(ingressPolicy, nil, nil, nil, nil) require.NotEqual(t, rs.key, "")