Skip to content

Commit 68a962b

Browse files
jgillichjgillich
committed
Fix empty resources spec field failing schema validation
In Go, when a struct field is not set, it becomes a struct with default values for all fields. These default values are included during serialization. This causes issues with schema validation where optional fields cannot be omitted because default values are considered invalid. This patch addresses this issue for `Resources` fields on several types by using a pointer value.
1 parent f105533 commit 68a962b

File tree

5 files changed

+35
-30
lines changed

5 files changed

+35
-30
lines changed

‎pkg/apis/acid.zalan.do/v1/postgresql_type.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ type PostgresSpec struct {
2727
PostgresqlParam `json:"postgresql"`
2828
Volume `json:"volume,omitempty"`
2929
Patroni `json:"patroni,omitempty"`
30-
Resources `json:"resources,omitempty"`
30+
*Resources `json:"resources,omitempty"`
3131

3232
EnableConnectionPooler *bool `json:"enableConnectionPooler,omitempty"`
3333
EnableReplicaConnectionPooler *bool `json:"enableReplicaConnectionPooler,omitempty"`
@@ -190,7 +190,7 @@ type CloneDescription struct {
190190

191191
// Sidecar defines a container to be run in the same pod as the Postgres container.
192192
type Sidecar struct {
193-
Resources `json:"resources,omitempty"`
193+
*Resources `json:"resources,omitempty"`
194194
Name string `json:"name,omitempty"`
195195
DockerImage string `json:"image,omitempty"`
196196
Ports []v1.ContainerPort `json:"ports,omitempty"`
@@ -223,5 +223,5 @@ type ConnectionPooler struct {
223223
DockerImage string `json:"dockerImage,omitempty"`
224224
MaxDBConnections *int32 `json:"maxDBConnections,omitempty"`
225225

226-
Resources `json:"resources,omitempty"`
226+
*Resources `json:"resources,omitempty"`
227227
}

‎pkg/apis/acid.zalan.do/v1/util_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ var unmarshalCluster = []struct {
163163
"kind": "Postgresql","apiVersion": "acid.zalan.do/v1",
164164
"metadata": {"name": "acid-testcluster1"}, "spec": {"teamId": 100}}`), &tmp).Error(),
165165
},
166-
marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":null},"status":"Invalid"}`),
166+
marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"teamId":"","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":null},"status":"Invalid"}`),
167167
err: nil},
168168
{
169169
about: "example with /status subresource",
@@ -184,7 +184,7 @@ var unmarshalCluster = []struct {
184184
"kind": "Postgresql","apiVersion": "acid.zalan.do/v1",
185185
"metadata": {"name": "acid-testcluster1"}, "spec": {"teamId": 100}}`), &tmp).Error(),
186186
},
187-
marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":null},"status":{"PostgresClusterStatus":"Invalid"}}`),
187+
marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"teamId":"","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":null},"status":{"PostgresClusterStatus":"Invalid"}}`),
188188
err: nil},
189189
{
190190
about: "example with detailed input manifest and deprecated pod_priority_class_name -> podPriorityClassName",
@@ -300,7 +300,7 @@ var unmarshalCluster = []struct {
300300
MaximumLagOnFailover: 33554432,
301301
Slots: map[string]map[string]string{"permanent_logical_1": {"type": "logical", "database": "foo", "plugin": "pgoutput"}},
302302
},
303-
Resources: Resources{
303+
Resources: &Resources{
304304
ResourceRequests: ResourceDescription{CPU: "10m", Memory: "50Mi"},
305305
ResourceLimits: ResourceDescription{CPU: "300m", Memory: "3000Mi"},
306306
},
@@ -351,7 +351,7 @@ var unmarshalCluster = []struct {
351351
Status: PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid},
352352
Error: errors.New("name must match {TEAM}-{NAME} format").Error(),
353353
},
354-
marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"teapot-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null} ,"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":null},"status":{"PostgresClusterStatus":"Invalid"}}`),
354+
marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"teapot-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"teamId":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":null},"status":{"PostgresClusterStatus":"Invalid"}}`),
355355
err: nil},
356356
{
357357
about: "example with clone",
@@ -373,7 +373,7 @@ var unmarshalCluster = []struct {
373373
},
374374
Error: "",
375375
},
376-
marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":{"cluster":"team-batman"}},"status":{"PostgresClusterStatus":""}}`),
376+
marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"teamId":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"clone":{"cluster":"team-batman"}},"status":{"PostgresClusterStatus":""}}`),
377377
err: nil},
378378
{
379379
about: "standby example",
@@ -395,7 +395,7 @@ var unmarshalCluster = []struct {
395395
},
396396
Error: "",
397397
},
398-
marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"resources":{"requests":{"cpu":"","memory":""},"limits":{"cpu":"","memory":""}},"teamId":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"standby":{"s3_wal_path":"s3://custom/path/to/bucket/"}},"status":{"PostgresClusterStatus":""}}`),
398+
marshal: []byte(`{"kind":"Postgresql","apiVersion":"acid.zalan.do/v1","metadata":{"name":"acid-testcluster1","creationTimestamp":null},"spec":{"postgresql":{"version":"","parameters":null},"volume":{"size":"","storageClass":""},"patroni":{"initdb":null,"pg_hba":null,"ttl":0,"loop_wait":0,"retry_timeout":0,"maximum_lag_on_failover":0,"slots":null},"teamId":"acid","allowedSourceRanges":null,"numberOfInstances":0,"users":null,"standby":{"s3_wal_path":"s3://custom/path/to/bucket/"}},"status":{"PostgresClusterStatus":""}}`),
399399
err: nil},
400400
{
401401
about: "expect error on malformatted JSON",

‎pkg/cluster/cluster_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func TestStatefulSetAnnotations(t *testing.T) {
5151
testName := "CheckStatefulsetAnnotations"
5252
spec := acidv1.PostgresSpec{
5353
TeamID: "myapp", NumberOfInstances: 1,
54-
Resources: acidv1.Resources{
54+
Resources: &acidv1.Resources{
5555
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
5656
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
5757
},

‎pkg/cluster/k8sres.go‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,18 @@ func (c *Cluster) makeDefaultResources() acidv1.Resources {
133133
}
134134
}
135135

136-
func generateResourceRequirements(resources acidv1.Resources, defaultResources acidv1.Resources) (*v1.ResourceRequirements, error) {
136+
func generateResourceRequirements(resources *acidv1.Resources, defaultResources acidv1.Resources) (*v1.ResourceRequirements, error) {
137137
var err error
138138

139-
specRequests := resources.ResourceRequests
140-
specLimits := resources.ResourceLimits
139+
var specRequests acidv1.ResourceDescription
140+
var specLimits acidv1.ResourceDescription
141+
if resources == nil {
142+
specRequests = acidv1.ResourceDescription{}
143+
specLimits = acidv1.ResourceDescription{}
144+
} else {
145+
specRequests = resources.ResourceRequests
146+
specLimits = resources.ResourceLimits
147+
}
141148

142149
result := v1.ResourceRequirements{}
143150

@@ -497,16 +504,14 @@ func generateSidecarContainers(sidecars []acidv1.Sidecar,
497504
if len(sidecars) > 0 {
498505
result := make([]v1.Container, 0)
499506
for index, sidecar := range sidecars {
507+
var resourcesSpec acidv1.Resources
508+
if sidecar.Resources == nil {
509+
resourcesSpec = acidv1.Resources{}
510+
} else {
511+
sidecar.Resources.DeepCopyInto(&resourcesSpec)
512+
}
500513

501-
resources, err := generateResourceRequirements(
502-
makeResources(
503-
sidecar.Resources.ResourceRequests.CPU,
504-
sidecar.Resources.ResourceRequests.Memory,
505-
sidecar.Resources.ResourceLimits.CPU,
506-
sidecar.Resources.ResourceLimits.Memory,
507-
),
508-
defaultResources,
509-
)
514+
resources, err := generateResourceRequirements(&resourcesSpec, defaultResources)
510515
if err != nil {
511516
return nil, err
512517
}
@@ -1349,7 +1354,7 @@ func generateScalyrSidecarSpec(clusterName, APIKey, serverURL, dockerImage strin
13491354
scalyrCPULimit,
13501355
scalyrMemoryLimit,
13511356
)
1352-
resourceRequirementsScalyrSidecar, err := generateResourceRequirements(resourcesScalyrSidecar, defaultResources)
1357+
resourceRequirementsScalyrSidecar, err := generateResourceRequirements(&resourcesScalyrSidecar, defaultResources)
13531358
if err != nil {
13541359
return nil, fmt.Errorf("invalid resources for Scalyr sidecar: %v", err)
13551360
}

‎pkg/cluster/k8sres_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -893,7 +893,7 @@ func TestNodeAffinity(t *testing.T) {
893893
makeSpec := func(nodeAffinity *v1.NodeAffinity) acidv1.PostgresSpec {
894894
return acidv1.PostgresSpec{
895895
TeamID: "myapp", NumberOfInstances: 1,
896-
Resources: acidv1.Resources{
896+
Resources: &acidv1.Resources{
897897
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
898898
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
899899
},
@@ -989,7 +989,7 @@ func TestTLS(t *testing.T) {
989989
},
990990
Spec: acidv1.PostgresSpec{
991991
TeamID: "myapp", NumberOfInstances: 1,
992-
Resources: acidv1.Resources{
992+
Resources: &acidv1.Resources{
993993
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
994994
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
995995
},
@@ -1108,7 +1108,7 @@ func TestAdditionalVolume(t *testing.T) {
11081108
},
11091109
Spec: acidv1.PostgresSpec{
11101110
TeamID: "myapp", NumberOfInstances: 1,
1111-
Resources: acidv1.Resources{
1111+
Resources: &acidv1.Resources{
11121112
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
11131113
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
11141114
},
@@ -1214,7 +1214,7 @@ func TestSidecars(t *testing.T) {
12141214
},
12151215
},
12161216
TeamID: "myapp", NumberOfInstances: 1,
1217-
Resources: acidv1.Resources{
1217+
Resources: &acidv1.Resources{
12181218
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
12191219
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
12201220
},
@@ -1227,7 +1227,7 @@ func TestSidecars(t *testing.T) {
12271227
},
12281228
acidv1.Sidecar{
12291229
Name: "cluster-specific-sidecar-with-resources",
1230-
Resources: acidv1.Resources{
1230+
Resources: &acidv1.Resources{
12311231
ResourceRequests: acidv1.ResourceDescription{CPU: "210m", Memory: "0.8Gi"},
12321232
ResourceLimits: acidv1.ResourceDescription{CPU: "510m", Memory: "1.4Gi"},
12331233
},
@@ -1389,7 +1389,7 @@ func TestGenerateService(t *testing.T) {
13891389
var enableLB bool = true
13901390
spec = acidv1.PostgresSpec{
13911391
TeamID: "myapp", NumberOfInstances: 1,
1392-
Resources: acidv1.Resources{
1392+
Resources: &acidv1.Resources{
13931393
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
13941394
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
13951395
},
@@ -1402,7 +1402,7 @@ func TestGenerateService(t *testing.T) {
14021402
},
14031403
acidv1.Sidecar{
14041404
Name: "cluster-specific-sidecar-with-resources",
1405-
Resources: acidv1.Resources{
1405+
Resources: &acidv1.Resources{
14061406
ResourceRequests: acidv1.ResourceDescription{CPU: "210m", Memory: "0.8Gi"},
14071407
ResourceLimits: acidv1.ResourceDescription{CPU: "510m", Memory: "1.4Gi"},
14081408
},

0 commit comments

Comments
 (0)