From c5f88d8ec23c1cfda56b5fac1a8334d17701dee6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Guilherme=20Vanz?= Date: Fri, 10 Jan 2025 16:22:02 -0300 Subject: [PATCH 1/3] chore(tests): move types factories to policies/v1 package. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the factories used in tests to build our types to the policies/v1 package. Therefore, all the tests under /api and /internal/controller directories can use them. Signed-off-by: José Guilherme Vanz --- Makefile | 6 +- .../v1/admissionpolicy_webhook_test.go | 17 +- .../v1/admissionpolicygroup_webhook_test.go | 16 +- .../v1/clusteradmissionpolicy_webhook_test.go | 14 +- ...lusteradmissionpolicygroup_webhook_test.go | 16 +- api/policies/v1/factories.go | 469 ++++++++++++++++++ api/policies/v1/policy_utils_test.go | 127 ----- api/policies/v1/policy_validation_test.go | 432 ++++++++-------- .../admissionpolicy_controller_test.go | 48 +- .../admissionpolicygroup_controller_test.go | 28 +- .../clusteradmissionpolicy_controller_test.go | 40 +- ...teradmissionpolicygroup_controller_test.go | 30 +- internal/controller/factories_test.go | 318 ------------ .../policyserver_controller_test.go | 148 +++--- internal/controller/utils_test.go | 2 + 15 files changed, 940 insertions(+), 771 deletions(-) create mode 100644 api/policies/v1/factories.go delete mode 100644 api/policies/v1/policy_utils_test.go delete mode 100644 internal/controller/factories_test.go diff --git a/Makefile b/Makefile index 0b1d0949..21f963e9 100644 --- a/Makefile +++ b/Makefile @@ -76,7 +76,7 @@ test: unit-tests integration-tests ## Run tests. .PHONY: unit-tests unit-tests: manifests generate fmt vet ## Run unit tests. - go test $$(go list ./... | grep -v /internal/controller) -race -test.v -coverprofile=coverage/unit-tests/coverage.txt -covermode=atomic + go test $$(go list ./... | grep -v /internal/controller) -race -test.v -coverprofile=coverage/unit-tests/coverage.txt -covermode=atomic -tags=testing # Integration tests are split into two targets to allow for running tests # that require a real cluster to be run separately from those that can be run using envtest. @@ -88,7 +88,7 @@ integration-tests: integration-tests-envtest integration-tests-real-cluster ## R .PHONY: integration-tests-envtest integration-tests-envtest: manifests generate fmt vet ginkgo envtest ## Run integration tests that do not require a real cluster only (using envtest). KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" \ - $(GINKGO) -v -github-output -timeout=$(TEST_TIMEOUT) -label-filter="!real-cluster" \ + $(GINKGO) -v -github-output -timeout=$(TEST_TIMEOUT) -label-filter="!real-cluster" -tags=testing \ -output-dir=./coverage/integration-tests/ -coverprofile=coverage-envtest.txt -covermode=atomic -coverpkg=all \ ./internal/controller/ @@ -97,7 +97,7 @@ integration-tests-real-cluster: manifests generate fmt ginkgo vet ## Run integra K3S_TESTCONTAINER_VERSION="$(K3S_TESTCONTAINER_VERSION)" POLICY_SERVER_VERSION="$(POLICY_SERVER_VERSION)" \ POLICY_SERVER_REPOSITORY="$(POLICY_SERVER_REPOSITORY)" $(GINKGO) -p -v -github-output -timeout=$(TEST_TIMEOUT) \ -label-filter="real-cluster" -output-dir=./coverage/integration-tests/ -coverprofile=coverage-real-cluster.txt \ - -covermode=atomic -coverpkg=all ./internal/controller/ + -tags=testing -covermode=atomic -coverpkg=all ./internal/controller/ .PHONY: lint lint: golangci-lint ## Run golangci-lint linter diff --git a/api/policies/v1/admissionpolicy_webhook_test.go b/api/policies/v1/admissionpolicy_webhook_test.go index 2ecc992f..00a89dbc 100644 --- a/api/policies/v1/admissionpolicy_webhook_test.go +++ b/api/policies/v1/admissionpolicy_webhook_test.go @@ -1,9 +1,11 @@ +//go:build testing + /* 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 + 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, @@ -11,7 +13,6 @@ 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 v1 import ( @@ -23,7 +24,7 @@ import ( ) func TestAdmissionPolicyDefault(t *testing.T) { - policy := admissionPolicyFactory() + policy := AdmissionPolicy{} policy.Default() require.Equal(t, constants.DefaultPolicyServer, policy.GetPolicyServer()) @@ -31,23 +32,23 @@ func TestAdmissionPolicyDefault(t *testing.T) { } func TestAdmissionPolicyValidateCreate(t *testing.T) { - policy := admissionPolicyFactory() + policy := NewAdmissionPolicyFactory().Build() warnings, err := policy.ValidateCreate() require.NoError(t, err) require.Empty(t, warnings) } func TestAdmissionPolicyValidateUpdate(t *testing.T) { - oldPolicy := admissionPolicyFactory() - newPolicy := admissionPolicyFactory() + oldPolicy := NewAdmissionPolicyFactory().Build() + newPolicy := NewAdmissionPolicyFactory().Build() warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.NoError(t, err) require.Empty(t, warnings) } func TestAdmissionPolicyValidateUpdateWithInvalidOldPolicy(t *testing.T) { - oldPolicy := clusterAdmissionPolicyFactory(nil, nil, "", "protect") - newPolicy := admissionPolicyFactory() + oldPolicy := NewClusterAdmissionPolicyFactory().Build() + newPolicy := NewAdmissionPolicyFactory().Build() warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.Empty(t, warnings) require.ErrorContains(t, err, "object is not of type AdmissionPolicy") diff --git a/api/policies/v1/admissionpolicygroup_webhook_test.go b/api/policies/v1/admissionpolicygroup_webhook_test.go index 802d462e..9c34597c 100644 --- a/api/policies/v1/admissionpolicygroup_webhook_test.go +++ b/api/policies/v1/admissionpolicygroup_webhook_test.go @@ -1,3 +1,5 @@ +//go:build testing + /* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -23,7 +25,7 @@ import ( ) func TestAdmissionPolicyGroupDefault(t *testing.T) { - policy := admissionPolicyGroupFactory() + policy := AdmissionPolicyGroup{} policy.Default() require.Equal(t, constants.DefaultPolicyServer, policy.GetPolicyServer()) @@ -31,14 +33,14 @@ func TestAdmissionPolicyGroupDefault(t *testing.T) { } func TestAdmissionPolicyGroupValidateCreate(t *testing.T) { - policy := admissionPolicyGroupFactory() + policy := NewAdmissionPolicyGroupFactory().Build() warnings, err := policy.ValidateCreate() require.NoError(t, err) require.Empty(t, warnings) } func TestClusterAdmissionPolicyValidateCreateWithNoMembers(t *testing.T) { - policy := admissionPolicyGroupFactory() + policy := NewAdmissionPolicyGroupFactory().Build() policy.Spec.Policies = nil warnings, err := policy.ValidateCreate() require.Error(t, err) @@ -47,16 +49,16 @@ func TestClusterAdmissionPolicyValidateCreateWithNoMembers(t *testing.T) { } func TestAdmissionPolicyGroupValidateUpdate(t *testing.T) { - oldPolicy := admissionPolicyGroupFactory() - newPolicy := admissionPolicyGroupFactory() + oldPolicy := NewAdmissionPolicyGroupFactory().Build() + newPolicy := NewAdmissionPolicyGroupFactory().Build() warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.NoError(t, err) require.Empty(t, warnings) } func TestAdmissionPolicyGroupValidateUpdateWithInvalidOldPolicy(t *testing.T) { - oldPolicy := clusterAdmissionPolicyGroupFactory() - newPolicy := admissionPolicyGroupFactory() + oldPolicy := NewClusterAdmissionPolicyGroupFactory().Build() + newPolicy := NewAdmissionPolicyGroupFactory().Build() warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.Empty(t, warnings) require.ErrorContains(t, err, "object is not of type AdmissionPolicyGroup") diff --git a/api/policies/v1/clusteradmissionpolicy_webhook_test.go b/api/policies/v1/clusteradmissionpolicy_webhook_test.go index 72117b91..4c623e4c 100644 --- a/api/policies/v1/clusteradmissionpolicy_webhook_test.go +++ b/api/policies/v1/clusteradmissionpolicy_webhook_test.go @@ -1,3 +1,5 @@ +//go:build testing + /* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -23,7 +25,7 @@ import ( ) func TestClusterAdmissionPolicyDefault(t *testing.T) { - policy := clusterAdmissionPolicyFactory(nil, nil, "", "protect") + policy := ClusterAdmissionPolicy{} policy.Default() require.Equal(t, constants.DefaultPolicyServer, policy.GetPolicyServer()) @@ -31,23 +33,23 @@ func TestClusterAdmissionPolicyDefault(t *testing.T) { } func TestClusterAdmissionPolicyValidateCreate(t *testing.T) { - policy := clusterAdmissionPolicyFactory(nil, nil, "", "protect") + policy := NewClusterAdmissionPolicyFactory().Build() warnings, err := policy.ValidateCreate() require.NoError(t, err) require.Empty(t, warnings) } func TestClusterAdmissionPolicyValidateUpdate(t *testing.T) { - oldPolicy := clusterAdmissionPolicyFactory(nil, nil, "", "protect") - newPolicy := clusterAdmissionPolicyFactory(nil, nil, "", "protect") + oldPolicy := NewClusterAdmissionPolicyFactory().Build() + newPolicy := NewClusterAdmissionPolicyFactory().Build() warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.NoError(t, err) require.Empty(t, warnings) } func TestClusterAdmissionPolicyValidateUpdateWithInvalidOldPolicy(t *testing.T) { - oldPolicy := admissionPolicyFactory() - newPolicy := clusterAdmissionPolicyFactory(nil, nil, "", "protect") + oldPolicy := NewAdmissionPolicyFactory().Build() + newPolicy := NewClusterAdmissionPolicyFactory().Build() warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.Empty(t, warnings) require.ErrorContains(t, err, "object is not of type ClusterAdmissionPolicy") diff --git a/api/policies/v1/clusteradmissionpolicygroup_webhook_test.go b/api/policies/v1/clusteradmissionpolicygroup_webhook_test.go index 2f3d92f4..9851b666 100644 --- a/api/policies/v1/clusteradmissionpolicygroup_webhook_test.go +++ b/api/policies/v1/clusteradmissionpolicygroup_webhook_test.go @@ -1,3 +1,5 @@ +//go:build testing + /* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -23,7 +25,7 @@ import ( ) func TestClusterClusterAdmissionPolicyDefault(t *testing.T) { - policy := clusterAdmissionPolicyGroupFactory() + policy := ClusterAdmissionPolicyGroup{} policy.Default() require.Equal(t, constants.DefaultPolicyServer, policy.GetPolicyServer()) @@ -31,14 +33,14 @@ func TestClusterClusterAdmissionPolicyDefault(t *testing.T) { } func TestClusterClusterAdmissionPolicyValidateCreate(t *testing.T) { - policy := clusterAdmissionPolicyGroupFactory() + policy := NewClusterAdmissionPolicyGroupFactory().Build() warnings, err := policy.ValidateCreate() require.NoError(t, err) require.Empty(t, warnings) } func TestClusterClusterAdmissionPolicyValidateCreateWithNoMembers(t *testing.T) { - policy := clusterAdmissionPolicyGroupFactory() + policy := NewClusterAdmissionPolicyGroupFactory().Build() policy.Spec.Policies = nil warnings, err := policy.ValidateCreate() require.Error(t, err) @@ -47,16 +49,16 @@ func TestClusterClusterAdmissionPolicyValidateCreateWithNoMembers(t *testing.T) } func TestClusterClusterAdmissionPolicyValidateUpdate(t *testing.T) { - oldPolicy := clusterAdmissionPolicyGroupFactory() - newPolicy := clusterAdmissionPolicyGroupFactory() + oldPolicy := NewClusterAdmissionPolicyGroupFactory().Build() + newPolicy := NewClusterAdmissionPolicyGroupFactory().Build() warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.NoError(t, err) require.Empty(t, warnings) } func TestClusterClusterAdmissionPolicyValidateUpdateWithInvalidOldPolicy(t *testing.T) { - oldPolicy := admissionPolicyGroupFactory() - newPolicy := clusterAdmissionPolicyGroupFactory() + oldPolicy := NewAdmissionPolicyGroupFactory().Build() + newPolicy := NewClusterAdmissionPolicyGroupFactory().Build() warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.Empty(t, warnings) require.ErrorContains(t, err, "object is not of type ClusterAdmissionPolicyGroup") diff --git a/api/policies/v1/factories.go b/api/policies/v1/factories.go new file mode 100644 index 00000000..f7a64245 --- /dev/null +++ b/api/policies/v1/factories.go @@ -0,0 +1,469 @@ +//go:build testing + +package v1 + +import ( + "fmt" + "math/rand" + "os" + + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/kubewarden/kubewarden-controller/internal/constants" +) + +const ( + integrationTestsFinalizer = "integration-tests-safety-net-finalizer" + defaultKubewardenRepository = "ghcr.io/kubewarden/policy-server" + maxNameSuffixLength = 8 +) + +type AdmissionPolicyFactory struct { + name string + namespace string + policyServer string + mutating bool + rules []admissionregistrationv1.RuleWithOperations + module string + matchConds []admissionregistrationv1.MatchCondition +} + +func NewAdmissionPolicyFactory() *AdmissionPolicyFactory { + return &AdmissionPolicyFactory{ + name: newName("admission-policy"), + namespace: "default", + policyServer: "", + mutating: false, + rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Create, + admissionregistrationv1.Update, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"Pods"}, + }, + }, + }, + module: "registry://ghcr.io/kubewarden/tests/pod-privileged:v0.2.5", + matchConds: []admissionregistrationv1.MatchCondition{ + {Name: "noop", Expression: "true"}, + }, + } +} + +func (fac *AdmissionPolicyFactory) WithName(name string) *AdmissionPolicyFactory { + fac.name = name + return fac +} + +func (fac *AdmissionPolicyFactory) WithNamespace(namespace string) *AdmissionPolicyFactory { + fac.namespace = namespace + return fac +} + +func (fac *AdmissionPolicyFactory) WithPolicyServer(policyServer string) *AdmissionPolicyFactory { + fac.policyServer = policyServer + return fac +} + +func (fac *AdmissionPolicyFactory) WithMutating(mutating bool) *AdmissionPolicyFactory { + fac.mutating = mutating + return fac +} + +func (fac *AdmissionPolicyFactory) WithRules(rules []admissionregistrationv1.RuleWithOperations) *AdmissionPolicyFactory { + fac.rules = rules + return fac +} + +func (fac *AdmissionPolicyFactory) WithMatchConditions(matchConditions []admissionregistrationv1.MatchCondition) *AdmissionPolicyFactory { + fac.matchConds = matchConditions + return fac +} + +func (fac *AdmissionPolicyFactory) Build() *AdmissionPolicy { + policy := AdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: fac.name, + Namespace: fac.namespace, + Finalizers: []string{ + // On a real cluster the Kubewarden finalizer is added by our mutating + // webhook. This is not running now, hence we have to manually add the finalizer + constants.KubewardenFinalizer, + // By adding this finalizer automatically, we ensure that when + // testing removal of finalizers on deleted objects, that they will + // exist at all times + integrationTestsFinalizer, + }, + }, + Spec: AdmissionPolicySpec{ + PolicySpec: PolicySpec{ + PolicyServer: fac.policyServer, + Module: fac.module, + Rules: fac.rules, + Mutating: fac.mutating, + MatchConditions: fac.matchConds, + }, + }, + } + return &policy +} + +type ClusterAdmissionPolicyFactory struct { + name string + policyServer string + mutating bool + rules []admissionregistrationv1.RuleWithOperations + module string + contextAwareResources []ContextAwareResource + matchConds []admissionregistrationv1.MatchCondition + mode PolicyMode +} + +func NewClusterAdmissionPolicyFactory() *ClusterAdmissionPolicyFactory { + return &ClusterAdmissionPolicyFactory{ + name: newName("cluster-admission"), + policyServer: "", + mutating: false, + rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Create, + admissionregistrationv1.Update, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"Pods"}, + }, + }, + }, + module: "registry://ghcr.io/kubewarden/tests/pod-privileged:v0.2.5", + contextAwareResources: []ContextAwareResource{}, + matchConds: []admissionregistrationv1.MatchCondition{ + {Name: "noop", Expression: "true"}, + }, + mode: "protect", + } +} + +func (fac *ClusterAdmissionPolicyFactory) WithName(name string) *ClusterAdmissionPolicyFactory { + fac.name = name + return fac +} + +func (fac *ClusterAdmissionPolicyFactory) WithPolicyServer(policyServer string) *ClusterAdmissionPolicyFactory { + fac.policyServer = policyServer + return fac +} + +func (fac *ClusterAdmissionPolicyFactory) WithMutating(mutating bool) *ClusterAdmissionPolicyFactory { + fac.mutating = mutating + return fac +} + +func (fac *ClusterAdmissionPolicyFactory) WithContextAwareResources(resources []ContextAwareResource) *ClusterAdmissionPolicyFactory { + fac.contextAwareResources = resources + return fac +} + +func (fac *ClusterAdmissionPolicyFactory) WithRules(rules []admissionregistrationv1.RuleWithOperations) *ClusterAdmissionPolicyFactory { + fac.rules = rules + return fac +} + +func (fac *ClusterAdmissionPolicyFactory) WithMatchConditions(matchConditions []admissionregistrationv1.MatchCondition) *ClusterAdmissionPolicyFactory { + fac.matchConds = matchConditions + return fac +} + +func (fac *ClusterAdmissionPolicyFactory) WithMode(mode PolicyMode) *ClusterAdmissionPolicyFactory { + fac.mode = mode + return fac +} + +func (fac *ClusterAdmissionPolicyFactory) Build() *ClusterAdmissionPolicy { + clusterAdmissionPolicy := ClusterAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: fac.name, + Finalizers: []string{ + // On a real cluster the Kubewarden finalizer is added by our mutating + // webhook. This is not running now, hence we have to manually add the finalizer + constants.KubewardenFinalizer, + // By adding this finalizer automatically, we ensure that when + // testing removal of finalizers on deleted objects, that they will + // exist at all times + integrationTestsFinalizer, + }}, + Spec: ClusterAdmissionPolicySpec{ + ContextAwareResources: fac.contextAwareResources, + PolicySpec: PolicySpec{ + PolicyServer: fac.policyServer, + Module: fac.module, + Rules: fac.rules, + Mutating: fac.mutating, + MatchConditions: fac.matchConds, + Mode: fac.mode, + }, + }, + } + return &clusterAdmissionPolicy +} + +type PolicyServerBuilder struct { + name string +} + +func NewPolicyServerFactory() *PolicyServerBuilder { + return &PolicyServerBuilder{ + name: newName("policy-server"), + } +} + +func (fac *PolicyServerBuilder) WithName(name string) *PolicyServerBuilder { + fac.name = name + return fac +} + +func (fac *PolicyServerBuilder) Build() *PolicyServer { + policyServer := PolicyServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: fac.name, + Finalizers: []string{ + // On a real cluster the Kubewarden finalizer is added by our mutating + // webhook. This is not running now, hence we have to manually add the finalizer + constants.KubewardenFinalizer, + // By adding this finalizer automatically, we ensure that when + // testing removal of finalizers on deleted objects, that they will + // exist at all times + integrationTestsFinalizer, + }, + }, + Spec: PolicyServerSpec{ + Image: policyServerRepository() + ":" + policyServerVersion(), + Replicas: 1, + }, + } + return &policyServer +} + +type AdmissionPolicyGroupFactory struct { + name string + namespace string + policyServer string + rules []admissionregistrationv1.RuleWithOperations + expression string + policyMembers PolicyGroupMembers + matchConds []admissionregistrationv1.MatchCondition +} + +func NewAdmissionPolicyGroupFactory() *AdmissionPolicyGroupFactory { + return &AdmissionPolicyGroupFactory{ + name: newName("admissing-policy-group"), + namespace: "default", + policyServer: "", + rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Create, + admissionregistrationv1.Update, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"Pods"}, + }, + }, + }, + expression: "pod_privileged()", + policyMembers: PolicyGroupMembers{ + "pod_privileged": { + Module: "registry://ghcr.io/kubewarden/tests/pod-privileged:v0.2.5", + }, + }, + matchConds: []admissionregistrationv1.MatchCondition{ + {Name: "noop", Expression: "true"}, + }, + } +} + +func (fac *AdmissionPolicyGroupFactory) WithName(name string) *AdmissionPolicyGroupFactory { + fac.name = name + return fac +} + +func (fac *AdmissionPolicyGroupFactory) WithNamespace(namespace string) *AdmissionPolicyGroupFactory { + fac.namespace = namespace + return fac +} + +func (fac *AdmissionPolicyGroupFactory) WithPolicyServer(policyServer string) *AdmissionPolicyGroupFactory { + fac.policyServer = policyServer + return fac +} + +func (fac *AdmissionPolicyGroupFactory) WithRules(rules []admissionregistrationv1.RuleWithOperations) *AdmissionPolicyGroupFactory { + fac.rules = rules + return fac +} + +func (fac *AdmissionPolicyGroupFactory) WithMatchConditions(matchContions []admissionregistrationv1.MatchCondition) *AdmissionPolicyGroupFactory { + fac.matchConds = matchContions + return fac +} + +func (fac *AdmissionPolicyGroupFactory) Build() *AdmissionPolicyGroup { + return &AdmissionPolicyGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: fac.name, + Namespace: fac.namespace, + Finalizers: []string{ + // On a real cluster the Kubewarden finalizer is added by our mutating + // webhook. This is not running now, hence we have to manually add the finalizer + constants.KubewardenFinalizer, + // By adding this finalizer automatically, we ensure that when + // testing removal of finalizers on deleted objects, that they will + // exist at all times + integrationTestsFinalizer, + }, + }, + Spec: AdmissionPolicyGroupSpec{ + PolicyGroupSpec: PolicyGroupSpec{ + PolicyServer: fac.policyServer, + Policies: fac.policyMembers, + Expression: fac.expression, + Rules: fac.rules, + MatchConditions: fac.matchConds, + }, + }, + } +} + +type ClusterAdmissionPolicyGroupFactory struct { + name string + policyServer string + rules []admissionregistrationv1.RuleWithOperations + expression string + policyMembers PolicyGroupMembers + matchConds []admissionregistrationv1.MatchCondition +} + +func NewClusterAdmissionPolicyGroupFactory() *ClusterAdmissionPolicyGroupFactory { + return &ClusterAdmissionPolicyGroupFactory{ + name: newName("cluster-admission-policy-group"), + policyServer: "", + rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Create, + admissionregistrationv1.Update, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"Pods"}, + }, + }, + }, + expression: "pod_privileged() && user_group_psp()", + policyMembers: PolicyGroupMembers{ + "pod_privileged": { + Module: "registry://ghcr.io/kubewarden/tests/pod-privileged:v0.2.5", + }, + "user_group_psp": { + Module: "registry://ghcr.io/kubewarden/tests/user-group-psp:v0.4.9", + }, + }, + matchConds: []admissionregistrationv1.MatchCondition{ + {Name: "noop", Expression: "true"}, + }, + } +} + +func (fac *ClusterAdmissionPolicyGroupFactory) WithName(name string) *ClusterAdmissionPolicyGroupFactory { + fac.name = name + return fac +} + +func (fac *ClusterAdmissionPolicyGroupFactory) WithPolicyServer(policyServer string) *ClusterAdmissionPolicyGroupFactory { + fac.policyServer = policyServer + return fac +} + +func (fac *ClusterAdmissionPolicyGroupFactory) WithMembers(members PolicyGroupMembers) *ClusterAdmissionPolicyGroupFactory { + fac.policyMembers = members + return fac +} + +func (fac *ClusterAdmissionPolicyGroupFactory) WithRules(rules []admissionregistrationv1.RuleWithOperations) *ClusterAdmissionPolicyGroupFactory { + fac.rules = rules + return fac +} + +func (fac *ClusterAdmissionPolicyGroupFactory) WithMatchConditions(matchConditions []admissionregistrationv1.MatchCondition) *ClusterAdmissionPolicyGroupFactory { + fac.matchConds = matchConditions + return fac +} + +func (fac *ClusterAdmissionPolicyGroupFactory) Build() *ClusterAdmissionPolicyGroup { + clusterAdmissionPolicy := ClusterAdmissionPolicyGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: fac.name, + Finalizers: []string{ + // On a real cluster the Kubewarden finalizer is added by our mutating + // webhook. This is not running now, hence we have to manually add the finalizer + constants.KubewardenFinalizer, + // By adding this finalizer automatically, we ensure that when + // testing removal of finalizers on deleted objects, that they will + // exist at all times + integrationTestsFinalizer, + }, + }, + Spec: ClusterAdmissionPolicyGroupSpec{ + PolicyGroupSpec: PolicyGroupSpec{ + PolicyServer: fac.policyServer, + Policies: fac.policyMembers, + Expression: fac.expression, + Rules: fac.rules, + MatchConditions: fac.matchConds, + }, + }, + } + return &clusterAdmissionPolicy +} + +func policyServerRepository() string { + repository, ok := os.LookupEnv("POLICY_SERVER_REPOSITORY") + if !ok { + return defaultKubewardenRepository + } + return repository +} + +func policyServerVersion() string { + version, ok := os.LookupEnv("POLICY_SERVER_VERSION") + if !ok { + return "latest" + } + + return version +} + +func randStringRunes(n int) string { + letterRunes := []rune("abcdefghijklmnopqrstuvwxyz1234567890") + b := make([]rune, n) + for i := range b { + //nolint:gosec // this is a test code. + b[i] = letterRunes[rand.Intn(len(letterRunes))] + } + + return string(b) +} + +func newName(prefix string) string { + return fmt.Sprintf("%s-%s", prefix, randStringRunes(maxNameSuffixLength)) +} diff --git a/api/policies/v1/policy_utils_test.go b/api/policies/v1/policy_utils_test.go deleted file mode 100644 index 49af3303..00000000 --- a/api/policies/v1/policy_utils_test.go +++ /dev/null @@ -1,127 +0,0 @@ -/* -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 v1 - -import ( - admissionregistrationv1 "k8s.io/api/admissionregistration/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" -) - -func admissionPolicyFactory() *AdmissionPolicy { - return &AdmissionPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testing-policy", - Namespace: "default", - }, - Spec: AdmissionPolicySpec{ - PolicySpec: PolicySpec{ - PolicyServer: "", - Settings: runtime.RawExtension{ - Raw: []byte("{}"), - }, - Rules: getRules(nil), - Mode: "protect", - }, - }, - } -} - -func admissionPolicyGroupFactory() *AdmissionPolicyGroup { - return &AdmissionPolicyGroup{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testing-policy-group", - Namespace: "default", - }, - Spec: AdmissionPolicyGroupSpec{ - PolicyGroupSpec: PolicyGroupSpec{ - PolicyServer: "", - Rules: getRules(nil), - Mode: "protect", - Expression: "mypolicy()", - Message: "This is a test policy", - Policies: map[string]PolicyGroupMember{ - "mypolicy": { - Module: "ghcr.io/kubewarden/tests/user-group-psp:v0.4.9", - Settings: runtime.RawExtension{}, - ContextAwareResources: []ContextAwareResource{}, - }, - }, - }, - }, - } -} - -func clusterAdmissionPolicyFactory(customRules []admissionregistrationv1.RuleWithOperations, matchConds []admissionregistrationv1.MatchCondition, policyServer string, policyMode PolicyMode) *ClusterAdmissionPolicy { - return &ClusterAdmissionPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testing-policy", - Namespace: "default", - }, - Spec: ClusterAdmissionPolicySpec{ - PolicySpec: PolicySpec{ - PolicyServer: policyServer, - Settings: runtime.RawExtension{ - Raw: []byte("{}"), - }, - Rules: getRules(customRules), - Mode: policyMode, - MatchConditions: matchConds, - }, - }, - } -} - -func clusterAdmissionPolicyGroupFactory() *ClusterAdmissionPolicyGroup { - return &ClusterAdmissionPolicyGroup{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testing-cluster-policy-group", - Namespace: "default", - }, - Spec: ClusterAdmissionPolicyGroupSpec{ - PolicyGroupSpec: PolicyGroupSpec{ - PolicyServer: "", - Rules: getRules(nil), - Mode: "protect", - MatchConditions: []admissionregistrationv1.MatchCondition{}, - Expression: "mypolicy()", - Message: "This is a test policy", - Policies: map[string]PolicyGroupMember{ - "mypolicy": { - Module: "ghcr.io/kubewarden/tests/user-group-psp:v0.4.9", - Settings: runtime.RawExtension{}, - ContextAwareResources: []ContextAwareResource{}, - }, - }, - }, - }, - } -} - -func getRules(customRules []admissionregistrationv1.RuleWithOperations) []admissionregistrationv1.RuleWithOperations { - rules := customRules - - if rules == nil { - rules = append(rules, admissionregistrationv1.RuleWithOperations{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"*"}, - APIVersions: []string{"*"}, - Resources: []string{"*/*"}, - }, - }) - } - return rules -} diff --git a/api/policies/v1/policy_validation_test.go b/api/policies/v1/policy_validation_test.go index cd6133b6..64f06ac5 100644 --- a/api/policies/v1/policy_validation_test.go +++ b/api/policies/v1/policy_validation_test.go @@ -1,3 +1,5 @@ +//go:build testing + /* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -29,137 +31,172 @@ func TestValidateRulesField(t *testing.T) { expectedErrorMessage string // use empty string when no error is expected }{ { - "with valid APIVersion and resources. But with empty APIGroup", clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{""}, - APIVersions: []string{"v1"}, - Resources: []string{"pods"}, - }, - }}, nil, "default", "protect"), + "with valid APIVersion and resources. But with empty APIGroup", + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"pods"}, + }, + }, + }). + WithPolicyServer("default").Build(), "", }, { "with valid APIVersion, Resources and APIGroup", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"apps"}, + APIVersions: []string{"v1"}, + Resources: []string{"deployments"}, + }, + }, + }). + WithPolicyServer("default").Build(), "", }, { "with no operations and API groups and resources", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{}). + WithPolicyServer("default").Build(), "spec.rules: Required value: a value must be specified", }, { "with empty objects", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{}}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{{}}). + WithPolicyServer("default").Build(), "spec.rules.operations: Required value: a value must be specified", }, { "with no operations", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"*"}, - APIVersions: []string{"*"}, - Resources: []string{"*/*"}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }}, + }). + WithPolicyServer("default").Build(), "spec.rules.operations: Required value: a value must be specified", }, { "with null operations", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: nil, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"*"}, - APIVersions: []string{"*"}, - Resources: []string{"*/*"}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{{ + Operations: nil, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }}). + WithPolicyServer("default").Build(), "spec.rules.operations: Required value: a value must be specified", }, { "with empty operations string", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{""}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"*"}, - APIVersions: []string{"*"}, - Resources: []string{"*/*"}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{""}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }}). + WithPolicyServer("default").Build(), "spec.rules.operations[0]: Required value: must be non-empty", }, { "with no apiVersion", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"*"}, - APIVersions: []string{}, - Resources: []string{"*/*"}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{}, + Resources: []string{"*/*"}, + }, + }}). + WithPolicyServer("default").Build(), "spec.rules: Required value: apiVersions and resources must have specified values", }, { "with no resources", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"*"}, - APIVersions: []string{"*"}, - Resources: []string{}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{}, + }, + }}).WithPolicyServer("default").Build(), "spec.rules: Required value: apiVersions and resources must have specified values", }, { "with empty apiVersion string", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"*"}, - APIVersions: []string{""}, - Resources: []string{"*/*"}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{""}, + Resources: []string{"*/*"}, + }, + }}).WithPolicyServer("defaule").Build(), "spec.rules.rule.apiVersions[0]: Required value: must be non-empty", }, { "with empty resources string", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"*"}, - APIVersions: []string{"*"}, - Resources: []string{""}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{""}, + }, + }}).WithPolicyServer("default").Build(), "spec.rules.rule.resources[0]: Required value: must be non-empty", }, { "with some of the resources are empty strings", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{""}, - APIVersions: []string{"v1"}, - Resources: []string{"", "pods"}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"", "pods"}, + }, + }}).WithPolicyServer("default").Build(), "spec.rules.rule.resources[0]: Required value: must be non-empty", }, { "with all operations and API groups and resources", - clusterAdmissionPolicyFactory(nil, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules([]admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }}).Build(), "", }, } @@ -179,6 +216,12 @@ func TestValidateRulesField(t *testing.T) { } func TestValidateMatchConditionsField(t *testing.T) { + defaultRules := []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{APIGroups: []string{"apps"}, APIVersions: []string{"v1"}, Resources: []string{"deployments"}}, + }, + } tests := []struct { name string policy Policy @@ -186,49 +229,54 @@ func TestValidateMatchConditionsField(t *testing.T) { }{ { "with empty MatchConditions", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{APIGroups: []string{"apps"}, APIVersions: []string{"v1"}, Resources: []string{"deployments"}}, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("default"). + Build(), "", }, { "with valid MatchConditions", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{APIGroups: []string{"apps"}, APIVersions: []string{"v1"}, Resources: []string{"deployments"}}, - }}, []admissionregistrationv1.MatchCondition{ - { - Name: "foo", - Expression: "true", - }, - }, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions([]admissionregistrationv1.MatchCondition{ + { + Name: "foo", + Expression: "true", + }, + }). + WithPolicyServer("default"). + Build(), "", }, { "with non-boolean MatchConditions", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{APIGroups: []string{"apps"}, APIVersions: []string{"v1"}, Resources: []string{"deployments"}}, - }}, []admissionregistrationv1.MatchCondition{ - { - Name: "foo", - Expression: "1 + 1", - }, - }, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions([]admissionregistrationv1.MatchCondition{ + { + Name: "foo", + Expression: "1 + 1", + }, + }). + WithPolicyServer("default"). + Build(), "Invalid value: \"1 + 1\": must evaluate to bool", }, { "with invalid expression in MatchConditions", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{APIGroups: []string{"apps"}, APIVersions: []string{"v1"}, Resources: []string{"deployments"}}, - }}, []admissionregistrationv1.MatchCondition{ - { - Name: "foo", - Expression: "invalid expression", - }, - }, "default", "protect"), "Syntax error: extraneous input 'expression'", + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions([]admissionregistrationv1.MatchCondition{ + { + Name: "foo", + Expression: "invalid expression", + }, + }). + WithPolicyServer("default"). + Build(), + "Syntax error: extraneous input 'expression'", }, } @@ -247,6 +295,14 @@ func TestValidateMatchConditionsField(t *testing.T) { } func TestValidatePolicyServerField(t *testing.T) { + defaultRules := []admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"apps"}, + APIVersions: []string{"v1"}, + Resources: []string{"deployments"}, + }, + }} tests := []struct { name string oldPolicy Policy @@ -255,42 +311,34 @@ func TestValidatePolicyServerField(t *testing.T) { }{ { "policy server unchanged", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "old-policy-server", "monitor"), - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "old-policy-server", "monitor"), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("old-policy-server"). + WithMode("monitor"). + Build(), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("old-policy-server"). + WithMode("monitor"). + Build(), "", }, { "policy server changed", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "old-policy-server", "monitor"), - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "new-policy-server", "monitor"), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("old-policy-server"). + WithMode("monitor"). + Build(), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("new-policy-server"). + WithMode("monitor"). + Build(), "spec.policyServer: Forbidden: the field is immutable", }, } @@ -309,6 +357,14 @@ func TestValidatePolicyServerField(t *testing.T) { } func TestValidatePolicyModeField(t *testing.T) { + defaultRules := []admissionregistrationv1.RuleWithOperations{{ + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"apps"}, + APIVersions: []string{"v1"}, + Resources: []string{"deployments"}, + }, + }} tests := []struct { name string oldPolicy Policy @@ -317,62 +373,50 @@ func TestValidatePolicyModeField(t *testing.T) { }{ { "policy mode unchanged", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "default", "protect"), - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("default"). + WithMode("protect"). + Build(), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("default"). + WithMode("protect"). + Build(), "", }, { "policy mode changed from monitor to protect", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "default", "monitor"), - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "default", "protect"), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("default"). + WithMode("monitor"). + Build(), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("default"). + WithMode("protect"). + Build(), "", }, { "policy mode changed from protect to monitor", - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "default", "protect"), - clusterAdmissionPolicyFactory([]admissionregistrationv1.RuleWithOperations{{ - Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{"apps"}, - APIVersions: []string{"v1"}, - Resources: []string{"deployments"}, - }, - }}, nil, "default", "monitor"), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("default"). + WithMode("protect"). + Build(), + NewClusterAdmissionPolicyFactory(). + WithRules(defaultRules). + WithMatchConditions(nil). + WithPolicyServer("default"). + WithMode("monitor"). + Build(), "spec.mode: Forbidden: field cannot transition from protect to monitor. Recreate instead.", }, } diff --git a/internal/controller/admissionpolicy_controller_test.go b/internal/controller/admissionpolicy_controller_test.go index 6ebf3204..c7ff52fe 100644 --- a/internal/controller/admissionpolicy_controller_test.go +++ b/internal/controller/admissionpolicy_controller_test.go @@ -1,3 +1,5 @@ +//go:build testing + /* Copyright 2022. @@ -53,16 +55,17 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() { BeforeAll(func() { policyServerName = newName("policy-server") - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory(). + WithName(policyServerName). + Build() createPolicyServerAndWaitForItsService(ctx, policyServer) policyName = newName("validating-policy") - policy = newAdmissionPolicyFactory(). - withName(policyName). - withNamespace(policyNamespace). - withPolicyServer(policyServerName). - withMutating(false). - build() + policy = policiesv1.NewAdmissionPolicyFactory(). + WithName(policyName). + WithNamespace(policyNamespace). + WithPolicyServer(policyServerName). + Build() Expect(k8sClient.Create(ctx, policy)).To(Succeed()) }) @@ -183,16 +186,18 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() { BeforeAll(func() { policyServerName = newName("policy-server") - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory(). + WithName(policyServerName). + Build() createPolicyServerAndWaitForItsService(ctx, policyServer) policyName = newName("mutating-policy") - policy = newAdmissionPolicyFactory(). - withName(policyName). - withNamespace(policyNamespace). - withPolicyServer(policyServerName). - withMutating(true). - build() + policy = policiesv1.NewAdmissionPolicyFactory(). + WithName(policyName). + WithNamespace(policyNamespace). + WithPolicyServer(policyServerName). + WithMutating(true). + Build() Expect(k8sClient.Create(ctx, policy)).To(Succeed()) }) @@ -309,7 +314,10 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() { It("should set policy status to unscheduled when creating an AdmissionPolicy without a PolicyServer assigned", func() { policyName := newName("unscheduled-policy") Expect( - k8sClient.Create(ctx, newAdmissionPolicyFactory().withName(policyName).withNamespace(policyNamespace).build()), + k8sClient.Create(ctx, policiesv1.NewAdmissionPolicyFactory(). + WithName(policyName). + WithNamespace(policyNamespace). + Build()), ).To(haveSucceededOrAlreadyExisted()) Eventually(func() (*policiesv1.AdmissionPolicy, error) { @@ -325,7 +333,11 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() { BeforeAll(func() { Expect( - k8sClient.Create(ctx, newAdmissionPolicyFactory().withName(policyName).withNamespace(policyNamespace).withPolicyServer(policyServerName).build()), + k8sClient.Create(ctx, policiesv1.NewAdmissionPolicyFactory(). + WithName(policyName). + WithNamespace(policyNamespace). + WithPolicyServer(policyServerName). + Build()), ).To(haveSucceededOrAlreadyExisted()) }) @@ -340,7 +352,9 @@ var _ = Describe("AdmissionPolicy controller", Label("real-cluster"), func() { It("should set the policy status to active when the PolicyServer is created", func() { By("creating the PolicyServer") Expect( - k8sClient.Create(ctx, newPolicyServerFactory().withName(policyServerName).build()), + k8sClient.Create(ctx, policiesv1.NewPolicyServerFactory(). + WithName(policyServerName). + Build()), ).To(haveSucceededOrAlreadyExisted()) By("changing the policy status to pending") diff --git a/internal/controller/admissionpolicygroup_controller_test.go b/internal/controller/admissionpolicygroup_controller_test.go index 66fff667..ab868b19 100644 --- a/internal/controller/admissionpolicygroup_controller_test.go +++ b/internal/controller/admissionpolicygroup_controller_test.go @@ -1,3 +1,5 @@ +//go:build testing + /* Copyright 2022. @@ -52,10 +54,16 @@ var _ = Describe("AdmissionPolicyGroup controller", Label("real-cluster"), func( BeforeAll(func() { policyServerName = newName("policy-server") - createPolicyServerAndWaitForItsService(ctx, newPolicyServerFactory().withName(policyServerName).build()) + createPolicyServerAndWaitForItsService(ctx, policiesv1.NewPolicyServerFactory(). + WithName(policyServerName). + Build()) policyName = newName("validating-policy") - policy = newAdmissionPolicyGroupFactory().withName(policyName).withNamespace(policyNamespace).withPolicyServer(policyServerName).build() + policy = policiesv1.NewAdmissionPolicyGroupFactory(). + WithName(policyName). + WithNamespace(policyNamespace). + WithPolicyServer(policyServerName). + Build() Expect(k8sClient.Create(ctx, policy)).To(Succeed()) }) @@ -172,7 +180,11 @@ var _ = Describe("AdmissionPolicyGroup controller", Label("real-cluster"), func( It("should set policy status to unscheduled when creating an AdmissionPolicyGroup without a PolicyServer assigned", func() { policyName := newName("unscheduled-policy") Expect( - k8sClient.Create(ctx, newAdmissionPolicyGroupFactory().withName(policyName).withNamespace(policyNamespace).withPolicyServer("").build()), + k8sClient.Create(ctx, policiesv1.NewAdmissionPolicyGroupFactory(). + WithName(policyName). + WithNamespace(policyNamespace). + WithPolicyServer(""). + Build()), ).To(haveSucceededOrAlreadyExisted()) Eventually(func() (*policiesv1.AdmissionPolicyGroup, error) { @@ -188,7 +200,11 @@ var _ = Describe("AdmissionPolicyGroup controller", Label("real-cluster"), func( BeforeAll(func() { Expect( - k8sClient.Create(ctx, newAdmissionPolicyGroupFactory().withName(policyName).withNamespace(policyNamespace).withPolicyServer(policyServerName).build()), + k8sClient.Create(ctx, policiesv1.NewAdmissionPolicyGroupFactory(). + WithName(policyName). + WithNamespace(policyNamespace). + WithPolicyServer(policyServerName). + Build()), ).To(haveSucceededOrAlreadyExisted()) }) @@ -203,7 +219,9 @@ var _ = Describe("AdmissionPolicyGroup controller", Label("real-cluster"), func( It("should set the policy status to active when the PolicyServer is created", func() { By("creating the PolicyServer") Expect( - k8sClient.Create(ctx, newPolicyServerFactory().withName(policyServerName).build()), + k8sClient.Create(ctx, policiesv1.NewPolicyServerFactory(). + WithName(policyServerName). + Build()), ).To(haveSucceededOrAlreadyExisted()) By("changing the policy status to pending") diff --git a/internal/controller/clusteradmissionpolicy_controller_test.go b/internal/controller/clusteradmissionpolicy_controller_test.go index a11672fc..9fd79c60 100644 --- a/internal/controller/clusteradmissionpolicy_controller_test.go +++ b/internal/controller/clusteradmissionpolicy_controller_test.go @@ -1,3 +1,5 @@ +//go:build testing + /* Copyright 2022. @@ -40,10 +42,16 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun BeforeAll(func() { policyServerName = newName("policy-server") - createPolicyServerAndWaitForItsService(ctx, newPolicyServerFactory().withName(policyServerName).build()) + createPolicyServerAndWaitForItsService(ctx, policiesv1.NewPolicyServerFactory(). + WithName(policyServerName). + Build()) policyName = newName("validating-policy") - policy = newClusterAdmissionPolicyFactory().withName(policyName).withPolicyServer(policyServerName).withMutating(false).build() + policy = policiesv1.NewClusterAdmissionPolicyFactory(). + WithName(policyName). + WithPolicyServer(policyServerName). + WithMutating(false). + Build() Expect(k8sClient.Create(ctx, policy)).To(Succeed()) }) @@ -148,10 +156,16 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun BeforeAll(func() { policyServerName = newName("policy-server") - createPolicyServerAndWaitForItsService(ctx, newPolicyServerFactory().withName(policyServerName).build()) + createPolicyServerAndWaitForItsService(ctx, policiesv1.NewPolicyServerFactory(). + WithName(policyServerName). + Build()) policyName = newName("mutating-policy") - policy = newClusterAdmissionPolicyFactory().withName(policyName).withPolicyServer(policyServerName).withMutating(true).build() + policy = policiesv1.NewClusterAdmissionPolicyFactory(). + WithName(policyName). + WithPolicyServer(policyServerName). + WithMutating(true). + Build() Expect(k8sClient.Create(ctx, policy)).To(Succeed()) }) @@ -251,7 +265,9 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun It("should set policy status to unscheduled when creating an ClusterAdmissionPolicy without a PolicyServer assigned", func() { policyName := newName("unscheduled-policy") Expect( - k8sClient.Create(ctx, newClusterAdmissionPolicyFactory().withName(policyName).withPolicyServer("").withMutating(false).build()), + k8sClient.Create(ctx, policiesv1.NewClusterAdmissionPolicyFactory(). + WithName(policyName). + Build()), ).To(haveSucceededOrAlreadyExisted()) Eventually(func() (*policiesv1.ClusterAdmissionPolicy, error) { @@ -267,13 +283,19 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun BeforeAll(func() { Expect( - k8sClient.Create(ctx, newClusterAdmissionPolicyFactory().withName(policyName).withPolicyServer(policyServerName).withMutating(false).build()), + k8sClient.Create(ctx, policiesv1.NewClusterAdmissionPolicyFactory(). + WithName(policyName). + WithPolicyServer(policyServerName). + Build()), ).To(haveSucceededOrAlreadyExisted()) }) It("should set the policy status to scheduled", func() { Expect( - k8sClient.Create(ctx, newClusterAdmissionPolicyFactory().withName(policyName).withPolicyServer(policyServerName).withMutating(false).build()), + k8sClient.Create(ctx, policiesv1.NewClusterAdmissionPolicyFactory(). + WithName(policyName). + WithPolicyServer(policyServerName). + Build()), ).To(haveSucceededOrAlreadyExisted()) Eventually(func() (*policiesv1.ClusterAdmissionPolicy, error) { @@ -286,7 +308,9 @@ var _ = Describe("ClusterAdmissionPolicy controller", Label("real-cluster"), fun It("should set the policy status to active when the PolicyServer is created", func() { By("creating the PolicyServer") Expect( - k8sClient.Create(ctx, newPolicyServerFactory().withName(policyServerName).build()), + k8sClient.Create(ctx, policiesv1.NewPolicyServerFactory(). + WithName(policyServerName). + Build()), ).To(haveSucceededOrAlreadyExisted()) By("changing the policy status to pending") diff --git a/internal/controller/clusteradmissionpolicygroup_controller_test.go b/internal/controller/clusteradmissionpolicygroup_controller_test.go index b621fcbc..7a8f8a60 100644 --- a/internal/controller/clusteradmissionpolicygroup_controller_test.go +++ b/internal/controller/clusteradmissionpolicygroup_controller_test.go @@ -1,3 +1,5 @@ +//go:build testing + /* Copyright 2022. @@ -40,10 +42,15 @@ var _ = Describe("ClusterAdmissionPolicyGroup controller", Label("real-cluster") BeforeAll(func() { policyServerName = newName("policy-server") - createPolicyServerAndWaitForItsService(ctx, newPolicyServerFactory().withName(policyServerName).build()) + createPolicyServerAndWaitForItsService(ctx, policiesv1.NewPolicyServerFactory(). + WithName(policyServerName). + Build()) policyName = newName("validating-policy") - policy = newClusterAdmissionPolicyGroupFactory().withName(policyName).withPolicyServer(policyServerName).build() + policy = policiesv1.NewClusterAdmissionPolicyGroupFactory(). + WithName(policyName). + WithPolicyServer(policyServerName). + Build() Expect(k8sClient.Create(ctx, policy)).To(Succeed()) }) @@ -144,7 +151,10 @@ var _ = Describe("ClusterAdmissionPolicyGroup controller", Label("real-cluster") It("should set policy status to unscheduled when creating an ClusterAdmissionPolicyGroup without a PolicyServer assigned", func() { policyName := newName("unscheduled-policy") Expect( - k8sClient.Create(ctx, newClusterAdmissionPolicyGroupFactory().withName(policyName).withPolicyServer("").build()), + k8sClient.Create(ctx, policiesv1.NewClusterAdmissionPolicyGroupFactory(). + WithName(policyName). + WithPolicyServer(""). + Build()), ).To(haveSucceededOrAlreadyExisted()) Eventually(func() (*policiesv1.ClusterAdmissionPolicyGroup, error) { @@ -160,13 +170,19 @@ var _ = Describe("ClusterAdmissionPolicyGroup controller", Label("real-cluster") BeforeAll(func() { Expect( - k8sClient.Create(ctx, newClusterAdmissionPolicyGroupFactory().withName(policyName).withPolicyServer(policyServerName).build()), + k8sClient.Create(ctx, policiesv1.NewClusterAdmissionPolicyGroupFactory(). + WithName(policyName). + WithPolicyServer(policyServerName). + Build()), ).To(haveSucceededOrAlreadyExisted()) }) It("should set the policy status to scheduled", func() { Expect( - k8sClient.Create(ctx, newClusterAdmissionPolicyGroupFactory().withName(policyName).withPolicyServer(policyServerName).build()), + k8sClient.Create(ctx, policiesv1.NewClusterAdmissionPolicyGroupFactory(). + WithName(policyName). + WithPolicyServer(policyServerName). + Build()), ).To(haveSucceededOrAlreadyExisted()) Eventually(func() (*policiesv1.ClusterAdmissionPolicyGroup, error) { @@ -179,7 +195,9 @@ var _ = Describe("ClusterAdmissionPolicyGroup controller", Label("real-cluster") It("should set the policy status to active when the PolicyServer is created", func() { By("creating the PolicyServer") Expect( - k8sClient.Create(ctx, newPolicyServerFactory().withName(policyServerName).build()), + k8sClient.Create(ctx, policiesv1.NewPolicyServerFactory(). + WithName(policyServerName). + Build()), ).To(haveSucceededOrAlreadyExisted()) By("changing the policy status to pending") diff --git a/internal/controller/factories_test.go b/internal/controller/factories_test.go deleted file mode 100644 index fd1d21e1..00000000 --- a/internal/controller/factories_test.go +++ /dev/null @@ -1,318 +0,0 @@ -package controller - -import ( - "os" - - admissionregistrationv1 "k8s.io/api/admissionregistration/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - policiesv1 "github.com/kubewarden/kubewarden-controller/api/policies/v1" - "github.com/kubewarden/kubewarden-controller/internal/constants" -) - -type admissionPolicyFactory struct { - name string - namespace string - policyServer string - mutating bool - rules []admissionregistrationv1.RuleWithOperations - module string -} - -func newAdmissionPolicyFactory() *admissionPolicyFactory { - return &admissionPolicyFactory{ - name: newName("validating-policy"), - namespace: "", - policyServer: "", - mutating: false, - rules: []admissionregistrationv1.RuleWithOperations{}, - module: "registry://ghcr.io/kubewarden/tests/pod-privileged:v0.2.5", - } -} - -func (fac *admissionPolicyFactory) withName(name string) *admissionPolicyFactory { - fac.name = name - return fac -} - -func (fac *admissionPolicyFactory) withNamespace(namespace string) *admissionPolicyFactory { - fac.namespace = namespace - return fac -} - -func (fac *admissionPolicyFactory) withPolicyServer(policyServer string) *admissionPolicyFactory { - fac.policyServer = policyServer - return fac -} - -func (fac *admissionPolicyFactory) withMutating(mutating bool) *admissionPolicyFactory { - fac.mutating = mutating - return fac -} - -func (fac *admissionPolicyFactory) build() *policiesv1.AdmissionPolicy { - policy := policiesv1.AdmissionPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: fac.name, - Namespace: fac.namespace, - Finalizers: []string{ - // On a real cluster the Kubewarden finalizer is added by our mutating - // webhook. This is not running now, hence we have to manually add the finalizer - constants.KubewardenFinalizer, - // By adding this finalizer automatically, we ensure that when - // testing removal of finalizers on deleted objects, that they will - // exist at all times - integrationTestsFinalizer, - }, - }, - Spec: policiesv1.AdmissionPolicySpec{ - PolicySpec: policiesv1.PolicySpec{ - PolicyServer: fac.policyServer, - Module: fac.module, - Rules: fac.rules, - Mutating: fac.mutating, - MatchConditions: []admissionregistrationv1.MatchCondition{ - { - Name: "noop", - Expression: "true", - }, - }, - }, - }, - } - return &policy -} - -type clusterAdmissionPolicyFactory struct { - name string - policyServer string - mutating bool - rules []admissionregistrationv1.RuleWithOperations - module string -} - -func newClusterAdmissionPolicyFactory() *clusterAdmissionPolicyFactory { - return &clusterAdmissionPolicyFactory{ - name: newName("validating-cluster-policy"), - policyServer: "", - mutating: false, - rules: []admissionregistrationv1.RuleWithOperations{}, - module: "registry://ghcr.io/kubewarden/tests/pod-privileged:v0.2.5", - } -} - -func (fac *clusterAdmissionPolicyFactory) withName(name string) *clusterAdmissionPolicyFactory { - fac.name = name - return fac -} - -func (fac *clusterAdmissionPolicyFactory) withPolicyServer(policyServer string) *clusterAdmissionPolicyFactory { - fac.policyServer = policyServer - return fac -} - -func (fac *clusterAdmissionPolicyFactory) withMutating(mutating bool) *clusterAdmissionPolicyFactory { - fac.mutating = mutating - return fac -} - -func (fac *clusterAdmissionPolicyFactory) build() *policiesv1.ClusterAdmissionPolicy { - clusterAdmissionPolicy := policiesv1.ClusterAdmissionPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: fac.name, - Finalizers: []string{ - // On a real cluster the Kubewarden finalizer is added by our mutating - // webhook. This is not running now, hence we have to manually add the finalizer - constants.KubewardenFinalizer, - // By adding this finalizer automatically, we ensure that when - // testing removal of finalizers on deleted objects, that they will - // exist at all times - integrationTestsFinalizer, - }}, - Spec: policiesv1.ClusterAdmissionPolicySpec{ - PolicySpec: policiesv1.PolicySpec{ - PolicyServer: fac.policyServer, - Module: fac.module, - Rules: fac.rules, - Mutating: fac.mutating, - MatchConditions: []admissionregistrationv1.MatchCondition{ - {Name: "noop", Expression: "true"}, - }, - }, - }, - } - return &clusterAdmissionPolicy -} - -type policyServerBuilder struct { - name string -} - -func newPolicyServerFactory() *policyServerBuilder { - return &policyServerBuilder{ - name: newName("policy-server"), - } -} - -func (fac *policyServerBuilder) withName(name string) *policyServerBuilder { - fac.name = name - return fac -} - -func (fac *policyServerBuilder) build() *policiesv1.PolicyServer { - policyServer := policiesv1.PolicyServer{ - ObjectMeta: metav1.ObjectMeta{ - Name: fac.name, - Finalizers: []string{ - // On a real cluster the Kubewarden finalizer is added by our mutating - // webhook. This is not running now, hence we have to manually add the finalizer - constants.KubewardenFinalizer, - // By adding this finalizer automatically, we ensure that when - // testing removal of finalizers on deleted objects, that they will - // exist at all times - integrationTestsFinalizer, - }, - }, - Spec: policiesv1.PolicyServerSpec{ - Image: policyServerRepository() + ":" + policyServerVersion(), - Replicas: 1, - }, - } - return &policyServer -} - -type admissionPolicyGroupFactory struct { - name string - namespace string - policyServer string -} - -func newAdmissionPolicyGroupFactory() *admissionPolicyGroupFactory { - return &admissionPolicyGroupFactory{ - name: newName("validating-policygroup"), - namespace: "", - policyServer: "", - } -} - -func (fac *admissionPolicyGroupFactory) withName(name string) *admissionPolicyGroupFactory { - fac.name = name - return fac -} - -func (fac *admissionPolicyGroupFactory) withNamespace(namespace string) *admissionPolicyGroupFactory { - fac.namespace = namespace - return fac -} - -func (fac *admissionPolicyGroupFactory) withPolicyServer(policyServer string) *admissionPolicyGroupFactory { - fac.policyServer = policyServer - return fac -} - -func (fac *admissionPolicyGroupFactory) build() *policiesv1.AdmissionPolicyGroup { - admissionPolicy := policiesv1.AdmissionPolicyGroup{ - ObjectMeta: metav1.ObjectMeta{ - Name: fac.name, - Namespace: fac.namespace, - Finalizers: []string{ - // On a real cluster the Kubewarden finalizer is added by our mutating - // webhook. This is not running now, hence we have to manually add the finalizer - constants.KubewardenFinalizer, - // By adding this finalizer automatically, we ensure that when - // testing removal of finalizers on deleted objects, that they will - // exist at all times - integrationTestsFinalizer, - }, - }, - Spec: policiesv1.AdmissionPolicyGroupSpec{ - PolicyGroupSpec: policiesv1.PolicyGroupSpec{ - PolicyServer: fac.policyServer, - Policies: policiesv1.PolicyGroupMembers{ - "pod-privileged": { - Module: "registry://ghcr.io/kubewarden/tests/pod-privileged:v0.2.5", - }, - }, - Rules: []admissionregistrationv1.RuleWithOperations{}, - MatchConditions: []admissionregistrationv1.MatchCondition{ - {Name: "noop", Expression: "true"}, - }, - }, - }, - } - return &admissionPolicy -} - -type clusterAdmissionPolicyGroupFactory struct { - name string - policyServer string -} - -func newClusterAdmissionPolicyGroupFactory() *clusterAdmissionPolicyGroupFactory { - return &clusterAdmissionPolicyGroupFactory{ - name: newName("validating-policygroup"), - policyServer: "", - } -} - -func (fac *clusterAdmissionPolicyGroupFactory) withName(name string) *clusterAdmissionPolicyGroupFactory { - fac.name = name - return fac -} - -func (fac *clusterAdmissionPolicyGroupFactory) withPolicyServer(policyServer string) *clusterAdmissionPolicyGroupFactory { - fac.policyServer = policyServer - return fac -} - -func (fac *clusterAdmissionPolicyGroupFactory) build() *policiesv1.ClusterAdmissionPolicyGroup { - clusterAdmissionPolicy := policiesv1.ClusterAdmissionPolicyGroup{ - ObjectMeta: metav1.ObjectMeta{ - Name: fac.name, - Finalizers: []string{ - // On a real cluster the Kubewarden finalizer is added by our mutating - // webhook. This is not running now, hence we have to manually add the finalizer - constants.KubewardenFinalizer, - // By adding this finalizer automatically, we ensure that when - // testing removal of finalizers on deleted objects, that they will - // exist at all times - integrationTestsFinalizer, - }, - }, - Spec: policiesv1.ClusterAdmissionPolicyGroupSpec{ - PolicyGroupSpec: policiesv1.PolicyGroupSpec{ - Rules: []admissionregistrationv1.RuleWithOperations{}, - PolicyServer: fac.policyServer, - MatchConditions: []admissionregistrationv1.MatchCondition{ - {Name: "noop", Expression: "true"}, - }, - Policies: policiesv1.PolicyGroupMembers{ - "pod-privileged": { - Module: "registry://ghcr.io/kubewarden/tests/pod-privileged:v0.2.5", - }, - "user-group-psp": { - Module: "registry://ghcr.io/kubewarden/tests/user-group-psp:v0.4.9", - }, - }, - }, - }, - } - return &clusterAdmissionPolicy -} - -func policyServerRepository() string { - repository, ok := os.LookupEnv("POLICY_SERVER_REPOSITORY") - if !ok { - return defaultKubewardenRepository - } - return repository -} - -func policyServerVersion() string { - version, ok := os.LookupEnv("POLICY_SERVER_VERSION") - if !ok { - return "latest" - } - - return version -} diff --git a/internal/controller/policyserver_controller_test.go b/internal/controller/policyserver_controller_test.go index 5bb69b4f..f0e4f506 100644 --- a/internal/controller/policyserver_controller_test.go +++ b/internal/controller/policyserver_controller_test.go @@ -1,3 +1,5 @@ +//go:build testing + /* copyright 2022. @@ -48,7 +50,7 @@ var _ = Describe("PolicyServer controller", func() { When("creating a PolicyServer", func() { It("should use the policy server tolerations configuration in the policy server deployment", func() { tolerationSeconds := int64(10) - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() policyServer.Spec.Tolerations = []corev1.Toleration{{ Key: "key1", Operator: corev1.TolerationOpEqual, @@ -89,7 +91,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should use the policy server affinity configuration in the policy server deployment", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() policyServer.Spec.Affinity = corev1.Affinity{ NodeAffinity: &corev1.NodeAffinity{ RequiredDuringSchedulingIgnoredDuringExecution: &corev1.NodeSelector{ @@ -128,7 +130,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create policy server deployment with some default configuration", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) deployment, err := getTestPolicyServerDeployment(ctx, policyServerName) @@ -178,7 +180,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create the policy server deployment and use the user defined security contexts", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() runAsUser := int64(1000) privileged := true runAsNonRoot := false @@ -229,7 +231,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create the policy server configmap empty if no policies are assigned ", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) configmap, err := getTestPolicyServerConfigMap(ctx, policyServerName) @@ -244,47 +246,57 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create the policy server configmap with the assigned policies", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) - admissionPolicy := newAdmissionPolicyFactory().withName(newName("admission-policy")).withNamespace("default").withPolicyServer(policyServerName).build() + admissionPolicy := policiesv1.NewAdmissionPolicyFactory(). + WithPolicyServer(policyServerName). + Build() Expect(k8sClient.Create(ctx, admissionPolicy)).To(Succeed()) - clusterAdmissionPolicy := newClusterAdmissionPolicyFactory().withName(newName("cluster-admission")).withPolicyServer(policyServerName).withMutating(false).build() - clusterAdmissionPolicy.Spec.ContextAwareResources = []policiesv1.ContextAwareResource{ - { - APIVersion: "v1", - Kind: "Pod", - }, - { - APIVersion: "v1", - Kind: "Deployment", - }, - } + clusterAdmissionPolicy := policiesv1.NewClusterAdmissionPolicyFactory(). + WithPolicyServer(policyServerName). + WithContextAwareResources([]policiesv1.ContextAwareResource{ + { + APIVersion: "v1", + Kind: "Pod", + }, + { + APIVersion: "v1", + Kind: "Deployment", + }, + }). + Build() Expect(k8sClient.Create(ctx, clusterAdmissionPolicy)).To(Succeed()) - admissionPolicyGroup := newAdmissionPolicyGroupFactory().withName(newName("admissing-policy-group")).withNamespace("default").withPolicyServer(policyServerName).build() + admissionPolicyGroup := policiesv1.NewAdmissionPolicyGroupFactory(). + WithPolicyServer(policyServerName). + Build() Expect(k8sClient.Create(ctx, admissionPolicyGroup)).To(Succeed()) - clusterPolicyGroup := newClusterAdmissionPolicyGroupFactory().withName(newName("cluster-admission-policy-group")).withPolicyServer(policyServerName).build() - podPrivilegedPolicy := clusterPolicyGroup.Spec.Policies["pod-privileged"] - podPrivilegedPolicy.ContextAwareResources = []policiesv1.ContextAwareResource{ - { - APIVersion: "v1", - Kind: "Pod", - }, - } - clusterPolicyGroup.Spec.Policies["pod-privileged"] = podPrivilegedPolicy - - userGroupPolicy := clusterPolicyGroup.Spec.Policies["user-group-psp"] - userGroupPolicy.ContextAwareResources = []policiesv1.ContextAwareResource{ - { - APIVersion: "v1", - Kind: "Deployment", - }, - } - clusterPolicyGroup.Spec.Policies["user-group-psp"] = userGroupPolicy - + clusterPolicyGroup := policiesv1.NewClusterAdmissionPolicyGroupFactory(). + WithPolicyServer(policyServerName). + WithMembers(policiesv1.PolicyGroupMembers{ + "pod_privileged": { + Module: "registry://ghcr.io/kubewarden/tests/pod-privileged:v0.2.5", + ContextAwareResources: []policiesv1.ContextAwareResource{ + { + APIVersion: "v1", + Kind: "Pod", + }, + }, + }, + "user_group_psp": { + Module: "registry://ghcr.io/kubewarden/tests/user-group-psp:v0.4.9", + ContextAwareResources: []policiesv1.ContextAwareResource{ + { + APIVersion: "v1", + Kind: "Deployment", + }, + }, + }, + }). + Build() Expect(k8sClient.Create(ctx, clusterPolicyGroup)).To(Succeed()) policiesMap := policyConfigEntryMap{} @@ -393,8 +405,8 @@ var _ = Describe("PolicyServer controller", func() { "Name": Equal(admissionPolicyGroup.GetName()), }), "policies": MatchKeys(IgnoreExtras, Keys{ - "pod-privileged": MatchKeys(IgnoreExtras, Keys{ - "module": Equal(admissionPolicyGroup.GetPolicyGroupMembers()["pod-privileged"].Module), + "pod_privileged": MatchKeys(IgnoreExtras, Keys{ + "module": Equal(admissionPolicyGroup.GetPolicyGroupMembers()["pod_privileged"].Module), }), }), "policyMode": Equal(string(admissionPolicyGroup.GetPolicyMode())), @@ -407,16 +419,16 @@ var _ = Describe("PolicyServer controller", func() { "Name": Equal(clusterPolicyGroup.GetName()), }), "policies": MatchKeys(IgnoreExtras, Keys{ - "pod-privileged": MatchAllKeys(Keys{ - "module": Equal(clusterPolicyGroup.GetPolicyGroupMembers()["pod-privileged"].Module), + "pod_privileged": MatchAllKeys(Keys{ + "module": Equal(clusterPolicyGroup.GetPolicyGroupMembers()["pod_privileged"].Module), "settings": Ignore(), "contextAwareResources": And(ContainElement(MatchAllKeys(Keys{ "apiVersion": Equal("v1"), "kind": Equal("Pod"), })), HaveLen(1)), }), - "user-group-psp": MatchAllKeys(Keys{ - "module": Equal(clusterPolicyGroup.GetPolicyGroupMembers()["user-group-psp"].Module), + "user_group_psp": MatchAllKeys(Keys{ + "module": Equal(clusterPolicyGroup.GetPolicyGroupMembers()["user_group_psp"].Module), "settings": Ignore(), "contextAwareResources": And(ContainElement(MatchAllKeys(Keys{ "apiVersion": Equal("v1"), @@ -441,7 +453,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create the policy server configmap with the sources authorities", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() policyServer.Spec.InsecureSources = []string{"localhost:5000"} policyServer.Spec.SourceAuthorities = map[string][]string{ "myprivateregistry:5000": {"cert1", "cert2"}, @@ -481,7 +493,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create PodDisruptionBudget when policy server has MinAvailable configuration set", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() minAvailable := intstr.FromInt(2) policyServer.Spec.MinAvailable = &minAvailable createPolicyServerAndWaitForItsService(ctx, policyServer) @@ -493,7 +505,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create PodDisruptionBudget when policy server has MaxUnavailable configuration set", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() maxUnavailable := intstr.FromInt(2) policyServer.Spec.MaxUnavailable = &maxUnavailable createPolicyServerAndWaitForItsService(ctx, policyServer) @@ -505,7 +517,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should not create PodDisruptionBudget when policy server has no PDB configuration", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) Consistently(func() error { @@ -515,7 +527,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create the PolicyServer deployment with the limits and the requests", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() policyServer.Spec.Limits = corev1.ResourceList{ "cpu": resource.MustParse("100m"), "memory": resource.MustParse("1Gi"), @@ -534,7 +546,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create deployment with owner reference", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) Eventually(func() error { @@ -561,7 +573,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create configmap with owner reference", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) Eventually(func() error { @@ -587,7 +599,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create service with owner reference", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) Eventually(func() error { @@ -613,7 +625,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should create the policy server secrets", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) Eventually(func() error { @@ -645,7 +657,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should set the configMap version as a deployment annotation", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) configmap, err := getTestPolicyServerConfigMap(ctx, policyServerName) @@ -667,7 +679,7 @@ var _ = Describe("PolicyServer controller", func() { }) It("should update the configMap version after adding a policy", func() { - policyServer := newPolicyServerFactory().withName(policyServerName).build() + policyServer := policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) initalConfigMap, err := getTestPolicyServerConfigMap(ctx, policyServerName) @@ -688,7 +700,10 @@ var _ = Describe("PolicyServer controller", func() { }, timeout, pollInterval).Should(Succeed()) policyName := newName("validating-policy") - policy := newClusterAdmissionPolicyFactory().withName(policyName).withPolicyServer(policyServerName).withMutating(false).build() + policy := policiesv1.NewClusterAdmissionPolicyFactory(). + WithName(policyName). + WithPolicyServer(policyServerName). + Build() Expect(k8sClient.Create(ctx, policy)).To(Succeed()) Eventually(func() error { @@ -718,7 +733,7 @@ var _ = Describe("PolicyServer controller", func() { var policyServer *policiesv1.PolicyServer BeforeEach(func() { - policyServer = newPolicyServerFactory().withName(policyServerName).build() + policyServer = policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build() createPolicyServerAndWaitForItsService(ctx, policyServer) }) @@ -998,13 +1013,13 @@ var _ = Describe("PolicyServer controller", func() { When("deleting a PolicyServer", func() { BeforeEach(func() { - createPolicyServerAndWaitForItsService(ctx, newPolicyServerFactory().withName(policyServerName).build()) + createPolicyServerAndWaitForItsService(ctx, policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build()) }) Context("with no assigned policies", func() { It("should get its finalizer removed", func() { Expect( - k8sClient.Delete(ctx, newPolicyServerFactory().withName(policyServerName).build()), + k8sClient.Delete(ctx, policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build()), ).To(Succeed()) Eventually(func() (*policiesv1.PolicyServer, error) { @@ -1036,7 +1051,7 @@ var _ = Describe("PolicyServer controller", func() { }, timeout, pollInterval).Should(Succeed()) Expect( - k8sClient.Delete(ctx, newPolicyServerFactory().withName(policyServerName).build()), + k8sClient.Delete(ctx, policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build()), ).To(Succeed()) Eventually(func() (*policiesv1.PolicyServer, error) { @@ -1053,7 +1068,10 @@ var _ = Describe("PolicyServer controller", func() { BeforeEach(func() { policyName = newName("policy") Expect( - k8sClient.Create(ctx, newClusterAdmissionPolicyFactory().withName(policyName).withPolicyServer(policyServerName).withMutating(false).build()), + k8sClient.Create(ctx, policiesv1.NewClusterAdmissionPolicyFactory(). + WithName(policyName). + WithPolicyServer(policyServerName). + Build()), ).To(Succeed()) Eventually(func() error { _, err := getTestClusterAdmissionPolicy(ctx, policyName) @@ -1068,7 +1086,7 @@ var _ = Describe("PolicyServer controller", func() { It("should delete assigned policies", func() { Expect( - k8sClient.Delete(ctx, newPolicyServerFactory().withName(policyServerName).build()), + k8sClient.Delete(ctx, policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build()), ).To(Succeed()) Eventually(func() (*policiesv1.ClusterAdmissionPolicy, error) { @@ -1099,7 +1117,7 @@ var _ = Describe("PolicyServer controller", func() { }, timeout, pollInterval).Should(Succeed()) Expect( - k8sClient.Delete(ctx, newPolicyServerFactory().withName(policyServerName).build()), + k8sClient.Delete(ctx, policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build()), ).To(Succeed()) Eventually(func() (*policiesv1.ClusterAdmissionPolicy, error) { @@ -1114,7 +1132,7 @@ var _ = Describe("PolicyServer controller", func() { It("should not delete its managed resources until all the scheduled policies are gone", func() { Expect( - k8sClient.Delete(ctx, newPolicyServerFactory().withName(policyServerName).build()), + k8sClient.Delete(ctx, policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build()), ).To(Succeed()) Eventually(func() (*policiesv1.ClusterAdmissionPolicy, error) { @@ -1142,7 +1160,7 @@ var _ = Describe("PolicyServer controller", func() { }).Should(Succeed()) Expect( - k8sClient.Delete(ctx, newPolicyServerFactory().withName(policyServerName).build()), + k8sClient.Delete(ctx, policiesv1.NewPolicyServerFactory().WithName(policyServerName).Build()), ).To(Succeed()) // wait for the reconciliation loop of the ClusterAdmissionPolicy to remove the resource diff --git a/internal/controller/utils_test.go b/internal/controller/utils_test.go index ad7cc98c..c6d9743b 100644 --- a/internal/controller/utils_test.go +++ b/internal/controller/utils_test.go @@ -1,3 +1,5 @@ +//go:build testing + /* Copyright 2022. From 0031697fb6d18f9b0e95ff6ba762de8a9c2ed95b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Guilherme=20Vanz?= Date: Fri, 10 Jan 2025 17:05:17 -0300 Subject: [PATCH 2/3] feat(tests): webhook integration tests. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds tests to ensure that the function used by the webhooks server is calling the correct validation functions. Signed-off-by: José Guilherme Vanz --- .../v1/admissionpolicy_webhook_test.go | 117 ++++++++++++++++++ .../v1/admissionpolicygroup_webhook_test.go | 117 ++++++++++++++++++ .../v1/clusteradmissionpolicy_webhook_test.go | 117 ++++++++++++++++++ ...lusteradmissionpolicygroup_webhook_test.go | 117 ++++++++++++++++++ api/policies/v1/factories.go | 24 ++++ 5 files changed, 492 insertions(+) diff --git a/api/policies/v1/admissionpolicy_webhook_test.go b/api/policies/v1/admissionpolicy_webhook_test.go index 00a89dbc..7a65940d 100644 --- a/api/policies/v1/admissionpolicy_webhook_test.go +++ b/api/policies/v1/admissionpolicy_webhook_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/stretchr/testify/require" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" "github.com/kubewarden/kubewarden-controller/internal/constants" ) @@ -44,6 +45,37 @@ func TestAdmissionPolicyValidateUpdate(t *testing.T) { warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.NoError(t, err) require.Empty(t, warnings) + + oldPolicy = NewAdmissionPolicyFactory(). + WithMode("monitor"). + Build() + newPolicy = NewAdmissionPolicyFactory(). + WithMode("protect"). + Build() + warnings, err = newPolicy.ValidateUpdate(oldPolicy) + require.NoError(t, err) + require.Empty(t, warnings) +} + +func TestInvalidAdmissionPolicyValidateUpdate(t *testing.T) { + oldPolicy := NewAdmissionPolicyFactory(). + WithPolicyServer("old"). + Build() + newPolicy := NewAdmissionPolicyFactory(). + WithPolicyServer("new"). + Build() + warnings, err := newPolicy.ValidateUpdate(oldPolicy) + require.Error(t, err) + require.Empty(t, warnings) + + newPolicy = NewAdmissionPolicyFactory(). + WithPolicyServer("new"). + WithMode("monitor"). + Build() + + warnings, err = newPolicy.ValidateUpdate(oldPolicy) + require.Error(t, err) + require.Empty(t, warnings) } func TestAdmissionPolicyValidateUpdateWithInvalidOldPolicy(t *testing.T) { @@ -53,3 +85,88 @@ func TestAdmissionPolicyValidateUpdateWithInvalidOldPolicy(t *testing.T) { require.Empty(t, warnings) require.ErrorContains(t, err, "object is not of type AdmissionPolicy") } + +func TestInvalidAdmissionPolicyCreation(t *testing.T) { + policy := NewAdmissionPolicyFactory(). + WithPolicyServer(""). + WithRules([]admissionregistrationv1.RuleWithOperations{ + {}, + { + Operations: []admissionregistrationv1.OperationType{}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }}, + { + Operations: nil, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{""}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{""}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{""}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"", "pods"}, + }, + }, + }). + WithMatchConditions([]admissionregistrationv1.MatchCondition{ + { + Name: "foo", + Expression: "1 + 1", + }, + { + Name: "foo", + Expression: "invalid expression", + }, + }). + Build() + warnings, err := policy.ValidateCreate() + require.Error(t, err) + require.Empty(t, warnings) +} diff --git a/api/policies/v1/admissionpolicygroup_webhook_test.go b/api/policies/v1/admissionpolicygroup_webhook_test.go index 9c34597c..014edc5a 100644 --- a/api/policies/v1/admissionpolicygroup_webhook_test.go +++ b/api/policies/v1/admissionpolicygroup_webhook_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/require" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" "github.com/kubewarden/kubewarden-controller/internal/constants" ) @@ -54,6 +55,37 @@ func TestAdmissionPolicyGroupValidateUpdate(t *testing.T) { warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.NoError(t, err) require.Empty(t, warnings) + + oldPolicy = NewAdmissionPolicyGroupFactory(). + WithMode("monitor"). + Build() + newPolicy = NewAdmissionPolicyGroupFactory(). + WithMode("protect"). + Build() + warnings, err = newPolicy.ValidateUpdate(oldPolicy) + require.NoError(t, err) + require.Empty(t, warnings) +} + +func TestInvalidAdmissionPolicyGroupValidateUpdate(t *testing.T) { + oldPolicy := NewAdmissionPolicyFactory(). + WithPolicyServer("old"). + Build() + newPolicy := NewAdmissionPolicyFactory(). + WithPolicyServer("new"). + Build() + warnings, err := newPolicy.ValidateUpdate(oldPolicy) + require.Error(t, err) + require.Empty(t, warnings) + + newPolicy = NewAdmissionPolicyFactory(). + WithPolicyServer("new"). + WithMode("monitor"). + Build() + + warnings, err = newPolicy.ValidateUpdate(oldPolicy) + require.Error(t, err) + require.Empty(t, warnings) } func TestAdmissionPolicyGroupValidateUpdateWithInvalidOldPolicy(t *testing.T) { @@ -63,3 +95,88 @@ func TestAdmissionPolicyGroupValidateUpdateWithInvalidOldPolicy(t *testing.T) { require.Empty(t, warnings) require.ErrorContains(t, err, "object is not of type AdmissionPolicyGroup") } + +func TestInvalidAdmissionPolicyGroupCreation(t *testing.T) { + policy := NewAdmissionPolicyGroupFactory(). + WithPolicyServer(""). + WithRules([]admissionregistrationv1.RuleWithOperations{ + {}, + { + Operations: []admissionregistrationv1.OperationType{}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }}, + { + Operations: nil, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{""}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{""}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{""}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"", "pods"}, + }, + }, + }). + WithMatchConditions([]admissionregistrationv1.MatchCondition{ + { + Name: "foo", + Expression: "1 + 1", + }, + { + Name: "foo", + Expression: "invalid expression", + }, + }). + Build() + warnings, err := policy.ValidateCreate() + require.Error(t, err) + require.Empty(t, warnings) +} diff --git a/api/policies/v1/clusteradmissionpolicy_webhook_test.go b/api/policies/v1/clusteradmissionpolicy_webhook_test.go index 4c623e4c..ec4c9820 100644 --- a/api/policies/v1/clusteradmissionpolicy_webhook_test.go +++ b/api/policies/v1/clusteradmissionpolicy_webhook_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/require" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" "github.com/kubewarden/kubewarden-controller/internal/constants" ) @@ -45,6 +46,37 @@ func TestClusterAdmissionPolicyValidateUpdate(t *testing.T) { warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.NoError(t, err) require.Empty(t, warnings) + + oldPolicy = NewClusterAdmissionPolicyFactory(). + WithMode("monitor"). + Build() + newPolicy = NewClusterAdmissionPolicyFactory(). + WithMode("protect"). + Build() + warnings, err = newPolicy.ValidateUpdate(oldPolicy) + require.NoError(t, err) + require.Empty(t, warnings) +} + +func TestInvalidClusterAdmissionPolicyValidateUpdate(t *testing.T) { + oldPolicy := NewClusterAdmissionPolicyFactory(). + WithPolicyServer("old"). + Build() + newPolicy := NewClusterAdmissionPolicyFactory(). + WithPolicyServer("new"). + Build() + warnings, err := newPolicy.ValidateUpdate(oldPolicy) + require.Error(t, err) + require.Empty(t, warnings) + + newPolicy = NewClusterAdmissionPolicyFactory(). + WithPolicyServer("new"). + WithMode("monitor"). + Build() + + warnings, err = newPolicy.ValidateUpdate(oldPolicy) + require.Error(t, err) + require.Empty(t, warnings) } func TestClusterAdmissionPolicyValidateUpdateWithInvalidOldPolicy(t *testing.T) { @@ -54,3 +86,88 @@ func TestClusterAdmissionPolicyValidateUpdateWithInvalidOldPolicy(t *testing.T) require.Empty(t, warnings) require.ErrorContains(t, err, "object is not of type ClusterAdmissionPolicy") } + +func TestInvalidClusterAdmissionPolicyCreation(t *testing.T) { + policy := NewClusterAdmissionPolicyFactory(). + WithPolicyServer(""). + WithRules([]admissionregistrationv1.RuleWithOperations{ + {}, + { + Operations: []admissionregistrationv1.OperationType{}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }}, + { + Operations: nil, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{""}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{""}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{""}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"", "pods"}, + }, + }, + }). + WithMatchConditions([]admissionregistrationv1.MatchCondition{ + { + Name: "foo", + Expression: "1 + 1", + }, + { + Name: "foo", + Expression: "invalid expression", + }, + }). + Build() + warnings, err := policy.ValidateCreate() + require.Error(t, err) + require.Empty(t, warnings) +} diff --git a/api/policies/v1/clusteradmissionpolicygroup_webhook_test.go b/api/policies/v1/clusteradmissionpolicygroup_webhook_test.go index 9851b666..a1faa20e 100644 --- a/api/policies/v1/clusteradmissionpolicygroup_webhook_test.go +++ b/api/policies/v1/clusteradmissionpolicygroup_webhook_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/require" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" "github.com/kubewarden/kubewarden-controller/internal/constants" ) @@ -54,6 +55,37 @@ func TestClusterClusterAdmissionPolicyValidateUpdate(t *testing.T) { warnings, err := newPolicy.ValidateUpdate(oldPolicy) require.NoError(t, err) require.Empty(t, warnings) + + oldPolicy = NewClusterAdmissionPolicyGroupFactory(). + WithMode("monitor"). + Build() + newPolicy = NewClusterAdmissionPolicyGroupFactory(). + WithMode("protect"). + Build() + warnings, err = newPolicy.ValidateUpdate(oldPolicy) + require.NoError(t, err) + require.Empty(t, warnings) +} + +func TestInvalidClusterAdmissionPolicyGroupValidateUpdate(t *testing.T) { + oldPolicy := NewClusterAdmissionPolicyFactory(). + WithPolicyServer("old"). + Build() + newPolicy := NewClusterAdmissionPolicyFactory(). + WithPolicyServer("new"). + Build() + warnings, err := newPolicy.ValidateUpdate(oldPolicy) + require.Error(t, err) + require.Empty(t, warnings) + + newPolicy = NewClusterAdmissionPolicyFactory(). + WithPolicyServer("new"). + WithMode("monitor"). + Build() + + warnings, err = newPolicy.ValidateUpdate(oldPolicy) + require.Error(t, err) + require.Empty(t, warnings) } func TestClusterClusterAdmissionPolicyValidateUpdateWithInvalidOldPolicy(t *testing.T) { @@ -63,3 +95,88 @@ func TestClusterClusterAdmissionPolicyValidateUpdateWithInvalidOldPolicy(t *test require.Empty(t, warnings) require.ErrorContains(t, err, "object is not of type ClusterAdmissionPolicyGroup") } + +func TestInvalidClusterAdmissionPolicyGroupCreation(t *testing.T) { + policy := NewClusterAdmissionPolicyGroupFactory(). + WithPolicyServer(""). + WithRules([]admissionregistrationv1.RuleWithOperations{ + {}, + { + Operations: []admissionregistrationv1.OperationType{}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }}, + { + Operations: nil, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{""}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{""}, + Resources: []string{"*/*"}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{"*"}, + APIVersions: []string{"*"}, + Resources: []string{""}, + }, + }, + { + Operations: []admissionregistrationv1.OperationType{admissionregistrationv1.OperationAll}, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"", "pods"}, + }, + }, + }). + WithMatchConditions([]admissionregistrationv1.MatchCondition{ + { + Name: "foo", + Expression: "1 + 1", + }, + { + Name: "foo", + Expression: "invalid expression", + }, + }). + Build() + warnings, err := policy.ValidateCreate() + require.Error(t, err) + require.Empty(t, warnings) +} diff --git a/api/policies/v1/factories.go b/api/policies/v1/factories.go index f7a64245..d5a6bee2 100644 --- a/api/policies/v1/factories.go +++ b/api/policies/v1/factories.go @@ -27,6 +27,7 @@ type AdmissionPolicyFactory struct { rules []admissionregistrationv1.RuleWithOperations module string matchConds []admissionregistrationv1.MatchCondition + mode PolicyMode } func NewAdmissionPolicyFactory() *AdmissionPolicyFactory { @@ -52,6 +53,7 @@ func NewAdmissionPolicyFactory() *AdmissionPolicyFactory { matchConds: []admissionregistrationv1.MatchCondition{ {Name: "noop", Expression: "true"}, }, + mode: "protect", } } @@ -85,6 +87,11 @@ func (fac *AdmissionPolicyFactory) WithMatchConditions(matchConditions []admissi return fac } +func (fac *AdmissionPolicyFactory) WithMode(mode PolicyMode) *AdmissionPolicyFactory { + fac.mode = mode + return fac +} + func (fac *AdmissionPolicyFactory) Build() *AdmissionPolicy { policy := AdmissionPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -107,6 +114,7 @@ func (fac *AdmissionPolicyFactory) Build() *AdmissionPolicy { Rules: fac.rules, Mutating: fac.mutating, MatchConditions: fac.matchConds, + Mode: fac.mode, }, }, } @@ -259,6 +267,7 @@ type AdmissionPolicyGroupFactory struct { expression string policyMembers PolicyGroupMembers matchConds []admissionregistrationv1.MatchCondition + mode PolicyMode } func NewAdmissionPolicyGroupFactory() *AdmissionPolicyGroupFactory { @@ -288,6 +297,7 @@ func NewAdmissionPolicyGroupFactory() *AdmissionPolicyGroupFactory { matchConds: []admissionregistrationv1.MatchCondition{ {Name: "noop", Expression: "true"}, }, + mode: "protect", } } @@ -316,6 +326,11 @@ func (fac *AdmissionPolicyGroupFactory) WithMatchConditions(matchContions []admi return fac } +func (fac *AdmissionPolicyGroupFactory) WithMode(mode PolicyMode) *AdmissionPolicyGroupFactory { + fac.mode = mode + return fac +} + func (fac *AdmissionPolicyGroupFactory) Build() *AdmissionPolicyGroup { return &AdmissionPolicyGroup{ ObjectMeta: metav1.ObjectMeta{ @@ -338,6 +353,7 @@ func (fac *AdmissionPolicyGroupFactory) Build() *AdmissionPolicyGroup { Expression: fac.expression, Rules: fac.rules, MatchConditions: fac.matchConds, + Mode: fac.mode, }, }, } @@ -350,6 +366,7 @@ type ClusterAdmissionPolicyGroupFactory struct { expression string policyMembers PolicyGroupMembers matchConds []admissionregistrationv1.MatchCondition + mode PolicyMode } func NewClusterAdmissionPolicyGroupFactory() *ClusterAdmissionPolicyGroupFactory { @@ -381,6 +398,7 @@ func NewClusterAdmissionPolicyGroupFactory() *ClusterAdmissionPolicyGroupFactory matchConds: []admissionregistrationv1.MatchCondition{ {Name: "noop", Expression: "true"}, }, + mode: "protect", } } @@ -409,6 +427,11 @@ func (fac *ClusterAdmissionPolicyGroupFactory) WithMatchConditions(matchConditio return fac } +func (fac *ClusterAdmissionPolicyGroupFactory) WithMode(mode PolicyMode) *ClusterAdmissionPolicyGroupFactory { + fac.mode = mode + return fac +} + func (fac *ClusterAdmissionPolicyGroupFactory) Build() *ClusterAdmissionPolicyGroup { clusterAdmissionPolicy := ClusterAdmissionPolicyGroup{ ObjectMeta: metav1.ObjectMeta{ @@ -430,6 +453,7 @@ func (fac *ClusterAdmissionPolicyGroupFactory) Build() *ClusterAdmissionPolicyGr Expression: fac.expression, Rules: fac.rules, MatchConditions: fac.matchConds, + Mode: fac.mode, }, }, } From f46ce66f5571d1cce621efea659068cddaeacd67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Guilherme=20Vanz?= Date: Fri, 10 Jan 2025 18:54:59 -0300 Subject: [PATCH 3/3] chore: cleans up policy MatchConditions validation code. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the code from policy_validation_matchconditions.go file into policy_validation.go file, removing the unnecessary code. Signed-off-by: José Guilherme Vanz --- .golangci.yml | 6 - api/policies/v1/policy_validation.go | 114 ++++++++++---- .../v1/policy_validation_matchconditions.go | 140 ------------------ api/policies/v1/policy_validation_test.go | 3 +- 4 files changed, 90 insertions(+), 173 deletions(-) delete mode 100644 api/policies/v1/policy_validation_matchconditions.go diff --git a/.golangci.yml b/.golangci.yml index c291b40f..f4f35aa7 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -373,9 +373,3 @@ issues: - path: 'api/policies/v1/(.+)_webhook.go' linters: - dupl - - path: "api/policies/v1/policy_validation_matchconditions.go" - # this file is imported from k8s.io/kubernetes, ignore lint errors - linters: - - errorlint - - gochecknoglobals - - mnd diff --git a/api/policies/v1/policy_validation.go b/api/policies/v1/policy_validation.go index a155eb51..fc8a9b01 100644 --- a/api/policies/v1/policy_validation.go +++ b/api/policies/v1/policy_validation.go @@ -17,19 +17,34 @@ limitations under the License. package v1 import ( - admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + "fmt" + "strings" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" + plugincel "k8s.io/apiserver/pkg/admission/plugin/cel" + "k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions" + "k8s.io/apiserver/pkg/cel" + "k8s.io/apiserver/pkg/cel/environment" +) - apierrors "k8s.io/apimachinery/pkg/api/errors" +// nonStrictStatelessCELCompiler is a cel Compiler that does not enforce strict cost enforcement. +// +//nolint:gochecknoglobals // lets keep the compiler available for the how module +var ( + nonStrictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), false)) ) +const maxMatchConditionsCount = 64 + func validatePolicyCreate(policy Policy) field.ErrorList { var allErrors field.ErrorList allErrors = append(allErrors, validateRulesField(policy)...) - allErrors = append(allErrors, validateMatchConditionsField(policy)...) - + allErrors = append(allErrors, validateMatchConditions(policy.GetMatchConditions(), field.NewPath("spec").Child("matchConditions"))...) return allErrors } @@ -37,7 +52,7 @@ func validatePolicyUpdate(oldPolicy, newPolicy Policy) field.ErrorList { var allErrors field.ErrorList allErrors = append(allErrors, validateRulesField(newPolicy)...) - allErrors = append(allErrors, validateMatchConditionsField(newPolicy)...) + allErrors = append(allErrors, validateMatchConditions(newPolicy.GetMatchConditions(), field.NewPath("spec").Child("matchConditions"))...) if err := validatePolicyServerField(oldPolicy, newPolicy); err != nil { allErrors = append(allErrors, err) } @@ -104,27 +119,6 @@ func checkRulesArrayForEmptyString(rulesArray []string, rulesField *field.Path) return allErrors } -func validateMatchConditionsField(policy Policy) field.ErrorList { - // taken from the configuration for validating MutatingWebhookConfiguration: - // https://github.com/kubernetes/kubernetes/blob/c052f64689ee26aace4689f2433c5c7dcf1931ad/pkg/apis/admissionregistration/validation/validation.go#L257 - opts := validationOptions{ - ignoreMatchConditions: false, - allowParamsInMatchConditions: false, - requireNoSideEffects: true, - requireRecognizedAdmissionReviewVersion: true, - requireUniqueWebhookNames: true, - allowInvalidLabelValueInSelector: false, - // strictCostEnforcement enables cost enforcement for CEL. - // This is enabled with the StrictCostEnforcementForWebhooks feature gate - // (alpha on v1.30). Don't check it for now. Nevertheless, will get - // checked by the k8s API on WebhookConfiguration creation if the feature - // gate is enabled. - strictCostEnforcement: false, - } - - return validateMatchConditions(policy.GetMatchConditions(), opts, field.NewPath("spec").Child("matchConditions")) -} - func validatePolicyServerField(oldPolicy, newPolicy Policy) *field.Error { if oldPolicy.GetPolicyServer() != newPolicy.GetPolicyServer() { return field.Forbidden(field.NewPath("spec").Child("policyServer"), "the field is immutable") @@ -149,3 +143,71 @@ func prepareInvalidAPIError(policy Policy, errorList field.ErrorList) *apierrors errorList, ) } + +func validateMatchConditions(m []admissionregistrationv1.MatchCondition, fldPath *field.Path) field.ErrorList { + var allErrors field.ErrorList + conditionNames := sets.NewString() + if len(m) > maxMatchConditionsCount { + allErrors = append(allErrors, field.TooMany(fldPath, len(m), maxMatchConditionsCount)) + } + for i, matchCondition := range m { + allErrors = append(allErrors, validateMatchCondition(&matchCondition, fldPath.Index(i))...) + if len(matchCondition.Name) > 0 { + if conditionNames.Has(matchCondition.Name) { + allErrors = append(allErrors, field.Duplicate(fldPath.Index(i).Child("name"), matchCondition.Name)) + } else { + conditionNames.Insert(matchCondition.Name) + } + } + } + return allErrors +} + +func validateMatchCondition(v *admissionregistrationv1.MatchCondition, fldPath *field.Path) field.ErrorList { + var allErrors field.ErrorList + trimmedExpression := strings.TrimSpace(v.Expression) + if len(trimmedExpression) == 0 { + allErrors = append(allErrors, field.Required(fldPath.Child("expression"), "")) + } else { + allErrors = append(allErrors, validateMatchConditionsExpression(trimmedExpression, fldPath.Child("expression"))...) + } + if len(v.Name) == 0 { + allErrors = append(allErrors, field.Required(fldPath.Child("name"), "")) + } else { + for _, msg := range validation.IsQualifiedName(v.Name) { + allErrors = append(allErrors, field.Invalid(fldPath, v.Name, msg)) + } + } + return allErrors +} + +func convertCELErrorToValidationError(fldPath *field.Path, expression plugincel.ExpressionAccessor, err error) *field.Error { + //nolint:errorlint // The code is not only checking the type. It is also using the errors fields. + if celErr, ok := err.(*cel.Error); ok { + switch celErr.Type { + case cel.ErrorTypeRequired: + return field.Required(fldPath, celErr.Detail) + case cel.ErrorTypeInvalid: + return field.Invalid(fldPath, expression.GetExpression(), celErr.Detail) + case cel.ErrorTypeInternal: + return field.InternalError(fldPath, celErr) + } + } + return field.InternalError(fldPath, fmt.Errorf("unsupported error type: %w", err)) +} + +func validateMatchConditionsExpression(expressionStr string, fldPath *field.Path) field.ErrorList { + var allErrors field.ErrorList + expression := &matchconditions.MatchCondition{ + Expression: expressionStr, + } + result := nonStrictStatelessCELCompiler.CompileCELExpression(expression, plugincel.OptionalVariableDeclarations{ + HasParams: false, + HasAuthorizer: true, + StrictCost: false, + }, environment.NewExpressions) + if result.Error != nil { + allErrors = append(allErrors, convertCELErrorToValidationError(fldPath, expression, result.Error)) + } + return allErrors +} diff --git a/api/policies/v1/policy_validation_matchconditions.go b/api/policies/v1/policy_validation_matchconditions.go deleted file mode 100644 index 0314563c..00000000 --- a/api/policies/v1/policy_validation_matchconditions.go +++ /dev/null @@ -1,140 +0,0 @@ -package v1 - -/* -The following code is taken from the Kubernetes source code -at pkg/apis/admissionregistration/validation/validation.go -*/ - -/* -Copyright 2017 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. -*/ - -import ( - "fmt" - "strings" - - admissionregistrationv1 "k8s.io/api/admissionregistration/v1" - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/validation" - "k8s.io/apimachinery/pkg/util/validation/field" - plugincel "k8s.io/apiserver/pkg/admission/plugin/cel" - "k8s.io/apiserver/pkg/admission/plugin/webhook/matchconditions" - "k8s.io/apiserver/pkg/cel" - "k8s.io/apiserver/pkg/cel/environment" -) - -type validationOptions struct { - ignoreMatchConditions bool - allowParamsInMatchConditions bool - requireNoSideEffects bool - requireRecognizedAdmissionReviewVersion bool - requireUniqueWebhookNames bool - allowInvalidLabelValueInSelector bool - preexistingExpressions preexistingExpressions - strictCostEnforcement bool -} - -type preexistingExpressions struct { - matchConditionExpressions sets.Set[string] -} - -func validateMatchConditions(m []admissionregistrationv1.MatchCondition, opts validationOptions, fldPath *field.Path) field.ErrorList { - var allErrors field.ErrorList - conditionNames := sets.NewString() - if len(m) > 64 { - allErrors = append(allErrors, field.TooMany(fldPath, len(m), 64)) - } - for i, matchCondition := range m { - allErrors = append(allErrors, validateMatchCondition(&matchCondition, opts, fldPath.Index(i))...) - if len(matchCondition.Name) > 0 { - if conditionNames.Has(matchCondition.Name) { - allErrors = append(allErrors, field.Duplicate(fldPath.Index(i).Child("name"), matchCondition.Name)) - } else { - conditionNames.Insert(matchCondition.Name) - } - } - } - return allErrors -} - -func validateMatchCondition(v *admissionregistrationv1.MatchCondition, opts validationOptions, fldPath *field.Path) field.ErrorList { - var allErrors field.ErrorList - trimmedExpression := strings.TrimSpace(v.Expression) - if len(trimmedExpression) == 0 { - allErrors = append(allErrors, field.Required(fldPath.Child("expression"), "")) - } else { - allErrors = append(allErrors, validateMatchConditionsExpression(trimmedExpression, opts, fldPath.Child("expression"))...) - } - if len(v.Name) == 0 { - allErrors = append(allErrors, field.Required(fldPath.Child("name"), "")) - } else { - for _, msg := range validation.IsQualifiedName(v.Name) { - allErrors = append(allErrors, field.Invalid(fldPath, v.Name, msg)) - } - } - return allErrors -} - -func validateCELCondition(compiler plugincel.Compiler, expression plugincel.ExpressionAccessor, variables plugincel.OptionalVariableDeclarations, envType environment.Type, fldPath *field.Path) field.ErrorList { - var allErrors field.ErrorList - result := compiler.CompileCELExpression(expression, variables, envType) - if result.Error != nil { - allErrors = append(allErrors, convertCELErrorToValidationError(fldPath, expression, result.Error)) - } - return allErrors -} - -func convertCELErrorToValidationError(fldPath *field.Path, expression plugincel.ExpressionAccessor, err error) *field.Error { - if celErr, ok := err.(*cel.Error); ok { - switch celErr.Type { - case cel.ErrorTypeRequired: - return field.Required(fldPath, celErr.Detail) - case cel.ErrorTypeInvalid: - return field.Invalid(fldPath, expression.GetExpression(), celErr.Detail) - case cel.ErrorTypeInternal: - return field.InternalError(fldPath, celErr) - } - } - return field.InternalError(fldPath, fmt.Errorf("unsupported error type: %w", err)) -} - -func validateMatchConditionsExpression(expression string, opts validationOptions, fldPath *field.Path) field.ErrorList { - envType := environment.NewExpressions - if opts.preexistingExpressions.matchConditionExpressions.Has(expression) { - envType = environment.StoredExpressions - } - var compiler plugincel.Compiler - if opts.strictCostEnforcement { - compiler = strictStatelessCELCompiler - } else { - compiler = nonStrictStatelessCELCompiler - } - return validateCELCondition(compiler, &matchconditions.MatchCondition{ - Expression: expression, - }, plugincel.OptionalVariableDeclarations{ - HasParams: opts.allowParamsInMatchConditions, - HasAuthorizer: true, - StrictCost: opts.strictCostEnforcement, - }, envType, fldPath) -} - -// statelessCELCompiler does not support variable composition (and thus is stateless). It should be used when -// variable composition is not allowed, for example, when validating MatchConditions. -// strictStatelessCELCompiler is a cel Compiler that enforces strict cost enforcement. -// nonStrictStatelessCELCompiler is a cel Compiler that does not enforce strict cost enforcement. -var ( - strictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true)) - nonStrictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), false)) -) diff --git a/api/policies/v1/policy_validation_test.go b/api/policies/v1/policy_validation_test.go index 64f06ac5..b16ef809 100644 --- a/api/policies/v1/policy_validation_test.go +++ b/api/policies/v1/policy_validation_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/require" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + "k8s.io/apimachinery/pkg/util/validation/field" ) func TestValidateRulesField(t *testing.T) { @@ -282,7 +283,7 @@ func TestValidateMatchConditionsField(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - allErrors := validateMatchConditionsField(test.policy) + allErrors := validateMatchConditions(test.policy.GetMatchConditions(), field.NewPath("spec").Child("matchConditions")) if test.expectedErrorMessage != "" { err := prepareInvalidAPIError(test.policy, allErrors)