From a4551e682b1de95e283bbb5cbd8810606ca6c543 Mon Sep 17 00:00:00 2001 From: David Kowalski Date: Tue, 23 Jun 2020 17:27:26 +0200 Subject: [PATCH 1/6] Add set-default-fsgroup flag to the operator --- cmd/manager/main.go | 6 ++++++ pkg/controller/common/defaults/pod_template.go | 13 ++++++++++++- pkg/controller/common/operator/flags.go | 1 + pkg/controller/common/operator/parameters.go | 3 +++ pkg/controller/elasticsearch/driver/nodes.go | 2 +- pkg/controller/elasticsearch/nodespec/podspec.go | 9 +++++++++ .../elasticsearch/nodespec/podspec_test.go | 15 ++++++++++++++- .../elasticsearch/nodespec/resources.go | 3 ++- .../elasticsearch/nodespec/statefulset.go | 3 ++- 9 files changed, 50 insertions(+), 5 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index b2586769cf..a1d9bda6ed 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -174,6 +174,11 @@ func init() { DefaultWebhookConfigurationName, fmt.Sprintf("Name of the Kubernetes ValidatingWebhookConfiguration resource (defaults to %s). Only used when enable-webhook is true.", DefaultWebhookConfigurationName), ) + Cmd.Flags().Bool( + operator.SetDefaultFsGroupFlag, + true, + "Enables setting the default filesystem group in Pods security context", + ) // enable using dashed notation in flags and underscores in env viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_")) @@ -322,6 +327,7 @@ func execute() { RotateBefore: certRotateBefore, }, MaxConcurrentReconciles: viper.GetInt(operator.MaxConcurrentReconcilesFlag), + SetDefaultFsGroup: viper.GetBool(operator.SetDefaultFsGroupFlag), Tracer: tracer, } diff --git a/pkg/controller/common/defaults/pod_template.go b/pkg/controller/common/defaults/pod_template.go index d9687b1999..afac68eef6 100644 --- a/pkg/controller/common/defaults/pod_template.go +++ b/pkg/controller/common/defaults/pod_template.go @@ -7,10 +7,11 @@ package defaults import ( "sort" + corev1 "k8s.io/api/core/v1" "github.com/elastic/cloud-on-k8s/pkg/controller/common/container" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings" "github.com/elastic/cloud-on-k8s/pkg/utils/maps" - corev1 "k8s.io/api/core/v1" + "github.com/elastic/cloud-on-k8s/pkg/utils/pointer" ) // PodDownwardEnvVars returns default environment variables created from the downward API. @@ -300,3 +301,13 @@ func (b *PodTemplateBuilder) WithAutomountServiceAccountToken() *PodTemplateBuil } return b } + +func (b *PodTemplateBuilder) WithFsGroup(defaultFsGroup int64) *PodTemplateBuilder { + if b.PodTemplate.Spec.SecurityContext == nil { + b.PodTemplate.Spec.SecurityContext = &corev1.PodSecurityContext{} + } + if b.PodTemplate.Spec.SecurityContext.FSGroup == nil { + b.PodTemplate.Spec.SecurityContext.FSGroup = pointer.Int64(defaultFsGroup) + } + return b +} diff --git a/pkg/controller/common/operator/flags.go b/pkg/controller/common/operator/flags.go index 9f5fd45d91..25e9017ed6 100644 --- a/pkg/controller/common/operator/flags.go +++ b/pkg/controller/common/operator/flags.go @@ -20,6 +20,7 @@ const ( MetricsPortFlag = "metrics-port" NamespacesFlag = "namespaces" OperatorNamespaceFlag = "operator-namespace" + SetDefaultFsGroupFlag = "set-default-fsgroup" WebhookCertDirFlag = "webhook-cert-dir" WebhookSecretFlag = "webhook-secret" WebhookConfigurationNameFlag = "webhook-configuration-name" diff --git a/pkg/controller/common/operator/parameters.go b/pkg/controller/common/operator/parameters.go index c29e4eb9e2..a7c3c062b4 100644 --- a/pkg/controller/common/operator/parameters.go +++ b/pkg/controller/common/operator/parameters.go @@ -26,6 +26,9 @@ type Parameters struct { CertRotation certificates.RotationParams // MaxConcurrentReconciles controls the number of goroutines per controller. MaxConcurrentReconciles int + // SetDefaultFsGroup determines whether the operator should set the default + // filesystem group for Pod security context. + SetDefaultFsGroup bool // Tracer is a shared APM tracer instance or nil Tracer *apm.Tracer } diff --git a/pkg/controller/elasticsearch/driver/nodes.go b/pkg/controller/elasticsearch/driver/nodes.go index 75e2618b43..d1108a5420 100644 --- a/pkg/controller/elasticsearch/driver/nodes.go +++ b/pkg/controller/elasticsearch/driver/nodes.go @@ -54,7 +54,7 @@ func (d *defaultDriver) reconcileNodeSpecs( return results.WithError(err) } - expectedResources, err := nodespec.BuildExpectedResources(d.ES, keystoreResources, actualStatefulSets) + expectedResources, err := nodespec.BuildExpectedResources(d.ES, keystoreResources, actualStatefulSets, d.OperatorParameters.SetDefaultFsGroup) if err != nil { return results.WithError(err) } diff --git a/pkg/controller/elasticsearch/nodespec/podspec.go b/pkg/controller/elasticsearch/nodespec/podspec.go index b7964a8694..64c842e5f1 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec.go +++ b/pkg/controller/elasticsearch/nodespec/podspec.go @@ -25,12 +25,17 @@ import ( "github.com/elastic/cloud-on-k8s/pkg/utils/k8s" ) +const ( + defaultFsGroup = 1000 +) + // BuildPodTemplateSpec builds a new PodTemplateSpec for an Elasticsearch node. func BuildPodTemplateSpec( es esv1.Elasticsearch, nodeSet esv1.NodeSet, cfg settings.CanonicalConfig, keystoreResources *keystore.Resources, + setDefaultFsGroup bool, ) (corev1.PodTemplateSpec, error) { volumes, volumeMounts := buildVolumes(es.Name, nodeSet, keystoreResources) labels, err := buildLabels(es, cfg, nodeSet, keystoreResources) @@ -51,6 +56,10 @@ func BuildPodTemplateSpec( } defaultContainerPorts := getDefaultContainerPorts(es) + if setDefaultFsGroup { + builder = builder.WithFsGroup(defaultFsGroup) + } + builder = builder. WithResources(DefaultResources). WithTerminationGracePeriod(DefaultTerminationGracePeriodSeconds). diff --git a/pkg/controller/elasticsearch/nodespec/podspec_test.go b/pkg/controller/elasticsearch/nodespec/podspec_test.go index 2645fc717e..6699f0f7db 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec_test.go +++ b/pkg/controller/elasticsearch/nodespec/podspec_test.go @@ -87,6 +87,19 @@ var sampleES = esv1.Elasticsearch{ }, } +func TestBuildPodTemplateSpecWithDefaultFsGroup(t *testing.T) { + nodeSet := sampleES.Spec.NodeSets[0] + ver, err := version.Parse(sampleES.Spec.Version) + require.NoError(t, err) + cfg, err := settings.NewMergedESConfig(sampleES.Name, *ver, sampleES.Spec.HTTP, *nodeSet.Config) + require.NoError(t, err) + + actual, err := BuildPodTemplateSpec(sampleES, sampleES.Spec.NodeSets[0], cfg, nil, true) + require.NoError(t, err) + + require.Equal(t, int64(1000), *actual.Spec.SecurityContext.FSGroup) +} + func TestBuildPodTemplateSpec(t *testing.T) { nodeSet := sampleES.Spec.NodeSets[0] ver, err := version.Parse(sampleES.Spec.Version) @@ -94,7 +107,7 @@ func TestBuildPodTemplateSpec(t *testing.T) { cfg, err := settings.NewMergedESConfig(sampleES.Name, *ver, sampleES.Spec.HTTP, *nodeSet.Config) require.NoError(t, err) - actual, err := BuildPodTemplateSpec(sampleES, sampleES.Spec.NodeSets[0], cfg, nil) + actual, err := BuildPodTemplateSpec(sampleES, sampleES.Spec.NodeSets[0], cfg, nil, false) require.NoError(t, err) // build expected PodTemplateSpec diff --git a/pkg/controller/elasticsearch/nodespec/resources.go b/pkg/controller/elasticsearch/nodespec/resources.go index 7c989f9b0f..9514e40697 100644 --- a/pkg/controller/elasticsearch/nodespec/resources.go +++ b/pkg/controller/elasticsearch/nodespec/resources.go @@ -38,6 +38,7 @@ func BuildExpectedResources( es esv1.Elasticsearch, keystoreResources *keystore.Resources, existingStatefulSets sset.StatefulSetList, + setDefaultFsGroup bool, ) (ResourcesList, error) { nodesResources := make(ResourcesList, 0, len(es.Spec.NodeSets)) @@ -58,7 +59,7 @@ func BuildExpectedResources( } // build stateful set and associated headless service - statefulSet, err := BuildStatefulSet(es, nodeSpec, cfg, keystoreResources, existingStatefulSets) + statefulSet, err := BuildStatefulSet(es, nodeSpec, cfg, keystoreResources, existingStatefulSets, setDefaultFsGroup) if err != nil { return nil, err } diff --git a/pkg/controller/elasticsearch/nodespec/statefulset.go b/pkg/controller/elasticsearch/nodespec/statefulset.go index f5507bb4be..1327675c1a 100644 --- a/pkg/controller/elasticsearch/nodespec/statefulset.go +++ b/pkg/controller/elasticsearch/nodespec/statefulset.go @@ -64,6 +64,7 @@ func BuildStatefulSet( cfg settings.CanonicalConfig, keystoreResources *keystore.Resources, existingStatefulSets sset.StatefulSetList, + setDefaultFsGroup bool, ) (appsv1.StatefulSet, error) { statefulSetName := esv1.StatefulSet(es.Name, nodeSet.Name) @@ -75,7 +76,7 @@ func BuildStatefulSet( nodeSet.VolumeClaimTemplates, nodeSet.PodTemplate.Spec, esvolume.DefaultVolumeClaimTemplates..., ) // build pod template - podTemplate, err := BuildPodTemplateSpec(es, nodeSet, cfg, keystoreResources) + podTemplate, err := BuildPodTemplateSpec(es, nodeSet, cfg, keystoreResources, setDefaultFsGroup) if err != nil { return appsv1.StatefulSet{}, err } From 29d37d1ddc35c3b33578178268db6ee3def4b3d2 Mon Sep 17 00:00:00 2001 From: David Kowalski Date: Tue, 28 Jul 2020 16:18:25 +0200 Subject: [PATCH 2/6] Conditionally supply fsGroup based on both the setting and ES version --- cmd/manager/main.go | 2 +- .../common/defaults/pod_template.go | 2 +- pkg/controller/common/operator/flags.go | 2 +- pkg/controller/common/operator/parameters.go | 2 +- pkg/controller/common/version/version.go | 5 ++ .../elasticsearch/nodespec/podspec.go | 6 +- .../elasticsearch/nodespec/podspec_test.go | 56 ++++++++++++++++--- 7 files changed, 62 insertions(+), 13 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index a1d9bda6ed..74b5aebfa6 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -177,7 +177,7 @@ func init() { Cmd.Flags().Bool( operator.SetDefaultFsGroupFlag, true, - "Enables setting the default filesystem group in Pods security context", + "Enables setting the default filesystem group in Pods security context on ES 8.0+", ) // enable using dashed notation in flags and underscores in env diff --git a/pkg/controller/common/defaults/pod_template.go b/pkg/controller/common/defaults/pod_template.go index afac68eef6..4c594a5498 100644 --- a/pkg/controller/common/defaults/pod_template.go +++ b/pkg/controller/common/defaults/pod_template.go @@ -7,11 +7,11 @@ package defaults import ( "sort" - corev1 "k8s.io/api/core/v1" "github.com/elastic/cloud-on-k8s/pkg/controller/common/container" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings" "github.com/elastic/cloud-on-k8s/pkg/utils/maps" "github.com/elastic/cloud-on-k8s/pkg/utils/pointer" + corev1 "k8s.io/api/core/v1" ) // PodDownwardEnvVars returns default environment variables created from the downward API. diff --git a/pkg/controller/common/operator/flags.go b/pkg/controller/common/operator/flags.go index 25e9017ed6..e025fa7016 100644 --- a/pkg/controller/common/operator/flags.go +++ b/pkg/controller/common/operator/flags.go @@ -20,7 +20,7 @@ const ( MetricsPortFlag = "metrics-port" NamespacesFlag = "namespaces" OperatorNamespaceFlag = "operator-namespace" - SetDefaultFsGroupFlag = "set-default-fsgroup" + SetDefaultFsGroupFlag = "set-default-fsgroup" WebhookCertDirFlag = "webhook-cert-dir" WebhookSecretFlag = "webhook-secret" WebhookConfigurationNameFlag = "webhook-configuration-name" diff --git a/pkg/controller/common/operator/parameters.go b/pkg/controller/common/operator/parameters.go index a7c3c062b4..4a299ac863 100644 --- a/pkg/controller/common/operator/parameters.go +++ b/pkg/controller/common/operator/parameters.go @@ -27,7 +27,7 @@ type Parameters struct { // MaxConcurrentReconciles controls the number of goroutines per controller. MaxConcurrentReconciles int // SetDefaultFsGroup determines whether the operator should set the default - // filesystem group for Pod security context. + // filesystem group for Pod security context on ES 8.0+. Ignored pre-8.0. SetDefaultFsGroup bool // Tracer is a shared APM tracer instance or nil Tracer *apm.Tracer diff --git a/pkg/controller/common/version/version.go b/pkg/controller/common/version/version.go index 23aec8d861..7c64bc3d67 100644 --- a/pkg/controller/common/version/version.go +++ b/pkg/controller/common/version/version.go @@ -23,6 +23,11 @@ var ( SupportedBeatVersions = MinMaxVersion{Min: From(7, 0, 0), Max: From(8, 99, 99)} ) +var ( + // MinDefaultFSGroupVersion is the minimal version for which the operator will inspect DefaultFSGroup flag. + MinDefaultFSGroupVersion = MustParse("8.0.0") +) + // MinMaxVersion holds the minimum and maximum supported versions. type MinMaxVersion struct { Min Version diff --git a/pkg/controller/elasticsearch/nodespec/podspec.go b/pkg/controller/elasticsearch/nodespec/podspec.go index 64c842e5f1..83e8f2c812 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec.go +++ b/pkg/controller/elasticsearch/nodespec/podspec.go @@ -56,7 +56,11 @@ func BuildPodTemplateSpec( } defaultContainerPorts := getDefaultContainerPorts(es) - if setDefaultFsGroup { + ver, err := version.Parse(es.Spec.Version) + if err != nil { + return corev1.PodTemplateSpec{}, err + } + if ver.IsSameOrAfter(version.MinDefaultFSGroupVersion) && setDefaultFsGroup { builder = builder.WithFsGroup(defaultFsGroup) } diff --git a/pkg/controller/elasticsearch/nodespec/podspec_test.go b/pkg/controller/elasticsearch/nodespec/podspec_test.go index 6699f0f7db..e3e270f574 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec_test.go +++ b/pkg/controller/elasticsearch/nodespec/podspec_test.go @@ -14,6 +14,7 @@ import ( "github.com/elastic/cloud-on-k8s/pkg/controller/common/version" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/initcontainer" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings" + "github.com/elastic/cloud-on-k8s/pkg/utils/pointer" "github.com/go-test/deep" "github.com/stretchr/testify/assert" @@ -88,16 +89,55 @@ var sampleES = esv1.Elasticsearch{ } func TestBuildPodTemplateSpecWithDefaultFsGroup(t *testing.T) { - nodeSet := sampleES.Spec.NodeSets[0] - ver, err := version.Parse(sampleES.Spec.Version) - require.NoError(t, err) - cfg, err := settings.NewMergedESConfig(sampleES.Name, *ver, sampleES.Spec.HTTP, *nodeSet.Config) - require.NoError(t, err) + for _, tt := range []struct { + name string + version version.Version + setDefaultFSGroup bool + wantFSGroup *int64 + }{ + { + name: "pre-8.0, setting off", + version: version.MustParse("7.8.0"), + setDefaultFSGroup: false, + wantFSGroup: nil, + }, + { + name: "pre-8.0, setting on", + version: version.MustParse("7.8.0"), + setDefaultFSGroup: true, + wantFSGroup: nil, + }, + { + name: "8.0+, setting off", + version: version.MustParse("8.0.0"), + setDefaultFSGroup: false, + wantFSGroup: nil, + }, + { + name: "8.0+, setting on", + version: version.MustParse("8.0.0"), + setDefaultFSGroup: true, + wantFSGroup: pointer.Int64(1000), + }, + } { + t.Run(tt.name, func(t *testing.T) { + es := *sampleES.DeepCopy() + es.Spec.Version = tt.version.String() + nodeSet := es.Spec.NodeSets[0] - actual, err := BuildPodTemplateSpec(sampleES, sampleES.Spec.NodeSets[0], cfg, nil, true) - require.NoError(t, err) + cfg, err := settings.NewMergedESConfig(es.Name, tt.version, es.Spec.HTTP, *nodeSet.Config) + require.NoError(t, err) + + actual, err := BuildPodTemplateSpec(es, es.Spec.NodeSets[0], cfg, nil, tt.setDefaultFSGroup) + require.NoError(t, err) - require.Equal(t, int64(1000), *actual.Spec.SecurityContext.FSGroup) + if tt.wantFSGroup == nil { + require.Nil(t, actual.Spec.SecurityContext) + } else { + require.Equal(t, *tt.wantFSGroup, *actual.Spec.SecurityContext.FSGroup) + } + }) + } } func TestBuildPodTemplateSpec(t *testing.T) { From 1c94a8664e50860806b61a27b2d97132129af28a Mon Sep 17 00:00:00 2001 From: David Kowalski Date: Tue, 25 Aug 2020 14:10:16 +0200 Subject: [PATCH 3/6] Change to setting entire context --- cmd/manager/main.go | 10 ++--- .../common/defaults/pod_template.go | 11 ------ pkg/controller/common/operator/flags.go | 38 +++++++++---------- pkg/controller/common/operator/parameters.go | 6 +-- pkg/controller/elasticsearch/driver/nodes.go | 2 +- .../elasticsearch/nodespec/podspec.go | 9 +++-- .../elasticsearch/nodespec/resources.go | 4 +- .../elasticsearch/nodespec/statefulset.go | 4 +- 8 files changed, 38 insertions(+), 46 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 74b5aebfa6..0a4b03989d 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -175,9 +175,9 @@ func init() { fmt.Sprintf("Name of the Kubernetes ValidatingWebhookConfiguration resource (defaults to %s). Only used when enable-webhook is true.", DefaultWebhookConfigurationName), ) Cmd.Flags().Bool( - operator.SetDefaultFsGroupFlag, + operator.SetDefaultSecurityContextFlag, true, - "Enables setting the default filesystem group in Pods security context on ES 8.0+", + "Enables setting the default security context of Elasticsearch 8.0+ Pods, ignored pre-8.0", ) // enable using dashed notation in flags and underscores in env @@ -326,9 +326,9 @@ func execute() { Validity: certValidity, RotateBefore: certRotateBefore, }, - MaxConcurrentReconciles: viper.GetInt(operator.MaxConcurrentReconcilesFlag), - SetDefaultFsGroup: viper.GetBool(operator.SetDefaultFsGroupFlag), - Tracer: tracer, + MaxConcurrentReconciles: viper.GetInt(operator.MaxConcurrentReconcilesFlag), + SetDefaultSecurityContext: viper.GetBool(operator.SetDefaultSecurityContextFlag), + Tracer: tracer, } if viper.GetBool(operator.EnableWebhookFlag) { diff --git a/pkg/controller/common/defaults/pod_template.go b/pkg/controller/common/defaults/pod_template.go index 4c594a5498..d9687b1999 100644 --- a/pkg/controller/common/defaults/pod_template.go +++ b/pkg/controller/common/defaults/pod_template.go @@ -10,7 +10,6 @@ import ( "github.com/elastic/cloud-on-k8s/pkg/controller/common/container" "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings" "github.com/elastic/cloud-on-k8s/pkg/utils/maps" - "github.com/elastic/cloud-on-k8s/pkg/utils/pointer" corev1 "k8s.io/api/core/v1" ) @@ -301,13 +300,3 @@ func (b *PodTemplateBuilder) WithAutomountServiceAccountToken() *PodTemplateBuil } return b } - -func (b *PodTemplateBuilder) WithFsGroup(defaultFsGroup int64) *PodTemplateBuilder { - if b.PodTemplate.Spec.SecurityContext == nil { - b.PodTemplate.Spec.SecurityContext = &corev1.PodSecurityContext{} - } - if b.PodTemplate.Spec.SecurityContext.FSGroup == nil { - b.PodTemplate.Spec.SecurityContext.FSGroup = pointer.Int64(defaultFsGroup) - } - return b -} diff --git a/pkg/controller/common/operator/flags.go b/pkg/controller/common/operator/flags.go index e025fa7016..becbf61145 100644 --- a/pkg/controller/common/operator/flags.go +++ b/pkg/controller/common/operator/flags.go @@ -5,23 +5,23 @@ package operator const ( - AutoPortForwardFlag = "auto-port-forward" - CACertRotateBeforeFlag = "ca-cert-rotate-before" - CACertValidityFlag = "ca-cert-validity" - CertRotateBeforeFlag = "cert-rotate-before" - CertValidityFlag = "cert-validity" - ContainerRegistryFlag = "container-registry" - DebugHTTPListenFlag = "debug-http-listen" - EnableTracingFlag = "enable-tracing" - EnableWebhookFlag = "enable-webhook" - EnforceRBACOnRefsFlag = "enforce-rbac-on-refs" - ManageWebhookCertsFlag = "manage-webhook-certs" - MaxConcurrentReconcilesFlag = "max-concurrent-reconciles" - MetricsPortFlag = "metrics-port" - NamespacesFlag = "namespaces" - OperatorNamespaceFlag = "operator-namespace" - SetDefaultFsGroupFlag = "set-default-fsgroup" - WebhookCertDirFlag = "webhook-cert-dir" - WebhookSecretFlag = "webhook-secret" - WebhookConfigurationNameFlag = "webhook-configuration-name" + AutoPortForwardFlag = "auto-port-forward" + CACertRotateBeforeFlag = "ca-cert-rotate-before" + CACertValidityFlag = "ca-cert-validity" + CertRotateBeforeFlag = "cert-rotate-before" + CertValidityFlag = "cert-validity" + ContainerRegistryFlag = "container-registry" + DebugHTTPListenFlag = "debug-http-listen" + EnableTracingFlag = "enable-tracing" + EnableWebhookFlag = "enable-webhook" + EnforceRBACOnRefsFlag = "enforce-rbac-on-refs" + ManageWebhookCertsFlag = "manage-webhook-certs" + MaxConcurrentReconcilesFlag = "max-concurrent-reconciles" + MetricsPortFlag = "metrics-port" + NamespacesFlag = "namespaces" + OperatorNamespaceFlag = "operator-namespace" + SetDefaultSecurityContextFlag = "set-default-security-context" + WebhookCertDirFlag = "webhook-cert-dir" + WebhookSecretFlag = "webhook-secret" + WebhookConfigurationNameFlag = "webhook-configuration-name" ) diff --git a/pkg/controller/common/operator/parameters.go b/pkg/controller/common/operator/parameters.go index 4a299ac863..954031e913 100644 --- a/pkg/controller/common/operator/parameters.go +++ b/pkg/controller/common/operator/parameters.go @@ -26,9 +26,9 @@ type Parameters struct { CertRotation certificates.RotationParams // MaxConcurrentReconciles controls the number of goroutines per controller. MaxConcurrentReconciles int - // SetDefaultFsGroup determines whether the operator should set the default - // filesystem group for Pod security context on ES 8.0+. Ignored pre-8.0. - SetDefaultFsGroup bool + // SetDefaultSecurityContext enables setting the default security context + // of Elasticsearch 8.0+ Pods, ignored pre-8.0 + SetDefaultSecurityContext bool // Tracer is a shared APM tracer instance or nil Tracer *apm.Tracer } diff --git a/pkg/controller/elasticsearch/driver/nodes.go b/pkg/controller/elasticsearch/driver/nodes.go index d1108a5420..7d2012e6d8 100644 --- a/pkg/controller/elasticsearch/driver/nodes.go +++ b/pkg/controller/elasticsearch/driver/nodes.go @@ -54,7 +54,7 @@ func (d *defaultDriver) reconcileNodeSpecs( return results.WithError(err) } - expectedResources, err := nodespec.BuildExpectedResources(d.ES, keystoreResources, actualStatefulSets, d.OperatorParameters.SetDefaultFsGroup) + expectedResources, err := nodespec.BuildExpectedResources(d.ES, keystoreResources, actualStatefulSets, d.OperatorParameters.SetDefaultSecurityContext) if err != nil { return results.WithError(err) } diff --git a/pkg/controller/elasticsearch/nodespec/podspec.go b/pkg/controller/elasticsearch/nodespec/podspec.go index 83e8f2c812..0bcd5ad4bd 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec.go +++ b/pkg/controller/elasticsearch/nodespec/podspec.go @@ -8,6 +8,7 @@ import ( "crypto/sha256" "fmt" + "github.com/elastic/cloud-on-k8s/pkg/utils/pointer" corev1 "k8s.io/api/core/v1" esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1" @@ -35,7 +36,7 @@ func BuildPodTemplateSpec( nodeSet esv1.NodeSet, cfg settings.CanonicalConfig, keystoreResources *keystore.Resources, - setDefaultFsGroup bool, + setDefaultSecurityContext bool, ) (corev1.PodTemplateSpec, error) { volumes, volumeMounts := buildVolumes(es.Name, nodeSet, keystoreResources) labels, err := buildLabels(es, cfg, nodeSet, keystoreResources) @@ -60,8 +61,10 @@ func BuildPodTemplateSpec( if err != nil { return corev1.PodTemplateSpec{}, err } - if ver.IsSameOrAfter(version.MinDefaultFSGroupVersion) && setDefaultFsGroup { - builder = builder.WithFsGroup(defaultFsGroup) + if ver.IsSameOrAfter(version.MinDefaultFSGroupVersion) && setDefaultSecurityContext { + builder = builder.WithPodSecurityContext(corev1.PodSecurityContext{ + FSGroup: pointer.Int64(defaultFsGroup), + }) } builder = builder. diff --git a/pkg/controller/elasticsearch/nodespec/resources.go b/pkg/controller/elasticsearch/nodespec/resources.go index 9514e40697..587136cd0d 100644 --- a/pkg/controller/elasticsearch/nodespec/resources.go +++ b/pkg/controller/elasticsearch/nodespec/resources.go @@ -38,7 +38,7 @@ func BuildExpectedResources( es esv1.Elasticsearch, keystoreResources *keystore.Resources, existingStatefulSets sset.StatefulSetList, - setDefaultFsGroup bool, + setDefaultSecurityContext bool, ) (ResourcesList, error) { nodesResources := make(ResourcesList, 0, len(es.Spec.NodeSets)) @@ -59,7 +59,7 @@ func BuildExpectedResources( } // build stateful set and associated headless service - statefulSet, err := BuildStatefulSet(es, nodeSpec, cfg, keystoreResources, existingStatefulSets, setDefaultFsGroup) + statefulSet, err := BuildStatefulSet(es, nodeSpec, cfg, keystoreResources, existingStatefulSets, setDefaultSecurityContext) if err != nil { return nil, err } diff --git a/pkg/controller/elasticsearch/nodespec/statefulset.go b/pkg/controller/elasticsearch/nodespec/statefulset.go index 1327675c1a..3217d904dd 100644 --- a/pkg/controller/elasticsearch/nodespec/statefulset.go +++ b/pkg/controller/elasticsearch/nodespec/statefulset.go @@ -64,7 +64,7 @@ func BuildStatefulSet( cfg settings.CanonicalConfig, keystoreResources *keystore.Resources, existingStatefulSets sset.StatefulSetList, - setDefaultFsGroup bool, + setDefaultSecurityContext bool, ) (appsv1.StatefulSet, error) { statefulSetName := esv1.StatefulSet(es.Name, nodeSet.Name) @@ -76,7 +76,7 @@ func BuildStatefulSet( nodeSet.VolumeClaimTemplates, nodeSet.PodTemplate.Spec, esvolume.DefaultVolumeClaimTemplates..., ) // build pod template - podTemplate, err := BuildPodTemplateSpec(es, nodeSet, cfg, keystoreResources, setDefaultFsGroup) + podTemplate, err := BuildPodTemplateSpec(es, nodeSet, cfg, keystoreResources, setDefaultSecurityContext) if err != nil { return appsv1.StatefulSet{}, err } From 7fdb806524577a9b7611a36f660115493acc63ea Mon Sep 17 00:00:00 2001 From: David Kowalski Date: Tue, 25 Aug 2020 14:45:52 +0200 Subject: [PATCH 4/6] Fix UTs, rename min version variable --- cmd/manager/main.go | 2 +- pkg/controller/common/version/version.go | 4 +- .../elasticsearch/nodespec/podspec.go | 4 +- .../elasticsearch/nodespec/podspec_test.go | 93 +++++++++++++------ 4 files changed, 69 insertions(+), 34 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 01ff120fdb..2237e7bacb 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -218,7 +218,7 @@ func Command() *cobra.Command { DefaultWebhookName, "Name of the Kubernetes ValidatingWebhookConfiguration resource. Only used when enable-webhook is true.", ) - Cmd.Flags().Bool( + cmd.Flags().Bool( operator.SetDefaultSecurityContextFlag, true, "Enables setting the default security context of Elasticsearch 8.0+ Pods, ignored pre-8.0", diff --git a/pkg/controller/common/version/version.go b/pkg/controller/common/version/version.go index a2a407a8b3..507cdb3896 100644 --- a/pkg/controller/common/version/version.go +++ b/pkg/controller/common/version/version.go @@ -24,8 +24,8 @@ var ( ) var ( - // MinDefaultFSGroupVersion is the minimal version for which the operator will inspect DefaultFSGroup flag. - MinDefaultFSGroupVersion = MustParse("8.0.0") + // MinDefaultSecurityContextVersion is the minimal version for which the operator will inspect set-default-security-context flag. + MinDefaultSecurityContextVersion = MustParse("8.0.0") ) // MinMaxVersion holds the minimum and maximum supported versions. diff --git a/pkg/controller/elasticsearch/nodespec/podspec.go b/pkg/controller/elasticsearch/nodespec/podspec.go index 0a31372a9b..328a4a94af 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec.go +++ b/pkg/controller/elasticsearch/nodespec/podspec.go @@ -8,7 +8,6 @@ import ( "crypto/sha256" "fmt" - "github.com/elastic/cloud-on-k8s/pkg/utils/pointer" corev1 "k8s.io/api/core/v1" esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1" @@ -24,6 +23,7 @@ import ( "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings" esvolume "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/volume" "github.com/elastic/cloud-on-k8s/pkg/utils/k8s" + "github.com/elastic/cloud-on-k8s/pkg/utils/pointer" ) const ( @@ -61,7 +61,7 @@ func BuildPodTemplateSpec( if err != nil { return corev1.PodTemplateSpec{}, err } - if ver.IsSameOrAfter(version.MinDefaultFSGroupVersion) && setDefaultSecurityContext { + if ver.IsSameOrAfter(version.MinDefaultSecurityContextVersion) && setDefaultSecurityContext { builder = builder.WithPodSecurityContext(corev1.PodSecurityContext{ FSGroup: pointer.Int64(defaultFsGroup), }) diff --git a/pkg/controller/elasticsearch/nodespec/podspec_test.go b/pkg/controller/elasticsearch/nodespec/podspec_test.go index e3e270f574..debdd35cba 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec_test.go +++ b/pkg/controller/elasticsearch/nodespec/podspec_test.go @@ -88,54 +88,89 @@ var sampleES = esv1.Elasticsearch{ }, } -func TestBuildPodTemplateSpecWithDefaultFsGroup(t *testing.T) { +func TestBuildPodTemplateSpecWithDefaultSecurityContext(t *testing.T) { for _, tt := range []struct { - name string - version version.Version - setDefaultFSGroup bool - wantFSGroup *int64 + name string + version version.Version + setDefaultFSGroup bool + userSecurityContext *corev1.PodSecurityContext + wantSecurityContext *corev1.PodSecurityContext }{ { - name: "pre-8.0, setting off", - version: version.MustParse("7.8.0"), - setDefaultFSGroup: false, - wantFSGroup: nil, + name: "pre-8.0, setting off, no user context", + version: version.MustParse("7.8.0"), + setDefaultFSGroup: false, + userSecurityContext: nil, + wantSecurityContext: nil, }, { - name: "pre-8.0, setting on", - version: version.MustParse("7.8.0"), - setDefaultFSGroup: true, - wantFSGroup: nil, + name: "pre-8.0, setting off, user context", + version: version.MustParse("7.8.0"), + setDefaultFSGroup: false, + userSecurityContext: &corev1.PodSecurityContext{FSGroup: pointer.Int64(123)}, + wantSecurityContext: &corev1.PodSecurityContext{FSGroup: pointer.Int64(123)}, }, { - name: "8.0+, setting off", - version: version.MustParse("8.0.0"), - setDefaultFSGroup: false, - wantFSGroup: nil, + name: "pre-8.0, setting on, no user context", + version: version.MustParse("7.8.0"), + setDefaultFSGroup: true, + userSecurityContext: nil, + wantSecurityContext: nil, }, { - name: "8.0+, setting on", - version: version.MustParse("8.0.0"), - setDefaultFSGroup: true, - wantFSGroup: pointer.Int64(1000), + name: "pre-8.0, setting on, user context", + version: version.MustParse("7.8.0"), + setDefaultFSGroup: true, + userSecurityContext: &corev1.PodSecurityContext{FSGroup: pointer.Int64(123)}, + wantSecurityContext: &corev1.PodSecurityContext{FSGroup: pointer.Int64(123)}, + }, + { + name: "8.0+, setting off, no user context", + version: version.MustParse("8.0.0"), + setDefaultFSGroup: false, + userSecurityContext: nil, + wantSecurityContext: nil, + }, + { + name: "8.0+, setting off, user context", + version: version.MustParse("8.0.0"), + setDefaultFSGroup: false, + userSecurityContext: &corev1.PodSecurityContext{FSGroup: pointer.Int64(123)}, + wantSecurityContext: &corev1.PodSecurityContext{FSGroup: pointer.Int64(123)}, + }, + { + name: "8.0+, setting on, no user context", + version: version.MustParse("8.0.0"), + setDefaultFSGroup: true, + userSecurityContext: nil, + wantSecurityContext: &corev1.PodSecurityContext{FSGroup: pointer.Int64(1000)}, + }, + { + name: "8.0+, setting on, user context", + version: version.MustParse("8.0.0"), + setDefaultFSGroup: true, + userSecurityContext: &corev1.PodSecurityContext{FSGroup: pointer.Int64(123)}, + wantSecurityContext: &corev1.PodSecurityContext{FSGroup: pointer.Int64(123)}, + }, + { + name: "8.0+, setting on, empty user context", + version: version.MustParse("8.0.0"), + setDefaultFSGroup: true, + userSecurityContext: &corev1.PodSecurityContext{}, + wantSecurityContext: &corev1.PodSecurityContext{}, }, } { t.Run(tt.name, func(t *testing.T) { es := *sampleES.DeepCopy() es.Spec.Version = tt.version.String() - nodeSet := es.Spec.NodeSets[0] + es.Spec.NodeSets[0].PodTemplate.Spec.SecurityContext = tt.userSecurityContext - cfg, err := settings.NewMergedESConfig(es.Name, tt.version, es.Spec.HTTP, *nodeSet.Config) + cfg, err := settings.NewMergedESConfig(es.Name, tt.version, es.Spec.HTTP, *es.Spec.NodeSets[0].Config) require.NoError(t, err) actual, err := BuildPodTemplateSpec(es, es.Spec.NodeSets[0], cfg, nil, tt.setDefaultFSGroup) require.NoError(t, err) - - if tt.wantFSGroup == nil { - require.Nil(t, actual.Spec.SecurityContext) - } else { - require.Equal(t, *tt.wantFSGroup, *actual.Spec.SecurityContext.FSGroup) - } + require.Equal(t, tt.wantSecurityContext, actual.Spec.SecurityContext) }) } } From bec3fdd056e70312d0bf856f2d789d49520132e8 Mon Sep 17 00:00:00 2001 From: David Kowalski Date: Mon, 31 Aug 2020 10:38:06 +0200 Subject: [PATCH 5/6] PR fixes --- cmd/manager/main.go | 2 +- pkg/controller/common/operator/parameters.go | 2 +- pkg/controller/common/version/version.go | 5 ----- pkg/controller/elasticsearch/nodespec/podspec.go | 15 +++++++++++---- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 2237e7bacb..4312bf664f 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -221,7 +221,7 @@ func Command() *cobra.Command { cmd.Flags().Bool( operator.SetDefaultSecurityContextFlag, true, - "Enables setting the default security context of Elasticsearch 8.0+ Pods, ignored pre-8.0", + "Enables setting the default security context with fsGroup=1000 for Elasticsearch 8.0+ Pods. Ignored pre-8.0.\n", ) // hide development mode flags from the usage message diff --git a/pkg/controller/common/operator/parameters.go b/pkg/controller/common/operator/parameters.go index 954031e913..18d0ff8ae7 100644 --- a/pkg/controller/common/operator/parameters.go +++ b/pkg/controller/common/operator/parameters.go @@ -27,7 +27,7 @@ type Parameters struct { // MaxConcurrentReconciles controls the number of goroutines per controller. MaxConcurrentReconciles int // SetDefaultSecurityContext enables setting the default security context - // of Elasticsearch 8.0+ Pods, ignored pre-8.0 + // with fsGroup=1000 for Elasticsearch 8.0+ Pods. Ignored pre-8.0 SetDefaultSecurityContext bool // Tracer is a shared APM tracer instance or nil Tracer *apm.Tracer diff --git a/pkg/controller/common/version/version.go b/pkg/controller/common/version/version.go index 507cdb3896..a12751b1f2 100644 --- a/pkg/controller/common/version/version.go +++ b/pkg/controller/common/version/version.go @@ -23,11 +23,6 @@ var ( SupportedBeatVersions = MinMaxVersion{Min: From(7, 0, 0), Max: From(8, 99, 99)} ) -var ( - // MinDefaultSecurityContextVersion is the minimal version for which the operator will inspect set-default-security-context flag. - MinDefaultSecurityContextVersion = MustParse("8.0.0") -) - // MinMaxVersion holds the minimum and maximum supported versions. type MinMaxVersion struct { Min Version diff --git a/pkg/controller/elasticsearch/nodespec/podspec.go b/pkg/controller/elasticsearch/nodespec/podspec.go index 328a4a94af..b42f39454f 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec.go +++ b/pkg/controller/elasticsearch/nodespec/podspec.go @@ -26,9 +26,16 @@ import ( "github.com/elastic/cloud-on-k8s/pkg/utils/pointer" ) -const ( - defaultFsGroup = 1000 -) +// Starting 8.0.0, the Elasticsearch container does not run with the root user anymore. As a result, +// we cannot chown the mounted volumes to the right user (id 1000) in an init container. +// Instead, we can rely on Kubernetes `securityContext.fsGroup` feature: by setting it to 1000 +// mounted volumes can correctly be accessed by the default container user. +// On some restricted environments (custom PSPs or Openshift), setting the Pod security context +// is forbidden: the user can either set `--set-default-security-context=false`, or override the +// podTemplate securityContext to an empty value. +var minDefaultSecurityContextVersion = version.MustParse("8.0.0") + +const defaultFsGroup = 1000 // BuildPodTemplateSpec builds a new PodTemplateSpec for an Elasticsearch node. func BuildPodTemplateSpec( @@ -61,7 +68,7 @@ func BuildPodTemplateSpec( if err != nil { return corev1.PodTemplateSpec{}, err } - if ver.IsSameOrAfter(version.MinDefaultSecurityContextVersion) && setDefaultSecurityContext { + if ver.IsSameOrAfter(minDefaultSecurityContextVersion) && setDefaultSecurityContext { builder = builder.WithPodSecurityContext(corev1.PodSecurityContext{ FSGroup: pointer.Int64(defaultFsGroup), }) From 039c005993f1929ec46bb29f72302fd04559eefd Mon Sep 17 00:00:00 2001 From: David Kowalski Date: Mon, 31 Aug 2020 15:33:19 +0200 Subject: [PATCH 6/6] PR fixes --- cmd/manager/main.go | 2 +- hack/manifest-gen/assets/charts/eck/templates/configmap.yaml | 1 + hack/manifest-gen/assets/charts/eck/values.yaml | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 4312bf664f..949eaa4188 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -221,7 +221,7 @@ func Command() *cobra.Command { cmd.Flags().Bool( operator.SetDefaultSecurityContextFlag, true, - "Enables setting the default security context with fsGroup=1000 for Elasticsearch 8.0+ Pods. Ignored pre-8.0.\n", + "Enables setting the default security context with fsGroup=1000 for Elasticsearch 8.0+ Pods. Ignored pre-8.0.", ) // hide development mode flags from the usage message diff --git a/hack/manifest-gen/assets/charts/eck/templates/configmap.yaml b/hack/manifest-gen/assets/charts/eck/templates/configmap.yaml index 35de14c9bc..c621cc477b 100644 --- a/hack/manifest-gen/assets/charts/eck/templates/configmap.yaml +++ b/hack/manifest-gen/assets/charts/eck/templates/configmap.yaml @@ -14,6 +14,7 @@ data: ca-cert-rotate-before: {{ .Values.config.ca.rotateBefore }} cert-validity: {{ .Values.config.certificates.validity }} cert-rotate-before: {{ .Values.config.certificates.rotateBefore }} + set-default-security-context: {{ .Values.config.setDefaultSecurityContext }} {{- if .Values.config.tracing.enabled }} enable-tracing: true {{- end }} diff --git a/hack/manifest-gen/assets/charts/eck/values.yaml b/hack/manifest-gen/assets/charts/eck/values.yaml index 392a2f0117..dd5bcd6d5d 100644 --- a/hack/manifest-gen/assets/charts/eck/values.yaml +++ b/hack/manifest-gen/assets/charts/eck/values.yaml @@ -58,3 +58,5 @@ config: beat: manageAutoDiscoverRBAC: true + + setDefaultSecurityContext: true