diff --git a/api/configuration/v1alpha1/kongroute_types.go b/api/configuration/v1alpha1/kongroute_types.go index 4461e58..e86b0e2 100644 --- a/api/configuration/v1alpha1/kongroute_types.go +++ b/api/configuration/v1alpha1/kongroute_types.go @@ -37,8 +37,6 @@ import ( // +kubebuilder:printcolumn:name="Programmed",description="The Resource is Programmed on Konnect",type=string,JSONPath=`.status.conditions[?(@.type=='Programmed')].status` // +kubebuilder:validation:XValidation:rule="!has(oldSelf.spec.konnect.authRef) || has(self.spec.konnect.authRef)", message="Konnect Configuration's API auth ref reference is required once set" // +kubebuilder:validation:XValidation:rule="self.spec.protocols.exists(p, p == 'http') ? (has(self.spec.hosts) || has(self.spec.methods) || has(self.spec.paths) || has(self.spec.paths) || has(self.spec.paths) || has(self.spec.headers) ) : true", message="If protocols has 'http', at least one of 'hosts', 'methods', 'paths' or 'headers' must be set" -// +kubebuilder:validation:XValidation:rule="!self.status.conditions.exists(c, c.type == 'Programmed' && c.status == 'True') ? true : self.spec.konnect.authRef == oldSelf.spec.konnect.authRef", message="spec.konnect.authRef is immutable when entity is already Programmed." -// +kubebuilder:validation:XValidation:rule="!self.status.conditions.exists(c, c.type == 'APIAuthValid' && c.status == 'True') ? true : self.spec.konnect.authRef == oldSelf.spec.konnect.authRef", message="spec.konnect.authRef is immutable when entity refers to a Valid API Auth Configuration." type KongRoute struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/config/crd/bases/configuration.konghq.com_kongroutes.yaml b/config/crd/bases/configuration.konghq.com_kongroutes.yaml index 85145ac..b17d9eb 100644 --- a/config/crd/bases/configuration.konghq.com_kongroutes.yaml +++ b/config/crd/bases/configuration.konghq.com_kongroutes.yaml @@ -361,13 +361,6 @@ spec: rule: 'self.spec.protocols.exists(p, p == ''http'') ? (has(self.spec.hosts) || has(self.spec.methods) || has(self.spec.paths) || has(self.spec.paths) || has(self.spec.paths) || has(self.spec.headers) ) : true' - - message: spec.konnect.authRef is immutable when entity is already Programmed. - rule: '!self.status.conditions.exists(c, c.type == ''Programmed'' && c.status - == ''True'') ? true : self.spec.konnect.authRef == oldSelf.spec.konnect.authRef' - - message: spec.konnect.authRef is immutable when entity refers to a Valid - API Auth Configuration. - rule: '!self.status.conditions.exists(c, c.type == ''APIAuthValid'' && c.status - == ''True'') ? true : self.spec.konnect.authRef == oldSelf.spec.konnect.authRef' served: true storage: true subresources: diff --git a/go.mod b/go.mod index 8e4b478..68ccd23 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,8 @@ require ( sigs.k8s.io/gateway-api v1.1.0 ) +require github.com/evanphx/json-patch/v5 v5.9.0 // indirect + require ( github.com/cenkalti/backoff/v4 v4.3.0 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect diff --git a/go.sum b/go.sum index 465caf3..83470c6 100644 --- a/go.sum +++ b/go.sum @@ -12,6 +12,8 @@ github.com/ericlagergren/decimal v0.0.0-20240411145413-00de7ca16731 h1:R/ZjJpjQK github.com/ericlagergren/decimal v0.0.0-20240411145413-00de7ca16731/go.mod h1:M9R1FoZ3y//hwwnJtO51ypFGwm8ZfpxPT/ZLtO1mcgQ= github.com/evanphx/json-patch v5.9.0+incompatible h1:fBXyNpNMuTTDdquAq/uisOr2lShz4oaXpDTX2bLe7ls= github.com/evanphx/json-patch v5.9.0+incompatible/go.mod h1:50XU6AFN0ol/bzJsmQLiYLvXMP4fmwYFNcr97nuDLSk= +github.com/evanphx/json-patch/v5 v5.9.0 h1:kcBlZQbplgElYIlo/n1hJbls2z/1awpXxpRi0/FOJfg= +github.com/evanphx/json-patch/v5 v5.9.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq14uClGH4abBuQ= github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/zapr v1.3.0 h1:XGdV8XW8zdwFiwOA2Dryh1gj2KRQyOOoNmBy4EplIcQ= diff --git a/test/crdsvalidation/kongpluginbindings/kongpluginbindings_test.go b/test/crdsvalidation/kongpluginbindings/kongpluginbindings_test.go index ebc1881..e235cce 100644 --- a/test/crdsvalidation/kongpluginbindings/kongpluginbindings_test.go +++ b/test/crdsvalidation/kongpluginbindings/kongpluginbindings_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/config" configurationv1alpha1client "github.com/kong/kubernetes-configuration/pkg/clientset/typed/configuration/v1alpha1" @@ -26,10 +27,13 @@ func TestKongPluginBindings(t *testing.T) { for _, tc := range tcsGroup.TestCases { tc := tc t.Run(tc.Name, func(t *testing.T) { - kpb, err := cl.KongPluginBindings(tc.KongPluginBinding.Namespace).Create(ctx, &tc.KongPluginBinding, metav1.CreateOptions{}) + cl := cl.KongPluginBindings(tc.KongPluginBinding.Namespace) + kpb, err := cl.Create(ctx, &tc.KongPluginBinding, metav1.CreateOptions{}) if tc.ExpectedErrorMessage == nil { assert.NoError(t, err) - assert.NoError(t, cl.KongPluginBindings(kpb.Namespace).Delete(ctx, kpb.Name, metav1.DeleteOptions{})) + t.Cleanup(func() { + assert.NoError(t, client.IgnoreNotFound(cl.Delete(ctx, kpb.Name, metav1.DeleteOptions{}))) + }) } else { require.NotNil(t, err) assert.Contains(t, err.Error(), *tc.ExpectedErrorMessage) diff --git a/test/crdsvalidation/konnectcontrolplane/konnectcontrolplane_test.go b/test/crdsvalidation/konnectcontrolplane/konnectcontrolplane_test.go new file mode 100644 index 0000000..77b7021 --- /dev/null +++ b/test/crdsvalidation/konnectcontrolplane/konnectcontrolplane_test.go @@ -0,0 +1,56 @@ +package kongpluginbindings + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/config" + + konnectv1alpha1client "github.com/kong/kubernetes-configuration/pkg/clientset/typed/konnect/v1alpha1" + "github.com/kong/kubernetes-configuration/test/crdsvalidation/konnectcontrolplane/testcases" +) + +func TestKonnectControlPlane(t *testing.T) { + ctx := context.Background() + cfg, err := config.GetConfig() + require.NoError(t, err, "error loading Kubernetes config") + cl, err := konnectv1alpha1client.NewForConfig(cfg) + require.NoError(t, err, "error creating konnectv1alpha1client client") + + for _, tcsGroup := range testcases.TestCases { + tcsGroup := tcsGroup + t.Run(tcsGroup.Name, func(t *testing.T) { + for _, tc := range tcsGroup.TestCases { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + cl := cl.KonnectControlPlanes(tc.KonnectControlPlane.Namespace) + kcp, err := cl.Create(ctx, &tc.KonnectControlPlane, metav1.CreateOptions{}) + if tc.ExpectedErrorMessage == nil { + assert.NoError(t, err) + t.Cleanup(func() { + assert.NoError(t, client.IgnoreNotFound(cl.Delete(ctx, kcp.Name, metav1.DeleteOptions{}))) + }) + + // Update the object and check if the update is allowed. + if tc.Update != nil { + tc.Update(kcp) + + _, err := cl.Update(ctx, kcp, metav1.UpdateOptions{}) + if tc.ExpectedUpdateErrorMessage != nil { + require.NotNil(t, err) + assert.Contains(t, err.Error(), *tc.ExpectedUpdateErrorMessage) + } + } + } else { + require.NotNil(t, err) + assert.Contains(t, err.Error(), *tc.ExpectedErrorMessage) + } + }) + } + }) + } +} diff --git a/test/crdsvalidation/konnectcontrolplane/testcases/common.go b/test/crdsvalidation/konnectcontrolplane/testcases/common.go new file mode 100644 index 0000000..ab8b9db --- /dev/null +++ b/test/crdsvalidation/konnectcontrolplane/testcases/common.go @@ -0,0 +1,37 @@ +package testcases + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + konnectv1alpha1 "github.com/kong/kubernetes-configuration/api/konnect/v1alpha1" +) + +// kcpTestCase is a test case related to KonnectControlPlane validation. +type kcpTestCase struct { + Name string + KonnectControlPlane konnectv1alpha1.KonnectControlPlane + Update func(*konnectv1alpha1.KonnectControlPlane) + ExpectedErrorMessage *string + ExpectedUpdateErrorMessage *string +} + +// kcpTestCasesGroup is a group of test cases related to KonnectControlPlane validation. +// The grouping is done by a common name. +type kcpTestCasesGroup struct { + Name string + TestCases []kcpTestCase +} + +// TestCases is a collection of all test cases groups related to KonnectControlPlane validation. +var TestCases = []kcpTestCasesGroup{} + +func init() { + TestCases = append(TestCases, + updatesNotAllowedForStatus, + ) +} + +var commonObjectMeta = metav1.ObjectMeta{ + GenerateName: "test-konnect-control-plane", + Namespace: "default", +} diff --git a/test/crdsvalidation/konnectcontrolplane/testcases/update-not-allowed-for-status.go b/test/crdsvalidation/konnectcontrolplane/testcases/update-not-allowed-for-status.go new file mode 100644 index 0000000..b03ad19 --- /dev/null +++ b/test/crdsvalidation/konnectcontrolplane/testcases/update-not-allowed-for-status.go @@ -0,0 +1,107 @@ +package testcases + +import ( + "github.com/Kong/sdk-konnect-go/models/components" + "github.com/samber/lo" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + konnectv1alpha1 "github.com/kong/kubernetes-configuration/api/konnect/v1alpha1" +) + +// updatesNotAllowedForStatus are test cases checking if updates to konnect.authRef +// are indeed blocked for some status conditions. +var updatesNotAllowedForStatus = kcpTestCasesGroup{ + Name: "updates not allowed for status conditions", + TestCases: []kcpTestCase{ + { + Name: "konnect.authRef change is not allowed for Programmed=True", + KonnectControlPlane: konnectv1alpha1.KonnectControlPlane{ + ObjectMeta: commonObjectMeta, + Spec: konnectv1alpha1.KonnectControlPlaneSpec{ + CreateControlPlaneRequest: components.CreateControlPlaneRequest{ + Name: "cp-1", + ClusterType: lo.ToPtr(components.ClusterTypeClusterTypeControlPlane), + }, + KonnectConfiguration: konnectv1alpha1.KonnectConfiguration{ + APIAuthConfigurationRef: konnectv1alpha1.KonnectAPIAuthConfigurationRef{ + Name: "name-1", + }, + }, + }, + Status: konnectv1alpha1.KonnectControlPlaneStatus{ + Conditions: []metav1.Condition{ + { + Type: "Programmed", + Status: metav1.ConditionTrue, + }, + }, + }, + }, + Update: func(kcp *konnectv1alpha1.KonnectControlPlane) { + kcp.Spec.KonnectConfiguration.APIAuthConfigurationRef.Name = "name-2" + }, + ExpectedUpdateErrorMessage: lo.ToPtr("spec.konnect.authRef is immutable when entity is already Programme"), + }, + { + Name: "konnect.authRef change is not allowed for APIAuthValid=True", + KonnectControlPlane: konnectv1alpha1.KonnectControlPlane{ + ObjectMeta: commonObjectMeta, + Spec: konnectv1alpha1.KonnectControlPlaneSpec{ + CreateControlPlaneRequest: components.CreateControlPlaneRequest{ + Name: "cp-1", + ClusterType: lo.ToPtr(components.ClusterTypeClusterTypeControlPlane), + }, + KonnectConfiguration: konnectv1alpha1.KonnectConfiguration{ + APIAuthConfigurationRef: konnectv1alpha1.KonnectAPIAuthConfigurationRef{ + Name: "name-1", + }, + }, + }, + Status: konnectv1alpha1.KonnectControlPlaneStatus{ + Conditions: []metav1.Condition{ + { + Type: "APIAuthValid", + Status: metav1.ConditionTrue, + }, + }, + }, + }, + Update: func(kcp *konnectv1alpha1.KonnectControlPlane) { + kcp.Spec.KonnectConfiguration.APIAuthConfigurationRef.Name = "name-2" + }, + ExpectedUpdateErrorMessage: lo.ToPtr("spec.konnect.authRef is immutable when entity is already Programme"), + }, + { + Name: "konnect.authRef change is allowed when cp is not Programmed=True nor APIAuthValid=True", + KonnectControlPlane: konnectv1alpha1.KonnectControlPlane{ + ObjectMeta: commonObjectMeta, + Spec: konnectv1alpha1.KonnectControlPlaneSpec{ + CreateControlPlaneRequest: components.CreateControlPlaneRequest{ + Name: "cp-1", + ClusterType: lo.ToPtr(components.ClusterTypeClusterTypeControlPlane), + }, + KonnectConfiguration: konnectv1alpha1.KonnectConfiguration{ + APIAuthConfigurationRef: konnectv1alpha1.KonnectAPIAuthConfigurationRef{ + Name: "name-1", + }, + }, + }, + Status: konnectv1alpha1.KonnectControlPlaneStatus{ + Conditions: []metav1.Condition{ + { + Type: "APIAuthValid", + Status: metav1.ConditionFalse, + }, + { + Type: "Programmed", + Status: metav1.ConditionFalse, + }, + }, + }, + }, + Update: func(kcp *konnectv1alpha1.KonnectControlPlane) { + kcp.Spec.KonnectConfiguration.APIAuthConfigurationRef.Name = "name-2" + }, + }, + }, +}