diff --git a/operator/cnpg-plugins/sidecar-injector/internal/config/config.go b/operator/cnpg-plugins/sidecar-injector/internal/config/config.go index c2df32128..d5d933faa 100644 --- a/operator/cnpg-plugins/sidecar-injector/internal/config/config.go +++ b/operator/cnpg-plugins/sidecar-injector/internal/config/config.go @@ -27,6 +27,7 @@ const ( documentDbCredentialSecretParameter = "documentDbCredentialSecret" otelCollectorImageParameter = "otelCollectorImage" otelConfigMapNameParameter = "otelConfigMapName" + otelConfigHashParameter = "otelConfigHash" otelMemoryRequestParameter = "otelMemoryRequest" otelMemoryLimitParameter = "otelMemoryLimit" otelCPURequestParameter = "otelCpuRequest" @@ -47,6 +48,7 @@ type Configuration struct { DocumentDbCredentialSecret string OtelCollectorImage string OtelConfigMapName string + OtelConfigHash string OTelMemoryRequest string OTelMemoryLimit string OTelCPURequest string @@ -84,6 +86,8 @@ func FromParameters( gatewayImage := helper.Parameters[gatewayImageParameter] credentialSecret := helper.Parameters[documentDbCredentialSecretParameter] pullPolicy := parsePullPolicy(helper.Parameters[gatewayImagePullPolicyParameter]) + otelCollectorImage := helper.Parameters[otelCollectorImageParameter] + otelConfigMapName := helper.Parameters[otelConfigMapNameParameter] validateQuantityParameters(helper, &validationErrors, gatewayMemoryRequestParameter, gatewayMemoryLimitParameter, @@ -108,6 +112,37 @@ func FromParameters( } } + requiredOtelParameters := []string{ + otelCollectorImageParameter, + otelConfigMapNameParameter, + } + otelParameters := append([]string{ + prometheusPortParameter, + otelConfigHashParameter, + otelMemoryRequestParameter, + otelMemoryLimitParameter, + otelCPURequestParameter, + otelCPULimitParameter, + }, requiredOtelParameters...) + otelConfigured := false + for _, parameter := range otelParameters { + otelConfigured = otelConfigured || helper.Parameters[parameter] != "" + } + if otelConfigured { + for _, parameter := range requiredOtelParameters { + if helper.Parameters[parameter] == "" { + validationErrors = append( + validationErrors, + validation.BuildErrorForParameter( + helper, + parameter, + "required when any OTel sidecar parameter is configured", + ), + ) + } + } + } + configuration := &Configuration{ Labels: labels, Annotations: annotations, @@ -118,8 +153,9 @@ func FromParameters( GatewayCPURequest: helper.Parameters[gatewayCPURequestParameter], GatewayCPULimit: helper.Parameters[gatewayCPULimitParameter], DocumentDbCredentialSecret: credentialSecret, - OtelCollectorImage: helper.Parameters[otelCollectorImageParameter], - OtelConfigMapName: helper.Parameters[otelConfigMapNameParameter], + OtelCollectorImage: otelCollectorImage, + OtelConfigMapName: otelConfigMapName, + OtelConfigHash: helper.Parameters[otelConfigHashParameter], OTelMemoryRequest: helper.Parameters[otelMemoryRequestParameter], OTelMemoryLimit: helper.Parameters[otelMemoryLimitParameter], OTelCPURequest: helper.Parameters[otelCPURequestParameter], @@ -231,10 +267,16 @@ func (config *Configuration) ToParameters() (map[string]string, error) { setIfNotEmpty(gatewayCPURequestParameter, config.GatewayCPURequest) setIfNotEmpty(gatewayCPULimitParameter, config.GatewayCPULimit) result[documentDbCredentialSecretParameter] = config.DocumentDbCredentialSecret + setIfNotEmpty(otelCollectorImageParameter, config.OtelCollectorImage) + setIfNotEmpty(otelConfigMapNameParameter, config.OtelConfigMapName) + setIfNotEmpty(otelConfigHashParameter, config.OtelConfigHash) setIfNotEmpty(otelMemoryRequestParameter, config.OTelMemoryRequest) setIfNotEmpty(otelMemoryLimitParameter, config.OTelMemoryLimit) setIfNotEmpty(otelCPURequestParameter, config.OTelCPURequest) setIfNotEmpty(otelCPULimitParameter, config.OTelCPULimit) + if config.PrometheusPort > 0 { + result[prometheusPortParameter] = strconv.FormatInt(int64(config.PrometheusPort), 10) + } return result, nil } diff --git a/operator/cnpg-plugins/sidecar-injector/internal/config/config_test.go b/operator/cnpg-plugins/sidecar-injector/internal/config/config_test.go index 1712b6038..585e47510 100644 --- a/operator/cnpg-plugins/sidecar-injector/internal/config/config_test.go +++ b/operator/cnpg-plugins/sidecar-injector/internal/config/config_test.go @@ -84,12 +84,105 @@ func TestFromParameters(t *testing.T) { } }) + t.Run("parses OTel monitoring parameters", func(t *testing.T) { + helper := &common.Plugin{Parameters: map[string]string{ + "otelCollectorImage": "otel/opentelemetry-collector-contrib:test", + "otelConfigMapName": "demo-otel-config", + "otelConfigHash": "abc123", + }} + config, errs := FromParameters(helper) + if len(errs) != 0 { + t.Fatalf("unexpected validation errors: %v", errs) + } + if config.OtelCollectorImage != "otel/opentelemetry-collector-contrib:test" { + t.Errorf("OtelCollectorImage = %q", config.OtelCollectorImage) + } + if config.OtelConfigMapName != "demo-otel-config" { + t.Errorf("OtelConfigMapName = %q", config.OtelConfigMapName) + } + if config.OtelConfigHash != "abc123" { + t.Errorf("OtelConfigHash = %q", config.OtelConfigHash) + } + }) + + for _, tt := range []struct { + name string + parameters map[string]string + wantErrors int + }{ + { + name: "rejects collector image without config map", + parameters: map[string]string{ + "otelCollectorImage": "otel/opentelemetry-collector-contrib:test", + }, + wantErrors: 1, + }, + { + name: "rejects config map without collector image", + parameters: map[string]string{ + "otelConfigMapName": "demo-otel-config", + }, + wantErrors: 1, + }, + { + name: "rejects optional OTel parameter without required parameters", + parameters: map[string]string{ + "prometheusPort": "8888", + }, + wantErrors: 2, + }, + { + name: "rejects config hash without required parameters", + parameters: map[string]string{ + "otelConfigHash": "abc123", + }, + wantErrors: 2, + }, + { + name: "rejects memory request without required parameters", + parameters: map[string]string{ + "otelMemoryRequest": "64Mi", + }, + wantErrors: 2, + }, + { + name: "rejects memory limit without required parameters", + parameters: map[string]string{ + "otelMemoryLimit": "128Mi", + }, + wantErrors: 2, + }, + { + name: "rejects CPU request without required parameters", + parameters: map[string]string{ + "otelCpuRequest": "100m", + }, + wantErrors: 2, + }, + { + name: "rejects CPU limit without required parameters", + parameters: map[string]string{ + "otelCpuLimit": "300m", + }, + wantErrors: 2, + }, + } { + t.Run(tt.name, func(t *testing.T) { + _, errs := FromParameters(&common.Plugin{Parameters: tt.parameters}) + if len(errs) != tt.wantErrors { + t.Fatalf("validation errors = %d, want %d: %v", len(errs), tt.wantErrors, errs) + } + }) + } + t.Run("resource parameters from parameters", func(t *testing.T) { helper := &common.Plugin{Parameters: map[string]string{ "gatewayMemoryRequest": "768Mi", "gatewayMemoryLimit": "3Gi", "gatewayCpuRequest": "500m", "gatewayCpuLimit": "2", + "otelCollectorImage": "otel:latest", + "otelConfigMapName": "otel-config", "otelMemoryRequest": "64Mi", "otelMemoryLimit": "128Mi", "otelCpuRequest": "100m", @@ -130,6 +223,9 @@ func TestToParametersRoundTrip(t *testing.T) { GatewayMemoryLimit: "3Gi", GatewayCPURequest: "500m", GatewayCPULimit: "2", + OtelCollectorImage: "otel:latest", + OtelConfigMapName: "otel-config", + OtelConfigHash: "abc123", OTelMemoryRequest: "64Mi", OTelMemoryLimit: "128Mi", OTelCPURequest: "100m", @@ -164,6 +260,15 @@ func TestToParametersRoundTrip(t *testing.T) { if restored.GatewayCPULimit != original.GatewayCPULimit { t.Errorf("round-trip gateway cpu limit = %q, want %q", restored.GatewayCPULimit, original.GatewayCPULimit) } + if restored.OtelCollectorImage != original.OtelCollectorImage { + t.Errorf("round-trip OTel collector image = %q, want %q", restored.OtelCollectorImage, original.OtelCollectorImage) + } + if restored.OtelConfigMapName != original.OtelConfigMapName { + t.Errorf("round-trip OTel config map = %q, want %q", restored.OtelConfigMapName, original.OtelConfigMapName) + } + if restored.OtelConfigHash != original.OtelConfigHash { + t.Errorf("round-trip OTel config hash = %q, want %q", restored.OtelConfigHash, original.OtelConfigHash) + } if restored.OTelMemoryRequest != original.OTelMemoryRequest { t.Errorf("round-trip otel memory request = %q, want %q", restored.OTelMemoryRequest, original.OTelMemoryRequest) } diff --git a/operator/cnpg-plugins/sidecar-injector/internal/lifecycle/lifecycle.go b/operator/cnpg-plugins/sidecar-injector/internal/lifecycle/lifecycle.go index d2e0536f7..5bfcfbe09 100644 --- a/operator/cnpg-plugins/sidecar-injector/internal/lifecycle/lifecycle.go +++ b/operator/cnpg-plugins/sidecar-injector/internal/lifecycle/lifecycle.go @@ -270,7 +270,7 @@ func (impl Implementation) reconcileMetadata( }) } - otelSidecar := newOtelCollectorSidecar(configuration.OtelCollectorImage, cluster.Name) + otelSidecar := newOtelCollectorSidecar(configuration.OtelCollectorImage) if resources := buildResources( configuration.OTelCPURequest, configuration.OTelCPULimit, @@ -490,6 +490,7 @@ func injectGatewayOTelEnv(pod *corev1.Pod) { // otelCollectorContainerName is the name of the injected OpenTelemetry // Collector sidecar. const otelCollectorContainerName = "otel-collector" +const otelMonitorRoleName = "otel_monitor" // gatewaySecurityContext returns the SecurityContext for the documentdb-gateway // sidecar: the shared PSA-restricted hardening plus an explicit UID/GID of @@ -506,8 +507,8 @@ func gatewaySecurityContext() *corev1.SecurityContext { // the caller). It carries the shared PSA-restricted SecurityContext without an // explicit UID so the upstream collector image keeps its own baked-in non-root // user (UID 10001); PSA "restricted" only requires runAsNonRoot, not a fixed -// UID. clusterName selects the CNPG-managed "-app" credential secret. -func newOtelCollectorSidecar(image, clusterName string) *corev1.Container { +// UID. +func newOtelCollectorSidecar(image string) *corev1.Container { return &corev1.Container{ Name: otelCollectorContainerName, Image: image, @@ -515,10 +516,9 @@ func newOtelCollectorSidecar(image, clusterName string) *corev1.Container { "--config=file:/config/static.yaml", "--config=file:/config/dynamic.yaml", }, - // PGUSER and PGPASSWORD are sourced from the CNPG-managed application secret - // ("-app"). CNPG auto-creates this secret with "username" and "password" - // keys for the application database user. The OTel Collector's sqlquery receiver - // uses these credentials to connect to PostgreSQL and collect health metrics. + // PostgreSQL currently uses trust authentication, so injecting a password + // would not enforce access control. Use the dedicated password-disabled + // identity directly until database authentication is tightened. Env: []corev1.EnvVar{ { Name: "POD_NAME", @@ -529,26 +529,8 @@ func newOtelCollectorSidecar(image, clusterName string) *corev1.Container { }, }, { - Name: "PGUSER", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: clusterName + "-app", - }, - Key: "username", - }, - }, - }, - { - Name: "PGPASSWORD", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: clusterName + "-app", - }, - Key: "password", - }, - }, + Name: "PGUSER", + Value: otelMonitorRoleName, }, }, VolumeMounts: []corev1.VolumeMount{ diff --git a/operator/cnpg-plugins/sidecar-injector/internal/lifecycle/lifecycle_test.go b/operator/cnpg-plugins/sidecar-injector/internal/lifecycle/lifecycle_test.go index e7c4b4700..03b5699f9 100644 --- a/operator/cnpg-plugins/sidecar-injector/internal/lifecycle/lifecycle_test.go +++ b/operator/cnpg-plugins/sidecar-injector/internal/lifecycle/lifecycle_test.go @@ -352,7 +352,7 @@ func TestGatewaySecurityContext_PSARestrictedAsUID1000(t *testing.T) { // restricted and, unlike the gateway, does not force a UID so the collector // image keeps its own non-root user (UID 10001). func TestNewOtelCollectorSidecar_Hardened(t *testing.T) { - c := newOtelCollectorSidecar("otel/opentelemetry-collector-contrib:test", "demo") + c := newOtelCollectorSidecar("otel/opentelemetry-collector-contrib:test") if c.Name != otelCollectorContainerName { t.Fatalf("container name = %q, want %q", c.Name, otelCollectorContainerName) @@ -365,3 +365,22 @@ func TestNewOtelCollectorSidecar_Hardened(t *testing.T) { t.Errorf("otel-collector must not force a GID, got %d", *c.SecurityContext.RunAsGroup) } } + +// TestNewOtelCollectorSidecar_MonitorUser asserts the sidecar connects using +// the dedicated monitoring identity without injecting an unused password. +func TestNewOtelCollectorSidecar_MonitorUser(t *testing.T) { + c := newOtelCollectorSidecar("otel/opentelemetry-collector-contrib:test") + + for _, env := range c.Env { + if env.Name == "PGPASSWORD" { + t.Fatal("PGPASSWORD must not be injected while PostgreSQL uses trust authentication") + } + if env.Name == "PGUSER" { + if env.Value != otelMonitorRoleName { + t.Fatalf("PGUSER = %q, want %q", env.Value, otelMonitorRoleName) + } + return + } + } + t.Fatal("missing PGUSER env var") +} diff --git a/operator/cnpg-plugins/sidecar-injector/internal/operator/validation.go b/operator/cnpg-plugins/sidecar-injector/internal/operator/validation.go index b191ddc4f..fabd1e9c6 100644 --- a/operator/cnpg-plugins/sidecar-injector/internal/operator/validation.go +++ b/operator/cnpg-plugins/sidecar-injector/internal/operator/validation.go @@ -66,7 +66,10 @@ func (Implementation) ValidateClusterChange( var newConfiguration *config.Configuration newConfiguration, result.ValidationErrors = config.FromParameters(newClusterHelper) oldConfiguration, _ := config.FromParameters(oldClusterHelper) - result.ValidationErrors = config.ValidateChanges(oldConfiguration, newConfiguration, newClusterHelper) + result.ValidationErrors = append( + result.ValidationErrors, + config.ValidateChanges(oldConfiguration, newConfiguration, newClusterHelper)..., + ) return result, nil } diff --git a/operator/cnpg-plugins/sidecar-injector/internal/operator/validation_test.go b/operator/cnpg-plugins/sidecar-injector/internal/operator/validation_test.go new file mode 100644 index 000000000..0c63b44d2 --- /dev/null +++ b/operator/cnpg-plugins/sidecar-injector/internal/operator/validation_test.go @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package operator + +import ( + "context" + "encoding/json" + "testing" + + cnpgv1 "github.com/cloudnative-pg/api/pkg/api/v1" + cnpgoperator "github.com/cloudnative-pg/cnpg-i/pkg/operator" + + "github.com/documentdb/cnpg-i-sidecar-injector/pkg/metadata" +) + +func TestValidateClusterChangeRetainsParameterErrors(t *testing.T) { + oldCluster := &cnpgv1.Cluster{} + newCluster := &cnpgv1.Cluster{ + Spec: cnpgv1.ClusterSpec{ + Plugins: []cnpgv1.PluginConfiguration{ + { + Name: metadata.PluginName, + Parameters: map[string]string{ + "otelCollectorImage": "otel/opentelemetry-collector-contrib:test", + }, + }, + }, + }, + } + + oldJSON, err := json.Marshal(oldCluster) + if err != nil { + t.Fatalf("marshal old cluster: %v", err) + } + newJSON, err := json.Marshal(newCluster) + if err != nil { + t.Fatalf("marshal new cluster: %v", err) + } + + result, err := (Implementation{}).ValidateClusterChange( + context.Background(), + &cnpgoperator.OperatorValidateClusterChangeRequest{ + OldCluster: oldJSON, + NewCluster: newJSON, + }, + ) + if err != nil { + t.Fatalf("ValidateClusterChange() error: %v", err) + } + if got, want := len(result.ValidationErrors), 1; got != want { + t.Fatalf("validation errors = %d, want %d: %v", got, want, result.ValidationErrors) + } +} diff --git a/operator/documentdb-helm-chart/templates/05_clusterrole.yaml b/operator/documentdb-helm-chart/templates/05_clusterrole.yaml index cff4bae4f..b94fff5b3 100644 --- a/operator/documentdb-helm-chart/templates/05_clusterrole.yaml +++ b/operator/documentdb-helm-chart/templates/05_clusterrole.yaml @@ -31,7 +31,7 @@ rules: resources: ["serviceexports", "multiclusterservices", "serviceimports", "internalserviceexports"] verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] # Secrets: certificate_controller reads cert-manager-issued TLS secrets to -# stamp into Cluster spec; no controller writes Secrets. Read-only. +# stamp into Cluster spec. No controller writes Secrets. - apiGroups: [""] resources: ["secrets"] verbs: ["get", "list", "watch"] diff --git a/operator/documentdb-helm-chart/tests/05_clusterrole_test.yaml b/operator/documentdb-helm-chart/tests/05_clusterrole_test.yaml index 0d749fb16..6512b0623 100644 --- a/operator/documentdb-helm-chart/tests/05_clusterrole_test.yaml +++ b/operator/documentdb-helm-chart/tests/05_clusterrole_test.yaml @@ -102,7 +102,7 @@ tests: resources: ["serviceexports", "multiclusterservices", "serviceimports", "internalserviceexports"] verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] - - it: should include secrets permissions (read-only) + - it: should include read-only secrets permissions asserts: - contains: path: rules diff --git a/operator/src/config/rbac/role.yaml b/operator/src/config/rbac/role.yaml index 7c481a3a1..38de72b77 100644 --- a/operator/src/config/rbac/role.yaml +++ b/operator/src/config/rbac/role.yaml @@ -8,8 +8,9 @@ rules: - "" resources: - persistentvolumeclaims - - secrets verbs: + - create + - delete - get - list - watch @@ -23,6 +24,14 @@ rules: - patch - update - watch +- apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch - apiGroups: - cert-manager.io resources: diff --git a/operator/src/internal/cnpg/cnpg_cluster.go b/operator/src/internal/cnpg/cnpg_cluster.go index 0268222ea..677b0d844 100644 --- a/operator/src/internal/cnpg/cnpg_cluster.go +++ b/operator/src/internal/cnpg/cnpg_cluster.go @@ -139,6 +139,7 @@ func GetCnpgClusterSpec(req ctrl.Request, documentdb *dbpreview.DocumentDB, docu spec.MaxStopDelay = getMaxStopDelayOrDefault(documentdb) applyPostgresProcessIdentity(&spec, documentdb) applyIOUringSeccomp(&spec, documentdb) + applyOtelMonitorRole(&spec, documentdb) return spec }(), @@ -380,7 +381,49 @@ func applyIOUringSeccomp(spec *cnpgv1.ClusterSpec, documentdb *dbpreview.Documen } } -// buildPostgresConfiguration returns the cnpgv1.PostgresConfiguration block +// applyOtelMonitorRole declares the PostgreSQL identity used by the OTel +// Collector sidecar. When monitoring is disabled, the role remains declared +// with ensure=absent so CNPG removes any role left by an earlier configuration. +// +// PostgreSQL host authentication currently uses trust, so a generated password +// would not be checked. Disable the role password explicitly until authentication +// is tightened rather than creating an unused credential and widening Secret RBAC. +func applyOtelMonitorRole(spec *cnpgv1.ClusterSpec, documentdb *dbpreview.DocumentDB) { + if documentdb == nil { + return + } + if spec.Managed == nil { + spec.Managed = &cnpgv1.ManagedConfiguration{} + } + role := absentOtelMonitorRole() + if documentdb.Spec.Monitoring != nil && documentdb.Spec.Monitoring.Enabled { + // The current health query is SELECT 1, so the role needs LOGIN only + // and is not granted broad monitoring memberships. + role = cnpgv1.RoleConfiguration{ + Name: otelcfg.MonitorRoleName, + Comment: "Dedicated role for the OTel Collector monitoring sidecar", + Ensure: cnpgv1.EnsurePresent, + Login: true, + DisablePassword: true, + // Set the CNPG/CRD-defaulted fields explicitly so the desired role + // matches the API-server-defaulted form stored on the live cluster, + // keeping SyncCnpgCluster's diff stable (no perpetual re-patching). + ConnectionLimit: -1, + Inherit: pointer.Bool(true), + } + } + spec.Managed.Roles = append(spec.Managed.Roles, role) +} + +func absentOtelMonitorRole() cnpgv1.RoleConfiguration { + return cnpgv1.RoleConfiguration{ + Name: otelcfg.MonitorRoleName, + Ensure: cnpgv1.EnsureAbsent, + ConnectionLimit: -1, + Inherit: pointer.Bool(true), + } +} + // for the cluster. // // The operator declares the DocumentDB extension via CNPG's Extensions diff --git a/operator/src/internal/cnpg/cnpg_cluster_test.go b/operator/src/internal/cnpg/cnpg_cluster_test.go index 29c2c56b3..9a2c80f56 100644 --- a/operator/src/internal/cnpg/cnpg_cluster_test.go +++ b/operator/src/internal/cnpg/cnpg_cluster_test.go @@ -908,6 +908,65 @@ var _ = Describe("GetCnpgClusterSpec", func() { Expect(pluginParams).NotTo(HaveKey("otelConfigMapName")) }) + It("declares a password-disabled otel_monitor role when monitoring is enabled", func() { + req := ctrl.Request{} + req.Name = "test-cluster" + req.Namespace = "default" + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: dbpreview.DocumentDBSpec{ + InstancesPerNode: 1, + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{ + PvcSize: "10Gi", + }, + }, + Monitoring: &dbpreview.MonitoringSpec{Enabled: true}, + }, + } + + cluster := GetCnpgClusterSpec(req, documentdb, "test-image:latest", "test-sa", "", true, log) + + Expect(cluster.Spec.Managed).NotTo(BeNil()) + Expect(cluster.Spec.Managed.Roles).To(HaveLen(1)) + role := cluster.Spec.Managed.Roles[0] + Expect(role.Name).To(Equal("otel_monitor")) + Expect(role.Login).To(BeTrue()) + Expect(role.Ensure).To(Equal(cnpgv1.EnsurePresent)) + Expect(role.InRoles).To(BeEmpty()) + Expect(role.PasswordSecret).To(BeNil()) + Expect(role.DisablePassword).To(BeTrue()) + Expect(role.Superuser).To(BeFalse()) + Expect(role.CreateDB).To(BeFalse()) + Expect(role.CreateRole).To(BeFalse()) + }) + + It("declares the otel_monitor role absent when monitoring is disabled", func() { + req := ctrl.Request{} + req.Name = "test-cluster" + req.Namespace = "default" + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cluster", Namespace: "default"}, + Spec: dbpreview.DocumentDBSpec{ + InstancesPerNode: 1, + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{PvcSize: "10Gi"}, + }, + Monitoring: &dbpreview.MonitoringSpec{Enabled: false}, + }, + } + + cluster := GetCnpgClusterSpec(req, documentdb, "test-image:latest", "test-sa", "", true, log) + + Expect(cluster.Spec.Managed).NotTo(BeNil()) + Expect(cluster.Spec.Managed.Roles).To(Equal([]cnpgv1.RoleConfiguration{absentOtelMonitorRole()})) + }) + It("propagates spec.imagePullSecrets to the CNPG cluster spec", func() { req := ctrl.Request{} req.Name = "test-cluster" diff --git a/operator/src/internal/cnpg/cnpg_patch.go b/operator/src/internal/cnpg/cnpg_patch.go index 3fb57af48..bf0717ecb 100644 --- a/operator/src/internal/cnpg/cnpg_patch.go +++ b/operator/src/internal/cnpg/cnpg_patch.go @@ -27,6 +27,8 @@ const ( PatchPathCertificates = "/spec/certificates" PatchPathPostgresPgHBA = "/spec/postgresql/pg_hba" PatchPathManagedServices = "/spec/managed/services/additional" + PatchPathManaged = "/spec/managed" + PatchPathManagedRoles = "/spec/managed/roles" PatchPathSynchronous = "/spec/postgresql/synchronous" PatchPathBootstrap = "/spec/bootstrap" diff --git a/operator/src/internal/cnpg/cnpg_sync.go b/operator/src/internal/cnpg/cnpg_sync.go index fdc34df02..b971db691 100644 --- a/operator/src/internal/cnpg/cnpg_sync.go +++ b/operator/src/internal/cnpg/cnpg_sync.go @@ -15,6 +15,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" + otelcfg "github.com/documentdb/documentdb-operator/internal/otel" util "github.com/documentdb/documentdb-operator/internal/utils" ) @@ -257,6 +258,13 @@ func SyncCnpgCluster( patchOps = append(patchOps, certificatesPatch) } + // Managed roles (the OTel monitoring role) are reconciled independently of + // managed.services (owned by the replication flow via extraOps) so the two + // never clobber each other. + if patch := managedRolesPatch(current, desired); patch != nil { + patchOps = append(patchOps, *patch) + } + // Extra operations (e.g., replication changes) patchOps = append(patchOps, extraOps...) @@ -332,3 +340,65 @@ func getParam(params map[string]string, key string) string { } return params[key] } + +// managedRoles returns the cluster's managed roles, nil-safe against an absent +// managed configuration. +func managedRoles(cluster *cnpgv1.Cluster) []cnpgv1.RoleConfiguration { + if cluster == nil || cluster.Spec.Managed == nil { + return nil + } + return cluster.Spec.Managed.Roles +} + +// managedRolesPatch returns the JSON Patch operation needed to reconcile +// spec.managed.roles. Only the operator-owned OTel role is changed; unrelated +// managed roles and managed services remain untouched. +func managedRolesPatch(current, desired *cnpgv1.Cluster) *JSONPatch { + currentRoles := managedRoles(current) + desiredRoles := managedRoles(desired) + + desiredRole := absentOtelMonitorRole() + for _, role := range desiredRoles { + if role.Name == otelcfg.MonitorRoleName { + desiredRole = role + break + } + } + + roles := make([]cnpgv1.RoleConfiguration, 0, len(currentRoles)+1) + found := false + for _, role := range currentRoles { + if role.Name == otelcfg.MonitorRoleName { + if !found { + roles = append(roles, desiredRole) + found = true + } + continue + } + roles = append(roles, role) + } + if !found { + roles = append(roles, desiredRole) + } + + if reflect.DeepEqual(currentRoles, roles) { + return nil + } + if current.Spec.Managed == nil { + managed := cnpgv1.ManagedConfiguration{} + if desired.Spec.Managed != nil { + managed = *desired.Spec.Managed + } + managed.Roles = roles + return &JSONPatch{ + Op: PatchOpAdd, + Path: PatchPathManaged, + Value: &managed, + } + } + return &JSONPatch{ + Op: PatchOpAdd, + Path: PatchPathManagedRoles, + Value: roles, + } +} diff --git a/operator/src/internal/cnpg/cnpg_sync_test.go b/operator/src/internal/cnpg/cnpg_sync_test.go index 92dcdc176..6ac4c030f 100644 --- a/operator/src/internal/cnpg/cnpg_sync_test.go +++ b/operator/src/internal/cnpg/cnpg_sync_test.go @@ -636,6 +636,166 @@ var _ = Describe("SyncCnpgCluster - mutable spec fields", func() { }) }) +var _ = Describe("SyncCnpgCluster - managed roles", func() { + const namespace = "test-ns" + + otelRole := func() cnpgv1.RoleConfiguration { + return cnpgv1.RoleConfiguration{ + Name: "otel_monitor", + Ensure: cnpgv1.EnsurePresent, + Login: true, + DisablePassword: true, + ConnectionLimit: -1, + Inherit: pointer.Bool(true), + } + } + absentOtelRole := func() cnpgv1.RoleConfiguration { + return cnpgv1.RoleConfiguration{ + Name: "otel_monitor", + Ensure: cnpgv1.EnsureAbsent, + ConnectionLimit: -1, + Inherit: pointer.Bool(true), + } + } + + It("adds the managed role when monitoring is enabled on a cluster without managed config", func() { + current := baseCluster("test-cluster", namespace) + Expect(current.Spec.Managed).To(BeNil()) + desired := current.DeepCopy() + desired.Spec.Managed = &cnpgv1.ManagedConfiguration{ + Roles: []cnpgv1.RoleConfiguration{otelRole()}, + } + + c := buildFakeClient(current).Build() + Expect(SyncCnpgCluster(context.Background(), c, current, desired, nil)).To(Succeed()) + + updated := &cnpgv1.Cluster{} + Expect(c.Get(context.Background(), types.NamespacedName{Name: "test-cluster", Namespace: namespace}, updated)).To(Succeed()) + Expect(updated.Spec.Managed).NotTo(BeNil()) + Expect(updated.Spec.Managed.Roles).To(HaveLen(1)) + Expect(updated.Spec.Managed.Roles[0].Name).To(Equal("otel_monitor")) + Expect(updated.Spec.Managed.Roles[0].DisablePassword).To(BeTrue()) + }) + + It("marks the managed role absent when monitoring is disabled", func() { + current := baseCluster("test-cluster", namespace) + current.Spec.Managed = &cnpgv1.ManagedConfiguration{ + Roles: []cnpgv1.RoleConfiguration{otelRole()}, + } + desired := current.DeepCopy() + desired.Spec.Managed.Roles = []cnpgv1.RoleConfiguration{absentOtelRole()} + + patch := managedRolesPatch(current, desired) + Expect(patch).NotTo(BeNil()) + Expect(patch.Op).To(Equal(PatchOpAdd)) + Expect(patch.Path).To(Equal(PatchPathManagedRoles)) + Expect(patch.Value).To(Equal([]cnpgv1.RoleConfiguration{absentOtelRole()})) + + c := buildFakeClient(current).Build() + Expect(SyncCnpgCluster(context.Background(), c, current, desired, nil)).To(Succeed()) + + updated := &cnpgv1.Cluster{} + Expect(c.Get(context.Background(), types.NamespacedName{Name: "test-cluster", Namespace: namespace}, updated)).To(Succeed()) + Expect(managedRoles(updated)).To(Equal([]cnpgv1.RoleConfiguration{absentOtelRole()})) + }) + + It("keeps the absent role declaration until monitoring is re-enabled", func() { + current := baseCluster("test-cluster", namespace) + current.Spec.Managed = &cnpgv1.ManagedConfiguration{ + Roles: []cnpgv1.RoleConfiguration{absentOtelRole()}, + } + desired := current.DeepCopy() + + Expect(managedRolesPatch(current, desired)).To(BeNil()) + }) + + It("adds an absent declaration to clean up an orphaned database role", func() { + current := baseCluster("test-cluster", namespace) + desired := current.DeepCopy() + desired.Spec.Managed = &cnpgv1.ManagedConfiguration{ + Roles: []cnpgv1.RoleConfiguration{absentOtelRole()}, + } + + c := buildFakeClient(current).Build() + Expect(SyncCnpgCluster(context.Background(), c, current, desired, nil)).To(Succeed()) + + updated := &cnpgv1.Cluster{} + Expect(c.Get(context.Background(), types.NamespacedName{Name: "test-cluster", Namespace: namespace}, updated)).To(Succeed()) + Expect(updated.Spec.Managed.Roles).To(Equal([]cnpgv1.RoleConfiguration{absentOtelRole()})) + }) + + It("preserves other managed roles when marking the monitor role absent", func() { + current := baseCluster("test-cluster", namespace) + customRole := cnpgv1.RoleConfiguration{Name: "custom_role", Login: true} + current.Spec.Managed = &cnpgv1.ManagedConfiguration{ + Roles: []cnpgv1.RoleConfiguration{customRole, otelRole()}, + } + desired := current.DeepCopy() + desired.Spec.Managed.Roles = []cnpgv1.RoleConfiguration{absentOtelRole()} + + patch := managedRolesPatch(current, desired) + Expect(patch).NotTo(BeNil()) + Expect(patch.Value).To(Equal([]cnpgv1.RoleConfiguration{ + customRole, + absentOtelRole(), + })) + }) + + It("preserves managed.services when patching only roles", func() { + current := baseCluster("test-cluster", namespace) + current.Spec.Managed = &cnpgv1.ManagedConfiguration{ + Services: &cnpgv1.ManagedServices{ + Additional: []cnpgv1.ManagedService{ + { + SelectorType: cnpgv1.ServiceSelectorTypeRW, + ServiceTemplate: cnpgv1.ServiceTemplateSpec{ + ObjectMeta: cnpgv1.Metadata{Name: "extra-svc"}, + }, + }, + }, + }, + } + desired := current.DeepCopy() + desired.Spec.Managed.Roles = []cnpgv1.RoleConfiguration{otelRole()} + + c := buildFakeClient(current).Build() + Expect(SyncCnpgCluster(context.Background(), c, current, desired, nil)).To(Succeed()) + + updated := &cnpgv1.Cluster{} + Expect(c.Get(context.Background(), types.NamespacedName{Name: "test-cluster", Namespace: namespace}, updated)).To(Succeed()) + Expect(updated.Spec.Managed.Roles).To(HaveLen(1)) + Expect(updated.Spec.Managed.Services).NotTo(BeNil()) + Expect(updated.Spec.Managed.Services.Additional).To(HaveLen(1)) + Expect(updated.Spec.Managed.Services.Additional[0].ServiceTemplate.ObjectMeta.Name).To(Equal("extra-svc")) + }) + + It("does not patch when managed roles are unchanged", func() { + current := baseCluster("test-cluster", namespace) + current.Spec.Managed = &cnpgv1.ManagedConfiguration{ + Roles: []cnpgv1.RoleConfiguration{otelRole()}, + } + desired := current.DeepCopy() + + c := buildFakeClient(current).Build() + Expect(SyncCnpgCluster(context.Background(), c, current, desired, nil)).To(Succeed()) + + updated := &cnpgv1.Cluster{} + Expect(c.Get(context.Background(), types.NamespacedName{Name: "test-cluster", Namespace: namespace}, updated)).To(Succeed()) + Expect(updated.Spec.Managed.Roles).To(HaveLen(1)) + // No spec drift, so no restart annotation is added. + Expect(updated.Annotations).ToNot(HaveKey("kubectl.kubernetes.io/restartedAt")) + }) + + It("defaults a missing desired monitor role to absent", func() { + current := baseCluster("test-cluster", namespace) + desired := current.DeepCopy() + + patch := managedRolesPatch(current, desired) + Expect(patch).NotTo(BeNil()) + Expect(patch.Path).To(Equal(PatchPathManaged)) + }) +}) + var _ = Describe("Helper functions", func() { It("findExtensionImage returns -1 for cluster without extensions", func() { cluster := &cnpgv1.Cluster{ diff --git a/operator/src/internal/controller/documentdb_controller.go b/operator/src/internal/controller/documentdb_controller.go index 4a8b7dd43..45f8e4ac8 100644 --- a/operator/src/internal/controller/documentdb_controller.go +++ b/operator/src/internal/controller/documentdb_controller.go @@ -173,21 +173,16 @@ func (r *DocumentDBReconciler) Reconcile(ctx context.Context, req ctrl.Request) return result, nil } - // Reconcile OTel Collector ConfigMap when monitoring is enabled. - // When monitoring is disabled or removed, delete the ConfigMap. + // Reconcile OTel resources when monitoring is enabled. // The sidecar itself is added/removed via CNPG plugin parameters; // the operator triggers a rolling restart (via restart annotation) // and CNPG manages the pod rollout. - if documentdb.Spec.Monitoring != nil && documentdb.Spec.Monitoring.Enabled { + monitoringEnabled := documentdb.Spec.Monitoring != nil && documentdb.Spec.Monitoring.Enabled + if monitoringEnabled { if err := r.reconcileOtelConfigMap(ctx, documentdb, req.Namespace); err != nil { logger.Error(err, "Failed to reconcile OTel ConfigMap") return ctrl.Result{RequeueAfter: RequeueAfterShort}, nil } - } else { - if err := r.deleteOtelConfigMap(ctx, documentdb.Name, req.Namespace); err != nil { - logger.Error(err, "Failed to clean up OTel ConfigMap") - return ctrl.Result{RequeueAfter: RequeueAfterShort}, nil - } } if err := r.Client.Get(ctx, types.NamespacedName{Name: desiredCnpgCluster.Name, Namespace: req.Namespace}, currentCnpgCluster); err != nil { @@ -219,6 +214,16 @@ func (r *DocumentDBReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{RequeueAfter: RequeueAfterShort}, nil } + // Delete the OTel ConfigMap only after the CNPG Cluster spec no longer + // references it. If cluster sync fails, retaining it keeps the existing + // sidecar configuration functional and allows a subsequent reconcile to retry. + if !monitoringEnabled { + if err := r.deleteOtelConfigMap(ctx, documentdb.Name, req.Namespace); err != nil { + logger.Error(err, "Failed to clean up OTel ConfigMap") + return ctrl.Result{RequeueAfter: RequeueAfterShort}, nil + } + } + if slices.Contains(currentCnpgCluster.Status.InstancesStatus[cnpgv1.PodHealthy], currentCnpgCluster.Status.CurrentPrimary) && replicationContext.IsPrimary() { // Check if permissions have already been granted checkCommand := "SELECT 1 FROM pg_roles WHERE rolname = 'streaming_replica' AND pg_has_role('streaming_replica', 'documentdb_admin_role', 'USAGE');" diff --git a/operator/src/internal/controller/documentdb_controller_test.go b/operator/src/internal/controller/documentdb_controller_test.go index 547f11e0e..c77531fa3 100644 --- a/operator/src/internal/controller/documentdb_controller_test.go +++ b/operator/src/internal/controller/documentdb_controller_test.go @@ -23,6 +23,7 @@ import ( kubefake "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/record" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -2924,6 +2925,98 @@ var _ = Describe("DocumentDB Controller", func() { Expect(result.Requeue).To(BeFalse()) }) + It("retains the OTel ConfigMap when cluster sync fails while disabling monitoring", func() { + Expect(rbacv1.AddToScheme(scheme)).To(Succeed()) + + documentdb := &dbpreview.DocumentDB{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + Finalizers: []string{documentDBFinalizer}, + }, + Spec: dbpreview.DocumentDBSpec{ + InstancesPerNode: 1, + Resource: dbpreview.Resource{ + Storage: dbpreview.StorageConfiguration{PvcSize: "1Gi"}, + }, + }, + } + configMapName := documentDBName + "-otel-config" + cnpgCluster := &cnpgv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: documentDBName, + Namespace: documentDBNamespace, + }, + Spec: cnpgv1.ClusterSpec{ + Instances: 1, + PostgresConfiguration: cnpgv1.PostgresConfiguration{ + Extensions: []cnpgv1.ExtensionConfiguration{ + { + Name: "documentdb", + ImageVolumeSource: corev1.ImageVolumeSource{ + Reference: util.DEFAULT_DOCUMENTDB_IMAGE, + }, + }, + }, + }, + Plugins: []cnpgv1.PluginConfiguration{ + { + Name: util.DEFAULT_SIDECAR_INJECTOR_PLUGIN, + Enabled: ptr.To(true), + Parameters: map[string]string{ + "gatewayImage": util.DEFAULT_GATEWAY_IMAGE, + "documentDbCredentialSecret": util.DEFAULT_DOCUMENTDB_CREDENTIALS_SECRET, + "otelCollectorImage": util.DEFAULT_OTEL_COLLECTOR_IMAGE, + "otelConfigMapName": configMapName, + }, + }, + }, + Managed: &cnpgv1.ManagedConfiguration{ + Roles: []cnpgv1.RoleConfiguration{ + { + Name: "otel_monitor", + Ensure: cnpgv1.EnsurePresent, + Login: true, + DisablePassword: true, + ConnectionLimit: -1, + Inherit: ptr.To(true), + }, + }, + }, + }, + } + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: configMapName, Namespace: documentDBNamespace}, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(documentdb, cnpgCluster, configMap). + WithStatusSubresource(&dbpreview.DocumentDB{}). + WithInterceptorFuncs(interceptor.Funcs{ + Patch: func(ctx context.Context, c client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + if _, ok := obj.(*cnpgv1.Cluster); ok { + return fmt.Errorf("simulated CNPG sync failure") + } + return c.Patch(ctx, obj, patch, opts...) + }, + }). + Build() + + reconciler := &DocumentDBReconciler{ + Client: fakeClient, + Scheme: scheme, + Recorder: recorder, + } + result, err := reconciler.Reconcile(ctx, ctrl.Request{ + NamespacedName: types.NamespacedName{Name: documentDBName, Namespace: documentDBNamespace}, + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(RequeueAfterShort)) + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: configMapName, Namespace: documentDBNamespace}, &corev1.ConfigMap{})).To(Succeed()) + }) + It("should add restart annotation when TLS secret name changes", func() { Expect(rbacv1.AddToScheme(scheme)).To(Succeed()) @@ -3342,4 +3435,5 @@ var _ = Describe("DocumentDB Controller", func() { Expect(err.Error()).To(ContainSubstring("failed to delete OTel ConfigMap")) }) }) + }) diff --git a/operator/src/internal/controller/physical_replication.go b/operator/src/internal/controller/physical_replication.go index 827fdbd85..b31f9c9b8 100644 --- a/operator/src/internal/controller/physical_replication.go +++ b/operator/src/internal/controller/physical_replication.go @@ -127,22 +127,7 @@ func (r *DocumentDBReconciler) AddClusterReplicationToClusterSpec( if replicationContext.IsAzureFleetNetworking() { // need to create services for each of the other clusters - cnpgCluster.Spec.Managed = &cnpgv1.ManagedConfiguration{ - Services: &cnpgv1.ManagedServices{ - Additional: []cnpgv1.ManagedService{}, - }, - } - for serviceName := range replicationContext.GenerateOutgoingServiceNames(documentdb.Name, documentdb.Namespace) { - cnpgCluster.Spec.Managed.Services.Additional = append(cnpgCluster.Spec.Managed.Services.Additional, - cnpgv1.ManagedService{ - SelectorType: cnpgv1.ServiceSelectorTypeRW, - ServiceTemplate: cnpgv1.ServiceTemplateSpec{ - ObjectMeta: cnpgv1.Metadata{ - Name: serviceName, - }, - }, - }) - } + addAzureFleetManagedServices(cnpgCluster, replicationContext, documentdb) } selfHost := replicationContext.CNPGClusterName + "-rw." + documentdb.Namespace + ".svc" cnpgCluster.Spec.ExternalClusters = []cnpgv1.ExternalCluster{ @@ -202,6 +187,31 @@ func (r *DocumentDBReconciler) AddClusterReplicationToClusterSpec( return nil } +// addAzureFleetManagedServices populates spec.managed.services.additional with a +// managed RW service per outgoing fleet member. It preserves any existing managed +// configuration (e.g. the OTel monitoring role set by GetCnpgClusterSpec) rather +// than replacing the whole managed block, so enabling fleet networking does not +// drop managed roles. +func addAzureFleetManagedServices(cnpgCluster *cnpgv1.Cluster, replicationContext *util.ReplicationContext, documentdb *dbpreview.DocumentDB) { + if cnpgCluster.Spec.Managed == nil { + cnpgCluster.Spec.Managed = &cnpgv1.ManagedConfiguration{} + } + cnpgCluster.Spec.Managed.Services = &cnpgv1.ManagedServices{ + Additional: []cnpgv1.ManagedService{}, + } + for serviceName := range replicationContext.GenerateOutgoingServiceNames(documentdb.Name, documentdb.Namespace) { + cnpgCluster.Spec.Managed.Services.Additional = append(cnpgCluster.Spec.Managed.Services.Additional, + cnpgv1.ManagedService{ + SelectorType: cnpgv1.ServiceSelectorTypeRW, + ServiceTemplate: cnpgv1.ServiceTemplateSpec{ + ObjectMeta: cnpgv1.Metadata{ + Name: serviceName, + }, + }, + }) + } +} + func (r *DocumentDBReconciler) CreateIstioRemoteServices(ctx context.Context, replicationContext *util.ReplicationContext, documentdb *dbpreview.DocumentDB) error { // Create dummy -rw services for remote clusters so DNS resolution works // These services have non-matching selectors, so they have no local endpoints diff --git a/operator/src/internal/controller/physical_replication_test.go b/operator/src/internal/controller/physical_replication_test.go index 2db056e17..b8744b924 100644 --- a/operator/src/internal/controller/physical_replication_test.go +++ b/operator/src/internal/controller/physical_replication_test.go @@ -1241,3 +1241,44 @@ var _ = Describe("AddClusterReplicationToClusterSpec - cert management fields", })) }) }) + +var _ = Describe("addAzureFleetManagedServices", func() { + newContext := func() *util.ReplicationContext { + return &util.ReplicationContext{ + CrossCloudNetworkingStrategy: util.AzureFleet, + CNPGClusterName: "cluster-a", + OtherCNPGClusterNames: []string{"cluster-b"}, + } + } + + It("adds managed RW services for outgoing fleet members", func() { + documentdb := baseDocumentDB("docdb-fleet", "default") + cnpgCluster := &cnpgv1.Cluster{} + + addAzureFleetManagedServices(cnpgCluster, newContext(), documentdb) + + Expect(cnpgCluster.Spec.Managed).NotTo(BeNil()) + Expect(cnpgCluster.Spec.Managed.Services).NotTo(BeNil()) + Expect(cnpgCluster.Spec.Managed.Services.Additional).To(HaveLen(1)) + Expect(cnpgCluster.Spec.Managed.Services.Additional[0].SelectorType).To(Equal(cnpgv1.ServiceSelectorTypeRW)) + }) + + It("preserves pre-existing managed roles when adding fleet services", func() { + documentdb := baseDocumentDB("docdb-fleet", "default") + cnpgCluster := &cnpgv1.Cluster{ + Spec: cnpgv1.ClusterSpec{ + Managed: &cnpgv1.ManagedConfiguration{ + Roles: []cnpgv1.RoleConfiguration{ + {Name: "otel_monitor", Login: true, DisablePassword: true}, + }, + }, + }, + } + + addAzureFleetManagedServices(cnpgCluster, newContext(), documentdb) + + Expect(cnpgCluster.Spec.Managed.Roles).To(HaveLen(1)) + Expect(cnpgCluster.Spec.Managed.Roles[0].Name).To(Equal("otel_monitor")) + Expect(cnpgCluster.Spec.Managed.Services.Additional).To(HaveLen(1)) + }) +}) diff --git a/operator/src/internal/controller/pv_controller.go b/operator/src/internal/controller/pv_controller.go index 7b23feca0..fda27942b 100644 --- a/operator/src/internal/controller/pv_controller.go +++ b/operator/src/internal/controller/pv_controller.go @@ -72,8 +72,10 @@ type PersistentVolumeReconciler struct { client.Client } +// Package-level RBAC also covers PVC lifecycle operations performed by the +// DocumentDB reconciler. // +kubebuilder:rbac:groups="",resources=persistentvolumes,verbs=get;list;watch;update;patch -// +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list;watch +// +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list;watch;create;delete // +kubebuilder:rbac:groups=storage.k8s.io,resources=storageclasses,verbs=get;list;watch func (r *PersistentVolumeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { diff --git a/operator/src/internal/otel/base_config.yaml b/operator/src/internal/otel/base_config.yaml index 0b82af385..1ef124baf 100644 --- a/operator/src/internal/otel/base_config.yaml +++ b/operator/src/internal/otel/base_config.yaml @@ -7,7 +7,7 @@ receivers: # no Go code changes needed. sqlquery: driver: postgres - datasource: "host=localhost port=5432 user=${env:PGUSER} password=${env:PGPASSWORD} dbname=postgres sslmode=disable" + datasource: "host=localhost port=5432 user=${env:PGUSER} dbname=postgres sslmode=disable" collection_interval: "30s" queries: - sql: "SELECT 1 as up" diff --git a/operator/src/internal/otel/config.go b/operator/src/internal/otel/config.go index c76bebc81..e87eb7e65 100644 --- a/operator/src/internal/otel/config.go +++ b/operator/src/internal/otel/config.go @@ -19,6 +19,10 @@ var baseConfigYAML []byte const defaultPrometheusPort = 8888 +// MonitorRoleName is the dedicated PostgreSQL identity the OTel Collector +// sidecar uses for its health-check query. +const MonitorRoleName = "otel_monitor" + // collectorConfig represents the OTel Collector configuration structure. type collectorConfig struct { Receivers map[string]any `yaml:"receivers,omitempty"` diff --git a/operator/src/internal/otel/config_test.go b/operator/src/internal/otel/config_test.go index 24546ae33..0032c3312 100644 --- a/operator/src/internal/otel/config_test.go +++ b/operator/src/internal/otel/config_test.go @@ -25,6 +25,12 @@ var _ = Describe("ConfigMapName", func() { }) }) +var _ = Describe("MonitorRoleName", func() { + It("is the dedicated least-privilege monitoring role", func() { + Expect(MonitorRoleName).To(Equal("otel_monitor")) + }) +}) + // parseCfg is a helper to unmarshal YAML into a collectorConfig struct. func parseCfg(yamlStr string) collectorConfig { var cfg collectorConfig @@ -44,6 +50,11 @@ var _ = Describe("base_config.yaml embed", func() { Expect(cfg.Exporters).To(BeEmpty()) }) + It("does not configure a password while PostgreSQL uses trust authentication", func() { + Expect(string(baseConfigYAML)).To(ContainSubstring("user=${env:PGUSER}")) + Expect(string(baseConfigYAML)).NotTo(ContainSubstring("PGPASSWORD")) + }) + It("declares a cgroup-aware memory_limiter processor", func() { var cfg collectorConfig Expect(yaml.Unmarshal(baseConfigYAML, &cfg)).To(Succeed())