From 83ac613b89cea1ca45f74316a8752b6f0e4e2493 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 11 Jan 2017 17:17:01 -0500 Subject: [PATCH 1/2] Fix up existing NetworkPolicy validation Paths were wrong for most errors. Field name was wrong for namespaceSelector. --- pkg/apis/extensions/validation/validation.go | 25 ++++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index d6762d6d346..0dd521a0c17 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -828,26 +828,25 @@ func ValidateNetworkPolicySpec(spec *extensions.NetworkPolicySpec, fldPath *fiel allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(&spec.PodSelector, fldPath.Child("podSelector"))...) // Validate ingress rules. - for _, i := range spec.Ingress { + for i, ingress := range spec.Ingress { + ingressPath := fldPath.Child("ingress").Index(i) // TODO: Update From to be a pointer to slice as soon as auto-generation supports it. - for _, f := range i.From { + for i, from := range ingress.From { + fromPath := ingressPath.Child("from").Index(i) numFroms := 0 - if f.PodSelector != nil { + if from.PodSelector != nil { numFroms++ - allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(f.PodSelector, fldPath.Child("podSelector"))...) + allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(from.PodSelector, fromPath.Child("podSelector"))...) } - if f.NamespaceSelector != nil { - if numFroms > 0 { - allErrs = append(allErrs, field.Forbidden(fldPath, "may not specify more than 1 from type")) - } else { - numFroms++ - allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(f.NamespaceSelector, fldPath.Child("namespaces"))...) - } + if from.NamespaceSelector != nil { + numFroms++ + allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(from.NamespaceSelector, fromPath.Child("namespaceSelector"))...) } if numFroms == 0 { - // At least one of PodSelector and NamespaceSelector must be defined. - allErrs = append(allErrs, field.Required(fldPath, "must specify a from type")) + allErrs = append(allErrs, field.Required(fromPath, "must specify a from type")) + } else if numFroms > 1 { + allErrs = append(allErrs, field.Forbidden(fromPath, "may not specify more than 1 from type")) } } } From 1f6735c518e3e1b53820251a0a1705e5ac20c7a3 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 11 Jan 2017 17:41:20 -0500 Subject: [PATCH 2/2] Validate NetworkPolicy Ports Protocol must be "TCP", "UDP", or nil. Integer-valued port must be 1-65535. String-valued port must be a syntactically valid ContainerPort name. --- pkg/apis/extensions/validation/validation.go | 17 ++++ .../extensions/validation/validation_test.go | 82 +++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/pkg/apis/extensions/validation/validation.go b/pkg/apis/extensions/validation/validation.go index 0dd521a0c17..0f9d03dca24 100644 --- a/pkg/apis/extensions/validation/validation.go +++ b/pkg/apis/extensions/validation/validation.go @@ -830,6 +830,23 @@ func ValidateNetworkPolicySpec(spec *extensions.NetworkPolicySpec, fldPath *fiel // Validate ingress rules. for i, ingress := range spec.Ingress { ingressPath := fldPath.Child("ingress").Index(i) + for i, port := range ingress.Ports { + portPath := ingressPath.Child("ports").Index(i) + if port.Protocol != nil && *port.Protocol != api.ProtocolTCP && *port.Protocol != api.ProtocolUDP { + allErrs = append(allErrs, field.NotSupported(portPath.Child("protocol"), *port.Protocol, []string{string(api.ProtocolTCP), string(api.ProtocolUDP)})) + } + if port.Port != nil { + if port.Port.Type == intstr.Int { + for _, msg := range validation.IsValidPortNum(int(port.Port.IntVal)) { + allErrs = append(allErrs, field.Invalid(portPath.Child("port"), port.Port.IntVal, msg)) + } + } else { + for _, msg := range validation.IsValidPortName(port.Port.StrVal) { + allErrs = append(allErrs, field.Invalid(portPath.Child("port"), port.Port.StrVal, msg)) + } + } + } + } // TODO: Update From to be a pointer to slice as soon as auto-generation supports it. for i, from := range ingress.From { fromPath := ingressPath.Child("from").Index(i) diff --git a/pkg/apis/extensions/validation/validation_test.go b/pkg/apis/extensions/validation/validation_test.go index 915a0289b3d..d454e6787f2 100644 --- a/pkg/apis/extensions/validation/validation_test.go +++ b/pkg/apis/extensions/validation/validation_test.go @@ -2031,6 +2031,10 @@ func TestValidatePSPVolumes(t *testing.T) { } func TestValidateNetworkPolicy(t *testing.T) { + protocolTCP := api.ProtocolTCP + protocolUDP := api.ProtocolUDP + protocolICMP := api.Protocol("ICMP") + successCases := []extensions.NetworkPolicy{ { ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"}, @@ -2055,6 +2059,36 @@ func TestValidateNetworkPolicy(t *testing.T) { }, }, }, + { + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"}, + Spec: extensions.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + Ingress: []extensions.NetworkPolicyIngressRule{ + { + Ports: []extensions.NetworkPolicyPort{ + { + Protocol: nil, + Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 80}, + }, + { + Protocol: &protocolTCP, + Port: nil, + }, + { + Protocol: &protocolTCP, + Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 443}, + }, + { + Protocol: &protocolUDP, + Port: &intstr.IntOrString{Type: intstr.String, StrVal: "dns"}, + }, + }, + }, + }, + }, + }, { ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"}, Spec: extensions.NetworkPolicySpec{ @@ -2145,6 +2179,54 @@ func TestValidateNetworkPolicy(t *testing.T) { }, }, }, + "invalid ingress.ports.protocol": { + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"}, + Spec: extensions.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + Ingress: []extensions.NetworkPolicyIngressRule{ + { + Ports: []extensions.NetworkPolicyPort{ + { + Protocol: &protocolICMP, + Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 80}, + }, + }, + }, + }, + }, + }, + "invalid ingress.ports.port (int)": { + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"}, + Spec: extensions.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + Ingress: []extensions.NetworkPolicyIngressRule{ + { + Ports: []extensions.NetworkPolicyPort{ + { + Protocol: &protocolTCP, + Port: &intstr.IntOrString{Type: intstr.Int, IntVal: 123456789}, + }, + }, + }, + }, + }, + }, + "invalid ingress.ports.port (str)": { + ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"}, + Spec: extensions.NetworkPolicySpec{ + PodSelector: metav1.LabelSelector{}, + Ingress: []extensions.NetworkPolicyIngressRule{ + { + Ports: []extensions.NetworkPolicyPort{ + { + Protocol: &protocolTCP, + Port: &intstr.IntOrString{Type: intstr.String, StrVal: "!@#$"}, + }, + }, + }, + }, + }, + }, "invalid ingress.from.podSelector": { ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"}, Spec: extensions.NetworkPolicySpec{