From f27a666f3b0fd8812ab8fa2a12246dc9c84350b0 Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Mon, 20 Apr 2026 10:27:51 +0200 Subject: [PATCH 01/11] pg_hba validation --- pkg/postgresql/cluster/core/hba.go | 121 +++--- pkg/postgresql/cluster/core/hba_test.go | 356 ++++++++++++++++++ .../validation/postgrescluster_validation.go | 55 +++ .../postgrescluster_validation_test.go | 192 ++++++++++ .../postgresclusterclass_validation.go | 55 +++ .../postgresclusterclass_validation_test.go | 191 ++++++++++ pkg/splunk/enterprise/validation/registry.go | 17 +- 7 files changed, 914 insertions(+), 73 deletions(-) create mode 100644 pkg/postgresql/cluster/core/hba_test.go create mode 100644 pkg/splunk/enterprise/validation/postgrescluster_validation.go create mode 100644 pkg/splunk/enterprise/validation/postgrescluster_validation_test.go create mode 100644 pkg/splunk/enterprise/validation/postgresclusterclass_validation.go create mode 100644 pkg/splunk/enterprise/validation/postgresclusterclass_validation_test.go diff --git a/pkg/postgresql/cluster/core/hba.go b/pkg/postgresql/cluster/core/hba.go index 099597696..b22073723 100644 --- a/pkg/postgresql/cluster/core/hba.go +++ b/pkg/postgresql/cluster/core/hba.go @@ -33,20 +33,19 @@ var hbaConnectionTypes = map[string]bool{ } var hbaAuthMethods = map[string]bool{ - "trust": true, - "reject": true, - "scram-sha-256": true, - "md5": true, - "password": true, - "gss": true, - "sspi": true, - "ident": true, - "peer": true, - "pam": true, - "ldap": true, - "radius": true, - "cert": true, - "oauth": true, + "trust": true, + "reject": true, + "scram-sha-256": true, + "md5": true, + "password": true, + "gss": true, + "sspi": true, + "ident": true, + "peer": true, + "pam": true, + "ldap": true, + "radius": true, + "cert": true, } var hbaSpecialAddresses = map[string]bool{ @@ -56,38 +55,31 @@ var hbaSpecialAddresses = map[string]bool{ } // tokenPattern splits on whitespace while keeping double-quoted strings intact. +// Matches pgtoolkit's regex: (?:"+.*?"+|\S)+ var hbaTokenPattern = regexp.MustCompile(`(?:"+.*?"+|\S)+`) -// hbaLabelPattern matches a valid DNS label sequence (hostname or domain suffix). -var hbaLabelPattern = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*$`) - -// RuleError describes a validation error for a single pg_hba.conf rule. -type RuleError struct { - Index int - Message string -} +// hostnamePattern matches valid hostname characters. +var hbaHostnamePattern = regexp.MustCompile(`^[a-zA-Z0-9._-]+$`) // ValidateRules validates a slice of pg_hba.conf rule strings. -func ValidateRules(rules []string) []RuleError { - var errs []RuleError +// Returns nil if all rules are valid, or an error listing each invalid rule. +func ValidateRules(rules []string) error { + var errs []string for i, rule := range rules { - for _, msg := range validateRule(rule) { - errs = append(errs, RuleError{Index: i, Message: msg}) + if ruleErrs := validateRule(rule); len(ruleErrs) > 0 { + for _, e := range ruleErrs { + errs = append(errs, fmt.Sprintf("rule %d: %s", i+1, e)) + } } } - return errs + if len(errs) == 0 { + return nil + } + return fmt.Errorf("invalid pg_hba.conf rules:\n %s", strings.Join(errs, "\n ")) } -// validateRule validates a single pg_hba rule using positional parsing. -// pg_hba.conf has two formats: -// -// local DATABASE USER METHOD [OPTIONS] -// host* DATABASE USER ADDRESS METHOD [OPTIONS] -// host* DATABASE USER IP-ADDRESS NETMASK METHOD [OPTIONS] -// -// Validation order: connection type → minimum field count → auth method -// (at a fixed positional index) → address for host* types. The IP+netmask -// form is detected by checking whether tokens[4] parses as a valid IP. +// validateRule parses and validates a single pg_hba rule. +// Returns a list of validation errors (empty means valid). func validateRule(rule string) []string { trimmed := strings.TrimSpace(rule) if trimmed == "" { @@ -101,41 +93,46 @@ func validateRule(rule string) []string { var errs []string + // Layer 0: connection type connType := tokens[0] if !hbaConnectionTypes[connType] { return []string{fmt.Sprintf("unknown connection type %q", connType)} } - isLocal := connType == "local" - minFields := 5 // TYPE DATABASE USER ADDRESS METHOD - if isLocal { - minFields = 4 // local DATABASE USER METHOD - } - if len(tokens) < minFields { - return []string{fmt.Sprintf("too few fields: expected at least %d (%s DATABASE USER %sMETHOD), got %d", - minFields, connType, map[bool]string{true: "", false: "ADDRESS "}[isLocal], len(tokens))} + // Separate common fields from auth options (tokens containing "=") + var commonFields []string + for _, t := range tokens { + if !strings.Contains(t, "=") { + commonFields = append(commonFields, t) + } } - methodIdx := 3 // local: tokens[3] - if !isLocal { - if len(tokens) > 5 && net.ParseIP(tokens[4]) != nil { - methodIdx = 5 - } else { - methodIdx = 4 + // Layer 1: field count + isLocal := connType == "local" + if isLocal { + // local DATABASE USER METHOD + if len(commonFields) < 4 { + return []string{fmt.Sprintf("too few fields: expected at least 4 (local DATABASE USER METHOD), got %d", len(commonFields))} + } + } else { + // host DATABASE USER ADDRESS METHOD (or ADDRESS MASK METHOD) + if len(commonFields) < 5 { + return []string{fmt.Sprintf("too few fields: expected at least 5 (%s DATABASE USER ADDRESS METHOD), got %d", connType, len(commonFields))} } } - if methodIdx >= len(tokens) { - return []string{fmt.Sprintf("too few fields: missing auth method")} - } - method := tokens[methodIdx] + + // Layer 2: auth method (last common field) + method := commonFields[len(commonFields)-1] if !hbaAuthMethods[method] { errs = append(errs, fmt.Sprintf("unknown auth method %q", method)) } + // Layer 3: address validation (for non-local types) if !isLocal { - address := tokens[3] - if methodIdx == 5 { - if addrErr := validateIPNetmask(tokens[3], tokens[4]); addrErr != "" { + address := commonFields[3] + // 6+ common fields means IP + separate netmask format + if len(commonFields) >= 6 { + if addrErr := validateIPNetmask(commonFields[3], commonFields[4]); addrErr != "" { errs = append(errs, addrErr) } } else { @@ -187,11 +184,7 @@ func validateAddress(address string) string { // Domain suffix match: .example.com if strings.HasPrefix(address, ".") && len(address) > 1 { - suffix := address[1:] - if hbaLabelPattern.MatchString(suffix) { - return "" - } - return fmt.Sprintf("invalid domain suffix %q", address) + return "" } // CIDR notation @@ -208,7 +201,7 @@ func validateAddress(address string) string { } // Hostname - if hbaLabelPattern.MatchString(address) { + if hbaHostnamePattern.MatchString(address) { return "" } diff --git a/pkg/postgresql/cluster/core/hba_test.go b/pkg/postgresql/cluster/core/hba_test.go new file mode 100644 index 000000000..cbbbdc5e7 --- /dev/null +++ b/pkg/postgresql/cluster/core/hba_test.go @@ -0,0 +1,356 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package core + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateRules(t *testing.T) { + t.Run("nil slice returns nil", func(t *testing.T) { + assert.NoError(t, ValidateRules(nil)) + }) + + t.Run("empty slice returns nil", func(t *testing.T) { + assert.NoError(t, ValidateRules([]string{})) + }) + + t.Run("all valid rules returns nil", func(t *testing.T) { + rules := []string{ + "local all all trust", + "host all all 0.0.0.0/0 scram-sha-256", + "hostssl all all 192.168.1.0/24 md5", + } + assert.NoError(t, ValidateRules(rules)) + }) + + t.Run("mixed valid and invalid returns error with correct indices", func(t *testing.T) { + rules := []string{ + "host all all 0.0.0.0/0 scram-sha-256", + "hostx all all 0.0.0.0/0 md5", + "host all all 0.0.0.0/0 md5", + } + err := ValidateRules(rules) + require.Error(t, err) + assert.Contains(t, err.Error(), "rule 2:") + assert.NotContains(t, err.Error(), "rule 1:") + assert.NotContains(t, err.Error(), "rule 3:") + }) + + t.Run("multiple errors in different rules", func(t *testing.T) { + rules := []string{ + "hostx all all 0.0.0.0/0 md5", + "host all all 192.168.0.0/33 bogus", + } + err := ValidateRules(rules) + require.Error(t, err) + assert.Contains(t, err.Error(), "rule 1:") + assert.Contains(t, err.Error(), "rule 2:") + }) +} + +func TestValidateRule(t *testing.T) { + // === Valid rules === + + validRules := []struct { + name string + rule string + }{ + {"local basic", "local all all trust"}, + {"local with peer", "local postgres postgres peer"}, + {"host CIDR IPv4", "host all all 0.0.0.0/0 scram-sha-256"}, + {"hostssl CIDR", "hostssl all all 192.168.1.0/24 md5"}, + {"hostnossl reject", "hostnossl all all 0.0.0.0/0 reject"}, + {"hostgssenc", "hostgssenc all all 0.0.0.0/0 gss"}, + {"hostnogssenc", "hostnogssenc all all 0.0.0.0/0 scram-sha-256"}, + {"host replication", "host replication all 10.0.0.0/8 password"}, + {"host samehost", "host all all samehost trust"}, + {"host samenet", "host all all samenet trust"}, + {"host address all", "host all all all scram-sha-256"}, + {"host domain suffix", "host all all .example.com cert"}, + {"host IPv6", "host all all ::1/128 scram-sha-256"}, + {"host IPv6 all", "host all all ::0/0 md5"}, + {"host IP+netmask", "host all all 192.168.1.1 255.255.255.0 md5"}, + {"host IP+netmask /8", "host all all 10.0.0.0 255.0.0.0 md5"}, + {"inline comment", "host all all 0.0.0.0/0 md5 # office access"}, + {"inline comment with spaces", "host all all 0.0.0.0/0 md5 # allow all"}, + {"full-line comment", "# this is a comment"}, + {"comment-only with spaces", " # indented comment"}, + {"host auth options", "host all all 0.0.0.0/0 ldap ldapserver=ldap.example.com ldapport=389"}, + {"host quoted option", "host all all 0.0.0.0/0 ident map=omicron"}, + {"host quoted value", `host all all 0.0.0.0/0 ldap ldapprefix="cn="`}, + {"comma-separated db", "host db1,db2 all 0.0.0.0/0 md5"}, + {"comma-separated user", "host all user1,user2 0.0.0.0/0 md5"}, + {"host hostname", "host all all myhost.example.com md5"}, + {"host with sspi", "host all all 0.0.0.0/0 sspi"}, + {"host with ident", "host all all 0.0.0.0/0 ident"}, + {"host with pam", "host all all 0.0.0.0/0 pam"}, + {"host with radius", "host all all 0.0.0.0/0 radius"}, + {"empty string", ""}, + {"whitespace only", " "}, + } + + for _, tc := range validRules { + t.Run("valid/"+tc.name, func(t *testing.T) { + errs := validateRule(tc.rule) + assert.Empty(t, errs, "expected no errors for rule %q, got: %v", tc.rule, errs) + }) + } + + // === Layer 0: connection type errors === + + t.Run("layer0/unknown connection type", func(t *testing.T) { + errs := validateRule("hostx all all 0.0.0.0/0 md5") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], `unknown connection type "hostx"`) + }) + + t.Run("layer0/uppercase connection type", func(t *testing.T) { + errs := validateRule("HOST all all 0.0.0.0/0 md5") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], `unknown connection type "HOST"`) + }) + + // === Layer 1: field count errors === + + t.Run("layer1/host missing method", func(t *testing.T) { + errs := validateRule("host all all 0.0.0.0/0") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], "too few fields") + }) + + t.Run("layer1/host only three fields", func(t *testing.T) { + errs := validateRule("host all all") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], "too few fields") + }) + + t.Run("layer1/local missing user and method", func(t *testing.T) { + errs := validateRule("local all") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], "too few fields") + }) + + t.Run("layer1/local missing method", func(t *testing.T) { + errs := validateRule("local all all") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], "too few fields") + }) + + // === Layer 2: auth method errors === + + t.Run("layer2/unknown auth method", func(t *testing.T) { + errs := validateRule("host all all 0.0.0.0/0 bogus") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], `unknown auth method "bogus"`) + }) + + t.Run("layer2/typo scram-sha256", func(t *testing.T) { + errs := validateRule("host all all 0.0.0.0/0 scram-sha256") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], `unknown auth method "scram-sha256"`) + }) + + t.Run("layer2/local unknown method", func(t *testing.T) { + errs := validateRule("local all all unknown") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], `unknown auth method "unknown"`) + }) + + // === Layer 3: address errors === + + t.Run("layer3/invalid CIDR mask too large", func(t *testing.T) { + errs := validateRule("host all all 192.168.0.0/33 md5") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], "invalid CIDR") + }) + + t.Run("layer3/invalid IP in CIDR", func(t *testing.T) { + errs := validateRule("host all all 256.1.1.1/24 md5") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], "invalid CIDR") + }) + + t.Run("layer3/garbage address", func(t *testing.T) { + errs := validateRule("host all all not@valid md5") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], "invalid address") + }) + + // === Layer 3: netmask errors === + + t.Run("layer3/non-contiguous netmask", func(t *testing.T) { + errs := validateRule("host all all 10.0.0.1 255.0.255.0 md5") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], "not a contiguous subnet mask") + }) + + t.Run("layer3/invalid IP in netmask pair", func(t *testing.T) { + errs := validateRule("host all all 999.0.0.1 255.255.255.0 md5") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], "invalid IP address") + }) + + t.Run("layer3/garbage netmask", func(t *testing.T) { + errs := validateRule("host all all 10.0.0.1 notamask md5") + require.Len(t, errs, 1) + assert.Contains(t, errs[0], "invalid netmask") + }) + + // === Multiple errors in one rule === + + t.Run("multiple/bad method and bad address", func(t *testing.T) { + errs := validateRule("host all all 192.168.0.0/33 bogus") + assert.Len(t, errs, 2) + }) +} + +func TestTokenize(t *testing.T) { + t.Run("simple fields", func(t *testing.T) { + tokens := tokenize("host all all 0.0.0.0/0 md5") + assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "md5"}, tokens) + }) + + t.Run("multiple spaces", func(t *testing.T) { + tokens := tokenize("host all all 0.0.0.0/0 md5") + assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "md5"}, tokens) + }) + + t.Run("quoted string preserved", func(t *testing.T) { + tokens := tokenize(`host all all 0.0.0.0/0 ldap ldapprefix="cn="`) + assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "ldap", `ldapprefix="cn="`}, tokens) + }) + + t.Run("auth option with equals", func(t *testing.T) { + tokens := tokenize("host all all 0.0.0.0/0 ident map=omicron") + assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "ident", "map=omicron"}, tokens) + }) + + t.Run("empty string", func(t *testing.T) { + tokens := tokenize("") + assert.Empty(t, tokens) + }) + + t.Run("inline comment stripped", func(t *testing.T) { + tokens := tokenize("host all all 0.0.0.0/0 md5 # office access") + assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "md5"}, tokens) + }) + + t.Run("full-line comment", func(t *testing.T) { + tokens := tokenize("# this is a comment") + assert.Empty(t, tokens) + }) + + t.Run("hash inside quotes not treated as comment", func(t *testing.T) { + tokens := tokenize(`host all all 0.0.0.0/0 ldap ldapprefix="cn=#test"`) + assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "ldap", `ldapprefix="cn=#test"`}, tokens) + }) +} + +func TestStripComment(t *testing.T) { + t.Run("no comment", func(t *testing.T) { + assert.Equal(t, "host all all 0.0.0.0/0 md5", stripComment("host all all 0.0.0.0/0 md5")) + }) + + t.Run("inline comment", func(t *testing.T) { + assert.Equal(t, "host all all 0.0.0.0/0 md5 ", stripComment("host all all 0.0.0.0/0 md5 # comment")) + }) + + t.Run("full-line comment", func(t *testing.T) { + assert.Equal(t, "", stripComment("# full line comment")) + }) + + t.Run("hash inside quotes preserved", func(t *testing.T) { + assert.Equal(t, `host all all 0.0.0.0/0 ldap ldapprefix="cn=#x"`, stripComment(`host all all 0.0.0.0/0 ldap ldapprefix="cn=#x"`)) + }) + + t.Run("hash after closing quote", func(t *testing.T) { + assert.Equal(t, `host all all 0.0.0.0/0 ldap ldapprefix="cn" `, stripComment(`host all all 0.0.0.0/0 ldap ldapprefix="cn" # comment`)) + }) +} + +func TestValidateIPNetmask(t *testing.T) { + t.Run("valid IPv4", func(t *testing.T) { + assert.Empty(t, validateIPNetmask("192.168.1.0", "255.255.255.0")) + }) + + t.Run("valid /8", func(t *testing.T) { + assert.Empty(t, validateIPNetmask("10.0.0.0", "255.0.0.0")) + }) + + t.Run("invalid IP", func(t *testing.T) { + result := validateIPNetmask("999.0.0.1", "255.255.255.0") + assert.Contains(t, result, "invalid IP address") + }) + + t.Run("invalid mask not an IP", func(t *testing.T) { + result := validateIPNetmask("10.0.0.1", "notamask") + assert.Contains(t, result, "invalid netmask") + }) + + t.Run("non-contiguous mask", func(t *testing.T) { + result := validateIPNetmask("10.0.0.1", "255.0.255.0") + assert.Contains(t, result, "not a contiguous subnet mask") + }) +} + +func TestValidateAddress(t *testing.T) { + validAddresses := []string{ + "0.0.0.0/0", + "192.168.1.0/24", + "10.0.0.0/8", + "::1/128", + "::0/0", + "all", + "samehost", + "samenet", + ".example.com", + ".sub.domain.com", + "192.168.1.1", + "myhost.example.com", + "my-host", + "localhost", + } + + for _, addr := range validAddresses { + t.Run("valid/"+addr, func(t *testing.T) { + assert.Empty(t, validateAddress(addr)) + }) + } + + invalidAddresses := []struct { + name string + address string + errMsg string + }{ + {"CIDR mask too large", "192.168.0.0/33", "invalid CIDR"}, + {"invalid IP in CIDR", "256.1.1.1/24", "invalid CIDR"}, + {"bad CIDR format", "999.999.999.999/32", "invalid CIDR"}, + {"special chars", "host@name", "invalid address"}, + {"spaces in addr", "my host", "invalid address"}, + } + + for _, tc := range invalidAddresses { + t.Run("invalid/"+tc.name, func(t *testing.T) { + result := validateAddress(tc.address) + assert.Contains(t, result, tc.errMsg) + }) + } +} diff --git a/pkg/splunk/enterprise/validation/postgrescluster_validation.go b/pkg/splunk/enterprise/validation/postgrescluster_validation.go new file mode 100644 index 000000000..eb4e7353e --- /dev/null +++ b/pkg/splunk/enterprise/validation/postgrescluster_validation.go @@ -0,0 +1,55 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" + + enterpriseApi "github.com/splunk/splunk-operator/api/v4" + hba "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core" +) + +// ValidatePostgresClusterCreate validates a PostgresCluster on CREATE. +func ValidatePostgresClusterCreate(obj *enterpriseApi.PostgresCluster) field.ErrorList { + var allErrs field.ErrorList + + if len(obj.Spec.PgHBA) > 0 { + if err := hba.ValidateRules(obj.Spec.PgHBA); err != nil { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec").Child("pgHBA"), + obj.Spec.PgHBA, + err.Error())) + } + } + + return allErrs +} + +// ValidatePostgresClusterUpdate validates a PostgresCluster on UPDATE. +func ValidatePostgresClusterUpdate(obj, oldObj *enterpriseApi.PostgresCluster) field.ErrorList { + return ValidatePostgresClusterCreate(obj) +} + +// GetPostgresClusterWarningsOnCreate returns warnings for PostgresCluster CREATE. +func GetPostgresClusterWarningsOnCreate(obj *enterpriseApi.PostgresCluster) []string { + return nil +} + +// GetPostgresClusterWarningsOnUpdate returns warnings for PostgresCluster UPDATE. +func GetPostgresClusterWarningsOnUpdate(obj, oldObj *enterpriseApi.PostgresCluster) []string { + return nil +} diff --git a/pkg/splunk/enterprise/validation/postgrescluster_validation_test.go b/pkg/splunk/enterprise/validation/postgrescluster_validation_test.go new file mode 100644 index 000000000..684e2eb2f --- /dev/null +++ b/pkg/splunk/enterprise/validation/postgrescluster_validation_test.go @@ -0,0 +1,192 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "testing" + + enterpriseApi "github.com/splunk/splunk-operator/api/v4" + "github.com/stretchr/testify/assert" +) + +func TestValidatePostgresClusterCreate(t *testing.T) { + tests := []struct { + name string + obj *enterpriseApi.PostgresCluster + wantErrCount int + wantErrField string + }{ + { + name: "valid - no pgHBA rules", + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + }, + }, + wantErrCount: 0, + }, + { + name: "valid - empty pgHBA", + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{}, + }, + }, + wantErrCount: 0, + }, + { + name: "valid - correct pgHBA rules", + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "hostnossl all all 0.0.0.0/0 reject", + "hostssl all all 0.0.0.0/0 scram-sha-256", + }, + }, + }, + wantErrCount: 0, + }, + { + name: "invalid - bad connection type", + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "hostx all all 0.0.0.0/0 md5", + }, + }, + }, + wantErrCount: 1, + wantErrField: "spec.pgHBA", + }, + { + name: "invalid - bad CIDR", + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "host all all 192.168.0.0/33 md5", + }, + }, + }, + wantErrCount: 1, + wantErrField: "spec.pgHBA", + }, + { + name: "invalid - bad auth method", + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "host all all 0.0.0.0/0 bogus-auth", + }, + }, + }, + wantErrCount: 1, + wantErrField: "spec.pgHBA", + }, + { + name: "invalid - missing fields", + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "host all all", + }, + }, + }, + wantErrCount: 1, + wantErrField: "spec.pgHBA", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := ValidatePostgresClusterCreate(tt.obj) + assert.Len(t, errs, tt.wantErrCount, "unexpected error count") + if tt.wantErrField != "" && len(errs) > 0 { + assert.Equal(t, tt.wantErrField, errs[0].Field, "unexpected error field") + } + }) + } +} + +func TestValidatePostgresClusterUpdate(t *testing.T) { + tests := []struct { + name string + obj *enterpriseApi.PostgresCluster + oldObj *enterpriseApi.PostgresCluster + wantErrCount int + }{ + { + name: "valid update - add pgHBA rules", + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{"host all all 0.0.0.0/0 scram-sha-256"}, + }, + }, + oldObj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + }, + }, + wantErrCount: 0, + }, + { + name: "invalid update - bad pgHBA", + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{"hostx all all 0.0.0.0/0 md5"}, + }, + }, + oldObj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + }, + }, + wantErrCount: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := ValidatePostgresClusterUpdate(tt.obj, tt.oldObj) + assert.Len(t, errs, tt.wantErrCount, "unexpected error count") + }) + } +} + +func TestGetPostgresClusterWarningsOnCreate(t *testing.T) { + obj := &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "dev"}, + } + assert.Empty(t, GetPostgresClusterWarningsOnCreate(obj)) +} + +func TestGetPostgresClusterWarningsOnUpdate(t *testing.T) { + obj := &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "dev"}, + } + oldObj := &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "dev"}, + } + assert.Empty(t, GetPostgresClusterWarningsOnUpdate(obj, oldObj)) +} diff --git a/pkg/splunk/enterprise/validation/postgresclusterclass_validation.go b/pkg/splunk/enterprise/validation/postgresclusterclass_validation.go new file mode 100644 index 000000000..f6d87b702 --- /dev/null +++ b/pkg/splunk/enterprise/validation/postgresclusterclass_validation.go @@ -0,0 +1,55 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" + + enterpriseApi "github.com/splunk/splunk-operator/api/v4" + hba "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core" +) + +// ValidatePostgresClusterClassCreate validates a PostgresClusterClass on CREATE. +func ValidatePostgresClusterClassCreate(obj *enterpriseApi.PostgresClusterClass) field.ErrorList { + var allErrs field.ErrorList + + if obj.Spec.Config != nil && len(obj.Spec.Config.PgHBA) > 0 { + if err := hba.ValidateRules(obj.Spec.Config.PgHBA); err != nil { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec").Child("config").Child("pgHBA"), + obj.Spec.Config.PgHBA, + err.Error())) + } + } + + return allErrs +} + +// ValidatePostgresClusterClassUpdate validates a PostgresClusterClass on UPDATE. +func ValidatePostgresClusterClassUpdate(obj, oldObj *enterpriseApi.PostgresClusterClass) field.ErrorList { + return ValidatePostgresClusterClassCreate(obj) +} + +// GetPostgresClusterClassWarningsOnCreate returns warnings for PostgresClusterClass CREATE. +func GetPostgresClusterClassWarningsOnCreate(obj *enterpriseApi.PostgresClusterClass) []string { + return nil +} + +// GetPostgresClusterClassWarningsOnUpdate returns warnings for PostgresClusterClass UPDATE. +func GetPostgresClusterClassWarningsOnUpdate(obj, oldObj *enterpriseApi.PostgresClusterClass) []string { + return nil +} diff --git a/pkg/splunk/enterprise/validation/postgresclusterclass_validation_test.go b/pkg/splunk/enterprise/validation/postgresclusterclass_validation_test.go new file mode 100644 index 000000000..fbcc4025e --- /dev/null +++ b/pkg/splunk/enterprise/validation/postgresclusterclass_validation_test.go @@ -0,0 +1,191 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "testing" + + enterpriseApi "github.com/splunk/splunk-operator/api/v4" + "github.com/stretchr/testify/assert" +) + +func TestValidatePostgresClusterClassCreate(t *testing.T) { + tests := []struct { + name string + obj *enterpriseApi.PostgresClusterClass + wantErrCount int + wantErrField string + }{ + { + name: "valid - no config", + obj: &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + }, + }, + wantErrCount: 0, + }, + { + name: "valid - config without pgHBA", + obj: &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{}, + }, + }, + wantErrCount: 0, + }, + { + name: "valid - correct pgHBA rules", + obj: &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{ + "hostnossl all all 0.0.0.0/0 reject", + "hostssl all all 0.0.0.0/0 scram-sha-256", + }, + }, + }, + }, + wantErrCount: 0, + }, + { + name: "invalid - bad connection type", + obj: &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{ + "hostx all all 0.0.0.0/0 md5", + }, + }, + }, + }, + wantErrCount: 1, + wantErrField: "spec.config.pgHBA", + }, + { + name: "invalid - bad CIDR in class", + obj: &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{ + "host all all 256.1.1.1/24 md5", + }, + }, + }, + }, + wantErrCount: 1, + wantErrField: "spec.config.pgHBA", + }, + { + name: "invalid - unknown auth method in class", + obj: &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{ + "host all all 0.0.0.0/0 bogus", + }, + }, + }, + }, + wantErrCount: 1, + wantErrField: "spec.config.pgHBA", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := ValidatePostgresClusterClassCreate(tt.obj) + assert.Len(t, errs, tt.wantErrCount, "unexpected error count") + if tt.wantErrField != "" && len(errs) > 0 { + assert.Equal(t, tt.wantErrField, errs[0].Field, "unexpected error field") + } + }) + } +} + +func TestValidatePostgresClusterClassUpdate(t *testing.T) { + tests := []struct { + name string + obj *enterpriseApi.PostgresClusterClass + oldObj *enterpriseApi.PostgresClusterClass + wantErrCount int + }{ + { + name: "valid update", + obj: &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{"host all all 0.0.0.0/0 scram-sha-256"}, + }, + }, + }, + oldObj: &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + }, + }, + wantErrCount: 0, + }, + { + name: "invalid update - bad pgHBA", + obj: &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{"host all all 0.0.0.0/0 fake-method"}, + }, + }, + }, + oldObj: &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + }, + }, + wantErrCount: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + errs := ValidatePostgresClusterClassUpdate(tt.obj, tt.oldObj) + assert.Len(t, errs, tt.wantErrCount, "unexpected error count") + }) + } +} + +func TestGetPostgresClusterClassWarningsOnCreate(t *testing.T) { + obj := &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{Provisioner: "postgresql.cnpg.io"}, + } + assert.Empty(t, GetPostgresClusterClassWarningsOnCreate(obj)) +} + +func TestGetPostgresClusterClassWarningsOnUpdate(t *testing.T) { + obj := &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{Provisioner: "postgresql.cnpg.io"}, + } + oldObj := &enterpriseApi.PostgresClusterClass{ + Spec: enterpriseApi.PostgresClusterClassSpec{Provisioner: "postgresql.cnpg.io"}, + } + assert.Empty(t, GetPostgresClusterClassWarningsOnUpdate(obj, oldObj)) +} diff --git a/pkg/splunk/enterprise/validation/registry.go b/pkg/splunk/enterprise/validation/registry.go index 5eab98402..05169b42a 100644 --- a/pkg/splunk/enterprise/validation/registry.go +++ b/pkg/splunk/enterprise/validation/registry.go @@ -20,7 +20,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" enterpriseApi "github.com/splunk/splunk-operator/api/v4" - pgwebhook "github.com/splunk/splunk-operator/pkg/postgresql/cluster/adapter/webhook" ) // GVR constants for all Splunk Enterprise CRDs @@ -195,10 +194,10 @@ var DefaultValidators = map[schema.GroupVersionResource]Validator{ }, PostgresClusterGVR: &GenericValidator[*enterpriseApi.PostgresCluster]{ - ValidateCreateFunc: pgwebhook.ValidatePostgresClusterCreate, - ValidateUpdateFunc: pgwebhook.ValidatePostgresClusterUpdate, - WarningsOnCreateFunc: pgwebhook.GetPostgresClusterWarningsOnCreate, - WarningsOnUpdateFunc: pgwebhook.GetPostgresClusterWarningsOnUpdate, + ValidateCreateFunc: ValidatePostgresClusterCreate, + ValidateUpdateFunc: ValidatePostgresClusterUpdate, + WarningsOnCreateFunc: GetPostgresClusterWarningsOnCreate, + WarningsOnUpdateFunc: GetPostgresClusterWarningsOnUpdate, GroupKind: schema.GroupKind{ Group: "enterprise.splunk.com", Kind: "PostgresCluster", @@ -206,10 +205,10 @@ var DefaultValidators = map[schema.GroupVersionResource]Validator{ }, PostgresClusterClassGVR: &GenericValidator[*enterpriseApi.PostgresClusterClass]{ - ValidateCreateFunc: pgwebhook.ValidatePostgresClusterClassCreate, - ValidateUpdateFunc: pgwebhook.ValidatePostgresClusterClassUpdate, - WarningsOnCreateFunc: pgwebhook.GetPostgresClusterClassWarningsOnCreate, - WarningsOnUpdateFunc: pgwebhook.GetPostgresClusterClassWarningsOnUpdate, + ValidateCreateFunc: ValidatePostgresClusterClassCreate, + ValidateUpdateFunc: ValidatePostgresClusterClassUpdate, + WarningsOnCreateFunc: GetPostgresClusterClassWarningsOnCreate, + WarningsOnUpdateFunc: GetPostgresClusterClassWarningsOnUpdate, GroupKind: schema.GroupKind{ Group: "enterprise.splunk.com", Kind: "PostgresClusterClass", From 0580d68dfba0262063e90da604ec5f191394775a Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Mon, 20 Apr 2026 10:34:57 +0200 Subject: [PATCH 02/11] comment fix --- pkg/postgresql/cluster/core/hba.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/pkg/postgresql/cluster/core/hba.go b/pkg/postgresql/cluster/core/hba.go index b22073723..986311494 100644 --- a/pkg/postgresql/cluster/core/hba.go +++ b/pkg/postgresql/cluster/core/hba.go @@ -33,19 +33,19 @@ var hbaConnectionTypes = map[string]bool{ } var hbaAuthMethods = map[string]bool{ - "trust": true, - "reject": true, - "scram-sha-256": true, - "md5": true, - "password": true, - "gss": true, - "sspi": true, - "ident": true, - "peer": true, - "pam": true, - "ldap": true, - "radius": true, - "cert": true, + "trust": true, + "reject": true, + "scram-sha-256": true, + "md5": true, + "password": true, + "gss": true, + "sspi": true, + "ident": true, + "peer": true, + "pam": true, + "ldap": true, + "radius": true, + "cert": true, } var hbaSpecialAddresses = map[string]bool{ @@ -55,7 +55,6 @@ var hbaSpecialAddresses = map[string]bool{ } // tokenPattern splits on whitespace while keeping double-quoted strings intact. -// Matches pgtoolkit's regex: (?:"+.*?"+|\S)+ var hbaTokenPattern = regexp.MustCompile(`(?:"+.*?"+|\S)+`) // hostnamePattern matches valid hostname characters. From fd0611dec0b08b6fac41eddb187cdf79712c1e9f Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Tue, 21 Apr 2026 15:40:26 +0200 Subject: [PATCH 03/11] integration tests --- .../postgres_webhook_integration_test.go | 547 ++++++++++++++++++ 1 file changed, 547 insertions(+) create mode 100644 pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go diff --git a/pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go b/pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go new file mode 100644 index 000000000..9b15a0395 --- /dev/null +++ b/pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go @@ -0,0 +1,547 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + admissionv1 "k8s.io/api/admission/v1" + authenticationv1 "k8s.io/api/authentication/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + + enterpriseApi "github.com/splunk/splunk-operator/api/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newPostgresClusterAdmissionReview(uid string, op admissionv1.Operation, obj *enterpriseApi.PostgresCluster, oldObj *enterpriseApi.PostgresCluster) *admissionv1.AdmissionReview { + ar := &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "admission.k8s.io/v1", + Kind: "AdmissionReview", + }, + Request: &admissionv1.AdmissionRequest{ + UID: types.UID(uid), + Kind: metav1.GroupVersionKind{ + Group: "enterprise.splunk.com", + Version: "v4", + Kind: "PostgresCluster", + }, + Resource: metav1.GroupVersionResource{ + Group: "enterprise.splunk.com", + Version: "v4", + Resource: "postgresclusters", + }, + Name: obj.Name, + Namespace: obj.Namespace, + Operation: op, + Object: runtime.RawExtension{ + Raw: mustMarshal(obj), + }, + UserInfo: authenticationv1.UserInfo{Username: "test-user"}, + }, + } + if oldObj != nil { + ar.Request.OldObject = runtime.RawExtension{ + Raw: mustMarshal(oldObj), + } + } + return ar +} + +func newPostgresClusterClassAdmissionReview(uid string, op admissionv1.Operation, obj *enterpriseApi.PostgresClusterClass, oldObj *enterpriseApi.PostgresClusterClass) *admissionv1.AdmissionReview { + ar := &admissionv1.AdmissionReview{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "admission.k8s.io/v1", + Kind: "AdmissionReview", + }, + Request: &admissionv1.AdmissionRequest{ + UID: types.UID(uid), + Kind: metav1.GroupVersionKind{ + Group: "enterprise.splunk.com", + Version: "v4", + Kind: "PostgresClusterClass", + }, + Resource: metav1.GroupVersionResource{ + Group: "enterprise.splunk.com", + Version: "v4", + Resource: "postgresclusterclasses", + }, + Name: obj.Name, + Operation: op, + Object: runtime.RawExtension{ + Raw: mustMarshal(obj), + }, + UserInfo: authenticationv1.UserInfo{Username: "test-user"}, + }, + } + if oldObj != nil { + ar.Request.OldObject = runtime.RawExtension{ + Raw: mustMarshal(oldObj), + } + } + return ar +} + +func sendAdmissionReview(t *testing.T, server *WebhookServer, ar *admissionv1.AdmissionReview) *admissionv1.AdmissionResponse { + t.Helper() + body, err := json.Marshal(ar) + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodPost, "/validate", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + + server.handleValidate(rr, req) + require.Equal(t, http.StatusOK, rr.Code) + + var response admissionv1.AdmissionReview + require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &response)) + require.NotNil(t, response.Response) + return response.Response +} + +func TestPostgresClusterPgHBAIntegration(t *testing.T) { + server := NewWebhookServer(WebhookServerOptions{ + Port: 9443, + Validators: DefaultValidators, + }) + + tests := []struct { + name string + obj *enterpriseApi.PostgresCluster + wantAllowed bool + wantMessage string + wantMessages []string + }{ + { + name: "valid - no pgHBA rules", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresCluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + }, + }, + wantAllowed: true, + }, + { + name: "valid - correct pgHBA rules", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresCluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "hostnossl all all 0.0.0.0/0 reject", + "hostssl all all 0.0.0.0/0 scram-sha-256", + "local all all peer", + }, + }, + }, + wantAllowed: true, + }, + { + name: "rejected - bad connection type", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresCluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "hostx all all 0.0.0.0/0 md5", + }, + }, + }, + wantAllowed: false, + wantMessage: "unknown connection type", + }, + { + name: "rejected - bad CIDR", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresCluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "host all all 192.168.0.0/33 md5", + }, + }, + }, + wantAllowed: false, + wantMessage: "invalid CIDR", + }, + { + name: "rejected - unknown auth method", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresCluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "host all all 0.0.0.0/0 bogus", + }, + }, + }, + wantAllowed: false, + wantMessage: "unknown auth method", + }, + { + name: "rejected - too few fields", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresCluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "host all all", + }, + }, + }, + wantAllowed: false, + wantMessage: "too few fields", + }, + { + name: "rejected - multiple bad rules reports all errors", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresCluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "hostssl all all 0.0.0.0/0 scram-sha-256", + "hostx all all 0.0.0.0/0 md5", + "host all all 10.0.0.0/8 bogus", + }, + }, + }, + wantAllowed: false, + wantMessages: []string{"rule 2", "rule 3"}, + }, + { + name: "valid - rules with auth options and comments", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresCluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "host all all 0.0.0.0/0 ldap ldapserver=ldap.example.com ldapport=389", + "host all all 0.0.0.0/0 md5 # office access", + }, + }, + }, + wantAllowed: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ar := newPostgresClusterAdmissionReview("uid-"+tt.name, admissionv1.Create, tt.obj, nil) + resp := sendAdmissionReview(t, server, ar) + + assert.Equal(t, tt.wantAllowed, resp.Allowed, "unexpected admission result") + if tt.wantMessage != "" { + require.NotNil(t, resp.Result) + assert.Contains(t, resp.Result.Message, tt.wantMessage) + } + for _, msg := range tt.wantMessages { + require.NotNil(t, resp.Result) + assert.Contains(t, resp.Result.Message, msg) + } + }) + } +} + +func TestPostgresClusterPgHBAUpdateIntegration(t *testing.T) { + server := NewWebhookServer(WebhookServerOptions{ + Port: 9443, + Validators: DefaultValidators, + }) + + oldObj := &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresCluster", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: "default", + }, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "dev", + PgHBA: []string{ + "host all all 0.0.0.0/0 scram-sha-256", + }, + }, + } + + t.Run("valid update - change rules", func(t *testing.T) { + newObj := oldObj.DeepCopy() + newObj.Spec.PgHBA = []string{ + "hostssl all all 0.0.0.0/0 scram-sha-256", + "local all all peer", + } + ar := newPostgresClusterAdmissionReview("uid-update-valid", admissionv1.Update, newObj, oldObj) + resp := sendAdmissionReview(t, server, ar) + assert.True(t, resp.Allowed) + }) + + t.Run("rejected update - invalid new rules", func(t *testing.T) { + newObj := oldObj.DeepCopy() + newObj.Spec.PgHBA = []string{ + "hostx all all 0.0.0.0/0 md5", + } + ar := newPostgresClusterAdmissionReview("uid-update-invalid", admissionv1.Update, newObj, oldObj) + resp := sendAdmissionReview(t, server, ar) + assert.False(t, resp.Allowed) + assert.Contains(t, resp.Result.Message, "unknown connection type") + }) +} + +func TestPostgresClusterClassPgHBAIntegration(t *testing.T) { + server := NewWebhookServer(WebhookServerOptions{ + Port: 9443, + Validators: DefaultValidators, + }) + + tests := []struct { + name string + obj *enterpriseApi.PostgresClusterClass + wantAllowed bool + wantMessage string + }{ + { + name: "valid - no pgHBA rules", + obj: &enterpriseApi.PostgresClusterClass{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresClusterClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "dev", + }, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + }, + }, + wantAllowed: true, + }, + { + name: "valid - correct pgHBA rules", + obj: &enterpriseApi.PostgresClusterClass{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresClusterClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "dev", + }, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{ + "hostnossl all all 0.0.0.0/0 reject", + "hostssl all all 0.0.0.0/0 scram-sha-256", + }, + }, + }, + }, + wantAllowed: true, + }, + { + name: "rejected - bad connection type", + obj: &enterpriseApi.PostgresClusterClass{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresClusterClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "dev", + }, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{ + "hostx all all 0.0.0.0/0 md5", + }, + }, + }, + }, + wantAllowed: false, + wantMessage: "unknown connection type", + }, + { + name: "rejected - invalid CIDR in class", + obj: &enterpriseApi.PostgresClusterClass{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresClusterClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "dev", + }, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{ + "host all all 256.1.1.1/24 md5", + }, + }, + }, + }, + wantAllowed: false, + wantMessage: "invalid CIDR", + }, + { + name: "rejected - unknown auth method in class", + obj: &enterpriseApi.PostgresClusterClass{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresClusterClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "dev", + }, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{ + "host all all 0.0.0.0/0 fake-method", + }, + }, + }, + }, + wantAllowed: false, + wantMessage: "unknown auth method", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ar := newPostgresClusterClassAdmissionReview("uid-"+tt.name, admissionv1.Create, tt.obj, nil) + resp := sendAdmissionReview(t, server, ar) + + assert.Equal(t, tt.wantAllowed, resp.Allowed, "unexpected admission result") + if tt.wantMessage != "" { + require.NotNil(t, resp.Result) + assert.Contains(t, resp.Result.Message, tt.wantMessage) + } + }) + } +} + +func TestPostgresClusterClassPgHBAUpdateIntegration(t *testing.T) { + server := NewWebhookServer(WebhookServerOptions{ + Port: 9443, + Validators: DefaultValidators, + }) + + oldObj := &enterpriseApi.PostgresClusterClass{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "enterprise.splunk.com/v4", + Kind: "PostgresClusterClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "dev", + }, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + PgHBA: []string{ + "host all all 0.0.0.0/0 scram-sha-256", + }, + }, + }, + } + + t.Run("valid update - change rules", func(t *testing.T) { + newObj := oldObj.DeepCopy() + newObj.Spec.Config.PgHBA = []string{ + "hostssl all all 0.0.0.0/0 scram-sha-256", + "hostnossl all all 0.0.0.0/0 reject", + } + ar := newPostgresClusterClassAdmissionReview("uid-class-update-valid", admissionv1.Update, newObj, oldObj) + resp := sendAdmissionReview(t, server, ar) + assert.True(t, resp.Allowed) + }) + + t.Run("rejected update - invalid new rules", func(t *testing.T) { + newObj := oldObj.DeepCopy() + newObj.Spec.Config.PgHBA = []string{ + "host all all 0.0.0.0/0 bogus", + } + ar := newPostgresClusterClassAdmissionReview("uid-class-update-invalid", admissionv1.Update, newObj, oldObj) + resp := sendAdmissionReview(t, server, ar) + assert.False(t, resp.Allowed) + assert.Contains(t, resp.Result.Message, "unknown auth method") + }) +} From dd96323a595452b743d4d8875d7ff6c51ae7b7e8 Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Wed, 22 Apr 2026 09:14:13 +0200 Subject: [PATCH 04/11] fix tests --- .../postgres_webhook_integration_test.go | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go b/pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go index 9b15a0395..2690a80ca 100644 --- a/pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go +++ b/pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go @@ -34,7 +34,8 @@ import ( "github.com/stretchr/testify/require" ) -func newPostgresClusterAdmissionReview(uid string, op admissionv1.Operation, obj *enterpriseApi.PostgresCluster, oldObj *enterpriseApi.PostgresCluster) *admissionv1.AdmissionReview { +func newPostgresClusterAdmissionReview(t *testing.T, uid string, op admissionv1.Operation, obj *enterpriseApi.PostgresCluster, oldObj *enterpriseApi.PostgresCluster) *admissionv1.AdmissionReview { + t.Helper() ar := &admissionv1.AdmissionReview{ TypeMeta: metav1.TypeMeta{ APIVersion: "admission.k8s.io/v1", @@ -56,20 +57,21 @@ func newPostgresClusterAdmissionReview(uid string, op admissionv1.Operation, obj Namespace: obj.Namespace, Operation: op, Object: runtime.RawExtension{ - Raw: mustMarshal(obj), + Raw: mustMarshal(t, obj), }, UserInfo: authenticationv1.UserInfo{Username: "test-user"}, }, } if oldObj != nil { ar.Request.OldObject = runtime.RawExtension{ - Raw: mustMarshal(oldObj), + Raw: mustMarshal(t, oldObj), } } return ar } -func newPostgresClusterClassAdmissionReview(uid string, op admissionv1.Operation, obj *enterpriseApi.PostgresClusterClass, oldObj *enterpriseApi.PostgresClusterClass) *admissionv1.AdmissionReview { +func newPostgresClusterClassAdmissionReview(t *testing.T, uid string, op admissionv1.Operation, obj *enterpriseApi.PostgresClusterClass, oldObj *enterpriseApi.PostgresClusterClass) *admissionv1.AdmissionReview { + t.Helper() ar := &admissionv1.AdmissionReview{ TypeMeta: metav1.TypeMeta{ APIVersion: "admission.k8s.io/v1", @@ -90,14 +92,14 @@ func newPostgresClusterClassAdmissionReview(uid string, op admissionv1.Operation Name: obj.Name, Operation: op, Object: runtime.RawExtension{ - Raw: mustMarshal(obj), + Raw: mustMarshal(t, obj), }, UserInfo: authenticationv1.UserInfo{Username: "test-user"}, }, } if oldObj != nil { ar.Request.OldObject = runtime.RawExtension{ - Raw: mustMarshal(oldObj), + Raw: mustMarshal(t, oldObj), } } return ar @@ -305,7 +307,7 @@ func TestPostgresClusterPgHBAIntegration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ar := newPostgresClusterAdmissionReview("uid-"+tt.name, admissionv1.Create, tt.obj, nil) + ar := newPostgresClusterAdmissionReview(t, "uid-"+tt.name, admissionv1.Create, tt.obj, nil) resp := sendAdmissionReview(t, server, ar) assert.Equal(t, tt.wantAllowed, resp.Allowed, "unexpected admission result") @@ -350,7 +352,7 @@ func TestPostgresClusterPgHBAUpdateIntegration(t *testing.T) { "hostssl all all 0.0.0.0/0 scram-sha-256", "local all all peer", } - ar := newPostgresClusterAdmissionReview("uid-update-valid", admissionv1.Update, newObj, oldObj) + ar := newPostgresClusterAdmissionReview(t, "uid-update-valid", admissionv1.Update, newObj, oldObj) resp := sendAdmissionReview(t, server, ar) assert.True(t, resp.Allowed) }) @@ -360,7 +362,7 @@ func TestPostgresClusterPgHBAUpdateIntegration(t *testing.T) { newObj.Spec.PgHBA = []string{ "hostx all all 0.0.0.0/0 md5", } - ar := newPostgresClusterAdmissionReview("uid-update-invalid", admissionv1.Update, newObj, oldObj) + ar := newPostgresClusterAdmissionReview(t, "uid-update-invalid", admissionv1.Update, newObj, oldObj) resp := sendAdmissionReview(t, server, ar) assert.False(t, resp.Allowed) assert.Contains(t, resp.Result.Message, "unknown connection type") @@ -487,7 +489,7 @@ func TestPostgresClusterClassPgHBAIntegration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ar := newPostgresClusterClassAdmissionReview("uid-"+tt.name, admissionv1.Create, tt.obj, nil) + ar := newPostgresClusterClassAdmissionReview(t, "uid-"+tt.name, admissionv1.Create, tt.obj, nil) resp := sendAdmissionReview(t, server, ar) assert.Equal(t, tt.wantAllowed, resp.Allowed, "unexpected admission result") @@ -529,7 +531,7 @@ func TestPostgresClusterClassPgHBAUpdateIntegration(t *testing.T) { "hostssl all all 0.0.0.0/0 scram-sha-256", "hostnossl all all 0.0.0.0/0 reject", } - ar := newPostgresClusterClassAdmissionReview("uid-class-update-valid", admissionv1.Update, newObj, oldObj) + ar := newPostgresClusterClassAdmissionReview(t, "uid-class-update-valid", admissionv1.Update, newObj, oldObj) resp := sendAdmissionReview(t, server, ar) assert.True(t, resp.Allowed) }) @@ -539,7 +541,7 @@ func TestPostgresClusterClassPgHBAUpdateIntegration(t *testing.T) { newObj.Spec.Config.PgHBA = []string{ "host all all 0.0.0.0/0 bogus", } - ar := newPostgresClusterClassAdmissionReview("uid-class-update-invalid", admissionv1.Update, newObj, oldObj) + ar := newPostgresClusterClassAdmissionReview(t, "uid-class-update-invalid", admissionv1.Update, newObj, oldObj) resp := sendAdmissionReview(t, server, ar) assert.False(t, resp.Allowed) assert.Contains(t, resp.Result.Message, "unknown auth method") From 213ab5beec5f0aa209fd8b442d8c09ff363aa0ce Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Wed, 22 Apr 2026 09:37:01 +0200 Subject: [PATCH 05/11] structure fix --- .../postgres_webhook_integration_test.go | 549 ------------------ .../validation/postgrescluster_validation.go | 55 -- .../postgrescluster_validation_test.go | 192 ------ .../postgresclusterclass_validation.go | 55 -- .../postgresclusterclass_validation_test.go | 191 ------ pkg/splunk/enterprise/validation/registry.go | 17 +- 6 files changed, 9 insertions(+), 1050 deletions(-) delete mode 100644 pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go delete mode 100644 pkg/splunk/enterprise/validation/postgrescluster_validation.go delete mode 100644 pkg/splunk/enterprise/validation/postgrescluster_validation_test.go delete mode 100644 pkg/splunk/enterprise/validation/postgresclusterclass_validation.go delete mode 100644 pkg/splunk/enterprise/validation/postgresclusterclass_validation_test.go diff --git a/pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go b/pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go deleted file mode 100644 index 2690a80ca..000000000 --- a/pkg/splunk/enterprise/validation/postgres_webhook_integration_test.go +++ /dev/null @@ -1,549 +0,0 @@ -/* -Copyright 2026. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package validation - -import ( - "bytes" - "encoding/json" - "net/http" - "net/http/httptest" - "testing" - - admissionv1 "k8s.io/api/admission/v1" - authenticationv1 "k8s.io/api/authentication/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - - enterpriseApi "github.com/splunk/splunk-operator/api/v4" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func newPostgresClusterAdmissionReview(t *testing.T, uid string, op admissionv1.Operation, obj *enterpriseApi.PostgresCluster, oldObj *enterpriseApi.PostgresCluster) *admissionv1.AdmissionReview { - t.Helper() - ar := &admissionv1.AdmissionReview{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "admission.k8s.io/v1", - Kind: "AdmissionReview", - }, - Request: &admissionv1.AdmissionRequest{ - UID: types.UID(uid), - Kind: metav1.GroupVersionKind{ - Group: "enterprise.splunk.com", - Version: "v4", - Kind: "PostgresCluster", - }, - Resource: metav1.GroupVersionResource{ - Group: "enterprise.splunk.com", - Version: "v4", - Resource: "postgresclusters", - }, - Name: obj.Name, - Namespace: obj.Namespace, - Operation: op, - Object: runtime.RawExtension{ - Raw: mustMarshal(t, obj), - }, - UserInfo: authenticationv1.UserInfo{Username: "test-user"}, - }, - } - if oldObj != nil { - ar.Request.OldObject = runtime.RawExtension{ - Raw: mustMarshal(t, oldObj), - } - } - return ar -} - -func newPostgresClusterClassAdmissionReview(t *testing.T, uid string, op admissionv1.Operation, obj *enterpriseApi.PostgresClusterClass, oldObj *enterpriseApi.PostgresClusterClass) *admissionv1.AdmissionReview { - t.Helper() - ar := &admissionv1.AdmissionReview{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "admission.k8s.io/v1", - Kind: "AdmissionReview", - }, - Request: &admissionv1.AdmissionRequest{ - UID: types.UID(uid), - Kind: metav1.GroupVersionKind{ - Group: "enterprise.splunk.com", - Version: "v4", - Kind: "PostgresClusterClass", - }, - Resource: metav1.GroupVersionResource{ - Group: "enterprise.splunk.com", - Version: "v4", - Resource: "postgresclusterclasses", - }, - Name: obj.Name, - Operation: op, - Object: runtime.RawExtension{ - Raw: mustMarshal(t, obj), - }, - UserInfo: authenticationv1.UserInfo{Username: "test-user"}, - }, - } - if oldObj != nil { - ar.Request.OldObject = runtime.RawExtension{ - Raw: mustMarshal(t, oldObj), - } - } - return ar -} - -func sendAdmissionReview(t *testing.T, server *WebhookServer, ar *admissionv1.AdmissionReview) *admissionv1.AdmissionResponse { - t.Helper() - body, err := json.Marshal(ar) - require.NoError(t, err) - - req := httptest.NewRequest(http.MethodPost, "/validate", bytes.NewReader(body)) - req.Header.Set("Content-Type", "application/json") - rr := httptest.NewRecorder() - - server.handleValidate(rr, req) - require.Equal(t, http.StatusOK, rr.Code) - - var response admissionv1.AdmissionReview - require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &response)) - require.NotNil(t, response.Response) - return response.Response -} - -func TestPostgresClusterPgHBAIntegration(t *testing.T) { - server := NewWebhookServer(WebhookServerOptions{ - Port: 9443, - Validators: DefaultValidators, - }) - - tests := []struct { - name string - obj *enterpriseApi.PostgresCluster - wantAllowed bool - wantMessage string - wantMessages []string - }{ - { - name: "valid - no pgHBA rules", - obj: &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - }, - }, - wantAllowed: true, - }, - { - name: "valid - correct pgHBA rules", - obj: &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "hostnossl all all 0.0.0.0/0 reject", - "hostssl all all 0.0.0.0/0 scram-sha-256", - "local all all peer", - }, - }, - }, - wantAllowed: true, - }, - { - name: "rejected - bad connection type", - obj: &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "hostx all all 0.0.0.0/0 md5", - }, - }, - }, - wantAllowed: false, - wantMessage: "unknown connection type", - }, - { - name: "rejected - bad CIDR", - obj: &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "host all all 192.168.0.0/33 md5", - }, - }, - }, - wantAllowed: false, - wantMessage: "invalid CIDR", - }, - { - name: "rejected - unknown auth method", - obj: &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "host all all 0.0.0.0/0 bogus", - }, - }, - }, - wantAllowed: false, - wantMessage: "unknown auth method", - }, - { - name: "rejected - too few fields", - obj: &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "host all all", - }, - }, - }, - wantAllowed: false, - wantMessage: "too few fields", - }, - { - name: "rejected - multiple bad rules reports all errors", - obj: &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "hostssl all all 0.0.0.0/0 scram-sha-256", - "hostx all all 0.0.0.0/0 md5", - "host all all 10.0.0.0/8 bogus", - }, - }, - }, - wantAllowed: false, - wantMessages: []string{"rule 2", "rule 3"}, - }, - { - name: "valid - rules with auth options and comments", - obj: &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "host all all 0.0.0.0/0 ldap ldapserver=ldap.example.com ldapport=389", - "host all all 0.0.0.0/0 md5 # office access", - }, - }, - }, - wantAllowed: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ar := newPostgresClusterAdmissionReview(t, "uid-"+tt.name, admissionv1.Create, tt.obj, nil) - resp := sendAdmissionReview(t, server, ar) - - assert.Equal(t, tt.wantAllowed, resp.Allowed, "unexpected admission result") - if tt.wantMessage != "" { - require.NotNil(t, resp.Result) - assert.Contains(t, resp.Result.Message, tt.wantMessage) - } - for _, msg := range tt.wantMessages { - require.NotNil(t, resp.Result) - assert.Contains(t, resp.Result.Message, msg) - } - }) - } -} - -func TestPostgresClusterPgHBAUpdateIntegration(t *testing.T) { - server := NewWebhookServer(WebhookServerOptions{ - Port: 9443, - Validators: DefaultValidators, - }) - - oldObj := &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresCluster", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster", - Namespace: "default", - }, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "host all all 0.0.0.0/0 scram-sha-256", - }, - }, - } - - t.Run("valid update - change rules", func(t *testing.T) { - newObj := oldObj.DeepCopy() - newObj.Spec.PgHBA = []string{ - "hostssl all all 0.0.0.0/0 scram-sha-256", - "local all all peer", - } - ar := newPostgresClusterAdmissionReview(t, "uid-update-valid", admissionv1.Update, newObj, oldObj) - resp := sendAdmissionReview(t, server, ar) - assert.True(t, resp.Allowed) - }) - - t.Run("rejected update - invalid new rules", func(t *testing.T) { - newObj := oldObj.DeepCopy() - newObj.Spec.PgHBA = []string{ - "hostx all all 0.0.0.0/0 md5", - } - ar := newPostgresClusterAdmissionReview(t, "uid-update-invalid", admissionv1.Update, newObj, oldObj) - resp := sendAdmissionReview(t, server, ar) - assert.False(t, resp.Allowed) - assert.Contains(t, resp.Result.Message, "unknown connection type") - }) -} - -func TestPostgresClusterClassPgHBAIntegration(t *testing.T) { - server := NewWebhookServer(WebhookServerOptions{ - Port: 9443, - Validators: DefaultValidators, - }) - - tests := []struct { - name string - obj *enterpriseApi.PostgresClusterClass - wantAllowed bool - wantMessage string - }{ - { - name: "valid - no pgHBA rules", - obj: &enterpriseApi.PostgresClusterClass{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresClusterClass", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "dev", - }, - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - }, - }, - wantAllowed: true, - }, - { - name: "valid - correct pgHBA rules", - obj: &enterpriseApi.PostgresClusterClass{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresClusterClass", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "dev", - }, - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{ - "hostnossl all all 0.0.0.0/0 reject", - "hostssl all all 0.0.0.0/0 scram-sha-256", - }, - }, - }, - }, - wantAllowed: true, - }, - { - name: "rejected - bad connection type", - obj: &enterpriseApi.PostgresClusterClass{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresClusterClass", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "dev", - }, - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{ - "hostx all all 0.0.0.0/0 md5", - }, - }, - }, - }, - wantAllowed: false, - wantMessage: "unknown connection type", - }, - { - name: "rejected - invalid CIDR in class", - obj: &enterpriseApi.PostgresClusterClass{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresClusterClass", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "dev", - }, - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{ - "host all all 256.1.1.1/24 md5", - }, - }, - }, - }, - wantAllowed: false, - wantMessage: "invalid CIDR", - }, - { - name: "rejected - unknown auth method in class", - obj: &enterpriseApi.PostgresClusterClass{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresClusterClass", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "dev", - }, - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{ - "host all all 0.0.0.0/0 fake-method", - }, - }, - }, - }, - wantAllowed: false, - wantMessage: "unknown auth method", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ar := newPostgresClusterClassAdmissionReview(t, "uid-"+tt.name, admissionv1.Create, tt.obj, nil) - resp := sendAdmissionReview(t, server, ar) - - assert.Equal(t, tt.wantAllowed, resp.Allowed, "unexpected admission result") - if tt.wantMessage != "" { - require.NotNil(t, resp.Result) - assert.Contains(t, resp.Result.Message, tt.wantMessage) - } - }) - } -} - -func TestPostgresClusterClassPgHBAUpdateIntegration(t *testing.T) { - server := NewWebhookServer(WebhookServerOptions{ - Port: 9443, - Validators: DefaultValidators, - }) - - oldObj := &enterpriseApi.PostgresClusterClass{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "enterprise.splunk.com/v4", - Kind: "PostgresClusterClass", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "dev", - }, - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{ - "host all all 0.0.0.0/0 scram-sha-256", - }, - }, - }, - } - - t.Run("valid update - change rules", func(t *testing.T) { - newObj := oldObj.DeepCopy() - newObj.Spec.Config.PgHBA = []string{ - "hostssl all all 0.0.0.0/0 scram-sha-256", - "hostnossl all all 0.0.0.0/0 reject", - } - ar := newPostgresClusterClassAdmissionReview(t, "uid-class-update-valid", admissionv1.Update, newObj, oldObj) - resp := sendAdmissionReview(t, server, ar) - assert.True(t, resp.Allowed) - }) - - t.Run("rejected update - invalid new rules", func(t *testing.T) { - newObj := oldObj.DeepCopy() - newObj.Spec.Config.PgHBA = []string{ - "host all all 0.0.0.0/0 bogus", - } - ar := newPostgresClusterClassAdmissionReview(t, "uid-class-update-invalid", admissionv1.Update, newObj, oldObj) - resp := sendAdmissionReview(t, server, ar) - assert.False(t, resp.Allowed) - assert.Contains(t, resp.Result.Message, "unknown auth method") - }) -} diff --git a/pkg/splunk/enterprise/validation/postgrescluster_validation.go b/pkg/splunk/enterprise/validation/postgrescluster_validation.go deleted file mode 100644 index eb4e7353e..000000000 --- a/pkg/splunk/enterprise/validation/postgrescluster_validation.go +++ /dev/null @@ -1,55 +0,0 @@ -/* -Copyright 2026. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package validation - -import ( - "k8s.io/apimachinery/pkg/util/validation/field" - - enterpriseApi "github.com/splunk/splunk-operator/api/v4" - hba "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core" -) - -// ValidatePostgresClusterCreate validates a PostgresCluster on CREATE. -func ValidatePostgresClusterCreate(obj *enterpriseApi.PostgresCluster) field.ErrorList { - var allErrs field.ErrorList - - if len(obj.Spec.PgHBA) > 0 { - if err := hba.ValidateRules(obj.Spec.PgHBA); err != nil { - allErrs = append(allErrs, field.Invalid( - field.NewPath("spec").Child("pgHBA"), - obj.Spec.PgHBA, - err.Error())) - } - } - - return allErrs -} - -// ValidatePostgresClusterUpdate validates a PostgresCluster on UPDATE. -func ValidatePostgresClusterUpdate(obj, oldObj *enterpriseApi.PostgresCluster) field.ErrorList { - return ValidatePostgresClusterCreate(obj) -} - -// GetPostgresClusterWarningsOnCreate returns warnings for PostgresCluster CREATE. -func GetPostgresClusterWarningsOnCreate(obj *enterpriseApi.PostgresCluster) []string { - return nil -} - -// GetPostgresClusterWarningsOnUpdate returns warnings for PostgresCluster UPDATE. -func GetPostgresClusterWarningsOnUpdate(obj, oldObj *enterpriseApi.PostgresCluster) []string { - return nil -} diff --git a/pkg/splunk/enterprise/validation/postgrescluster_validation_test.go b/pkg/splunk/enterprise/validation/postgrescluster_validation_test.go deleted file mode 100644 index 684e2eb2f..000000000 --- a/pkg/splunk/enterprise/validation/postgrescluster_validation_test.go +++ /dev/null @@ -1,192 +0,0 @@ -/* -Copyright 2026. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package validation - -import ( - "testing" - - enterpriseApi "github.com/splunk/splunk-operator/api/v4" - "github.com/stretchr/testify/assert" -) - -func TestValidatePostgresClusterCreate(t *testing.T) { - tests := []struct { - name string - obj *enterpriseApi.PostgresCluster - wantErrCount int - wantErrField string - }{ - { - name: "valid - no pgHBA rules", - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - }, - }, - wantErrCount: 0, - }, - { - name: "valid - empty pgHBA", - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{}, - }, - }, - wantErrCount: 0, - }, - { - name: "valid - correct pgHBA rules", - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "hostnossl all all 0.0.0.0/0 reject", - "hostssl all all 0.0.0.0/0 scram-sha-256", - }, - }, - }, - wantErrCount: 0, - }, - { - name: "invalid - bad connection type", - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "hostx all all 0.0.0.0/0 md5", - }, - }, - }, - wantErrCount: 1, - wantErrField: "spec.pgHBA", - }, - { - name: "invalid - bad CIDR", - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "host all all 192.168.0.0/33 md5", - }, - }, - }, - wantErrCount: 1, - wantErrField: "spec.pgHBA", - }, - { - name: "invalid - bad auth method", - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "host all all 0.0.0.0/0 bogus-auth", - }, - }, - }, - wantErrCount: 1, - wantErrField: "spec.pgHBA", - }, - { - name: "invalid - missing fields", - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{ - "host all all", - }, - }, - }, - wantErrCount: 1, - wantErrField: "spec.pgHBA", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - errs := ValidatePostgresClusterCreate(tt.obj) - assert.Len(t, errs, tt.wantErrCount, "unexpected error count") - if tt.wantErrField != "" && len(errs) > 0 { - assert.Equal(t, tt.wantErrField, errs[0].Field, "unexpected error field") - } - }) - } -} - -func TestValidatePostgresClusterUpdate(t *testing.T) { - tests := []struct { - name string - obj *enterpriseApi.PostgresCluster - oldObj *enterpriseApi.PostgresCluster - wantErrCount int - }{ - { - name: "valid update - add pgHBA rules", - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{"host all all 0.0.0.0/0 scram-sha-256"}, - }, - }, - oldObj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - }, - }, - wantErrCount: 0, - }, - { - name: "invalid update - bad pgHBA", - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - PgHBA: []string{"hostx all all 0.0.0.0/0 md5"}, - }, - }, - oldObj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "dev", - }, - }, - wantErrCount: 1, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - errs := ValidatePostgresClusterUpdate(tt.obj, tt.oldObj) - assert.Len(t, errs, tt.wantErrCount, "unexpected error count") - }) - } -} - -func TestGetPostgresClusterWarningsOnCreate(t *testing.T) { - obj := &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{Class: "dev"}, - } - assert.Empty(t, GetPostgresClusterWarningsOnCreate(obj)) -} - -func TestGetPostgresClusterWarningsOnUpdate(t *testing.T) { - obj := &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{Class: "dev"}, - } - oldObj := &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{Class: "dev"}, - } - assert.Empty(t, GetPostgresClusterWarningsOnUpdate(obj, oldObj)) -} diff --git a/pkg/splunk/enterprise/validation/postgresclusterclass_validation.go b/pkg/splunk/enterprise/validation/postgresclusterclass_validation.go deleted file mode 100644 index f6d87b702..000000000 --- a/pkg/splunk/enterprise/validation/postgresclusterclass_validation.go +++ /dev/null @@ -1,55 +0,0 @@ -/* -Copyright 2026. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package validation - -import ( - "k8s.io/apimachinery/pkg/util/validation/field" - - enterpriseApi "github.com/splunk/splunk-operator/api/v4" - hba "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core" -) - -// ValidatePostgresClusterClassCreate validates a PostgresClusterClass on CREATE. -func ValidatePostgresClusterClassCreate(obj *enterpriseApi.PostgresClusterClass) field.ErrorList { - var allErrs field.ErrorList - - if obj.Spec.Config != nil && len(obj.Spec.Config.PgHBA) > 0 { - if err := hba.ValidateRules(obj.Spec.Config.PgHBA); err != nil { - allErrs = append(allErrs, field.Invalid( - field.NewPath("spec").Child("config").Child("pgHBA"), - obj.Spec.Config.PgHBA, - err.Error())) - } - } - - return allErrs -} - -// ValidatePostgresClusterClassUpdate validates a PostgresClusterClass on UPDATE. -func ValidatePostgresClusterClassUpdate(obj, oldObj *enterpriseApi.PostgresClusterClass) field.ErrorList { - return ValidatePostgresClusterClassCreate(obj) -} - -// GetPostgresClusterClassWarningsOnCreate returns warnings for PostgresClusterClass CREATE. -func GetPostgresClusterClassWarningsOnCreate(obj *enterpriseApi.PostgresClusterClass) []string { - return nil -} - -// GetPostgresClusterClassWarningsOnUpdate returns warnings for PostgresClusterClass UPDATE. -func GetPostgresClusterClassWarningsOnUpdate(obj, oldObj *enterpriseApi.PostgresClusterClass) []string { - return nil -} diff --git a/pkg/splunk/enterprise/validation/postgresclusterclass_validation_test.go b/pkg/splunk/enterprise/validation/postgresclusterclass_validation_test.go deleted file mode 100644 index fbcc4025e..000000000 --- a/pkg/splunk/enterprise/validation/postgresclusterclass_validation_test.go +++ /dev/null @@ -1,191 +0,0 @@ -/* -Copyright 2026. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package validation - -import ( - "testing" - - enterpriseApi "github.com/splunk/splunk-operator/api/v4" - "github.com/stretchr/testify/assert" -) - -func TestValidatePostgresClusterClassCreate(t *testing.T) { - tests := []struct { - name string - obj *enterpriseApi.PostgresClusterClass - wantErrCount int - wantErrField string - }{ - { - name: "valid - no config", - obj: &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - }, - }, - wantErrCount: 0, - }, - { - name: "valid - config without pgHBA", - obj: &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{}, - }, - }, - wantErrCount: 0, - }, - { - name: "valid - correct pgHBA rules", - obj: &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{ - "hostnossl all all 0.0.0.0/0 reject", - "hostssl all all 0.0.0.0/0 scram-sha-256", - }, - }, - }, - }, - wantErrCount: 0, - }, - { - name: "invalid - bad connection type", - obj: &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{ - "hostx all all 0.0.0.0/0 md5", - }, - }, - }, - }, - wantErrCount: 1, - wantErrField: "spec.config.pgHBA", - }, - { - name: "invalid - bad CIDR in class", - obj: &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{ - "host all all 256.1.1.1/24 md5", - }, - }, - }, - }, - wantErrCount: 1, - wantErrField: "spec.config.pgHBA", - }, - { - name: "invalid - unknown auth method in class", - obj: &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{ - "host all all 0.0.0.0/0 bogus", - }, - }, - }, - }, - wantErrCount: 1, - wantErrField: "spec.config.pgHBA", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - errs := ValidatePostgresClusterClassCreate(tt.obj) - assert.Len(t, errs, tt.wantErrCount, "unexpected error count") - if tt.wantErrField != "" && len(errs) > 0 { - assert.Equal(t, tt.wantErrField, errs[0].Field, "unexpected error field") - } - }) - } -} - -func TestValidatePostgresClusterClassUpdate(t *testing.T) { - tests := []struct { - name string - obj *enterpriseApi.PostgresClusterClass - oldObj *enterpriseApi.PostgresClusterClass - wantErrCount int - }{ - { - name: "valid update", - obj: &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{"host all all 0.0.0.0/0 scram-sha-256"}, - }, - }, - }, - oldObj: &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - }, - }, - wantErrCount: 0, - }, - { - name: "invalid update - bad pgHBA", - obj: &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - Config: &enterpriseApi.PostgresClusterClassConfig{ - PgHBA: []string{"host all all 0.0.0.0/0 fake-method"}, - }, - }, - }, - oldObj: &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{ - Provisioner: "postgresql.cnpg.io", - }, - }, - wantErrCount: 1, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - errs := ValidatePostgresClusterClassUpdate(tt.obj, tt.oldObj) - assert.Len(t, errs, tt.wantErrCount, "unexpected error count") - }) - } -} - -func TestGetPostgresClusterClassWarningsOnCreate(t *testing.T) { - obj := &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{Provisioner: "postgresql.cnpg.io"}, - } - assert.Empty(t, GetPostgresClusterClassWarningsOnCreate(obj)) -} - -func TestGetPostgresClusterClassWarningsOnUpdate(t *testing.T) { - obj := &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{Provisioner: "postgresql.cnpg.io"}, - } - oldObj := &enterpriseApi.PostgresClusterClass{ - Spec: enterpriseApi.PostgresClusterClassSpec{Provisioner: "postgresql.cnpg.io"}, - } - assert.Empty(t, GetPostgresClusterClassWarningsOnUpdate(obj, oldObj)) -} diff --git a/pkg/splunk/enterprise/validation/registry.go b/pkg/splunk/enterprise/validation/registry.go index 05169b42a..5eab98402 100644 --- a/pkg/splunk/enterprise/validation/registry.go +++ b/pkg/splunk/enterprise/validation/registry.go @@ -20,6 +20,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" enterpriseApi "github.com/splunk/splunk-operator/api/v4" + pgwebhook "github.com/splunk/splunk-operator/pkg/postgresql/cluster/adapter/webhook" ) // GVR constants for all Splunk Enterprise CRDs @@ -194,10 +195,10 @@ var DefaultValidators = map[schema.GroupVersionResource]Validator{ }, PostgresClusterGVR: &GenericValidator[*enterpriseApi.PostgresCluster]{ - ValidateCreateFunc: ValidatePostgresClusterCreate, - ValidateUpdateFunc: ValidatePostgresClusterUpdate, - WarningsOnCreateFunc: GetPostgresClusterWarningsOnCreate, - WarningsOnUpdateFunc: GetPostgresClusterWarningsOnUpdate, + ValidateCreateFunc: pgwebhook.ValidatePostgresClusterCreate, + ValidateUpdateFunc: pgwebhook.ValidatePostgresClusterUpdate, + WarningsOnCreateFunc: pgwebhook.GetPostgresClusterWarningsOnCreate, + WarningsOnUpdateFunc: pgwebhook.GetPostgresClusterWarningsOnUpdate, GroupKind: schema.GroupKind{ Group: "enterprise.splunk.com", Kind: "PostgresCluster", @@ -205,10 +206,10 @@ var DefaultValidators = map[schema.GroupVersionResource]Validator{ }, PostgresClusterClassGVR: &GenericValidator[*enterpriseApi.PostgresClusterClass]{ - ValidateCreateFunc: ValidatePostgresClusterClassCreate, - ValidateUpdateFunc: ValidatePostgresClusterClassUpdate, - WarningsOnCreateFunc: GetPostgresClusterClassWarningsOnCreate, - WarningsOnUpdateFunc: GetPostgresClusterClassWarningsOnUpdate, + ValidateCreateFunc: pgwebhook.ValidatePostgresClusterClassCreate, + ValidateUpdateFunc: pgwebhook.ValidatePostgresClusterClassUpdate, + WarningsOnCreateFunc: pgwebhook.GetPostgresClusterClassWarningsOnCreate, + WarningsOnUpdateFunc: pgwebhook.GetPostgresClusterClassWarningsOnUpdate, GroupKind: schema.GroupKind{ Group: "enterprise.splunk.com", Kind: "PostgresClusterClass", From d7e6ab38ab4744bf1bb09fc33c396e4d7dbaf795 Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Wed, 22 Apr 2026 17:52:09 +0200 Subject: [PATCH 06/11] fix tests --- pkg/postgresql/cluster/core/hba.go | 81 +++--- pkg/postgresql/cluster/core/hba_test.go | 356 ------------------------ 2 files changed, 41 insertions(+), 396 deletions(-) delete mode 100644 pkg/postgresql/cluster/core/hba_test.go diff --git a/pkg/postgresql/cluster/core/hba.go b/pkg/postgresql/cluster/core/hba.go index 986311494..4a59d147e 100644 --- a/pkg/postgresql/cluster/core/hba.go +++ b/pkg/postgresql/cluster/core/hba.go @@ -46,6 +46,7 @@ var hbaAuthMethods = map[string]bool{ "ldap": true, "radius": true, "cert": true, + "oauth": true, } var hbaSpecialAddresses = map[string]bool{ @@ -57,24 +58,24 @@ var hbaSpecialAddresses = map[string]bool{ // tokenPattern splits on whitespace while keeping double-quoted strings intact. var hbaTokenPattern = regexp.MustCompile(`(?:"+.*?"+|\S)+`) -// hostnamePattern matches valid hostname characters. -var hbaHostnamePattern = regexp.MustCompile(`^[a-zA-Z0-9._-]+$`) +// hbaLabelPattern matches a valid DNS label sequence (hostname or domain suffix). +var hbaLabelPattern = regexp.MustCompile(`^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?)*$`) + +// RuleError describes a validation error for a single pg_hba.conf rule. +type RuleError struct { + Index int + Message string +} // ValidateRules validates a slice of pg_hba.conf rule strings. -// Returns nil if all rules are valid, or an error listing each invalid rule. -func ValidateRules(rules []string) error { - var errs []string +func ValidateRules(rules []string) []RuleError { + var errs []RuleError for i, rule := range rules { - if ruleErrs := validateRule(rule); len(ruleErrs) > 0 { - for _, e := range ruleErrs { - errs = append(errs, fmt.Sprintf("rule %d: %s", i+1, e)) - } + for _, msg := range validateRule(rule) { + errs = append(errs, RuleError{Index: i, Message: msg}) } } - if len(errs) == 0 { - return nil - } - return fmt.Errorf("invalid pg_hba.conf rules:\n %s", strings.Join(errs, "\n ")) + return errs } // validateRule parses and validates a single pg_hba rule. @@ -98,40 +99,36 @@ func validateRule(rule string) []string { return []string{fmt.Sprintf("unknown connection type %q", connType)} } - // Separate common fields from auth options (tokens containing "=") - var commonFields []string - for _, t := range tokens { - if !strings.Contains(t, "=") { - commonFields = append(commonFields, t) - } - } - - // Layer 1: field count isLocal := connType == "local" + minFields := 5 // TYPE DATABASE USER ADDRESS METHOD if isLocal { - // local DATABASE USER METHOD - if len(commonFields) < 4 { - return []string{fmt.Sprintf("too few fields: expected at least 4 (local DATABASE USER METHOD), got %d", len(commonFields))} - } - } else { - // host DATABASE USER ADDRESS METHOD (or ADDRESS MASK METHOD) - if len(commonFields) < 5 { - return []string{fmt.Sprintf("too few fields: expected at least 5 (%s DATABASE USER ADDRESS METHOD), got %d", connType, len(commonFields))} - } + minFields = 4 // local DATABASE USER METHOD + } + if len(tokens) < minFields { + return []string{fmt.Sprintf("too few fields: expected at least %d (%s DATABASE USER %sMETHOD), got %d", + minFields, connType, map[bool]string{true: "", false: "ADDRESS "}[isLocal], len(tokens))} } - // Layer 2: auth method (last common field) - method := commonFields[len(commonFields)-1] + methodIdx := 3 // local: tokens[3] + if !isLocal { + if len(tokens) > 5 && net.ParseIP(tokens[4]) != nil { + methodIdx = 5 + } else { + methodIdx = 4 + } + } + if methodIdx >= len(tokens) { + return []string{fmt.Sprintf("too few fields: missing auth method")} + } + method := tokens[methodIdx] if !hbaAuthMethods[method] { errs = append(errs, fmt.Sprintf("unknown auth method %q", method)) } - // Layer 3: address validation (for non-local types) if !isLocal { - address := commonFields[3] - // 6+ common fields means IP + separate netmask format - if len(commonFields) >= 6 { - if addrErr := validateIPNetmask(commonFields[3], commonFields[4]); addrErr != "" { + address := tokens[3] + if methodIdx == 5 { + if addrErr := validateIPNetmask(tokens[3], tokens[4]); addrErr != "" { errs = append(errs, addrErr) } } else { @@ -183,7 +180,11 @@ func validateAddress(address string) string { // Domain suffix match: .example.com if strings.HasPrefix(address, ".") && len(address) > 1 { - return "" + suffix := address[1:] + if hbaLabelPattern.MatchString(suffix) { + return "" + } + return fmt.Sprintf("invalid domain suffix %q", address) } // CIDR notation @@ -200,7 +201,7 @@ func validateAddress(address string) string { } // Hostname - if hbaHostnamePattern.MatchString(address) { + if hbaLabelPattern.MatchString(address) { return "" } diff --git a/pkg/postgresql/cluster/core/hba_test.go b/pkg/postgresql/cluster/core/hba_test.go deleted file mode 100644 index cbbbdc5e7..000000000 --- a/pkg/postgresql/cluster/core/hba_test.go +++ /dev/null @@ -1,356 +0,0 @@ -/* -Copyright 2026. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package core - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestValidateRules(t *testing.T) { - t.Run("nil slice returns nil", func(t *testing.T) { - assert.NoError(t, ValidateRules(nil)) - }) - - t.Run("empty slice returns nil", func(t *testing.T) { - assert.NoError(t, ValidateRules([]string{})) - }) - - t.Run("all valid rules returns nil", func(t *testing.T) { - rules := []string{ - "local all all trust", - "host all all 0.0.0.0/0 scram-sha-256", - "hostssl all all 192.168.1.0/24 md5", - } - assert.NoError(t, ValidateRules(rules)) - }) - - t.Run("mixed valid and invalid returns error with correct indices", func(t *testing.T) { - rules := []string{ - "host all all 0.0.0.0/0 scram-sha-256", - "hostx all all 0.0.0.0/0 md5", - "host all all 0.0.0.0/0 md5", - } - err := ValidateRules(rules) - require.Error(t, err) - assert.Contains(t, err.Error(), "rule 2:") - assert.NotContains(t, err.Error(), "rule 1:") - assert.NotContains(t, err.Error(), "rule 3:") - }) - - t.Run("multiple errors in different rules", func(t *testing.T) { - rules := []string{ - "hostx all all 0.0.0.0/0 md5", - "host all all 192.168.0.0/33 bogus", - } - err := ValidateRules(rules) - require.Error(t, err) - assert.Contains(t, err.Error(), "rule 1:") - assert.Contains(t, err.Error(), "rule 2:") - }) -} - -func TestValidateRule(t *testing.T) { - // === Valid rules === - - validRules := []struct { - name string - rule string - }{ - {"local basic", "local all all trust"}, - {"local with peer", "local postgres postgres peer"}, - {"host CIDR IPv4", "host all all 0.0.0.0/0 scram-sha-256"}, - {"hostssl CIDR", "hostssl all all 192.168.1.0/24 md5"}, - {"hostnossl reject", "hostnossl all all 0.0.0.0/0 reject"}, - {"hostgssenc", "hostgssenc all all 0.0.0.0/0 gss"}, - {"hostnogssenc", "hostnogssenc all all 0.0.0.0/0 scram-sha-256"}, - {"host replication", "host replication all 10.0.0.0/8 password"}, - {"host samehost", "host all all samehost trust"}, - {"host samenet", "host all all samenet trust"}, - {"host address all", "host all all all scram-sha-256"}, - {"host domain suffix", "host all all .example.com cert"}, - {"host IPv6", "host all all ::1/128 scram-sha-256"}, - {"host IPv6 all", "host all all ::0/0 md5"}, - {"host IP+netmask", "host all all 192.168.1.1 255.255.255.0 md5"}, - {"host IP+netmask /8", "host all all 10.0.0.0 255.0.0.0 md5"}, - {"inline comment", "host all all 0.0.0.0/0 md5 # office access"}, - {"inline comment with spaces", "host all all 0.0.0.0/0 md5 # allow all"}, - {"full-line comment", "# this is a comment"}, - {"comment-only with spaces", " # indented comment"}, - {"host auth options", "host all all 0.0.0.0/0 ldap ldapserver=ldap.example.com ldapport=389"}, - {"host quoted option", "host all all 0.0.0.0/0 ident map=omicron"}, - {"host quoted value", `host all all 0.0.0.0/0 ldap ldapprefix="cn="`}, - {"comma-separated db", "host db1,db2 all 0.0.0.0/0 md5"}, - {"comma-separated user", "host all user1,user2 0.0.0.0/0 md5"}, - {"host hostname", "host all all myhost.example.com md5"}, - {"host with sspi", "host all all 0.0.0.0/0 sspi"}, - {"host with ident", "host all all 0.0.0.0/0 ident"}, - {"host with pam", "host all all 0.0.0.0/0 pam"}, - {"host with radius", "host all all 0.0.0.0/0 radius"}, - {"empty string", ""}, - {"whitespace only", " "}, - } - - for _, tc := range validRules { - t.Run("valid/"+tc.name, func(t *testing.T) { - errs := validateRule(tc.rule) - assert.Empty(t, errs, "expected no errors for rule %q, got: %v", tc.rule, errs) - }) - } - - // === Layer 0: connection type errors === - - t.Run("layer0/unknown connection type", func(t *testing.T) { - errs := validateRule("hostx all all 0.0.0.0/0 md5") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], `unknown connection type "hostx"`) - }) - - t.Run("layer0/uppercase connection type", func(t *testing.T) { - errs := validateRule("HOST all all 0.0.0.0/0 md5") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], `unknown connection type "HOST"`) - }) - - // === Layer 1: field count errors === - - t.Run("layer1/host missing method", func(t *testing.T) { - errs := validateRule("host all all 0.0.0.0/0") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], "too few fields") - }) - - t.Run("layer1/host only three fields", func(t *testing.T) { - errs := validateRule("host all all") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], "too few fields") - }) - - t.Run("layer1/local missing user and method", func(t *testing.T) { - errs := validateRule("local all") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], "too few fields") - }) - - t.Run("layer1/local missing method", func(t *testing.T) { - errs := validateRule("local all all") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], "too few fields") - }) - - // === Layer 2: auth method errors === - - t.Run("layer2/unknown auth method", func(t *testing.T) { - errs := validateRule("host all all 0.0.0.0/0 bogus") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], `unknown auth method "bogus"`) - }) - - t.Run("layer2/typo scram-sha256", func(t *testing.T) { - errs := validateRule("host all all 0.0.0.0/0 scram-sha256") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], `unknown auth method "scram-sha256"`) - }) - - t.Run("layer2/local unknown method", func(t *testing.T) { - errs := validateRule("local all all unknown") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], `unknown auth method "unknown"`) - }) - - // === Layer 3: address errors === - - t.Run("layer3/invalid CIDR mask too large", func(t *testing.T) { - errs := validateRule("host all all 192.168.0.0/33 md5") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], "invalid CIDR") - }) - - t.Run("layer3/invalid IP in CIDR", func(t *testing.T) { - errs := validateRule("host all all 256.1.1.1/24 md5") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], "invalid CIDR") - }) - - t.Run("layer3/garbage address", func(t *testing.T) { - errs := validateRule("host all all not@valid md5") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], "invalid address") - }) - - // === Layer 3: netmask errors === - - t.Run("layer3/non-contiguous netmask", func(t *testing.T) { - errs := validateRule("host all all 10.0.0.1 255.0.255.0 md5") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], "not a contiguous subnet mask") - }) - - t.Run("layer3/invalid IP in netmask pair", func(t *testing.T) { - errs := validateRule("host all all 999.0.0.1 255.255.255.0 md5") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], "invalid IP address") - }) - - t.Run("layer3/garbage netmask", func(t *testing.T) { - errs := validateRule("host all all 10.0.0.1 notamask md5") - require.Len(t, errs, 1) - assert.Contains(t, errs[0], "invalid netmask") - }) - - // === Multiple errors in one rule === - - t.Run("multiple/bad method and bad address", func(t *testing.T) { - errs := validateRule("host all all 192.168.0.0/33 bogus") - assert.Len(t, errs, 2) - }) -} - -func TestTokenize(t *testing.T) { - t.Run("simple fields", func(t *testing.T) { - tokens := tokenize("host all all 0.0.0.0/0 md5") - assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "md5"}, tokens) - }) - - t.Run("multiple spaces", func(t *testing.T) { - tokens := tokenize("host all all 0.0.0.0/0 md5") - assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "md5"}, tokens) - }) - - t.Run("quoted string preserved", func(t *testing.T) { - tokens := tokenize(`host all all 0.0.0.0/0 ldap ldapprefix="cn="`) - assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "ldap", `ldapprefix="cn="`}, tokens) - }) - - t.Run("auth option with equals", func(t *testing.T) { - tokens := tokenize("host all all 0.0.0.0/0 ident map=omicron") - assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "ident", "map=omicron"}, tokens) - }) - - t.Run("empty string", func(t *testing.T) { - tokens := tokenize("") - assert.Empty(t, tokens) - }) - - t.Run("inline comment stripped", func(t *testing.T) { - tokens := tokenize("host all all 0.0.0.0/0 md5 # office access") - assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "md5"}, tokens) - }) - - t.Run("full-line comment", func(t *testing.T) { - tokens := tokenize("# this is a comment") - assert.Empty(t, tokens) - }) - - t.Run("hash inside quotes not treated as comment", func(t *testing.T) { - tokens := tokenize(`host all all 0.0.0.0/0 ldap ldapprefix="cn=#test"`) - assert.Equal(t, []string{"host", "all", "all", "0.0.0.0/0", "ldap", `ldapprefix="cn=#test"`}, tokens) - }) -} - -func TestStripComment(t *testing.T) { - t.Run("no comment", func(t *testing.T) { - assert.Equal(t, "host all all 0.0.0.0/0 md5", stripComment("host all all 0.0.0.0/0 md5")) - }) - - t.Run("inline comment", func(t *testing.T) { - assert.Equal(t, "host all all 0.0.0.0/0 md5 ", stripComment("host all all 0.0.0.0/0 md5 # comment")) - }) - - t.Run("full-line comment", func(t *testing.T) { - assert.Equal(t, "", stripComment("# full line comment")) - }) - - t.Run("hash inside quotes preserved", func(t *testing.T) { - assert.Equal(t, `host all all 0.0.0.0/0 ldap ldapprefix="cn=#x"`, stripComment(`host all all 0.0.0.0/0 ldap ldapprefix="cn=#x"`)) - }) - - t.Run("hash after closing quote", func(t *testing.T) { - assert.Equal(t, `host all all 0.0.0.0/0 ldap ldapprefix="cn" `, stripComment(`host all all 0.0.0.0/0 ldap ldapprefix="cn" # comment`)) - }) -} - -func TestValidateIPNetmask(t *testing.T) { - t.Run("valid IPv4", func(t *testing.T) { - assert.Empty(t, validateIPNetmask("192.168.1.0", "255.255.255.0")) - }) - - t.Run("valid /8", func(t *testing.T) { - assert.Empty(t, validateIPNetmask("10.0.0.0", "255.0.0.0")) - }) - - t.Run("invalid IP", func(t *testing.T) { - result := validateIPNetmask("999.0.0.1", "255.255.255.0") - assert.Contains(t, result, "invalid IP address") - }) - - t.Run("invalid mask not an IP", func(t *testing.T) { - result := validateIPNetmask("10.0.0.1", "notamask") - assert.Contains(t, result, "invalid netmask") - }) - - t.Run("non-contiguous mask", func(t *testing.T) { - result := validateIPNetmask("10.0.0.1", "255.0.255.0") - assert.Contains(t, result, "not a contiguous subnet mask") - }) -} - -func TestValidateAddress(t *testing.T) { - validAddresses := []string{ - "0.0.0.0/0", - "192.168.1.0/24", - "10.0.0.0/8", - "::1/128", - "::0/0", - "all", - "samehost", - "samenet", - ".example.com", - ".sub.domain.com", - "192.168.1.1", - "myhost.example.com", - "my-host", - "localhost", - } - - for _, addr := range validAddresses { - t.Run("valid/"+addr, func(t *testing.T) { - assert.Empty(t, validateAddress(addr)) - }) - } - - invalidAddresses := []struct { - name string - address string - errMsg string - }{ - {"CIDR mask too large", "192.168.0.0/33", "invalid CIDR"}, - {"invalid IP in CIDR", "256.1.1.1/24", "invalid CIDR"}, - {"bad CIDR format", "999.999.999.999/32", "invalid CIDR"}, - {"special chars", "host@name", "invalid address"}, - {"spaces in addr", "my host", "invalid address"}, - } - - for _, tc := range invalidAddresses { - t.Run("invalid/"+tc.name, func(t *testing.T) { - result := validateAddress(tc.address) - assert.Contains(t, result, tc.errMsg) - }) - } -} From a850bdae4667940a7ec36c60f3e132654e4d9c41 Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Thu, 23 Apr 2026 15:45:20 +0200 Subject: [PATCH 07/11] docs fix --- pkg/postgresql/cluster/core/hba.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/postgresql/cluster/core/hba.go b/pkg/postgresql/cluster/core/hba.go index 4a59d147e..099597696 100644 --- a/pkg/postgresql/cluster/core/hba.go +++ b/pkg/postgresql/cluster/core/hba.go @@ -78,8 +78,16 @@ func ValidateRules(rules []string) []RuleError { return errs } -// validateRule parses and validates a single pg_hba rule. -// Returns a list of validation errors (empty means valid). +// validateRule validates a single pg_hba rule using positional parsing. +// pg_hba.conf has two formats: +// +// local DATABASE USER METHOD [OPTIONS] +// host* DATABASE USER ADDRESS METHOD [OPTIONS] +// host* DATABASE USER IP-ADDRESS NETMASK METHOD [OPTIONS] +// +// Validation order: connection type → minimum field count → auth method +// (at a fixed positional index) → address for host* types. The IP+netmask +// form is detected by checking whether tokens[4] parses as a valid IP. func validateRule(rule string) []string { trimmed := strings.TrimSpace(rule) if trimmed == "" { @@ -93,7 +101,6 @@ func validateRule(rule string) []string { var errs []string - // Layer 0: connection type connType := tokens[0] if !hbaConnectionTypes[connType] { return []string{fmt.Sprintf("unknown connection type %q", connType)} From 1d863e9bddf253900c5294890c833bcdcc3165cc Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Wed, 22 Apr 2026 12:52:54 +0200 Subject: [PATCH 08/11] cross cluster and class validation --- cmd/main.go | 1 + .../postgres_webhook_integration_test.go | 165 +++++++++++ .../webhook/postgrescluster_validation.go | 104 ++++++- .../postgrescluster_validation_test.go | 271 +++++++++++++++++- pkg/splunk/enterprise/validation/context.go | 10 +- pkg/splunk/enterprise/validation/registry.go | 15 +- pkg/splunk/enterprise/validation/server.go | 7 +- pkg/splunk/enterprise/validation/validate.go | 11 +- 8 files changed, 568 insertions(+), 16 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index 20dda7e94..f1a77fc08 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -348,6 +348,7 @@ func main() { ReadTimeout: readTimeout, WriteTimeout: writeTimeout, Client: mgr.GetClient(), + Reader: mgr.GetAPIReader(), }) if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error { diff --git a/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go b/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go index c8f2eaf3d..74b11c38e 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go @@ -24,10 +24,12 @@ import ( "testing" admissionv1 "k8s.io/api/admission/v1" + "k8s.io/apimachinery/pkg/api/resource" authenticationv1 "k8s.io/api/authentication/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/splunk/splunk-operator/pkg/splunk/enterprise/validation" @@ -571,3 +573,166 @@ func TestPostgresClusterClassPgHBAUpdateIntegration(t *testing.T) { assert.Contains(t, resp.Result.Message, "unknown auth method") }) } + +func ptrBool(b bool) *bool { return &b } +func ptrString(s string) *string { return &s } +func ptrInt32(i int32) *int32 { return &i } + +func ptrQuantity(s string) *resource.Quantity { + q := resource.MustParse(s) + return &q +} + +func newFakeReader(objects ...runtime.Object) *fake.ClientBuilder { + s := runtime.NewScheme() + enterpriseApi.AddToScheme(s) + b := fake.NewClientBuilder().WithScheme(s) + for _, obj := range objects { + b = b.WithRuntimeObjects(obj) + } + return b +} + +func TestCrossResourceValidationIntegration(t *testing.T) { + prodClass := &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "prod"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptrInt32(3), + Storage: ptrQuantity("50Gi"), + PostgresVersion: ptrString("17"), + ConnectionPoolerEnabled: ptrBool(false), + }, + }, + } + + reader := newFakeReader(prodClass).Build() + + server := validation.NewWebhookServer(validation.WebhookServerOptions{ + Port: 9443, + Validators: validation.DefaultValidators, + Reader: reader, + }) + + tests := []struct { + name string + obj *enterpriseApi.PostgresCluster + wantAllowed bool + wantMessage string + }{ + { + name: "allowed - inherits all from class", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{Class: "prod"}, + }, + wantAllowed: true, + }, + { + name: "rejected - class not found", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{Class: "nonexistent"}, + }, + wantAllowed: false, + wantMessage: "PostgresClusterClass not found", + }, + { + name: "rejected - storage below class floor", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + Storage: ptrQuantity("10Gi"), + }, + }, + wantAllowed: false, + wantMessage: "storage cannot be lower than class default", + }, + { + name: "rejected - version below class floor", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptrString("16"), + }, + }, + wantAllowed: false, + wantMessage: "postgresVersion cannot be lower than class default", + }, + { + name: "rejected - pooler enabled when class disables", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + ConnectionPoolerEnabled: ptrBool(true), + }, + }, + wantAllowed: false, + wantMessage: "connectionPoolerEnabled cannot be enabled", + }, + { + name: "allowed - storage equal to class", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + Storage: ptrQuantity("50Gi"), + }, + }, + wantAllowed: true, + }, + { + name: "allowed - higher version", + obj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptrString("18"), + }, + }, + wantAllowed: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ar := newPostgresClusterAdmissionReview(t, "uid-xref-"+tt.name, admissionv1.Create, tt.obj, nil) + resp := sendAdmissionReview(t, server, ar) + + assert.Equal(t, tt.wantAllowed, resp.Allowed, "unexpected admission result") + if tt.wantMessage != "" { + require.NotNil(t, resp.Result) + assert.Contains(t, resp.Result.Message, tt.wantMessage) + } + }) + } +} + +func TestCrossResourceValidationDisabledWithoutReader(t *testing.T) { + server := validation.NewWebhookServer(validation.WebhookServerOptions{ + Port: 9443, + Validators: validation.DefaultValidators, + }) + + obj := &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{Class: "nonexistent"}, + } + + ar := newPostgresClusterAdmissionReview(t, "uid-no-reader", admissionv1.Create, obj, nil) + resp := sendAdmissionReview(t, server, ar) + + assert.True(t, resp.Allowed, "without a reader, cross-resource validation should be skipped") +} diff --git a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go index 7fc724c4f..3c968dedd 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go @@ -17,14 +17,18 @@ limitations under the License. package webhook import ( + "context" + "strconv" + "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/controller-runtime/pkg/client" enterpriseApi "github.com/splunk/splunk-operator/api/v4" hba "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core" ) // ValidatePostgresClusterCreate validates a PostgresCluster on CREATE. -func ValidatePostgresClusterCreate(obj *enterpriseApi.PostgresCluster) field.ErrorList { +func ValidatePostgresClusterCreate(obj *enterpriseApi.PostgresCluster, reader client.Reader) field.ErrorList { var allErrs field.ErrorList if len(obj.Spec.PgHBA) > 0 { @@ -37,12 +41,106 @@ func ValidatePostgresClusterCreate(obj *enterpriseApi.PostgresCluster) field.Err } } + if reader != nil { + allErrs = append(allErrs, validateAgainstClass(obj, reader)...) + } + return allErrs } // ValidatePostgresClusterUpdate validates a PostgresCluster on UPDATE. -func ValidatePostgresClusterUpdate(obj, oldObj *enterpriseApi.PostgresCluster) field.ErrorList { - return ValidatePostgresClusterCreate(obj) +func ValidatePostgresClusterUpdate(obj, oldObj *enterpriseApi.PostgresCluster, reader client.Reader) field.ErrorList { + return ValidatePostgresClusterCreate(obj, reader) +} + +func validateAgainstClass(obj *enterpriseApi.PostgresCluster, reader client.Reader) field.ErrorList { + var allErrs field.ErrorList + + class := &enterpriseApi.PostgresClusterClass{} + if err := reader.Get(context.Background(), client.ObjectKey{Name: obj.Spec.Class}, class); err != nil { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec").Child("class"), + obj.Spec.Class, + "referenced PostgresClusterClass not found")) + return allErrs + } + + classConfig := class.Spec.Config + + mergedInstances := obj.Spec.Instances + mergedVersion := obj.Spec.PostgresVersion + mergedStorage := obj.Spec.Storage + if classConfig != nil { + if mergedInstances == nil { + mergedInstances = classConfig.Instances + } + if mergedVersion == nil { + mergedVersion = classConfig.PostgresVersion + } + if mergedStorage == nil { + mergedStorage = classConfig.Storage + } + } + specPath := field.NewPath("spec") + if mergedInstances == nil { + allErrs = append(allErrs, field.Required(specPath.Child("instances"), + "must be set in PostgresCluster or PostgresClusterClass")) + } + if mergedVersion == nil { + allErrs = append(allErrs, field.Required(specPath.Child("postgresVersion"), + "must be set in PostgresCluster or PostgresClusterClass")) + } + if mergedStorage == nil { + allErrs = append(allErrs, field.Required(specPath.Child("storage"), + "must be set in PostgresCluster or PostgresClusterClass")) + } + + if classConfig == nil { + return allErrs + } + + if obj.Spec.Storage != nil && classConfig.Storage != nil { + if obj.Spec.Storage.Cmp(*classConfig.Storage) < 0 { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec").Child("storage"), + obj.Spec.Storage.String(), + "storage cannot be lower than class default ("+classConfig.Storage.String()+")")) + } + } + + if obj.Spec.PostgresVersion != nil && classConfig.PostgresVersion != nil { + clusterMajor := parseMajorVersion(*obj.Spec.PostgresVersion) + classMajor := parseMajorVersion(*classConfig.PostgresVersion) + if clusterMajor > 0 && classMajor > 0 && clusterMajor < classMajor { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec").Child("postgresVersion"), + *obj.Spec.PostgresVersion, + "postgresVersion cannot be lower than class default ("+*classConfig.PostgresVersion+")")) + } + } + + if obj.Spec.ConnectionPoolerEnabled != nil && *obj.Spec.ConnectionPoolerEnabled { + classDisabled := classConfig.ConnectionPoolerEnabled == nil || !*classConfig.ConnectionPoolerEnabled + if classDisabled { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec").Child("connectionPoolerEnabled"), + true, + "connectionPoolerEnabled cannot be enabled when disabled in PostgresClusterClass")) + } + } + + return allErrs +} + +func parseMajorVersion(version string) int { + for i, ch := range version { + if ch == '.' { + v, _ := strconv.Atoi(version[:i]) + return v + } + } + v, _ := strconv.Atoi(version) + return v } // GetPostgresClusterWarningsOnCreate returns warnings for PostgresCluster CREATE. diff --git a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go index 56ff34c9c..32cb1ecf6 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go @@ -19,8 +19,14 @@ package webhook import ( "testing" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + enterpriseApi "github.com/splunk/splunk-operator/api/v4" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestValidatePostgresClusterCreate(t *testing.T) { @@ -118,7 +124,7 @@ func TestValidatePostgresClusterCreate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - errs := ValidatePostgresClusterCreate(tt.obj) + errs := ValidatePostgresClusterCreate(tt.obj, nil) assert.Len(t, errs, tt.wantErrCount, "unexpected error count") if tt.wantErrField != "" && len(errs) > 0 { assert.Equal(t, tt.wantErrField, errs[0].Field, "unexpected error field") @@ -168,7 +174,7 @@ func TestValidatePostgresClusterUpdate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - errs := ValidatePostgresClusterUpdate(tt.obj, tt.oldObj) + errs := ValidatePostgresClusterUpdate(tt.obj, tt.oldObj, nil) assert.Len(t, errs, tt.wantErrCount, "unexpected error count") }) } @@ -190,3 +196,264 @@ func TestGetPostgresClusterWarningsOnUpdate(t *testing.T) { } assert.Empty(t, GetPostgresClusterWarningsOnUpdate(obj, oldObj)) } + +func newFakeReader(objects ...runtime.Object) *fake.ClientBuilder { + s := runtime.NewScheme() + enterpriseApi.AddToScheme(s) + b := fake.NewClientBuilder().WithScheme(s) + for _, obj := range objects { + b = b.WithRuntimeObjects(obj) + } + return b +} + +func ptrBool(b bool) *bool { return &b } +func ptrString(s string) *string { return &s } +func ptrInt32(i int32) *int32 { return &i } + +func ptrQuantity(s string) *resource.Quantity { + q := resource.MustParse(s) + return &q +} + +func TestValidateAgainstClass(t *testing.T) { + classWithDefaults := &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "prod"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptrInt32(3), + Storage: ptrQuantity("50Gi"), + PostgresVersion: ptrString("17"), + ConnectionPoolerEnabled: ptrBool(false), + }, + }, + } + + classWithPoolerEnabled := &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "pooler-class"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptrInt32(3), + Storage: ptrQuantity("50Gi"), + PostgresVersion: ptrString("17"), + ConnectionPoolerEnabled: ptrBool(true), + }, + }, + } + + tests := []struct { + name string + class *enterpriseApi.PostgresClusterClass + obj *enterpriseApi.PostgresCluster + wantErrCount int + wantErrField string + }{ + { + name: "class not found", + class: nil, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "nonexistent"}, + }, + wantErrCount: 1, + wantErrField: "spec.class", + }, + { + name: "valid - no overrides", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "prod"}, + }, + wantErrCount: 0, + }, + { + name: "valid - storage equal to class", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + Storage: ptrQuantity("50Gi"), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - storage higher than class", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + Storage: ptrQuantity("100Gi"), + }, + }, + wantErrCount: 0, + }, + { + name: "invalid - storage lower than class", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + Storage: ptrQuantity("10Gi"), + }, + }, + wantErrCount: 1, + wantErrField: "spec.storage", + }, + { + name: "valid - same postgres version", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptrString("17"), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - higher postgres version", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptrString("18"), + }, + }, + wantErrCount: 0, + }, + { + name: "invalid - lower postgres version", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptrString("16"), + }, + }, + wantErrCount: 1, + wantErrField: "spec.postgresVersion", + }, + { + name: "valid - minor version higher", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptrString("17.2"), + }, + }, + wantErrCount: 0, + }, + { + name: "invalid - connection pooler enabled when class disables", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + ConnectionPoolerEnabled: ptrBool(true), + }, + }, + wantErrCount: 1, + wantErrField: "spec.connectionPoolerEnabled", + }, + { + name: "valid - connection pooler disabled when class disables", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + ConnectionPoolerEnabled: ptrBool(false), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - connection pooler enabled when class enables", + class: classWithPoolerEnabled, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "pooler-class", + ConnectionPoolerEnabled: ptrBool(true), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - connection pooler unset (inherits class)", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "prod"}, + }, + wantErrCount: 0, + }, + { + name: "invalid - class has no config, cluster missing required fields", + class: &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "bare"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + }, + }, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "bare"}, + }, + wantErrCount: 3, + wantErrField: "spec.instances", + }, + { + name: "invalid - class config missing storage, cluster doesn't provide it", + class: &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "no-storage"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptrInt32(3), + PostgresVersion: ptrString("17"), + }, + }, + }, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "no-storage"}, + }, + wantErrCount: 1, + wantErrField: "spec.storage", + }, + { + name: "valid - cluster fills in what class is missing", + class: &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "minimal"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{}, + }, + }, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "minimal", + Instances: ptrInt32(1), + PostgresVersion: ptrString("17"), + Storage: ptrQuantity("10Gi"), + }, + }, + wantErrCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + builder := newFakeReader() + if tt.class != nil { + builder = newFakeReader(tt.class) + } + reader := builder.Build() + + errs := ValidatePostgresClusterCreate(tt.obj, reader) + require.Len(t, errs, tt.wantErrCount, "unexpected error count: %v", errs) + if tt.wantErrField != "" && len(errs) > 0 { + assert.Equal(t, tt.wantErrField, errs[0].Field, "unexpected error field") + } + }) + } +} diff --git a/pkg/splunk/enterprise/validation/context.go b/pkg/splunk/enterprise/validation/context.go index 7be272647..06516506e 100644 --- a/pkg/splunk/enterprise/validation/context.go +++ b/pkg/splunk/enterprise/validation/context.go @@ -27,9 +27,14 @@ import ( // ValidationContext provides context for validation operations that may need // to access Kubernetes resources type ValidationContext struct { - // Client is the Kubernetes client for resource lookups + // Client is the Kubernetes client for resource lookups (cached) Client client.Client + // Reader is a live API reader that bypasses the cache. + // Use this for cross-resource lookups where eventual consistency + // could cause false rejections. + Reader client.Reader + // Namespace is the namespace of the object being validated Namespace string @@ -38,9 +43,10 @@ type ValidationContext struct { } // NewValidationContext creates a new ValidationContext -func NewValidationContext(c client.Client, namespace string) *ValidationContext { +func NewValidationContext(c client.Client, reader client.Reader, namespace string) *ValidationContext { return &ValidationContext{ Client: c, + Reader: reader, Namespace: namespace, Ctx: context.Background(), } diff --git a/pkg/splunk/enterprise/validation/registry.go b/pkg/splunk/enterprise/validation/registry.go index 5eab98402..f959b8b3a 100644 --- a/pkg/splunk/enterprise/validation/registry.go +++ b/pkg/splunk/enterprise/validation/registry.go @@ -18,6 +18,7 @@ package validation import ( "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" enterpriseApi "github.com/splunk/splunk-operator/api/v4" pgwebhook "github.com/splunk/splunk-operator/pkg/postgresql/cluster/adapter/webhook" @@ -195,8 +196,18 @@ var DefaultValidators = map[schema.GroupVersionResource]Validator{ }, PostgresClusterGVR: &GenericValidator[*enterpriseApi.PostgresCluster]{ - ValidateCreateFunc: pgwebhook.ValidatePostgresClusterCreate, - ValidateUpdateFunc: pgwebhook.ValidatePostgresClusterUpdate, + ValidateCreateFunc: func(obj *enterpriseApi.PostgresCluster) field.ErrorList { + return pgwebhook.ValidatePostgresClusterCreate(obj, nil) + }, + ValidateUpdateFunc: func(obj, oldObj *enterpriseApi.PostgresCluster) field.ErrorList { + return pgwebhook.ValidatePostgresClusterUpdate(obj, oldObj, nil) + }, + ValidateCreateWithContextFunc: func(obj *enterpriseApi.PostgresCluster, vc *ValidationContext) field.ErrorList { + return pgwebhook.ValidatePostgresClusterCreate(obj, vc.Reader) + }, + ValidateUpdateWithContextFunc: func(obj *enterpriseApi.PostgresCluster, oldObj *enterpriseApi.PostgresCluster, vc *ValidationContext) field.ErrorList { + return pgwebhook.ValidatePostgresClusterUpdate(obj, oldObj, vc.Reader) + }, WarningsOnCreateFunc: pgwebhook.GetPostgresClusterWarningsOnCreate, WarningsOnUpdateFunc: pgwebhook.GetPostgresClusterWarningsOnUpdate, GroupKind: schema.GroupKind{ diff --git a/pkg/splunk/enterprise/validation/server.go b/pkg/splunk/enterprise/validation/server.go index 882f89878..3f1b94723 100644 --- a/pkg/splunk/enterprise/validation/server.go +++ b/pkg/splunk/enterprise/validation/server.go @@ -58,8 +58,11 @@ type WebhookServerOptions struct { // WriteTimeout is the maximum duration before timing out writes of the response (default: 10s) WriteTimeout time.Duration - // Client is the Kubernetes client for resource lookups (optional) + // Client is the Kubernetes client for resource lookups (optional, cached) Client client.Client + + // Reader is a live API reader that bypasses the cache (optional). + Reader client.Reader } // WebhookServer is the HTTP server for validation webhooks @@ -174,7 +177,7 @@ func (s *WebhookServer) HandleValidate(w http.ResponseWriter, r *http.Request) { "user", admissionReview.Request.UserInfo.Username) } - warnings, validationErr := ValidateWithClient(&admissionReview, s.options.Validators, s.options.Client) + warnings, validationErr := ValidateWithClient(&admissionReview, s.options.Validators, s.options.Client, s.options.Reader) response := &admissionv1.AdmissionResponse{ UID: admissionReview.Request.UID, diff --git a/pkg/splunk/enterprise/validation/validate.go b/pkg/splunk/enterprise/validation/validate.go index f49cbab3a..0deb426a0 100644 --- a/pkg/splunk/enterprise/validation/validate.go +++ b/pkg/splunk/enterprise/validation/validate.go @@ -43,13 +43,14 @@ func init() { // Validate performs validation on an AdmissionReview request // Returns warnings (even on success) and an error if validation fails func Validate(ar *admissionv1.AdmissionReview, validators map[schema.GroupVersionResource]Validator) ([]string, error) { - return ValidateWithClient(ar, validators, nil) + return ValidateWithClient(ar, validators, nil, nil) } // ValidateWithClient performs validation on an AdmissionReview request with a Kubernetes client // The client enables resource existence checks (e.g., verifying secrets exist) +// The reader provides live API reads bypassing the cache for cross-resource lookups // Returns warnings (even on success) and an error if validation fails -func ValidateWithClient(ar *admissionv1.AdmissionReview, validators map[schema.GroupVersionResource]Validator, k8sClient client.Client) ([]string, error) { +func ValidateWithClient(ar *admissionv1.AdmissionReview, validators map[schema.GroupVersionResource]Validator, k8sClient client.Client, reader client.Reader) ([]string, error) { if ar == nil || ar.Request == nil { return nil, fmt.Errorf("admission review or request is nil") } @@ -84,10 +85,10 @@ func ValidateWithClient(ar *admissionv1.AdmissionReview, validators map[schema.G } } - // Create validation context if client is available + // Create validation context if client or reader is available var vc *ValidationContext - if k8sClient != nil { - vc = NewValidationContext(k8sClient, req.Namespace) + if k8sClient != nil || reader != nil { + vc = NewValidationContext(k8sClient, reader, req.Namespace) } var fieldErrs field.ErrorList From 0c5136167c54f1475f408024f5a12499fff528c2 Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Wed, 22 Apr 2026 12:59:48 +0200 Subject: [PATCH 09/11] minnor version check --- .../webhook/postgrescluster_validation.go | 29 ++++--- .../postgrescluster_validation_test.go | 82 ++++++++++++++++++- 2 files changed, 98 insertions(+), 13 deletions(-) diff --git a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go index 3c968dedd..aa3ea0458 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go @@ -109,13 +109,17 @@ func validateAgainstClass(obj *enterpriseApi.PostgresCluster, reader client.Read } if obj.Spec.PostgresVersion != nil && classConfig.PostgresVersion != nil { - clusterMajor := parseMajorVersion(*obj.Spec.PostgresVersion) - classMajor := parseMajorVersion(*classConfig.PostgresVersion) - if clusterMajor > 0 && classMajor > 0 && clusterMajor < classMajor { - allErrs = append(allErrs, field.Invalid( - field.NewPath("spec").Child("postgresVersion"), - *obj.Spec.PostgresVersion, - "postgresVersion cannot be lower than class default ("+*classConfig.PostgresVersion+")")) + clusterMajor, clusterMinor := parseVersion(*obj.Spec.PostgresVersion) + classMajor, classMinor := parseVersion(*classConfig.PostgresVersion) + if clusterMajor > 0 && classMajor > 0 { + versionTooLow := clusterMajor < classMajor || + (clusterMajor == classMajor && classMinor >= 0 && clusterMinor >= 0 && clusterMinor < classMinor) + if versionTooLow { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec").Child("postgresVersion"), + *obj.Spec.PostgresVersion, + "postgresVersion cannot be lower than class default ("+*classConfig.PostgresVersion+")")) + } } } @@ -132,15 +136,16 @@ func validateAgainstClass(obj *enterpriseApi.PostgresCluster, reader client.Read return allErrs } -func parseMajorVersion(version string) int { +func parseVersion(version string) (major, minor int) { for i, ch := range version { if ch == '.' { - v, _ := strconv.Atoi(version[:i]) - return v + major, _ = strconv.Atoi(version[:i]) + minor, _ = strconv.Atoi(version[i+1:]) + return major, minor } } - v, _ := strconv.Atoi(version) - return v + major, _ = strconv.Atoi(version) + return major, -1 } // GetPostgresClusterWarningsOnCreate returns warnings for PostgresCluster CREATE. diff --git a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go index 32cb1ecf6..fb4656bf2 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go @@ -230,6 +230,18 @@ func TestValidateAgainstClass(t *testing.T) { }, } + classWithMinorVersion := &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "prod-pinned"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptrInt32(3), + Storage: ptrQuantity("50Gi"), + PostgresVersion: ptrString("17.2"), + }, + }, + } + classWithPoolerEnabled := &enterpriseApi.PostgresClusterClass{ ObjectMeta: metav1.ObjectMeta{Name: "pooler-class"}, Spec: enterpriseApi.PostgresClusterClassSpec{ @@ -336,16 +348,84 @@ func TestValidateAgainstClass(t *testing.T) { wantErrField: "spec.postgresVersion", }, { - name: "valid - minor version higher", + name: "valid - minor version ignored when class has major only", + class: classWithDefaults, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptrString("17.2"), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - lower minor ignored when class has major only", class: classWithDefaults, obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", + PostgresVersion: ptrString("17.0"), + }, + }, + wantErrCount: 0, + }, + { + name: "valid - cluster minor equal to class minor", + class: classWithMinorVersion, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod-pinned", PostgresVersion: ptrString("17.2"), }, }, wantErrCount: 0, }, + { + name: "valid - cluster minor higher than class minor", + class: classWithMinorVersion, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod-pinned", + PostgresVersion: ptrString("17.5"), + }, + }, + wantErrCount: 0, + }, + { + name: "invalid - cluster minor lower than class minor", + class: classWithMinorVersion, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod-pinned", + PostgresVersion: ptrString("17.1"), + }, + }, + wantErrCount: 1, + wantErrField: "spec.postgresVersion", + }, + { + name: "invalid - cluster major lower even with higher minor", + class: classWithMinorVersion, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod-pinned", + PostgresVersion: ptrString("16.9"), + }, + }, + wantErrCount: 1, + wantErrField: "spec.postgresVersion", + }, + { + name: "valid - cluster major higher than class with minor", + class: classWithMinorVersion, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod-pinned", + PostgresVersion: ptrString("18"), + }, + }, + wantErrCount: 0, + }, { name: "invalid - connection pooler enabled when class disables", class: classWithDefaults, From 96b28d303c4f86670d83a7968e3d24f80443cb35 Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Mon, 27 Apr 2026 09:18:52 +0200 Subject: [PATCH 10/11] remove API reader --- cmd/main.go | 1 - .../postgres_webhook_integration_test.go | 63 ++------- .../webhook/postgrescluster_validation.go | 101 ++++++------- .../postgrescluster_validation_test.go | 133 ++++++++---------- pkg/postgresql/cluster/core/cluster.go | 8 +- .../cluster/core/cluster_unit_test.go | 10 +- pkg/splunk/enterprise/validation/context.go | 10 +- pkg/splunk/enterprise/validation/registry.go | 4 +- pkg/splunk/enterprise/validation/server.go | 7 +- pkg/splunk/enterprise/validation/validate.go | 13 +- 10 files changed, 135 insertions(+), 215 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index f1a77fc08..20dda7e94 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -348,7 +348,6 @@ func main() { ReadTimeout: readTimeout, WriteTimeout: writeTimeout, Client: mgr.GetClient(), - Reader: mgr.GetAPIReader(), }) if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error { diff --git a/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go b/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go index 74b11c38e..95c997dd5 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go @@ -29,6 +29,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client/fake" enterpriseApi "github.com/splunk/splunk-operator/api/v4" @@ -574,15 +575,6 @@ func TestPostgresClusterClassPgHBAUpdateIntegration(t *testing.T) { }) } -func ptrBool(b bool) *bool { return &b } -func ptrString(s string) *string { return &s } -func ptrInt32(i int32) *int32 { return &i } - -func ptrQuantity(s string) *resource.Quantity { - q := resource.MustParse(s) - return &q -} - func newFakeReader(objects ...runtime.Object) *fake.ClientBuilder { s := runtime.NewScheme() enterpriseApi.AddToScheme(s) @@ -599,20 +591,20 @@ func TestCrossResourceValidationIntegration(t *testing.T) { Spec: enterpriseApi.PostgresClusterClassSpec{ Provisioner: "postgresql.cnpg.io", Config: &enterpriseApi.PostgresClusterClassConfig{ - Instances: ptrInt32(3), - Storage: ptrQuantity("50Gi"), - PostgresVersion: ptrString("17"), - ConnectionPoolerEnabled: ptrBool(false), + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17"), + ConnectionPoolerEnabled: ptr.To(false), }, }, } - reader := newFakeReader(prodClass).Build() + fakeClient := newFakeReader(prodClass).Build() server := validation.NewWebhookServer(validation.WebhookServerOptions{ Port: 9443, Validators: validation.DefaultValidators, - Reader: reader, + Client: fakeClient, }) tests := []struct { @@ -640,19 +632,6 @@ func TestCrossResourceValidationIntegration(t *testing.T) { wantAllowed: false, wantMessage: "PostgresClusterClass not found", }, - { - name: "rejected - storage below class floor", - obj: &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "prod", - Storage: ptrQuantity("10Gi"), - }, - }, - wantAllowed: false, - wantMessage: "storage cannot be lower than class default", - }, { name: "rejected - version below class floor", obj: &enterpriseApi.PostgresCluster{ @@ -660,36 +639,24 @@ func TestCrossResourceValidationIntegration(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", - PostgresVersion: ptrString("16"), + PostgresVersion: ptr.To("16"), }, }, wantAllowed: false, wantMessage: "postgresVersion cannot be lower than class default", }, { - name: "rejected - pooler enabled when class disables", + name: "rejected - pooler enabled but class has no cnpg.connectionPooler", obj: &enterpriseApi.PostgresCluster{ TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", - ConnectionPoolerEnabled: ptrBool(true), + ConnectionPoolerEnabled: ptr.To(true), }, }, wantAllowed: false, - wantMessage: "connectionPoolerEnabled cannot be enabled", - }, - { - name: "allowed - storage equal to class", - obj: &enterpriseApi.PostgresCluster{ - TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, - ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "prod", - Storage: ptrQuantity("50Gi"), - }, - }, - wantAllowed: true, + wantMessage: "connection pooler requires cnpg.connectionPooler configuration", }, { name: "allowed - higher version", @@ -698,7 +665,7 @@ func TestCrossResourceValidationIntegration(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", - PostgresVersion: ptrString("18"), + PostgresVersion: ptr.To("18"), }, }, wantAllowed: true, @@ -719,7 +686,7 @@ func TestCrossResourceValidationIntegration(t *testing.T) { } } -func TestCrossResourceValidationDisabledWithoutReader(t *testing.T) { +func TestCrossResourceValidationDisabledWithoutClient(t *testing.T) { server := validation.NewWebhookServer(validation.WebhookServerOptions{ Port: 9443, Validators: validation.DefaultValidators, @@ -731,8 +698,8 @@ func TestCrossResourceValidationDisabledWithoutReader(t *testing.T) { Spec: enterpriseApi.PostgresClusterSpec{Class: "nonexistent"}, } - ar := newPostgresClusterAdmissionReview(t, "uid-no-reader", admissionv1.Create, obj, nil) + ar := newPostgresClusterAdmissionReview(t, "uid-no-client", admissionv1.Create, obj, nil) resp := sendAdmissionReview(t, server, ar) - assert.True(t, resp.Allowed, "without a reader, cross-resource validation should be skipped") + assert.True(t, resp.Allowed, "without a client, cross-resource validation should be skipped") } diff --git a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go index aa3ea0458..24427425e 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go @@ -18,13 +18,15 @@ package webhook import ( "context" + "fmt" "strconv" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/controller-runtime/pkg/client" enterpriseApi "github.com/splunk/splunk-operator/api/v4" - hba "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core" + core "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core" ) // ValidatePostgresClusterCreate validates a PostgresCluster on CREATE. @@ -33,7 +35,7 @@ func ValidatePostgresClusterCreate(obj *enterpriseApi.PostgresCluster, reader cl if len(obj.Spec.PgHBA) > 0 { pgHBAPath := field.NewPath("spec").Child("pgHBA") - for _, re := range hba.ValidateRules(obj.Spec.PgHBA) { + for _, re := range core.ValidateRules(obj.Spec.PgHBA) { allErrs = append(allErrs, field.Invalid( pgHBAPath.Index(re.Index), obj.Spec.PgHBA[re.Index], @@ -58,81 +60,62 @@ func validateAgainstClass(obj *enterpriseApi.PostgresCluster, reader client.Read class := &enterpriseApi.PostgresClusterClass{} if err := reader.Get(context.Background(), client.ObjectKey{Name: obj.Spec.Class}, class); err != nil { - allErrs = append(allErrs, field.Invalid( - field.NewPath("spec").Child("class"), - obj.Spec.Class, - "referenced PostgresClusterClass not found")) + classPath := field.NewPath("spec").Child("class") + if apierrors.IsNotFound(err) { + allErrs = append(allErrs, field.Invalid(classPath, obj.Spec.Class, + "referenced PostgresClusterClass not found")) + } else { + allErrs = append(allErrs, field.InternalError(classPath, + fmt.Errorf("failed to look up PostgresClusterClass %q: %w", obj.Spec.Class, err))) + } return allErrs } - classConfig := class.Spec.Config - - mergedInstances := obj.Spec.Instances - mergedVersion := obj.Spec.PostgresVersion - mergedStorage := obj.Spec.Storage - if classConfig != nil { - if mergedInstances == nil { - mergedInstances = classConfig.Instances + merged, err := core.GetMergedConfig(class, obj) + if err != nil { + specPath := field.NewPath("spec") + if merged == nil || merged.Spec.Instances == nil { + allErrs = append(allErrs, field.Required(specPath.Child("instances"), + "must be set in PostgresCluster or PostgresClusterClass")) } - if mergedVersion == nil { - mergedVersion = classConfig.PostgresVersion + if merged == nil || merged.Spec.PostgresVersion == nil { + allErrs = append(allErrs, field.Required(specPath.Child("postgresVersion"), + "must be set in PostgresCluster or PostgresClusterClass")) } - if mergedStorage == nil { - mergedStorage = classConfig.Storage + if merged == nil || merged.Spec.Storage == nil { + allErrs = append(allErrs, field.Required(specPath.Child("storage"), + "must be set in PostgresCluster or PostgresClusterClass")) } - } - specPath := field.NewPath("spec") - if mergedInstances == nil { - allErrs = append(allErrs, field.Required(specPath.Child("instances"), - "must be set in PostgresCluster or PostgresClusterClass")) - } - if mergedVersion == nil { - allErrs = append(allErrs, field.Required(specPath.Child("postgresVersion"), - "must be set in PostgresCluster or PostgresClusterClass")) - } - if mergedStorage == nil { - allErrs = append(allErrs, field.Required(specPath.Child("storage"), - "must be set in PostgresCluster or PostgresClusterClass")) - } - - if classConfig == nil { return allErrs } - if obj.Spec.Storage != nil && classConfig.Storage != nil { - if obj.Spec.Storage.Cmp(*classConfig.Storage) < 0 { - allErrs = append(allErrs, field.Invalid( - field.NewPath("spec").Child("storage"), - obj.Spec.Storage.String(), - "storage cannot be lower than class default ("+classConfig.Storage.String()+")")) - } - } - - if obj.Spec.PostgresVersion != nil && classConfig.PostgresVersion != nil { - clusterMajor, clusterMinor := parseVersion(*obj.Spec.PostgresVersion) - classMajor, classMinor := parseVersion(*classConfig.PostgresVersion) - if clusterMajor > 0 && classMajor > 0 { - versionTooLow := clusterMajor < classMajor || - (clusterMajor == classMajor && classMinor >= 0 && clusterMinor >= 0 && clusterMinor < classMinor) - if versionTooLow { - allErrs = append(allErrs, field.Invalid( - field.NewPath("spec").Child("postgresVersion"), - *obj.Spec.PostgresVersion, - "postgresVersion cannot be lower than class default ("+*classConfig.PostgresVersion+")")) + if classConfig := class.Spec.Config; classConfig != nil { + if obj.Spec.PostgresVersion != nil && classConfig.PostgresVersion != nil { + clusterMajor, clusterMinor := parseVersion(*obj.Spec.PostgresVersion) + classMajor, classMinor := parseVersion(*classConfig.PostgresVersion) + if clusterMajor > 0 && classMajor > 0 { + versionTooLow := clusterMajor < classMajor || + (clusterMajor == classMajor && classMinor >= 0 && clusterMinor >= 0 && clusterMinor < classMinor) + if versionTooLow { + allErrs = append(allErrs, field.Invalid( + field.NewPath("spec").Child("postgresVersion"), + *obj.Spec.PostgresVersion, + "postgresVersion cannot be lower than class default ("+*classConfig.PostgresVersion+")")) + } } } - } - if obj.Spec.ConnectionPoolerEnabled != nil && *obj.Spec.ConnectionPoolerEnabled { - classDisabled := classConfig.ConnectionPoolerEnabled == nil || !*classConfig.ConnectionPoolerEnabled - if classDisabled { + poolerEnabled := (obj.Spec.ConnectionPoolerEnabled != nil && *obj.Spec.ConnectionPoolerEnabled) || + (obj.Spec.ConnectionPoolerEnabled == nil && classConfig.ConnectionPoolerEnabled != nil && *classConfig.ConnectionPoolerEnabled) + if poolerEnabled && (class.Spec.CNPG == nil || class.Spec.CNPG.ConnectionPooler == nil) { allErrs = append(allErrs, field.Invalid( field.NewPath("spec").Child("connectionPoolerEnabled"), true, - "connectionPoolerEnabled cannot be enabled when disabled in PostgresClusterClass")) + "connection pooler requires cnpg.connectionPooler configuration in PostgresClusterClass")) } } + _ = merged return allErrs } diff --git a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go index fb4656bf2..dec9671bb 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client/fake" enterpriseApi "github.com/splunk/splunk-operator/api/v4" @@ -207,25 +208,16 @@ func newFakeReader(objects ...runtime.Object) *fake.ClientBuilder { return b } -func ptrBool(b bool) *bool { return &b } -func ptrString(s string) *string { return &s } -func ptrInt32(i int32) *int32 { return &i } - -func ptrQuantity(s string) *resource.Quantity { - q := resource.MustParse(s) - return &q -} - func TestValidateAgainstClass(t *testing.T) { classWithDefaults := &enterpriseApi.PostgresClusterClass{ ObjectMeta: metav1.ObjectMeta{Name: "prod"}, Spec: enterpriseApi.PostgresClusterClassSpec{ Provisioner: "postgresql.cnpg.io", Config: &enterpriseApi.PostgresClusterClassConfig{ - Instances: ptrInt32(3), - Storage: ptrQuantity("50Gi"), - PostgresVersion: ptrString("17"), - ConnectionPoolerEnabled: ptrBool(false), + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17"), + ConnectionPoolerEnabled: ptr.To(false), }, }, } @@ -235,9 +227,9 @@ func TestValidateAgainstClass(t *testing.T) { Spec: enterpriseApi.PostgresClusterClassSpec{ Provisioner: "postgresql.cnpg.io", Config: &enterpriseApi.PostgresClusterClassConfig{ - Instances: ptrInt32(3), - Storage: ptrQuantity("50Gi"), - PostgresVersion: ptrString("17.2"), + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17.2"), }, }, } @@ -247,10 +239,13 @@ func TestValidateAgainstClass(t *testing.T) { Spec: enterpriseApi.PostgresClusterClassSpec{ Provisioner: "postgresql.cnpg.io", Config: &enterpriseApi.PostgresClusterClassConfig{ - Instances: ptrInt32(3), - Storage: ptrQuantity("50Gi"), - PostgresVersion: ptrString("17"), - ConnectionPoolerEnabled: ptrBool(true), + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17"), + ConnectionPoolerEnabled: ptr.To(true), + }, + CNPG: &enterpriseApi.CNPGConfig{ + ConnectionPooler: &enterpriseApi.ConnectionPoolerConfig{}, }, }, } @@ -279,47 +274,13 @@ func TestValidateAgainstClass(t *testing.T) { }, wantErrCount: 0, }, - { - name: "valid - storage equal to class", - class: classWithDefaults, - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "prod", - Storage: ptrQuantity("50Gi"), - }, - }, - wantErrCount: 0, - }, - { - name: "valid - storage higher than class", - class: classWithDefaults, - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "prod", - Storage: ptrQuantity("100Gi"), - }, - }, - wantErrCount: 0, - }, - { - name: "invalid - storage lower than class", - class: classWithDefaults, - obj: &enterpriseApi.PostgresCluster{ - Spec: enterpriseApi.PostgresClusterSpec{ - Class: "prod", - Storage: ptrQuantity("10Gi"), - }, - }, - wantErrCount: 1, - wantErrField: "spec.storage", - }, { name: "valid - same postgres version", class: classWithDefaults, obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", - PostgresVersion: ptrString("17"), + PostgresVersion: ptr.To("17"), }, }, wantErrCount: 0, @@ -330,7 +291,7 @@ func TestValidateAgainstClass(t *testing.T) { obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", - PostgresVersion: ptrString("18"), + PostgresVersion: ptr.To("18"), }, }, wantErrCount: 0, @@ -341,7 +302,7 @@ func TestValidateAgainstClass(t *testing.T) { obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", - PostgresVersion: ptrString("16"), + PostgresVersion: ptr.To("16"), }, }, wantErrCount: 1, @@ -353,7 +314,7 @@ func TestValidateAgainstClass(t *testing.T) { obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", - PostgresVersion: ptrString("17.2"), + PostgresVersion: ptr.To("17.2"), }, }, wantErrCount: 0, @@ -364,7 +325,7 @@ func TestValidateAgainstClass(t *testing.T) { obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", - PostgresVersion: ptrString("17.0"), + PostgresVersion: ptr.To("17.0"), }, }, wantErrCount: 0, @@ -375,7 +336,7 @@ func TestValidateAgainstClass(t *testing.T) { obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod-pinned", - PostgresVersion: ptrString("17.2"), + PostgresVersion: ptr.To("17.2"), }, }, wantErrCount: 0, @@ -386,7 +347,7 @@ func TestValidateAgainstClass(t *testing.T) { obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod-pinned", - PostgresVersion: ptrString("17.5"), + PostgresVersion: ptr.To("17.5"), }, }, wantErrCount: 0, @@ -397,7 +358,7 @@ func TestValidateAgainstClass(t *testing.T) { obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod-pinned", - PostgresVersion: ptrString("17.1"), + PostgresVersion: ptr.To("17.1"), }, }, wantErrCount: 1, @@ -409,7 +370,7 @@ func TestValidateAgainstClass(t *testing.T) { obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod-pinned", - PostgresVersion: ptrString("16.9"), + PostgresVersion: ptr.To("16.9"), }, }, wantErrCount: 1, @@ -421,53 +382,73 @@ func TestValidateAgainstClass(t *testing.T) { obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod-pinned", - PostgresVersion: ptrString("18"), + PostgresVersion: ptr.To("18"), }, }, wantErrCount: 0, }, { - name: "invalid - connection pooler enabled when class disables", + name: "invalid - pooler enabled but class has no cnpg.connectionPooler", class: classWithDefaults, obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", - ConnectionPoolerEnabled: ptrBool(true), + ConnectionPoolerEnabled: ptr.To(true), }, }, wantErrCount: 1, wantErrField: "spec.connectionPoolerEnabled", }, { - name: "valid - connection pooler disabled when class disables", + name: "valid - pooler disabled, class has no cnpg config", class: classWithDefaults, obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "prod", - ConnectionPoolerEnabled: ptrBool(false), + ConnectionPoolerEnabled: ptr.To(false), }, }, wantErrCount: 0, }, { - name: "valid - connection pooler enabled when class enables", + name: "valid - pooler enabled and class has cnpg.connectionPooler", class: classWithPoolerEnabled, obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "pooler-class", - ConnectionPoolerEnabled: ptrBool(true), + ConnectionPoolerEnabled: ptr.To(true), }, }, wantErrCount: 0, }, { - name: "valid - connection pooler unset (inherits class)", + name: "valid - pooler unset (inherits class)", class: classWithDefaults, obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{Class: "prod"}, }, wantErrCount: 0, }, + { + name: "invalid - class enables pooler but missing cnpg config", + class: &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "pooler-no-cnpg"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17"), + ConnectionPoolerEnabled: ptr.To(true), + }, + }, + }, + obj: &enterpriseApi.PostgresCluster{ + Spec: enterpriseApi.PostgresClusterSpec{Class: "pooler-no-cnpg"}, + }, + wantErrCount: 1, + wantErrField: "spec.connectionPoolerEnabled", + }, { name: "invalid - class has no config, cluster missing required fields", class: &enterpriseApi.PostgresClusterClass{ @@ -489,8 +470,8 @@ func TestValidateAgainstClass(t *testing.T) { Spec: enterpriseApi.PostgresClusterClassSpec{ Provisioner: "postgresql.cnpg.io", Config: &enterpriseApi.PostgresClusterClassConfig{ - Instances: ptrInt32(3), - PostgresVersion: ptrString("17"), + Instances: ptr.To(int32(3)), + PostgresVersion: ptr.To("17"), }, }, }, @@ -512,9 +493,9 @@ func TestValidateAgainstClass(t *testing.T) { obj: &enterpriseApi.PostgresCluster{ Spec: enterpriseApi.PostgresClusterSpec{ Class: "minimal", - Instances: ptrInt32(1), - PostgresVersion: ptrString("17"), - Storage: ptrQuantity("10Gi"), + Instances: ptr.To(int32(1)), + PostgresVersion: ptr.To("17"), + Storage: ptr.To(resource.MustParse("10Gi")), }, }, wantErrCount: 0, diff --git a/pkg/postgresql/cluster/core/cluster.go b/pkg/postgresql/cluster/core/cluster.go index a09e95780..0a1cadb49 100644 --- a/pkg/postgresql/cluster/core/cluster.go +++ b/pkg/postgresql/cluster/core/cluster.go @@ -142,7 +142,7 @@ func PostgresClusterService(ctx context.Context, rc *ReconcileContext, req ctrl. } // Merge PostgresClusterSpec on top of PostgresClusterClass defaults. - mergedConfig, err := getMergedConfig(clusterClass, postgresCluster) + mergedConfig, err := GetMergedConfig(clusterClass, postgresCluster) if err != nil { logger.Error(err, "Failed to merge PostgresCluster configuration") rc.emitWarning(postgresCluster, EventConfigMergeFailed, fmt.Sprintf("Failed to merge configuration: %v", err)) @@ -1380,9 +1380,9 @@ func isIntermediateState(state pgcConstants.State) bool { } } -// getMergedConfig overlays PostgresCluster spec on top of the class defaults. +// GetMergedConfig overlays PostgresCluster spec on top of the class defaults. // Class values are used only where the cluster spec is silent. -func getMergedConfig(class *enterprisev4.PostgresClusterClass, cluster *enterprisev4.PostgresCluster) (*MergedConfig, error) { +func GetMergedConfig(class *enterprisev4.PostgresClusterClass, cluster *enterprisev4.PostgresCluster) (*MergedConfig, error) { result := cluster.Spec.DeepCopy() // Config is optional on the class — apply defaults only when provided. @@ -1411,7 +1411,7 @@ func getMergedConfig(class *enterprisev4.PostgresClusterClass, cluster *enterpri } if result.Instances == nil || result.PostgresVersion == nil || result.Storage == nil { - return nil, fmt.Errorf("invalid configuration for class %s: instances, postgresVersion and storage are required", class.Name) + return &MergedConfig{Spec: result, CNPG: class.Spec.CNPG}, fmt.Errorf("invalid configuration for class %s: instances, postgresVersion and storage are required", class.Name) } if result.PostgreSQLConfig == nil { result.PostgreSQLConfig = make(map[string]string) diff --git a/pkg/postgresql/cluster/core/cluster_unit_test.go b/pkg/postgresql/cluster/core/cluster_unit_test.go index 6df965cc6..deadf88cd 100644 --- a/pkg/postgresql/cluster/core/cluster_unit_test.go +++ b/pkg/postgresql/cluster/core/cluster_unit_test.go @@ -404,7 +404,7 @@ func TestGetMergedConfig(t *testing.T) { }, } - cfg, err := getMergedConfig(baseClass, cluster) + cfg, err := GetMergedConfig(baseClass, cluster) require.NoError(t, err) assert.Equal(t, int32(5), *cfg.Spec.Instances) @@ -419,7 +419,7 @@ func TestGetMergedConfig(t *testing.T) { Spec: enterprisev4.PostgresClusterSpec{}, } - cfg, err := getMergedConfig(baseClass, cluster) + cfg, err := GetMergedConfig(baseClass, cluster) require.NoError(t, err) assert.Equal(t, int32(1), *cfg.Spec.Instances) @@ -437,7 +437,7 @@ func TestGetMergedConfig(t *testing.T) { Spec: enterprisev4.PostgresClusterSpec{}, } - _, err := getMergedConfig(emptyClass, cluster) + _, err := GetMergedConfig(emptyClass, cluster) require.Error(t, err) }) @@ -447,7 +447,7 @@ func TestGetMergedConfig(t *testing.T) { Spec: enterprisev4.PostgresClusterSpec{}, } - cfg, err := getMergedConfig(baseClass, cluster) + cfg, err := GetMergedConfig(baseClass, cluster) require.NoError(t, err) require.NotNil(t, cfg.CNPG) @@ -469,7 +469,7 @@ func TestGetMergedConfig(t *testing.T) { Spec: enterprisev4.PostgresClusterSpec{}, } - cfg, err := getMergedConfig(classWithNoMaps, cluster) + cfg, err := GetMergedConfig(classWithNoMaps, cluster) require.NoError(t, err) assert.NotNil(t, cfg.Spec.PostgreSQLConfig) diff --git a/pkg/splunk/enterprise/validation/context.go b/pkg/splunk/enterprise/validation/context.go index 06516506e..7be272647 100644 --- a/pkg/splunk/enterprise/validation/context.go +++ b/pkg/splunk/enterprise/validation/context.go @@ -27,14 +27,9 @@ import ( // ValidationContext provides context for validation operations that may need // to access Kubernetes resources type ValidationContext struct { - // Client is the Kubernetes client for resource lookups (cached) + // Client is the Kubernetes client for resource lookups Client client.Client - // Reader is a live API reader that bypasses the cache. - // Use this for cross-resource lookups where eventual consistency - // could cause false rejections. - Reader client.Reader - // Namespace is the namespace of the object being validated Namespace string @@ -43,10 +38,9 @@ type ValidationContext struct { } // NewValidationContext creates a new ValidationContext -func NewValidationContext(c client.Client, reader client.Reader, namespace string) *ValidationContext { +func NewValidationContext(c client.Client, namespace string) *ValidationContext { return &ValidationContext{ Client: c, - Reader: reader, Namespace: namespace, Ctx: context.Background(), } diff --git a/pkg/splunk/enterprise/validation/registry.go b/pkg/splunk/enterprise/validation/registry.go index f959b8b3a..3d7e47897 100644 --- a/pkg/splunk/enterprise/validation/registry.go +++ b/pkg/splunk/enterprise/validation/registry.go @@ -203,10 +203,10 @@ var DefaultValidators = map[schema.GroupVersionResource]Validator{ return pgwebhook.ValidatePostgresClusterUpdate(obj, oldObj, nil) }, ValidateCreateWithContextFunc: func(obj *enterpriseApi.PostgresCluster, vc *ValidationContext) field.ErrorList { - return pgwebhook.ValidatePostgresClusterCreate(obj, vc.Reader) + return pgwebhook.ValidatePostgresClusterCreate(obj, vc.Client) }, ValidateUpdateWithContextFunc: func(obj *enterpriseApi.PostgresCluster, oldObj *enterpriseApi.PostgresCluster, vc *ValidationContext) field.ErrorList { - return pgwebhook.ValidatePostgresClusterUpdate(obj, oldObj, vc.Reader) + return pgwebhook.ValidatePostgresClusterUpdate(obj, oldObj, vc.Client) }, WarningsOnCreateFunc: pgwebhook.GetPostgresClusterWarningsOnCreate, WarningsOnUpdateFunc: pgwebhook.GetPostgresClusterWarningsOnUpdate, diff --git a/pkg/splunk/enterprise/validation/server.go b/pkg/splunk/enterprise/validation/server.go index 3f1b94723..882f89878 100644 --- a/pkg/splunk/enterprise/validation/server.go +++ b/pkg/splunk/enterprise/validation/server.go @@ -58,11 +58,8 @@ type WebhookServerOptions struct { // WriteTimeout is the maximum duration before timing out writes of the response (default: 10s) WriteTimeout time.Duration - // Client is the Kubernetes client for resource lookups (optional, cached) + // Client is the Kubernetes client for resource lookups (optional) Client client.Client - - // Reader is a live API reader that bypasses the cache (optional). - Reader client.Reader } // WebhookServer is the HTTP server for validation webhooks @@ -177,7 +174,7 @@ func (s *WebhookServer) HandleValidate(w http.ResponseWriter, r *http.Request) { "user", admissionReview.Request.UserInfo.Username) } - warnings, validationErr := ValidateWithClient(&admissionReview, s.options.Validators, s.options.Client, s.options.Reader) + warnings, validationErr := ValidateWithClient(&admissionReview, s.options.Validators, s.options.Client) response := &admissionv1.AdmissionResponse{ UID: admissionReview.Request.UID, diff --git a/pkg/splunk/enterprise/validation/validate.go b/pkg/splunk/enterprise/validation/validate.go index 0deb426a0..1b8643074 100644 --- a/pkg/splunk/enterprise/validation/validate.go +++ b/pkg/splunk/enterprise/validation/validate.go @@ -43,14 +43,13 @@ func init() { // Validate performs validation on an AdmissionReview request // Returns warnings (even on success) and an error if validation fails func Validate(ar *admissionv1.AdmissionReview, validators map[schema.GroupVersionResource]Validator) ([]string, error) { - return ValidateWithClient(ar, validators, nil, nil) + return ValidateWithClient(ar, validators, nil) } // ValidateWithClient performs validation on an AdmissionReview request with a Kubernetes client -// The client enables resource existence checks (e.g., verifying secrets exist) -// The reader provides live API reads bypassing the cache for cross-resource lookups +// The client enables resource lookups (e.g., verifying secrets exist, cross-resource validation) // Returns warnings (even on success) and an error if validation fails -func ValidateWithClient(ar *admissionv1.AdmissionReview, validators map[schema.GroupVersionResource]Validator, k8sClient client.Client, reader client.Reader) ([]string, error) { +func ValidateWithClient(ar *admissionv1.AdmissionReview, validators map[schema.GroupVersionResource]Validator, k8sClient client.Client) ([]string, error) { if ar == nil || ar.Request == nil { return nil, fmt.Errorf("admission review or request is nil") } @@ -85,10 +84,10 @@ func ValidateWithClient(ar *admissionv1.AdmissionReview, validators map[schema.G } } - // Create validation context if client or reader is available + // Create validation context if client is available var vc *ValidationContext - if k8sClient != nil || reader != nil { - vc = NewValidationContext(k8sClient, reader, req.Namespace) + if k8sClient != nil { + vc = NewValidationContext(k8sClient, req.Namespace) } var fieldErrs field.ErrorList From 7622554e941f8e1c3723b5812cc93e37c5362389 Mon Sep 17 00:00:00 2001 From: Kamil Ubych Date: Mon, 27 Apr 2026 09:43:24 +0200 Subject: [PATCH 11/11] tests fix --- .../postgres_webhook_integration_test.go | 103 ++++++++++++++++++ .../webhook/postgrescluster_validation.go | 1 + .../postgrescluster_validation_test.go | 12 ++ pkg/splunk/enterprise/validation/validate.go | 2 +- 4 files changed, 117 insertions(+), 1 deletion(-) diff --git a/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go b/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go index 95c997dd5..5271b3fec 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go @@ -686,6 +686,109 @@ func TestCrossResourceValidationIntegration(t *testing.T) { } } +func TestCrossResourceValidationUpdateIntegration(t *testing.T) { + prodClass := &enterpriseApi.PostgresClusterClass{ + ObjectMeta: metav1.ObjectMeta{Name: "prod"}, + Spec: enterpriseApi.PostgresClusterClassSpec{ + Provisioner: "postgresql.cnpg.io", + Config: &enterpriseApi.PostgresClusterClassConfig{ + Instances: ptr.To(int32(3)), + Storage: ptr.To(resource.MustParse("50Gi")), + PostgresVersion: ptr.To("17"), + ConnectionPoolerEnabled: ptr.To(false), + }, + }, + } + + fakeClient := newFakeReader(prodClass).Build() + + server := validation.NewWebhookServer(validation.WebhookServerOptions{ + Port: 9443, + Validators: validation.DefaultValidators, + Client: fakeClient, + }) + + oldObj := &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("17"), + }, + } + + tests := []struct { + name string + newObj *enterpriseApi.PostgresCluster + wantAllowed bool + wantMessage string + }{ + { + name: "allowed - upgrade version", + newObj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("18"), + }, + }, + wantAllowed: true, + }, + { + name: "rejected - downgrade version below class floor", + newObj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("16"), + }, + }, + wantAllowed: false, + wantMessage: "postgresVersion cannot be lower than class default", + }, + { + name: "rejected - enable pooler without cnpg config", + newObj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + ConnectionPoolerEnabled: ptr.To(true), + }, + }, + wantAllowed: false, + wantMessage: "connection pooler requires cnpg.connectionPooler configuration", + }, + { + name: "allowed - no changes", + newObj: &enterpriseApi.PostgresCluster{ + TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"}, + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + Spec: enterpriseApi.PostgresClusterSpec{ + Class: "prod", + PostgresVersion: ptr.To("17"), + }, + }, + wantAllowed: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ar := newPostgresClusterAdmissionReview(t, "uid-xref-update-"+tt.name, admissionv1.Update, tt.newObj, oldObj) + resp := sendAdmissionReview(t, server, ar) + + assert.Equal(t, tt.wantAllowed, resp.Allowed, "unexpected admission result") + if tt.wantMessage != "" { + require.NotNil(t, resp.Result) + assert.Contains(t, resp.Result.Message, tt.wantMessage) + } + }) + } +} + func TestCrossResourceValidationDisabledWithoutClient(t *testing.T) { server := validation.NewWebhookServer(validation.WebhookServerOptions{ Port: 9443, diff --git a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go index 24427425e..02763f974 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go @@ -90,6 +90,7 @@ func validateAgainstClass(obj *enterpriseApi.PostgresCluster, reader client.Read } if classConfig := class.Spec.Config; classConfig != nil { + // Class version acts as a minimum floor for compliance; clusters may override higher but not lower. if obj.Spec.PostgresVersion != nil && classConfig.PostgresVersion != nil { clusterMajor, clusterMinor := parseVersion(*obj.Spec.PostgresVersion) classMajor, classMinor := parseVersion(*classConfig.PostgresVersion) diff --git a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go index dec9671bb..e678b5930 100644 --- a/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go +++ b/pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation_test.go @@ -256,6 +256,7 @@ func TestValidateAgainstClass(t *testing.T) { obj *enterpriseApi.PostgresCluster wantErrCount int wantErrField string + wantErrMsg string }{ { name: "class not found", @@ -265,6 +266,7 @@ func TestValidateAgainstClass(t *testing.T) { }, wantErrCount: 1, wantErrField: "spec.class", + wantErrMsg: "referenced PostgresClusterClass not found", }, { name: "valid - no overrides", @@ -307,6 +309,7 @@ func TestValidateAgainstClass(t *testing.T) { }, wantErrCount: 1, wantErrField: "spec.postgresVersion", + wantErrMsg: "postgresVersion cannot be lower than class default (17)", }, { name: "valid - minor version ignored when class has major only", @@ -363,6 +366,7 @@ func TestValidateAgainstClass(t *testing.T) { }, wantErrCount: 1, wantErrField: "spec.postgresVersion", + wantErrMsg: "postgresVersion cannot be lower than class default (17.2)", }, { name: "invalid - cluster major lower even with higher minor", @@ -375,6 +379,7 @@ func TestValidateAgainstClass(t *testing.T) { }, wantErrCount: 1, wantErrField: "spec.postgresVersion", + wantErrMsg: "postgresVersion cannot be lower than class default (17.2)", }, { name: "valid - cluster major higher than class with minor", @@ -398,6 +403,7 @@ func TestValidateAgainstClass(t *testing.T) { }, wantErrCount: 1, wantErrField: "spec.connectionPoolerEnabled", + wantErrMsg: "connection pooler requires cnpg.connectionPooler configuration in PostgresClusterClass", }, { name: "valid - pooler disabled, class has no cnpg config", @@ -448,6 +454,7 @@ func TestValidateAgainstClass(t *testing.T) { }, wantErrCount: 1, wantErrField: "spec.connectionPoolerEnabled", + wantErrMsg: "connection pooler requires cnpg.connectionPooler configuration in PostgresClusterClass", }, { name: "invalid - class has no config, cluster missing required fields", @@ -462,6 +469,7 @@ func TestValidateAgainstClass(t *testing.T) { }, wantErrCount: 3, wantErrField: "spec.instances", + wantErrMsg: "must be set in PostgresCluster or PostgresClusterClass", }, { name: "invalid - class config missing storage, cluster doesn't provide it", @@ -480,6 +488,7 @@ func TestValidateAgainstClass(t *testing.T) { }, wantErrCount: 1, wantErrField: "spec.storage", + wantErrMsg: "must be set in PostgresCluster or PostgresClusterClass", }, { name: "valid - cluster fills in what class is missing", @@ -515,6 +524,9 @@ func TestValidateAgainstClass(t *testing.T) { if tt.wantErrField != "" && len(errs) > 0 { assert.Equal(t, tt.wantErrField, errs[0].Field, "unexpected error field") } + if tt.wantErrMsg != "" && len(errs) > 0 { + assert.Contains(t, errs[0].Detail, tt.wantErrMsg, "unexpected error message") + } }) } } diff --git a/pkg/splunk/enterprise/validation/validate.go b/pkg/splunk/enterprise/validation/validate.go index 1b8643074..f49cbab3a 100644 --- a/pkg/splunk/enterprise/validation/validate.go +++ b/pkg/splunk/enterprise/validation/validate.go @@ -47,7 +47,7 @@ func Validate(ar *admissionv1.AdmissionReview, validators map[schema.GroupVersio } // ValidateWithClient performs validation on an AdmissionReview request with a Kubernetes client -// The client enables resource lookups (e.g., verifying secrets exist, cross-resource validation) +// The client enables resource existence checks (e.g., verifying secrets exist) // Returns warnings (even on success) and an error if validation fails func ValidateWithClient(ar *admissionv1.AdmissionReview, validators map[schema.GroupVersionResource]Validator, k8sClient client.Client) ([]string, error) { if ar == nil || ar.Request == nil {