From 86b131f141a601ef129607276c3c8907709e5b4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patryk=20Ma=C5=82ek?= Date: Wed, 30 Oct 2024 19:10:01 +0100 Subject: [PATCH] feat: use KongObjectRef in KongRoute --- .golangci.yaml | 2 - api/configuration/v1alpha1/object_ref.go | 8 +-- api/configuration/v1alpha1/service_ref.go | 17 ++----- .../v1alpha1/zz_generated.deepcopy.go | 17 +------ .../configuration.konghq.com_kongroutes.yaml | 4 ++ .../configuration.konghq.com_kongsnis.yaml | 5 +- docs/api-reference.md | 25 ++-------- test/crdsvalidation/kongroute_test.go | 49 ++++++++++++++++--- 8 files changed, 60 insertions(+), 67 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 75ff7d3d..4df23e6d 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -88,8 +88,6 @@ linters-settings: disabled: false arguments: - "checkPrivateReceivers" - # TODO: enable this when ready to refactor exported types that stutter at call site. - - "disableStutteringCheck" - name: context-as-argument # TODO: re-add this rule after https://github.com/golangci/golangci-lint/issues/3280 # is resolved and released. diff --git a/api/configuration/v1alpha1/object_ref.go b/api/configuration/v1alpha1/object_ref.go index 7eb24e2c..47a15f6a 100644 --- a/api/configuration/v1alpha1/object_ref.go +++ b/api/configuration/v1alpha1/object_ref.go @@ -1,15 +1,17 @@ package v1alpha1 +// TODO: https://github.com/Kong/kubernetes-configuration/issues/96 +// Change other types to use the generic `KongObjectRef` and move it to a common package +// to prevent possible import cycles. + // KongObjectRef is a reference to another object representing a Kong entity with deterministic type. // -// TODO: https://github.com/Kong/kubernetes-configuration/issues/96 -// change other types to use the generic `KongObjectRef` and move it to a common package to prevent possible import cycles. // +apireference:kgo:include type KongObjectRef struct { // Name is the name of the entity. // - // NOTE: the `Required` validation rule does not reject empty strings so we use `MinLength` to reject empty string here. // +kubebuilder:validation:MinLength=1 Name string `json:"name"` + // TODO: handle cross namespace references. } diff --git a/api/configuration/v1alpha1/service_ref.go b/api/configuration/v1alpha1/service_ref.go index b8e42878..c6479a72 100644 --- a/api/configuration/v1alpha1/service_ref.go +++ b/api/configuration/v1alpha1/service_ref.go @@ -6,25 +6,16 @@ const ( ) // ServiceRef is a reference to a KongService. +// // +kubebuilder:validation:XValidation:rule="self.type == 'namespacedRef' ? has(self.namespacedRef) : true", message="when type is namespacedRef, namespacedRef must be set" // +apireference:kgo:include type ServiceRef struct { // Type can be one of: // - namespacedRef + // + // +kubebuilder:validation:Enum:=namespacedRef Type string `json:"type,omitempty"` // NamespacedRef is a reference to a KongService. - NamespacedRef *NamespacedServiceRef `json:"namespacedRef,omitempty"` -} - -// NamespacedServiceRef is a namespaced reference to a KongService. -// -// NOTE: currently cross namespace references are not supported. -// +apireference:kgo:include -type NamespacedServiceRef struct { - // +kubebuilder:validation:Required - Name string `json:"name"` - - // TODO: handle cross namespace references. - // https://github.com/Kong/kubernetes-configuration/issues/106 + NamespacedRef *KongObjectRef `json:"namespacedRef,omitempty"` } diff --git a/api/configuration/v1alpha1/zz_generated.deepcopy.go b/api/configuration/v1alpha1/zz_generated.deepcopy.go index 36dc75a3..c46f65ae 100644 --- a/api/configuration/v1alpha1/zz_generated.deepcopy.go +++ b/api/configuration/v1alpha1/zz_generated.deepcopy.go @@ -2848,21 +2848,6 @@ func (in *KonnectNamespacedRef) DeepCopy() *KonnectNamespacedRef { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *NamespacedServiceRef) DeepCopyInto(out *NamespacedServiceRef) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NamespacedServiceRef. -func (in *NamespacedServiceRef) DeepCopy() *NamespacedServiceRef { - if in == nil { - return nil - } - out := new(NamespacedServiceRef) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ObjectReference) DeepCopyInto(out *ObjectReference) { *out = *in @@ -2933,7 +2918,7 @@ func (in *ServiceRef) DeepCopyInto(out *ServiceRef) { *out = *in if in.NamespacedRef != nil { in, out := &in.NamespacedRef, &out.NamespacedRef - *out = new(NamespacedServiceRef) + *out = new(KongObjectRef) **out = **in } } diff --git a/config/crd/bases/configuration.konghq.com_kongroutes.yaml b/config/crd/bases/configuration.konghq.com_kongroutes.yaml index 9c6efe11..f06f6087 100644 --- a/config/crd/bases/configuration.konghq.com_kongroutes.yaml +++ b/config/crd/bases/configuration.konghq.com_kongroutes.yaml @@ -207,6 +207,8 @@ spec: description: NamespacedRef is a reference to a KongService. properties: name: + description: Name is the name of the entity. + minLength: 1 type: string required: - name @@ -215,6 +217,8 @@ spec: description: |- Type can be one of: - namespacedRef + enum: + - namespacedRef type: string type: object x-kubernetes-validations: diff --git a/config/crd/bases/configuration.konghq.com_kongsnis.yaml b/config/crd/bases/configuration.konghq.com_kongsnis.yaml index 785741b2..8e9b72f3 100644 --- a/config/crd/bases/configuration.konghq.com_kongsnis.yaml +++ b/config/crd/bases/configuration.konghq.com_kongsnis.yaml @@ -49,10 +49,7 @@ spec: which the KongSNI is attached. properties: name: - description: |- - Name is the name of the entity. - - NOTE: the `Required` validation rule does not reject empty strings so we use `MinLength` to reject empty string here. + description: Name is the name of the entity. minLength: 1 type: string required: diff --git a/docs/api-reference.md b/docs/api-reference.md index 809a71ea..2a29b071 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1228,19 +1228,18 @@ _Appears in:_ #### KongObjectRef -KongObjectRef is a reference to another object representing a Kong entity with deterministic type.

