Skip to content

Commit de54038

Browse files
cbandycbandy
committed
Add a validated field for Postgres parameters
The validation rules of Kubernetes 1.29 (Beta in 1.25) allow for this kind of field. Issue: PGO-313
1 parent 8dbe427 commit de54038

File tree

17 files changed

+566
-159
lines changed

17 files changed

+566
-159
lines changed

‎config/crd/bases/postgres-operator.crunchydata.com_postgresclusters.yaml‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4370,6 +4370,7 @@ spec:
43704370
config:
43714371
properties:
43724372
files:
4373+
description: Files to mount under "/etc/postgres".
43734374
items:
43744375
description: |-
43754376
Projection that may be projected along with other supported volume types.
@@ -4688,6 +4689,53 @@ spec:
46884689
type: object
46894690
type: object
46904691
type: array
4692+
parameters:
4693+
additionalProperties:
4694+
anyOf:
4695+
- type: integer
4696+
- type: string
4697+
x-kubernetes-int-or-string: true
4698+
description: |-
4699+
Configuration parameters for the PostgreSQL server. Some values will
4700+
be reloaded without validation and some cause PostgreSQL to restart.
4701+
Some values cannot be changed at all.
4702+
More info: https://www.postgresql.org/docs/current/runtime-config.html
4703+
maxProperties: 50
4704+
type: object
4705+
x-kubernetes-map-type: granular
4706+
x-kubernetes-validations:
4707+
- message: 'cannot change PGDATA path: config_file, data_directory'
4708+
rule: '!has(self.config_file) && !has(self.data_directory)'
4709+
- message: cannot change external_pid_file
4710+
rule: '!has(self.external_pid_file)'
4711+
- message: 'cannot change authentication path: hba_file, ident_file'
4712+
rule: '!has(self.hba_file) && !has(self.ident_file)'
4713+
- message: 'network connectivity is always enabled: listen_addresses'
4714+
rule: '!has(self.listen_addresses)'
4715+
- message: change port using .spec.port instead
4716+
rule: '!has(self.port)'
4717+
- rule: '!has(self.ssl) && !self.exists(k, k.startsWith("ssl_"))'
4718+
- message: domain socket paths cannot be changed
4719+
rule: '!self.exists(k, k.startsWith("unix_socket_"))'
4720+
- message: wal_level must be "replica" or higher
4721+
rule: '!has(self.wal_level) || self.wal_level in ["replica","logical"]'
4722+
- message: wal_log_hints are always enabled
4723+
rule: '!has(self.wal_log_hints)'
4724+
- rule: '!has(self.archive_mode) && !has(self.archive_command)
4725+
&& !has(self.restore_command)'
4726+
- rule: '!has(self.recovery_target) && !self.exists(k, k.startsWith("recovery_target_"))'
4727+
- message: hot_standby is always enabled
4728+
rule: '!has(self.hot_standby)'
4729+
- rule: '!has(self.synchronous_standby_names)'
4730+
- rule: '!has(self.primary_conninfo) && !has(self.primary_slot_name)'
4731+
- message: delayed replication is not supported at this time
4732+
rule: '!has(self.recovery_min_apply_delay)'
4733+
- message: cluster_name is derived from the PostgresCluster name
4734+
rule: '!has(self.cluster_name)'
4735+
- message: disabling logging_collector is unsafe
4736+
rule: '!has(self.logging_collector)'
4737+
- message: log_file_mode cannot be changed
4738+
rule: '!has(self.log_file_mode)'
46914739
type: object
46924740
customReplicationTLSSecret:
46934741
description: |-

