Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validate labels in ANP/ACNP/CG #3331

Merged
merged 1 commit into from Feb 24, 2022
Merged

Validate labels in ANP/ACNP/CG #3331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 35 additions & 0 deletions pkg/controller/networkpolicy/validate.go
View file
Open in desktop
Expand Up @@ -27,6 +27,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apiserver/pkg/authentication/serviceaccount"
"k8s.io/klog/v2"

Expand Down Expand Up @@ -472,6 +473,9 @@ func (v *antreaPolicyValidator) validateAppliedTo(ingress, egress []crdv1alpha1.
if eachAppliedTo.ServiceAccount != nil && appliedToFieldsNum > 1 {
return "serviceAccount cannot be set with other peers in appliedTo", false
}
if reason, allowed := checkSelectorsLabels(eachAppliedTo.PodSelector, eachAppliedTo.NamespaceSelector, eachAppliedTo.ExternalEntitySelector); !allowed {
return reason, allowed
}
}
return "", true
}
Expand Down Expand Up @@ -511,6 +515,9 @@ func (v *antreaPolicyValidator) validatePeers(ingress, egress []crdv1alpha1.Rule
if peer.ServiceAccount != nil && peerFieldsNum > 1 {
return "serviceAccount cannot be set with other peers in rules", false
}
if reason, allowed := checkSelectorsLabels(peer.PodSelector, peer.NamespaceSelector, peer.ExternalEntitySelector); !allowed {
return reason, allowed
}
}
return "", true
}
Expand Down Expand Up @@ -549,6 +556,31 @@ func numFieldsSetInPeer(peer crdv1alpha1.NetworkPolicyPeer) int {
return num
}

// checkSelectorsLabels validates labels used in all selectors passed in.
func checkSelectorsLabels(selectors ...*metav1.LabelSelector) (string, bool) {
validateLabels := func(labels map[string]string) (string, bool) {
for k, v := range labels {
err := validation.IsQualifiedName(k)
if err != nil {
return fmt.Sprintf("Invalid label key: %s: %s", k, strings.Join(err, "; ")), false
}
err = validation.IsValidLabelValue(v)
if err != nil {
return fmt.Sprintf("Invalid label value: %s: %s", v, strings.Join(err, "; ")), false
}
}
return "", true
}
for _, selector := range selectors {
if selector != nil {
if reason, allowed := validateLabels(selector.MatchLabels); !allowed {
return reason, allowed
}
}
}
return "", true
}

// validateTierForPolicy validates whether a referenced Tier exists.
func (v *antreaPolicyValidator) validateTierForPolicy(tier string) (string, bool) {
// "tier" must exist before referencing
Expand Down Expand Up @@ -706,6 +738,9 @@ func validateAntreaGroupSpec(s crdv1alpha2.GroupSpec) (string, bool) {
}
selector, serviceRef, ipBlock, ipBlocks, childGroups := 0, 0, 0, 0, 0
if s.NamespaceSelector != nil || s.ExternalEntitySelector != nil || s.PodSelector != nil {
if reason, allowed := checkSelectorsLabels(s.PodSelector, s.NamespaceSelector, s.ExternalEntitySelector); !allowed {
return reason, allowed
}
selector = 1
}
if s.IPBlock != nil {
Expand Down
129 changes: 129 additions & 0 deletions pkg/controller/networkpolicy/validate_test.go
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1"
crdv1alpha2 "antrea.io/antrea/pkg/apis/crd/v1alpha2"
)

func TestValidateAntreaPolicy(t *testing.T) {
Expand Down Expand Up @@ -940,6 +941,84 @@ func TestValidateAntreaPolicy(t *testing.T) {
},
expectedReason: "",
},
{
name: "acnp-invalid-label-key-applied-to",
policy: &crdv1alpha1.ClusterNetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "acnp-invalid-label-key-applied-to",
},
Spec: crdv1alpha1.ClusterNetworkPolicySpec{
AppliedTo: []crdv1alpha1.NetworkPolicyPeer{
{
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"foo=": "bar"},
},
},
},
},
},
expectedReason: "Invalid label key: foo=: name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')",
},
{
name: "acnp-invalid-label-value-applied-to",
policy: &crdv1alpha1.ClusterNetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "acnp-invalid-label-value-applied-to",
},
Spec: crdv1alpha1.ClusterNetworkPolicySpec{
Ingress: []crdv1alpha1.Rule{
{
Action: &allowAction,
From: []crdv1alpha1.NetworkPolicyPeer{
{
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"foo": "bar"},
},
},
},
AppliedTo: []crdv1alpha1.NetworkPolicyPeer{
{
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"foo1": "bar="},
},
},
},
},
},
},
},
expectedReason: "Invalid label value: bar=: a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')",
},
{
name: "acnp-invalid-label-key-rule",
policy: &crdv1alpha1.ClusterNetworkPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "acnp-invalid-label-key-rule",
},
Spec: crdv1alpha1.ClusterNetworkPolicySpec{
AppliedTo: []crdv1alpha1.NetworkPolicyPeer{
{
NamespaceSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"foo": "bar"},
},
},
},
Ingress: []crdv1alpha1.Rule{
{
Action: &allowAction,
From: []crdv1alpha1.NetworkPolicyPeer{
{
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"foo=": "bar1"},
},
},
},
},
},
},
},
expectedReason: "Invalid label key: foo=: name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -955,3 +1034,53 @@ func TestValidateAntreaPolicy(t *testing.T) {
})
}
}

func TestValidateClusterAntreaGroup(t *testing.T) {
tests := []struct {
name string
group *crdv1alpha2.ClusterGroup
expectedReason string
}{
{
name: "cg-invalid-label-key",
group: &crdv1alpha2.ClusterGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "cg-invalid-label-key",
},
Spec: crdv1alpha2.GroupSpec{
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"foo=": "bar"},
},
},
},
expectedReason: "Invalid label key: foo=: name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')",
},
{
name: "cg-invalid-label-value",
group: &crdv1alpha2.ClusterGroup{
ObjectMeta: metav1.ObjectMeta{
Name: "cg-invalid-label-value",
},
Spec: crdv1alpha2.GroupSpec{
PodSelector: &metav1.LabelSelector{
MatchLabels: map[string]string{"foo": "bar="},
},
},
},
expectedReason: "Invalid label value: bar=: a valid label must be an empty string or consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyValue', or 'my_value', or '12345', regex used for validation is '(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?')",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
_, c := newController()
v := NewNetworkPolicyValidator(c.NetworkPolicyController)
actualReason, allowed := v.validateAntreaGroup(tt.group, nil, admv1.Create, authenticationv1.UserInfo{})
assert.Equal(t, tt.expectedReason, actualReason)
if tt.expectedReason == "" {
assert.True(t, allowed)
} else {
assert.False(t, allowed)
}
})
}
}