From 140d53476fa54a293958152faf6c1a8f7b32388a Mon Sep 17 00:00:00 2001 From: Kevin Conner Date: Mon, 24 Nov 2025 17:37:13 -0800 Subject: [PATCH] feat: SECURESIGN-3184: ensure ServiceMonitors are only created when running on OpenShift or explicitly configured Signed-off-by: Kevin Conner --- api/v1alpha1/common.go | 13 +++ api/v1alpha1/common_test.go | 80 +++++++++++++++++++ api/v1alpha1/zz_generated.deepcopy.go | 17 ++-- config/crd/bases/rhtas.redhat.com_ctlogs.yaml | 5 ++ .../crd/bases/rhtas.redhat.com_fulcios.yaml | 5 ++ config/crd/bases/rhtas.redhat.com_rekors.yaml | 5 ++ .../bases/rhtas.redhat.com_securesigns.yaml | 25 ++++++ ...rhtas.redhat.com_timestampauthorities.yaml | 5 ++ .../crd/bases/rhtas.redhat.com_trillians.yaml | 5 ++ .../controller/ctlog/actions/monitoring.go | 4 +- .../controller/fulcio/actions/monitoring.go | 4 +- .../rekor/actions/monitor/helper.go | 4 +- .../rekor/actions/server/monitoring.go | 4 +- .../trillian/actions/logserver/monitoring.go | 4 +- .../trillian/actions/logsigner/monitoring.go | 4 +- internal/controller/tsa/actions/monitoring.go | 4 +- 16 files changed, 175 insertions(+), 13 deletions(-) create mode 100644 api/v1alpha1/common_test.go diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index dee84edc3..12fcfa919 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -35,6 +35,19 @@ type MonitoringConfig struct { //+kubebuilder:validation:XValidation:rule=(self || !oldSelf),message=Feature cannot be disabled //+kubebuilder:default:=true Enabled bool `json:"enabled"` + // If true, the Operator will create ServiceMonitor resources for metrics collection. + // When not specified, defaults to true on OpenShift and false on other platforms. + //+optional + ServiceMonitor *bool `json:"serviceMonitor,omitempty"` +} + +// IsServiceMonitorEnabled returns whether ServiceMonitor resources should be created. +// If ServiceMonitor is explicitly set, returns that value, otherwise returns the default. +func (m *MonitoringConfig) IsServiceMonitorEnabled(defaultVal bool) bool { + if m.ServiceMonitor != nil { + return *m.ServiceMonitor + } + return defaultVal } type MonitoringWithTLogConfig struct { diff --git a/api/v1alpha1/common_test.go b/api/v1alpha1/common_test.go new file mode 100644 index 000000000..8b43e2356 --- /dev/null +++ b/api/v1alpha1/common_test.go @@ -0,0 +1,80 @@ +package v1alpha1 + +import ( + "testing" + + "k8s.io/utils/ptr" +) + +func TestMonitoringConfig_IsServiceMonitorEnabled(t *testing.T) { + tests := []struct { + name string + config MonitoringConfig + isOpenShift bool + expectedResult bool + }{ + { + name: "ServiceMonitor explicitly set to true", + config: MonitoringConfig{ + Enabled: true, + ServiceMonitor: ptr.To(true), + }, + isOpenShift: false, + expectedResult: true, + }, + { + name: "ServiceMonitor explicitly set to false", + config: MonitoringConfig{ + Enabled: true, + ServiceMonitor: ptr.To(false), + }, + isOpenShift: true, + expectedResult: false, + }, + { + name: "ServiceMonitor nil on OpenShift defaults to true", + config: MonitoringConfig{ + Enabled: true, + ServiceMonitor: nil, + }, + isOpenShift: true, + expectedResult: true, + }, + { + name: "ServiceMonitor nil on non-OpenShift defaults to false", + config: MonitoringConfig{ + Enabled: true, + ServiceMonitor: nil, + }, + isOpenShift: false, + expectedResult: false, + }, + { + name: "ServiceMonitor explicitly true overrides platform on non-OpenShift", + config: MonitoringConfig{ + Enabled: true, + ServiceMonitor: ptr.To(true), + }, + isOpenShift: false, + expectedResult: true, + }, + { + name: "ServiceMonitor explicitly false overrides platform on OpenShift", + config: MonitoringConfig{ + Enabled: true, + ServiceMonitor: ptr.To(false), + }, + isOpenShift: true, + expectedResult: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.config.IsServiceMonitorEnabled(tt.isOpenShift) + if result != tt.expectedResult { + t.Errorf("IsServiceMonitorEnabled() = %v, want %v", result, tt.expectedResult) + } + }) + } +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b082eb7b7..8ae01e7d2 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -184,7 +184,7 @@ func (in *CTlogSpec) DeepCopyInto(out *CTlogSpec) { *out = make([]SecretKeySelector, len(*in)) copy(*out, *in) } - out.Monitoring = in.Monitoring + in.Monitoring.DeepCopyInto(&out.Monitoring) in.Trillian.DeepCopyInto(&out.Trillian) if in.ServerConfigRef != nil { in, out := &in.ServerConfigRef, &out.ServerConfigRef @@ -514,7 +514,7 @@ func (in *FulcioSpec) DeepCopyInto(out *FulcioSpec) { in.Ctlog.DeepCopyInto(&out.Ctlog) in.Config.DeepCopyInto(&out.Config) in.Certificate.DeepCopyInto(&out.Certificate) - out.Monitoring = in.Monitoring + in.Monitoring.DeepCopyInto(&out.Monitoring) if in.TrustedCA != nil { in, out := &in.TrustedCA, &out.TrustedCA *out = new(LocalObjectReference) @@ -602,6 +602,11 @@ func (in *LocalObjectReference) DeepCopy() *LocalObjectReference { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MonitoringConfig) DeepCopyInto(out *MonitoringConfig) { *out = *in + if in.ServiceMonitor != nil { + in, out := &in.ServiceMonitor, &out.ServiceMonitor + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MonitoringConfig. @@ -617,7 +622,7 @@ func (in *MonitoringConfig) DeepCopy() *MonitoringConfig { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MonitoringWithTLogConfig) DeepCopyInto(out *MonitoringWithTLogConfig) { *out = *in - out.MonitoringConfig = in.MonitoringConfig + in.MonitoringConfig.DeepCopyInto(&out.MonitoringConfig) out.TLog = in.TLog } @@ -921,7 +926,7 @@ func (in *RekorSpec) DeepCopyInto(out *RekorSpec) { } in.Trillian.DeepCopyInto(&out.Trillian) in.ExternalAccess.DeepCopyInto(&out.ExternalAccess) - out.Monitoring = in.Monitoring + in.Monitoring.DeepCopyInto(&out.Monitoring) in.RekorSearchUI.DeepCopyInto(&out.RekorSearchUI) in.Signer.DeepCopyInto(&out.Signer) in.Attestations.DeepCopyInto(&out.Attestations) @@ -1348,7 +1353,7 @@ func (in *TimestampAuthoritySpec) DeepCopyInto(out *TimestampAuthoritySpec) { in.PodRequirements.DeepCopyInto(&out.PodRequirements) in.ExternalAccess.DeepCopyInto(&out.ExternalAccess) in.Signer.DeepCopyInto(&out.Signer) - out.Monitoring = in.Monitoring + in.Monitoring.DeepCopyInto(&out.Monitoring) if in.TrustedCA != nil { in, out := &in.TrustedCA, &out.TrustedCA *out = new(LocalObjectReference) @@ -1589,7 +1594,7 @@ func (in *TrillianService) DeepCopy() *TrillianService { func (in *TrillianSpec) DeepCopyInto(out *TrillianSpec) { *out = *in in.Db.DeepCopyInto(&out.Db) - out.Monitoring = in.Monitoring + in.Monitoring.DeepCopyInto(&out.Monitoring) in.LogServer.DeepCopyInto(&out.LogServer) in.LogSigner.DeepCopyInto(&out.LogSigner) if in.TrustedCA != nil { diff --git a/config/crd/bases/rhtas.redhat.com_ctlogs.yaml b/config/crd/bases/rhtas.redhat.com_ctlogs.yaml index 08bcdacae..9f409b1fe 100644 --- a/config/crd/bases/rhtas.redhat.com_ctlogs.yaml +++ b/config/crd/bases/rhtas.redhat.com_ctlogs.yaml @@ -971,6 +971,11 @@ spec: x-kubernetes-validations: - message: Feature cannot be disabled rule: (self || !oldSelf) + serviceMonitor: + description: |- + If true, the Operator will create ServiceMonitor resources for metrics collection. + When not specified, defaults to true on OpenShift and false on other platforms. + type: boolean required: - enabled type: object diff --git a/config/crd/bases/rhtas.redhat.com_fulcios.yaml b/config/crd/bases/rhtas.redhat.com_fulcios.yaml index 17dbbb567..1fc4c1d09 100644 --- a/config/crd/bases/rhtas.redhat.com_fulcios.yaml +++ b/config/crd/bases/rhtas.redhat.com_fulcios.yaml @@ -1290,6 +1290,11 @@ spec: x-kubernetes-validations: - message: Feature cannot be disabled rule: (self || !oldSelf) + serviceMonitor: + description: |- + If true, the Operator will create ServiceMonitor resources for metrics collection. + When not specified, defaults to true on OpenShift and false on other platforms. + type: boolean required: - enabled type: object diff --git a/config/crd/bases/rhtas.redhat.com_rekors.yaml b/config/crd/bases/rhtas.redhat.com_rekors.yaml index 185d9ca60..2440dff08 100644 --- a/config/crd/bases/rhtas.redhat.com_rekors.yaml +++ b/config/crd/bases/rhtas.redhat.com_rekors.yaml @@ -1263,6 +1263,11 @@ spec: x-kubernetes-validations: - message: Feature cannot be disabled rule: (self || !oldSelf) + serviceMonitor: + description: |- + If true, the Operator will create ServiceMonitor resources for metrics collection. + When not specified, defaults to true on OpenShift and false on other platforms. + type: boolean tlog: description: Configuration for Rekor transparency log monitoring properties: diff --git a/config/crd/bases/rhtas.redhat.com_securesigns.yaml b/config/crd/bases/rhtas.redhat.com_securesigns.yaml index 81653c326..2382e3951 100644 --- a/config/crd/bases/rhtas.redhat.com_securesigns.yaml +++ b/config/crd/bases/rhtas.redhat.com_securesigns.yaml @@ -994,6 +994,11 @@ spec: x-kubernetes-validations: - message: Feature cannot be disabled rule: (self || !oldSelf) + serviceMonitor: + description: |- + If true, the Operator will create ServiceMonitor resources for metrics collection. + When not specified, defaults to true on OpenShift and false on other platforms. + type: boolean required: - enabled type: object @@ -2526,6 +2531,11 @@ spec: x-kubernetes-validations: - message: Feature cannot be disabled rule: (self || !oldSelf) + serviceMonitor: + description: |- + If true, the Operator will create ServiceMonitor resources for metrics collection. + When not specified, defaults to true on OpenShift and false on other platforms. + type: boolean required: - enabled type: object @@ -3876,6 +3886,11 @@ spec: x-kubernetes-validations: - message: Feature cannot be disabled rule: (self || !oldSelf) + serviceMonitor: + description: |- + If true, the Operator will create ServiceMonitor resources for metrics collection. + When not specified, defaults to true on OpenShift and false on other platforms. + type: boolean tlog: description: Configuration for Rekor transparency log monitoring properties: @@ -5494,6 +5509,11 @@ spec: x-kubernetes-validations: - message: Feature cannot be disabled rule: (self || !oldSelf) + serviceMonitor: + description: |- + If true, the Operator will create ServiceMonitor resources for metrics collection. + When not specified, defaults to true on OpenShift and false on other platforms. + type: boolean required: - enabled type: object @@ -8631,6 +8651,11 @@ spec: x-kubernetes-validations: - message: Feature cannot be disabled rule: (self || !oldSelf) + serviceMonitor: + description: |- + If true, the Operator will create ServiceMonitor resources for metrics collection. + When not specified, defaults to true on OpenShift and false on other platforms. + type: boolean required: - enabled type: object diff --git a/config/crd/bases/rhtas.redhat.com_timestampauthorities.yaml b/config/crd/bases/rhtas.redhat.com_timestampauthorities.yaml index c54345948..c9e02a24f 100644 --- a/config/crd/bases/rhtas.redhat.com_timestampauthorities.yaml +++ b/config/crd/bases/rhtas.redhat.com_timestampauthorities.yaml @@ -1003,6 +1003,11 @@ spec: x-kubernetes-validations: - message: Feature cannot be disabled rule: (self || !oldSelf) + serviceMonitor: + description: |- + If true, the Operator will create ServiceMonitor resources for metrics collection. + When not specified, defaults to true on OpenShift and false on other platforms. + type: boolean required: - enabled type: object diff --git a/config/crd/bases/rhtas.redhat.com_trillians.yaml b/config/crd/bases/rhtas.redhat.com_trillians.yaml index a24a71657..883bf14b5 100644 --- a/config/crd/bases/rhtas.redhat.com_trillians.yaml +++ b/config/crd/bases/rhtas.redhat.com_trillians.yaml @@ -205,6 +205,11 @@ spec: x-kubernetes-validations: - message: Feature cannot be disabled rule: (self || !oldSelf) + serviceMonitor: + description: |- + If true, the Operator will create ServiceMonitor resources for metrics collection. + When not specified, defaults to true on OpenShift and false on other platforms. + type: boolean required: - enabled type: object diff --git a/internal/controller/ctlog/actions/monitoring.go b/internal/controller/ctlog/actions/monitoring.go index 1975438a4..e95dffd69 100644 --- a/internal/controller/ctlog/actions/monitoring.go +++ b/internal/controller/ctlog/actions/monitoring.go @@ -30,7 +30,9 @@ func (i monitoringAction) Name() string { func (i monitoringAction) CanHandle(_ context.Context, instance *rhtasv1alpha1.CTlog) bool { c := meta.FindStatusCondition(instance.Status.Conditions, constants.Ready) - return (c.Reason == constants.Creating || c.Reason == constants.Ready) && instance.Spec.Monitoring.Enabled + return (c.Reason == constants.Creating || c.Reason == constants.Ready) && + instance.Spec.Monitoring.Enabled && + instance.Spec.Monitoring.IsServiceMonitorEnabled(kubernetes.IsOpenShift()) } func (i monitoringAction) Handle(ctx context.Context, instance *rhtasv1alpha1.CTlog) *action.Result { diff --git a/internal/controller/fulcio/actions/monitoring.go b/internal/controller/fulcio/actions/monitoring.go index fe159068e..9ccf1524a 100644 --- a/internal/controller/fulcio/actions/monitoring.go +++ b/internal/controller/fulcio/actions/monitoring.go @@ -30,7 +30,9 @@ func (i monitoringAction) Name() string { func (i monitoringAction) CanHandle(_ context.Context, instance *rhtasv1alpha1.Fulcio) bool { c := meta.FindStatusCondition(instance.Status.Conditions, constants.Ready) - return (c.Reason == constants.Creating || c.Reason == constants.Ready) && instance.Spec.Monitoring.Enabled + return (c.Reason == constants.Creating || c.Reason == constants.Ready) && + instance.Spec.Monitoring.Enabled && + instance.Spec.Monitoring.IsServiceMonitorEnabled(kubernetes.IsOpenShift()) } func (i monitoringAction) Handle(ctx context.Context, instance *rhtasv1alpha1.Fulcio) *action.Result { diff --git a/internal/controller/rekor/actions/monitor/helper.go b/internal/controller/rekor/actions/monitor/helper.go index 3e2ee2c10..027769793 100644 --- a/internal/controller/rekor/actions/monitor/helper.go +++ b/internal/controller/rekor/actions/monitor/helper.go @@ -3,8 +3,10 @@ package monitor import ( "github.com/securesign/operator/api/v1alpha1" "github.com/securesign/operator/internal/utils" + "github.com/securesign/operator/internal/utils/kubernetes" ) func enabled(instance *v1alpha1.Rekor) bool { - return utils.IsEnabled(&instance.Spec.Monitoring.TLog.Enabled) + return utils.IsEnabled(&instance.Spec.Monitoring.TLog.Enabled) && + instance.Spec.Monitoring.IsServiceMonitorEnabled(kubernetes.IsOpenShift()) } diff --git a/internal/controller/rekor/actions/server/monitoring.go b/internal/controller/rekor/actions/server/monitoring.go index 709b248c5..fd2f0bc8d 100644 --- a/internal/controller/rekor/actions/server/monitoring.go +++ b/internal/controller/rekor/actions/server/monitoring.go @@ -32,7 +32,9 @@ func (i monitoringAction) Name() string { func (i monitoringAction) CanHandle(_ context.Context, instance *rhtasv1alpha1.Rekor) bool { c := meta.FindStatusCondition(instance.Status.Conditions, constants.Ready) - return (c.Reason == constants.Creating || c.Reason == constants.Ready) && instance.Spec.Monitoring.Enabled + return (c.Reason == constants.Creating || c.Reason == constants.Ready) && + instance.Spec.Monitoring.Enabled && + instance.Spec.Monitoring.IsServiceMonitorEnabled(kubernetes.IsOpenShift()) } func (i monitoringAction) Handle(ctx context.Context, instance *rhtasv1alpha1.Rekor) *action.Result { diff --git a/internal/controller/trillian/actions/logserver/monitoring.go b/internal/controller/trillian/actions/logserver/monitoring.go index 8a83351af..52cb8ba67 100644 --- a/internal/controller/trillian/actions/logserver/monitoring.go +++ b/internal/controller/trillian/actions/logserver/monitoring.go @@ -32,7 +32,9 @@ func (i monitoringAction) Name() string { func (i monitoringAction) CanHandle(_ context.Context, instance *rhtasv1alpha1.Trillian) bool { c := meta.FindStatusCondition(instance.Status.Conditions, constants.Ready) - return (c.Reason == constants.Creating || c.Reason == constants.Ready) && instance.Spec.Monitoring.Enabled + return (c.Reason == constants.Creating || c.Reason == constants.Ready) && + instance.Spec.Monitoring.Enabled && + instance.Spec.Monitoring.IsServiceMonitorEnabled(kubernetes.IsOpenShift()) } func (i monitoringAction) Handle(ctx context.Context, instance *rhtasv1alpha1.Trillian) *action.Result { diff --git a/internal/controller/trillian/actions/logsigner/monitoring.go b/internal/controller/trillian/actions/logsigner/monitoring.go index a53d82998..160986c10 100644 --- a/internal/controller/trillian/actions/logsigner/monitoring.go +++ b/internal/controller/trillian/actions/logsigner/monitoring.go @@ -32,7 +32,9 @@ func (i monitoringAction) Name() string { func (i monitoringAction) CanHandle(_ context.Context, instance *rhtasv1alpha1.Trillian) bool { c := meta.FindStatusCondition(instance.Status.Conditions, constants.Ready) - return (c.Reason == constants.Creating || c.Reason == constants.Ready) && instance.Spec.Monitoring.Enabled + return (c.Reason == constants.Creating || c.Reason == constants.Ready) && + instance.Spec.Monitoring.Enabled && + instance.Spec.Monitoring.IsServiceMonitorEnabled(kubernetes.IsOpenShift()) } func (i monitoringAction) Handle(ctx context.Context, instance *rhtasv1alpha1.Trillian) *action.Result { diff --git a/internal/controller/tsa/actions/monitoring.go b/internal/controller/tsa/actions/monitoring.go index 1b30faa75..03b9fa2a4 100644 --- a/internal/controller/tsa/actions/monitoring.go +++ b/internal/controller/tsa/actions/monitoring.go @@ -30,7 +30,9 @@ func (i monitoringAction) Name() string { func (i monitoringAction) CanHandle(_ context.Context, instance *rhtasv1alpha1.TimestampAuthority) bool { c := meta.FindStatusCondition(instance.Status.Conditions, constants.Ready) - return (c.Reason == constants.Creating || c.Reason == constants.Ready) && instance.Spec.Monitoring.Enabled + return (c.Reason == constants.Creating || c.Reason == constants.Ready) && + instance.Spec.Monitoring.Enabled && + instance.Spec.Monitoring.IsServiceMonitorEnabled(kubernetes.IsOpenShift()) } func (i monitoringAction) Handle(ctx context.Context, instance *rhtasv1alpha1.TimestampAuthority) *action.Result {