‎internal/config/config.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,24 @@ func defaultFromEnv(value, key string) string {
2222
// FetchKeyCommand returns the fetch_key_cmd value stored in the encryption_key_command
2323
// variable used to enable TDE.
2424
func FetchKeyCommand(spec *v1beta1.PostgresClusterSpec) string {
25+
if parameters := spec.Config.Parameters; parameters != nil {
26+
if v, ok := parameters["encryption_key_command"]; ok {
27+
return v.String()
28+
}
29+
}
30+
2531
if spec.Patroni != nil {
26-
if spec.Patroni.DynamicConfiguration != nil {
27-
configuration := spec.Patroni.DynamicConfiguration
28-
if configuration != nil {
29-
if postgresql, ok := configuration["postgresql"].(map[string]any); ok {
30-
if parameters, ok := postgresql["parameters"].(map[string]any); ok {
31-
if parameters["encryption_key_command"] != nil {
32-
return fmt.Sprintf("%s", parameters["encryption_key_command"])
33-
}
32+
if configuration := spec.Patroni.DynamicConfiguration; configuration != nil {
33+
if postgresql, ok := configuration["postgresql"].(map[string]any); ok {
34+
if parameters, ok := postgresql["parameters"].(map[string]any); ok {
35+
if parameters["encryption_key_command"] != nil {
36+
return fmt.Sprintf("%s", parameters["encryption_key_command"])
3437
}
3538
}
3639
}
3740
}
3841
}
42+
3943
return ""
4044
}
4145

‎internal/config/config_test.go‎

Lines changed: 98 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -15,68 +15,115 @@ import (
1515
)
1616

1717
func TestFetchKeyCommand(t *testing.T) {
18-
19-
spec1 := v1beta1.PostgresClusterSpec{}
20-
assert.Assert(t, FetchKeyCommand(&spec1) == "")
21-
22-
spec2 := v1beta1.PostgresClusterSpec{
23-
Patroni: &v1beta1.PatroniSpec{},
24-
}
25-
assert.Assert(t, FetchKeyCommand(&spec2) == "")
26-
27-
spec3 := v1beta1.PostgresClusterSpec{
28-
Patroni: &v1beta1.PatroniSpec{
29-
DynamicConfiguration: map[string]any{},
30-
},
31-
}
32-
assert.Assert(t, FetchKeyCommand(&spec3) == "")
33-
34-
spec4 := v1beta1.PostgresClusterSpec{
35-
Patroni: &v1beta1.PatroniSpec{
36-
DynamicConfiguration: map[string]any{
37-
"postgresql": map[string]any{},
18+
t.Run("missing", func(t *testing.T) {
19+
spec1 := v1beta1.PostgresClusterSpec{}
20+
assert.Assert(t, FetchKeyCommand(&spec1) == "")
21+
22+
spec2 := v1beta1.PostgresClusterSpec{
23+
Patroni: &v1beta1.PatroniSpec{},
24+
}
25+
assert.Assert(t, FetchKeyCommand(&spec2) == "")
26+
27+
spec3 := v1beta1.PostgresClusterSpec{
28+
Patroni: &v1beta1.PatroniSpec{
29+
DynamicConfiguration: map[string]any{},
3830
},
39-
},
40-
}
41-
assert.Assert(t, FetchKeyCommand(&spec4) == "")
31+
}
32+
assert.Assert(t, FetchKeyCommand(&spec3) == "")
4233

43-
spec5 := v1beta1.PostgresClusterSpec{
44-
Patroni: &v1beta1.PatroniSpec{
45-
DynamicConfiguration: map[string]any{
46-
"postgresql": map[string]any{
47-
"parameters": map[string]any{},
34+
spec4 := v1beta1.PostgresClusterSpec{
35+
Patroni: &v1beta1.PatroniSpec{
36+
DynamicConfiguration: map[string]any{
37+
"postgresql": map[string]any{},
4838
},
4939
},
50-
},
51-
}
52-
assert.Assert(t, FetchKeyCommand(&spec5) == "")
53-
54-
spec6 := v1beta1.PostgresClusterSpec{
55-
Patroni: &v1beta1.PatroniSpec{
56-
DynamicConfiguration: map[string]any{
57-
"postgresql": map[string]any{
58-
"parameters": map[string]any{
59-
"encryption_key_command": "",
40+
}
41+
assert.Assert(t, FetchKeyCommand(&spec4) == "")
42+
43+
spec5 := v1beta1.PostgresClusterSpec{
44+
Patroni: &v1beta1.PatroniSpec{
45+
DynamicConfiguration: map[string]any{
46+
"postgresql": map[string]any{
47+
"parameters": map[string]any{},
6048
},
6149
},
6250
},
63-
},
64-
}
65-
assert.Assert(t, FetchKeyCommand(&spec6) == "")
66-
67-
spec7 := v1beta1.PostgresClusterSpec{
68-
Patroni: &v1beta1.PatroniSpec{
69-
DynamicConfiguration: map[string]any{
70-
"postgresql": map[string]any{
71-
"parameters": map[string]any{
72-
"encryption_key_command": "echo mykey",
51+
}
52+
assert.Assert(t, FetchKeyCommand(&spec5) == "")
53+
})
54+
55+
t.Run("blank", func(t *testing.T) {
56+
var spec1 v1beta1.PostgresClusterSpec
57+
assert.NilError(t, yaml.Unmarshal([]byte(`{
58+
patroni: {
59+
dynamicConfiguration: {
60+
postgresql: {
61+
parameters: {
62+
encryption_key_command: "",
63+
},
7364
},
7465
},
7566
},
76-
},
77-
}
78-
assert.Assert(t, FetchKeyCommand(&spec7) == "echo mykey")
67+
}`), &spec1))
68+
assert.Equal(t, "", FetchKeyCommand(&spec1))
69+
70+
var spec2 v1beta1.PostgresClusterSpec
71+
assert.NilError(t, yaml.Unmarshal([]byte(`{
72+
config: {
73+
parameters: {
74+
encryption_key_command: "",
75+
},
76+
},
77+
}`), &spec2))
78+
assert.Equal(t, "", FetchKeyCommand(&spec2))
79+
})
80+
81+
t.Run("exists", func(t *testing.T) {
82+
var spec1 v1beta1.PostgresClusterSpec
83+
assert.NilError(t, yaml.Unmarshal([]byte(`{
84+
patroni: {
85+
dynamicConfiguration: {
86+
postgresql: {
87+
parameters: {
88+
encryption_key_command: "echo mykey",
89+
},
90+
},
91+
},
92+
},
93+
}`), &spec1))
94+
assert.Equal(t, "echo mykey", FetchKeyCommand(&spec1))
95+
96+
var spec2 v1beta1.PostgresClusterSpec
97+
assert.NilError(t, yaml.Unmarshal([]byte(`{
98+
config: {
99+
parameters: {
100+
encryption_key_command: "cat somefile",
101+
},
102+
},
103+
}`), &spec2))
104+
assert.Equal(t, "cat somefile", FetchKeyCommand(&spec2))
105+
})
79106

107+
t.Run("config.parameters takes precedence", func(t *testing.T) {
108+
var spec v1beta1.PostgresClusterSpec
109+
assert.NilError(t, yaml.Unmarshal([]byte(`{
110+
config: {
111+
parameters: {
112+
encryption_key_command: "cat somefile",
113+
},
114+
},
115+
patroni: {
116+
dynamicConfiguration: {
117+
postgresql: {
118+
parameters: {
119+
encryption_key_command: "echo mykey",
120+
},
121+
},
122+
},
123+
},
124+
}`), &spec))
125+
assert.Equal(t, "cat somefile", FetchKeyCommand(&spec))
126+
})
80127
}
81128

82129
func TestPGAdminContainerImage(t *testing.T) {

‎internal/patroni/config.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"strings"
1111

1212
corev1 "k8s.io/api/core/v1"
13+
"k8s.io/apimachinery/pkg/util/intstr"
1314
"sigs.k8s.io/yaml"
1415

1516
"github.com/crunchydata/postgres-operator/internal/config"
@@ -244,7 +245,11 @@ func DynamicConfiguration(
244245
parameters[k] = v
245246
}
246247
}
247-
// Override the above with mandatory parameters.
248+
// Copy spec.config.parameters over spec.patroni...parameters.
249+
for k, v := range spec.Config.Parameters {
250+
parameters[k] = v
251+
}
252+
// Override all of the above with mandatory parameters.
248253
if pgParameters.Mandatory != nil {
249254
for k, v := range pgParameters.Mandatory.AsMap() {
250255

@@ -254,8 +259,15 @@ func DynamicConfiguration(
254259
// that out as well.
255260
if k == "shared_preload_libraries" {
256261
// Load mandatory libraries ahead of user-defined libraries.
257-
if s, ok := parameters[k].(string); ok && len(s) > 0 {
258-
v = v + "," + s
262+
switch s := parameters[k].(type) {
263+
case string:
264+
if len(s) > 0 {
265+
v = v + "," + s
266+
}
267+
case intstr.IntOrString:
268+
if len(s.StrVal) > 0 {
269+
v = v + "," + s.StrVal
270+
}
259271
}
260272
// Load "citus" ahead of any other libraries.
261273
// - https://github.com/citusdata/citus/blob/v12.0.0/src/backend/distributed/shared_library_init.c#L417-L419

0 commit comments

Comments
 (0)