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.

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

Add conformance test for .spec.Priority field in ANP #113

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 additions & 0 deletions conformance/base/admin_network_policy/core-priority-field.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
apiVersion: policy.networking.k8s.io/v1alpha1
kind: AdminNetworkPolicy
metadata:
name: priority-50-example
spec:
priority: 50
subject:
pods:
namespaceSelector:
matchLabels:
conformance-house: gryffindor
podSelector:
matchLabels:
conformance-house: gryffindor
ingress:
- name: "deny-all-ingress-from-slytherin"
action: "Deny"
from:
- pods:
namespaces:
namespaceSelector:
matchLabels:
conformance-house: slytherin
podSelector:
matchLabels:
conformance-house: slytherin
egress:
- name: "deny-all-egress-to-slytherin"
action: "Deny"
to:
- pods:
namespaces:
namespaceSelector:
matchLabels:
conformance-house: slytherin
podSelector:
matchLabels:
conformance-house: slytherin
---
apiVersion: policy.networking.k8s.io/v1alpha1
kind: AdminNetworkPolicy
metadata:
name: old-priority-60-new-priority-40-example
spec:
priority: 60 # will be updated to 40 in the tests thus taking higher precedence over the deny at priority 50
subject:
pods:
namespaceSelector:
matchLabels:
conformance-house: gryffindor
podSelector:
matchLabels:
conformance-house: gryffindor
ingress:
- name: "pass-all-ingress-from-slytherin"
action: "Pass"
from:
- pods:
namespaces:
namespaceSelector:
matchLabels:
conformance-house: slytherin
podSelector:
matchLabels:
conformance-house: slytherin
egress:
- name: "pass-all-egress-to-slytherin"
action: "Pass"
to:
- pods:
namespaces:
namespaceSelector:
matchLabels:
conformance-house: slytherin
podSelector:
matchLabels:
conformance-house: slytherin
---
apiVersion: policy.networking.k8s.io/v1alpha1
kind: BaselineAdminNetworkPolicy
metadata:
name: default
spec:
subject:
pods:
namespaceSelector:
matchLabels:
conformance-house: gryffindor
podSelector:
matchLabels:
conformance-house: gryffindor
ingress:
- name: "allow-all-ingress-from-slytherin"
action: "Allow"
from:
- pods:
namespaces:
namespaceSelector:
matchLabels:
conformance-house: slytherin
podSelector:
matchLabels:
conformance-house: slytherin
egress:
- name: "allow-all-egress-to-slytherin"
action: "Allow"
to:
- pods:
namespaces:
namespaceSelector:
matchLabels:
conformance-house: slytherin
podSelector:
matchLabels:
conformance-house: slytherin
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ spec:
matchLabels:
conformance-house: slytherin
egress:
- name: "deny-all-egress-from-slytherin" # test will update to pass
- name: "deny-all-egress-to-slytherin" # test will update to pass
action: "Deny" # test will update to pass
to:
- namespaces:
Expand Down Expand Up @@ -64,7 +64,7 @@ spec:
matchLabels:
conformance-house: slytherin
egress:
- name: "deny-all-egress-from-slytherin"
- name: "deny-all-egress-to-slytherin"
action: "Deny"
to:
- namespaces:
Expand Down
3 changes: 2 additions & 1 deletion conformance/tests/admin-network-policy-core-integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ var AdminNetworkPolicyIntegration = suite.ConformanceTest{
Description: "Tests integration support for gress traffic between ANP, NP and BANP using PASS action based on a server and client model",
Features: []suite.SupportedFeature{
suite.SupportAdminNetworkPolicy,
suite.SupportBaselineAdminNetworkPolicy,
},
Manifests: []string{"tests/admin-network-policy-core-integration_base.yaml"},
Manifests: []string{"base/api_integration/core-anp-np-banp.yaml"},
Test: func(t *testing.T, s *suite.ConformanceTestSuite) {

t.Run("Should Deny traffic from slytherin to gryffindor respecting ANP", func(t *testing.T) {
Expand Down
144 changes: 144 additions & 0 deletions conformance/tests/admin-network-policy-core-priority.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/*
Copyright 2022 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package tests

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
v1 "k8s.io/api/core/v1"
"k8s.io/kubernetes/test/e2e/framework"
"sigs.k8s.io/controller-runtime/pkg/client"

"sigs.k8s.io/network-policy-api/apis/v1alpha1"
"sigs.k8s.io/network-policy-api/conformance/utils/kubernetes"
"sigs.k8s.io/network-policy-api/conformance/utils/suite"
)

func init() {
ConformanceTests = append(ConformanceTests,
AdminNetworkPolicyPriorityField,
)
}

var AdminNetworkPolicyPriorityField = suite.ConformanceTest{
ShortName: "AdminNetworkPolicyPriorityField",
Description: "Tests support for admin network policy API's .spec.priority field based on a server and client model",
Features: []suite.SupportedFeature{
suite.SupportAdminNetworkPolicy,
suite.SupportBaselineAdminNetworkPolicy, // priority change of ANP should play well with existing BANP's
},
Manifests: []string{"base/admin_network_policy/core-priority-field.yaml"},
Test: func(t *testing.T, s *suite.ConformanceTestSuite) {

t.Run("Should Deny traffic from slytherin to gryffindor respecting ANP", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), s.TimeoutConfig.GetTimeout)
defer cancel()
// This test uses `priority-50-example` ANP; takes precedence over old-priority-60-new-priority-40-example ANP
// harry-potter-0 is our server pod in gryffindor namespace
clientPod := &v1.Pod{}
err := s.Client.Get(ctx, client.ObjectKey{
Namespace: "network-policy-conformance-gryffindor",
Name: "harry-potter-0",
}, clientPod)
framework.ExpectNoError(err, "unable to fetch the server pod")
// draco-malfoy-0 is our client pod in slytherin namespace
// ensure ingress is DENIED to gryffindor from slytherin
// inressRule at index0 will take effect
success := kubernetes.PokeServer(t, "network-policy-conformance-slytherin", "draco-malfoy-0", "tcp",
clientPod.Status.PodIP, int32(80), s.TimeoutConfig.RequestTimeout, false)
assert.Equal(t, true, success)
// draco-malfoy-1 is our client pod in slytherin namespace
success = kubernetes.PokeServer(t, "network-policy-conformance-slytherin", "draco-malfoy-1", "tcp",
clientPod.Status.PodIP, int32(8080), s.TimeoutConfig.RequestTimeout, false)
assert.Equal(t, true, success)
})

t.Run("Should Deny traffic to slytherin from gryffindor respecting ANP", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), s.TimeoutConfig.GetTimeout)
defer cancel()
// This test uses `priority-50-example` ANP; takes precedence over old-priority-60-new-priority-40-example ANP
// draco-malfoy-0 is our server pod in slytherin namespace
clientPod := &v1.Pod{}
err := s.Client.Get(ctx, client.ObjectKey{
Namespace: "network-policy-conformance-slytherin",
Name: "draco-malfoy-0",
}, clientPod)
framework.ExpectNoError(err, "unable to fetch the server pod")
// harry-potter-0 is our client pod in gryffindor namespace
// ensure ingress is DENIED to gryffindor from slytherin
// egressRule at index0 will take effect
success := kubernetes.PokeServer(t, "network-policy-conformance-gryffindor", "harry-potter-0", "tcp",
clientPod.Status.PodIP, int32(80), s.TimeoutConfig.RequestTimeout, false)
assert.Equal(t, true, success)
// harry-potter-1 is our client pod in gryffindor namespace
success = kubernetes.PokeServer(t, "network-policy-conformance-gryffindor", "harry-potter-1", "tcp",
clientPod.Status.PodIP, int32(8080), s.TimeoutConfig.RequestTimeout, false)
assert.Equal(t, true, success)
})

t.Run("Should respect ANP priority field; thus passing both ingress and egress traffic over to BANP", func(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), s.TimeoutConfig.GetTimeout)
defer cancel()
// This test uses `old-priority-60-new-priority-40-example` ANP
anp := &v1alpha1.AdminNetworkPolicy{}
err := s.Client.Get(ctx, client.ObjectKey{
Name: "old-priority-60-new-priority-40-example",
}, anp)
framework.ExpectNoError(err, "unable to fetch the admin network policy")
// change priority from 60 to 40
anp.Spec.Priority = 40
err = s.Client.Update(ctx, anp)
framework.ExpectNoError(err, "unable to update the admin network policy")
// harry-potter-0 is our server pod in gryffindor namespace
clientPod := &v1.Pod{}
err = s.Client.Get(ctx, client.ObjectKey{
Namespace: "network-policy-conformance-gryffindor",
Name: "harry-potter-0",
}, clientPod)
framework.ExpectNoError(err, "unable to fetch the server pod")
// draco-malfoy-0 is our client pod in slytherin namespace
// ensure ingress is PASSED to gryffindor from slytherin - the baseline admin network policy ALLOW should take effect
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we really tell the difference here between PASS and ALLOW?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i.e there's not really a way to verify wether it was

  1. an Explicit ANP Allow ||
  2. an Explicit BANP Allow ||
  3. an full pass through to Allow

Copy link
Member

@astoycos astoycos May 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you could eliminate 3. with a large-scale baseline deny

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah measuring pass and allow has been tricky actually.. good point here.
if there is something to catch this underneath like we do in the integration tests:

t.Run("Should support a 'pass-egress' policy for ANP and respect the match for network policy", func(t *testing.T) {
then we have a proper check, otherwise the two pretty much behave the exact same way.

// inressRule at index0 will take effect
success := kubernetes.PokeServer(t, "network-policy-conformance-slytherin", "draco-malfoy-0", "tcp",
clientPod.Status.PodIP, int32(80), s.TimeoutConfig.RequestTimeout, true)
assert.Equal(t, true, success)
// draco-malfoy-1 is our client pod in slytherin namespace
success = kubernetes.PokeServer(t, "network-policy-conformance-slytherin", "draco-malfoy-1", "tcp",
clientPod.Status.PodIP, int32(8080), s.TimeoutConfig.RequestTimeout, true)
assert.Equal(t, true, success)

// draco-malfoy-0 is our server pod in slytherin namespace
err = s.Client.Get(ctx, client.ObjectKey{
Namespace: "network-policy-conformance-slytherin",
Name: "draco-malfoy-0",
}, clientPod)
framework.ExpectNoError(err, "unable to fetch the server pod")
// harry-potter-0 is our client pod in gryffindor namespace
// ensure ingress is PASSED to gryffindor from slytherin - the baseline admin network policy ALLOW should take effect
// egressRule at index0 will take effect
success = kubernetes.PokeServer(t, "network-policy-conformance-gryffindor", "harry-potter-0", "tcp",
clientPod.Status.PodIP, int32(80), s.TimeoutConfig.RequestTimeout, true)
assert.Equal(t, true, success)
// harry-potter-1 is our client pod in gryffindor namespace
success = kubernetes.PokeServer(t, "network-policy-conformance-gryffindor", "harry-potter-1", "tcp",
clientPod.Status.PodIP, int32(8080), s.TimeoutConfig.RequestTimeout, true)
assert.Equal(t, true, success)
})
},
}