Skip to content

Add support for future flag to protect our service from being deploye…#1871

Open
mploski wants to merge 1 commit intofeature/database-controllersfrom
feature/enable-feature-flags
Open

Add support for future flag to protect our service from being deploye…#1871
mploski wants to merge 1 commit intofeature/database-controllersfrom
feature/enable-feature-flags

Conversation

@mploski
Copy link
Copy Markdown
Collaborator

@mploski mploski commented Apr 23, 2026

Description

Enables the PostgresController feature gate mechanism across all three Postgres CRs — PostgresCluster, PostgresClusterClass, and PostgresDatabase. When the gate is off (default, Alpha), the validation webhook rejects any attempt to create or update these resources, preventing them from being deployed to environments where the controller is not active. This protects production clusters from silently accumulating CRs that no controller will reconcile.

Key Changes

  • pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go — added PostgresController feature gate guard (was missing)
  • pkg/postgresql/database/adapter/webhook/postgresdatabase_validation.go — new validator for PostgresDatabase with feature gate guard, placed in its own package under database/adapter/webhook/
  • pkg/splunk/enterprise/validation/registry.go — fixed three bugs: duplicate map key (PostgresClusterClassGVR used twice), wrong GVR resource name (postgresdatabase → postgresdatabases), typo in function nam (ValidatePostgresDatabaaseUpdate);
  • wires PostgresDatabase into the webhook registry
  • pkg/postgresql/shared/testhelpers/featuregates.go — new shared EnableFeatureGate(gates ...featuregate.Feature) helper reusable across all postgres test packages
  • docs/FeatureGates.md — added PostgresController to the gates table; added testing guidance for gated validators

Testing and Verification

  • All three validators have unit tests covering valid/invalid inputs and an explicit TestValidate*FeatureGateDisabled test with t.Cleanup restoring gate state between tests
  • pkg/postgresql/cluster/adapter/webhook and pkg/postgresql/database/adapter/webhook test suites pass cleanly

ok github.com/splunk/splunk-operator/pkg/postgresql/cluster/adapter/webhook
ok github.com/splunk/splunk-operator/pkg/postgresql/database/adapter/webhook
ok github.com/splunk/splunk-operator/pkg/splunk/enterprise/validation

Related Issues

Jira tickets, GitHub issues, Support tickets...

PR Checklist

  • Code changes adhere to the project's coding standards.
  • Relevant unit and integration tests are included.
  • Documentation has been updated accordingly.
  • All tests pass locally.
  • The PR description follows the project's guidelines.

@github-actions
Copy link
Copy Markdown
Contributor

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contribution License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment with the exact sentence copied from below.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request

}

// ValidatePostgresDatabaseUpdate validates a PostgresDatabase on UPDATE.
func ValidatePostgresDatabaseUpdate(obj, oldObj *enterpriseApi.PostgresDatabase) field.ErrorList {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: if we only alias the method here, maybe It would be fine to limit the functionality to only one method with a more direct name? fe. IsPostgresEnabled. Or is there potential that the update/create validation will diverge in the future?

obj *enterpriseApi.PostgresDatabase
wantErrCount int
}{
{
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a possibility, that this test will expand in the future? What I mean, is the table driven pattern neccessary here? Or maybe the below "create(...)"(similar kind) test can be merged with this one to provide table driven n > 1 testcases scenario?


func TestValidatePostgresDatabaseCreateFeatureGateDisabled(t *testing.T) {
config.DefaultMutableFeatureGate.SetFromMap(map[string]bool{string(config.PostgresController): false})
t.Cleanup(func() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would It be worth It to have this enabled as a fixture before each testcases?
It's a redundancy, but ensures hermetic test development effort from our side and makes tests resilient and immune to human error (not cleaning up once and influencing n > 1 tests later).

Copy link
Copy Markdown

@M4KIF M4KIF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a few comments in tests and .../webhook/...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants