github
/
k3s
mirror of https://github.com/k3s-io/k3s
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #39769 from danwinship/networkpolicy-validation 

Automatic merge from submit-queue (batch tested with PRs 40497, 39769, 40554, 40569, 40597)

NetworkPolicy validation improvements

I noticed while implementing NetworkPolicy that I we weren't validating the "Ports" field at all.

The docs are actually completely silent about what a string-valued Port field is supposed to mean. I had guessed it meant to call `net.LookupPort()` on it (ie, map it from /etc/services) but in every other case where we have an IntOrString-valued Port field in an API struct, it refers to a named ContainerPort. But that would be extremely awkward to implement in this case; a policy specifying a named port could end up mapping to a different numeric port on every container in the namespace... Do other people actually implement string-valued ports that way? Or, for that matter, implement string-valued ports at all? (Related: I hadn't noticed until now that you can leave the Port value unspecified, allowing you to say "allow to all UDP ports, but no TCP ports" or "allow to all TCP ports, but no UDP ports". That seems like something that ended up in the spec just because it was possible, not because it was actually useful...)

@kubernetes/sig-network-misc
pull/6/head
Kubernetes Submit Queue 2017-01-27 17:38:25 -08:00 committed by GitHub
parent fe2829c16d 1f6735c518
commit c776d0978b
2 changed files with 112 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
pkg/apis/extensions/validation/validation.go
View File

@ -828,26 +828,42 @@ 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 {
// TODO: Update From to be a pointer to slice as soon as auto-generation supports it.
for _, f := range i.From {
numFroms := 0
if f.PodSelector != nil {
numFroms++
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(f.PodSelector, fldPath.Child("podSelector"))...)
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 f.NamespaceSelector != nil {
if numFroms > 0 {
allErrs = append(allErrs, field.Forbidden(fldPath, "may not specify more than 1 from type"))
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 {
numFroms++
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(f.NamespaceSelector, fldPath.Child("namespaces"))...)
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)
numFroms := 0
if from.PodSelector != nil {
numFroms++
allErrs = append(allErrs, unversionedvalidation.ValidateLabelSelector(from.PodSelector, fromPath.Child("podSelector"))...)
}
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"))
}
}
}

82
pkg/apis/extensions/validation/validation_test.go
View File

@ -2032,6 +2032,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"},
@ -2056,6 +2060,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{
@ -2146,6 +2180,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{