Skip to content

feat: set defaults for optional struct typed fields#3104

Draft
sandromodarelli wants to merge 3 commits intomainfrom
fix/issue-2588
Draft

feat: set defaults for optional struct typed fields#3104
sandromodarelli wants to merge 3 commits intomainfrom
fix/issue-2588

Conversation

@sandromodarelli
Copy link
Contributor

What this PR does / why we need it:

Sets default values for optional and struct typed fields

Which issue this PR fixes

Fixes #2588

Special notes for your reviewer:

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect significant changes

// Licensing defines the configuration for Konnect licensing.
//
// +optional
// +kubebuilder:default={state: "disabled", storageState: "disabled"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

storageState is currently enabled by default

.

Do we actually want to change the default?

Copy link
Contributor Author

@sandromodarelli sandromodarelli Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is that there's a validation ensuring that storageState is enabled only if state is enabled.

// +kubebuilder:validation:XValidation:message="storageState can only be set to enabled when licensing is enabled",rule="!has(self.storageState) || self.storageState == 'disabled' || self.state == 'enabled'"
.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I understand.

So I think we want to still keep the default (meaning that license storage should still be enabled by default) but allow users to not set the licensing and keep the licensing disabled by default.

I've tested the following change and it passed the crd validation tests:

diff --git a/api/gateway-operator/v2beta1/controlplane_types.go b/api/gateway-operator/v2beta1/controlplane_types.go
index 0ac7bbbfe..9c0ad9a5a 100644
--- a/api/gateway-operator/v2beta1/controlplane_types.go
+++ b/api/gateway-operator/v2beta1/controlplane_types.go
@@ -532,7 +532,7 @@ type ControlPlaneKonnectOptions struct {
 	// Licensing defines the configuration for Konnect licensing.
 	//
 	// +optional
-	// +kubebuilder:default={state: "disabled", storageState: "disabled"}
+	// +kubebuilder:default={state: "disabled"}
 	Licensing *ControlPlaneKonnectLicensing `json:"licensing,omitempty"`
 
 	// NodeRefreshPeriod is the period for refreshing the node information in Konnect.
@@ -585,7 +585,7 @@ type ControlPlaneKonnectLicensing struct {
 	// Only effective when State is set to enabled.
 	//
 	// +optional
-	// +kubebuilder:default=disabled
+	// +kubebuilder:default=enabled
 	// +kubebuilder:validation:Enum=enabled;disabled
 	StorageState *ControlPlaneKonnectLicensingState `json:"storageState,omitempty"`
 }
diff --git a/config/crd/kong-operator/gateway-operator.konghq.com_controlplanes.yaml b/config/crd/kong-operator/gateway-operator.konghq.com_controlplanes.yaml
index fa069a90d..d1e66682f 100644
--- a/config/crd/kong-operator/gateway-operator.konghq.com_controlplanes.yaml
+++ b/config/crd/kong-operator/gateway-operator.konghq.com_controlplanes.yaml
@@ -9037,7 +9037,6 @@ spec:
                   licensing:
                     default:
                       state: disabled
-                      storageState: disabled
                     description: Licensing defines the configuration for Konnect licensing.
                     properties:
                       initialPollingPeriod:
@@ -9057,7 +9056,7 @@ spec:
                         - disabled
                         type: string
                       storageState:
-                        default: disabled
+                        default: enabled
                         description: |-
                           StorageState indicates whether to store licenses fetched from Konnect
                           to Secrets locally to use them later when connection to Konnect is broken.
diff --git a/config/crd/kong-operator/gateway-operator.konghq.com_gatewayconfigurations.yaml b/config/crd/kong-operator/gateway-operator.konghq.com_gatewayconfigurations.yaml
index 905052d94..3a1cdbc60 100644
--- a/config/crd/kong-operator/gateway-operator.konghq.com_gatewayconfigurations.yaml
+++ b/config/crd/kong-operator/gateway-operator.konghq.com_gatewayconfigurations.yaml
@@ -18724,7 +18724,6 @@ spec:
                       licensing:
                         default:
                           state: disabled
-                          storageState: disabled
                         description: Licensing defines the configuration for Konnect
                           licensing.
                         properties:
@@ -18745,7 +18744,7 @@ spec:
                             - disabled
                             type: string
                           storageState:
-                            default: disabled
+                            default: enabled
                             description: |-
                               StorageState indicates whether to store licenses fetched from Konnect
                               to Secrets locally to use them later when connection to Konnect is broken.

Copy link
Contributor Author

@sandromodarelli sandromodarelli Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately also your proposal is hitting tests'issues https://github.com/Kong/kong-operator/actions/runs/21215878134/job/61036773952?pr=3104#step:7:6669.
when licensing is not null (default to {state: disabled} automatically the storageState value is enabled).
I think that the best for now would be to revert the default for this field and maybe track in a separated issue. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if we can not touch it in this issue, I concur, let's handle it separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CRD default fields, enclosed by structs that are used as optional fields do not take effect

2 participants