feat: Multi StackConfigPolicy per cluster #8793

tehbooom · 2025-08-18T18:45:14Z

This PR allows for a cluster to have multiple StackConfigPolicies (SCPs). Avoids conflicts of the same resources based on the weights of the policy with the lowest weight having the highest priority. If two policies have the same weight and the same resources it will fail and the phase will be changed to CONFLICT.

Here is a simple table showing what should pass and what should fail

Weight	Resource	Result
Same	Different	✅
Same	Same	❌
Different	Same	✅
Different	Different	✅

The following was tested and passed interchangeably.

---
# different weights same policy should pass with policy-1 winning
apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: cluster-policy-1
spec:
  weight: -1
  elasticsearch:
    snapshotRepositories:
      policy-1-backups:
        type: s3
        settings:
          bucket: "policy-1-backups"
          region: "us-west-2"
---
apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: cluster-policy-2
spec:
  weight: 11
  elasticsearch:
    snapshotRepositories:
      policy-1-backups:
        type: s3
        settings:
          bucket: "policy-2-backups"
          region: "us-east-1"
---
## Different weights different policies should pass
apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: cluster-policy-1
spec:
  weight: 10
  elasticsearch:
    snapshotRepositories:
      policy-1-backups:
        type: s3
        settings:
          bucket: "policy-1-backups"
          region: "us-west-2"
---
apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: cluster-policy-2
spec:
  weight: 11
  elasticsearch:
    snapshotRepositories:
      policy-2-backups:
        type: s3
        settings:
          bucket: "policy-2-backups"
          region: "us-east-1"
---
## Same weight different repositories should pass
apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: cluster-policy-1
spec:
  weight: 10
  elasticsearch:
    snapshotRepositories:
      policy-1-backups:
        type: s3
        settings:
          bucket: "policy-1-backups"
          region: "us-west-2"
---
apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: cluster-policy-2
spec:
  weight: 10
  elasticsearch:
    snapshotRepositories:
      policy-2-backups:
        type: s3
        settings:
          bucket: "policy-3-backups"
          region: "us-east-1"
---
# Policy 2 wins with telemetry.optIn set to true
apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: kibana-policy-1
spec:
  weight: 1
  kibana:
    config:
      telemetry.optIn: false
---
apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: kibana-policy-2
spec:
  weight: 0
  kibana:
    config:
      telemetry.optIn: true

The following will fail

---
# Same weight same policy must fail
apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: cluster-policy-1
spec:
  weight: 10
  elasticsearch:
    snapshotRepositories:
      policy-1-backups:
        type: s3
        settings:
          bucket: "policy-1-backups"
          region: "us-west-2"
---
apiVersion: stackconfigpolicy.k8s.elastic.co/v1alpha1
kind: StackConfigPolicy
metadata:
  name: cluster-policy-2
spec:
  weight: 10
  elasticsearch:
    snapshotRepositories:
      policy-1-backups:
        type: s3
        settings:
          bucket: "policy-2-backups"
          region: "us-east-1"

This was also tested with a ECK 3.1.0 then upgraded. Previous SCPs will have a default value of 0.

…e values are allowed

prodsecmachine · 2025-08-18T18:45:27Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Licenses	0	0	0	0	0 issues
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

github-actions · 2025-08-18T18:46:04Z

🔍 Preview links for changed docs

docs/reference/api-reference/main.md

Copilot

Pull Request Overview

This PR implements support for multiple StackConfigPolicies (SCPs) per cluster by introducing a weight-based priority system. Policies with lower weights take precedence over those with higher weights, and conflicts are detected when policies with the same weight attempt to configure the same resources.

