Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: make konnect entities spec.konnect.authRef immutable only when entity is already programmed #24

Merged
merged 2 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func init() {
// +kubebuilder:object:root=true
// +kubebuilder:object:generate=true
// +kubebuilder:subresource:status
// +kubebuilder:printcolumn:name="Valid",description="The API authentication information is valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status`
// +kubebuilder:printcolumn:name="Valid",description="The API authentication information is valid",type=string,JSONPath=`.status.conditions[?(@.type=='APIAuthValid')].status`
// +kubebuilder:printcolumn:name="OrgID",description="Konnect Organization ID this API authentication configuration belongs to.",type=string,JSONPath=`.status.organizationID`
// +kubebuilder:printcolumn:name="ServerURL",description="Configured server URL.",type=string,JSONPath=`.status.serverURL`
// +kubebuilder:validation:XValidation:rule="self.spec.type != 'token' || (self.spec.token.startsWith('spat_') || self.spec.token.startsWith('kpat_'))", message="Konnect tokens have to start with spat_ or kpat_"
Expand Down
1 change: 0 additions & 1 deletion api/konnect/v1alpha1/konnect_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ type KonnectConfiguration struct {
// that should be used for this Konnect Configuration.
//
// +kubebuilder:validation:Required
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="authRef is immutable"
APIAuthConfigurationRef KonnectAPIAuthConfigurationRef `json:"authRef"`

// NOTE: Place for extending the KonnectConfiguration object.
Expand Down
2 changes: 2 additions & 0 deletions api/konnect/v1alpha1/konnect_controlplane_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ func init() {
// +kubebuilder:printcolumn:name="Programmed",description="The Resource is Programmed on Konnect",type=string,JSONPath=`.status.conditions[?(@.type=='Programmed')].status`
// +kubebuilder:printcolumn:name="ID",description="Konnect ID",type=string,JSONPath=`.status.id`
// +kubebuilder:printcolumn:name="OrgID",description="Konnect Organization ID this resource belongs to.",type=string,JSONPath=`.status.organizationID`
// +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 KonnectControlPlane struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,6 @@ spec:
required:
- name
type: object
x-kubernetes-validations:
- message: authRef is immutable
rule: self == oldSelf
required:
- authRef
type: object
Expand Down
3 changes: 0 additions & 3 deletions config/crd/bases/configuration.konghq.com_kongroutes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,6 @@ spec:
required:
- name
type: object
x-kubernetes-validations:
- message: authRef is immutable
rule: self == oldSelf
required:
- authRef
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ spec:
versions:
- additionalPrinterColumns:
- description: The API authentication information is valid
jsonPath: .status.conditions[?(@.type=='Valid')].status
jsonPath: .status.conditions[?(@.type=='APIAuthValid')].status
name: Valid
type: string
- description: Konnect Organization ID this API authentication configuration belongs
Expand Down
11 changes: 8 additions & 3 deletions config/crd/bases/konnect.konghq.com_konnectcontrolplanes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,6 @@ spec:
required:
- name
type: object
x-kubernetes-validations:
- message: authRef is immutable
rule: self == oldSelf
required:
- authRef
type: object
Expand Down Expand Up @@ -230,6 +227,14 @@ spec:
type: string
type: object
type: object
x-kubernetes-validations:
- 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:
Expand Down
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/kong/kubernetes-configuration
go 1.22.4

require (
github.com/Kong/sdk-konnect-go v0.0.0-20240723160412-999d9a987e1a
github.com/Kong/sdk-konnect-go v0.0.1
github.com/kong/go-kong v0.57.1
github.com/stretchr/testify v1.9.0
k8s.io/api v0.30.3
Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
github.com/Kong/sdk-konnect-go v0.0.0-20240723160412-999d9a987e1a h1:0mQhPVVA2/+uTVmoKrEIGf+0eTrNyr80Ssv1zGs/1Lk=
github.com/Kong/sdk-konnect-go v0.0.0-20240723160412-999d9a987e1a/go.mod h1:ipu67aQNnwDzu/LXKePG46cVqkkZnAHKWpsbhTEI8xE=
github.com/Kong/sdk-konnect-go v0.0.1 h1:yxDRzT7gBriM9ZD3MDJCoEVBwtzEpcujuxLK1Ga5ObM=
github.com/Kong/sdk-konnect-go v0.0.1/go.mod h1:75YzLhfnYfmCvBJgkafzVuREwBAec2/jihCW2fyn6hY=
github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand All @@ -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=
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
})
}
}
37 changes: 37 additions & 0 deletions test/crdsvalidation/konnectcontrolplane/testcases/common.go
Original file line number Diff line number Diff line change
@@ -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",
}
Original file line number Diff line number Diff line change
@@ -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"
},
},
},
}