diff --git a/config/crds/v1/all-crds.yaml b/config/crds/v1/all-crds.yaml index 749f0672cc1..4a7bc3e2a5a 100644 --- a/config/crds/v1/all-crds.yaml +++ b/config/crds/v1/all-crds.yaml @@ -10673,6 +10673,9 @@ spec: - jsonPath: .metadata.creationTimestamp name: Age type: date + - jsonPath: .spec.weight + name: Weight + type: integer name: v1alpha1 schema: openAPIV3Schema: @@ -10932,6 +10935,13 @@ spec: - secretName type: object type: array + weight: + default: 0 + description: |- + Weight determines the priority of this policy when multiple policies target the same resource. + Lower weight values take precedence. Defaults to 0. + format: int32 + type: integer type: object status: properties: diff --git a/config/crds/v1/resources/stackconfigpolicy.k8s.elastic.co_stackconfigpolicies.yaml b/config/crds/v1/resources/stackconfigpolicy.k8s.elastic.co_stackconfigpolicies.yaml index 146a6d3ab74..df6bb09927c 100644 --- a/config/crds/v1/resources/stackconfigpolicy.k8s.elastic.co_stackconfigpolicies.yaml +++ b/config/crds/v1/resources/stackconfigpolicy.k8s.elastic.co_stackconfigpolicies.yaml @@ -29,6 +29,9 @@ spec: - jsonPath: .metadata.creationTimestamp name: Age type: date + - jsonPath: .spec.weight + name: Weight + type: integer name: v1alpha1 schema: openAPIV3Schema: @@ -288,6 +291,13 @@ spec: - secretName type: object type: array + weight: + default: 0 + description: |- + Weight determines the priority of this policy when multiple policies target the same resource. + Lower weight values take precedence. Defaults to 0. + format: int32 + type: integer type: object status: properties: 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 f1d9bcf4f79..67fe5100fd5 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 @@ -10743,6 +10743,9 @@ spec: - jsonPath: .metadata.creationTimestamp name: Age type: date + - jsonPath: .spec.weight + name: Weight + type: integer name: v1alpha1 schema: openAPIV3Schema: @@ -11002,6 +11005,13 @@ spec: - secretName type: object type: array + weight: + default: 0 + description: |- + Weight determines the priority of this policy when multiple policies target the same resource. + Lower weight values take precedence. Defaults to 0. + format: int32 + type: integer type: object status: properties: diff --git a/docs/reference/api-reference/main.md b/docs/reference/api-reference/main.md index 7bbb407bfd7..ac31396a12d 100644 --- a/docs/reference/api-reference/main.md +++ b/docs/reference/api-reference/main.md @@ -1991,6 +1991,8 @@ Package v1alpha1 contains API schema definitions for managing StackConfigPolicy | *`secureSettings`* __[SecretSource](#secretsource) array__ | SecureSettings are additional Secrets that contain data to be configured to Elasticsearch's keystore. | + + ### IndexTemplates [#indextemplates] @@ -2068,6 +2070,7 @@ StackConfigPolicy represents a StackConfigPolicy resource in a Kubernetes cluste | Field | Description | | --- | --- | | *`resourceSelector`* __[LabelSelector](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.32/#labelselector-v1-meta)__ | | +| *`weight`* __integer__ | Weight determines the priority of this policy when multiple policies target the same resource.
Lower weight values take precedence. Defaults to 0. | | *`secureSettings`* __[SecretSource](#secretsource) array__ | Deprecated: SecureSettings only applies to Elasticsearch and is deprecated. It must be set per application instead. | | *`elasticsearch`* __[ElasticsearchConfigPolicySpec](#elasticsearchconfigpolicyspec)__ | | | *`kibana`* __[KibanaConfigPolicySpec](#kibanaconfigpolicyspec)__ | | diff --git a/pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go b/pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go index 058f197d231..d6f5a8fd6a3 100644 --- a/pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go +++ b/pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go @@ -34,6 +34,7 @@ func init() { // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.readyCount",description="Resources configured" // +kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" +// +kubebuilder:printcolumn:name="Weight",type="integer",JSONPath=".spec.weight" // +kubebuilder:subresource:status // +kubebuilder:storageversion type StackConfigPolicy struct { @@ -55,6 +56,10 @@ type StackConfigPolicyList struct { type StackConfigPolicySpec struct { ResourceSelector metav1.LabelSelector `json:"resourceSelector,omitempty"` + // Weight determines the priority of this policy when multiple policies target the same resource. + // Lower weight values take precedence. Defaults to 0. + // +kubebuilder:default=0 + Weight int32 `json:"weight,omitempty"` // Deprecated: SecureSettings only applies to Elasticsearch and is deprecated. It must be set per application instead. SecureSettings []commonv1.SecretSource `json:"secureSettings,omitempty"` Elasticsearch ElasticsearchConfigPolicySpec `json:"elasticsearch,omitempty"` @@ -94,6 +99,53 @@ type ElasticsearchConfigPolicySpec struct { SecureSettings []commonv1.SecretSource `json:"secureSettings,omitempty"` } +// GetElasticsearchNamespacedSecureSettings returns the Elasticsearch secure settings from this policy +// as NamespacedSecretSources, with each secret source namespaced to the policy's namespace. +// Returns nil if the policy is nil or has no Elasticsearch secure settings defined. +func (p *StackConfigPolicy) GetElasticsearchNamespacedSecureSettings() []commonv1.NamespacedSecretSource { + if p == nil { + return nil + } + return toNamespacedSecretSources(&p.Spec.Elasticsearch, p.Namespace) +} + +// GetKibanaNamespacedSecureSettings returns the Kibana secure settings from this policy +// as NamespacedSecretSources, with each secret source namespaced to the policy's namespace. +// Returns nil if the policy is nil or has no Kibana secure settings defined. +func (p *StackConfigPolicy) GetKibanaNamespacedSecureSettings() []commonv1.NamespacedSecretSource { + if p == nil { + return nil + } + return toNamespacedSecretSources(&p.Spec.Kibana, p.Namespace) +} + +// HasSecureSettings represents a ConfigPolicySpec that has secure settings. +// +kubebuilder:object:generate=false +type HasSecureSettings interface { + GetSecureSettings() []commonv1.SecretSource +} + +func toNamespacedSecretSources(hasSecureSettings HasSecureSettings, inNamespace string) []commonv1.NamespacedSecretSource { + secureSettings := hasSecureSettings.GetSecureSettings() + namespacedSecretSources := make([]commonv1.NamespacedSecretSource, len(secureSettings)) + for i, s := range secureSettings { + namespacedSecretSources[i] = commonv1.NamespacedSecretSource{ + Namespace: inNamespace, + SecretName: s.SecretName, + Entries: s.Entries, + } + } + return namespacedSecretSources +} + +// GetSecureSettings returns the secure settings of the ElasticsearchConfigPolicySpec. +func (e *ElasticsearchConfigPolicySpec) GetSecureSettings() []commonv1.SecretSource { + if e == nil { + return nil + } + return e.SecureSettings +} + type KibanaConfigPolicySpec struct { // Config holds the settings that go into kibana.yml. // +kubebuilder:pruning:PreserveUnknownFields @@ -103,6 +155,14 @@ type KibanaConfigPolicySpec struct { SecureSettings []commonv1.SecretSource `json:"secureSettings,omitempty"` } +// GetSecureSettings returns the secure settings of the KibanaConfigPolicySpec. +func (k *KibanaConfigPolicySpec) GetSecureSettings() []commonv1.SecretSource { + if k == nil { + return nil + } + return k.SecureSettings +} + type ResourceType string const ( diff --git a/pkg/controller/common/reconciler/secret.go b/pkg/controller/common/reconciler/secret.go index d280023d4b8..e4f2774c40e 100644 --- a/pkg/controller/common/reconciler/secret.go +++ b/pkg/controller/common/reconciler/secret.go @@ -6,7 +6,9 @@ package reconciler import ( "context" + "encoding/json" "reflect" + "strings" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -28,6 +30,7 @@ const ( SoftOwnerNamespaceLabel = "eck.k8s.elastic.co/owner-namespace" SoftOwnerNameLabel = "eck.k8s.elastic.co/owner-name" SoftOwnerKindLabel = "eck.k8s.elastic.co/owner-kind" + SoftOwnerRefsAnnotation = "eck.k8s.elastic.co/owner-refs" ) func WithPostUpdate(f func()) func(p *Params) { @@ -92,6 +95,49 @@ func SoftOwnerRefFromLabels(labels map[string]string) (SoftOwnerRef, bool) { return SoftOwnerRef{Namespace: namespace, Name: name, Kind: kind}, true } +// SoftOwnerRefs returns the soft owner references of the given object. +func SoftOwnerRefs(obj metav1.Object) ([]SoftOwnerRef, error) { + // Check if this Secret has a soft-owner kind label set + ownerKind, exists := obj.GetLabels()[SoftOwnerKindLabel] + if !exists { + // Not a soft-owned secret + return nil, nil + } + + // Check for multi-policy ownership (annotation-based) + if ownerRefsBytes, exists := obj.GetAnnotations()[SoftOwnerRefsAnnotation]; exists { + // Multi-policy soft owned secret - parse the list of owners + var ownerRefsSlice []string + if err := json.Unmarshal([]byte(ownerRefsBytes), &ownerRefsSlice); err != nil { + return nil, err + } + + // Convert the list to []SoftOwnerRef + var ownerRefsNsn []SoftOwnerRef + for _, nsnStr := range ownerRefsSlice { + // Split the string format "namespace/name" into components + nsnComponents := strings.Split(nsnStr, string(types.Separator)) + if len(nsnComponents) != 2 { + // Skip malformed entries + continue + } + ownerRefsNsn = append(ownerRefsNsn, SoftOwnerRef{Namespace: nsnComponents[0], Name: nsnComponents[1], Kind: ownerKind}) + } + + return ownerRefsNsn, nil + } + + // Fall back to single-policy ownership (label-based) + currentOwner, referenced := SoftOwnerRefFromLabels(obj.GetLabels()) + if !referenced { + // No soft owner found in labels + return nil, nil + } + + // Return the single owner as a slice with one element + return []SoftOwnerRef{currentOwner}, nil +} + // ReconcileSecretNoOwnerRef should be called to reconcile a Secret for which we explicitly don't want // an owner reference to be set, and want existing ownerReferences from previous operator versions to be removed, // because of this k8s bug: https://github.com/kubernetes/kubernetes/issues/65200 (fixed in k8s 1.20). @@ -200,43 +246,54 @@ func GarbageCollectAllSoftOwnedOrphanSecrets(ctx context.Context, c k8s.Client, var secrets corev1.SecretList if err := c.List(ctx, &secrets, - client.HasLabels{SoftOwnerNamespaceLabel, SoftOwnerNameLabel, SoftOwnerKindLabel}, + client.HasLabels{SoftOwnerKindLabel}, ); err != nil { return err } // remove any secret whose owner doesn't exist for i := range secrets.Items { secret := secrets.Items[i] - softOwner, referenced := SoftOwnerRefFromLabels(secret.Labels) - if !referenced { - continue - } - if restrictedToOwnerNamespace(softOwner.Kind) && softOwner.Namespace != secret.Namespace { - // Secret references an owner in a different namespace: this likely results - // from a "manual" copy of the secret in another namespace, not handled by the operator. - // We don't want to touch that secret. - continue + softOwners, err := SoftOwnerRefs(&secret) + if err != nil { + return err } - owner, managed := ownerKinds[softOwner.Kind] - if !managed { + if len(softOwners) == 0 { continue } - owner = k8s.DeepCopyObject(owner) - err := c.Get(ctx, types.NamespacedName{Namespace: softOwner.Namespace, Name: softOwner.Name}, owner) - if err != nil { - if apierrors.IsNotFound(err) { - // owner doesn't exit anymore - ulog.FromContext(ctx).Info("Deleting secret as part of garbage collection", - "namespace", secret.Namespace, "secret_name", secret.Name, - "owner_kind", softOwner.Kind, "owner_namespace", softOwner.Namespace, "owner_name", softOwner.Name, - ) - options := client.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &secret.UID}} - if err := c.Delete(ctx, &secret, &options); err != nil && !apierrors.IsNotFound(err) { - return err - } + + missingOwners := make(map[types.NamespacedName]client.Object) + for _, softOwner := range softOwners { + if restrictedToOwnerNamespace(softOwner.Kind) && softOwner.Namespace != secret.Namespace { + // Secret references an owner in a different namespace: this likely results + // from a "manual" copy of the secret in another namespace, not handled by the operator. + // We don't want to touch that secret. continue } - return err + owner, managed := ownerKinds[softOwner.Kind] + if !managed { + continue + } + owner = k8s.DeepCopyObject(owner) + err := c.Get(ctx, types.NamespacedName{Namespace: softOwner.Namespace, Name: softOwner.Name}, owner) + if err != nil { + if apierrors.IsNotFound(err) { + // owner doesn't exit anymore + ulog.FromContext(ctx).Info("Deleting secret as part of garbage collection", + "namespace", secret.Namespace, "secret_name", secret.Name, + "owner_kind", softOwner.Kind, "owner_namespace", softOwner.Namespace, "owner_name", softOwner.Name, + ) + missingOwners[types.NamespacedName{Namespace: softOwner.Namespace, Name: softOwner.Name}] = owner + continue + } + return err + } + } + + if len(missingOwners) == len(softOwners) { + options := client.DeleteOptions{Preconditions: &metav1.Preconditions{UID: &secret.UID}} + if err := c.Delete(ctx, &secret, &options); err != nil && !apierrors.IsNotFound(err) { + return err + } } // owner still exists, keep the secret } diff --git a/pkg/controller/common/reconciler/secret_test.go b/pkg/controller/common/reconciler/secret_test.go index e4e616616eb..e7d1bc8f5e8 100644 --- a/pkg/controller/common/reconciler/secret_test.go +++ b/pkg/controller/common/reconciler/secret_test.go @@ -9,6 +9,7 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -257,6 +258,15 @@ func ownedSecret(namespace, name, ownerNs, ownerName, ownerKind string) *corev1. }}} } +func ownedSecretMultiRefs(namespace, name, ownerRefs, ownerKind string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: name, Labels: map[string]string{ + SoftOwnerKindLabel: ownerKind, + }, Annotations: map[string]string{ + SoftOwnerRefsAnnotation: ownerRefs, + }}} +} + func TestGarbageCollectSoftOwnedSecrets(t *testing.T) { tests := []struct { name string @@ -433,6 +443,37 @@ func TestGarbageCollectAllSoftOwnedOrphanSecrets(t *testing.T) { ownedSecret("ns", "secret-1", "another-namespace", sampleOwner().Name, sampleOwner().Kind), }, }, + { + name: "secret with multiple soft-owners that all exist", + runtimeObjs: []client.Object{ + &policyv1alpha1.StackConfigPolicy{ObjectMeta: metav1.ObjectMeta{Name: "policy-1", Namespace: "namespace-1"}}, + &policyv1alpha1.StackConfigPolicy{ObjectMeta: metav1.ObjectMeta{Name: "policy-2", Namespace: "namespace-2"}}, + &policyv1alpha1.StackConfigPolicy{ObjectMeta: metav1.ObjectMeta{Name: "policy-3", Namespace: "namespace-3"}}, + ownedSecretMultiRefs("ns", "secret-1", `["namespace-1/policy-1","namespace-2/policy-2","namespace-3/policy-3"]`, "StackConfigPolicy"), + }, + wantObjs: []client.Object{ + ownedSecretMultiRefs("ns", "secret-1", `["namespace-1/policy-1","namespace-2/policy-2","namespace-3/policy-3"]`, "StackConfigPolicy"), + }, + }, + { + name: "secret with multiple soft-owners that all exist but some in different namespace", + runtimeObjs: []client.Object{ + &policyv1alpha1.StackConfigPolicy{ObjectMeta: metav1.ObjectMeta{Name: "policy-1", Namespace: "namespace-1"}}, + &policyv1alpha1.StackConfigPolicy{ObjectMeta: metav1.ObjectMeta{Name: "policy-2", Namespace: "namespace-other"}}, + &policyv1alpha1.StackConfigPolicy{ObjectMeta: metav1.ObjectMeta{Name: "policy-3", Namespace: "namespace-3"}}, + ownedSecretMultiRefs("ns", "secret-1", `["namespace-1/policy-1","namespace-2/policy-2","namespace-3/policy-3"]`, "StackConfigPolicy"), + }, + wantObjs: []client.Object{ + ownedSecretMultiRefs("ns", "secret-1", `["namespace-1/policy-1","namespace-2/policy-2","namespace-3/policy-3"]`, "StackConfigPolicy"), + }, + }, + { + name: "secret with multiple soft-owners that none exists", + runtimeObjs: []client.Object{ + ownedSecretMultiRefs("ns", "secret-1", `["namespace-1/policy-1","namespace-2/policy-2","namespace-3/policy-3"]`, "StackConfigPolicy"), + }, + wantObjs: []client.Object{}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -546,3 +587,132 @@ func TestSoftOwnerRefFromLabels(t *testing.T) { }) } } + +//nolint:thelper +func TestSoftOwnerRefs(t *testing.T) { + tests := []struct { + name string + secret *corev1.Secret + validate func(t *testing.T, owners []SoftOwnerRef, err error) + }{ + { + name: "returns multi-owner policies from annotation", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + Annotations: map[string]string{ + SoftOwnerRefsAnnotation: `["namespace-1/policy-1","namespace-2/policy-2"]`, + }, + }, + }, + validate: func(t *testing.T, owners []SoftOwnerRef, err error) { + require.NoError(t, err) + require.Len(t, owners, 2) + assert.Contains(t, owners, SoftOwnerRef{Name: "policy-1", Namespace: "namespace-1", Kind: policyv1alpha1.Kind}) + assert.Contains(t, owners, SoftOwnerRef{Name: "policy-2", Namespace: "namespace-2", Kind: policyv1alpha1.Kind}) + }, + }, + { + name: "returns single-owner policy from labels", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + SoftOwnerKindLabel: policyv1alpha1.Kind, + SoftOwnerNameLabel: "single-policy", + SoftOwnerNamespaceLabel: "single-namespace", + }, + }, + }, + validate: func(t *testing.T, owners []SoftOwnerRef, err error) { + require.NoError(t, err) + require.Len(t, owners, 1) + assert.Equal(t, SoftOwnerRef{Name: "single-policy", Namespace: "single-namespace", Kind: policyv1alpha1.Kind}, owners[0]) + }, + }, + { + name: "returns nil when secret has kind label but no owner labels", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + SoftOwnerKindLabel: policyv1alpha1.Kind, + "other-label": "other-value", + }, + }, + }, + validate: func(t *testing.T, owners []SoftOwnerRef, err error) { + require.NoError(t, err) + assert.Nil(t, owners) + }, + }, + { + name: "returns nil for non-policy-owned secret", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + "some-other-label": "some-value", + }, + }, + }, + validate: func(t *testing.T, owners []SoftOwnerRef, err error) { + require.NoError(t, err) + assert.Nil(t, owners) + }, + }, + { + name: "returns error for invalid JSON in annotation", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + Annotations: map[string]string{ + SoftOwnerRefsAnnotation: `invalid-json`, + }, + }, + }, + validate: func(t *testing.T, owners []SoftOwnerRef, err error) { + require.Error(t, err) + assert.Nil(t, owners) + }, + }, + { + name: "skips malformed namespaced names in annotation", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + Annotations: map[string]string{ + SoftOwnerRefsAnnotation: `["namespace-1/policy-1","malformed","too/many/slashes"]`, + }, + }, + }, + validate: func(t *testing.T, owners []SoftOwnerRef, err error) { + require.NoError(t, err) + require.Len(t, owners, 1) + assert.Equal(t, SoftOwnerRef{Name: "policy-1", Namespace: "namespace-1", Kind: policyv1alpha1.Kind}, owners[0]) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + owners, err := SoftOwnerRefs(tt.secret) + tt.validate(t, owners, err) + }) + } +} diff --git a/pkg/controller/elasticsearch/filesettings/file_settings.go b/pkg/controller/elasticsearch/filesettings/file_settings.go index 3bdbab5084d..00ef7798e07 100644 --- a/pkg/controller/elasticsearch/filesettings/file_settings.go +++ b/pkg/controller/elasticsearch/filesettings/file_settings.go @@ -81,12 +81,12 @@ func newEmptySettingsState() SettingsState { } // updateState updates the Settings state from a StackConfigPolicy for a given Elasticsearch. -func (s *Settings) updateState(es types.NamespacedName, policy policyv1alpha1.StackConfigPolicy) error { - p := policy.DeepCopy() // be sure to not mutate the original policy +func (s *Settings) updateState(es types.NamespacedName, esConfigPolicy policyv1alpha1.ElasticsearchConfigPolicySpec) error { + esConfigPolicy = *esConfigPolicy.DeepCopy() // be sure to not mutate the original es config policy state := newEmptySettingsState() // mutate Snapshot Repositories - if p.Spec.Elasticsearch.SnapshotRepositories != nil { - for name, untypedDefinition := range p.Spec.Elasticsearch.SnapshotRepositories.Data { + if esConfigPolicy.SnapshotRepositories != nil { + for name, untypedDefinition := range esConfigPolicy.SnapshotRepositories.Data { definition, ok := untypedDefinition.(map[string]interface{}) if !ok { return fmt.Errorf(`invalid type (%T) for definition of snapshot repository %q of Elasticsearch "%s/%s"`, untypedDefinition, name, es.Namespace, es.Name) @@ -95,31 +95,31 @@ func (s *Settings) updateState(es types.NamespacedName, policy policyv1alpha1.St if err != nil { return err } - p.Spec.Elasticsearch.SnapshotRepositories.Data[name] = repoSettings + esConfigPolicy.SnapshotRepositories.Data[name] = repoSettings } - state.SnapshotRepositories = p.Spec.Elasticsearch.SnapshotRepositories + state.SnapshotRepositories = esConfigPolicy.SnapshotRepositories } // just copy other settings - if p.Spec.Elasticsearch.ClusterSettings != nil { - state.ClusterSettings = p.Spec.Elasticsearch.ClusterSettings + if esConfigPolicy.ClusterSettings != nil { + state.ClusterSettings = esConfigPolicy.ClusterSettings } - if p.Spec.Elasticsearch.SnapshotLifecyclePolicies != nil { - state.SLM = p.Spec.Elasticsearch.SnapshotLifecyclePolicies + if esConfigPolicy.SnapshotLifecyclePolicies != nil { + state.SLM = esConfigPolicy.SnapshotLifecyclePolicies } - if p.Spec.Elasticsearch.SecurityRoleMappings != nil { - state.RoleMappings = p.Spec.Elasticsearch.SecurityRoleMappings + if esConfigPolicy.SecurityRoleMappings != nil { + state.RoleMappings = esConfigPolicy.SecurityRoleMappings } - if p.Spec.Elasticsearch.IndexLifecyclePolicies != nil { - state.IndexLifecyclePolicies = p.Spec.Elasticsearch.IndexLifecyclePolicies + if esConfigPolicy.IndexLifecyclePolicies != nil { + state.IndexLifecyclePolicies = esConfigPolicy.IndexLifecyclePolicies } - if p.Spec.Elasticsearch.IngestPipelines != nil { - state.IngestPipelines = p.Spec.Elasticsearch.IngestPipelines + if esConfigPolicy.IngestPipelines != nil { + state.IngestPipelines = esConfigPolicy.IngestPipelines } - if p.Spec.Elasticsearch.IndexTemplates.ComposableIndexTemplates != nil { - state.IndexTemplates.ComposableIndexTemplates = p.Spec.Elasticsearch.IndexTemplates.ComposableIndexTemplates + if esConfigPolicy.IndexTemplates.ComposableIndexTemplates != nil { + state.IndexTemplates.ComposableIndexTemplates = esConfigPolicy.IndexTemplates.ComposableIndexTemplates } - if p.Spec.Elasticsearch.IndexTemplates.ComponentTemplates != nil { - state.IndexTemplates.ComponentTemplates = p.Spec.Elasticsearch.IndexTemplates.ComponentTemplates + if esConfigPolicy.IndexTemplates.ComponentTemplates != nil { + state.IndexTemplates.ComponentTemplates = esConfigPolicy.IndexTemplates.ComponentTemplates } s.State = state return nil diff --git a/pkg/controller/elasticsearch/filesettings/file_settings_test.go b/pkg/controller/elasticsearch/filesettings/file_settings_test.go index 4c0b9a862e9..e62fd0c6333 100644 --- a/pkg/controller/elasticsearch/filesettings/file_settings_test.go +++ b/pkg/controller/elasticsearch/filesettings/file_settings_test.go @@ -436,7 +436,7 @@ func Test_updateState(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { settings := Settings{} - err := settings.updateState(esSample, tt.args.policy) + err := settings.updateState(esSample, tt.args.policy.Spec.Elasticsearch) if tt.wantErr != nil { assert.Equal(t, tt.wantErr, err) return diff --git a/pkg/controller/elasticsearch/filesettings/reconciler.go b/pkg/controller/elasticsearch/filesettings/reconciler.go index 63688838ac8..ade522d43b8 100644 --- a/pkg/controller/elasticsearch/filesettings/reconciler.go +++ b/pkg/controller/elasticsearch/filesettings/reconciler.go @@ -29,7 +29,7 @@ var ( // managedAnnotations are the annotations managed by the operator for the stack config policy related secrets, which means that the operator // will always take precedence to update or remove these annotations. - managedAnnotations = []string{commonannotation.SecureSettingsSecretsAnnotationName, commonannotation.SettingsHashAnnotationName, commonannotation.ElasticsearchConfigAndSecretMountsHashAnnotation, commonannotation.KibanaConfigHashAnnotation} + managedAnnotations = []string{commonannotation.SecureSettingsSecretsAnnotationName, commonannotation.SettingsHashAnnotationName, commonannotation.ElasticsearchConfigAndSecretMountsHashAnnotation, commonannotation.KibanaConfigHashAnnotation, reconciler.SoftOwnerRefsAnnotation} ) // ReconcileEmptyFileSettingsSecret reconciles an empty File settings Secret for the given Elasticsearch only when there is no Secret. @@ -53,7 +53,7 @@ func ReconcileEmptyFileSettingsSecret( // extract the metadata that should be propagated to children meta := metadata.Propagate(&es, metadata.Metadata{Labels: label.NewLabels(k8s.ExtractNamespacedName(&es))}) // no secret, reconcile a new empty file settings - expectedSecret, _, err := NewSettingsSecretWithVersion(k8s.ExtractNamespacedName(&es), nil, nil, meta) + expectedSecret, _, err := NewSettingsSecretWithVersion(k8s.ExtractNamespacedName(&es), nil, nil, nil, meta) if err != nil { return err } diff --git a/pkg/controller/elasticsearch/filesettings/secret.go b/pkg/controller/elasticsearch/filesettings/secret.go index 69e9604d4b7..790f322d61d 100644 --- a/pkg/controller/elasticsearch/filesettings/secret.go +++ b/pkg/controller/elasticsearch/filesettings/secret.go @@ -34,18 +34,18 @@ const ( // The Settings version is updated using the current timestamp only when the Settings have changed. // If the new settings from the policy changed compared to the actual from the secret, the settings version is // updated -func NewSettingsSecretWithVersion(es types.NamespacedName, currentSecret *corev1.Secret, policy *policyv1alpha1.StackConfigPolicy, meta metadata.Metadata) (corev1.Secret, int64, error) { +func NewSettingsSecretWithVersion(es types.NamespacedName, currentSecret *corev1.Secret, esConfigPolicy *policyv1alpha1.ElasticsearchConfigPolicySpec, namespacedSecretSources []commonv1.NamespacedSecretSource, meta metadata.Metadata) (corev1.Secret, int64, error) { newVersion := time.Now().UnixNano() - return newSettingsSecret(newVersion, es, currentSecret, policy, meta) + return newSettingsSecret(newVersion, es, currentSecret, esConfigPolicy, namespacedSecretSources, meta) } // NewSettingsSecret returns a new SettingsSecret for a given Elasticsearch and StackConfigPolicy. -func newSettingsSecret(version int64, es types.NamespacedName, currentSecret *corev1.Secret, policy *policyv1alpha1.StackConfigPolicy, meta metadata.Metadata) (corev1.Secret, int64, error) { +func newSettingsSecret(version int64, es types.NamespacedName, currentSecret *corev1.Secret, esConfigPolicy *policyv1alpha1.ElasticsearchConfigPolicySpec, namespacedSecretSources []commonv1.NamespacedSecretSource, meta metadata.Metadata) (corev1.Secret, int64, error) { settings := NewEmptySettings(version) // update the settings according to the config policy - if policy != nil { - err := settings.updateState(es, *policy) + if esConfigPolicy != nil { + err := settings.updateState(es, *esConfigPolicy) if err != nil { return corev1.Secret{}, 0, err } @@ -84,14 +84,9 @@ func newSettingsSecret(version int64, es types.NamespacedName, currentSecret *co }, } - if policy != nil { - // set this policy as soft owner of this Secret - SetSoftOwner(settingsSecret, *policy) - - // add the Secure Settings Secret sources to the Settings Secret - if err := setSecureSettings(settingsSecret, *policy); err != nil { - return corev1.Secret{}, 0, err - } + // add the Secure Settings Secret sources to the Settings Secret + if err := setSecureSettings(settingsSecret, namespacedSecretSources); err != nil { + return corev1.Secret{}, 0, err } // Add a label to reset secret on deletion of the stack config policy @@ -134,24 +129,11 @@ func SetSoftOwner(settingsSecret *corev1.Secret, policy policyv1alpha1.StackConf } // setSecureSettings stores the SecureSettings Secret sources referenced in the given StackConfigPolicy in the annotation of the Settings Secret. -func setSecureSettings(settingsSecret *corev1.Secret, policy policyv1alpha1.StackConfigPolicy) error { - //nolint:staticcheck - if len(policy.Spec.SecureSettings) == 0 && len(policy.Spec.Elasticsearch.SecureSettings) == 0 { +func setSecureSettings(settingsSecret *corev1.Secret, secretSources []commonv1.NamespacedSecretSource) error { + if len(secretSources) == 0 { return nil } - var secretSources []commonv1.NamespacedSecretSource //nolint:prealloc - // Common secureSettings field, this is mainly there to maintain backwards compatibility - //nolint:staticcheck - for _, src := range policy.Spec.SecureSettings { - secretSources = append(secretSources, commonv1.NamespacedSecretSource{Namespace: policy.GetNamespace(), SecretName: src.SecretName, Entries: src.Entries}) - } - - // SecureSettings field under Elasticsearch in the StackConfigPolicy - for _, src := range policy.Spec.Elasticsearch.SecureSettings { - secretSources = append(secretSources, commonv1.NamespacedSecretSource{Namespace: policy.GetNamespace(), SecretName: src.SecretName, Entries: src.Entries}) - } - bytes, err := json.Marshal(secretSources) if err != nil { return err @@ -160,19 +142,6 @@ func setSecureSettings(settingsSecret *corev1.Secret, policy policyv1alpha1.Stac return nil } -// CanBeOwnedBy return true if the Settings Secret can be owned by the given StackConfigPolicy, either because the Secret -// belongs to no one or because it already belongs to the given policy. -func CanBeOwnedBy(settingsSecret corev1.Secret, policy policyv1alpha1.StackConfigPolicy) (reconciler.SoftOwnerRef, bool) { - currentOwner, referenced := reconciler.SoftOwnerRefFromLabels(settingsSecret.Labels) - // either there is no soft owner - if !referenced { - return reconciler.SoftOwnerRef{}, true - } - // or the owner is already the given policy - canBeOwned := currentOwner.Kind == policyv1alpha1.Kind && currentOwner.Namespace == policy.Namespace && currentOwner.Name == policy.Name - return currentOwner, canBeOwned -} - // getSecureSettings returns the SecureSettings Secret sources stores in an annotation of the given file settings Secret. func getSecureSettings(settingsSecret corev1.Secret) ([]commonv1.NamespacedSecretSource, error) { rawString, ok := settingsSecret.Annotations[commonannotation.SecureSettingsSecretsAnnotationName] diff --git a/pkg/controller/elasticsearch/filesettings/secret_test.go b/pkg/controller/elasticsearch/filesettings/secret_test.go index 8353ee957cc..de1a592b2fd 100644 --- a/pkg/controller/elasticsearch/filesettings/secret_test.go +++ b/pkg/controller/elasticsearch/filesettings/secret_test.go @@ -38,7 +38,7 @@ func Test_NewSettingsSecret(t *testing.T) { // no policy expectedVersion := int64(1) - secret, reconciledVersion, err := newSettingsSecret(expectedVersion, es, nil, nil, metadata.Metadata{}) + secret, reconciledVersion, err := newSettingsSecret(expectedVersion, es, nil, nil, nil, metadata.Metadata{}) assert.NoError(t, err) assert.Equal(t, "esNs", secret.Namespace) assert.Equal(t, "esName-es-file-settings", secret.Name) @@ -47,7 +47,7 @@ func Test_NewSettingsSecret(t *testing.T) { // policy expectedVersion = int64(2) - secret, reconciledVersion, err = newSettingsSecret(expectedVersion, es, &secret, &policy, metadata.Metadata{}) + secret, reconciledVersion, err = newSettingsSecret(expectedVersion, es, &secret, &policy.Spec.Elasticsearch, policy.GetElasticsearchNamespacedSecureSettings(), metadata.Metadata{}) assert.NoError(t, err) assert.Equal(t, "esNs", secret.Namespace) assert.Equal(t, "esName-es-file-settings", secret.Name) @@ -79,14 +79,14 @@ func Test_SettingsSecret_hasChanged(t *testing.T) { expectedEmptySettings := NewEmptySettings(expectedVersion) // no policy -> emptySettings - secret, reconciledVersion, err := newSettingsSecret(expectedVersion, es, nil, nil, metadata.Metadata{}) + secret, reconciledVersion, err := newSettingsSecret(expectedVersion, es, nil, nil, nil, metadata.Metadata{}) assert.NoError(t, err) assert.Equal(t, false, hasChanged(secret, expectedEmptySettings)) assert.Equal(t, expectedVersion, reconciledVersion) // policy without settings -> emptySettings sameSettings := NewEmptySettings(expectedVersion) - err = sameSettings.updateState(es, policy) + err = sameSettings.updateState(es, policy.Spec.Elasticsearch) assert.NoError(t, err) assert.Equal(t, false, hasChanged(secret, sameSettings)) assert.Equal(t, strconv.FormatInt(expectedVersion, 10), sameSettings.Metadata.Version) @@ -95,59 +95,12 @@ func Test_SettingsSecret_hasChanged(t *testing.T) { newVersion := int64(2) newSettings := NewEmptySettings(newVersion) - err = newSettings.updateState(es, otherPolicy) + err = newSettings.updateState(es, otherPolicy.Spec.Elasticsearch) assert.NoError(t, err) assert.Equal(t, true, hasChanged(secret, newSettings)) assert.Equal(t, strconv.FormatInt(newVersion, 10), newSettings.Metadata.Version) } -func Test_SettingsSecret_setSoftOwner_canBeOwnedBy(t *testing.T) { - es := types.NamespacedName{ - Namespace: "esNs", - Name: "esName", - } - policy := policyv1alpha1.StackConfigPolicy{ - TypeMeta: metav1.TypeMeta{ - Kind: policyv1alpha1.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "policyNs", - Name: "policyName", - }, - } - otherPolicy := policyv1alpha1.StackConfigPolicy{ - TypeMeta: metav1.TypeMeta{ - Kind: policyv1alpha1.Kind, - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "otherPolicyNs", - Name: "otherPolicyName", - }, - } - - // empty settings can be owned by any policy - secret, _, err := NewSettingsSecretWithVersion(es, nil, nil, metadata.Metadata{}) - assert.NoError(t, err) - _, canBeOwned := CanBeOwnedBy(secret, policy) - assert.Equal(t, true, canBeOwned) - _, canBeOwned = CanBeOwnedBy(secret, otherPolicy) - assert.Equal(t, true, canBeOwned) - - // set a policy soft owner - SetSoftOwner(&secret, policy) - _, canBeOwned = CanBeOwnedBy(secret, policy) - assert.Equal(t, true, canBeOwned) - _, canBeOwned = CanBeOwnedBy(secret, otherPolicy) - assert.Equal(t, false, canBeOwned) - - // update the policy soft owner - SetSoftOwner(&secret, otherPolicy) - _, canBeOwned = CanBeOwnedBy(secret, policy) - assert.Equal(t, false, canBeOwned) - _, canBeOwned = CanBeOwnedBy(secret, otherPolicy) - assert.Equal(t, true, canBeOwned) -} - func Test_SettingsSecret_setSecureSettings_getSecureSettings(t *testing.T) { es := types.NamespacedName{ Namespace: "esNs", @@ -159,7 +112,9 @@ func Test_SettingsSecret_setSecureSettings_getSecureSettings(t *testing.T) { Name: "policyName", }, Spec: policyv1alpha1.StackConfigPolicySpec{ - SecureSettings: nil, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + SecureSettings: nil, + }, }} otherPolicy := policyv1alpha1.StackConfigPolicy{ ObjectMeta: metav1.ObjectMeta{ @@ -167,23 +122,25 @@ func Test_SettingsSecret_setSecureSettings_getSecureSettings(t *testing.T) { Name: "otherPolicyName", }, Spec: policyv1alpha1.StackConfigPolicySpec{ - SecureSettings: []commonv1.SecretSource{{SecretName: "secure-settings-secret"}}, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + SecureSettings: []commonv1.SecretSource{{SecretName: "secure-settings-secret"}}, + }, }} - secret, _, err := NewSettingsSecretWithVersion(es, nil, nil, metadata.Metadata{}) + secret, _, err := NewSettingsSecretWithVersion(es, nil, nil, nil, metadata.Metadata{}) assert.NoError(t, err) secureSettings, err := getSecureSettings(secret) assert.NoError(t, err) assert.Equal(t, []commonv1.NamespacedSecretSource{}, secureSettings) - err = setSecureSettings(&secret, policy) + err = setSecureSettings(&secret, policy.GetElasticsearchNamespacedSecureSettings()) assert.NoError(t, err) secureSettings, err = getSecureSettings(secret) assert.NoError(t, err) assert.Equal(t, []commonv1.NamespacedSecretSource{}, secureSettings) - err = setSecureSettings(&secret, otherPolicy) + err = setSecureSettings(&secret, otherPolicy.GetElasticsearchNamespacedSecureSettings()) assert.NoError(t, err) secureSettings, err = getSecureSettings(secret) assert.NoError(t, err) diff --git a/pkg/controller/stackconfigpolicy/controller.go b/pkg/controller/stackconfigpolicy/controller.go index b2cefc8ea52..c78ef27f35b 100644 --- a/pkg/controller/stackconfigpolicy/controller.go +++ b/pkg/controller/stackconfigpolicy/controller.go @@ -6,6 +6,7 @@ package stackconfigpolicy import ( "context" + "errors" "fmt" "reflect" "regexp" @@ -110,13 +111,24 @@ func addWatches(mgr manager.Manager, c controller.Controller, r *ReconcileStackC func reconcileRequestForSoftOwnerPolicy() handler.TypedEventHandler[*corev1.Secret, reconcile.Request] { return handler.TypedEnqueueRequestsFromMapFunc[*corev1.Secret](func(ctx context.Context, secret *corev1.Secret) []reconcile.Request { - softOwner, referenced := reconciler.SoftOwnerRefFromLabels(secret.GetLabels()) - if !referenced || softOwner.Kind != policyv1alpha1.Kind { + softOwners, err := reconciler.SoftOwnerRefs(secret) + if err != nil { + ulog.Log.Error(err, "Fail to get soft-owner policies of secret", "secret_name", secret.GetName(), "secret_namespace", secret.GetNamespace()) return nil } - return []reconcile.Request{ - {NamespacedName: types.NamespacedName{Namespace: softOwner.Namespace, Name: softOwner.Name}}, + + if len(softOwners) == 0 { + return nil + } + + requests := make([]reconcile.Request, len(softOwners)) + for idx, nsn := range softOwners { + if nsn.Kind != policyv1alpha1.Kind { + continue + } + requests[idx] = reconcile.Request{NamespacedName: types.NamespacedName{Namespace: nsn.Namespace, Name: nsn.Name}} } + return requests }) } @@ -124,8 +136,7 @@ func reconcileRequestForSoftOwnerPolicy() handler.TypedEventHandler[*corev1.Secr func reconcileRequestForAllPolicies(clnt k8s.Client) handler.TypedEventHandler[client.Object, reconcile.Request] { return handler.TypedEnqueueRequestsFromMapFunc[client.Object](func(ctx context.Context, es client.Object) []reconcile.Request { var stackConfigList policyv1alpha1.StackConfigPolicyList - err := clnt.List(context.Background(), &stackConfigList) - if err != nil { + if err := clnt.List(context.Background(), &stackConfigList); err != nil { ulog.Log.Error(err, "Fail to list StackConfigurationList while watching Elasticsearch") return nil } @@ -161,8 +172,7 @@ func (r *ReconcileStackConfigPolicy) Reconcile(ctx context.Context, request reco // retrieve the StackConfigPolicy resource var policy policyv1alpha1.StackConfigPolicy - err := r.Client.Get(ctx, request.NamespacedName, &policy) - if err != nil { + if err := r.Client.Get(ctx, request.NamespacedName, &policy); err != nil { if apierrors.IsNotFound(err) { return reconcile.Result{}, r.onDelete(ctx, types.NamespacedName{ @@ -206,12 +216,12 @@ type esMap map[types.NamespacedName]esv1.Elasticsearch // instances configured by a StackConfigPolicy. type kbMap map[types.NamespacedName]kibanav1.Kibana -func (r *ReconcileStackConfigPolicy) doReconcile(ctx context.Context, policy policyv1alpha1.StackConfigPolicy) (*reconciler.Results, policyv1alpha1.StackConfigPolicyStatus) { +func (r *ReconcileStackConfigPolicy) doReconcile(ctx context.Context, reconcilingPolicy policyv1alpha1.StackConfigPolicy) (*reconciler.Results, policyv1alpha1.StackConfigPolicyStatus) { log := ulog.FromContext(ctx) log.V(1).Info("Reconcile StackConfigPolicy") results := reconciler.NewResult(ctx) - status := policyv1alpha1.NewStatus(policy) + status := policyv1alpha1.NewStatus(reconcilingPolicy) defer status.Update() // Enterprise license check @@ -222,22 +232,27 @@ func (r *ReconcileStackConfigPolicy) doReconcile(ctx context.Context, policy pol if !enabled { msg := "StackConfigPolicy is an enterprise feature. Enterprise features are disabled" log.Info(msg) - r.recorder.Eventf(&policy, corev1.EventTypeWarning, events.EventReconciliationError, msg) + r.recorder.Eventf(&reconcilingPolicy, corev1.EventTypeWarning, events.EventReconciliationError, msg) // we don't have a good way of watching for the license level to change so just requeue with a reasonably long delay return results.WithRequeue(5 * time.Minute), status } // run validation in case the webhook is disabled - if err := r.validate(ctx, &policy); err != nil { + if err := r.validate(ctx, &reconcilingPolicy); err != nil { status.Phase = policyv1alpha1.InvalidPhase - r.recorder.Eventf(&policy, corev1.EventTypeWarning, events.EventReasonValidation, err.Error()) + r.recorder.Eventf(&reconcilingPolicy, corev1.EventTypeWarning, events.EventReasonValidation, err.Error()) + return results.WithError(err), status + } + + var policyList policyv1alpha1.StackConfigPolicyList + if err := r.Client.List(ctx, &policyList); err != nil { return results.WithError(err), status } // reconcile elasticsearch resources - results, status = r.reconcileElasticsearchResources(ctx, policy, status) + results, status = r.reconcileElasticsearchResources(ctx, reconcilingPolicy, status, policyList.Items) // reconcile kibana resources - kibanaResults, status := r.reconcileKibanaResources(ctx, policy, status) + kibanaResults, status := r.reconcileKibanaResources(ctx, reconcilingPolicy, status, policyList.Items) // Combine results from kibana reconciliation with results from Elasticsearch reconciliation results.WithResults(kibanaResults) @@ -250,7 +265,7 @@ func (r *ReconcileStackConfigPolicy) doReconcile(ctx context.Context, policy pol return results, status } -func (r *ReconcileStackConfigPolicy) reconcileElasticsearchResources(ctx context.Context, policy policyv1alpha1.StackConfigPolicy, status policyv1alpha1.StackConfigPolicyStatus) (*reconciler.Results, policyv1alpha1.StackConfigPolicyStatus) { +func (r *ReconcileStackConfigPolicy) reconcileElasticsearchResources(ctx context.Context, reconcilingPolicy policyv1alpha1.StackConfigPolicy, status policyv1alpha1.StackConfigPolicyStatus, allPolicies []policyv1alpha1.StackConfigPolicy) (*reconciler.Results, policyv1alpha1.StackConfigPolicyStatus) { defer tracing.Span(&ctx)() log := ulog.FromContext(ctx) log.V(1).Info("Reconcile Elasticsearch resources") @@ -259,17 +274,17 @@ func (r *ReconcileStackConfigPolicy) reconcileElasticsearchResources(ctx context // prepare the selector to find Elastic resources to configure selector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{ - MatchLabels: policy.Spec.ResourceSelector.MatchLabels, - MatchExpressions: policy.Spec.ResourceSelector.MatchExpressions, + MatchLabels: reconcilingPolicy.Spec.ResourceSelector.MatchLabels, + MatchExpressions: reconcilingPolicy.Spec.ResourceSelector.MatchExpressions, }) if err != nil { return results.WithError(err), status } listOpts := client.ListOptions{LabelSelector: selector} - // restrict the search to the policy namespace if it is different from the operator namespace - if policy.Namespace != r.params.OperatorNamespace { - listOpts.Namespace = policy.Namespace + // restrict the search to the reconcilingPolicy namespace if it is different from the operator namespace + if reconcilingPolicy.Namespace != r.params.OperatorNamespace { + listOpts.Namespace = reconcilingPolicy.Namespace } // find the list of Elasticsearch to configure @@ -278,6 +293,7 @@ func (r *ReconcileStackConfigPolicy) reconcileElasticsearchResources(ctx context return results.WithError(err), status } + reconcilingPolicyNsn := k8s.ExtractNamespacedName(&reconcilingPolicy) configuredResources := esMap{} for _, es := range esList.Items { log.V(1).Info("Reconcile StackConfigPolicy", "es_namespace", es.Namespace, "es_name", es.Name) @@ -294,7 +310,7 @@ func (r *ReconcileStackConfigPolicy) reconcileElasticsearchResources(ctx context } if v.LT(filesettings.FileBasedSettingsMinPreVersion) { err = fmt.Errorf("invalid version to configure resource Elasticsearch %s/%s: actual %s, expected >= %s", es.Namespace, es.Name, v, filesettings.FileBasedSettingsMinVersion) - r.recorder.Eventf(&policy, corev1.EventTypeWarning, events.EventReasonUnexpected, err.Error()) + r.recorder.Eventf(&reconcilingPolicy, corev1.EventTypeWarning, events.EventReasonUnexpected, err.Error()) results.WithError(err) err = status.AddPolicyErrorFor(esNsn, policyv1alpha1.ErrorPhase, err.Error(), policyv1alpha1.ElasticsearchResourceType) if err != nil { @@ -314,33 +330,41 @@ func (r *ReconcileStackConfigPolicy) reconcileElasticsearchResources(ctx context return results.WithError(err), status } - // check that there is no other policy that already owns the Settings Secret - currentOwner, ok := filesettings.CanBeOwnedBy(actualSettingsSecret, policy) - if !ok { - err = fmt.Errorf("conflict: resource Elasticsearch %s/%s already configured by StackConfigpolicy %s/%s", es.Namespace, es.Name, currentOwner.Namespace, currentOwner.Name) - r.recorder.Eventf(&policy, corev1.EventTypeWarning, events.EventReasonUnexpected, err.Error()) - results.WithError(err) - err = status.AddPolicyErrorFor(esNsn, policyv1alpha1.ConflictPhase, err.Error(), policyv1alpha1.ElasticsearchResourceType) - if err != nil { + // build the final config by merging all policies that target the given Elasticsearch cluster + esConfigPolicyFinal, err := getConfigPolicyForElasticsearch(&es, allPolicies, r.params) + switch { + case errors.Is(err, errMergeConflict): + log.Info("StackConfigPolicy merge conflict for Elasticsearch", "es_namespace", es.Namespace, "es_name", es.Name, "error", err) + if err = status.AddPolicyErrorFor(esNsn, policyv1alpha1.ConflictPhase, err.Error(), policyv1alpha1.ElasticsearchResourceType); err != nil { return results.WithError(err), status } continue + case err != nil: + return results.WithError(err), status } // extract the metadata that should be propagated to children meta := metadata.Propagate(&es, metadata.Metadata{Labels: eslabel.NewLabels(k8s.ExtractNamespacedName(&es))}) // create the expected Settings Secret - expectedSecret, expectedVersion, err := filesettings.NewSettingsSecretWithVersion(esNsn, &actualSettingsSecret, &policy, meta) + expectedSecret, expectedVersion, err := filesettings.NewSettingsSecretWithVersion(esNsn, &actualSettingsSecret, &esConfigPolicyFinal.Spec, esConfigPolicyFinal.SecretSources, meta) if err != nil { return results.WithError(err), status } + // We must keep track of the soft owner references of the file-settings secret to ensure that the secret is reconciled + // back to an empty one when no policies are targeting it (look at resetOrphanSoftOwnedFileSettingSecrets) + if err := setMultipleSoftOwners(&expectedSecret, esConfigPolicyFinal.PolicyRefs); err != nil { + return results.WithError(err), status + } + if err := filesettings.ReconcileSecret(ctx, r.Client, expectedSecret, &es); err != nil { return results.WithError(err), status } - // Copy all the Secrets that are present in spec.elasticsearch.secretMounts - if err := reconcileSecretMounts(ctx, r.Client, es, &policy, meta); err != nil { + // Copy all the Secrets that are present in the reconciling policy. This is required to ensure that the + // ES cluster pods can mount the secrets that may be referenced in the Elasticsearch configuration after + // merging all policies. + if err := reconcileSecretMounts(ctx, r.Client, es, &reconcilingPolicy, meta); err != nil { if apierrors.IsNotFound(err) { err = status.AddPolicyErrorFor(esNsn, policyv1alpha1.ErrorPhase, err.Error(), policyv1alpha1.ElasticsearchResourceType) if err != nil { @@ -352,7 +376,11 @@ func (r *ReconcileStackConfigPolicy) reconcileElasticsearchResources(ctx context } // create expected elasticsearch config secret - expectedConfigSecret, err := newElasticsearchConfigSecret(policy, es) + expectedConfigSecret, err := newElasticsearchConfigSecret(esConfigPolicyFinal.Spec, es) + if err != nil { + return results.WithError(err), status + } + err = setMultipleSoftOwners(&expectedConfigSecret, esConfigPolicyFinal.PolicyRefs) if err != nil { return results.WithError(err), status } @@ -362,7 +390,7 @@ func (r *ReconcileStackConfigPolicy) reconcileElasticsearchResources(ctx context } // Check if required Elasticsearch config and secret mounts are applied. - configAndSecretMountsApplied, err := elasticsearchConfigAndSecretMountsApplied(ctx, r.Client, policy, es) + configAndSecretMountsApplied, err := elasticsearchConfigAndSecretMountsApplied(ctx, r.Client, esConfigPolicyFinal.Spec, es) if err != nil { return results.WithError(err), status } @@ -386,18 +414,18 @@ func (r *ReconcileStackConfigPolicy) reconcileElasticsearchResources(ctx context } // Add dynamic watches on the additional secret mounts - // This will also remove dynamic watches for secrets that no longer are refrenced in the stackconfigpolicy - if err = r.addDynamicWatchesOnAdditionalSecretMounts(policy); err != nil { + // This will also remove dynamic watches for secrets that no longer are referenced in the stackconfigpolicy + if err = r.addDynamicWatchesOnAdditionalSecretMounts(reconcilingPolicy); err != nil { return results.WithError(err), status } - // reset/delete Settings secrets for resources no longer selected by this policy - results.WithError(handleOrphanSoftOwnedSecrets(ctx, r.Client, k8s.ExtractNamespacedName(&policy), configuredResources, nil, policyv1alpha1.ElasticsearchResourceType)) + // reset/delete Settings secrets for resources no longer selected by this reconcilingPolicy + results.WithError(handleOrphanSoftOwnedSecrets(ctx, r.Client, reconcilingPolicyNsn, configuredResources, nil, policyv1alpha1.ElasticsearchResourceType)) return results, status } -func (r *ReconcileStackConfigPolicy) reconcileKibanaResources(ctx context.Context, policy policyv1alpha1.StackConfigPolicy, status policyv1alpha1.StackConfigPolicyStatus) (*reconciler.Results, policyv1alpha1.StackConfigPolicyStatus) { +func (r *ReconcileStackConfigPolicy) reconcileKibanaResources(ctx context.Context, reconcilingPolicy policyv1alpha1.StackConfigPolicy, status policyv1alpha1.StackConfigPolicyStatus, allPolicies []policyv1alpha1.StackConfigPolicy) (*reconciler.Results, policyv1alpha1.StackConfigPolicyStatus) { defer tracing.Span(&ctx)() log := ulog.FromContext(ctx) log.V(1).Info("Reconcile Kibana Resources") @@ -406,17 +434,17 @@ func (r *ReconcileStackConfigPolicy) reconcileKibanaResources(ctx context.Contex // prepare the selector to find Kibana resources to configure selector, err := metav1.LabelSelectorAsSelector(&metav1.LabelSelector{ - MatchLabels: policy.Spec.ResourceSelector.MatchLabels, - MatchExpressions: policy.Spec.ResourceSelector.MatchExpressions, + MatchLabels: reconcilingPolicy.Spec.ResourceSelector.MatchLabels, + MatchExpressions: reconcilingPolicy.Spec.ResourceSelector.MatchExpressions, }) if err != nil { return results.WithError(err), status } listOpts := client.ListOptions{LabelSelector: selector} - // restrict the search to the policy namespace if it is different from the operator namespace - if policy.Namespace != r.params.OperatorNamespace { - listOpts.Namespace = policy.Namespace + // restrict the search to the reconcilingPolicy namespace if it is different from the operator namespace + if reconcilingPolicy.Namespace != r.params.OperatorNamespace { + listOpts.Namespace = reconcilingPolicy.Namespace } // find the list of Kibana to configure @@ -433,29 +461,25 @@ func (r *ReconcileStackConfigPolicy) reconcileKibanaResources(ctx context.Contex // keep the list of Kibana to be configured kibanaNsn := k8s.ExtractNamespacedName(&kibana) - // check that there is no other policy that already owns the kibana config secret - currentOwner, ok, err := canBeOwned(ctx, r.Client, policy, kibana) - if err != nil { - return results.WithError(err), status - } - - // record error if already owned by another stack config policy - if !ok { - err := fmt.Errorf("conflict: resource Kibana %s/%s already configured by StackConfigpolicy %s/%s", kibana.Namespace, kibana.Name, currentOwner.Namespace, currentOwner.Name) - r.recorder.Eventf(&policy, corev1.EventTypeWarning, events.EventReasonUnexpected, err.Error()) - results.WithError(err) - if err := status.AddPolicyErrorFor(kibanaNsn, policyv1alpha1.ConflictPhase, err.Error(), policyv1alpha1.KibanaResourceType); err != nil { + // build the final config by merging all policies that target the given Kibana instance + kbnPolicyConfigFinal, err := getConfigPolicyForKibana(&kibana, allPolicies, r.params) + switch { + case errors.Is(err, errMergeConflict): + log.Info("StackConfigPolicy merge conflict for Kibana", "kibana_namespace", kibana.Namespace, "kibana_name", kibana.Name, "error", err) + if err = status.AddPolicyErrorFor(kibanaNsn, policyv1alpha1.ConflictPhase, err.Error(), policyv1alpha1.KibanaResourceType); err != nil { return results.WithError(err), status } continue + case err != nil: + return results.WithError(err), status } // Create the Secret that holds the Kibana configuration. - if policy.Spec.Kibana.Config != nil { + if kbnPolicyConfigFinal.Spec.Config != nil { // Only add to configured resources if Kibana config is set. - // This will help clean up the config secret if config gets removed from the stack config policy. + // This will help clean up the config secret if config gets removed from the stack config reconcilingPolicy. configuredResources[kibanaNsn] = kibana - expectedConfigSecret, err := newKibanaConfigSecret(policy, kibana) + expectedConfigSecret, err := newKibanaConfigSecret(kbnPolicyConfigFinal.Spec, kbnPolicyConfigFinal.SecretSources, kibana, kbnPolicyConfigFinal.PolicyRefs) if err != nil { return results.WithError(err), status } @@ -466,7 +490,7 @@ func (r *ReconcileStackConfigPolicy) reconcileKibanaResources(ctx context.Contex } // Check if required Kibana configs are applied. - configApplied, err := kibanaConfigApplied(r.Client, policy, kibana) + configApplied, err := kibanaConfigApplied(r.Client, kbnPolicyConfigFinal.Spec, kibana) if err != nil { return results.WithError(err), status } @@ -478,8 +502,8 @@ func (r *ReconcileStackConfigPolicy) reconcileKibanaResources(ctx context.Contex } } - // delete Settings secrets for resources no longer selected by this policy - results.WithError(deleteOrphanSoftOwnedSecrets(ctx, r.Client, k8s.ExtractNamespacedName(&policy), nil, configuredResources, policyv1alpha1.KibanaResourceType)) + // delete Settings secrets for resources no longer selected by this reconcilingPolicy + results.WithError(deleteOrphanSoftOwnedSecrets(ctx, r.Client, k8s.ExtractNamespacedName(&reconcilingPolicy), nil, configuredResources, policyv1alpha1.KibanaResourceType)) return results, status } @@ -564,8 +588,7 @@ func handleOrphanSoftOwnedSecrets( configuredKibanaResources kbMap, resourceType policyv1alpha1.ResourceType, ) error { - err := resetOrphanSoftOwnedFileSettingSecrets(ctx, c, softOwner, configuredESResources, resourceType) - if err != nil { + if err := resetOrphanSoftOwnedFileSettingSecrets(ctx, c, softOwner, configuredESResources, resourceType); err != nil { return err } return deleteOrphanSoftOwnedSecrets(ctx, c, softOwner, configuredESResources, configuredKibanaResources, resourceType) @@ -585,8 +608,6 @@ func resetOrphanSoftOwnedFileSettingSecrets( log := ulog.FromContext(ctx) var secrets corev1.SecretList matchLabels := client.MatchingLabels{ - reconciler.SoftOwnerNamespaceLabel: softOwner.Namespace, - reconciler.SoftOwnerNameLabel: softOwner.Name, reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, commonlabels.StackConfigPolicyOnDeleteLabelName: commonlabels.OrphanSecretResetOnPolicyDelete, } @@ -605,6 +626,13 @@ func resetOrphanSoftOwnedFileSettingSecrets( } for i := range secrets.Items { s := secrets.Items[i] + owned, err := isPolicySoftOwner(&s, softOwner) + if err != nil { + return err + } + if !owned { + continue + } configuredApplicationType := s.Labels[commonv1.TypeLabelName] switch configuredApplicationType { case eslabel.Type: @@ -616,13 +644,8 @@ func resetOrphanSoftOwnedFileSettingSecrets( continue } - log.V(1).Info("Reconcile empty file settings Secret for Elasticsearch", - "es_namespace", namespacedName.Namespace, "es_name", namespacedName.Name, - "owner_namespace", softOwner.Namespace, "owner_name", softOwner.Name) - var es esv1.Elasticsearch - err := c.Get(ctx, namespacedName, &es) - if err != nil && !apierrors.IsNotFound(err) { + if err := c.Get(ctx, namespacedName, &es); err != nil && !apierrors.IsNotFound(err) { return err } if apierrors.IsNotFound(err) { @@ -630,9 +653,24 @@ func resetOrphanSoftOwnedFileSettingSecrets( return nil } - if err := filesettings.ReconcileEmptyFileSettingsSecret(ctx, c, es, false); err != nil { + remainingOwners, err := removePolicySoftOwner(&s, softOwner) + if err != nil { return err } + + if remainingOwners == 0 { + log.V(1).Info("Reconcile empty file settings Secret for Elasticsearch", + "es_namespace", namespacedName.Namespace, "es_name", namespacedName.Name, + "owner_namespace", softOwner.Namespace, "owner_name", softOwner.Name) + + if err := filesettings.ReconcileEmptyFileSettingsSecret(ctx, c, es, false); err != nil && !apierrors.IsNotFound(err) { + return err + } + } else { + if err := filesettings.ReconcileSecret(ctx, c, s, &es); err != nil && !apierrors.IsNotFound(err) { + return err + } + } case kblabel.Type: // Currently we do not reset labels for kibana, so we shouldn't hit this. // Implement if needed in the future @@ -656,8 +694,6 @@ func deleteOrphanSoftOwnedSecrets( ) error { var secrets corev1.SecretList matchLabels := client.MatchingLabels{ - reconciler.SoftOwnerNamespaceLabel: softOwner.Namespace, - reconciler.SoftOwnerNameLabel: softOwner.Name, reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, commonlabels.StackConfigPolicyOnDeleteLabelName: commonlabels.OrphanSecretDeleteOnPolicyDelete, } @@ -676,8 +712,16 @@ func deleteOrphanSoftOwnedSecrets( for i := range secrets.Items { secret := secrets.Items[i] - configuredApplicationType := secret.Labels[commonv1.TypeLabelName] + owned, err := isPolicySoftOwner(&secret, softOwner) + if err != nil { + return err + } + if !owned { + continue + } + var ownerObject client.Object + configuredApplicationType := secret.Labels[commonv1.TypeLabelName] switch configuredApplicationType { case eslabel.Type: namespacedName := types.NamespacedName{ @@ -688,6 +732,25 @@ func deleteOrphanSoftOwnedSecrets( if _, exist := configuredESResources[namespacedName]; exist { continue } + + if len(secret.OwnerReferences) == 0 { + break + } + + var es esv1.Elasticsearch + err := c.Get(ctx, types.NamespacedName{ + Namespace: secret.Namespace, + Name: secret.Labels[eslabel.ClusterNameLabelName], + }, &es) + switch { + case err == nil: + ownerObject = &es + case apierrors.IsNotFound(err): + break + default: + return err + } + case kblabel.Type: namespacedName := types.NamespacedName{ Namespace: secret.Namespace, @@ -697,15 +760,44 @@ func deleteOrphanSoftOwnedSecrets( if _, exist := configuredKibanaResources[namespacedName]; exist { continue } + + if len(secret.OwnerReferences) == 0 { + break + } + + var kbn kibanav1.Kibana + err := c.Get(ctx, types.NamespacedName{ + Namespace: secret.Namespace, + Name: secret.Labels[kblabel.KibanaNameLabelName], + }, &kbn) + switch { + case err == nil: + ownerObject = &kbn + case apierrors.IsNotFound(err): + break + default: + return err + } default: return fmt.Errorf("secret configured for unknown application type %s", configuredApplicationType) } - // given kibana/elasticsearch cluster is no longer managed by stack config policy, delete secret. - err := c.Delete(ctx, &secret) - if err != nil && !apierrors.IsNotFound(err) { + remainingOwners, err := removePolicySoftOwner(&secret, softOwner) + if err != nil { return err } + + if remainingOwners == 0 { + // given kibana/elasticsearch cluster is no longer managed by stack config policy, delete secret. + err = c.Delete(ctx, &secret) + if err != nil && !apierrors.IsNotFound(err) { + return err + } + } else { + if err := filesettings.ReconcileSecret(ctx, c, secret, ownerObject); err != nil && !apierrors.IsNotFound(err) { + return err + } + } } return nil diff --git a/pkg/controller/stackconfigpolicy/controller_test.go b/pkg/controller/stackconfigpolicy/controller_test.go index 851c861bed7..423e678bd46 100644 --- a/pkg/controller/stackconfigpolicy/controller_test.go +++ b/pkg/controller/stackconfigpolicy/controller_test.go @@ -29,11 +29,13 @@ import ( "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/hash" commonlabels "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/labels" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/license" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/operator" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/reconciler" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/watches" esclient "github.com/elastic/cloud-on-k8s/v3/pkg/controller/elasticsearch/client" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/elasticsearch/filesettings" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/elasticsearch/label" + eslabel "github.com/elastic/cloud-on-k8s/v3/pkg/controller/elasticsearch/label" "github.com/elastic/cloud-on-k8s/v3/pkg/utils/k8s" "github.com/elastic/cloud-on-k8s/v3/pkg/utils/net" ) @@ -363,42 +365,6 @@ func TestReconcileStackConfigPolicy_Reconcile(t *testing.T) { wantErr: false, wantRequeueAfter: true, }, - { - name: "Reconcile Kibana already owned by another policy", - args: args{ - client: k8s.NewFakeClient(&policyFixture, &kibanaFixture, MkKibanaConfigSecret("ns", "another-policy", "ns", "testvalue")), - licenseChecker: &license.MockLicenseChecker{EnterpriseEnabled: true}, - }, - post: func(r ReconcileStackConfigPolicy, recorder record.FakeRecorder) { - events := fetchEvents(&recorder) - assert.ElementsMatch(t, []string{"Warning Unexpected conflict: resource Kibana ns/test-kb already configured by StackConfigpolicy ns/another-policy"}, events) - - policy := r.getPolicy(t, k8s.ExtractNamespacedName(&policyFixture)) - assert.Equal(t, 1, policy.Status.Resources) - assert.Equal(t, 0, policy.Status.Ready) - assert.Equal(t, policyv1alpha1.ConflictPhase, policy.Status.Phase) - }, - wantErr: true, - wantRequeueAfter: true, - }, - { - name: "Reconcile Elasticsearch already owned by another policy", - args: args{ - client: k8s.NewFakeClient(&policyFixture, &esFixture, conflictingSecretFixture), - licenseChecker: &license.MockLicenseChecker{EnterpriseEnabled: true}, - }, - post: func(r ReconcileStackConfigPolicy, recorder record.FakeRecorder) { - events := fetchEvents(&recorder) - assert.ElementsMatch(t, []string{"Warning Unexpected conflict: resource Elasticsearch ns/test-es already configured by StackConfigpolicy ns/another-policy"}, events) - - policy := r.getPolicy(t, k8s.ExtractNamespacedName(&policyFixture)) - assert.Equal(t, 1, policy.Status.Resources) - assert.Equal(t, 0, policy.Status.Ready) - assert.Equal(t, policyv1alpha1.ConflictPhase, policy.Status.Phase) - }, - wantErr: true, - wantRequeueAfter: true, - }, { name: "Elasticsearch cluster in old version without support for file based settings", args: args{ @@ -712,6 +678,280 @@ func TestReconcileStackConfigPolicy_Reconcile(t *testing.T) { } } +//nolint:thelper +func TestReconcileStackConfigPolicy_MultipleStackConfigPolicies(t *testing.T) { + // Setup: Create an Elasticsearch cluster and multiple StackConfigPolicies with different weights + esFixture := esv1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "test-es", + Labels: map[string]string{"env": "prod"}, + }, + Spec: esv1.ElasticsearchSpec{Version: "8.6.1"}, + } + + // Policy with weight 10 (applied first) + policy1 := policyv1alpha1.StackConfigPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "policy-low", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + Weight: 10, + ResourceSelector: metav1.LabelSelector{MatchLabels: map[string]string{"env": "prod"}}, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]interface{}{ + "indices.recovery.max_bytes_per_sec": "40mb", + }}, + Config: &commonv1.Config{Data: map[string]interface{}{ + "logger.org.elasticsearch.discovery": "INFO", + }}, + }, + }, + } + + // Policy with weight 20 (applied second, overrides policy1) + policy2 := policyv1alpha1.StackConfigPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "policy-high", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + Weight: 20, + ResourceSelector: metav1.LabelSelector{MatchLabels: map[string]string{"env": "prod"}}, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]interface{}{ + "indices.recovery.max_bytes_per_sec": "50mb", // Overrides policy1 + }}, + Config: &commonv1.Config{Data: map[string]interface{}{ + "logger.org.elasticsearch.gateway": "DEBUG", // Additional setting + }}, + SecretMounts: []policyv1alpha1.SecretMount{ + { + SecretName: "test-secret", + MountPath: "/usr/test", + }, + }, + }, + }, + } + + // Policy with same weight as policy2 (should cause conflict) + policy3Conflicting := policyv1alpha1.StackConfigPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "policy-conflict", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + Weight: 20, // Same weight as policy2 + ResourceSelector: metav1.LabelSelector{MatchLabels: map[string]string{"env": "prod"}}, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]interface{}{ + "indices.recovery.max_bytes_per_sec": "60mb", + }}, + }, + }, + } + + // Initial empty file settings secret (will be populated by controller) + esFileSettingsSecret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "test-es-es-file-settings", + Labels: map[string]string{ + commonv1.TypeLabelName: "elasticsearch", + eslabel.ClusterNameLabelName: "test-es", + commonlabels.StackConfigPolicyOnDeleteLabelName: commonlabels.OrphanSecretResetOnPolicyDelete, + }, + }, + Data: map[string][]byte{"settings.json": []byte(`{"metadata":{"version":"1","compatibility":"8.4.0"},"state":{"cluster_settings":{},"snapshot_repositories":{},"slm":{},"role_mappings":{},"autoscaling":{},"ilm":{},"ingest_pipelines":{},"index_templates":{"component_templates":{},"composable_index_templates":{}}}}`)}, + } + + esPod := getEsPod("ns", map[string]string{}) + + // Source secret that will be mounted (exists in policy namespace) + sourceSecret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "ns", + }, + Data: map[string][]byte{ + "key1": []byte("value1"), + }, + } + + tests := []struct { + name string + policies []policyv1alpha1.StackConfigPolicy + reconcilePolicy string // Which policy to reconcile + wantResources int + wantReady int + wantPhase policyv1alpha1.PolicyPhase + wantErr bool + wantRequeueAfter bool + validateSettings func(t *testing.T, r ReconcileStackConfigPolicy) + }{ + { + name: "Multiple policies with different weights merge successfully", + policies: []policyv1alpha1.StackConfigPolicy{policy1, policy2}, + reconcilePolicy: "policy-low", + wantResources: 1, + wantReady: 0, + wantPhase: policyv1alpha1.ApplyingChangesPhase, + wantErr: false, + wantRequeueAfter: true, + validateSettings: func(t *testing.T, r ReconcileStackConfigPolicy) { + // Verify the file settings secret was updated + esNsn := k8s.ExtractNamespacedName(&esFixture) + + settingsSecretNsn := types.NamespacedName{ + Namespace: esNsn.Namespace, + Name: esv1.FileSettingsSecretName(esNsn.Name), + } + + var secret corev1.Secret + err := r.Client.Get(context.Background(), settingsSecretNsn, &secret) + require.NoError(t, err) + + // Verify the file settings secret has merged config + settings := r.getSettings(t, settingsSecretNsn) + + // Check if ClusterSettings was populated + if settings.State.ClusterSettings == nil { + t.Logf("Secret data: %s", string(secret.Data["settings.json"])) + t.Fatal("ClusterSettings should not be nil after reconciliation") + } + require.NotNil(t, settings.State.ClusterSettings.Data, "ClusterSettings.Data should not be nil") + + // Should have the value from policy2 (higher weight) + assert.EqualValues(t, map[string]any{ + "indices": map[string]any{ + "recovery": map[string]any{ + "max_bytes_per_sec": "40mb", + }, + }, + }, settings.State.ClusterSettings.Data, "ClusterSettings.Data") + + owners, err := reconciler.SoftOwnerRefs(&secret) + assert.NoError(t, err) + assert.Len(t, owners, 2, "esConfigSecret should be owned by 2 policies") + // Verify both policies are in the owner list + assert.Contains(t, owners, reconciler.SoftOwnerRef{Namespace: "ns", Name: "policy-low", Kind: policyv1alpha1.Kind}, "policy-low should be an owner of esConfigSecret") + assert.Contains(t, owners, reconciler.SoftOwnerRef{Namespace: "ns", Name: "policy-high", Kind: policyv1alpha1.Kind}, "policy-high should be an owner of esConfigSecret") + }, + }, + { + name: "Policies with same weight cause conflict", + policies: []policyv1alpha1.StackConfigPolicy{policy2, policy3Conflicting}, + reconcilePolicy: "policy-high", + wantResources: 1, + wantReady: 0, + wantPhase: policyv1alpha1.ConflictPhase, + wantErr: false, + wantRequeueAfter: true, + validateSettings: func(t *testing.T, r ReconcileStackConfigPolicy) { + // Verify policy status shows conflict + policy := r.getPolicy(t, types.NamespacedName{Namespace: "ns", Name: "policy-high"}) + + esStatus := policy.Status.Details["elasticsearch"]["ns/test-es"] + assert.Equal(t, policyv1alpha1.ConflictPhase, esStatus.Phase) + }, + }, + { + name: "Reconciling second policy sees merged state", + policies: []policyv1alpha1.StackConfigPolicy{policy1, policy2}, + reconcilePolicy: "policy-high", + wantResources: 1, + wantReady: 0, + wantPhase: policyv1alpha1.ApplyingChangesPhase, + wantErr: false, + wantRequeueAfter: true, + validateSettings: func(t *testing.T, r ReconcileStackConfigPolicy) { + // Verify elasticsearch config secret exists with merged config + var esConfigSecret corev1.Secret + err := r.Client.Get(context.Background(), types.NamespacedName{ + Namespace: "ns", + Name: esv1.StackConfigElasticsearchConfigSecretName("test-es"), + }, &esConfigSecret) + assert.NoError(t, err) + + // Verify elasticsearch config secret is soft-owned by multiple policies + owners, err := reconciler.SoftOwnerRefs(&esConfigSecret) + assert.NoError(t, err) + assert.Len(t, owners, 2, "esConfigSecret should be owned by 2 policies") + // Verify both policies are in the owner list + assert.Contains(t, owners, reconciler.SoftOwnerRef{Namespace: "ns", Name: "policy-low", Kind: policyv1alpha1.Kind}, "policy-low should be an owner of esConfigSecret") + assert.Contains(t, owners, reconciler.SoftOwnerRef{Namespace: "ns", Name: "policy-high", Kind: policyv1alpha1.Kind}, "policy-high should be an owner of esConfigSecret") + + // Verify secret mount secret exists + var secretMountSecret corev1.Secret + err = r.Client.Get(context.Background(), types.NamespacedName{ + Namespace: "ns", + Name: esv1.StackConfigAdditionalSecretName("test-es", "test-secret"), + }, &secretMountSecret) + assert.NoError(t, err) + + // Verify secret mount secret is soft-owned by SINGLE policy (the one that defines SecretMounts) + // Only policy-high has SecretMounts, so it should be the only owner + owners, err = reconciler.SoftOwnerRefs(&secretMountSecret) + assert.NoError(t, err) + assert.Len(t, owners, 1, "secretMountSecret should be owned by 1 policy") + // Verify only policy-high is the owner + assert.Contains(t, owners, reconciler.SoftOwnerRef{Namespace: "ns", Name: "policy-high", Kind: policyv1alpha1.Kind}, "policy-high should be an owner of secretMountSecret") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create client with all resources + clientObjects := []client.Object{&esFixture, &esFileSettingsSecret, esPod, &sourceSecret} + for i := range tt.policies { + clientObjects = append(clientObjects, &tt.policies[i]) + } + + fakeRecorder := record.NewFakeRecorder(100) + reconciler := ReconcileStackConfigPolicy{ + Client: k8s.NewFakeClient(clientObjects...), + esClientProvider: fakeClientProvider(esclient.FileSettings{Version: 1}, nil), + recorder: fakeRecorder, + licenseChecker: &license.MockLicenseChecker{EnterpriseEnabled: true}, + params: operator.Parameters{ + OperatorNamespace: "elastic-system", + }, + dynamicWatches: watches.NewDynamicWatches(), + } + + // Reconcile the specified policy + got, err := reconciler.Reconcile(context.Background(), reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: "ns", + Name: tt.reconcilePolicy, + }, + }) + + if (err != nil) != tt.wantErr { + t.Errorf("Reconcile() error = %v, wantErr %v", err, tt.wantErr) + return + } + if (got.RequeueAfter > 0) != tt.wantRequeueAfter { + t.Errorf("Reconcile() got = %v, wantRequeueAfter %v", got, tt.wantRequeueAfter) + } + + // Verify policy status + policy := reconciler.getPolicy(t, types.NamespacedName{Namespace: "ns", Name: tt.reconcilePolicy}) + assert.Equal(t, tt.wantResources, policy.Status.Resources) + assert.Equal(t, tt.wantReady, policy.Status.Ready) + assert.Equal(t, tt.wantPhase, policy.Status.Phase) + + // Run custom validation if provided + if tt.validateSettings != nil { + tt.validateSettings(t, reconciler) + } + }) + } +} + func Test_cleanStackTrace(t *testing.T) { stacktrace := "Error processing slm state change: java.lang.IllegalArgumentException: Error on validating SLM requests\n\tat org.elasticsearch.ilm@8.6.1/org.elasticsearch.xpack.slm.action.ReservedSnapshotAction.prepare(ReservedSnapshotAction.java:66)\n\tat org.elasticsearch.ilm@8.6.1/org.elasticsearch.xpack.slm.action.ReservedSnapshotAction.transform(ReservedSnapshotAction.java:77)\n\tat org.elasticsearch.server@8.6.1/org.elasticsearch.reservedstate.service.ReservedClusterStateService.trialRun(ReservedClusterStateService.java:328)\n\tat org.elasticsearch.server@8.6.1/org.elasticsearch.reservedstate.service.ReservedClusterStateService.process(ReservedClusterStateService.java:169)\n\tat org.elasticsearch.server@8.6.1/org.elasticsearch.reservedstate.service.ReservedClusterStateService.process(ReservedClusterStateService.java:122)\n\tat org.elasticsearch.server@8.6.1/org.elasticsearch.reservedstate.service.FileSettingsService.processFileSettings(FileSettingsService.java:389)\n\tat org.elasticsearch.server@8.6.1/org.elasticsearch.reservedstate.service.FileSettingsService.lambda$startWatcher$3(FileSettingsService.java:312)\n\tat java.base/java.lang.Thread.run(Thread.java:833)\n\tSuppressed: java.lang.IllegalArgumentException: no such repository [badrepo]\n\t\tat org.elasticsearch.ilm@8.6.1/org.elasticsearch.xpack.slm.SnapshotLifecycleService.validateRepositoryExists(SnapshotLifecycleService.java:244)\n\t\tat org.elasticsearch.ilm@8.6.1/org.elasticsearch.xpack.slm.action.ReservedSnapshotAction.prepare(ReservedSnapshotAction.java:57)\n\t\t... 7 more\n" err := cleanStackTrace([]string{stacktrace}) diff --git a/pkg/controller/stackconfigpolicy/elasticsearch_config_settings.go b/pkg/controller/stackconfigpolicy/elasticsearch_config_settings.go index f190cb637b6..cee3455ab4b 100644 --- a/pkg/controller/stackconfigpolicy/elasticsearch_config_settings.go +++ b/pkg/controller/stackconfigpolicy/elasticsearch_config_settings.go @@ -33,19 +33,19 @@ const ( SecretsMountKey = "secretMounts.json" ) -func newElasticsearchConfigSecret(policy policyv1alpha1.StackConfigPolicy, es esv1.Elasticsearch) (corev1.Secret, error) { +func newElasticsearchConfigSecret(esConfig policyv1alpha1.ElasticsearchConfigPolicySpec, es esv1.Elasticsearch) (corev1.Secret, error) { data := make(map[string][]byte) - if len(policy.Spec.Elasticsearch.SecretMounts) > 0 { - secretMountBytes, err := json.Marshal(policy.Spec.Elasticsearch.SecretMounts) + if len(esConfig.SecretMounts) > 0 { + secretMountBytes, err := json.Marshal(esConfig.SecretMounts) if err != nil { return corev1.Secret{}, err } data[SecretsMountKey] = secretMountBytes } - elasticsearchAndMountsConfigHash := getElasticsearchConfigAndMountsHash(policy.Spec.Elasticsearch.Config, policy.Spec.Elasticsearch.SecretMounts) - if policy.Spec.Elasticsearch.Config != nil { - configDataJSONBytes, err := policy.Spec.Elasticsearch.Config.MarshalJSON() + elasticsearchAndMountsConfigHash := getElasticsearchConfigAndMountsHash(esConfig.Config, esConfig.SecretMounts) + if esConfig.Config != nil { + configDataJSONBytes, err := esConfig.Config.MarshalJSON() if err != nil { return corev1.Secret{}, err } @@ -67,9 +67,6 @@ func newElasticsearchConfigSecret(policy policyv1alpha1.StackConfigPolicy, es es Data: data, } - // Set StackConfigPolicy as the soft owner - filesettings.SetSoftOwner(&elasticsearchConfigSecret, policy) - // Add label to delete secret on deletion of the stack config policy elasticsearchConfigSecret.Labels[commonlabels.StackConfigPolicyOnDeleteLabelName] = commonlabels.OrphanSecretDeleteOnPolicyDelete @@ -126,7 +123,7 @@ func getElasticsearchConfigAndMountsHash(elasticsearchConfig *commonv1.Config, s } // elasticsearchConfigAndSecretMountsApplied checks if the Elasticsearch config and secret mounts from the stack config policy have been applied to the Elasticsearch cluster. -func elasticsearchConfigAndSecretMountsApplied(ctx context.Context, c k8s.Client, policy policyv1alpha1.StackConfigPolicy, es esv1.Elasticsearch) (bool, error) { +func elasticsearchConfigAndSecretMountsApplied(ctx context.Context, c k8s.Client, esConfigPolicy policyv1alpha1.ElasticsearchConfigPolicySpec, es esv1.Elasticsearch) (bool, error) { // Get Pods for the given Elasticsearch podList := corev1.PodList{} if err := c.List(ctx, &podList, client.InNamespace(es.Namespace), client.MatchingLabels{ @@ -135,7 +132,7 @@ func elasticsearchConfigAndSecretMountsApplied(ctx context.Context, c k8s.Client return false, err } - elasticsearchAndMountsConfigHash := getElasticsearchConfigAndMountsHash(policy.Spec.Elasticsearch.Config, policy.Spec.Elasticsearch.SecretMounts) + elasticsearchAndMountsConfigHash := getElasticsearchConfigAndMountsHash(esConfigPolicy.Config, esConfigPolicy.SecretMounts) for _, esPod := range podList.Items { if esPod.Annotations[commonannotation.ElasticsearchConfigAndSecretMountsHashAnnotation] != elasticsearchAndMountsConfigHash { return false, nil diff --git a/pkg/controller/stackconfigpolicy/kibana_config_settings.go b/pkg/controller/stackconfigpolicy/kibana_config_settings.go index b7406610524..7b3b77a516d 100644 --- a/pkg/controller/stackconfigpolicy/kibana_config_settings.go +++ b/pkg/controller/stackconfigpolicy/kibana_config_settings.go @@ -5,15 +5,10 @@ package stackconfigpolicy import ( - "context" "encoding/json" - "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/metadata" - corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" commonv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/common/v1" kibanav1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/kibana/v1" @@ -21,8 +16,7 @@ import ( commonannotation "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/annotation" "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/hash" commonlabels "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/labels" - "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/reconciler" - "github.com/elastic/cloud-on-k8s/v3/pkg/controller/elasticsearch/filesettings" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/metadata" kblabel "github.com/elastic/cloud-on-k8s/v3/pkg/controller/kibana/label" "github.com/elastic/cloud-on-k8s/v3/pkg/utils/k8s" ) @@ -31,12 +25,17 @@ const ( KibanaConfigKey = "kibana.json" ) -func newKibanaConfigSecret(policy policyv1alpha1.StackConfigPolicy, kibana kibanav1.Kibana) (corev1.Secret, error) { - kibanaConfigHash := getKibanaConfigHash(policy.Spec.Kibana.Config) +func newKibanaConfigSecret( + kbConfigPolicy policyv1alpha1.KibanaConfigPolicySpec, + namespacedSecretSources []commonv1.NamespacedSecretSource, + kibana kibanav1.Kibana, + policyRefs []policyv1alpha1.StackConfigPolicy, +) (corev1.Secret, error) { + kibanaConfigHash := getKibanaConfigHash(kbConfigPolicy.Config) configDataJSONBytes := []byte("") var err error - if policy.Spec.Kibana.Config != nil { - if configDataJSONBytes, err = policy.Spec.Kibana.Config.MarshalJSON(); err != nil { + if kbConfigPolicy.Config != nil { + if configDataJSONBytes, err = kbConfigPolicy.Config.MarshalJSON(); err != nil { return corev1.Secret{}, err } } @@ -58,14 +57,15 @@ func newKibanaConfigSecret(policy policyv1alpha1.StackConfigPolicy, kibana kiban }, } - // Set policy as the soft owner - filesettings.SetSoftOwner(&kibanaConfigSecret, policy) - // Add label to delete secret on deletion of the stack config policy kibanaConfigSecret.Labels[commonlabels.StackConfigPolicyOnDeleteLabelName] = commonlabels.OrphanSecretDeleteOnPolicyDelete // Add SecureSettings as annotation - if err = setKibanaSecureSettings(&kibanaConfigSecret, policy); err != nil { + if err = setKibanaSecureSettings(&kibanaConfigSecret, namespacedSecretSources); err != nil { + return kibanaConfigSecret, err + } + + if err = setMultipleSoftOwners(&kibanaConfigSecret, policyRefs); err != nil { return kibanaConfigSecret, err } @@ -83,13 +83,13 @@ func GetPolicyConfigSecretName(kibanaName string) string { return kibanaName + "-kb-policy-config" } -func kibanaConfigApplied(c k8s.Client, policy policyv1alpha1.StackConfigPolicy, kb kibanav1.Kibana) (bool, error) { +func kibanaConfigApplied(c k8s.Client, kbConfigPolicy policyv1alpha1.KibanaConfigPolicySpec, kb kibanav1.Kibana) (bool, error) { existingKibanaPods, err := k8s.PodsMatchingLabels(c, kb.Namespace, map[string]string{"kibana.k8s.elastic.co/name": kb.Name}) if err != nil || len(existingKibanaPods) == 0 { return false, err } - kibanaConfigHash := getKibanaConfigHash(policy.Spec.Kibana.Config) + kibanaConfigHash := getKibanaConfigHash(kbConfigPolicy.Config) for _, kbPod := range existingKibanaPods { if kbPod.Annotations[commonannotation.KibanaConfigHashAnnotation] != kibanaConfigHash { return false, nil @@ -99,45 +99,13 @@ func kibanaConfigApplied(c k8s.Client, policy policyv1alpha1.StackConfigPolicy, return true, nil } -func canBeOwned(ctx context.Context, c k8s.Client, policy policyv1alpha1.StackConfigPolicy, kb kibanav1.Kibana) (reconciler.SoftOwnerRef, bool, error) { - // Check if the secret already exists - var kibanaConfigSecret corev1.Secret - err := c.Get(ctx, types.NamespacedName{ - Name: GetPolicyConfigSecretName(kb.Name), - Namespace: kb.Namespace, - }, &kibanaConfigSecret) - if err != nil && !apierrors.IsNotFound(err) { - return reconciler.SoftOwnerRef{}, false, err - } - - if apierrors.IsNotFound(err) { - // Secret does not exist, return true - return reconciler.SoftOwnerRef{}, true, nil - } - - currentOwner, referenced := reconciler.SoftOwnerRefFromLabels(kibanaConfigSecret.Labels) - // either there is no soft owner - if !referenced { - return currentOwner, true, nil - } - // or the owner is already the given policy - canBeOwned := currentOwner.Kind == policyv1alpha1.Kind && currentOwner.Namespace == policy.Namespace && currentOwner.Name == policy.Name - return currentOwner, canBeOwned, nil -} - // setKibanaSecureSettings stores the SecureSettings Secret sources referenced in the given StackConfigPolicy for Kibana in the annotation of the Kibana config Secret. -func setKibanaSecureSettings(settingsSecret *corev1.Secret, policy policyv1alpha1.StackConfigPolicy) error { - if len(policy.Spec.Kibana.SecureSettings) == 0 { +func setKibanaSecureSettings(settingsSecret *corev1.Secret, namespacedSecretSources []commonv1.NamespacedSecretSource) error { + if len(namespacedSecretSources) == 0 { return nil } - var secretSources []commonv1.NamespacedSecretSource //nolint:prealloc - // SecureSettings field under Kibana in the StackConfigPolicy - for _, src := range policy.Spec.Kibana.SecureSettings { - secretSources = append(secretSources, commonv1.NamespacedSecretSource{Namespace: policy.GetNamespace(), SecretName: src.SecretName, Entries: src.Entries}) - } - - bytes, err := json.Marshal(secretSources) + bytes, err := json.Marshal(namespacedSecretSources) if err != nil { return err } diff --git a/pkg/controller/stackconfigpolicy/kibana_config_settings_test.go b/pkg/controller/stackconfigpolicy/kibana_config_settings_test.go index 1397e3edefc..21a16783b64 100644 --- a/pkg/controller/stackconfigpolicy/kibana_config_settings_test.go +++ b/pkg/controller/stackconfigpolicy/kibana_config_settings_test.go @@ -5,7 +5,6 @@ package stackconfigpolicy import ( - "context" "testing" "github.com/stretchr/testify/require" @@ -15,7 +14,6 @@ import ( commonv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/common/v1" kibanav1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/kibana/v1" policyv1alpha1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/stackconfigpolicy/v1alpha1" - "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/reconciler" "github.com/elastic/cloud-on-k8s/v3/pkg/utils/k8s" ) @@ -69,12 +67,11 @@ func Test_newKibanaConfigSecret(t *testing.T) { "kibana.k8s.elastic.co/name": "test-kb", "common.k8s.elastic.co/type": "kibana", "eck.k8s.elastic.co/owner-kind": "StackConfigPolicy", - "eck.k8s.elastic.co/owner-name": "test-policy", - "eck.k8s.elastic.co/owner-namespace": "test-policy-ns", }, Annotations: map[string]string{ "policy.k8s.elastic.co/kibana-config-hash": "3077592849", "policy.k8s.elastic.co/secure-settings-secrets": `[{"namespace":"test-policy-ns","secretName":"shared-secret"}]`, + "eck.k8s.elastic.co/owner-refs": `["test-policy-ns/test-policy"]`, }, }, Data: map[string][]byte{ @@ -86,7 +83,7 @@ func Test_newKibanaConfigSecret(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := newKibanaConfigSecret(*tt.args.policy, tt.args.kb) + got, err := newKibanaConfigSecret(tt.args.policy.Spec.Kibana, tt.args.policy.GetKibanaNamespacedSecureSettings(), tt.args.kb, []policyv1alpha1.StackConfigPolicy{*tt.args.policy}) require.NoError(t, err) require.Equal(t, tt.want, got) }) @@ -193,141 +190,13 @@ func Test_kibanaConfigApplied(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := kibanaConfigApplied(tt.args.client, *tt.args.policy, tt.args.kb) + got, err := kibanaConfigApplied(tt.args.client, tt.args.policy.Spec.Kibana, tt.args.kb) require.NoError(t, err) require.Equal(t, tt.want, got) }) } } -func Test_canBeOwned(t *testing.T) { - type args struct { - kb kibanav1.Kibana - policy *policyv1alpha1.StackConfigPolicy - client k8s.Client - } - - tests := []struct { - name string - args args - wantSecretRef reconciler.SoftOwnerRef - wantCanbeOwned bool - }{ - { - name: "secret owned by current policy", - args: args{ - kb: kibanav1.Kibana{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-kb", - Namespace: "test-ns", - }, - }, - policy: &policyv1alpha1.StackConfigPolicy{ - TypeMeta: metav1.TypeMeta{ - Kind: "StackConfigPolicy", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-policy", - Namespace: "test-policy-ns", - }, - Spec: policyv1alpha1.StackConfigPolicySpec{ - Kibana: policyv1alpha1.KibanaConfigPolicySpec{ - Config: &commonv1.Config{ - Data: map[string]interface{}{ - "xpack.canvas.enabled": true, - }, - }, - }, - }, - }, - client: k8s.NewFakeClient(MkKibanaConfigSecret("test-ns", "test-policy", "test-policy-ns", "3077592849")), - }, - wantSecretRef: reconciler.SoftOwnerRef{ - Namespace: "test-policy-ns", - Name: "test-policy", - Kind: "StackConfigPolicy", - }, - wantCanbeOwned: true, - }, - { - name: "secret owned by another policy", - args: args{ - kb: kibanav1.Kibana{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-kb", - Namespace: "test-ns", - }, - }, - policy: &policyv1alpha1.StackConfigPolicy{ - TypeMeta: metav1.TypeMeta{ - Kind: "StackConfigPolicy", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-policy", - Namespace: "test-policy-ns", - }, - Spec: policyv1alpha1.StackConfigPolicySpec{ - Kibana: policyv1alpha1.KibanaConfigPolicySpec{ - Config: &commonv1.Config{ - Data: map[string]interface{}{ - "xpack.canvas.enabled": true, - }, - }, - }, - }, - }, - client: k8s.NewFakeClient(MkKibanaConfigSecret("test-ns", "test-another-policy", "test-policy-ns", "3077592849")), - }, - wantSecretRef: reconciler.SoftOwnerRef{ - Namespace: "test-policy-ns", - Name: "test-another-policy", - Kind: "StackConfigPolicy", - }, - wantCanbeOwned: false, - }, - { - name: "secret does not exist", - args: args{ - kb: kibanav1.Kibana{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-kb", - Namespace: "test-ns", - }, - }, - policy: &policyv1alpha1.StackConfigPolicy{ - TypeMeta: metav1.TypeMeta{ - Kind: "StackConfigPolicy", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-policy", - Namespace: "test-policy-ns", - }, - Spec: policyv1alpha1.StackConfigPolicySpec{ - Kibana: policyv1alpha1.KibanaConfigPolicySpec{ - Config: &commonv1.Config{ - Data: map[string]interface{}{ - "xpack.canvas.enabled": true, - }, - }, - }, - }, - }, - client: k8s.NewFakeClient(), - }, - wantCanbeOwned: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - secretRef, canBeOwned, err := canBeOwned(context.Background(), tt.args.client, *tt.args.policy, tt.args.kb) - require.NoError(t, err) - require.Equal(t, tt.wantSecretRef, secretRef) - require.Equal(t, tt.wantCanbeOwned, canBeOwned) - }) - } -} - func mkKibanaPod(namespace string, hashapplied bool, hashValue string) *corev1.Pod { pod := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -356,11 +225,10 @@ func MkKibanaConfigSecret(namespace string, owningPolicyName string, owningPolic "kibana.k8s.elastic.co/name": "test-kb", "common.k8s.elastic.co/type": "kibana", "eck.k8s.elastic.co/owner-kind": "StackConfigPolicy", - "eck.k8s.elastic.co/owner-name": owningPolicyName, - "eck.k8s.elastic.co/owner-namespace": owningPolicyNamespace, }, Annotations: map[string]string{ "policy.k8s.elastic.co/kibana-config-hash": hashValue, + "eck.k8s.elastic.co/owner-refs": `["` + owningPolicyNamespace + `/` + owningPolicyName + `"]`, }, }, Data: map[string][]byte{ diff --git a/pkg/controller/stackconfigpolicy/ownership.go b/pkg/controller/stackconfigpolicy/ownership.go new file mode 100644 index 00000000000..60a56468d52 --- /dev/null +++ b/pkg/controller/stackconfigpolicy/ownership.go @@ -0,0 +1,183 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License 2.0; +// you may not use this file except in compliance with the Elastic License 2.0. + +package stackconfigpolicy + +import ( + "encoding/json" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" + + policyv1alpha1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/stackconfigpolicy/v1alpha1" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/reconciler" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/elasticsearch/filesettings" + "github.com/elastic/cloud-on-k8s/v3/pkg/utils/k8s" +) + +// setSingleSoftOwner marks a Secret as soft-owned by a single StackConfigPolicy. +// This uses labels (reconciler.SoftOwnerKindLabel, reconciler.SoftOwnerNameLabel, reconciler.SoftOwnerNamespaceLabel) +// to store the ownership relationship, allowing the policy to manage +// the Secret's lifecycle without using Kubernetes OwnerReferences. +func setSingleSoftOwner(secret *corev1.Secret, policy policyv1alpha1.StackConfigPolicy) { + if secret == nil { + return + } + + if secret.Annotations != nil { + // Remove multi-owner annotation if it exists + delete(secret.Annotations, reconciler.SoftOwnerRefsAnnotation) + } + + filesettings.SetSoftOwner(secret, policy) +} + +// setMultipleSoftOwners marks a Secret as soft-owned by multiple StackConfigPolicies. +// Unlike single ownership (which uses labels), multiple ownership stores a JSON-encoded +// map of owner references in annotations to accommodate multiple policies. +// +// The function sets: +// - The label reconciler.SoftOwnerKindLabel indicating the soft owner kind to policyv1alpha1.Kind +// - The annotation reconciler.SoftOwnerRefsAnnotation containing a JSON map of all owner namespaced names +// +// Returns an error if JSON marshaling fails. +func setMultipleSoftOwners(secret *corev1.Secret, policies []policyv1alpha1.StackConfigPolicy) error { + if secret == nil { + return nil + } + + if secret.Labels == nil { + secret.Labels = map[string]string{} + } else { + // Remove single owner labels if they exist + delete(secret.Labels, reconciler.SoftOwnerNamespaceLabel) + delete(secret.Labels, reconciler.SoftOwnerNameLabel) + } + + // Mark this Secret as being soft-owned by StackConfigPolicy resources + secret.Labels[reconciler.SoftOwnerKindLabel] = policyv1alpha1.Kind + + if secret.Annotations == nil { + secret.Annotations = map[string]string{} + } + + // Build a set of owner references using namespaced names as keys. + ownerRefs := sets.Set[string]{} + for _, p := range policies { + ownerRefs.Insert(k8s.ExtractNamespacedName(&p).String()) + } + // Store the owner references as a JSON-encoded annotation + ownerRefsBytes, err := json.Marshal(sets.List(ownerRefs)) + if err != nil { + return err + } + + secret.Annotations[reconciler.SoftOwnerRefsAnnotation] = string(ownerRefsBytes) + return nil +} + +// isPolicySoftOwner checks if the given StackConfigPolicy is a soft owner of the Secret. +// It handles both single-owner (reconciler.SoftOwnerNameLabel, reconciler.SoftOwnerNamespaceLabel) +// and multi-owner (reconciler.SoftOwnerRefsAnnotation) scenarios. +// Returns true or false depending on whether the policy is an owner of the secret +// and an error if there's a problem unmarshalling the owner references +func isPolicySoftOwner(secret *corev1.Secret, policyNsn types.NamespacedName) (bool, error) { + if secret == nil { + return false, nil + } + + // Check if this Secret is soft-owned by a StackConfigPolicy + if ownerKind := secret.Labels[reconciler.SoftOwnerKindLabel]; ownerKind != policyv1alpha1.Kind { + // Not a policy soft-owned secret + return false, nil + } + + // Check for multi-policy ownership (annotation-based) + if ownerRefsBytes, exists := secret.Annotations[reconciler.SoftOwnerRefsAnnotation]; exists { + // Multi-policy soft owned secret - parse the JSON map of owners + var ownerRefsSlice []string + if err := json.Unmarshal([]byte(ownerRefsBytes), &ownerRefsSlice); err != nil { + return false, err + } + + ownerRefs := sets.New(ownerRefsSlice...) + return ownerRefs.Has(types.NamespacedName{Name: policyNsn.Name, Namespace: policyNsn.Namespace}.String()), nil + } + + // Fall back to single-policy ownership (label-based) + currentOwner, referenced := reconciler.SoftOwnerRefFromLabels(secret.Labels) + if !referenced { + // No soft owner found in labels + return false, nil + } + + // Check if the single owner matches the given policy + return currentOwner.Name == policyNsn.Name && currentOwner.Namespace == policyNsn.Namespace, nil +} + +// removePolicySoftOwner removes a StackConfigPolicy if it is soft owning the given secret. +// It handles both single-owner (reconciler.SoftOwnerNameLabel, reconciler.SoftOwnerNamespaceLabel) +// and multi-owner (reconciler.SoftOwnerRefsAnnotation) scenarios. +// +// For single-owner secrets: +// - If the owner matches, removes all soft owner labels/annotations +// - If the owner doesn't match, leaves the secret unchanged +// +// For multi-owner secrets: +// - Removes the policy from the JSON map in annotations +// - Updates the annotation with the remaining owners or removes the annotation if no owners remain +// +// Returns the number of remaining owners after removal and an error if there's a problem with JSON marshaling/unmarshalling +func removePolicySoftOwner(secret *corev1.Secret, policyNsn types.NamespacedName) (int, error) { + if secret == nil { + return 0, nil + } + + // Check for multi-policy ownership (annotation-based) + if ownerRefsBytes, exists := secret.Annotations[reconciler.SoftOwnerRefsAnnotation]; exists { + // Multi-policy soft owned secret - parse and update the set + var ownerRefsSlice []string + if err := json.Unmarshal([]byte(ownerRefsBytes), &ownerRefsSlice); err != nil { + return 0, err + } + + ownerRefs := sets.New(ownerRefsSlice...) + // Remove the specified policy from the set + ownerRefs.Delete(types.NamespacedName{Name: policyNsn.Name, Namespace: policyNsn.Namespace}.String()) + if ownerRefs.Len() == 0 { + // No owners remain, remove the annotation + delete(secret.Annotations, reconciler.SoftOwnerRefsAnnotation) + return 0, nil + } + + // Marshal the updated owner list back to JSON + ownerRefsBytes, err := json.Marshal(sets.List(ownerRefs)) + if err != nil { + return 0, err + } + + // Update the annotation with the new owner list + secret.Annotations[reconciler.SoftOwnerRefsAnnotation] = string(ownerRefsBytes) + return len(ownerRefs), nil + } + + // Handle single-policy ownership (label-based) + currentOwner, referenced := reconciler.SoftOwnerRefFromLabels(secret.Labels) + if !referenced { + // No soft owner found + return 0, nil + } + + // Check if the single owner matches the policy to be removed + if currentOwner.Name == policyNsn.Name && currentOwner.Namespace == policyNsn.Namespace { + // Remove the soft owner labels since this was the only owner + delete(secret.Labels, reconciler.SoftOwnerNamespaceLabel) + delete(secret.Labels, reconciler.SoftOwnerNameLabel) + return 0, nil + } + + // The policy to remove doesn't match the current owner, so no change + return 1, nil +} diff --git a/pkg/controller/stackconfigpolicy/ownership_test.go b/pkg/controller/stackconfigpolicy/ownership_test.go new file mode 100644 index 00000000000..109f98c92ab --- /dev/null +++ b/pkg/controller/stackconfigpolicy/ownership_test.go @@ -0,0 +1,551 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License 2.0; +// you may not use this file except in compliance with the Elastic License 2.0. + +package stackconfigpolicy + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + policyv1alpha1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/stackconfigpolicy/v1alpha1" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/reconciler" +) + +//nolint:thelper +func Test_setSingleSoftOwner(t *testing.T) { + tests := []struct { + name string + secret *corev1.Secret + policy policyv1alpha1.StackConfigPolicy + validate func(t *testing.T, secret *corev1.Secret, policy policyv1alpha1.StackConfigPolicy) + }{ + { + name: "overwrites existing soft owner labels", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: "old-kind", + reconciler.SoftOwnerNameLabel: "old-policy", + reconciler.SoftOwnerNamespaceLabel: "old-namespace", + "existing-label": "existing-value", + }, + Annotations: map[string]string{ + reconciler.SoftOwnerRefsAnnotation: "{}", + "existing-annotation": "existing-value", + }, + }, + }, + policy: policyv1alpha1.StackConfigPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-policy", + Namespace: "new-namespace", + }, + }, + validate: func(t *testing.T, secret *corev1.Secret, policy policyv1alpha1.StackConfigPolicy) { + assert.NotNil(t, secret.Labels) + assert.Equal(t, policyv1alpha1.Kind, secret.Labels[reconciler.SoftOwnerKindLabel]) + assert.Equal(t, "new-policy", secret.Labels[reconciler.SoftOwnerNameLabel]) + assert.Equal(t, "new-namespace", secret.Labels[reconciler.SoftOwnerNamespaceLabel]) + assert.Equal(t, "existing-value", secret.Labels["existing-label"]) + assert.Equal(t, "existing-value", secret.Annotations["existing-annotation"]) + assert.NotContains(t, secret.Annotations, reconciler.SoftOwnerRefsAnnotation) + }, + }, + { + name: "returns nil for nil secret", + secret: nil, + validate: func(t *testing.T, secret *corev1.Secret, policy policyv1alpha1.StackConfigPolicy) { + assert.Nil(t, secret) + }, + }, + { + name: "secret with nil labels and annotations", + secret: &corev1.Secret{}, + validate: func(t *testing.T, secret *corev1.Secret, policy policyv1alpha1.StackConfigPolicy) { + assert.NotNil(t, secret) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + setSingleSoftOwner(tt.secret, tt.policy) + tt.validate(t, tt.secret, tt.policy) + }) + } +} + +//nolint:thelper +func Test_setMultipleSoftOwners(t *testing.T) { + tests := []struct { + name string + secret *corev1.Secret + policies []policyv1alpha1.StackConfigPolicy + validate func(t *testing.T, secret *corev1.Secret, policies []policyv1alpha1.StackConfigPolicy, err error) + }{ + { + name: "removes single-owner labels and sets multi-owner annotation", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + reconciler.SoftOwnerNameLabel: "old-single-policy", + reconciler.SoftOwnerNamespaceLabel: "old-namespace", + "existing-label": "existing-value", + }, + Annotations: map[string]string{ + reconciler.SoftOwnerRefsAnnotation: "replaced-value", + "existing-annotation": "existing-value", + }, + }, + }, + policies: []policyv1alpha1.StackConfigPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "policy-1", + Namespace: "namespace-1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "policy-2", + Namespace: "namespace-2", + }, + }, + { + // should be deduplicated + ObjectMeta: metav1.ObjectMeta{ + Name: "policy-2", + Namespace: "namespace-2", + }, + }, + }, + validate: func(t *testing.T, secret *corev1.Secret, policies []policyv1alpha1.StackConfigPolicy, err error) { + require.NoError(t, err) + + // Verify single-owner labels are removed + assert.NotContains(t, secret.Labels, reconciler.SoftOwnerNameLabel) + assert.NotContains(t, secret.Labels, reconciler.SoftOwnerNamespaceLabel) + + // Verify kind label is still set + assert.Equal(t, policyv1alpha1.Kind, secret.Labels[reconciler.SoftOwnerKindLabel]) + + // Verify existing label is preserved + assert.Equal(t, "existing-value", secret.Labels["existing-label"]) + + // Verify existing annotation is preserved + assert.Equal(t, "existing-value", secret.Annotations["existing-annotation"]) + + // Verify multi-owner annotation is set with both policies + ownerRefsJSON := secret.Annotations[reconciler.SoftOwnerRefsAnnotation] + assert.NotEmpty(t, ownerRefsJSON) + + var ownerRefs []string + err = json.Unmarshal([]byte(ownerRefsJSON), &ownerRefs) + require.NoError(t, err) + assert.EqualValues(t, []string{ + types.NamespacedName{Name: "policy-1", Namespace: "namespace-1"}.String(), + types.NamespacedName{Name: "policy-2", Namespace: "namespace-2"}.String(), + }, ownerRefs) + }, + }, + { + name: "returns nil for nil secret", + secret: nil, + validate: func(t *testing.T, secret *corev1.Secret, policies []policyv1alpha1.StackConfigPolicy, err error) { + assert.Nil(t, err) + assert.Nil(t, secret) + assert.Len(t, policies, 0) + }, + }, + { + name: "secret with nil labels and annotations", + secret: &corev1.Secret{}, + validate: func(t *testing.T, secret *corev1.Secret, policies []policyv1alpha1.StackConfigPolicy, err error) { + assert.Nil(t, err) + assert.NotNil(t, secret) + assert.Len(t, policies, 0) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := setMultipleSoftOwners(tt.secret, tt.policies) + tt.validate(t, tt.secret, tt.policies, err) + }) + } +} + +//nolint:thelper +func Test_removePolicySoftOwner(t *testing.T) { + tests := []struct { + name string + secret *corev1.Secret + policyToRemove types.NamespacedName + validate func(t *testing.T, secret *corev1.Secret, remainingCount int, err error) + }{ + { + name: "removes policy from multi-owner with remaining owners", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + Annotations: map[string]string{ + reconciler.SoftOwnerRefsAnnotation: `["namespace-1/policy-1","namespace-2/policy-2","namespace-3/policy-3"]`, + }, + }, + }, + policyToRemove: types.NamespacedName{Name: "policy-2", Namespace: "namespace-2"}, + validate: func(t *testing.T, secret *corev1.Secret, remainingCount int, err error) { + require.NoError(t, err) + assert.Equal(t, 2, remainingCount) + + // Verify the annotation still exists with remaining owners + ownerRefsJSON := secret.Annotations[reconciler.SoftOwnerRefsAnnotation] + assert.NotEmpty(t, ownerRefsJSON) + + var ownerRefs []string + err = json.Unmarshal([]byte(ownerRefsJSON), &ownerRefs) + require.NoError(t, err) + assert.Len(t, ownerRefs, 2) + + // Verify policy-2 was removed + assert.EqualValues(t, []string{ + "namespace-1/policy-1", + "namespace-3/policy-3", + }, ownerRefs) + }, + }, + { + name: "removes last policy from multi-owner and cleans up annotation", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + Annotations: map[string]string{ + reconciler.SoftOwnerRefsAnnotation: `["namespace-1/policy-1"]`, + "other-annotation": "preserved", + }, + }, + }, + policyToRemove: types.NamespacedName{Name: "policy-1", Namespace: "namespace-1"}, + validate: func(t *testing.T, secret *corev1.Secret, remainingCount int, err error) { + require.NoError(t, err) + assert.Equal(t, 0, remainingCount) + + // Verify the annotation was removed + assert.NotContains(t, secret.Annotations, reconciler.SoftOwnerRefsAnnotation) + + // Verify other annotations are preserved + assert.Equal(t, "preserved", secret.Annotations["other-annotation"]) + }, + }, + { + name: "removes matching single-owner and cleans up labels", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + reconciler.SoftOwnerNameLabel: "single-policy", + reconciler.SoftOwnerNamespaceLabel: "single-namespace", + "other-label": "preserved", + }, + }, + }, + policyToRemove: types.NamespacedName{Name: "single-policy", Namespace: "single-namespace"}, + validate: func(t *testing.T, secret *corev1.Secret, remainingCount int, err error) { + require.NoError(t, err) + assert.Equal(t, 0, remainingCount) + + // Verify all soft owner labels were removed + assert.NotContains(t, secret.Labels, reconciler.SoftOwnerNameLabel) + assert.NotContains(t, secret.Labels, reconciler.SoftOwnerNamespaceLabel) + + // Verify other labels are preserved + assert.Equal(t, "preserved", secret.Labels["other-label"]) + }, + }, + { + name: "returns 1 when policy doesn't match single-owner", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + reconciler.SoftOwnerNameLabel: "existing-policy", + reconciler.SoftOwnerNamespaceLabel: "existing-namespace", + }, + }, + }, + policyToRemove: types.NamespacedName{Name: "different-policy", Namespace: "different-namespace"}, + validate: func(t *testing.T, secret *corev1.Secret, remainingCount int, err error) { + require.NoError(t, err) + assert.Equal(t, 1, remainingCount) + + // Verify labels remain unchanged + assert.Equal(t, policyv1alpha1.Kind, secret.Labels[reconciler.SoftOwnerKindLabel]) + assert.Equal(t, "existing-policy", secret.Labels[reconciler.SoftOwnerNameLabel]) + assert.Equal(t, "existing-namespace", secret.Labels[reconciler.SoftOwnerNamespaceLabel]) + }, + }, + { + name: "returns 0 for nil secret", + secret: nil, + policyToRemove: types.NamespacedName{Name: "policy", Namespace: "namespace"}, + validate: func(t *testing.T, secret *corev1.Secret, remainingCount int, err error) { + require.NoError(t, err) + assert.Equal(t, 0, remainingCount) + }, + }, + { + name: "returns 0 for non-owned secret", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + "some-label": "some-value", + }, + }, + }, + policyToRemove: types.NamespacedName{Name: "policy", Namespace: "namespace"}, + validate: func(t *testing.T, secret *corev1.Secret, remainingCount int, err error) { + require.NoError(t, err) + assert.Equal(t, 0, remainingCount) + }, + }, + { + name: "returns error for invalid JSON in annotation", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + Annotations: map[string]string{ + reconciler.SoftOwnerRefsAnnotation: `invalid-json`, + }, + }, + }, + policyToRemove: types.NamespacedName{Name: "policy", Namespace: "namespace"}, + validate: func(t *testing.T, secret *corev1.Secret, remainingCount int, err error) { + require.Error(t, err) + assert.Equal(t, 0, remainingCount) + }, + }, + { + name: "returns 0 when removing non-existent policy from multi-owner", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + Annotations: map[string]string{ + reconciler.SoftOwnerRefsAnnotation: `["namespace-1/policy-1","namespace-2/policy-2"]`, + }, + }, + }, + policyToRemove: types.NamespacedName{Name: "non-existent", Namespace: "namespace-3"}, + validate: func(t *testing.T, secret *corev1.Secret, remainingCount int, err error) { + require.NoError(t, err) + assert.Equal(t, 2, remainingCount) + + // Verify both original policies remain + var ownerRefs []string + err = json.Unmarshal([]byte(secret.Annotations[reconciler.SoftOwnerRefsAnnotation]), &ownerRefs) + require.NoError(t, err) + assert.Len(t, ownerRefs, 2) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + remainingCount, err := removePolicySoftOwner(tt.secret, tt.policyToRemove) + tt.validate(t, tt.secret, remainingCount, err) + }) + } +} + +//nolint:thelper +func Test_isPolicySoftOwner(t *testing.T) { + tests := []struct { + name string + secret *corev1.Secret + policyNsn types.NamespacedName + validate func(t *testing.T, isOwner bool, err error) + }{ + { + name: "returns true when policy is owner in multi-owner", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + Annotations: map[string]string{ + reconciler.SoftOwnerRefsAnnotation: `["namespace-1/policy-1","namespace-2/policy-2"]`, + }, + }, + }, + policyNsn: types.NamespacedName{Name: "policy-1", Namespace: "namespace-1"}, + validate: func(t *testing.T, isOwner bool, err error) { + require.NoError(t, err) + assert.True(t, isOwner) + }, + }, + { + name: "returns false when policy is not owner in multi-owner", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + Annotations: map[string]string{ + reconciler.SoftOwnerRefsAnnotation: `["namespace-1/policy-1","namespace-2/policy-2"]`, + }, + }, + }, + policyNsn: types.NamespacedName{Name: "policy-3", Namespace: "namespace-3"}, + validate: func(t *testing.T, isOwner bool, err error) { + require.NoError(t, err) + assert.False(t, isOwner) + }, + }, + { + name: "returns true when policy matches single-owner", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + reconciler.SoftOwnerNameLabel: "single-policy", + reconciler.SoftOwnerNamespaceLabel: "single-namespace", + }, + }, + }, + policyNsn: types.NamespacedName{Name: "single-policy", Namespace: "single-namespace"}, + validate: func(t *testing.T, isOwner bool, err error) { + require.NoError(t, err) + assert.True(t, isOwner) + }, + }, + { + name: "returns false when policy doesn't match single-owner", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + reconciler.SoftOwnerNameLabel: "single-policy", + reconciler.SoftOwnerNamespaceLabel: "single-namespace", + }, + }, + }, + policyNsn: types.NamespacedName{Name: "different-policy", Namespace: "different-namespace"}, + validate: func(t *testing.T, isOwner bool, err error) { + require.NoError(t, err) + assert.False(t, isOwner) + }, + }, + { + name: "returns false for nil secret", + secret: nil, + policyNsn: types.NamespacedName{Name: "policy", Namespace: "namespace"}, + validate: func(t *testing.T, isOwner bool, err error) { + require.NoError(t, err) + assert.False(t, isOwner) + }, + }, + { + name: "returns false for non-policy-owned secret", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + "some-other-label": "some-value", + }, + }, + }, + policyNsn: types.NamespacedName{Name: "policy", Namespace: "namespace"}, + validate: func(t *testing.T, isOwner bool, err error) { + require.NoError(t, err) + assert.False(t, isOwner) + }, + }, + { + name: "returns error for invalid JSON in annotation", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + Annotations: map[string]string{ + reconciler.SoftOwnerRefsAnnotation: `invalid-json`, + }, + }, + }, + policyNsn: types.NamespacedName{Name: "policy", Namespace: "namespace"}, + validate: func(t *testing.T, isOwner bool, err error) { + require.Error(t, err) + assert.False(t, isOwner) + }, + }, + { + name: "returns false when secret has kind label but no owner references", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-secret", + Namespace: "test-namespace", + Labels: map[string]string{ + reconciler.SoftOwnerKindLabel: policyv1alpha1.Kind, + }, + }, + }, + policyNsn: types.NamespacedName{Name: "policy", Namespace: "namespace"}, + validate: func(t *testing.T, isOwner bool, err error) { + require.NoError(t, err) + assert.False(t, isOwner) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + isOwner, err := isPolicySoftOwner(tt.secret, tt.policyNsn) + tt.validate(t, isOwner, err) + }) + } +} diff --git a/pkg/controller/stackconfigpolicy/stackconfigpolicy.go b/pkg/controller/stackconfigpolicy/stackconfigpolicy.go new file mode 100644 index 00000000000..d0f3c67378c --- /dev/null +++ b/pkg/controller/stackconfigpolicy/stackconfigpolicy.go @@ -0,0 +1,401 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License 2.0; +// you may not use this file except in compliance with the Elastic License 2.0. + +package stackconfigpolicy + +import ( + "cmp" + "errors" + "fmt" + "maps" + "slices" + "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + + commonv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/common/v1" + esv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/elasticsearch/v1" + kbv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/kibana/v1" + policyv1alpha1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/stackconfigpolicy/v1alpha1" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/operator" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/settings" + "github.com/elastic/cloud-on-k8s/v3/pkg/utils/k8s" +) + +var errMergeConflict = errors.New("merge conflict") + +// configPolicy is a generic container for merged StackConfigPolicy specifications. +// It holds the merged spec of type T (either ElasticsearchConfigPolicySpec or KibanaConfigPolicySpec), +// along with metadata about which policies were merged, any conflicts encountered, and aggregated +// secret sources. The extractFunc and mergeFunc callbacks allow customization of how specs are +// extracted from policies and merged together. +type configPolicy[T any] struct { + // Spec is the merged config policy specification + Spec T + // extractFunc extracts the relevant spec (ES or Kibana) from a StackConfigPolicy + extractFunc func(p *policyv1alpha1.StackConfigPolicy) (spec T) + // mergeFunc merges a source spec into the destination spec, handling conflicts + mergeFunc func(dstSpec *T, srcSpec T, srcPolicy *policyv1alpha1.StackConfigPolicy) error + // SecretSources contains aggregated secure settings secret sources, keyed by StackConfigPolicy namespace + SecretSources []commonv1.NamespacedSecretSource + // PolicyRefs contains references to all policies that targeted and were merged for this object + PolicyRefs []policyv1alpha1.StackConfigPolicy +} + +// merge processes all provided policies, filters those targeting the given object, and merges them +// in order of their weight (highest weight first). Policies with the same weight are flagged as conflicts. +// The merge operation is customized through the configPolicy's extractFunc and mergeFunc callbacks. +func merge[T any]( + c *configPolicy[T], + obj metav1.Object, + allPolicies []policyv1alpha1.StackConfigPolicy, + operatorNamespace string, +) error { + if len(allPolicies) == 0 { + return nil + } + + policiesByWeight := make(map[int32]policyv1alpha1.StackConfigPolicy) + for _, p := range allPolicies { + matches, err := doesPolicyMatchObject(&p, obj, operatorNamespace) + if err != nil { + return err + } + if !matches { + // policy does not target the given k8s object + continue + } + pWeight := p.Spec.Weight + if pExisting, exists := policiesByWeight[pWeight]; exists { + pNsn := k8s.ExtractNamespacedName(&p) + pExistingNsn := k8s.ExtractNamespacedName(&pExisting) + err := fmt.Errorf("%w: policies %q and %q have the same weight %d", errMergeConflict, pNsn, pExistingNsn, pWeight) + return err + } + + policiesByWeight[pWeight] = p + c.PolicyRefs = append(c.PolicyRefs, p) + } + + slices.SortFunc(c.PolicyRefs, func(p1, p2 policyv1alpha1.StackConfigPolicy) int { + return cmp.Compare(p2.Spec.Weight, p1.Spec.Weight) + }) + + for _, p := range c.PolicyRefs { + srcSpec := c.extractFunc(&p) + if err := c.mergeFunc(&c.Spec, srcSpec, &p); err != nil { + return err + } + } + + return nil +} + +// getConfigPolicyForElasticsearch builds a merged stack config policy for the given Elasticsearch cluster. +// It processes all provided policies, filtering those that target the Elasticsearch cluster, and merges them +// in order of their weight (lowest to highest). Policies with the same weight are flagged as conflicts. +// Returns an esPolicyConfig containing the merged configuration and any error occurred during merging. +func getConfigPolicyForElasticsearch(es *esv1.Elasticsearch, allPolicies []policyv1alpha1.StackConfigPolicy, params operator.Parameters) (*configPolicy[policyv1alpha1.ElasticsearchConfigPolicySpec], error) { + mergedPolicies := 0 + sMntAggr := secretMountsAggregator{} + sSrcAggr := secretSourceAggregator{} + cfgPolicy := &configPolicy[policyv1alpha1.ElasticsearchConfigPolicySpec]{ + extractFunc: func(p *policyv1alpha1.StackConfigPolicy) policyv1alpha1.ElasticsearchConfigPolicySpec { + return p.Spec.Elasticsearch + }, + mergeFunc: func(dst *policyv1alpha1.ElasticsearchConfigPolicySpec, src policyv1alpha1.ElasticsearchConfigPolicySpec, srcPolicy *policyv1alpha1.StackConfigPolicy) error { + var err error + if mergedPolicies == 0 { + // First policy: copy directly without merging/canonicalizing to avoid unnecessary differences + specCopy := src.DeepCopy() + // SecureSettings are aggregated by sSrcAggr + specCopy.SecureSettings = nil + // SecretMounts are aggregated by sMntAggr + specCopy.SecretMounts = nil + *dst = *specCopy + } else { + if err = mergeElasticsearchSpecs(dst, &src); err != nil { + return err + } + } + + if dst.SecretMounts, err = sMntAggr.aggregate(dst.SecretMounts, srcPolicy.Spec.Elasticsearch.SecretMounts, srcPolicy); err != nil { + return err + } + + sSrcAggr.aggregate(srcPolicy.Spec.Elasticsearch.SecureSettings, srcPolicy) + mergedPolicies++ + return nil + }, + } + + if err := merge(cfgPolicy, es, allPolicies, params.OperatorNamespace); err != nil { + return cfgPolicy, err + } + cfgPolicy.SecretSources = sSrcAggr.namespacedSecretSources + return cfgPolicy, nil +} + +// mergeElasticsearchSpecs merges src policyv1alpha1.ElasticsearchConfigPolicySpec into dst. +func mergeElasticsearchSpecs(dst, src *policyv1alpha1.ElasticsearchConfigPolicySpec) error { + var err error + fields := []struct { + dst **commonv1.Config + src *commonv1.Config + merge func(*commonv1.Config, *commonv1.Config) (*commonv1.Config, error) + }{ + {&dst.ClusterSettings, src.ClusterSettings, deepMergeConfig}, + {&dst.SnapshotRepositories, src.SnapshotRepositories, mergeConfig}, + {&dst.SnapshotLifecyclePolicies, src.SnapshotLifecyclePolicies, deepMergeConfig}, + {&dst.SecurityRoleMappings, src.SecurityRoleMappings, deepMergeConfig}, + {&dst.IndexLifecyclePolicies, src.IndexLifecyclePolicies, deepMergeConfig}, + {&dst.IngestPipelines, src.IngestPipelines, deepMergeConfig}, + {&dst.IndexTemplates.ComposableIndexTemplates, src.IndexTemplates.ComposableIndexTemplates, deepMergeConfig}, + {&dst.IndexTemplates.ComponentTemplates, src.IndexTemplates.ComponentTemplates, deepMergeConfig}, + {&dst.Config, src.Config, deepMergeConfig}, + } + for _, f := range fields { + *f.dst, err = f.merge(*f.dst, f.src) + if err != nil { + return err + } + } + return nil +} + +// getConfigPolicyForKibana builds a merged stack config policy for the given Kibana instance. +// It processes all provided policies, filtering those that target the Kibana instance, and merges them +// in order of their weight (lowest to highest). Policies with the same weight are flagged as conflicts. +// Returns an kbnPolicyConfig containing the merged configuration and any error occurred during merging. +func getConfigPolicyForKibana(kbn *kbv1.Kibana, allPolicies []policyv1alpha1.StackConfigPolicy, params operator.Parameters) (*configPolicy[policyv1alpha1.KibanaConfigPolicySpec], error) { + mergedPolicies := 0 + sSrcAggr := secretSourceAggregator{} + cfgPolicy := &configPolicy[policyv1alpha1.KibanaConfigPolicySpec]{ + extractFunc: func(p *policyv1alpha1.StackConfigPolicy) policyv1alpha1.KibanaConfigPolicySpec { + return p.Spec.Kibana + }, + mergeFunc: func(dst *policyv1alpha1.KibanaConfigPolicySpec, src policyv1alpha1.KibanaConfigPolicySpec, srcPolicy *policyv1alpha1.StackConfigPolicy) error { + if mergedPolicies == 0 { + // First policy: copy directly without merging/canonicalizing to avoid unnecessary differences + specCopy := src.DeepCopy() + // SecureSettings are aggregated by sSrcAggr + specCopy.SecureSettings = nil + *dst = *specCopy + } else { + var err error + if dst.Config, err = deepMergeConfig(dst.Config, src.Config); err != nil { + return err + } + } + + sSrcAggr.aggregate(srcPolicy.Spec.Kibana.SecureSettings, srcPolicy) + mergedPolicies++ + return nil + }, + } + + if err := merge(cfgPolicy, kbn, allPolicies, params.OperatorNamespace); err != nil { + return cfgPolicy, err + } + cfgPolicy.SecretSources = sSrcAggr.namespacedSecretSources + return cfgPolicy, nil +} + +// doesPolicyMatchObject checks if the given StackConfigPolicy targets the given object. +// A policy targets an object if both following conditions are met: +// 1. The policy is in either the operator namespace or the same namespace as the object +// 2. The policy's label selector matches the object's labels +// Returns true if the policy targets the object, false otherwise, and an error if the label selector is invalid. +func doesPolicyMatchObject(policy *policyv1alpha1.StackConfigPolicy, obj metav1.Object, operatorNamespace string) (bool, error) { + // Convert the label selector to a selector object + selector, err := metav1.LabelSelectorAsSelector(&policy.Spec.ResourceSelector) + if err != nil { + return false, err + } + + // Check namespace restrictions; the policy must be in operator namespace or same namespace as the target object + if policy.Namespace != operatorNamespace && policy.Namespace != obj.GetNamespace() { + return false, nil + } + + // Check if the label selector matches the Elasticsearch labels + if !selector.Matches(labels.Set(obj.GetLabels())) { + return false, nil + } + + return true, nil +} + +// deepMergeConfig merges the source Config into the destination Config using canonical configuration merging. +// The merge is performed at the field level, with source values overriding destination values. +// If src is nil, dst is returned unchanged. If dst is nil, it is initialized before merging. +// Returns the merged config and any error occurred during config parsing or merging. +func deepMergeConfig(dst *commonv1.Config, src *commonv1.Config) (*commonv1.Config, error) { + if src == nil { + return dst, nil + } + + var dstCanonicalConfig *settings.CanonicalConfig + var err error + if dst == nil { + dst = &commonv1.Config{} + dstCanonicalConfig = settings.NewCanonicalConfig() + } else { + dstCanonicalConfig, err = settings.NewCanonicalConfigFrom(dst.DeepCopy().Data) + if err != nil { + return nil, err + } + } + + srcCanonicalConfig, err := settings.NewCanonicalConfigFrom(src.DeepCopy().Data) + if err != nil { + return nil, err + } + + if err = dstCanonicalConfig.MergeWith(srcCanonicalConfig); err != nil { + return nil, err + } + + dst.Data = nil + if err = dstCanonicalConfig.Unpack(&dst.Data); err != nil { + return nil, err + } + + return dst, nil +} + +// mergeConfig merges the source Config into the destination Config by replacing entire top-level keys. +// Unlike deepMergeConfig which performs recursive merging, this function replaces each top-level key +// in dst with the corresponding value from src. Both configs are first canonicalized to ensure +// consistent structure. If src is nil, dst is returned unchanged. If dst is nil, it is initialized. +// Returns the merged config and any error occurred during config parsing or unpacking. +func mergeConfig(dst *commonv1.Config, src *commonv1.Config) (*commonv1.Config, error) { + if src == nil { + return dst, nil + } + + var dstCanonicalConfig *settings.CanonicalConfig + var err error + if dst == nil { + dst = &commonv1.Config{} + dstCanonicalConfig = settings.NewCanonicalConfig() + } else { + dstCanonicalConfig, err = settings.NewCanonicalConfigFrom(dst.DeepCopy().Data) + if err != nil { + return nil, err + } + } + + srcCanonicalConfig, err := settings.NewCanonicalConfigFrom(src.DeepCopy().Data) + if err != nil { + return nil, err + } + + dst.Data = nil + if err = dstCanonicalConfig.Unpack(&dst.Data); err != nil { + return nil, err + } + + srcCfg := &commonv1.Config{} + if err = srcCanonicalConfig.Unpack(&srcCfg.Data); err != nil { + return nil, err + } + + maps.Copy(dst.Data, srcCfg.Data) + + return dst, nil +} + +// secretMountsAggregator aggregates secret mounts from multiple policies while detecting conflicts. +// It tracks which policy defines each secret name and mount path to ensure no two policies +// attempt to mount different secrets to the same path or mount the same secret name twice. +type secretMountsAggregator struct { + policiesByMountPath map[string]*policyv1alpha1.StackConfigPolicy + policiesBySecretName map[string]*policyv1alpha1.StackConfigPolicy + appliedPolicies int +} + +// aggregate merges source secret mounts into destination, checking for conflicts on secret names +// and mount paths. Returns the merged slice of secret mounts sorted deterministically when +// multiple policies have been applied, or an error if conflicts are detected. +func (s *secretMountsAggregator) aggregate(dst []policyv1alpha1.SecretMount, src []policyv1alpha1.SecretMount, srcPolicy *policyv1alpha1.StackConfigPolicy) ([]policyv1alpha1.SecretMount, error) { + if src == nil { + return dst, nil + } + + s.appliedPolicies++ + + if s.policiesBySecretName == nil { + s.policiesBySecretName = make(map[string]*policyv1alpha1.StackConfigPolicy) + } + if s.policiesByMountPath == nil { + s.policiesByMountPath = make(map[string]*policyv1alpha1.StackConfigPolicy) + } + + srcPolicyNsn := k8s.ExtractNamespacedName(srcPolicy) + + // Merge in source entries, checking for conflicts + for _, secretMount := range src { + if existingPolicy, exists := s.policiesBySecretName[secretMount.SecretName]; exists { + existingPolicyNsn := k8s.ExtractNamespacedName(existingPolicy) + err := fmt.Errorf("%w: secret with name %q is defined in policies %q, %q", errMergeConflict, secretMount.SecretName, + srcPolicyNsn.String(), existingPolicyNsn.String()) + return nil, err + } + if existingPolicy, exists := s.policiesByMountPath[secretMount.MountPath]; exists { + existingPolicyNsn := k8s.ExtractNamespacedName(existingPolicy) + err := fmt.Errorf("%w: secret mount path %q is defined in policies %q, %q", errMergeConflict, secretMount.MountPath, + srcPolicyNsn.String(), existingPolicyNsn.String()) + return nil, err + } + s.policiesBySecretName[secretMount.SecretName] = srcPolicy + s.policiesByMountPath[secretMount.MountPath] = srcPolicy + dst = append(dst, secretMount) + } + + if s.appliedPolicies > 1 { + // we want sort only when we have applied more than one policy to guarantee deterministic order, otherwise + // leave namespacedSecretSources as they came to not cause undesired differences + slices.SortFunc(dst, func(a, b policyv1alpha1.SecretMount) int { + return strings.Compare(a.SecretName, b.SecretName) + }) + } + return dst, nil +} + +// secretSourceAggregator aggregates secure settings secret sources from multiple policies. +// It organizes secret sources by policy namespace and ensures deterministic ordering when +// multiple policies contribute sources. +type secretSourceAggregator struct { + appliedPolicies int + namespacedSecretSources []commonv1.NamespacedSecretSource +} + +// aggregate merges source secure settings into the aggregator, organizing them by the source +// policy's namespace. Secret sources are sorted deterministically when multiple policies have +// been applied to ensure consistent results. +func (s *secretSourceAggregator) aggregate(src []commonv1.SecretSource, srcPolicy *policyv1alpha1.StackConfigPolicy) { + if src == nil { + return + } + s.appliedPolicies++ + + srcPolicyNamespace := srcPolicy.GetNamespace() + for _, ss := range src { + s.namespacedSecretSources = append(s.namespacedSecretSources, commonv1.NamespacedSecretSource{ + Namespace: srcPolicyNamespace, + SecretName: ss.SecretName, + Entries: ss.Entries, + }) + } + + if s.appliedPolicies > 1 { + // we want sort only when we have applied more than one policy to guarantee deterministic order, otherwise + // leave namespacedSecretSources as they came to not cause undesired differences + slices.SortFunc(s.namespacedSecretSources, func(a, b commonv1.NamespacedSecretSource) int { + return strings.Compare(a.SecretName, b.SecretName) + }) + } +} diff --git a/pkg/controller/stackconfigpolicy/stackconfigpolicy_test.go b/pkg/controller/stackconfigpolicy/stackconfigpolicy_test.go new file mode 100644 index 00000000000..fb5bf3dc1fb --- /dev/null +++ b/pkg/controller/stackconfigpolicy/stackconfigpolicy_test.go @@ -0,0 +1,1147 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License 2.0; +// you may not use this file except in compliance with the Elastic License 2.0. + +package stackconfigpolicy + +import ( + "testing" + + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + commonv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/common/v1" + esv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/elasticsearch/v1" + kbv1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/kibana/v1" + policyv1alpha1 "github.com/elastic/cloud-on-k8s/v3/pkg/apis/stackconfigpolicy/v1alpha1" + "github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/operator" +) + +func Test_getStackPolicyConfigForElasticsearch(t *testing.T) { + for _, tc := range []struct { + name string + policyNamespace string + operatorNamespace string + targetElasticsearch *esv1.Elasticsearch + stackConfigPolicies []policyv1alpha1.StackConfigPolicy + expectedConfigPolicy policyv1alpha1.StackConfigPolicy + expectedSecretSources []commonv1.NamespacedSecretSource + expectedPolicyRefs map[string]struct{} + expectedMergeConflict bool + }{ + { + name: "merges without overwrites", + targetElasticsearch: &esv1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + Labels: map[string]string{ + "test": "test", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy1", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 1, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "test1.name": "policy1", + }}, + SecureSettings: []commonv1.SecretSource{ + { + SecretName: "test-secret-policy1", + Entries: []commonv1.KeyToPath{ + {Key: "test", Path: "/test-policy1"}, + }, + }, + }, + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "secret-policy1", MountPath: "/secret-policy1"}, + }, + SnapshotRepositories: &commonv1.Config{Data: map[string]any{ + "policy-backups.type": "fs", + "policy-backups": map[string]any{ + "settings.location": "/backups", + }, + }}, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy2", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: -1, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "test2.name": "policy2", + }}, + SecureSettings: []commonv1.SecretSource{ + { + SecretName: "test-secret-policy2", + Entries: []commonv1.KeyToPath{ + {Key: "test1", Path: "/test1-policy2"}, + {Key: "test2", Path: "/test2-policy2"}, + }, + }, + }, + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "secret-policy2", MountPath: "/secret-policy2"}, + }, + SnapshotRepositories: &commonv1.Config{Data: map[string]any{ + "policy-2-backups.type": "s3", + "policy-2-backups.settings": map[string]any{ + "bucket": "policy-2-backups", + "region": "us-west-2", + }, + }}, + }, + }, + }, + }, + expectedConfigPolicy: policyv1alpha1.StackConfigPolicy{ + Spec: policyv1alpha1.StackConfigPolicySpec{ + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "test1.name": "policy1", + "test2.name": "policy2", + }}, + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "secret-policy1", MountPath: "/secret-policy1"}, + {SecretName: "secret-policy2", MountPath: "/secret-policy2"}, + }, + SnapshotRepositories: &commonv1.Config{Data: map[string]any{ + "policy-2-backups.type": "s3", + "policy-2-backups.settings": map[string]any{ + "bucket": "policy-2-backups", + "region": "us-west-2", + }, + "policy-backups.type": "fs", + "policy-backups": map[string]any{ + "settings.location": "/backups", + }, + }}, + }, + }, + }, + expectedSecretSources: []commonv1.NamespacedSecretSource{ + { + SecretName: "test-secret-policy1", + Namespace: "test", + Entries: []commonv1.KeyToPath{ + {Key: "test", Path: "/test-policy1"}, + }, + }, + { + SecretName: "test-secret-policy2", + Namespace: "test", + Entries: []commonv1.KeyToPath{ + {Key: "test1", Path: "/test1-policy2"}, + {Key: "test2", Path: "/test2-policy2"}, + }, + }, + }, + expectedPolicyRefs: map[string]struct{}{ + "test/policy1": {}, + "test/policy2": {}, + }, + }, { + name: "merges with overwrites", + targetElasticsearch: &esv1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + Labels: map[string]string{ + "test": "test", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy1", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 1, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "test.name": "policy1", + }}, + SecureSettings: []commonv1.SecretSource{ + { + SecretName: "test", + Entries: []commonv1.KeyToPath{ + {Key: "test1", Path: "/test1-policy1"}, + {Key: "test2", Path: "/test2-policy1"}, + }, + }, + }, + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "secret-policy-1", MountPath: "/secret-policy1"}, + }, + SnapshotRepositories: &commonv1.Config{Data: map[string]any{ + "policy-2-backups.type": "fs", + "policy-2-backups.settings": map[string]any{ + "location": "/tmp/location", + }, + }}, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy2", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: -1, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "test.name": "policy2", + }}, + SecureSettings: []commonv1.SecretSource{ + { + SecretName: "test", + Entries: []commonv1.KeyToPath{ + {Key: "test1", Path: "/test1-policy2"}, + {Key: "test2", Path: "/test2-policy2"}, + }, + }, + }, + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "secret-policy-2", MountPath: "/secret-policy2"}, + }, + SnapshotRepositories: &commonv1.Config{Data: map[string]any{ + "policy-2-backups.type": "s3", + "policy-2-backups.settings": map[string]any{ + "bucket": "policy-2-backups", + "region": "us-west-2", + }, + }}, + }, + }, + }, + }, + expectedConfigPolicy: policyv1alpha1.StackConfigPolicy{ + Spec: policyv1alpha1.StackConfigPolicySpec{ + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "test.name": "policy2", + }}, + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "secret-policy-1", MountPath: "/secret-policy1"}, + {SecretName: "secret-policy-2", MountPath: "/secret-policy2"}, + }, + SnapshotRepositories: &commonv1.Config{Data: map[string]any{ + "policy-2-backups.type": "s3", + "policy-2-backups.settings": map[string]any{ + "bucket": "policy-2-backups", + "region": "us-west-2", + }, + }}, + }, + }, + }, + expectedSecretSources: []commonv1.NamespacedSecretSource{ + { + SecretName: "test", + Namespace: "test", + Entries: []commonv1.KeyToPath{ + {Key: "test1", Path: "/test1-policy1"}, + {Key: "test2", Path: "/test2-policy1"}, + }, + }, + { + SecretName: "test", + Namespace: "test", + Entries: []commonv1.KeyToPath{ + {Key: "test1", Path: "/test1-policy2"}, + {Key: "test2", Path: "/test2-policy2"}, + }, + }, + }, + expectedPolicyRefs: map[string]struct{}{ + "test/policy1": {}, + "test/policy2": {}, + }, + }, + { + name: "detects policies weight conflicts", + targetElasticsearch: &esv1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + Labels: map[string]string{ + "test": "test", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + // Policy with unique weight - should be merged + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy1", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 1, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "from": "policy1", + }}, + }, + }, + }, + // Two policies with the same weight - should conflict and be skipped + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy2-conflict", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 5, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "conflict": "policy2", + }}, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy3-conflict", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 5, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "conflict": "policy3", + }}, + }, + }, + }, + // Another policy with unique weight - should be merged + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy4", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 10, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "from": "policy4", + }}, + }, + }, + }, + }, + expectedMergeConflict: true, + }, + { + name: "detects conflicts when same secret defined in multiple policies", + targetElasticsearch: &esv1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + Labels: map[string]string{ + "test": "test", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + // Policy 1 with lower weight - should be merged first + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy1", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 1, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "from": "policy1", + }}, + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "shared-secret", MountPath: "/mnt/policy1"}, + }, + }, + }, + }, + // Policy 2 with higher weight - attempts to define the same secret, should conflict + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy2", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 5, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "from": "policy2", + }}, + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "shared-secret", MountPath: "/mnt/policy2"}, + }, + }, + }, + }, + }, + expectedMergeConflict: true, + }, + { + name: "detects conflicts when same mount path defined in multiple policies", + targetElasticsearch: &esv1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + Labels: map[string]string{ + "test": "test", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + // Policy 1 with lower weight - should be merged first + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy1", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 1, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "from": "policy1", + }}, + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "secret1", MountPath: "/mnt/shared"}, + }, + }, + }, + }, + // Policy 2 with higher weight - attempts to use the same mount path, should conflict + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy2", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 5, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "from": "policy2", + }}, + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "secret2", MountPath: "/mnt/shared"}, + }, + }, + }, + }, + }, + expectedMergeConflict: true, + }, + { + name: "successfully merges when different secrets use different mount paths", + targetElasticsearch: &esv1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "test", + Labels: map[string]string{ + "test": "test", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy1", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 1, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "db-creds", MountPath: "/etc/db"}, + {SecretName: "api-keys", MountPath: "/etc/api"}, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy2", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"test": "test"}, + }, + Weight: 5, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "tls-cert", MountPath: "/etc/tls"}, + {SecretName: "backup-creds", MountPath: "/etc/backup"}, + }, + }, + }, + }, + }, + expectedConfigPolicy: policyv1alpha1.StackConfigPolicy{ + Spec: policyv1alpha1.StackConfigPolicySpec{ + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + SecretMounts: []policyv1alpha1.SecretMount{ + {SecretName: "api-keys", MountPath: "/etc/api"}, + {SecretName: "backup-creds", MountPath: "/etc/backup"}, + {SecretName: "db-creds", MountPath: "/etc/db"}, + {SecretName: "tls-cert", MountPath: "/etc/tls"}, + }, + }, + }, + }, + expectedPolicyRefs: map[string]struct{}{ + "test/policy1": {}, + "test/policy2": {}, + }, + }, + { + name: "elasticsearch different namespace", + operatorNamespace: "operator-namespace", + targetElasticsearch: &esv1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-es", + Namespace: "es-namespace", + Labels: map[string]string{ + "env": "production", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + // Policy in wrong namespace - should not match + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "wrong-namespace", + Name: "policy-wrong-ns", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"env": "production"}, + }, + Weight: 1, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "should-not": "be-included", + }}, + }, + }, + }, + }, + expectedConfigPolicy: policyv1alpha1.StackConfigPolicy{ + Spec: policyv1alpha1.StackConfigPolicySpec{ + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{}, + }, + }, + expectedPolicyRefs: map[string]struct{}{}, + }, + { + name: "elasticsearch non-matching labels", + targetElasticsearch: &esv1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-es", + Namespace: "test", + Labels: map[string]string{ + "env": "production", + "team": "platform", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + // Policy with non-matching label selector - should not match + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy-wrong-labels", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "env": "development", // doesn't match ES labels + }, + }, + Weight: 1, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "should-not": "be-included", + }}, + }, + }, + }, + // Policy with partially matching labels - should not match + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "policy-partial-match", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "env": "production", + "team": "platform", + "missing": "label", // this label doesn't exist on ES + }, + }, + Weight: 2, + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{ + ClusterSettings: &commonv1.Config{Data: map[string]any{ + "also-should-not": "be-included", + }}, + }, + }, + }, + }, + expectedConfigPolicy: policyv1alpha1.StackConfigPolicy{ + Spec: policyv1alpha1.StackConfigPolicySpec{ + Elasticsearch: policyv1alpha1.ElasticsearchConfigPolicySpec{}, + }, + }, + expectedPolicyRefs: map[string]struct{}{}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + esConfigPolicy, err := getConfigPolicyForElasticsearch(tc.targetElasticsearch, tc.stackConfigPolicies, operator.Parameters{ + OperatorNamespace: tc.operatorNamespace, + }) + // Check for expected conflict errors + if tc.expectedMergeConflict { + assert.ErrorIs(t, err, errMergeConflict, "getConfigPolicyForElasticsearch should return an error") + return + } + assert.NoError(t, err, "getConfigPolicyForElasticsearch should not return an error") + + // Compare secret sources if expected + if tc.expectedSecretSources != nil { + assert.EqualValues(t, tc.expectedSecretSources, esConfigPolicy.SecretSources) + } + + if len(tc.stackConfigPolicies) > 1 { + canonicaliseElasticsearchPolicyConfig(t, &tc.expectedConfigPolicy.Spec.Elasticsearch) + } + assert.EqualValues(t, tc.expectedConfigPolicy.Spec.Elasticsearch, esConfigPolicy.Spec) + + // Compare policy references by building a map from the actual refs + actualPolicyRefs := make(map[string]struct{}) + for _, policy := range esConfigPolicy.PolicyRefs { + nsn := types.NamespacedName{Namespace: policy.Namespace, Name: policy.Name} + actualPolicyRefs[nsn.String()] = struct{}{} + } + assert.EqualValues(t, tc.expectedPolicyRefs, actualPolicyRefs) + }) + } +} + +func Test_getPolicyConfigForKibana(t *testing.T) { + for _, tc := range []struct { + name string + policyNamespace string + operatorNamespace string + targetKibana *kbv1.Kibana + stackConfigPolicies []policyv1alpha1.StackConfigPolicy + expectedConfigPolicy policyv1alpha1.StackConfigPolicy + expectedSecretSources []commonv1.NamespacedSecretSource + expectedPolicyRefs map[string]struct{} + expectedMergeConflict bool + }{ + { + name: "merges Kibana configs without overwrites", + targetKibana: &kbv1.Kibana{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-kb", + Namespace: "test", + Labels: map[string]string{ + "app": "kibana", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "kb-policy1", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "kibana"}, + }, + Weight: 10, + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "xpack.canvas.enabled": true, + "logging.root.level": "info", + }}, + SecureSettings: []commonv1.SecretSource{ + { + SecretName: "kb-secret1", + Entries: []commonv1.KeyToPath{{Key: "key1", Path: "path1"}}, + }, + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "kb-policy2", + ResourceVersion: "2", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "kibana"}, + }, + Weight: 20, + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "xpack.reporting.enabled": false, + "server.maxPayload": float64(2097152), + }}, + SecureSettings: []commonv1.SecretSource{ + { + SecretName: "kb-secret2", + Entries: []commonv1.KeyToPath{{Key: "key2", Path: "path2"}}, + }, + }, + }, + }, + }, + }, + expectedConfigPolicy: policyv1alpha1.StackConfigPolicy{ + Spec: policyv1alpha1.StackConfigPolicySpec{ + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "xpack": map[string]any{ + "canvas": map[string]any{ + "enabled": true, + }, + "reporting": map[string]any{ + "enabled": false, + }, + }, + "logging": map[string]any{ + "root": map[string]any{ + "level": "info", + }, + }, + "server": map[string]any{ + "maxPayload": float64(2097152), + }, + }}, + }, + }, + }, + expectedSecretSources: []commonv1.NamespacedSecretSource{ + { + SecretName: "kb-secret1", + Namespace: "test", + Entries: []commonv1.KeyToPath{{Key: "key1", Path: "path1"}}, + }, + { + SecretName: "kb-secret2", + Namespace: "test", + Entries: []commonv1.KeyToPath{{Key: "key2", Path: "path2"}}, + }, + }, + expectedPolicyRefs: map[string]struct{}{ + "test/kb-policy1": {}, + "test/kb-policy2": {}, + }, + }, + { + name: "merges Kibana configs with overwrites - higher weight wins", + targetKibana: &kbv1.Kibana{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-kb", + Namespace: "test", + Labels: map[string]string{ + "env": "prod", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "kb-low-priority", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"env": "prod"}, + }, + Weight: 10, + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "logging.root.level": "info", + "server.port": uint64(5601), + }}, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "kb-high-priority", + ResourceVersion: "2", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"env": "prod"}, + }, + Weight: 20, + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "logging.root.level": "debug", // Override from policy with weight 10 + }}, + }, + }, + }, + }, + expectedConfigPolicy: policyv1alpha1.StackConfigPolicy{ + Spec: policyv1alpha1.StackConfigPolicySpec{ + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "logging": map[string]any{ + "root": map[string]any{ + "level": "info", + }, + }, + "server": map[string]any{ + "port": uint64(5601), + }, + }}, + }, + }, + }, + expectedPolicyRefs: map[string]struct{}{ + "test/kb-low-priority": {}, + "test/kb-high-priority": {}, + }, + }, + { + name: "Kibana policies with same weight cause conflict", + targetKibana: &kbv1.Kibana{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-kb", + Namespace: "test", + Labels: map[string]string{ + "env": "staging", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "kb-conflict-1", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"env": "staging"}, + }, + Weight: 15, + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "xpack.canvas.enabled": true, + }}, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "kb-conflict-2", + ResourceVersion: "2", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"env": "staging"}, + }, + Weight: 15, // Same weight as kb-conflict-1 + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "xpack.reporting.enabled": false, + }}, + }, + }, + }, + }, + expectedMergeConflict: true, + }, + { + name: "Kibana policy doesn't match due to namespace", + targetKibana: &kbv1.Kibana{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-kb", + Namespace: "prod", + Labels: map[string]string{ + "app": "kibana", + }, + }, + }, + policyNamespace: "dev", + operatorNamespace: "elastic-system", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "dev", + Name: "kb-policy-wrong-ns", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "kibana"}, + }, + Weight: 10, + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "xpack.canvas.enabled": true, + }}, + }, + }, + }, + }, + expectedConfigPolicy: policyv1alpha1.StackConfigPolicy{ + Spec: policyv1alpha1.StackConfigPolicySpec{ + Kibana: policyv1alpha1.KibanaConfigPolicySpec{}, + }, + }, + expectedPolicyRefs: map[string]struct{}{}, + }, + { + name: "Kibana policy doesn't match due to labels", + targetKibana: &kbv1.Kibana{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-kb", + Namespace: "test", + Labels: map[string]string{ + "app": "kibana", + "env": "prod", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "kb-policy-wrong-labels", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "kibana", + "env": "dev", // Different value + }, + }, + Weight: 10, + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "xpack.canvas.enabled": true, + }}, + }, + }, + }, + }, + expectedConfigPolicy: policyv1alpha1.StackConfigPolicy{ + Spec: policyv1alpha1.StackConfigPolicySpec{ + Kibana: policyv1alpha1.KibanaConfigPolicySpec{}, + }, + }, + expectedPolicyRefs: map[string]struct{}{}, + }, + { + name: "Single Kibana policy - no merging optimization", + targetKibana: &kbv1.Kibana{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-kb", + Namespace: "test", + Labels: map[string]string{ + "app": "kibana", + }, + }, + }, + policyNamespace: "test", + stackConfigPolicies: []policyv1alpha1.StackConfigPolicy{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "kb-single-policy", + ResourceVersion: "1", + }, + Spec: policyv1alpha1.StackConfigPolicySpec{ + ResourceSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "kibana"}, + }, + Weight: 10, + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "xpack.canvas.enabled": true, + "logging.root.level": "info", + }}, + }, + }, + }, + }, + expectedConfigPolicy: policyv1alpha1.StackConfigPolicy{ + Spec: policyv1alpha1.StackConfigPolicySpec{ + Kibana: policyv1alpha1.KibanaConfigPolicySpec{ + Config: &commonv1.Config{Data: map[string]any{ + "xpack.canvas.enabled": true, + "logging.root.level": "info", + }}, + }, + }, + }, + expectedPolicyRefs: map[string]struct{}{ + "test/kb-single-policy": {}, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + kbPolicyConfig, err := getConfigPolicyForKibana(tc.targetKibana, tc.stackConfigPolicies, operator.Parameters{ + OperatorNamespace: tc.operatorNamespace, + }) + // Verify conflict errors + if tc.expectedMergeConflict { + assert.ErrorIs(t, err, errMergeConflict) + return + } + assert.NoError(t, err) + + // Compare secret sources if expected + if tc.expectedSecretSources != nil { + assert.EqualValues(t, tc.expectedSecretSources, kbPolicyConfig.SecretSources) + } + + if len(tc.stackConfigPolicies) > 1 { + canonicaliseKibanaPolicyConfig(t, &tc.expectedConfigPolicy.Spec.Kibana) + } + assert.EqualValues(t, tc.expectedConfigPolicy.Spec.Kibana, kbPolicyConfig.Spec) + + // Compare policy references + actualPolicyRefs := make(map[string]struct{}) + for _, policy := range kbPolicyConfig.PolicyRefs { + nsn := types.NamespacedName{Namespace: policy.Namespace, Name: policy.Name} + actualPolicyRefs[nsn.String()] = struct{}{} + } + assert.EqualValues(t, tc.expectedPolicyRefs, actualPolicyRefs) + }) + } +} + +func canonicaliseElasticsearchPolicyConfig(t *testing.T, spec *policyv1alpha1.ElasticsearchConfigPolicySpec) { + t.Helper() + var err error + spec.ClusterSettings, err = deepMergeConfig(spec.ClusterSettings, spec.ClusterSettings) + assert.NoError(t, err) + spec.SnapshotRepositories, err = mergeConfig(spec.SnapshotRepositories, spec.SnapshotRepositories) + assert.NoError(t, err) + spec.SnapshotLifecyclePolicies, err = deepMergeConfig(spec.SnapshotLifecyclePolicies, spec.SnapshotLifecyclePolicies) + assert.NoError(t, err) + spec.SecurityRoleMappings, err = deepMergeConfig(spec.SecurityRoleMappings, spec.SecurityRoleMappings) + assert.NoError(t, err) + spec.IndexLifecyclePolicies, err = deepMergeConfig(spec.IndexLifecyclePolicies, spec.IndexLifecyclePolicies) + assert.NoError(t, err) + spec.IngestPipelines, err = deepMergeConfig(spec.IngestPipelines, spec.IngestPipelines) + assert.NoError(t, err) + spec.IndexTemplates.ComposableIndexTemplates, err = deepMergeConfig(spec.IndexTemplates.ComposableIndexTemplates, spec.IndexTemplates.ComposableIndexTemplates) + assert.NoError(t, err) + spec.IndexTemplates.ComponentTemplates, err = deepMergeConfig(spec.IndexTemplates.ComponentTemplates, spec.IndexTemplates.ComposableIndexTemplates) + assert.NoError(t, err) + spec.Config, err = deepMergeConfig(spec.Config, spec.Config) + assert.NoError(t, err) +} + +func canonicaliseKibanaPolicyConfig(t *testing.T, spec *policyv1alpha1.KibanaConfigPolicySpec) { + t.Helper() + var err error + spec.Config, err = deepMergeConfig(spec.Config, spec.Config) + assert.NoError(t, err) +}