-TODO: https://github.com/Kong/kubernetes-configuration/issues/96 -change other types to use the generic `KongObjectRef` and move it to a common package to prevent possible import cycles. +KongObjectRef is a reference to another object representing a Kong entity with deterministic type. | Field | Description | | --- | --- | -| `name` _string_ | Name is the name of the entity.

NOTE: the `Required` validation rule does not reject empty strings so we use `MinLength` to reject empty string here. | +| `name` _string_ | Name is the name of the entity. | _Appears in:_ - [KongSNISpec](#kongsnispec) +- [ServiceRef](#serviceref) #### KongPluginBindingSpec @@ -1596,22 +1595,6 @@ Namespace refers to a Kubernetes namespace. It must be a RFC 1123 label. _Appears in:_ - [ControllerReference](#controllerreference) -#### NamespacedServiceRef - - -NamespacedServiceRef is a namespaced reference to a KongService.

-NOTE: currently cross namespace references are not supported. - - - -| Field | Description | -| --- | --- | -| `name` _string_ | | - - -_Appears in:_ -- [ServiceRef](#serviceref) - #### ObjectName _Underlying type:_ `string` @@ -1687,7 +1670,7 @@ ServiceRef is a reference to a KongService. | Field | Description | | --- | --- | | `type` _string_ | Type can be one of: - namespacedRef | -| `namespacedRef` _[NamespacedServiceRef](#namespacedserviceref)_ | NamespacedRef is a reference to a KongService. | +| `namespacedRef` _[KongObjectRef](#kongobjectref)_ | NamespacedRef is a reference to a KongService. | _Appears in:_ diff --git a/test/crdsvalidation/kongroute_test.go b/test/crdsvalidation/kongroute_test.go index 48ec508e..8c73fecd 100644 --- a/test/crdsvalidation/kongroute_test.go +++ b/test/crdsvalidation/kongroute_test.go @@ -28,7 +28,7 @@ func TestKongRoute(t *testing.T) { }, ServiceRef: &configurationv1alpha1.ServiceRef{ Type: configurationv1alpha1.ServiceRefNamespacedRef, - NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + NamespacedRef: &configurationv1alpha1.KongObjectRef{ Name: "test-konnect-service", }, }, @@ -226,7 +226,7 @@ func TestKongRoute(t *testing.T) { Spec: configurationv1alpha1.KongRouteSpec{ ServiceRef: &configurationv1alpha1.ServiceRef{ Type: configurationv1alpha1.ServiceRefNamespacedRef, - NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{Name: "svc"}, + NamespacedRef: &configurationv1alpha1.KongObjectRef{Name: "svc"}, }, KongRouteAPISpec: configurationv1alpha1.KongRouteAPISpec{ Paths: []string{"/"}, @@ -241,7 +241,7 @@ func TestKongRoute(t *testing.T) { Spec: configurationv1alpha1.KongRouteSpec{ ServiceRef: &configurationv1alpha1.ServiceRef{ Type: configurationv1alpha1.ServiceRefNamespacedRef, - NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{Name: "svc"}, + NamespacedRef: &configurationv1alpha1.KongObjectRef{Name: "svc"}, }, KongRouteAPISpec: configurationv1alpha1.KongRouteAPISpec{ Protocols: []sdkkonnectcomp.RouteProtocols{"http"}, @@ -257,7 +257,7 @@ func TestKongRoute(t *testing.T) { Spec: configurationv1alpha1.KongRouteSpec{ ServiceRef: &configurationv1alpha1.ServiceRef{ Type: configurationv1alpha1.ServiceRefNamespacedRef, - NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{Name: "svc"}, + NamespacedRef: &configurationv1alpha1.KongObjectRef{Name: "svc"}, }, KongRouteAPISpec: configurationv1alpha1.KongRouteAPISpec{ Protocols: []sdkkonnectcomp.RouteProtocols{"http"}, @@ -278,7 +278,7 @@ func TestKongRoute(t *testing.T) { Spec: configurationv1alpha1.KongRouteSpec{ ServiceRef: &configurationv1alpha1.ServiceRef{ Type: configurationv1alpha1.ServiceRefNamespacedRef, - NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + NamespacedRef: &configurationv1alpha1.KongObjectRef{ Name: "test-konnect-service", }, }, @@ -288,6 +288,39 @@ func TestKongRoute(t *testing.T) { }, }, }, + { + Name: "NamespacedRef reference is invalid when empty name is provided", + TestObject: &configurationv1alpha1.KongRoute{ + ObjectMeta: commonObjectMeta, + Spec: configurationv1alpha1.KongRouteSpec{ + ServiceRef: &configurationv1alpha1.ServiceRef{ + Type: configurationv1alpha1.ServiceRefNamespacedRef, + NamespacedRef: &configurationv1alpha1.KongObjectRef{ + Name: "", + }, + }, + KongRouteAPISpec: configurationv1alpha1.KongRouteAPISpec{ + Paths: []string{"/"}, + }, + }, + }, + ExpectedErrorMessage: lo.ToPtr("spec.serviceRef.namespacedRef.name in body should be at least 1 chars long"), + }, + { + Name: "NamespacedRef reference is invalid when name is not provided", + TestObject: &configurationv1alpha1.KongRoute{ + ObjectMeta: commonObjectMeta, + Spec: configurationv1alpha1.KongRouteSpec{ + ServiceRef: &configurationv1alpha1.ServiceRef{ + Type: configurationv1alpha1.ServiceRefNamespacedRef, + }, + KongRouteAPISpec: configurationv1alpha1.KongRouteAPISpec{ + Paths: []string{"/"}, + }, + }, + }, + ExpectedErrorMessage: lo.ToPtr("when type is namespacedRef, namespacedRef must be set"), + }, { Name: "not providing namespacedRef when type is namespacedRef yields an error", TestObject: &configurationv1alpha1.KongRoute{ @@ -310,7 +343,7 @@ func TestKongRoute(t *testing.T) { Spec: configurationv1alpha1.KongRouteSpec{ ServiceRef: &configurationv1alpha1.ServiceRef{ Type: configurationv1alpha1.ServiceRefNamespacedRef, - NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + NamespacedRef: &configurationv1alpha1.KongObjectRef{ Name: "test-konnect-service", }, }, @@ -341,7 +374,7 @@ func TestKongRoute(t *testing.T) { Spec: configurationv1alpha1.KongRouteSpec{ ServiceRef: &configurationv1alpha1.ServiceRef{ Type: configurationv1alpha1.ServiceRefNamespacedRef, - NamespacedRef: &configurationv1alpha1.NamespacedServiceRef{ + NamespacedRef: &configurationv1alpha1.KongObjectRef{ Name: "test-konnect-service", }, }, @@ -363,7 +396,7 @@ func TestKongRoute(t *testing.T) { Update: func(ks *configurationv1alpha1.KongRoute) { ks.Spec.ServiceRef.Type = "otherRef" }, - ExpectedUpdateErrorMessage: lo.ToPtr("spec.serviceRef is immutable when an entity is already Programmed"), + ExpectedUpdateErrorMessage: lo.ToPtr("Unsupported value: \"otherRef\": supported values: \"namespacedRef\""), }, }.Run(t) })