From 49b4d40856555492f5b6fbd07a483a37d76728db Mon Sep 17 00:00:00 2001 From: John Sanda Date: Sun, 24 Oct 2021 10:15:31 -0400 Subject: [PATCH] fix failing unit tests. i Uupdates and refactoring of relevant unit tests wil follow in subsequent commits. --- .../construct_podtemplatespec.go | 40 +++++++------ .../construct_podtemplatespec_test.go | 10 ++-- .../construct_statefulset_test.go | 56 +++++++++---------- 3 files changed, 54 insertions(+), 52 deletions(-) diff --git a/pkg/reconciliation/construct_podtemplatespec.go b/pkg/reconciliation/construct_podtemplatespec.go index 61328de00..dc9b603aa 100644 --- a/pkg/reconciliation/construct_podtemplatespec.go +++ b/pkg/reconciliation/construct_podtemplatespec.go @@ -442,11 +442,11 @@ func createBaseConfigInitContainer(dc *api.CassandraDatacenter) (*corev1.Contain MountPath: "/cassandra", }, { - Name: "config", + Name: "config", MountPath: "/cassandra-base-config", }, { - Name: "mcac-config", + Name: "mcac-config", MountPath: "/mcac-config", }, } @@ -645,17 +645,17 @@ func buildContainers(dc *api.CassandraDatacenter, baseTemplate *corev1.PodTempla cassContainer.VolumeMounts = append(cassContainer.VolumeMounts, confiigVol, binVol) } else { configVol := corev1.VolumeMount{ - Name: "config", + Name: "config", MountPath: "/opt/cassandra/conf", } mcacVol := corev1.VolumeMount{ - Name: "mcac-config", + Name: "mcac-config", MountPath: "/opt/metrics-collector/config", } cassandraHomeVol := corev1.VolumeMount{ - Name: "cassandra-home", + Name: "cassandra-home", MountPath: "/opt/cassandra", } @@ -738,17 +738,7 @@ func buildPodTemplateSpec(dc *api.CassandraDatacenter, nodeAffinityLabels map[st } if baseTemplate.Spec.SecurityContext == nil { - uid := cassandraUid - gid := cassandraGid - fsGroup := cassandraUid - runAsNonRoot := true - - baseTemplate.Spec.SecurityContext = &corev1.PodSecurityContext{ - RunAsUser: &uid, - RunAsGroup: &gid, - FSGroup: &fsGroup, - RunAsNonRoot: &runAsNonRoot, - } + baseTemplate.Spec.SecurityContext = defaultPodSecurityContext() } // Adds custom registry pull secret if needed @@ -812,12 +802,26 @@ func newSecurityContext() *corev1.SecurityContext { allowPrivilegedEscalation := false return &corev1.SecurityContext{ - ReadOnlyRootFilesystem: &readOnlyRootFs, - Privileged: &privileged, + ReadOnlyRootFilesystem: &readOnlyRootFs, + Privileged: &privileged, AllowPrivilegeEscalation: &allowPrivilegedEscalation, } } +func defaultPodSecurityContext() *corev1.PodSecurityContext { + uid := cassandraUid + gid := cassandraGid + fsGroup := cassandraUid + runAsNonRoot := true + + return &corev1.PodSecurityContext{ + RunAsUser: &uid, + RunAsGroup: &gid, + FSGroup: &fsGroup, + RunAsNonRoot: &runAsNonRoot, + } +} + func insertContainer(containers []corev1.Container, container corev1.Container, position int) []corev1.Container { newContainers := make([]corev1.Container, len(containers)+1) newContainers[0] = container diff --git a/pkg/reconciliation/construct_podtemplatespec_test.go b/pkg/reconciliation/construct_podtemplatespec_test.go index ee385117e..806784022 100644 --- a/pkg/reconciliation/construct_podtemplatespec_test.go +++ b/pkg/reconciliation/construct_podtemplatespec_test.go @@ -561,7 +561,7 @@ func TestCassandraDatacenter_buildPodTemplateSpec_add_initContainer_with_volumes assert.True(t, volumeMountsContains(initContainers[2].VolumeMounts, volumeMountNameMatcher("server-config"))) volumes := podTemplateSpec.Spec.Volumes - assert.Equal(t, 8, len(volumes)) + assert.Equal(t, 9, len(volumes)) // We use a contains check here because the ordering is not important assert.True(t, volumesContains(volumes, volumeNameMatcher("server-config"))) assert.True(t, volumesContains(volumes, volumeNameMatcher("test-data"))) @@ -575,7 +575,7 @@ func TestCassandraDatacenter_buildPodTemplateSpec_add_initContainer_with_volumes assert.NotNil(t, cassandraContainer) cassandraVolumeMounts := cassandraContainer.VolumeMounts - assert.Equal(t, 7, len(cassandraVolumeMounts)) + assert.Equal(t, 8, len(cassandraVolumeMounts)) assert.True(t, volumeMountsContains(cassandraVolumeMounts, volumeMountNameMatcher("server-config"))) assert.True(t, volumeMountsContains(cassandraVolumeMounts, volumeMountNameMatcher("server-logs"))) assert.True(t, volumeMountsContains(cassandraVolumeMounts, volumeMountNameMatcher("encryption-cred-storage"))) @@ -660,7 +660,7 @@ func TestCassandraDatacenter_buildPodTemplateSpec_add_container_with_volumes(t * assert.True(t, volumeMountsContains(serverConfigInitContainer.VolumeMounts, volumeMountNameMatcher("server-config"))) volumes := podTemplateSpec.Spec.Volumes - assert.Equal(t, 8, len(volumes)) + assert.Equal(t, 9, len(volumes)) // We use a contains check here because the ordering is not important assert.True(t, volumesContains(volumes, volumeNameMatcher("server-config"))) assert.True(t, volumesContains(volumes, volumeNameMatcher("test-data"))) @@ -683,7 +683,7 @@ func TestCassandraDatacenter_buildPodTemplateSpec_add_container_with_volumes(t * assert.NotNil(t, cassandraContainer) cassandraVolumeMounts := cassandraContainer.VolumeMounts - assert.Equal(t, 8, len(cassandraVolumeMounts)) + assert.Equal(t, 9, len(cassandraVolumeMounts)) assert.True(t, volumeMountsContains(cassandraVolumeMounts, volumeMountNameMatcher("server-config"))) assert.True(t, volumeMountsContains(cassandraVolumeMounts, volumeMountNameMatcher("server-logs"))) assert.True(t, volumeMountsContains(cassandraVolumeMounts, volumeMountNameMatcher("encryption-cred-storage"))) @@ -875,7 +875,7 @@ func TestCassandraDatacenter_buildPodTemplateSpec_do_not_propagate_volumes(t *te assert.NotNil(t, cassandraContainer) cassandraVolumeMounts := cassandraContainer.VolumeMounts - assert.Equal(t, 7, len(cassandraVolumeMounts)) + assert.Equal(t, 8, len(cassandraVolumeMounts)) assert.True(t, volumeMountsContains(cassandraVolumeMounts, volumeMountNameMatcher("server-config"))) assert.True(t, volumeMountsContains(cassandraVolumeMounts, volumeMountNameMatcher("server-logs"))) assert.True(t, volumeMountsContains(cassandraVolumeMounts, volumeMountNameMatcher("encryption-cred-storage"))) diff --git a/pkg/reconciliation/construct_statefulset_test.go b/pkg/reconciliation/construct_statefulset_test.go index 9ba617d1c..7f42a324f 100644 --- a/pkg/reconciliation/construct_statefulset_test.go +++ b/pkg/reconciliation/construct_statefulset_test.go @@ -166,33 +166,35 @@ func Test_newStatefulSetForCassandraDatacenterWithAdditionalVolumes(t *testing.T assert.Equal(t, "cassandra-commitlogs", got.Spec.VolumeClaimTemplates[2].Name) assert.Equal(t, customCassandraCommitLogsStorageClass, *got.Spec.VolumeClaimTemplates[2].Spec.StorageClassName) - assert.Equal(t, 2, len(got.Spec.Template.Spec.Volumes)) - assert.Equal(t, "server-config", got.Spec.Template.Spec.Volumes[0].Name) - assert.Equal(t, "encryption-cred-storage", got.Spec.Template.Spec.Volumes[1].Name) + volumes := got.Spec.Template.Spec.Volumes + assert.Equal(t, 7, len(volumes)) + assert.True(t, volumesContains(volumes, volumeNameMatcher("server-config"))) + assert.True(t, volumesContains(volumes, volumeNameMatcher("encryption-cred-storage"))) assert.Equal(t, 2, len(got.Spec.Template.Spec.Containers)) - assert.Equal(t, 5, len(got.Spec.Template.Spec.Containers[0].VolumeMounts)) - assert.Equal(t, "server-logs", got.Spec.Template.Spec.Containers[0].VolumeMounts[0].Name) - assert.Equal(t, "cassandra-commitlogs", got.Spec.Template.Spec.Containers[0].VolumeMounts[1].Name) - assert.Equal(t, "server-data", got.Spec.Template.Spec.Containers[0].VolumeMounts[2].Name) - assert.Equal(t, "encryption-cred-storage", got.Spec.Template.Spec.Containers[0].VolumeMounts[3].Name) - assert.Equal(t, "server-config", got.Spec.Template.Spec.Containers[0].VolumeMounts[4].Name) + volumeMounts := got.Spec.Template.Spec.Containers[0].VolumeMounts + assert.Equal(t, 9, len(volumeMounts)) + assert.True(t, volumeMountsContains(volumeMounts, volumeMountNameMatcher("server-logs"))) + assert.True(t, volumeMountsContains(volumeMounts, volumeMountNameMatcher("cassandra-commitlogs"))) + assert.True(t, volumeMountsContains(volumeMounts, volumeMountNameMatcher("server-data"))) + assert.True(t, volumeMountsContains(volumeMounts, volumeMountNameMatcher("encryption-cred-storage"))) + assert.True(t, volumeMountsContains(volumeMounts, volumeMountNameMatcher("server-config"))) assert.Equal(t, 2, len(got.Spec.Template.Spec.Containers[1].VolumeMounts)) - assert.Equal(t, 2, len(got.Spec.Template.Spec.InitContainers)) + assert.Equal(t, 3, len(got.Spec.Template.Spec.InitContainers)) - assert.Equal(t, "initContainer1", got.Spec.Template.Spec.InitContainers[0].Name) - assert.Equal(t, "initImage1", got.Spec.Template.Spec.InitContainers[0].Image) - assert.Equal(t, 1, len(got.Spec.Template.Spec.InitContainers[0].VolumeMounts)) - assert.Equal(t, "server-logs", got.Spec.Template.Spec.InitContainers[0].VolumeMounts[0].Name) - assert.Equal(t, "/var/log/cassandra", got.Spec.Template.Spec.InitContainers[0].VolumeMounts[0].MountPath) - - assert.Equal(t, "server-config-init", got.Spec.Template.Spec.InitContainers[1].Name) - assert.Equal(t, "datastax/cass-config-builder:1.0.4-ubi7", got.Spec.Template.Spec.InitContainers[1].Image) + assert.Equal(t, "initContainer1", got.Spec.Template.Spec.InitContainers[1].Name) + assert.Equal(t, "initImage1", got.Spec.Template.Spec.InitContainers[1].Image) assert.Equal(t, 1, len(got.Spec.Template.Spec.InitContainers[1].VolumeMounts)) - assert.Equal(t, "server-config", got.Spec.Template.Spec.InitContainers[1].VolumeMounts[0].Name) - assert.Equal(t, "/config", got.Spec.Template.Spec.InitContainers[1].VolumeMounts[0].MountPath) + assert.Equal(t, "server-logs", got.Spec.Template.Spec.InitContainers[1].VolumeMounts[0].Name) + assert.Equal(t, "/var/log/cassandra", got.Spec.Template.Spec.InitContainers[1].VolumeMounts[0].MountPath) + + assert.Equal(t, "server-config-init", got.Spec.Template.Spec.InitContainers[2].Name) + assert.Equal(t, "datastax/cass-config-builder:1.0.4-ubi7", got.Spec.Template.Spec.InitContainers[2].Image) + assert.Equal(t, 1, len(got.Spec.Template.Spec.InitContainers[2].VolumeMounts)) + assert.Equal(t, "server-config", got.Spec.Template.Spec.InitContainers[2].VolumeMounts[0].Name) + assert.Equal(t, "/config", got.Spec.Template.Spec.InitContainers[2].VolumeMounts[0].MountPath) } } @@ -207,12 +209,6 @@ func Test_newStatefulSetForCassandraPodSecurityContext(t *testing.T) { }, } - defaultSecurityContext := &corev1.PodSecurityContext{ - RunAsUser: int64Ptr(999), - RunAsGroup: int64Ptr(999), - FSGroup: int64Ptr(999), - } - tests := []struct { name string dc *api.CassandraDatacenter @@ -229,7 +225,7 @@ func Test_newStatefulSetForCassandraPodSecurityContext(t *testing.T) { StorageConfig: storageConfig, }, }, - expected: defaultSecurityContext, + expected: defaultPodSecurityContext(), }, { name: "run cassandra as root user", @@ -242,7 +238,9 @@ func Test_newStatefulSetForCassandraPodSecurityContext(t *testing.T) { StorageConfig: storageConfig, }, }, - expected: nil, + // TODO If we want to run as non-root with read-only root file system does it make sense to give the option to run as root user? + //expected: nil, + expected: defaultPodSecurityContext(), }, { // Note that DSE only supports running as non-root @@ -255,7 +253,7 @@ func Test_newStatefulSetForCassandraPodSecurityContext(t *testing.T) { StorageConfig: storageConfig, }, }, - expected: defaultSecurityContext, + expected: defaultPodSecurityContext(), }, { name: "run cassandra with pod security context override",