Adds a weight field to StackConfigPolicy specs with default value 0
Implements conflict detection for policies with same weights targeting overlapping resources
Merges configurations from multiple policies based on weight priorities

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
pkg/apis/stackconfigpolicy/v1alpha1/stackconfigpolicy_types.go	Adds weight field to StackConfigPolicySpec
pkg/controller/stackconfigpolicy/weight_conflict_test.go	Comprehensive test suite for weight conflict detection logic
pkg/controller/stackconfigpolicy/controller.go	Core multi-policy support with conflict detection and policy merging
pkg/controller/stackconfigpolicy/elasticsearch_config_settings.go	Multi-policy versions of Elasticsearch configuration functions
pkg/controller/stackconfigpolicy/kibana_config_settings.go	Multi-policy versions of Kibana configuration functions
pkg/controller/elasticsearch/filesettings/secret.go	Multi-policy secret management with policy reference tracking
pkg/controller/elasticsearch/filesettings/file_settings.go	Policy merging logic for Elasticsearch file settings
pkg/controller/stackconfigpolicy/controller_test.go	Updates test expectations for multi-policy behavior
config/crds/v1/*.yaml	CRD updates to include weight field
docs/reference/api-reference/main.md	Documentation for new weight field

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

pkg/controller/stackconfigpolicy/elasticsearch_config_settings.go

pkg/controller/stackconfigpolicy/kibana_config_settings.go

pkg/controller/elasticsearch/filesettings/file_settings.go

pkg/controller/stackconfigpolicy/elasticsearch_config_settings.go

pkg/controller/stackconfigpolicy/kibana_config_settings.go

Copilot · 2025-08-22T08:14:41Z

pkg/controller/elasticsearch/filesettings/file_settings.go

+// For map-type values (like snapshot repositories), individual entries are merged rather than replaced
+func (s *Settings) mergeConfig(target, source *commonv1.Config) {
+	if source == nil || source.Data == nil {
+		return


The mergeConfig function modifies the target parameter but then creates a new Config object that is not assigned back to the original parameter. The line target = &commonv1.Config{Data: make(map[string]interface{})} only modifies the local variable, not the original pointer. This will cause the merge to fail silently.

Suggested change

return

func (s *Settings) mergeConfig(target, source *commonv1.Config) *commonv1.Config {

if source == nil || source.Data == nil {

return target

I think this is a valid copilot observation. Also I would expect unit tests for this kind of functionality. We do have more advanced merging through the CanonicalConfig abstraction in the code base. But there might be some gotchas with the more complex configuration objects that stack config policies support that might not be thinking of right now.

thbkrkr

Looks great, thank you! I like how this extends the original implementation to multiple SCPs without adding complexity. I didn’t do any testing on my side, but code-wise this looks good. 👍

pkg/controller/elasticsearch/filesettings/secret.go

pebrc · 2025-09-19T13:03:21Z

buildkite test this -f p=gke,t=TestStackConfigPolicy*

pebrc

I am generally positive about the approach but I feel like this needs more work, and test coverage. I did a quick smoke test and it does not work as advertised.

pebrc · 2025-09-19T13:21:52Z

test/e2e/es/stackconfigpolicy_test.go

+				},
+				ClusterSettings: &commonv1.Config{
+					Data: map[string]interface{}{
+						"persistent": map[string]interface{}{


this leads to an illegal setting "persistent"/"transient" are not supported via stack config policies

pebrc · 2025-09-19T13:32:14Z

pkg/controller/elasticsearch/filesettings/file_settings.go


+// mergeConfig merges source config into target config, with source taking precedence
+// For map-type values (like snapshot repositories), individual entries are merged rather than replaced
+func (s *Settings) mergeConfig(target, source *commonv1.Config) {


Why is this a receiver method of Settings?

pebrc · 2025-09-19T13:40:51Z

pkg/controller/elasticsearch/filesettings/file_settings.go

+	for key, value := range source.Data {
+		if targetValue, exists := target.Data[key]; exists {
+			if targetMap, targetIsMap := targetValue.(map[string]interface{}); targetIsMap {
+				if sourceMap, sourceIsMap := value.(map[string]interface{}); sourceIsMap {
+					for subKey, subValue := range sourceMap {
+						targetMap[subKey] = subValue
+					}
+					continue
+				}
+			}
+		}


This seems too simplistic given that for example cluster settings can both be in nested and flat syntax. Also it only merges one level deep?

pebrc · 2025-09-19T13:44:56Z

pkg/controller/elasticsearch/filesettings/file_settings.go

+// For map-type values (like snapshot repositories), individual entries are merged rather than replaced
+func (s *Settings) mergeConfig(target, source *commonv1.Config) {
+	if source == nil || source.Data == nil {
+		return


I think this is a valid copilot observation. Also I would expect unit tests for this kind of functionality. We do have more advanced merging through the CanonicalConfig abstraction in the code base. But there might be some gotchas with the more complex configuration objects that stack config policies support that might not be thinking of right now.

tehbooom · 2025-10-16T12:52:50Z

buildkite test this -f p=gke,t=TestStackConfigPolicy*

tehbooom added 5 commits August 18, 2025 12:12

feat: allow mulitple SCPs per cluster

d528988

fix: same weight different resources

e8a95dc

fix: revert comments back to original

706aca4

fix: Removed comment stating that 0 is highest priority since negativ…

f18b19e

…e values are allowed

fix: Update comments

e35848f

tehbooom added the >enhancement Enhancement of existing functionality label Aug 18, 2025

github-actions bot deployed to docs-preview August 18, 2025 18:45 View deployment

docs: Updated API comments

35d4caf

github-actions bot deployed to docs-preview August 18, 2025 19:17 View deployment

fix: linting

cc21222

github-actions bot deployed to docs-preview August 18, 2025 19:46 View deployment

fix: linting

a1b9c6c

github-actions bot deployed to docs-preview August 18, 2025 20:02 View deployment

fix: lint with golangci-lint v1.64.5

a2563fd

github-actions bot deployed to docs-preview August 19, 2025 12:39 View deployment

pebrc requested a review from Copilot August 22, 2025 08:11

Copilot AI reviewed Aug 22, 2025

View reviewed changes

thbkrkr reviewed Aug 26, 2025

View reviewed changes

pkg/controller/elasticsearch/filesettings/secret.go Show resolved Hide resolved

tehbooom added 3 commits August 28, 2025 13:45

fix: use sort.SliceStable for policies

8445011

fix: function naming

9ea2fae

test: add e2e tests for multi-scp

f7613d2

github-actions bot deployed to docs-preview August 28, 2025 17:46 View deployment

pebrc reviewed Sep 19, 2025

View reviewed changes

fix: Cluster settings use canonical config

4813911

github-actions bot deployed to docs-preview September 29, 2025 13:16 View deployment

fix: Flip weight priorities in scp tests

dc6a49c

github-actions bot deployed to docs-preview October 16, 2025 17:17 View deployment

-		return
+func (s *Settings) mergeConfig(target, source *commonv1.Config) *commonv1.Config {
+	if source == nil || source.Data == nil {
+		return target

feat: Multi StackConfigPolicy per cluster #8793

Are you sure you want to change the base?

feat: Multi StackConfigPolicy per cluster #8793

Uh oh!

Conversation

tehbooom commented Aug 18, 2025

Uh oh!

prodsecmachine commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

github-actions bot commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔍 Preview links for changed docs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

pebrc Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

thbkrkr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pebrc commented Sep 19, 2025

Uh oh!

pebrc left a comment

Choose a reason for hiding this comment

Uh oh!

pebrc Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

pebrc Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

pebrc Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

pebrc Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

tehbooom commented Oct 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

prodsecmachine commented Aug 18, 2025 •

edited

Loading

github-actions bot commented Aug 18, 2025 •

edited

Loading