Fix API design issues and address PR feedback

Addresses feedback from PR reviewers and linting issues.

Signed-off-by: Daniel Mellado <dmellado@fedoraproject.org>

Author: danielmellado

config/v1alpha1/tests/clustermonitorings.config.openshift.io/ClusterMonitoringConfig.yaml

@@ -359,15 +359,15 @@ tests:
           userDefined:
             mode: "Disabled"
           prometheusK8sConfig:
-            logLevel: "Info"
+            logLevel: "info"
       expected: |
         apiVersion: config.openshift.io/v1alpha1
         kind: ClusterMonitoring
         spec:
           userDefined:
             mode: "Disabled"
           prometheusK8sConfig:
-            logLevel: "Info"
+            logLevel: "info"
     - name: Should accept PrometheusK8sConfig with valid nodeSelector
       initial: |
         apiVersion: config.openshift.io/v1alpha1

config/v1alpha1/types_cluster_monitoring.go

@@ -297,19 +297,19 @@ const (
 )
 
 // logLevel defines the verbosity of logs emitted by Alertmanager.
-// Valid values are Error, Warn, Info and Debug.
-// +kubebuilder:validation:Enum=Error;Warn;Info;Debug
+// Valid values are error, warn, info and debug.
+// +kubebuilder:validation:Enum=error;warn;info;debug
 type LogLevel string
 
 const (
-	// Error only errors will be logged.
-	LogLevelError LogLevel = "Error"
-	// Warn, both warnings and errors will be logged.
-	LogLevelWarn LogLevel = "Warn"
-	// Info, general information, warnings, and errors will all be logged.
-	LogLevelInfo LogLevel = "Info"
-	// Debug, detailed debugging information will be logged.
-	LogLevelDebug LogLevel = "Debug"
+	// LogLevelError only errors will be logged.
+	LogLevelError LogLevel = "error"
+	// LogLevelWarn, both warnings and errors will be logged.
+	LogLevelWarn LogLevel = "warn"
+	// LogLevelInfo, general information, warnings, and errors will all be logged.
+	LogLevelInfo LogLevel = "info"
+	// LogLevelDebug, detailed debugging information will be logged.
+	LogLevelDebug LogLevel = "debug"
 )
 
 // ContainerResource defines a single resource requirement for a container.
@@ -446,7 +446,16 @@ type MetricsServerConfig struct {
 // +kubebuilder:validation:MinProperties=1
 type PrometheusK8sConfig struct {
 	// additionalAlertmanagerConfigs configures additional Alertmanager instances that receive alerts from
-	// the Prometheus component. By default, no additional Alertmanager instances are configured.
+	// the Prometheus component. This is useful for organizations that need to:
+	//   - Send alerts to external monitoring systems (like PagerDuty, Slack, or custom webhooks)
+	//   - Route different types of alerts to different teams or systems
+	//   - Integrate with existing enterprise alerting infrastructure
+	//   - Maintain separate alert routing for compliance or organizational requirements
+	// By default, no additional Alertmanager instances are configured.
+	// Maximum of 10 additional Alertmanager configurations can be specified.
+	// Each configuration must have a unique apiVersion.
+	// When omitted, no additional Alertmanager instances are configured (default behavior).
+	// When set to an empty array [], the behavior is the same as omitting the field.
 	// +optional
 	// +kubebuilder:validation:MaxItems=10
 	// +listType=map
@@ -471,6 +480,8 @@ type PrometheusK8sConfig struct {
 	// externalLabels defines labels to be added to any time series or alerts when
 	// communicating with external systems such as federation, remote storage,
 	// and Alertmanager. By default, no labels are added.
+	// When omitted, no external labels are applied (default behavior).
+	// When set to an empty struct {}, the behavior is the same as omitting the field.
 	// +optional
 	ExternalLabels ExternalLabels `json:"externalLabels,omitempty,omitzero"`
 	// logLevel defines the verbosity of logs emitted by Alertmanager.
@@ -619,21 +630,6 @@ const (
 	AlertmanagerAPIVersionV2 AlertmanagerAPIVersion = "v2"
 )
 
-// SecretKeyReference represents a reference to a secret key.
-type SecretKeyReference struct {
-	// name of the secret in the pod's namespace to select from.
-	// +required
-	// +kubebuilder:validation:MaxLength=253
-	Name *string `json:"name,omitempty"`
-	// key of the secret to select from. Must be a valid secret key.
-	// +required
-	// +kubebuilder:validation:MaxLength=253
-	Key *string `json:"key,omitempty"`
-	// optional specifies whether the Secret or its key must be defined
-	// +optional
-	// +kubebuilder:validation:Enum=true;false
-	Optional string `json:"optional,omitempty"`
-}
 
 type AlertmanagerScheme string
 
@@ -655,8 +651,11 @@ type AdditionalAlertmanagerConfig struct {
 	// to use when authenticating to Alertmanager.
 	// This is a custom type to allow for admission time validations.
 	// +optional
-	BearerToken SecretKeyReference `json:"bearerToken,omitempty,omitzero"`
-	// pathPrefix defines the path prefix to add in front of the push endpoint path.
+	BearerToken SecretKeySelector `json:"bearerToken,omitempty,omitzero"`
+	// pathPrefix defines an optional URL path prefix to prepend to the Alertmanager API endpoints.
+	// For example, if your Alertmanager is behind a reverse proxy at "/alertmanager/", 
+	// set this to "/alertmanager" so requests go to "/alertmanager/api/v1/alerts" instead of "/api/v1/alerts".
+	// This is commonly needed when Alertmanager is deployed behind ingress controllers or load balancers.
 	// +kubebuilder:validation:MaxLength=255
 	// +kubebuilder:validation:MinLength=1
 	// +optional
@@ -669,6 +668,7 @@ type AdditionalAlertmanagerConfig struct {
 	Scheme AlertmanagerScheme `json:"scheme,omitempty"`
 	// staticConfigs is a list of statically configured Alertmanager endpoints in the form
 	// of `<host>:<port>`. Each entry must be a valid hostname or IP address followed by a colon and a valid port number (1-65535).
+	// Maximum of 10 endpoints can be specified.
 	// +kubebuilder:validation:MaxItems=10
 	// +kubebuilder:validation:items:MaxLength=255
 	// +kubebuilder:validation:items:Pattern=`^([a-zA-Z0-9.-]+|\[[a-fA-F0-9:]+\]):([1-9][0-9]{0,4})$`
@@ -705,6 +705,8 @@ type Label struct {
 // ExternalLabels represents labels to be added to time series and alerts.
 type ExternalLabels struct {
 	// labels is a list of label key/value pairs.
+	// At least 1 label must be specified, with a maximum of 50 labels allowed.
+	// This field is required when ExternalLabels is specified - an empty array [] is not allowed.
 	// +required
 	// +kubebuilder:validation:MinItems=1
 	// +kubebuilder:validation:MaxItems=50
@@ -734,6 +736,7 @@ type RemoteWriteSpec struct {
 	// +kubebuilder:validation:MinLength=2
 	RemoteTimeout string `json:"remoteTimeout,omitempty"`
 	// writeRelabelConfigs is a list of relabeling rules to apply before sending data to the remote endpoint.
+	// Maximum of 10 relabeling rules can be specified.
 	// +optional
 	// +kubebuilder:validation:MaxItems=10
 	// +listType=atomic
@@ -770,9 +773,11 @@ type RelabelConfig struct {
 	// +kubebuilder:validation:MaxLength=255
 	Replacement *string `json:"replacement,omitempty"`
 	// action is the action to perform.
+	// Valid actions are: replace (default), keep, drop, hashmod, labelmap, labeldrop, labelkeep.
+	// When omitted, defaults to "replace".
 	// +optional
-	// +kubebuilder:validation:MaxLength=20
-	Action *string `json:"action,omitempty"`
+	// +kubebuilder:default=replace
+	Action *RelabelAction `json:"action,omitempty"`
 }
 
 // TLSConfig represents TLS configuration for Alertmanager connections.
@@ -795,7 +800,7 @@ type TLSConfig struct {
 	ServerName string `json:"serverName,omitempty"`
 	// insecureSkipVerify determines the policy for TLS certificate verification.
 	// Allowed values are "Verify" (default, secure) and "InsecureSkipVerify" (skip certificate verification, insecure).
-	// By default, certificate verification is performed ("Verify").
+	// When omitted, defaults to "Verify" (secure certificate verification is performed).
 	// +optional
 	// +kubebuilder:validation:Enum=Verify;InsecureSkipVerify
 	// +kubebuilder:default=Verify
@@ -840,22 +845,58 @@ type SecretKeySelector struct {
 	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:validation:MaxLength=253
 	Key string `json:"key,omitempty" protobuf:"bytes,2,opt,name=key"`
-	// optional specifies whether the Secret or its key must be defined
-	// +optional
-	// +kubebuilder:validation:Enum=true;false
-	Optional string `json:"optional,omitempty" protobuf:"varint,3,opt,name=optional"`
+	// optional specifies whether the Secret or its key must be defined.
+	// When set to "Required", the Secret and key must exist.
+	// When set to "Optional", missing Secret or key will not cause an error.
+	// When omitted, defaults to "Required".
+	// +optional
+	// +kubebuilder:validation:Enum=Required;Optional
+	// +kubebuilder:default=Required
+	Optional SecretRequirement `json:"optional,omitempty" protobuf:"varint,3,opt,name=optional"`
 }
 
+// SecretRequirement defines whether a secret reference is required or optional.
+// +kubebuilder:validation:Enum=Required;Optional
+type SecretRequirement string
+
+const (
+	// SecretRequirementRequired means the Secret and its key must exist.
+	SecretRequirementRequired SecretRequirement = "Required"
+	// SecretRequirementOptional means missing Secret or key will not cause an error.
+	SecretRequirementOptional SecretRequirement = "Optional"
+)
+
+// RelabelAction defines the action to perform in a relabeling rule.
+// +kubebuilder:validation:Enum=replace;keep;drop;hashmod;labelmap;labeldrop;labelkeep
+type RelabelAction string
+
+const (
+	// RelabelActionReplace replaces the target label with the replacement value.
+	RelabelActionReplace RelabelAction = "replace"
+	// RelabelActionKeep keeps metrics that match the regex.
+	RelabelActionKeep RelabelAction = "keep"
+	// RelabelActionDrop drops metrics that match the regex.
+	RelabelActionDrop RelabelAction = "drop"
+	// RelabelActionHashMod sets the target label to the modulus of a hash of the source labels.
+	RelabelActionHashMod RelabelAction = "hashmod"
+	// RelabelActionLabelMap maps label names based on regex matching.
+	RelabelActionLabelMap RelabelAction = "labelmap"
+	// RelabelActionLabelDrop removes labels that match the regex.
+	RelabelActionLabelDrop RelabelAction = "labeldrop"
+	// RelabelActionLabelKeep removes labels that do not match the regex.
+	RelabelActionLabelKeep RelabelAction = "labelkeep"
+)
+
 // CollectionProfile defines the metrics collection profile for Prometheus.
-// +kubebuilder:validation:Enum=Full;Minimal
+// +kubebuilder:validation:Enum=full;minimal
 type CollectionProfile string
 
 const (
 	// CollectionProfileFull means Prometheus collects all metrics that are exposed by the platform components.
-	CollectionProfileFull CollectionProfile = "Full"
+	CollectionProfileFull CollectionProfile = "full"
 	// CollectionProfileMinimal means Prometheus only collects metrics necessary for the default
 	// platform alerts, recording rules, telemetry and console dashboards.
-	CollectionProfileMinimal CollectionProfile = "Minimal"
+	CollectionProfileMinimal CollectionProfile = "minimal"
 )
 
 // AuditProfile defines the audit log level for the Metrics Server.