Fix empty resources spec field failing schema validation (#1589)

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.
This commit is contained in:
Jakob Gillich 2022-03-18 16:16:32 +01:00 committed by GitHub
parent 1d88009ec4
commit f3b83c0b05
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 35 additions and 30 deletions

View File

@ -27,7 +27,7 @@ type PostgresSpec struct {
PostgresqlParam `json:"postgresql"`
Volume `json:"volume,omitempty"`
Patroni `json:"patroni,omitempty"`
Resources `json:"resources,omitempty"`
*Resources `json:"resources,omitempty"`
EnableConnectionPooler *bool `json:"enableConnectionPooler,omitempty"`
EnableReplicaConnectionPooler *bool `json:"enableReplicaConnectionPooler,omitempty"`
@ -199,7 +199,7 @@ type CloneDescription struct {
// Sidecar defines a container to be run in the same pod as the Postgres container.
type Sidecar struct {
Resources `json:"resources,omitempty"`
*Resources `json:"resources,omitempty"`
Name string `json:"name,omitempty"`
DockerImage string `json:"image,omitempty"`
Ports []v1.ContainerPort `json:"ports,omitempty"`
@ -232,7 +232,7 @@ type ConnectionPooler struct {
DockerImage string `json:"dockerImage,omitempty"`
MaxDBConnections *int32 `json:"maxDBConnections,omitempty"`
Resources `json:"resources,omitempty"`
*Resources `json:"resources,omitempty"`
}
type Stream struct {

View File

@ -163,7 +163,7 @@ var unmarshalCluster = []struct {
"kind": "Postgresql","apiVersion": "acid.zalan.do/v1",
"metadata": {"name": "acid-testcluster1"}, "spec": {"teamId": 100}}`), &tmp).Error(),
},
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"}`),
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"}`),
err: nil},
{
about: "example with /status subresource",
@ -184,7 +184,7 @@ var unmarshalCluster = []struct {
"kind": "Postgresql","apiVersion": "acid.zalan.do/v1",
"metadata": {"name": "acid-testcluster1"}, "spec": {"teamId": 100}}`), &tmp).Error(),
},
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"}}`),
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"}}`),
err: nil},
{
about: "example with detailed input manifest and deprecated pod_priority_class_name -> podPriorityClassName",
@ -300,7 +300,7 @@ var unmarshalCluster = []struct {
MaximumLagOnFailover: 33554432,
Slots: map[string]map[string]string{"permanent_logical_1": {"type": "logical", "database": "foo", "plugin": "pgoutput"}},
},
Resources: Resources{
Resources: &Resources{
ResourceRequests: ResourceDescription{CPU: "10m", Memory: "50Mi"},
ResourceLimits: ResourceDescription{CPU: "300m", Memory: "3000Mi"},
},
@ -351,7 +351,7 @@ var unmarshalCluster = []struct {
Status: PostgresStatus{PostgresClusterStatus: ClusterStatusInvalid},
Error: errors.New("name must match {TEAM}-{NAME} format").Error(),
},
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"}}`),
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"}}`),
err: nil},
{
about: "example with clone",
@ -373,7 +373,7 @@ var unmarshalCluster = []struct {
},
Error: "",
},
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":""}}`),
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":""}}`),
err: nil},
{
about: "standby example",
@ -395,7 +395,7 @@ var unmarshalCluster = []struct {
},
Error: "",
},
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":""}}`),
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":""}}`),
err: nil},
{
about: "expect error on malformatted JSON",

View File

@ -60,7 +60,7 @@ func TestStatefulSetAnnotations(t *testing.T) {
testName := "CheckStatefulsetAnnotations"
spec := acidv1.PostgresSpec{
TeamID: "myapp", NumberOfInstances: 1,
Resources: acidv1.Resources{
Resources: &acidv1.Resources{
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
},

View File

@ -136,11 +136,18 @@ func (c *Cluster) makeDefaultResources() acidv1.Resources {
}
}
func generateResourceRequirements(resources acidv1.Resources, defaultResources acidv1.Resources) (*v1.ResourceRequirements, error) {
func generateResourceRequirements(resources *acidv1.Resources, defaultResources acidv1.Resources) (*v1.ResourceRequirements, error) {
var err error
specRequests := resources.ResourceRequests
specLimits := resources.ResourceLimits
var specRequests acidv1.ResourceDescription
var specLimits acidv1.ResourceDescription
if resources == nil {
specRequests = acidv1.ResourceDescription{}
specLimits = acidv1.ResourceDescription{}
} else {
specRequests = resources.ResourceRequests
specLimits = resources.ResourceLimits
}
result := v1.ResourceRequirements{}
@ -513,16 +520,14 @@ func generateSidecarContainers(sidecars []acidv1.Sidecar,
if len(sidecars) > 0 {
result := make([]v1.Container, 0)
for index, sidecar := range sidecars {
var resourcesSpec acidv1.Resources
if sidecar.Resources == nil {
resourcesSpec = acidv1.Resources{}
} else {
sidecar.Resources.DeepCopyInto(&resourcesSpec)
}
resources, err := generateResourceRequirements(
makeResources(
sidecar.Resources.ResourceRequests.CPU,
sidecar.Resources.ResourceRequests.Memory,
sidecar.Resources.ResourceLimits.CPU,
sidecar.Resources.ResourceLimits.Memory,
),
defaultResources,
)
resources, err := generateResourceRequirements(&resourcesSpec, defaultResources)
if err != nil {
return nil, err
}
@ -1387,7 +1392,7 @@ func generateScalyrSidecarSpec(clusterName, APIKey, serverURL, dockerImage strin
scalyrCPULimit,
scalyrMemoryLimit,
)
resourceRequirementsScalyrSidecar, err := generateResourceRequirements(resourcesScalyrSidecar, defaultResources)
resourceRequirementsScalyrSidecar, err := generateResourceRequirements(&resourcesScalyrSidecar, defaultResources)
if err != nil {
return nil, fmt.Errorf("invalid resources for Scalyr sidecar: %v", err)
}

View File

@ -915,7 +915,7 @@ func TestNodeAffinity(t *testing.T) {
makeSpec := func(nodeAffinity *v1.NodeAffinity) acidv1.PostgresSpec {
return acidv1.PostgresSpec{
TeamID: "myapp", NumberOfInstances: 1,
Resources: acidv1.Resources{
Resources: &acidv1.Resources{
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
},
@ -1011,7 +1011,7 @@ func TestTLS(t *testing.T) {
},
Spec: acidv1.PostgresSpec{
TeamID: "myapp", NumberOfInstances: 1,
Resources: acidv1.Resources{
Resources: &acidv1.Resources{
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
},
@ -1130,7 +1130,7 @@ func TestAdditionalVolume(t *testing.T) {
},
Spec: acidv1.PostgresSpec{
TeamID: "myapp", NumberOfInstances: 1,
Resources: acidv1.Resources{
Resources: &acidv1.Resources{
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
},
@ -1236,7 +1236,7 @@ func TestSidecars(t *testing.T) {
},
},
TeamID: "myapp", NumberOfInstances: 1,
Resources: acidv1.Resources{
Resources: &acidv1.Resources{
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
},
@ -1249,7 +1249,7 @@ func TestSidecars(t *testing.T) {
},
acidv1.Sidecar{
Name: "cluster-specific-sidecar-with-resources",
Resources: acidv1.Resources{
Resources: &acidv1.Resources{
ResourceRequests: acidv1.ResourceDescription{CPU: "210m", Memory: "0.8Gi"},
ResourceLimits: acidv1.ResourceDescription{CPU: "510m", Memory: "1.4Gi"},
},
@ -1411,7 +1411,7 @@ func TestGenerateService(t *testing.T) {
var enableLB bool = true
spec = acidv1.PostgresSpec{
TeamID: "myapp", NumberOfInstances: 1,
Resources: acidv1.Resources{
Resources: &acidv1.Resources{
ResourceRequests: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
ResourceLimits: acidv1.ResourceDescription{CPU: "1", Memory: "10"},
},
@ -1424,7 +1424,7 @@ func TestGenerateService(t *testing.T) {
},
acidv1.Sidecar{
Name: "cluster-specific-sidecar-with-resources",
Resources: acidv1.Resources{
Resources: &acidv1.Resources{
ResourceRequests: acidv1.ResourceDescription{CPU: "210m", Memory: "0.8Gi"},
ResourceLimits: acidv1.ResourceDescription{CPU: "510m", Memory: "1.4Gi"},
},