Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Dyanngg
Dyanngg committed Mar 24, 2021
1 parent 127f790 commit abb039b
Show file tree
Hide file tree
Showing 2 changed files with 148 additions and 33 deletions.
68 changes: 38 additions & 30 deletions pkg/controller/networkpolicy/clusternetworkpolicy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
secv1alpha1 "github.com/vmware-tanzu/antrea/pkg/apis/security/v1alpha1"
"github.com/vmware-tanzu/antrea/pkg/controller/networkpolicy/store"
antreatypes "github.com/vmware-tanzu/antrea/pkg/controller/types"
utilsets "github.com/vmware-tanzu/antrea/pkg/util/sets"
)

// addCNP receives ClusterNetworkPolicy ADD events and creates resources
Expand Down Expand Up @@ -135,9 +136,7 @@ func (n *NetworkPolicyController) filterPerNamespaceRuleACNPsByNSLabels(nsLabels
}
for _, np := range nps {
internalNP := np.(*antreatypes.NetworkPolicy)
//klog.Infof("NP %v has perNSSel", internalNP.SourceRef.Name)
for _, sel := range internalNP.PerNamespaceSelectors {
//klog.Infof("Evaluating selector %v", sel)
if sel.Matches(nsLabels) {
affectedPolicies.Insert(internalNP.SourceRef.Name)
break
Expand Down Expand Up @@ -171,21 +170,10 @@ func (n *NetworkPolicyController) updateNamespace(oldObj, curObj interface{}) {
oldNamespace, curNamespace := oldObj.(*v1.Namespace), curObj.(*v1.Namespace)
klog.V(2).Infof("Processing Namespace %s UPDATE event, labels: %v", curNamespace.Name, curNamespace.Labels)
oldLabelSet, curLabelSet := labels.Set(oldNamespace.Labels), labels.Set(curNamespace.Labels)
addedLabels, removedLabels := labels.Set{}, labels.Set{}
for k, v := range oldLabelSet {
if !curLabelSet.Has(k) || curLabelSet.Get(k) != v {
removedLabels[k] = v
}
}
for k, v := range curLabelSet {
if !oldLabelSet.Has(k) || oldLabelSet.Get(k) != v {
addedLabels[k] = v
}
}
affectedACNPsByLabelRemoval := n.filterPerNamespaceRuleACNPsByNSLabels(removedLabels)
affectedACNPsByLabelAdd := n.filterPerNamespaceRuleACNPsByNSLabels(addedLabels)
policiesToSync := affectedACNPsByLabelAdd.Union(affectedACNPsByLabelRemoval)
for _, cnpName := range policiesToSync.List() {
affectedACNPsByOldLabels := n.filterPerNamespaceRuleACNPsByNSLabels(oldLabelSet)
affectedACNPsByCurLabels := n.filterPerNamespaceRuleACNPsByNSLabels(curLabelSet)
affectedACNPs := utilsets.SymmetricDifference(affectedACNPsByOldLabels, affectedACNPsByCurLabels)
for _, cnpName := range affectedACNPs.List() {
cnp, err := n.cnpLister.Get(cnpName)
if err != nil {
klog.Errorf("Error getting Antrea ClusterNetworkPolicy %s", cnpName)
Expand Down Expand Up @@ -248,6 +236,22 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *secv1alpha1.C
// to re-calculate affected Namespaces.
var affectedNamespaceSelectors []labels.Selector

// If appliedTo is set at spec level and the ACNP has per-namespace rules, then each appliedTo needs
// to be split into appliedToGroups for each of its affected Namespace.
var clusterAppliedToAffectedNS []string
var atgForNamespace []string
if hasPerNamespaceRule && len(cnp.Spec.AppliedTo) > 0 {
for _, at := range cnp.Spec.AppliedTo {
affectedNS, selectors := n.getAffectedNamespacesForAppliedTo(at)
affectedNamespaceSelectors = append(affectedNamespaceSelectors, selectors...)
for _, ns := range affectedNS {
atg := n.createAppliedToGroup(ns, at.PodSelector, nil, at.ExternalEntitySelector)
atgNamesSet.Insert(atg)
clusterAppliedToAffectedNS = append(clusterAppliedToAffectedNS, ns)
atgForNamespace = append(atgForNamespace, atg)
}
}
}
var rules []controlplane.NetworkPolicyRule
processRules := func(cnpRules []secv1alpha1.Rule, direction controlplane.Direction) {
for idx, cnpRule := range cnpRules {
Expand Down Expand Up @@ -282,18 +286,23 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *secv1alpha1.C
addRule(n.toAntreaPeerForCRD(clusterPeers, cnp, direction, namedPortExists), direction, ruleATGNames)
}
if len(perNSPeers) > 0 {
ruleAppliedTos := cnp.Spec.AppliedTo
if len(cnpRule.AppliedTo) > 0 {
ruleAppliedTos = cnpRule.AppliedTo
}
for _, at := range ruleAppliedTos {
affectedNS, selectors := n.getAffectedNamespacesForAppliedTo(at)
affectedNamespaceSelectors = append(affectedNamespaceSelectors, selectors...)
for _, ns := range affectedNS {
atg := n.createAppliedToGroup(ns, at.PodSelector, nil, at.ExternalEntitySelector)
atgNamesSet.Insert(atg)
klog.V(4).Infof("Adding a new per-namespace rule with appliedTo %v for %s", atg, cnp.Name)
addRule(n.toNamespacedPeerForCRD(perNSPeers, ns), direction, []string{atg})
if len(cnp.Spec.AppliedTo) > 0 {
// Create a rule for each affected Namespace of appliedTo at spec level
for i := range clusterAppliedToAffectedNS {
klog.V(4).Infof("Adding a new per-namespace rule with appliedTo %v for rule %d of %s", clusterAppliedToAffectedNS[i], idx, cnp.Name)
addRule(n.toNamespacedPeerForCRD(perNSPeers, clusterAppliedToAffectedNS[i]), direction, []string{atgForNamespace[i]})
}
} else {
// Create a rule for each affected Namespace of appliedTo at rule level
for _, at := range cnpRule.AppliedTo {
affectedNS, selectors := n.getAffectedNamespacesForAppliedTo(at)
affectedNamespaceSelectors = append(affectedNamespaceSelectors, selectors...)
for _, ns := range affectedNS {
atg := n.createAppliedToGroup(ns, at.PodSelector, nil, at.ExternalEntitySelector)
atgNamesSet.Insert(atg)
klog.V(4).Infof("Adding a new per-namespace rule with appliedTo %v for rule %d of %s", atg, idx, cnp.Name)
addRule(n.toNamespacedPeerForCRD(perNSPeers, ns), direction, []string{atg})
}
}
}
}
Expand All @@ -308,7 +317,6 @@ func (n *NetworkPolicyController) processClusterNetworkPolicy(cnp *secv1alpha1.C
n.processClusterAppliedTo(cnp.Spec.AppliedTo, atgNamesSet)
}
tierPriority := n.getTierPriority(cnp.Spec.Tier)
klog.Infof("Before uniqueness compute, selectors are %v", affectedNamespaceSelectors)
internalNetworkPolicy := &antreatypes.NetworkPolicy{
Name: internalNetworkPolicyKeyFunc(cnp),
Generation: cnp.Generation,
Expand Down
113 changes: 110 additions & 3 deletions pkg/controller/networkpolicy/clusternetworkpolicy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,13 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {
}

allowAction := secv1alpha1.RuleActionAllow
dropAction := secv1alpha1.RuleActionDrop
protocolTCP := controlplane.ProtocolTCP
selectorA := metav1.LabelSelector{MatchLabels: map[string]string{"foo1": "bar1"}}
selectorB := metav1.LabelSelector{MatchLabels: map[string]string{"foo2": "bar2"}}
selectorC := metav1.LabelSelector{MatchLabels: map[string]string{"foo3": "bar3"}}
labelSelectorA, _ := metav1.LabelSelectorAsSelector(&selectorA)
labelSelectorB, _ := metav1.LabelSelectorAsSelector(&selectorB)
cgA := corev1a2.ClusterGroup{
ObjectMeta: metav1.ObjectMeta{Name: "cgA", UID: "uidA"},
Spec: corev1a2.GroupSpec{
Expand Down Expand Up @@ -769,15 +772,119 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {
},
AppliedToGroups: []string{
getNormalizedUID(toGroupSelector("nsA", nil, nil, nil).NormalizedName),
getNormalizedUID(toGroupSelector("", nil, &metav1.LabelSelector{}, nil).NormalizedName),
getNormalizedUID(toGroupSelector("nsB", nil, nil, nil).NormalizedName),
getNormalizedUID(toGroupSelector("", nil, &metav1.LabelSelector{}, nil).NormalizedName),
},
AppliedToPerRule: true,
PerNamespaceSelectors: []labels.Selector{labels.Everything()},
},
expectedAppliedToGroups: 3,
expectedAddressGroups: 3,
},
{
name: "with-per-namespace-rule-applied-to-per-rule",
inputPolicy: &secv1alpha1.ClusterNetworkPolicy{
ObjectMeta: metav1.ObjectMeta{Name: "cnpJ", UID: "uidJ"},
Spec: secv1alpha1.ClusterNetworkPolicySpec{
Priority: p10,
Ingress: []secv1alpha1.Rule{
{
AppliedTo: []secv1alpha1.NetworkPolicyPeer{
{
NamespaceSelector: &selectorA,
PodSelector: &selectorA,
},
},
Ports: []secv1alpha1.NetworkPolicyPort{
{
Port: &int80,
},
},
From: []secv1alpha1.NetworkPolicyPeer{
{
Namespaces: &secv1alpha1.PeerNamespaces{
Self: true,
},
PodSelector: &selectorA,
},
},
Action: &dropAction,
},
{
AppliedTo: []secv1alpha1.NetworkPolicyPeer{
{
NamespaceSelector: &selectorB,
},
},
Ports: []secv1alpha1.NetworkPolicyPort{
{
Port: &int81,
},
},
From: []secv1alpha1.NetworkPolicyPeer{
{
Namespaces: &secv1alpha1.PeerNamespaces{
Self: true,
},
},
},
Action: &dropAction,
},
},
},
},
expectedPolicy: &antreatypes.NetworkPolicy{
UID: "uidJ",
Name: "uidJ",
SourceRef: &controlplane.NetworkPolicyReference{
Type: controlplane.AntreaClusterNetworkPolicy,
Name: "cnpJ",
UID: "uidJ",
},
Priority: &p10,
TierPriority: &DefaultTierPriority,
Rules: []controlplane.NetworkPolicyRule{
{
Direction: controlplane.DirectionIn,
AppliedToGroups: []string{getNormalizedUID(toGroupSelector("nsA", &selectorA, nil, nil).NormalizedName)},
From: controlplane.NetworkPolicyPeer{
AddressGroups: []string{getNormalizedUID(toGroupSelector("nsA", &selectorA, nil, nil).NormalizedName)},
},
Services: []controlplane.Service{
{
Protocol: &protocolTCP,
Port: &int80,
},
},
Priority: 0,
Action: &dropAction,
},
{
Direction: controlplane.DirectionIn,
AppliedToGroups: []string{getNormalizedUID(toGroupSelector("nsB", nil, nil, nil).NormalizedName)},
From: controlplane.NetworkPolicyPeer{
AddressGroups: []string{getNormalizedUID(toGroupSelector("nsB", nil, nil, nil).NormalizedName)},
},
Services: []controlplane.Service{
{
Protocol: &protocolTCP,
Port: &int81,
},
},
Priority: 1,
Action: &dropAction,
},
},
AppliedToGroups: []string{
getNormalizedUID(toGroupSelector("nsA", &selectorA, nil, nil).NormalizedName),
getNormalizedUID(toGroupSelector("nsB", nil, nil, nil).NormalizedName),
},
AppliedToPerRule: true,
PerNamespaceSelectors: []labels.Selector{labelSelectorA, labelSelectorB},
},
expectedAppliedToGroups: 2,
expectedAddressGroups: 2,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -794,10 +901,10 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {
assert.Equal(t, tt.expectedPolicy.Name, actualPolicy.Name)
assert.Equal(t, tt.expectedPolicy.SourceRef, actualPolicy.SourceRef)
assert.Equal(t, tt.expectedPolicy.Priority, actualPolicy.Priority)
assert.Equal(t, tt.expectedPolicy.Rules, actualPolicy.Rules)
assert.Equal(t, tt.expectedPolicy.TierPriority, actualPolicy.TierPriority)
assert.Equal(t, tt.expectedPolicy.AppliedToPerRule, actualPolicy.AppliedToPerRule)
assert.Equal(t, tt.expectedPolicy.PerNamespaceSelectors, actualPolicy.PerNamespaceSelectors)
assert.ElementsMatch(t, tt.expectedPolicy.Rules, actualPolicy.Rules)
assert.ElementsMatch(t, tt.expectedPolicy.PerNamespaceSelectors, actualPolicy.PerNamespaceSelectors)
assert.ElementsMatch(t, tt.expectedPolicy.AppliedToGroups, actualPolicy.AppliedToGroups)
assert.Equal(t, tt.expectedAppliedToGroups, len(c.appliedToGroupStore.List()))
})
Expand Down

0 comments on commit abb039b

Please sign in to comment.