Skip to content
Draft
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
77 changes: 77 additions & 0 deletions config/v1/types_console.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,16 @@ type Console struct {

// ConsoleSpec is the specification of the desired behavior of the Console.
type ConsoleSpec struct {
// authentication configures console authentication behavior.
// When omitted, default authentication settings are used.
// +optional
Authentication ConsoleAuthentication `json:"authentication"`

// externalSecretStore configures integration with an external secret store
// for console secret management. When omitted, no external secret store is configured.
// +optional
// +openshift:enable:FeatureGate=ExternalSecretStore
ExternalSecretStore ExternalSecretStoreConfig `json:"externalSecretStore,omitempty,omitzero"`
}

// ConsoleStatus defines the observed status of the Console.
Expand Down Expand Up @@ -79,3 +87,72 @@ type ConsoleAuthentication struct {
// +kubebuilder:validation:Pattern=`^$|^((https):\/\/?)[^\s()<>]+(?:\([\w\d]+\)|([^[:punct:]\s]|\/?))$`
LogoutRedirect string `json:"logoutRedirect,omitempty"`
}

// ExternalSecretStoreType defines the type of external secret store.
// When set to Vault, HashiCorp Vault is used as the external secret store.
// +kubebuilder:validation:Enum=Vault
type ExternalSecretStoreType string

// ExternalSecretStoreConfig defines the configuration for integration with an
// external secret store.
// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'Vault' ? has(self.vault) : !has(self.vault)",message="vault configuration is required when type is Vault, and forbidden otherwise"

@openshift-api-review-bot openshift-api-review-bot Bot Jul 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: Redundant has() guard on required field (CEL simplification)

The has(self.type) guard is unnecessary because type is a +required field — it is always present when XValidation runs. This makes the CEL expression more verbose than needed.

Current:

// +kubebuilder:validation:XValidation:rule="has(self.type) && self.type == 'Vault' ? has(self.vault) : !has(self.vault)",message="vault configuration is required when type is Vault, and forbidden otherwise"

Suggested:

// +kubebuilder:validation:XValidation:rule="self.type == 'Vault' ? has(self.vault) : !has(self.vault)",message="vault configuration is required when type is Vault, and forbidden otherwise"

Since type is +required, has(self.type) is always true. The guard adds no protection and makes the expression harder to read.

type ExternalSecretStoreConfig struct {
// type specifies the type of external secret store to use.
// Currently supported values:
// - Vault
// +required

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: Enum value meaning not explained in field comment

The type field lists "Vault" as a supported value but does not explain what it means. Enum meanings must appear in the field's own comment (not just the type definition) since that is what users see in generated docs.

Current:

// type specifies the type of external secret store to use.
// Currently supported values:
// - Vault
// +required
Type ExternalSecretStoreType `json:"type"`

Suggested:

// type specifies the type of external secret store to use.
// Currently supported values:
// - Vault: HashiCorp Vault is used as the external secret store.
// +required
Type ExternalSecretStoreType `json:"type"`

Each enum value must be listed AND its meaning explained in the field's own comment using a descriptive pattern.

Type ExternalSecretStoreType `json:"type"`

// vault contains the configuration for a HashiCorp Vault secret store.
// This field is required when type is Vault, and forbidden otherwise.
// +optional
Vault VaultSecretStoreConfig `json:"vault,omitempty,omitzero"`
}

// VaultSecretStoreConfig defines the configuration for HashiCorp Vault integration.
type VaultSecretStoreConfig struct {
// serverAddress specifies the address of the Vault server.
// Must be a valid URL starting with https://.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issues: Documented constraint not enforced + MinLength/MaxLength not documented

Two problems with this field:

1) Documented constraint not enforced: The comment states "Must be a valid URL starting with https://" but there is no +kubebuilder:validation:Pattern marker to enforce this. The API will silently accept values that are not HTTPS URLs.

2) MinLength/MaxLength not documented: The field has MinLength=1 and MaxLength=512 markers but the comment does not document these length constraints.

Current:

// serverAddress specifies the address of the Vault server.
// Must be a valid URL starting with https://.
// +required
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=512
ServerAddress string `json:"serverAddress"`

Suggested:

// serverAddress specifies the address of the Vault server.
// Must be a valid URL starting with https://.
// The address must be between 1 and 512 characters long.
// +required
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=512
// +kubebuilder:validation:Pattern=`^https://`
ServerAddress string `json:"serverAddress"`

Either add a Pattern marker to enforce the HTTPS URL constraint, or remove the claim from the comment. All constraint markers must also be documented in the field's comment.

// +required
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=512
ServerAddress string `json:"serverAddress"`

