Skip to content

Commit

Permalink
Change trans SA position
Browse files Browse the repository at this point in the history
Signed-off-by: wgrayson <wgrayson@vmware.com>
  • Loading branch information
GraysonWu committed Jan 7, 2022
1 parent 0f46cc4 commit 01782b4
Show file tree
Hide file tree
Showing 11 changed files with 385 additions and 310 deletions.
18 changes: 14 additions & 4 deletions pkg/apis/crd/v1alpha1/types.go
Expand Up @@ -434,7 +434,10 @@ type NetworkPolicyPeer struct {
// Exact FQDNs, i.e. "google.com", "db-svc.default.svc.cluster.local"
// Wildcard expressions, i.e. "*wayfair.com".
FQDN string `json:"fqdn,omitempty"`
// ...
// Select all Pods with the ServiceAccount matched by this field, as
// workloads in AppliedTo/To/From fields.
// Cannot be set with any other selector.
// +optional
ServiceAccounts []ServiceAccount `json:"serviceAccounts,omitempty"`
}

Expand Down Expand Up @@ -595,8 +598,15 @@ type TierList struct {
}

type ServiceAccount struct {
Name string `json:"name,omitempty"`
Namespace string `json:"namespace,omitempty"`
LabelSelector *metav1.LabelSelector `json:"labelSelector,omitempty"`
// Name and Namespace must be used together to select a specific ServiceAccount.
// Name and Namespace cannot be set with any other fields.
Name string `json:"name,omitempty"`
Namespace string `json:"namespace,omitempty"`
// Select all ServiceAccounts matched by this selector. If set with
// NamespaceSelector, ServiceAccounts are matched from Namespaces matched by the
// NamespaceSelector.
// Cannot be set with any other fields except NamespaceSelector.
LabelSelector *metav1.LabelSelector `json:"labelSelector,omitempty"`
// Can be only used with LabelSelector.
NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"`
}
3 changes: 2 additions & 1 deletion pkg/controller/grouping/custom_label.go
Expand Up @@ -34,7 +34,8 @@ func getServiceAccountLabel(entityType entityType, obj metav1.Object) (string, s
return "", ""
}

