Webhook integration tests and clean up MatchCondition validation code

.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

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

api/policies/v1/admissionpolicy_webhook_test.go

@@ -1,54 +1,172 @@
+//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,
 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 (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 
 	"github.com/kubewarden/kubewarden-controller/internal/constants"
 )
 
 func TestAdmissionPolicyDefault(t *testing.T) {
-	policy := admissionPolicyFactory()
+	policy := AdmissionPolicy{}
 	policy.Default()
 
 	require.Equal(t, constants.DefaultPolicyServer, policy.GetPolicyServer())
 	require.Contains(t, policy.GetFinalizers(), constants.KubewardenFinalizer)
 }
 
 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)
+
+	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) {
-	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")
 }
+
+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)
+}

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.
@@ -18,27 +20,28 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
 
 	"github.com/kubewarden/kubewarden-controller/internal/constants"
 )
 
 func TestAdmissionPolicyGroupDefault(t *testing.T) {
-	policy := admissionPolicyGroupFactory()
+	policy := AdmissionPolicyGroup{}
 	policy.Default()
 
 	require.Equal(t, constants.DefaultPolicyServer, policy.GetPolicyServer())
 	require.Contains(t, policy.GetFinalizers(), constants.KubewardenFinalizer)
 }
 
 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,17 +50,133 @@ 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)
+
+	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) {
-	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")
 }
+
+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)
+}