diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index e05a54553..9bc39a9db 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1303,6 +1303,9 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef c.logger.Warningf("initContainers specified but disabled in configuration - next statefulset creation would fail") } initContainers = spec.InitContainers + if err := c.validateContainers(initContainers); err != nil { + return nil, fmt.Errorf("invalid init containers: %v", err) + } } // backward compatible check for InitContainers @@ -1455,6 +1458,10 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef sidecarContainers = patchSidecarContainers(sidecarContainers, volumeMounts, c.OpConfig.SuperUsername, c.credentialSecretName(c.OpConfig.SuperUsername)) + if err := c.validateContainers(sidecarContainers); err != nil { + return nil, fmt.Errorf("invalid sidecar containers: %v", err) + } + tolerationSpec := tolerations(&spec.Tolerations, c.OpConfig.PodToleration) effectivePodPriorityClassName := util.Coalesce(spec.PodPriorityClassName, c.OpConfig.PodPriorityClassName) @@ -2592,3 +2599,15 @@ func ensurePath(file string, defaultDir string, defaultFile string) string { } return file } + +func (c *Cluster) validateContainers(containers []v1.Container) error { + for i, container := range containers { + if container.Name == "" { + return fmt.Errorf("container[%d]: name is required", i) + } + if container.Image == "" { + return fmt.Errorf("container '%v': image is required", container.Name) + } + } + return nil +} diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 137c24081..6bd87366d 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -1935,7 +1935,8 @@ func TestAdditionalVolume(t *testing.T) { AdditionalVolumes: additionalVolumes, Sidecars: []acidv1.Sidecar{ { - Name: sidecarName, + Name: sidecarName, + DockerImage: "test-image", }, }, }, @@ -2163,10 +2164,12 @@ func TestSidecars(t *testing.T) { }, Sidecars: []acidv1.Sidecar{ { - Name: "cluster-specific-sidecar", + Name: "cluster-specific-sidecar", + DockerImage: "test-image", }, { - Name: "cluster-specific-sidecar-with-resources", + Name: "cluster-specific-sidecar-with-resources", + DockerImage: "test-image", Resources: &acidv1.Resources{ ResourceRequests: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("210m"), Memory: k8sutil.StringToPointer("0.8Gi")}, ResourceLimits: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("510m"), Memory: k8sutil.StringToPointer("1.4Gi")}, @@ -2201,7 +2204,8 @@ func TestSidecars(t *testing.T) { }, SidecarContainers: []v1.Container{ { - Name: "global-sidecar", + Name: "global-sidecar", + Image: "test-image", }, // will be replaced by a cluster specific sidecar with the same name { @@ -2271,6 +2275,7 @@ func TestSidecars(t *testing.T) { // cluster specific sidecar assert.Contains(t, s.Spec.Template.Spec.Containers, v1.Container{ Name: "cluster-specific-sidecar", + Image: "test-image", Env: env, Resources: generateKubernetesResources("200m", "500m", "0.7Gi", "1.3Gi"), ImagePullPolicy: v1.PullIfNotPresent, @@ -2297,6 +2302,7 @@ func TestSidecars(t *testing.T) { // global sidecar assert.Contains(t, s.Spec.Template.Spec.Containers, v1.Container{ Name: "global-sidecar", + Image: "test-image", Env: env, VolumeMounts: mounts, }) @@ -2325,6 +2331,180 @@ func TestSidecars(t *testing.T) { } +func TestContainerValidation(t *testing.T) { + testCases := []struct { + name string + spec acidv1.PostgresSpec + clusterConfig Config + expectedError string + }{ + { + name: "init container without image", + spec: acidv1.PostgresSpec{ + PostgresqlParam: acidv1.PostgresqlParam{ + PgVersion: "17", + }, + TeamID: "myapp", + NumberOfInstances: 1, + Volume: acidv1.Volume{ + Size: "1G", + }, + InitContainers: []v1.Container{ + { + Name: "invalid-initcontainer", + }, + }, + }, + clusterConfig: Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + ProtectedRoles: []string{"admin"}, + Auth: config.Auth{ + SuperUsername: superUserName, + ReplicationUsername: replicationUserName, + }, + }, + }, + expectedError: "image is required", + }, + { + name: "sidecar without name", + spec: acidv1.PostgresSpec{ + PostgresqlParam: acidv1.PostgresqlParam{ + PgVersion: "17", + }, + TeamID: "myapp", + NumberOfInstances: 1, + Volume: acidv1.Volume{ + Size: "1G", + }, + }, + clusterConfig: Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + ProtectedRoles: []string{"admin"}, + Auth: config.Auth{ + SuperUsername: superUserName, + ReplicationUsername: replicationUserName, + }, + SidecarContainers: []v1.Container{ + { + Image: "test-image", + }, + }, + }, + }, + expectedError: "name is required", + }, + { + name: "sidecar without image", + spec: acidv1.PostgresSpec{ + PostgresqlParam: acidv1.PostgresqlParam{ + PgVersion: "17", + }, + TeamID: "myapp", + NumberOfInstances: 1, + Volume: acidv1.Volume{ + Size: "1G", + }, + Sidecars: []acidv1.Sidecar{ + { + Name: "invalid-sidecar", + }, + }, + }, + clusterConfig: Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + ProtectedRoles: []string{"admin"}, + Auth: config.Auth{ + SuperUsername: superUserName, + ReplicationUsername: replicationUserName, + }, + }, + }, + expectedError: "image is required", + }, + { + name: "valid containers pass validation", + spec: acidv1.PostgresSpec{ + PostgresqlParam: acidv1.PostgresqlParam{ + PgVersion: "17", + }, + TeamID: "myapp", + NumberOfInstances: 1, + Volume: acidv1.Volume{ + Size: "1G", + }, + Sidecars: []acidv1.Sidecar{ + { + Name: "valid-sidecar", + DockerImage: "busybox:latest", + }, + }, + InitContainers: []v1.Container{ + { + Name: "valid-initcontainer", + Image: "alpine:latest", + }, + }, + }, + clusterConfig: Config{ + OpConfig: config.Config{ + PodManagementPolicy: "ordered_ready", + ProtectedRoles: []string{"admin"}, + Auth: config.Auth{ + SuperUsername: superUserName, + ReplicationUsername: replicationUserName, + }, + }, + }, + expectedError: "", + }, + { + name: "multiple invalid sidecars", + spec: acidv1.PostgresSpec{ + Sidecars: []acidv1.Sidecar{ + { + Name: "sidecar1", + }, + { + Name: "sidecar2", + }, + }, + }, + expectedError: "image is required", + }, + { + name: "empty container name and image", + spec: acidv1.PostgresSpec{ + InitContainers: []v1.Container{ + { + Name: "", + Image: "", + }, + }, + }, + expectedError: "name is required", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cluster := New(tc.clusterConfig, k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger, eventRecorder) + + _, err := cluster.generateStatefulSet(&tc.spec) + + if tc.expectedError != "" { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedError) + } else { + assert.NoError(t, err) + } + }) + } +} + func TestGeneratePodDisruptionBudget(t *testing.T) { testName := "Test PodDisruptionBudget spec generation" @@ -2618,7 +2798,8 @@ func TestGenerateService(t *testing.T) { Name: "cluster-specific-sidecar", }, { - Name: "cluster-specific-sidecar-with-resources", + Name: "cluster-specific-sidecar-with-resources", + DockerImage: "test-image", Resources: &acidv1.Resources{ ResourceRequests: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("210m"), Memory: k8sutil.StringToPointer("0.8Gi")}, ResourceLimits: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("510m"), Memory: k8sutil.StringToPointer("1.4Gi")}, @@ -2928,6 +3109,7 @@ func TestGenerateResourceRequirements(t *testing.T) { namespace := "default" clusterNameLabel := "cluster-name" sidecarName := "postgres-exporter" + dockerImage := "test-image" // enforceMinResourceLimits will be called 2 times emitting 4 events (2x cpu, 2x memory raise) // enforceMaxResourceRequests will be called 4 times emitting 6 events (2x cpu, 4x memory cap) @@ -2993,7 +3175,8 @@ func TestGenerateResourceRequirements(t *testing.T) { Spec: acidv1.PostgresSpec{ Sidecars: []acidv1.Sidecar{ { - Name: sidecarName, + Name: sidecarName, + DockerImage: dockerImage, }, }, TeamID: "acid", @@ -3232,7 +3415,8 @@ func TestGenerateResourceRequirements(t *testing.T) { Spec: acidv1.PostgresSpec{ Sidecars: []acidv1.Sidecar{ { - Name: sidecarName, + Name: sidecarName, + DockerImage: dockerImage, Resources: &acidv1.Resources{ ResourceRequests: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("10m"), Memory: k8sutil.StringToPointer("10Mi")}, ResourceLimits: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("100m"), Memory: k8sutil.StringToPointer("100Mi")}, @@ -3321,7 +3505,8 @@ func TestGenerateResourceRequirements(t *testing.T) { Spec: acidv1.PostgresSpec{ Sidecars: []acidv1.Sidecar{ { - Name: sidecarName, + Name: sidecarName, + DockerImage: dockerImage, Resources: &acidv1.Resources{ ResourceRequests: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("10m"), Memory: k8sutil.StringToPointer("10Mi")}, ResourceLimits: acidv1.ResourceDescription{CPU: k8sutil.StringToPointer("100m"), Memory: k8sutil.StringToPointer("100Mi")},