// getAllLabelsKey returns a key string of all labels of a labelItem.
// getAllLabelsKey returns a key string of all labels of a labelItem including
// custom labels.
func getAllLabelsKey(entityType entityType, obj metav1.Object) string {
labelsKey := labels.Set(obj.GetLabels()).String()
if key, value := getServiceAccountLabel(entityType, obj); key != "" {
Expand Down
16 changes: 12 additions & 4 deletions pkg/controller/grouping/group_entity_index.go
Expand Up @@ -359,14 +359,21 @@ func (i *GroupEntityIndex) deleteEntityFromLabelItem(label, entity string) *labe
// createLabelItem creates a labelItem based on the provided entityItem.
// It's called when there is no existing labelItem for a label set.
func (i *GroupEntityIndex) createLabelItem(entityType entityType, eItem *entityItem) *labelItem {
// In addCustomLabels func, we write custom labels into labelItem.labels. To avoid
// change the original labels and data race, use copy of
// eItem.entity.GetLabels() in labelItem.labels instead of the pointer.
entityLabelsCopy := map[string]string{}
for k, v := range eItem.entity.GetLabels() {
entityLabelsCopy[k] = v
}
lItem := &labelItem{
labels: eItem.entity.GetLabels(),
labels: entityLabelsCopy,
namespace: eItem.entity.GetNamespace(),
entityType: entityType,
entityItemKeys: sets.NewString(),
selectorItemKeys: sets.NewString(),
}
addCustomLabel(lItem, entityType, eItem.entity)
addCustomLabels(lItem, entityType, eItem.entity)
// Create the labelItem.
i.labelItems[eItem.labelItemKey] = lItem
// Add it to the labelItemIndex.
Expand Down Expand Up @@ -778,8 +785,9 @@ func getSelectorItemKey(selector *types.GroupSelector) string {
return selector.NormalizedName
}

// addCustomLabel ...
func addCustomLabel(labelItem *labelItem, entityType entityType, obj metav1.Object) {
// addCustomLabels adds custom labels to labelItem.labels.
// Custom labels include: ServiceAccountLabel.
func addCustomLabels(labelItem *labelItem, entityType entityType, obj metav1.Object) {
if key, value := getServiceAccountLabel(entityType, obj); key != "" {
if labelItem.labels == nil {
labelItem.labels = make(map[string]string)
Expand Down
250 changes: 115 additions & 135 deletions pkg/controller/networkpolicy/clusternetworkpolicy.go

Large diffs are not rendered by default.

98 changes: 89 additions & 9 deletions pkg/controller/networkpolicy/clusternetworkpolicy_test.go
Expand Up @@ -41,7 +41,7 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {
nsA := v1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "nsA",
Labels: map[string]string{"foo1": "bar1"},
Labels: map[string]string{"foo1": "bar1", "kubernetes.io/metadata.name": "nsA"},
},
}
nsB := v1.Namespace{
Expand Down Expand Up @@ -86,6 +86,7 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {

labelSelectorA, _ := metav1.LabelSelectorAsSelector(&selectorA)
labelSelectorB, _ := metav1.LabelSelectorAsSelector(&selectorB)
labelSelectorE, _ := metav1.LabelSelectorAsSelector(&selectorE)
cgA := crdv1alpha3.ClusterGroup{
ObjectMeta: metav1.ObjectMeta{Name: "cgA", UID: "uidA"},
Spec: crdv1alpha3.GroupSpec{
Expand Down Expand Up @@ -1155,7 +1156,7 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {
},
},
{
PodSelector: &selectorA,
PodSelector: &selectorA,
},
},
Priority: p10,
Expand Down Expand Up @@ -1209,7 +1210,7 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {
Spec: crdv1alpha1.ClusterNetworkPolicySpec{
AppliedTo: []crdv1alpha1.NetworkPolicyPeer{
{
PodSelector: &selectorA,
PodSelector: &selectorA,
},
},
Priority: p10,
Expand All @@ -1219,7 +1220,7 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {
{
ServiceAccounts: []crdv1alpha1.ServiceAccount{
{
Name: saA.Name,
Name: saA.Name,
Namespace: saA.Namespace,
},
},
Expand Down Expand Up @@ -1265,15 +1266,15 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {
{
To: []crdv1alpha1.NetworkPolicyPeer{
{
PodSelector: &selectorA,
PodSelector: &selectorA,
},
},
Action: &dropAction,
AppliedTo: []crdv1alpha1.NetworkPolicyPeer{
{
ServiceAccounts: []crdv1alpha1.ServiceAccount{
{
Name: saA.Name,
Name: saA.Name,
Namespace: saA.Namespace,
},
},
Expand All @@ -1299,17 +1300,95 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {
To: controlplane.NetworkPolicyPeer{
AddressGroups: []string{getNormalizedUID(toGroupSelector("", &selectorA, nil, nil).NormalizedName)},
},
Priority: 0,
Action: &dropAction,
Priority: 0,
Action: &dropAction,
AppliedToGroups: []string{getNormalizedUID(toGroupSelector("", &selectorD, &selectorE, nil).NormalizedName)},
},
},
AppliedToGroups: []string{getNormalizedUID(toGroupSelector("", &selectorD, &selectorE, nil).NormalizedName)},
AppliedToGroups: []string{getNormalizedUID(toGroupSelector("", &selectorD, &selectorE, nil).NormalizedName)},
AppliedToPerRule: true,
},
expectedAppliedToGroups: 1,
expectedAddressGroups: 1,
},
{
name: "service-account-per-namespace-rule",
inputPolicy: &crdv1alpha1.ClusterNetworkPolicy{
ObjectMeta: metav1.ObjectMeta{Name: "cnpR", UID: "uidR"},
Spec: crdv1alpha1.ClusterNetworkPolicySpec{
AppliedTo: []crdv1alpha1.NetworkPolicyPeer{
{
ServiceAccounts: []crdv1alpha1.ServiceAccount{
{
Name: saA.Name,
Namespace: saA.Namespace,
},
},
},
},
Priority: p10,
Egress: []crdv1alpha1.Rule{
{
To: []crdv1alpha1.NetworkPolicyPeer{
{
Namespaces: &crdv1alpha1.PeerNamespaces{
Match: crdv1alpha1.NamespaceMatchSelf,
},
},
},
Action: &dropAction,
},
{
To: []crdv1alpha1.NetworkPolicyPeer{
{
NamespaceSelector: &selectorB,
},
},
Action: &allowAction,
},
},
},
},
expectedPolicy: &antreatypes.NetworkPolicy{
UID: "uidR",
Name: "uidR",
SourceRef: &controlplane.NetworkPolicyReference{
Type: controlplane.AntreaClusterNetworkPolicy,
Name: "cnpR",
UID: "uidR",
},
Priority: &p10,
TierPriority: &DefaultTierPriority,
Rules: []controlplane.NetworkPolicyRule{
{
Direction: controlplane.DirectionOut,
To: controlplane.NetworkPolicyPeer{
AddressGroups: []string{getNormalizedUID(toGroupSelector("nsA", nil, nil, nil).NormalizedName)},
},
Priority: 0,
Action: &dropAction,
AppliedToGroups: []string{getNormalizedUID(toGroupSelector("nsA", &selectorD, nil, nil).NormalizedName)},
},
{
Direction: controlplane.DirectionOut,
To: controlplane.NetworkPolicyPeer{
AddressGroups: []string{getNormalizedUID(toGroupSelector("", nil, &selectorB, nil).NormalizedName)},
},
Priority: 1,
Action: &allowAction,
AppliedToGroups: []string{getNormalizedUID(toGroupSelector("", &selectorD, &selectorE, nil).NormalizedName)},
},
},
AppliedToGroups: []string{
getNormalizedUID(toGroupSelector("nsA", &selectorD, nil, nil).NormalizedName),
getNormalizedUID(toGroupSelector("", &selectorD, &selectorE, nil).NormalizedName),
},
AppliedToPerRule: true,
PerNamespaceSelectors: []labels.Selector{labelSelectorE},
},
expectedAppliedToGroups: 2,
expectedAddressGroups: 2,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -1324,6 +1403,7 @@ func TestProcessClusterNetworkPolicy(t *testing.T) {
c.tierStore.Add(&tierA)
}
actualPolicy := c.processClusterNetworkPolicy(tt.inputPolicy)
t.Logf("JB: %v", actualPolicy)
assert.Equal(t, tt.expectedPolicy.UID, actualPolicy.UID)
assert.Equal(t, tt.expectedPolicy.Name, actualPolicy.Name)
assert.Equal(t, tt.expectedPolicy.SourceRef, actualPolicy.SourceRef)
Expand Down
6 changes: 6 additions & 0 deletions pkg/controller/networkpolicy/crd_utils.go
Expand Up @@ -114,6 +114,12 @@ func (n *NetworkPolicyController) toAntreaPeerForCRD(peers []v1alpha1.NetworkPol
}
} else if peer.FQDN != "" {
fqdns = append(fqdns, peer.FQDN)
} else if len(peer.ServiceAccounts) > 0 {
newPeers := n.transServiceAccounts(peer.ServiceAccounts)
for _, newPeer := range newPeers {
normalizedUID := n.createAddressGroup(np.GetNamespace(), newPeer.PodSelector, newPeer.NamespaceSelector, nil)
addressGroups = append(addressGroups, normalizedUID)
}
} else {
normalizedUID := n.createAddressGroup(np.GetNamespace(), peer.PodSelector, peer.NamespaceSelector, peer.ExternalEntitySelector)
addressGroups = append(addressGroups, normalizedUID)
Expand Down
13 changes: 8 additions & 5 deletions pkg/controller/networkpolicy/networkpolicy_controller.go
Expand Up @@ -299,9 +299,15 @@ var cnpIndexers = cache.Indexers{
if peerHasServiceAccounts(ingressRule.From) {
return []string{HasServiceAccountsPeer}, nil
}
if peerHasServiceAccounts(ingressRule.AppliedTo) {
return []string{HasServiceAccountsPeer}, nil
}
}
for _, egressRule := range cnp.Spec.Egress {
if peerHasServiceAccounts(egressRule.From) {
if peerHasServiceAccounts(egressRule.To) {
return []string{HasServiceAccountsPeer}, nil
}
if peerHasServiceAccounts(egressRule.AppliedTo) {
return []string{HasServiceAccountsPeer}, nil
}
}
Expand Down Expand Up @@ -479,10 +485,7 @@ func (n *NetworkPolicyController) GetConnectedAgentNum() int {
func toGroupSelector(namespace string, podSelector, nsSelector, extEntitySelector *metav1.LabelSelector) *antreatypes.GroupSelector {
groupSelector := antreatypes.GroupSelector{}
if podSelector != nil {
pSelector, err := metav1.LabelSelectorAsSelector(podSelector)
if err != nil {
klog.Info(err.Error())
}
pSelector, _ := metav1.LabelSelectorAsSelector(podSelector)
groupSelector.PodSelector = pSelector
}
if extEntitySelector != nil {
Expand Down
8 changes: 7 additions & 1 deletion pkg/controller/networkpolicy/validate.go
Expand Up @@ -540,12 +540,18 @@ func numFieldsSetInPeer(peer crdv1alpha1.NetworkPolicyPeer) int {
return num
}

// validateServiceAccounts returns ...
// validateServiceAccounts validates if all ServiceAccounts:
// 1. Use Name and Namespace at the same time.
// 2. Don't use Name and LabelSelector at the same time.
// 3. Only use NamespaceSelector when LabelSelector is set.
func validateServiceAccounts(serviceAccounts []crdv1alpha1.ServiceAccount) (string, bool) {
for _, sa := range serviceAccounts {
if (sa.Name == "") != (sa.Namespace == "") {
return "inside serviceAccounts name and namespace must be used together", false
}
if sa.Name != "" && sa.LabelSelector != nil {
return "inside serviceAccounts name and labelSelector can't be used together", false
}
if sa.LabelSelector == nil && sa.NamespaceSelector != nil {
return "inside serviceAccounts namespaceSelector can't be used without labelSelector", false
}
Expand Down

0 comments on commit 01782b4

Please sign in to comment.