diff --git a/config/crds/v1/all-crds.yaml b/config/crds/v1/all-crds.yaml index bd1b9a61a0..7fce367a60 100644 --- a/config/crds/v1/all-crds.yaml +++ b/config/crds/v1/all-crds.yaml @@ -2648,6 +2648,30 @@ spec: This field cannot be used in combination with `name`. type: string type: object + config: + description: |- + Config holds additional OpenTelemetry collector configuration for the AutoOps agent. + User-supplied settings are merged between the operator baseline defaults and the + operator-owned mandatory settings, which always take final precedence. This allows + tuning knobs such as the sending_queue sizing, appending custom metricbeat modules, + or defining additional exporters and pipelines. Supplying at least one autoops_es + module replaces the operator's built-in modules entirely, giving full control over + which metricsets are collected and at what period. Elasticsearch connection details + (hosts, SSL), OTLP endpoint and authorization, and the healthcheck extension are + always injected by the operator and cannot be overridden. + At most one of [`config`, `configRef`] can be specified. + type: object + x-kubernetes-preserve-unknown-fields: true + configRef: + description: |- + ConfigRef references a Kubernetes Secret holding the AutoOps agent configuration + under the `autoops_es.yml` key. Applies the same merge semantics as `config`. + At most one of [`config`, `configRef`] can be specified. + properties: + secretName: + description: SecretName is the name of the secret. + type: string + type: object image: description: Image is the AutoOps Agent Docker image to deploy. type: string diff --git a/config/crds/v1/resources/autoops.k8s.elastic.co_autoopsagentpolicies.yaml b/config/crds/v1/resources/autoops.k8s.elastic.co_autoopsagentpolicies.yaml index 76833755b2..05c8887d6f 100644 --- a/config/crds/v1/resources/autoops.k8s.elastic.co_autoopsagentpolicies.yaml +++ b/config/crds/v1/resources/autoops.k8s.elastic.co_autoopsagentpolicies.yaml @@ -69,6 +69,30 @@ spec: This field cannot be used in combination with `name`. type: string type: object + config: + description: |- + Config holds additional OpenTelemetry collector configuration for the AutoOps agent. + User-supplied settings are merged between the operator baseline defaults and the + operator-owned mandatory settings, which always take final precedence. This allows + tuning knobs such as the sending_queue sizing, appending custom metricbeat modules, + or defining additional exporters and pipelines. Supplying at least one autoops_es + module replaces the operator's built-in modules entirely, giving full control over + which metricsets are collected and at what period. Elasticsearch connection details + (hosts, SSL), OTLP endpoint and authorization, and the healthcheck extension are + always injected by the operator and cannot be overridden. + At most one of [`config`, `configRef`] can be specified. + type: object + x-kubernetes-preserve-unknown-fields: true + configRef: + description: |- + ConfigRef references a Kubernetes Secret holding the AutoOps agent configuration + under the `autoops_es.yml` key. Applies the same merge semantics as `config`. + At most one of [`config`, `configRef`] can be specified. + properties: + secretName: + description: SecretName is the name of the secret. + type: string + type: object image: description: Image is the AutoOps Agent Docker image to deploy. type: string diff --git a/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml b/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml index c9789fa6cb..52e5b906bb 100644 --- a/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml +++ b/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml @@ -2669,6 +2669,30 @@ spec: This field cannot be used in combination with `name`. type: string type: object + config: + description: |- + Config holds additional OpenTelemetry collector configuration for the AutoOps agent. + User-supplied settings are merged between the operator baseline defaults and the + operator-owned mandatory settings, which always take final precedence. This allows + tuning knobs such as the sending_queue sizing, appending custom metricbeat modules, + or defining additional exporters and pipelines. Supplying at least one autoops_es + module replaces the operator's built-in modules entirely, giving full control over + which metricsets are collected and at what period. Elasticsearch connection details + (hosts, SSL), OTLP endpoint and authorization, and the healthcheck extension are + always injected by the operator and cannot be overridden. + At most one of [`config`, `configRef`] can be specified. + type: object + x-kubernetes-preserve-unknown-fields: true + configRef: + description: |- + ConfigRef references a Kubernetes Secret holding the AutoOps agent configuration + under the `autoops_es.yml` key. Applies the same merge semantics as `config`. + At most one of [`config`, `configRef`] can be specified. + properties: + secretName: + description: SecretName is the name of the secret. + type: string + type: object image: description: Image is the AutoOps Agent Docker image to deploy. type: string diff --git a/docs/reference/api-reference/main.md b/docs/reference/api-reference/main.md index bfdac162ce..f4746c497c 100644 --- a/docs/reference/api-reference/main.md +++ b/docs/reference/api-reference/main.md @@ -304,6 +304,8 @@ AutoOpsAgentPolicy represents an Elastic AutoOps Policy resource in a Kubernetes | *`podTemplate`* __[PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#podtemplatespec-v1-core)__ | PodTemplate provides customization options (labels, annotations, affinity rules, resource requests, and so on) for the Agent pods | | *`revisionHistoryLimit`* __integer__ | RevisionHistoryLimit is the number of revisions to retain to allow rollback in the underlying Deployment. | | *`serviceAccountName`* __string__ | ServiceAccountName is used to check access to Elasticsearch resources in different namespaces.
Can only be used if ECK is enforcing RBAC on references (--enforce-rbac-on-refs flag).
The service account must have "get" permission on elasticsearch.k8s.elastic.co/elasticsearches
in the target namespaces. | +| *`config`* __[Config](#config)__ | Config holds additional OpenTelemetry collector configuration for the AutoOps agent.
User-supplied settings are merged between the operator baseline defaults and the
operator-owned mandatory settings, which always take final precedence. This allows
tuning knobs such as the sending_queue sizing, appending custom metricbeat modules,
or defining additional exporters and pipelines. Supplying at least one autoops_es
module replaces the operator's built-in modules entirely, giving full control over
which metricsets are collected and at what period. Elasticsearch connection details
(hosts, SSL), OTLP endpoint and authorization, and the healthcheck extension are
always injected by the operator and cannot be overridden.
At most one of [`config`, `configRef`] can be specified. | +| *`configRef`* __[ConfigSource](#configsource)__ | ConfigRef references a Kubernetes Secret holding the AutoOps agent configuration
under the `autoops_es.yml` key. Applies the same merge semantics as `config`.
At most one of [`config`, `configRef`] can be specified. | ### AutoOpsRef [#autoopsref] @@ -539,6 +541,7 @@ Config represents untyped YAML configuration. :::{admonition} Appears In: * [AgentSpec](#agentspec) * [ApmServerSpec](#apmserverspec) +* [AutoOpsAgentPolicySpec](#autoopsagentpolicyspec) * [BeatSpec](#beatspec) * [ElasticsearchConfigPolicySpec](#elasticsearchconfigpolicyspec) * [EnterpriseSearchSpec](#enterprisesearchspec) @@ -576,6 +579,7 @@ ConfigSource references configuration settings. :::{admonition} Appears In: * [AgentSpec](#agentspec) +* [AutoOpsAgentPolicySpec](#autoopsagentpolicyspec) * [BeatSpec](#beatspec) * [EnterpriseSearchSpec](#enterprisesearchspec) * [EnterpriseSearchSpec](#enterprisesearchspec) diff --git a/pkg/apis/autoops/v1alpha1/autoopsagentpolicy_types.go b/pkg/apis/autoops/v1alpha1/autoopsagentpolicy_types.go index cc33740a9e..933f22a9d1 100644 --- a/pkg/apis/autoops/v1alpha1/autoopsagentpolicy_types.go +++ b/pkg/apis/autoops/v1alpha1/autoopsagentpolicy_types.go @@ -17,6 +17,8 @@ const ( Kind = "AutoOpsAgentPolicy" // AutoOpsAgentContainerName is the name of the main AutoOps Agent container in the pod. AutoOpsAgentContainerName = "autoops-agent" + // ConfigFileName is the key used in the Secret referenced by ConfigRef. + ConfigFileName = "autoops_es.yml" ) // +kubebuilder:object:root=true @@ -84,6 +86,26 @@ type AutoOpsAgentPolicySpec struct { // in the target namespaces. // +optional ServiceAccountName string `json:"serviceAccountName,omitempty"` + + // Config holds additional OpenTelemetry collector configuration for the AutoOps agent. + // User-supplied settings are merged between the operator baseline defaults and the + // operator-owned mandatory settings, which always take final precedence. This allows + // tuning knobs such as the sending_queue sizing, appending custom metricbeat modules, + // or defining additional exporters and pipelines. Supplying at least one autoops_es + // module replaces the operator's built-in modules entirely, giving full control over + // which metricsets are collected and at what period. Elasticsearch connection details + // (hosts, SSL), OTLP endpoint and authorization, and the healthcheck extension are + // always injected by the operator and cannot be overridden. + // At most one of [`config`, `configRef`] can be specified. + // +kubebuilder:validation:Optional + // +kubebuilder:pruning:PreserveUnknownFields + Config *commonv1.Config `json:"config,omitempty"` + + // ConfigRef references a Kubernetes Secret holding the AutoOps agent configuration + // under the `autoops_es.yml` key. Applies the same merge semantics as `config`. + // At most one of [`config`, `configRef`] can be specified. + // +kubebuilder:validation:Optional + ConfigRef *commonv1.ConfigSource `json:"configRef,omitempty"` } // AutoOpsRef defines a reference to a secret containing connection details for AutoOps via Cloud Connect. diff --git a/pkg/apis/autoops/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/autoops/v1alpha1/zz_generated.deepcopy.go index cfcbc8f0de..2b8c345b4b 100644 --- a/pkg/apis/autoops/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/autoops/v1alpha1/zz_generated.deepcopy.go @@ -9,6 +9,7 @@ package v1alpha1 import ( + "github.com/elastic/cloud-on-k8s/v3/pkg/apis/common/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -84,6 +85,15 @@ func (in *AutoOpsAgentPolicySpec) DeepCopyInto(out *AutoOpsAgentPolicySpec) { *out = new(int32) **out = **in } + if in.Config != nil { + in, out := &in.Config, &out.Config + *out = (*in).DeepCopy() + } + if in.ConfigRef != nil { + in, out := &in.ConfigRef, &out.ConfigRef + *out = new(v1.ConfigSource) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AutoOpsAgentPolicySpec. diff --git a/pkg/controller/autoops/configmap.go b/pkg/controller/autoops/configmap.go index 71c7587028..5f7ff46f8a 100644 --- a/pkg/controller/autoops/configmap.go +++ b/pkg/controller/autoops/configmap.go @@ -5,12 +5,10 @@ package autoops import ( - "bytes" "context" "fmt" "path/filepath" "reflect" - "text/template" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -18,58 +16,41 @@ import ( autoopsv1alpha1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/autoops/v1alpha1" commonv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/common/v1" esv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/elasticsearch/v1" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/annotation" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/certificates" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/events" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/metadata" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/reconciler" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/settings" "github.com/elastic/cloud-on-k8s/v3/pkg/utils/k8s" "github.com/elastic/cloud-on-k8s/v3/pkg/utils/maps" ) -const ( - // autoOpsESConfigFileName is the key name for the config file in the ConfigMap - autoOpsESConfigFileName = "autoops_es.yml" -) +// autoOpsBaselineConfig holds the user-tunable defaults. Users may override any field here +// via spec.config or spec.configRef. Everything the operator must always control lives in +// autoOpsMandatoryConfig. +const autoOpsBaselineConfig = ` +exporters: + otlphttp: + sending_queue: + batch: + flush_timeout: 11s + min_size: 1048576 + max_size: 4194304 + sizer: bytes + block_on_overflow: true + enabled: true + queue_size: 52428800 + sizer: bytes +` -// autoOpsESConfigTemplate contains the configuration template for the autoops agent -const autoOpsESConfigTemplate = ` -{{- define "ssl" -}} -{{- if .SSLEnabled}} - ssl.verification_mode: certificate - ssl.certificate_authorities: ["{{ .CACertPath }}"] -{{- if .ClientCertPath}} - ssl.certificate: "{{ .ClientCertPath }}" - ssl.key: "{{ .ClientKeyPath }}" -{{- end}} -{{- else}} - ssl.verification_mode: none -{{- end}} -{{- end -}} +// autoOpsMandatoryConfig is merged last so its values always take precedence over the +// baseline and any user-supplied config. SSL/hosts are not part of this config — they are +// injected per-module via injectModuleConnectionSettings. +const autoOpsMandatoryConfig = ` receivers: metricbeatreceiver: - metricbeat: - modules: - # Metrics - - module: autoops_es - hosts: ${env:AUTOOPS_ES_URL} -{{- template "ssl" . }} - period: 10s - metricsets: - - cat_shards - - cluster_health - - cluster_settings - - license - - node_stats - - tasks_management - # Templates - - module: autoops_es - hosts: ${env:AUTOOPS_ES_URL} -{{- template "ssl" . }} - period: 24h - metricsets: - - cat_template - - component_template - - index_template processors: - add_fields: target: autoops_es @@ -83,16 +64,6 @@ exporters: headers: Authorization: "AutoOpsToken ${env:AUTOOPS_TOKEN}" endpoint: ${env:AUTOOPS_OTEL_URL} - sending_queue: - batch: - flush_timeout: 11s - min_size: 1048576 - max_size: 4194304 - sizer: bytes - block_on_overflow: true - enabled: true - queue_size: 52428800 - sizer: bytes service: extensions: [healthcheckv2] pipelines: @@ -119,19 +90,54 @@ extensions: path: "/health/config" ` -// configTemplateData holds the data for rendering the config template -type configTemplateData struct { - SSLEnabled bool - CACertPath string - ClientCertPath string - ClientKeyPath string -} +const autoOpsDefaultModulesConfig = ` +receivers: + metricbeatreceiver: + metricbeat: + modules: + # Metrics + - module: autoops_es + period: 10s + metricsets: + - cat_shards + - cluster_health + - cluster_settings + - license + - node_stats + - tasks_management + # Templates + - module: autoops_es + period: 24h + metricsets: + - cat_template + - component_template + - index_template +` // ReconcileAutoOpsESConfigMap reconciles the ConfigMap containing the autoops configuration // specific to each ES instance. This also returns the config hash of the ConfigMap to avoid // retrieving it from the cache later and delaying the initial deployment. -func ReconcileAutoOpsESConfigMap(ctx context.Context, c k8s.Client, policy autoopsv1alpha1.AutoOpsAgentPolicy, es esv1.Elasticsearch) (*corev1.ConfigMap, error) { - expected, err := buildAutoOpsESConfigMap(policy, es) +func (r *AgentPolicyReconciler) ReconcileAutoOpsESConfigMap(ctx context.Context, policy autoopsv1alpha1.AutoOpsAgentPolicy, es esv1.Elasticsearch) (*corev1.ConfigMap, error) { + // Parse the inline config. NewCanonicalConfigFrom returns an empty config when Data is nil. + specConfig := policy.Spec.Config + if specConfig == nil { + specConfig = &commonv1.Config{} + } + userCfg, err := settings.NewCanonicalConfigFrom(specConfig.Data) + if err != nil { + msg := fmt.Sprintf("unable to parse spec.config: %s", err) + k8s.EmitEvent(r.Recorder(), &policy, corev1.EventTypeWarning, events.EventReasonValidation, events.EventActionAutoOpsReconciliation, msg) + return nil, fmt.Errorf("while parsing spec.config: %w", err) + } + + // Always call ParseConfigRef regardless of whether ConfigRef is set. + // It manages the dynamic watch lifecycle: registers when set, deregisters when cleared. + userSecretCfg, err := common.ParseConfigRef(r, &policy, policy.Spec.ConfigRef, autoopsv1alpha1.ConfigFileName) + if err != nil { + return nil, fmt.Errorf("while parsing spec.configRef: %w", err) + } + + expected, err := buildAutoOpsESConfigMap(policy, es, userCfg, userSecretCfg) if err != nil { return nil, err } @@ -140,7 +146,7 @@ func ReconcileAutoOpsESConfigMap(ctx context.Context, c k8s.Client, policy autoo err = reconciler.ReconcileResource( reconciler.Params{ Context: ctx, - Client: c, + Client: r.Client, Owner: &policy, Expected: &expected, Reconciled: reconciled, @@ -163,9 +169,88 @@ func ReconcileAutoOpsESConfigMap(ctx context.Context, c k8s.Client, policy autoo return reconciled, nil } +// hasUserAutoOpsESModules reports whether any of the supplied configs defines at least one +// autoops_es module entry under receivers.metricbeatreceiver.metricbeat.modules. When true +// the operator skips injecting its own default modules so the user has full control over +// which autoops_es data is collected (e.g. to drop the Templates module). +// A user config that only adds non-autoops_es modules returns false and the operator still +// injects its own defaults alongside the user's entries. +func hasUserAutoOpsESModules(cfgs ...*settings.CanonicalConfig) bool { + for _, cfg := range cfgs { + if cfg == nil { + continue + } + var raw map[string]any + if err := cfg.Unpack(&raw); err != nil { + continue + } + for _, m := range autoopsModules(raw) { + mod, ok := m.(map[string]any) + if ok && mod["module"] == "autoops_es" { + return true + } + } + } + return false +} + +// injectModuleConnectionSettings walks every autoops_es module entry in the merged config +// and injects the hosts and ssl fields. This is done post-merge because module entries may +// come from either the operator defaults or the user, while SSL paths are per-ES and only +// known at reconcile time. +func injectModuleConnectionSettings(cfg *settings.CanonicalConfig, sslEnabled bool, caCertPath, clientCertPath, clientKeyPath string) (*settings.CanonicalConfig, error) { + var raw map[string]any + if err := cfg.Unpack(&raw); err != nil { + return nil, fmt.Errorf("while unpacking config: %w", err) + } + + for _, m := range autoopsModules(raw) { + mod, ok := m.(map[string]any) + if !ok || mod["module"] != "autoops_es" { + continue + } + mod["hosts"] = "${env:AUTOOPS_ES_URL}" + ssl := map[string]any{} + if sslEnabled { + ssl["verification_mode"] = "certificate" + ssl["certificate_authorities"] = []string{caCertPath} + if clientCertPath != "" { + ssl["certificate"] = clientCertPath + ssl["key"] = clientKeyPath + } + } else { + ssl["verification_mode"] = "none" + } + mod["ssl"] = ssl + } + + return settings.NewCanonicalConfigFrom(raw) +} + +// autoopsModules extracts the modules slice at receivers.metricbeatreceiver.metricbeat.modules. +// Returns nil if the path does not exist or is not a slice. +func autoopsModules(raw map[string]any) []any { + receivers, ok := raw["receivers"].(map[string]any) + if !ok { + return nil + } + mbr, ok := receivers["metricbeatreceiver"].(map[string]any) + if !ok { + return nil + } + mb, ok := mbr["metricbeat"].(map[string]any) + if !ok { + return nil + } + modules, _ := mb["modules"].([]any) + return modules +} + // buildAutoOpsESConfigMap builds the expected ConfigMap for autoops configuration. -// SSL is enabled based on the Elasticsearch CRD's spec.http.tls configuration. -func buildAutoOpsESConfigMap(policy autoopsv1alpha1.AutoOpsAgentPolicy, es esv1.Elasticsearch) (corev1.ConfigMap, error) { +// Merge order: baseline → userCfg → userSecretCfg → defaultModules → mandatory. +// The mandatory config is applied last so operator-owned scalars always take precedence. +// SSL and hosts are injected into every autoops_es module post-merge. +func buildAutoOpsESConfigMap(policy autoopsv1alpha1.AutoOpsAgentPolicy, es esv1.Elasticsearch, userCfg, userSecretCfg *settings.CanonicalConfig) (corev1.ConfigMap, error) { labels := resourceLabelsFor(policy, es) meta := metadata.Propagate(&policy, metadata.Metadata{ Labels: maps.Merge(policy.GetLabels(), labels), @@ -183,12 +268,6 @@ func buildAutoOpsESConfigMap(policy autoopsv1alpha1.AutoOpsAgentPolicy, es esv1. ) } - tmpl, err := template.New("autoops-config").Parse(autoOpsESConfigTemplate) - if err != nil { - return corev1.ConfigMap{}, err - } - - var configBuf bytes.Buffer var clientCertPath, clientKeyPath string if annotation.HasClientAuthenticationRequired(&es) { clientCertDir := fmt.Sprintf("/mnt/elastic-internal/es-client-cert/%s-%s", es.Namespace, es.Name) @@ -196,14 +275,46 @@ func buildAutoOpsESConfigMap(policy autoopsv1alpha1.AutoOpsAgentPolicy, es esv1. clientKeyPath = filepath.Join(clientCertDir, certificates.KeyFileName) } - templateData := configTemplateData{ - SSLEnabled: sslEnabled, - CACertPath: caCertPath, - ClientCertPath: clientCertPath, - ClientKeyPath: clientKeyPath, + mandatoryCfg, err := settings.ParseConfig([]byte(autoOpsMandatoryConfig)) + if err != nil { + return corev1.ConfigMap{}, fmt.Errorf("while parsing mandatory config: %w", err) } - if err := tmpl.Execute(&configBuf, templateData); err != nil { - return corev1.ConfigMap{}, err + + baselineCfg, err := settings.ParseConfig([]byte(autoOpsBaselineConfig)) + if err != nil { + return corev1.ConfigMap{}, fmt.Errorf("while parsing baseline config: %w", err) + } + + // If neither user config source defines an autoops_es modules list, inject the operator + // defaults so the agent collects the standard metric and template sets. When the user + // does supply modules the operator's defaults are intentionally omitted, giving the user + // full control over which data is collected. + var defaultModulesCfg *settings.CanonicalConfig + if !hasUserAutoOpsESModules(userCfg, userSecretCfg) { + defaultModulesCfg, err = settings.ParseConfig([]byte(autoOpsDefaultModulesConfig)) + if err != nil { + return corev1.ConfigMap{}, fmt.Errorf("while parsing default modules config: %w", err) + } + } + + // Merge order: baseline → user → userSecret → defaultModules → mandatory. + // User config can override anything in the baseline (e.g. sending_queue tuning). + // Mandatory config is rendered last so all operator-owned fields always win. + if err := baselineCfg.MergeWith(userCfg, userSecretCfg, defaultModulesCfg, mandatoryCfg); err != nil { + return corev1.ConfigMap{}, fmt.Errorf("while merging config: %w", err) + } + + // Inject connection settings (hosts, SSL) into every autoops_es module in the merged + // config. This is done post-merge because the modules may come from the operator defaults + // or from the user, and the SSL paths are per-ES and only known at reconcile time. + finalCfg, err := injectModuleConnectionSettings(baselineCfg, sslEnabled, caCertPath, clientCertPath, clientKeyPath) + if err != nil { + return corev1.ConfigMap{}, fmt.Errorf("while injecting module connection settings: %w", err) + } + + rendered, err := finalCfg.Render() + if err != nil { + return corev1.ConfigMap{}, fmt.Errorf("while rendering config: %w", err) } // Use ES-specific ConfigMap name to allow per-ES configuration @@ -217,7 +328,7 @@ func buildAutoOpsESConfigMap(policy autoopsv1alpha1.AutoOpsAgentPolicy, es esv1. Annotations: meta.Annotations, }, Data: map[string]string{ - autoOpsESConfigFileName: configBuf.String(), + autoopsv1alpha1.ConfigFileName: string(rendered), }, }, nil } diff --git a/pkg/controller/autoops/configmap_test.go b/pkg/controller/autoops/configmap_test.go index 9d954a0cce..87efa54e67 100644 --- a/pkg/controller/autoops/configmap_test.go +++ b/pkg/controller/autoops/configmap_test.go @@ -5,17 +5,25 @@ package autoops import ( + "context" "testing" uyaml "github.com/elastic/go-ucfg/yaml" "github.com/go-test/deep" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + toolsevents "k8s.io/client-go/tools/events" + "sigs.k8s.io/controller-runtime/pkg/client" autoopsv1alpha1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/autoops/v1alpha1" commonv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/common/v1" esv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/elasticsearch/v1" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/annotation" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/scheme" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/watches" + "github.com/elastic/cloud-on-k8s/v3/pkg/utils/k8s" ) var defaultConfigNoSSL = []byte(` @@ -299,128 +307,534 @@ func mkES(sslEnabled bool) esv1.Elasticsearch { } } -func Test_buildAutoOpsESConfigMap(t *testing.T) { - type args struct { - policy func() autoopsv1alpha1.AutoOpsAgentPolicy - es func() esv1.Elasticsearch +// mkReconciler builds a minimal AgentPolicyReconciler for use in tests. +func mkReconciler(t *testing.T, objects ...client.Object) *AgentPolicyReconciler { + t.Helper() + scheme.SetupScheme() + c := k8s.NewFakeClient(objects...) + return &AgentPolicyReconciler{ + Client: c, + dynamicWatches: watches.NewDynamicWatches(), + recorder: toolsevents.NewFakeRecorder(10), } +} + +// asMap asserts v is map[string]any, failing the test immediately if not. +func asMap(t *testing.T, v any) map[string]any { + t.Helper() + m, ok := v.(map[string]any) + require.True(t, ok, "expected map[string]any, got %T", v) + return m +} + +// asSlice asserts v is []any, failing the test immediately if not. +func asSlice(t *testing.T, v any) []any { + t.Helper() + s, ok := v.([]any) + require.True(t, ok, "expected []any, got %T", v) + return s +} + +// unpackConfigYAML parses a YAML string from a ConfigMap into a map for assertions. +func unpackConfigYAML(t *testing.T, cm corev1.ConfigMap) map[string]any { + t.Helper() + data, ok := cm.Data[autoopsv1alpha1.ConfigFileName] + require.True(t, ok, "ConfigMap missing key %s", autoopsv1alpha1.ConfigFileName) + var out map[string]any + parsed, err := uyaml.NewConfig([]byte(data), commonv1.CfgOptions...) + require.NoError(t, err) + require.NoError(t, parsed.Unpack(&out)) + return out +} + +func Test_buildAutoOpsESConfigMap(t *testing.T) { + // validConfigRefYAML is a minimal valid config used by ConfigRef test cases. + validConfigRefYAML := []byte(` +receivers: + metricbeatreceiver: + metricbeat: + modules: + - module: autoops_es + hosts: ${env:AUTOOPS_ES_URL} + period: 10s + ssl: + verification_mode: none + metricsets: + - cluster_health +`) + tests := []struct { - name string - args args - want []byte - wantErr bool + name string + policy func() autoopsv1alpha1.AutoOpsAgentPolicy // defaults to mkPolicy() + es func() esv1.Elasticsearch // defaults to mkES(false) + specConfigYAML []byte // raw YAML parsed into policy.Spec.Config + secrets []client.Object + want []byte // exact YAML comparison via ucfg parse + check func(t *testing.T, cm corev1.ConfigMap) // custom assertions + wantErr bool }{ { name: "default config without SSL", - args: args{ - policy: mkPolicy, - es: func() esv1.Elasticsearch { return mkES(false) }, - }, want: defaultConfigNoSSL, }, { name: "default config with SSL", - args: args{ - policy: mkPolicy, - es: func() esv1.Elasticsearch { return mkES(true) }, - }, + es: func() esv1.Elasticsearch { return mkES(true) }, want: defaultConfigWithSSL, }, { name: "default config with SSL and client cert", - args: args{ - policy: mkPolicy, - es: func() esv1.Elasticsearch { - es := mkES(true) - es.Annotations = map[string]string{ - annotation.ClientAuthenticationRequiredAnnotation: "true", - } - return es - }, + es: func() esv1.Elasticsearch { + es := mkES(true) + es.Annotations = map[string]string{annotation.ClientAuthenticationRequiredAnnotation: "true"} + return es }, want: defaultConfigWithSSLAndClientCert, }, { name: "client auth annotation with SSL disabled does not render client cert paths", - args: args{ - policy: mkPolicy, - es: func() esv1.Elasticsearch { - es := mkES(false) - es.Annotations = map[string]string{ - annotation.ClientAuthenticationRequiredAnnotation: "true", - } - return es - }, + es: func() esv1.Elasticsearch { + es := mkES(false) + es.Annotations = map[string]string{annotation.ClientAuthenticationRequiredAnnotation: "true"} + return es }, want: defaultConfigNoSSL, }, { - name: "with metadata labels and annotations", - args: args{ - policy: func() autoopsv1alpha1.AutoOpsAgentPolicy { - p := mkPolicy() - p.ObjectMeta.Labels = map[string]string{ - "label1": "value1", - "label2": "value2", + name: "metadata labels and annotations propagated to ConfigMap", + policy: func() autoopsv1alpha1.AutoOpsAgentPolicy { + p := mkPolicy() + p.Labels = map[string]string{"label1": "value1", "label2": "value2"} + p.Annotations = map[string]string{"annotation1": "value1"} + return p + }, + es: func() esv1.Elasticsearch { + es := mkES(false) + es.Namespace = "test-namespace" + return es + }, + want: defaultConfigNoSSL, + }, + { + name: "nil user config - output is identical to default", + want: defaultConfigNoSSL, + }, + { + name: "user adds extra OTLP header - preserved alongside operator headers", + specConfigYAML: []byte(` +exporters: + otlphttp: + headers: + X-Custom-Header: my-value +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + got := unpackConfigYAML(t, cm) + headers := asMap(t, asMap(t, asMap(t, got["exporters"])["otlphttp"])["headers"]) + assert.Equal(t, "my-value", headers["X-Custom-Header"]) + assert.Equal(t, "AutoOpsToken ${env:AUTOOPS_TOKEN}", headers["Authorization"]) + }, + }, + { + // endpoint is in autoOpsMandatoryConfig, so it is re-applied after user config. + name: "user overrides endpoint - mandatory config wins", + specConfigYAML: []byte(` +exporters: + otlphttp: + endpoint: https://custom.example.com +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + otlp := asMap(t, asMap(t, unpackConfigYAML(t, cm)["exporters"])["otlphttp"]) + assert.Equal(t, "${env:AUTOOPS_OTEL_URL}", otlp["endpoint"]) + }, + }, + { + name: "user increases sending_queue size - user value wins", + specConfigYAML: []byte(` +exporters: + otlphttp: + sending_queue: + queue_size: 104857600 +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + got := unpackConfigYAML(t, cm) + sq := asMap(t, asMap(t, asMap(t, got["exporters"])["otlphttp"])["sending_queue"]) + assert.EqualValues(t, 104857600, sq["queue_size"], "user-supplied queue_size must win over the default 52428800") + // Other sending_queue fields from the operator default must still be present. + assert.Equal(t, true, sq["block_on_overflow"]) + assert.Equal(t, true, sq["enabled"]) + }, + }, + { + // Even an explicit empty string is overridden by autoOpsMandatoryConfig. + name: "user clears otlphttp endpoint - mandatory config re-applies env var reference", + specConfigYAML: []byte(` +exporters: + otlphttp: + endpoint: "" +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + otlp := asMap(t, asMap(t, unpackConfigYAML(t, cm)["exporters"])["otlphttp"]) + assert.Equal(t, "${env:AUTOOPS_OTEL_URL}", otlp["endpoint"]) + }, + }, + { + // AppendValues appends to existing lists: [] appends nothing, so healthcheckv2 stays. + name: "user provides empty service.extensions - healthcheckv2 preserved by AppendValues", + specConfigYAML: []byte(` +service: + extensions: [] +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + exts := asSlice(t, asMap(t, unpackConfigYAML(t, cm)["service"])["extensions"]) + found := false + for _, e := range exts { + if e == "healthcheckv2" { + found = true + } + } + assert.True(t, found, "healthcheckv2 must still be present") + }, + }, + { + // ucfg always deep-merges maps regardless of AppendValues: receivers: {} deep-merges + // with the existing receivers map, preserving metricbeatreceiver and all its content. + name: "user provides empty receivers map - original receivers preserved by map merge", + specConfigYAML: []byte(` +receivers: {} +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + got := unpackConfigYAML(t, cm) + receivers, ok := got["receivers"].(map[string]any) + require.True(t, ok, "receivers section must be present") + mbr, ok := receivers["metricbeatreceiver"].(map[string]any) + require.True(t, ok, "metricbeatreceiver must be present") + modules, ok := mbr["metricbeat"].(map[string]any)["modules"].([]any) + require.True(t, ok, "modules must be present") + found := false + for _, m := range modules { + if mod, ok := m.(map[string]any); ok && mod["module"] == "autoops_es" { + found = true } - p.ObjectMeta.Annotations = map[string]string{ - "annotation1": "value1", + } + assert.True(t, found, "autoops_es module must survive the empty-map merge") + }, + }, + { + // healthcheckv2.http.endpoint is in autoOpsMandatoryConfig and is always re-applied. + name: "user changes healthcheckv2 endpoint port - mandatory config re-applies correct port", + specConfigYAML: []byte(` +extensions: + healthcheckv2: + http: + endpoint: "0.0.0.0:9999" +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + got := unpackConfigYAML(t, cm) + hc := asMap(t, asMap(t, got["extensions"])["healthcheckv2"]) + assert.Equal(t, "0.0.0.0:13133", asMap(t, hc["http"])["endpoint"]) + }, + }, + { + // service.telemetry.logs.encoding is in autoOpsMandatoryConfig, so it is + // always re-applied after user config. + name: "user sets service.telemetry.logs.encoding - mandatory config wins", + specConfigYAML: []byte(` +service: + telemetry: + logs: + encoding: text +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + got := unpackConfigYAML(t, cm) + logs := asMap(t, asMap(t, asMap(t, got["service"])["telemetry"])["logs"]) + assert.Equal(t, "json", logs["encoding"]) + }, + }, + { + // AppendValues appends to existing lists. The user's module comes first (from the + // user config layer), and the operator's two autoops_es modules are appended after. + name: "user adds non-autoops_es module - appended after operator modules", + specConfigYAML: []byte(` +receivers: + metricbeatreceiver: + metricbeat: + modules: + - module: some_other_module + period: 10s +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + got := unpackConfigYAML(t, cm) + modules := asSlice(t, asMap(t, asMap(t, asMap(t, got["receivers"])["metricbeatreceiver"])["metricbeat"])["modules"]) + foundAutoOps, foundOther := false, false + for _, m := range modules { + if mod, ok := m.(map[string]any); ok { + switch mod["module"] { + case "autoops_es": + foundAutoOps = true + case "some_other_module": + foundOther = true + } } - return p + } + assert.True(t, foundAutoOps, "operator autoops_es modules must be preserved") + assert.True(t, foundOther, "user module must be appended") + }, + }, + { + // When the user provides an autoops_es module the operator skips injecting its own + // default modules (Metrics + Templates), giving the user full control over which + // autoops_es data is collected. The operator still injects connection settings + // (hosts, ssl) into all autoops_es modules and always merges mandatory fields last. + // Non-module user additions (exporters, pipelines) are present alongside the + // operator's mandatory ones. + name: "user provides autoops_es module - replaces operator defaults, custom exporter and pipeline added alongside", + specConfigYAML: []byte(` +receivers: + metricbeatreceiver: + metricbeat: + modules: + - module: autoops_es + period: 30s + metricsets: + - cluster_health +exporters: + debug: + verbosity: detailed +service: + pipelines: + logs/debug: + receivers: [metricbeatreceiver] + exporters: [debug] +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + got := unpackConfigYAML(t, cm) + + // Only the user's module is present — operator defaults are not injected when + // the user supplies at least one autoops_es module. + modules := asSlice(t, asMap(t, asMap(t, asMap(t, got["receivers"])["metricbeatreceiver"])["metricbeat"])["modules"]) + require.Len(t, modules, 1, "only the user's autoops_es module, operator defaults omitted") + + mod := asMap(t, modules[0]) + assert.Equal(t, "autoops_es", mod["module"]) + assert.Equal(t, "30s", mod["period"]) + // Operator injects connection settings regardless of module source. + assert.Equal(t, "${env:AUTOOPS_ES_URL}", mod["hosts"]) + assert.Equal(t, "none", asMap(t, mod["ssl"])["verification_mode"]) + + // User's exporter is present alongside the operator's otlphttp. + exporters := asMap(t, got["exporters"]) + _, hasDebug := exporters["debug"] + _, hasOTLP := exporters["otlphttp"] + assert.True(t, hasDebug, "user's debug exporter must be present") + assert.True(t, hasOTLP, "operator's otlphttp exporter must be present") + + // User's custom pipeline is present alongside the mandatory logs pipeline. + pipelines := asMap(t, asMap(t, got["service"])["pipelines"]) + _, hasLogs := pipelines["logs"] + _, hasDebugPipeline := pipelines["logs/debug"] + assert.True(t, hasLogs, "mandatory logs pipeline must be present") + assert.True(t, hasDebugPipeline, "user's logs/debug pipeline must be present") + }, + }, + { + // When the user provides autoops_es modules and SSL is enabled on the ES cluster, + // injectModuleConnectionSettings must always override any user-supplied ssl fields + // with the operator-managed values. The user-supplied ssl.verification_mode: none + // and a bogus CA path must be replaced by the operator's certificate settings. + name: "user provides autoops_es module with SSL-enabled ES - operator always overrides ssl fields", + es: func() esv1.Elasticsearch { return mkES(true) }, + specConfigYAML: []byte(` +receivers: + metricbeatreceiver: + metricbeat: + modules: + - module: autoops_es + period: 30s + metricsets: + - cluster_health + hosts: http://wrong-host:9200 + ssl: + verification_mode: none + certificate_authorities: + - /wrong/ca/path.crt +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + got := unpackConfigYAML(t, cm) + modules := asSlice(t, asMap(t, asMap(t, asMap(t, got["receivers"])["metricbeatreceiver"])["metricbeat"])["modules"]) + require.Len(t, modules, 1, "only the user's module, operator defaults omitted") + + mod := asMap(t, modules[0]) + assert.Equal(t, "${env:AUTOOPS_ES_URL}", mod["hosts"], "operator must override user-supplied hosts") + ssl := asMap(t, mod["ssl"]) + assert.Equal(t, "certificate", ssl["verification_mode"], "operator must override user-supplied verification_mode") + cas := asSlice(t, ssl["certificate_authorities"]) + require.Len(t, cas, 1) + assert.Equal(t, "/mnt/elastic-internal/es-ca/default-test-es/ca.crt", cas[0], "operator must override user-supplied CA path") + _, hasClientCert := ssl["certificate"] + assert.False(t, hasClientCert, "no client cert expected when client auth is not required") + }, + }, + { + // When client authentication is required the operator must inject ssl.certificate + // and ssl.key, overriding any user-supplied values for all ssl connection fields. + name: "user provides autoops_es module with SSL and client cert - operator always overrides all ssl fields", + es: func() esv1.Elasticsearch { + es := mkES(true) + es.Annotations = map[string]string{annotation.ClientAuthenticationRequiredAnnotation: "true"} + return es + }, + specConfigYAML: []byte(` +receivers: + metricbeatreceiver: + metricbeat: + modules: + - module: autoops_es + period: 30s + metricsets: + - cluster_health + hosts: http://wrong-host:9200 + ssl: + verification_mode: none + certificate_authorities: + - /wrong/ca/path.crt + certificate: /wrong/cert.crt + key: /wrong/key.key +`), + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + got := unpackConfigYAML(t, cm) + modules := asSlice(t, asMap(t, asMap(t, asMap(t, got["receivers"])["metricbeatreceiver"])["metricbeat"])["modules"]) + require.Len(t, modules, 1) + + mod := asMap(t, modules[0]) + assert.Equal(t, "${env:AUTOOPS_ES_URL}", mod["hosts"], "operator must override user-supplied hosts") + ssl := asMap(t, mod["ssl"]) + assert.Equal(t, "certificate", ssl["verification_mode"], "operator must override user-supplied verification_mode") + cas := asSlice(t, ssl["certificate_authorities"]) + require.Len(t, cas, 1) + assert.Equal(t, "/mnt/elastic-internal/es-ca/default-test-es/ca.crt", cas[0], "operator must override user-supplied CA path") + assert.Equal(t, "/mnt/elastic-internal/es-client-cert/default-test-es/tls.crt", ssl["certificate"], "operator must override user-supplied certificate path") + assert.Equal(t, "/mnt/elastic-internal/es-client-cert/default-test-es/tls.key", ssl["key"], "operator must override user-supplied key path") + }, + }, + // ConfigRef: secret contains valid config - autoops_es module present after merge. + { + name: "ConfigRef secret with valid config - merged successfully", + policy: func() autoopsv1alpha1.AutoOpsAgentPolicy { + p := mkPolicy() + p.Spec.ConfigRef = &commonv1.ConfigSource{SecretRef: commonv1.SecretRef{SecretName: "my-config-secret"}} + return p + }, + secrets: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "my-config-secret", Namespace: "default"}, + Data: map[string][]byte{autoopsv1alpha1.ConfigFileName: validConfigRefYAML}, }, - es: func() esv1.Elasticsearch { - es := mkES(false) - es.Namespace = "test-namespace" - return es + }, + check: func(t *testing.T, cm corev1.ConfigMap) { + t.Helper() + got := unpackConfigYAML(t, cm) + modules := asSlice(t, asMap(t, asMap(t, asMap(t, got["receivers"])["metricbeatreceiver"])["metricbeat"])["modules"]) + var autoOpsModule map[string]any + for _, m := range modules { + if mod, ok := m.(map[string]any); ok && mod["module"] == "autoops_es" { + autoOpsModule = mod + break + } + } + require.NotNil(t, autoOpsModule, "autoops_es module must be present") + assert.Equal(t, "${env:AUTOOPS_ES_URL}", autoOpsModule["hosts"], "operator must override user-supplied hosts") + ssl := asMap(t, autoOpsModule["ssl"]) + assert.Equal(t, "none", ssl["verification_mode"], "operator must set verification_mode to none for non-SSL ES") + }, + }, + { + name: "ConfigRef secret missing expected key - error", + policy: func() autoopsv1alpha1.AutoOpsAgentPolicy { + p := mkPolicy() + p.Spec.ConfigRef = &commonv1.ConfigSource{SecretRef: commonv1.SecretRef{SecretName: "my-config-secret"}} + return p + }, + secrets: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "my-config-secret", Namespace: "default"}, + Data: map[string][]byte{"wrong-key.yml": validConfigRefYAML}, }, }, - want: defaultConfigNoSSL, + wantErr: true, + }, + { + name: "ConfigRef secret not found - error", + policy: func() autoopsv1alpha1.AutoOpsAgentPolicy { + p := mkPolicy() + p.Spec.ConfigRef = &commonv1.ConfigSource{SecretRef: commonv1.SecretRef{SecretName: "nonexistent-secret"}} + return p + }, + secrets: []client.Object{}, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := buildAutoOpsESConfigMap(tt.args.policy(), tt.args.es()) - if (err != nil) != tt.wantErr { - t.Errorf("buildAutoOpsESConfigMap() error = %v, wantErr %v", err, tt.wantErr) - return + policy := mkPolicy() + if tt.policy != nil { + policy = tt.policy() } - if tt.wantErr { - return + es := mkES(false) + if tt.es != nil { + es = tt.es() } - if l := got.Labels[commonv1.RestrictWatchedResourcesLabelName]; l != commonv1.RestrictWatchedResourcesLabelValue { - t.Errorf("expected to have the watch label and it does not. Value = %s", l) - } - policy := tt.args.policy() - es := tt.args.es() - // Validate ConfigMap structure - expectedName := autoopsv1alpha1.Config(policy.GetName(), es) - if got.Name != expectedName { - t.Errorf("buildAutoOpsESConfigMap() ConfigMap name = %v, want %v", got.Name, expectedName) - } - if got.Namespace != policy.GetNamespace() { - t.Errorf("buildAutoOpsESConfigMap() ConfigMap namespace = %v, want %v", got.Namespace, policy.GetNamespace()) + if len(tt.specConfigYAML) > 0 { + parsed, err := uyaml.NewConfig(tt.specConfigYAML, commonv1.CfgOptions...) + require.NoError(t, err) + var data map[string]any + require.NoError(t, parsed.Unpack(&data)) + policy.Spec.Config = &commonv1.Config{Data: data} } - configYAML, exists := got.Data[autoOpsESConfigFileName] - if !exists { - t.Errorf("buildAutoOpsESConfigMap() ConfigMap missing key %v", autoOpsESConfigFileName) + + r := mkReconciler(t, tt.secrets...) + cm, err := r.ReconcileAutoOpsESConfigMap(context.Background(), policy, es) + + if tt.wantErr { + require.Error(t, err) return } - - // Parse both configs for comparison - var gotCfg map[string]any - gotParsed, err := uyaml.NewConfig([]byte(configYAML), commonv1.CfgOptions...) require.NoError(t, err) - require.NoError(t, gotParsed.Unpack(&gotCfg)) + require.NotNil(t, cm) - var wantCfg map[string]any - wantParsed, err := uyaml.NewConfig(tt.want, commonv1.CfgOptions...) - require.NoError(t, err) - require.NoError(t, wantParsed.Unpack(&wantCfg)) + // Always validate ConfigMap identity. + assert.Equal(t, autoopsv1alpha1.Config(policy.GetName(), es), cm.Name) + assert.Equal(t, policy.GetNamespace(), cm.Namespace) + _, hasKey := cm.Data[autoopsv1alpha1.ConfigFileName] + assert.True(t, hasKey, "ConfigMap must contain key %s", autoopsv1alpha1.ConfigFileName) - if diff := deep.Equal(wantCfg, gotCfg); diff != nil { - t.Errorf("buildAutoOpsESConfigMap() config mismatch: %v", diff) + if tt.want != nil { + var gotCfg, wantCfg map[string]any + gotParsed, err := uyaml.NewConfig([]byte(cm.Data[autoopsv1alpha1.ConfigFileName]), commonv1.CfgOptions...) + require.NoError(t, err) + require.NoError(t, gotParsed.Unpack(&gotCfg)) + wantParsed, err := uyaml.NewConfig(tt.want, commonv1.CfgOptions...) + require.NoError(t, err) + require.NoError(t, wantParsed.Unpack(&wantCfg)) + if diff := deep.Equal(wantCfg, gotCfg); diff != nil { + t.Errorf("config mismatch: %v", diff) + } + } + if tt.check != nil { + tt.check(t, *cm) } }) } diff --git a/pkg/controller/autoops/controller.go b/pkg/controller/autoops/controller.go index d5c3e43887..d314944dea 100644 --- a/pkg/controller/autoops/controller.go +++ b/pkg/controller/autoops/controller.go @@ -26,6 +26,7 @@ import ( esv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/elasticsearch/v1" autoopsvalidation "github.com/elastic/cloud-on-k8s/v3/pkg/controller/autoops/validation" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/driver" commonesclient "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/esclient" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/events" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/license" @@ -122,6 +123,7 @@ func reconcileRequestForAutoOpsPolicyFromDeployment() handler.TypedEventHandler[ } var _ reconcile.Reconciler = (*AgentPolicyReconciler)(nil) +var _ driver.Interface = (*AgentPolicyReconciler)(nil) // AgentPolicyReconciler reconciles an AutoOpsAgentPolicy object type AgentPolicyReconciler struct { @@ -228,6 +230,7 @@ func (r *AgentPolicyReconciler) onDelete(ctx context.Context, obj types.Namespac // Remove dynamic watches on secrets r.dynamicWatches.Secrets.RemoveHandlerForKey(configSecretWatchName(obj)) + r.dynamicWatches.Secrets.RemoveHandlerForKey(common.ConfigRefWatchName(obj)) // Cleanup API keys for all Elasticsearch clusters that match this policy. // Query for secrets labeled with this policy to find all associated ES clusters. @@ -282,7 +285,9 @@ func (r *AgentPolicyReconciler) reconcileWatches(policy autoopsv1alpha1.AutoOpsA secretNames := []string{policy.Spec.AutoOpsRef.SecretName} - // Set up dynamic watches for referenced secrets + // Set up dynamic watch for the AutoOpsRef secret. + // The configRef watch is managed by common.ParseConfigRef, called unconditionally + // in ReconcileAutoOpsESConfigMap, so we do not set it up here. return watches.WatchUserProvidedSecrets( watcher, r.dynamicWatches, @@ -295,3 +300,12 @@ func (r *AgentPolicyReconciler) reconcileWatches(policy autoopsv1alpha1.AutoOpsA func configSecretWatchName(watcher types.NamespacedName) string { return fmt.Sprintf("%s-%s-config-secret", watcher.Namespace, watcher.Name) } + +// K8sClient returns the Kubernetes client from the reconciler, satisfying driver.Interface. +func (r *AgentPolicyReconciler) K8sClient() k8s.Client { return r.Client } + +// DynamicWatches returns the set of dynamic watches from the reconciler, satisfying driver.Interface. +func (r *AgentPolicyReconciler) DynamicWatches() watches.DynamicWatches { return r.dynamicWatches } + +// Recorder returns the event recorder from the reconciler, satisfying driver.Interface. +func (r *AgentPolicyReconciler) Recorder() toolsevents.EventRecorder { return r.recorder } diff --git a/pkg/controller/autoops/expected.go b/pkg/controller/autoops/expected.go index f981d3835e..02b62e0a4a 100644 --- a/pkg/controller/autoops/expected.go +++ b/pkg/controller/autoops/expected.go @@ -109,7 +109,7 @@ func (r *AgentPolicyReconciler) buildDeployment(configHash string, policy autoop annotations := map[string]string{configHashAnnotationName: configHash} meta := metadata.Propagate(&policy, metadata.Metadata{Labels: labels, Annotations: annotations}) builder := defaults.NewPodTemplateBuilder(policy.Spec.PodTemplate, autoopsv1alpha1.AutoOpsAgentContainerName). - WithArgs("--config", path.Join(configVolumePath, autoOpsESConfigFileName)). + WithArgs("--config", path.Join(configVolumePath, autoopsv1alpha1.ConfigFileName)). WithLabels(meta.Labels). WithAnnotations(meta.Annotations). WithDockerImage(policy.Spec.Image, container.ImageRepository(container.AutoOpsAgentImage, v)). @@ -188,7 +188,7 @@ func buildConfigHash( ) (string, error) { configHash := fnv.New32a() - if configData, ok := configMap.Data[autoOpsESConfigFileName]; ok { + if configData, ok := configMap.Data[autoopsv1alpha1.ConfigFileName]; ok { _, _ = configHash.Write([]byte(configData)) } diff --git a/pkg/controller/autoops/expected_test.go b/pkg/controller/autoops/expected_test.go index 815e635d42..287941cfd9 100644 --- a/pkg/controller/autoops/expected_test.go +++ b/pkg/controller/autoops/expected_test.go @@ -147,7 +147,7 @@ func TestReconcileAutoOpsAgentPolicy_deploymentParams(t *testing.T) { Namespace: tt.args.autoops.Namespace, }, Data: map[string]string{ - autoOpsESConfigFileName: configData, + autoopsv1alpha1.ConfigFileName: configData, }, } diff --git a/pkg/controller/autoops/reconcile.go b/pkg/controller/autoops/reconcile.go index cb925425c4..4719ed82d7 100644 --- a/pkg/controller/autoops/reconcile.go +++ b/pkg/controller/autoops/reconcile.go @@ -192,7 +192,7 @@ func (r *AgentPolicyReconciler) internalReconcile( continue } - configMap, err := ReconcileAutoOpsESConfigMap(ctx, r.Client, policy, es) + configMap, err := r.ReconcileAutoOpsESConfigMap(ctx, policy, es) if err != nil { log.Error(err, "while reconciling AutoOps ES config map") state.ResourceError(es, "Failed to reconcile AutoOps ES config map", err) diff --git a/pkg/controller/autoops/reconcile_test.go b/pkg/controller/autoops/reconcile_test.go index af7381abd8..30128ce7e9 100644 --- a/pkg/controller/autoops/reconcile_test.go +++ b/pkg/controller/autoops/reconcile_test.go @@ -169,7 +169,7 @@ func TestAutoOpsAgentPolicyReconciler_internalReconcile(t *testing.T) { Namespace: "ns-1", }, Data: map[string]string{ - autoOpsESConfigFileName: "test-config", + autoopsv1alpha1.ConfigFileName: "test-config", }, }, &corev1.Secret{ @@ -217,7 +217,7 @@ func TestAutoOpsAgentPolicyReconciler_internalReconcile(t *testing.T) { Namespace: "ns-1", }, Data: map[string]string{ - autoOpsESConfigFileName: "test-config", + autoopsv1alpha1.ConfigFileName: "test-config", }, }, &corev1.Secret{ @@ -305,7 +305,7 @@ func TestAutoOpsAgentPolicyReconciler_internalReconcile(t *testing.T) { Namespace: "ns-1", }, Data: map[string]string{ - autoOpsESConfigFileName: "test-config", + autoopsv1alpha1.ConfigFileName: "test-config", }, }, &corev1.Secret{ diff --git a/pkg/controller/autoops/validation/validations.go b/pkg/controller/autoops/validation/validations.go index 2b7f620817..8895fe777b 100644 --- a/pkg/controller/autoops/validation/validations.go +++ b/pkg/controller/autoops/validation/validations.go @@ -29,6 +29,7 @@ func validations(ctx context.Context, checker license.Checker) []validation { }, checkConfigSecretName, checkResourceSelector, + checkSingleConfigSource, commonv1.PauseOrchestrationAnnotationCheck[*autoopsv1alpha1.AutoOpsAgentPolicy](), } } @@ -72,3 +73,14 @@ func checkResourceSelector(policy *autoopsv1alpha1.AutoOpsAgentPolicy) field.Err } return nil } + +func checkSingleConfigSource(policy *autoopsv1alpha1.AutoOpsAgentPolicy) field.ErrorList { + if policy.Spec.Config != nil && policy.Spec.ConfigRef != nil { + msg := "Specify at most one of [`config`, `configRef`], not both" + return field.ErrorList{ + field.Forbidden(field.NewPath("spec").Child("config"), msg), + field.Forbidden(field.NewPath("spec").Child("configRef"), msg), + } + } + return nil +} diff --git a/pkg/controller/autoops/validation/validations_test.go b/pkg/controller/autoops/validation/validations_test.go index 2d3e6fc7f5..1578f146fe 100644 --- a/pkg/controller/autoops/validation/validations_test.go +++ b/pkg/controller/autoops/validation/validations_test.go @@ -269,3 +269,47 @@ func TestValidate(t *testing.T) { }) } } + +func TestCheckSingleConfigSource(t *testing.T) { + tests := []struct { + name string + config *commonv1.Config + configRef *commonv1.ConfigSource + wantErrCount int + }{ + { + name: "both nil - no error", + config: nil, + configRef: nil, + wantErrCount: 0, + }, + { + name: "only Config set - no error", + config: &commonv1.Config{Data: map[string]any{"key": "value"}}, + configRef: nil, + wantErrCount: 0, + }, + { + name: "only ConfigRef set - no error", + config: nil, + configRef: &commonv1.ConfigSource{SecretRef: commonv1.SecretRef{SecretName: "my-secret"}}, + wantErrCount: 0, + }, + { + name: "both Config and ConfigRef set - two Forbidden errors", + config: &commonv1.Config{Data: map[string]any{"key": "value"}}, + configRef: &commonv1.ConfigSource{SecretRef: commonv1.SecretRef{SecretName: "my-secret"}}, + wantErrCount: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + policy := newPolicy("9.2.4") + policy.Spec.Config = tt.config + policy.Spec.ConfigRef = tt.configRef + errs := checkSingleConfigSource(policy) + require.Len(t, errs, tt.wantErrCount) + }) + } +}