// transitKeyName specifies the name of the transit encryption key in Vault.
// The name must not exceed 253 characters.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issues: Documented constraint not enforced + MinLength not documented

Two problems with this field:

1) Documented constraint not enforced: The comment states "The name must not exceed 253 characters" but there is no +kubebuilder:validation:MaxLength=253 marker to enforce this constraint.

2) MinLength not documented: The field has MinLength=1 but the comment does not mention that empty strings are rejected.

Current:

// transitKeyName specifies the name of the transit encryption key in Vault.
// The name must not exceed 253 characters.
// +required
// +kubebuilder:validation:MinLength=1
TransitKeyName string `json:"transitKeyName"`

Suggested:

// transitKeyName specifies the name of the transit encryption key in Vault.
// The name must be at least 1 character and must not exceed 253 characters.
// +required
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=253
TransitKeyName string `json:"transitKeyName"`

The claimed 253-character max must be enforced with a MaxLength marker, not just documented.

// +required
// +kubebuilder:validation:MinLength=1
TransitKeyName string `json:"transitKeyName"`

// transitMountPath specifies the mount path for the transit secrets engine.
// +required
// +kubebuilder:validation:MinLength=1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: MinLength not documented

The field has MinLength=1 but the comment does not mention that the value must be at least 1 character (i.e., empty strings are rejected).

Current:

// transitMountPath specifies the mount path for the transit secrets engine.
// +required
// +kubebuilder:validation:MinLength=1
TransitMountPath string `json:"transitMountPath"`

Suggested:

// transitMountPath specifies the mount path for the transit secrets engine.
// The path must be at least 1 character long.
// +required
// +kubebuilder:validation:MinLength=1
TransitMountPath string `json:"transitMountPath"`

All constraint markers must be documented in the field's comment so users know the validation rules without reading the markers.

TransitMountPath string `json:"transitMountPath"`

// namespace specifies the Vault namespace to use.
// +optional

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issues: Optional field missing omitted behavior + MinLength/MaxLength not documented

Two problems with this field:

1) Optional field does not explain omitted behavior: The namespace field is +optional but the comment does not explain what happens when the field is omitted.

2) MinLength/MaxLength not documented: The field has MinLength=1 and MaxLength=256 markers but the comment does not document these length constraints.

Current:

// namespace specifies the Vault namespace to use.
// +optional
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=256
Namespace string `json:"namespace,omitempty"`

Suggested:

// namespace specifies the Vault namespace to use.
// When omitted, the root namespace is used.
// The namespace must be between 1 and 256 characters long.
// +optional
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=256
Namespace string `json:"namespace,omitempty"`

Every optional field must explain the behavior when it is not provided. All constraint markers must also be documented in the field comment.

// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=256
Namespace string `json:"namespace,omitempty"`

// caCertificate contains the PEM-encoded CA certificate for TLS verification
// of the Vault server connection.
// When omitted, the system's trusted CA certificates are used.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: MinLength and MaxLength not documented

The field has MinLength=1 and MaxLength=65536 markers but the comment does not document these length constraints.

Current:

// caCertificate contains the PEM-encoded CA certificate for TLS verification
// of the Vault server connection.
// When omitted, the system's trusted CA certificates are used.
// +optional
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=65536
CACertificate string `json:"caCertificate,omitempty"`

Suggested:

// caCertificate contains the PEM-encoded CA certificate for TLS verification
// of the Vault server connection.
// When omitted, the system's trusted CA certificates are used.
// The certificate must be between 1 and 65536 characters long.
// +optional
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=65536
CACertificate string `json:"caCertificate,omitempty"`

All constraint markers must be documented in the field comment so users understand validation without reading the markers.

// +optional
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=65536
CACertificate string `json:"caCertificate,omitempty"`

// authSecret references a secret containing the authentication credentials
// for Vault. The secret must exist in the openshift-config namespace.
// +required
AuthSecret SecretNameReference `json:"authSecret"`

// refreshInterval specifies how often secrets are re-fetched from Vault,
// in seconds. The value must be between 30 and 3600.
// When omitted, the platform chooses a reasonable default.
// +optional
// +kubebuilder:validation:Minimum=30
// +kubebuilder:validation:Maximum=3600
RefreshInterval int32 `json:"refreshInterval,omitempty"`
}
9 changes: 9 additions & 0 deletions features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -1053,3 +1053,12 @@ var (
enable(inClusterProfile(SelfManaged), inDevPreviewNoUpgrade()).
mustRegister()
)

var (
FeatureGateExternalSecretStore = newFeatureGate("ExternalSecretStore").
reportProblemsToJiraComponent("Management-Console").
contactPerson("jhadvig").
productScope(ocpSpecific).
enable(inTechPreviewNoUpgrade()).
mustRegister()
)