[2.16] Only conditionally set Kibana default security context. (#8343) (

#8344) * Only conditionally set Kibana default security context. (#8343) * Use flag for whether we set pod security context. --------- Signed-off-by: Michael Montgomery <[email protected]> (cherry picked from commit 7792ac9) # Conflicts: # docs/release-notes/highlights-2.16.0.asciidoc * rem highlights for now --------- Co-authored-by: Michael Montgomery <[email protected]>
elastic · Dec 17, 2024 · 837bef4 · 837bef4
1 parent d6d1d8d
commit 837bef4
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 19 deletions.
diff --git a/cmd/manager/main.go b/cmd/manager/main.go
@@ -353,7 +353,7 @@ func Command() *cobra.Command {
 	cmd.Flags().String(
 		operator.SetDefaultSecurityContextFlag,
 		"auto-detect",
-		"Enables setting the default security context with fsGroup=1000 for Elasticsearch 8.0+ Pods. Ignored pre-8.0. Possible values: true, false, auto-detect",
+		"Enables setting the default security context with fsGroup=1000 for Elasticsearch 8.0+ Pods and Kibana 7.10+ Pods. Possible values: true, false, auto-detect",
 	)
 
 	// hide development mode flags from the usage message

diff --git a/pkg/controller/common/operator/parameters.go b/pkg/controller/common/operator/parameters.go
@@ -42,7 +42,7 @@ type Parameters struct {
 	// MaxConcurrentReconciles controls the number of goroutines per controller.
 	MaxConcurrentReconciles int
 	// SetDefaultSecurityContext enables setting the default security context
-	// with fsGroup=1000 for Elasticsearch 8.0+ Pods. Ignored pre-8.0
+	// with fsGroup=1000 for Elasticsearch 8.0+ Pods, and Kibana 7.10+ Pods.
 	SetDefaultSecurityContext bool
 	// ValidateStorageClass specifies whether the operator should retrieve storage classes to verify volume expansion support.
 	// Can be disabled if cluster-wide storage class RBAC access is not available.

diff --git a/pkg/controller/kibana/driver.go b/pkg/controller/kibana/driver.go
@@ -178,7 +178,7 @@ func (d *driver) Reconcile(
 	span, _ := apm.StartSpan(ctx, "reconcile_deployment", tracing.SpanTypeApp)
 	defer span.End()
 
-	deploymentParams, err := d.deploymentParams(ctx, kb, kibanaPolicyCfg.PodAnnotations, basePath)
+	deploymentParams, err := d.deploymentParams(ctx, kb, kibanaPolicyCfg.PodAnnotations, basePath, params.SetDefaultSecurityContext)
 	if err != nil {
 		return results.WithError(err)
 	}
@@ -225,7 +225,7 @@ func (d *driver) getStrategyType(kb *kbv1.Kibana) (appsv1.DeploymentStrategyType
 	return appsv1.RollingUpdateDeploymentStrategyType, nil
 }
 
-func (d *driver) deploymentParams(ctx context.Context, kb *kbv1.Kibana, policyAnnotations map[string]string, basePath string) (deployment.Params, error) {
+func (d *driver) deploymentParams(ctx context.Context, kb *kbv1.Kibana, policyAnnotations map[string]string, basePath string, setDefaultSecurityContext bool) (deployment.Params, error) {
 	initContainersParameters, err := newInitContainersParameters(kb)
 	if err != nil {
 		return deployment.Params{}, err
@@ -247,7 +247,7 @@ func (d *driver) deploymentParams(ctx context.Context, kb *kbv1.Kibana, policyAn
 	if err != nil {
 		return deployment.Params{}, err
 	}
-	kibanaPodSpec, err := NewPodTemplateSpec(ctx, d.client, *kb, keystoreResources, volumes, basePath)
+	kibanaPodSpec, err := NewPodTemplateSpec(ctx, d.client, *kb, keystoreResources, volumes, basePath, setDefaultSecurityContext)
 	if err != nil {
 		return deployment.Params{}, err
 	}

diff --git a/pkg/controller/kibana/driver_test.go b/pkg/controller/kibana/driver_test.go
@@ -19,6 +19,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	commonv1 "github.com/elastic/cloud-on-k8s/v2/pkg/apis/common/v1"
@@ -197,9 +198,10 @@ func Test_getStrategyType(t *testing.T) {
 
 func TestDriverDeploymentParams(t *testing.T) {
 	type args struct {
-		kb                func() *kbv1.Kibana
-		initialObjects    func() []client.Object
-		policyAnnotations map[string]string
+		kb                            func() *kbv1.Kibana
+		initialObjects                func() []client.Object
+		policyAnnotations             map[string]string
+		setDefaultSecurityContextFlag bool
 	}
 
 	tests := []struct {
@@ -371,11 +373,33 @@ func TestDriverDeploymentParams(t *testing.T) {
 					kb.Spec.Version = "7.10.0"
 					return kb
 				},
-				initialObjects: defaultInitialObjects,
+				initialObjects:                defaultInitialObjects,
+				setDefaultSecurityContextFlag: true,
 			},
 			want: func() deployment.Params {
 				p := expectedDeploymentParams()
 				p.PodTemplateSpec.Labels["kibana.k8s.elastic.co/version"] = "7.10.0"
+				p.PodTemplateSpec.Spec.SecurityContext = &corev1.PodSecurityContext{
+					FSGroup: ptr.To[int64](1000),
+				}
+				return p
+			}(),
+			wantErr: false,
+		},
+		{
+			name: "7.10+ does not contain default security context when flag is not set",
+			args: args{
+				kb: func() *kbv1.Kibana {
+					kb := kibanaFixture()
+					kb.Spec.Version = "7.10.0"
+					return kb
+				},
+				initialObjects:                defaultInitialObjects,
+				setDefaultSecurityContextFlag: false,
+			},
+			want: func() deployment.Params {
+				p := pre710(expectedDeploymentParams())
+				p.PodTemplateSpec.Labels["kibana.k8s.elastic.co/version"] = "7.10.0"
 				return p
 			}(),
 			wantErr: false,
@@ -392,7 +416,7 @@ func TestDriverDeploymentParams(t *testing.T) {
 			d, err := newDriver(client, w, record.NewFakeRecorder(100), kb, corev1.IPv4Protocol)
 			require.NoError(t, err)
 
-			got, err := d.deploymentParams(context.Background(), kb, tt.args.policyAnnotations, "")
+			got, err := d.deploymentParams(context.Background(), kb, tt.args.policyAnnotations, "", tt.args.setDefaultSecurityContextFlag)
 			if tt.wantErr {
 				require.Error(t, err)
 				return
@@ -642,7 +666,6 @@ func expectedDeploymentParams() deployment.Params {
 					SecurityContext: &defaultSecurityContext,
 				}},
 				AutomountServiceAccountToken: &falseVal,
-				SecurityContext:              &defaultPodSecurityContext,
 			},
 		},
 	}

diff --git a/pkg/controller/kibana/pod.go b/pkg/controller/kibana/pod.go
@@ -39,6 +39,8 @@ const (
 	TempVolumeMountPath          = "/tmp"
 	KibanaBasePathEnvName        = "SERVER_BASEPATH"
 	KibanaRewriteBasePathEnvName = "SERVER_REWRITEBASEPATH"
+	defaultFSGroup               = 1000
+	defaultFSUser                = 1000
 )
 
 var (
@@ -108,6 +110,7 @@ func NewPodTemplateSpec(
 	keystore *keystore.Resources,
 	volumes []volume.VolumeLike,
 	basePath string,
+	setDefaultSecurityContext bool,
 ) (corev1.PodTemplateSpec, error) {
 	labels := kb.GetIdentityLabels()
 	labels[kblabel.KibanaVersionLabelName] = kb.Spec.Version
@@ -137,9 +140,9 @@ func NewPodTemplateSpec(
 	// Limiting to 7.10.0 here as there was a bug in previous versions causing rebuilding
 	// of browser bundles to happen on plugin install, which would attempt a write to the
 	// root filesystem on restart.
-	if v.GTE(version.From(7, 10, 0)) {
-		builder.WithPodSecurityContext(defaultPodSecurityContext).
-			WithContainersSecurityContext(defaultSecurityContext).
+	if v.GTE(version.From(7, 10, 0)) && setDefaultSecurityContext {
+		builder.WithContainersSecurityContext(defaultSecurityContext).
+			WithPodSecurityContext(defaultPodSecurityContext).
 			WithVolumes(TempVolume.Volume()).WithVolumeMounts(TempVolume.VolumeMount()).
 			WithVolumes(PluginsVolume.Volume()).WithVolumeMounts(PluginsVolume.VolumeMount())
 	}

diff --git a/pkg/controller/kibana/pod_test.go b/pkg/controller/kibana/pod_test.go
@@ -222,7 +222,6 @@ func TestNewPodTemplateSpec(t *testing.T) {
 				assert.Len(t, pod.Spec.InitContainers[0].VolumeMounts, 5)
 				assert.Len(t, pod.Spec.Volumes, 3)
 				assert.Len(t, GetKibanaContainer(pod.Spec).VolumeMounts, 3)
-				assert.Equal(t, pod.Spec.SecurityContext, &defaultPodSecurityContext)
 				assert.Equal(t, GetKibanaContainer(pod.Spec).SecurityContext, &defaultSecurityContext)
 			},
 		},
@@ -373,7 +372,7 @@ func TestNewPodTemplateSpec(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			bp, err := GetKibanaBasePath(tt.kb)
 			require.NoError(t, err)
-			got, err := NewPodTemplateSpec(context.Background(), k8s.NewFakeClient(), tt.kb, tt.keystore, []commonvolume.VolumeLike{}, bp)
+			got, err := NewPodTemplateSpec(context.Background(), k8s.NewFakeClient(), tt.kb, tt.keystore, []commonvolume.VolumeLike{}, bp, true)
 			assert.NoError(t, err)
 			tt.assertions(got)
 		})

diff --git a/pkg/controller/kibana/securitycontext.go b/pkg/controller/kibana/securitycontext.go
@@ -19,10 +19,10 @@ var (
 		},
 		Privileged:             ptr.To(bool(false)),
 		ReadOnlyRootFilesystem: ptr.To(bool(true)),
-		RunAsUser:              ptr.To(int64(1000)),
-		RunAsGroup:             ptr.To(int64(1000)),
+		RunAsUser:              ptr.To(int64(defaultFSUser)),
+		RunAsGroup:             ptr.To(int64(defaultFSGroup)),
 	}
 	defaultPodSecurityContext = corev1.PodSecurityContext{
-		FSGroup: ptr.To(int64(1000)),
+		FSGroup: ptr.To(int64(defaultFSGroup)),
 	}
 )