From 42aec3eff47df329b9b212175c530456b2845b7e Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Thu, 23 Mar 2023 15:54:10 -0700 Subject: [PATCH] Don't set jmxremote.authenticate system property by default --- controllers/k8ssandra/auth_test.go | 16 ---------------- pkg/cassandra/auth.go | 15 --------------- pkg/reaper/datacenter.go | 26 ++++++++++++++++++++------ pkg/reaper/datacenter_test.go | 10 ++++++++++ 4 files changed, 30 insertions(+), 37 deletions(-) diff --git a/controllers/k8ssandra/auth_test.go b/controllers/k8ssandra/auth_test.go index 55b02e313..5e7a23cd6 100644 --- a/controllers/k8ssandra/auth_test.go +++ b/controllers/k8ssandra/auth_test.go @@ -197,16 +197,6 @@ func createSingleDcClusterAuth(t *testing.T, ctx context.Context, f *framework.F withDatacenter := f.NewWithDatacenter(ctx, dcKey) - t.Log("check that authentication is enabled in DC") - require.Eventually(t, withDatacenter(func(dc *cassdcapi.CassandraDatacenter) bool { - // there should be a JMX init container with 4 env vars - if dc.Spec.PodTemplateSpec != nil { - // the config should have JMX auth enabled - return assert.Contains(t, string(dc.Spec.Config), "-Dcom.sun.management.jmxremote.authenticate=true") - } - return false - }), timeout, interval) - t.Log("check that remote JMX is enabled") require.Eventually(t, withDatacenter(func(dc *cassdcapi.CassandraDatacenter) bool { if dc.Spec.PodTemplateSpec != nil { @@ -319,12 +309,6 @@ func createSingleDcClusterAuthExternalSecrets(t *testing.T, ctx context.Context, withDatacenter := f.NewWithDatacenter(ctx, dcKey) - t.Log("check that authentication is enabled in DC") - require.Eventually(t, withDatacenter(func(dc *cassdcapi.CassandraDatacenter) bool { - // the config should have JMX auth enabled - return assert.Contains(t, string(dc.Spec.Config), "-Dcom.sun.management.jmxremote.authenticate=true") - }), timeout, interval) - t.Log("check that remote JMX is enabled") require.Eventually(t, withDatacenter(func(dc *cassdcapi.CassandraDatacenter) bool { if dc.Spec.PodTemplateSpec != nil { diff --git a/pkg/cassandra/auth.go b/pkg/cassandra/auth.go index a30b9724b..8a38fd08c 100644 --- a/pkg/cassandra/auth.go +++ b/pkg/cassandra/auth.go @@ -1,8 +1,6 @@ package cassandra import ( - "fmt" - cassdcapi "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" api "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" corev1 "k8s.io/api/core/v1" @@ -13,19 +11,6 @@ func ApplyAuth(dcConfig *DatacenterConfig, authEnabled bool, useExternalSecrets dcConfig.CassandraConfig = ApplyAuthSettings(dcConfig.CassandraConfig, authEnabled, dcConfig.ServerType) - // By default, the Cassandra process will be started with LOCAL_JMX=yes, see cassandra-env.sh. This means that the - // Cassandra process will only be accessible with JMX from localhost. This is the safest and preferred setup: you - // still can use JMX by SSH'ing into the Cassandra pod, for example to run nodetool. But some components need remote - // JMX access (Reaper, metrics, etc.). Such components and their controllers are responsible for setting - // LOCAL_JMX=no whenever appropriate, to enable remote JMX access. - // However, authentication will get in the way, even if it's an orthogonal concern. Indeed, with LOCAL_JMX=yes - // cassandra-env.sh will infer that no JMX authentication should be used - // (com.sun.management.jmxremote.authenticate=false), whereas with LOCAL_JMX=no it will infer that authentication is - // required (com.sun.management.jmxremote.authenticate=true). We need to change that here and enable/disable - // authentication based on what the user specified, not what the script infers. - jmxAuthenticateOpt := fmt.Sprintf("-Dcom.sun.management.jmxremote.authenticate=%v", authEnabled) - addOptionIfMissing(dcConfig, jmxAuthenticateOpt) - // Use Cassandra internals for JMX authentication and authorization. This allows JMX clients to connect with the // superuser secret. if authEnabled && !useExternalSecrets { diff --git a/pkg/reaper/datacenter.go b/pkg/reaper/datacenter.go index c99bd4e1c..113fff098 100644 --- a/pkg/reaper/datacenter.go +++ b/pkg/reaper/datacenter.go @@ -3,22 +3,36 @@ package reaper import ( reaperapi "github.com/k8ssandra/k8ssandra-operator/apis/reaper/v1alpha1" "github.com/k8ssandra/k8ssandra-operator/pkg/cassandra" + "github.com/k8ssandra/k8ssandra-operator/pkg/utils" corev1 "k8s.io/api/core/v1" ) +const jmxAuthDisabledOption = "-Dcom.sun.management.jmxremote.authenticate=false" + func AddReaperSettingsToDcConfig(reaperTemplate *reaperapi.ReaperClusterTemplate, dcConfig *cassandra.DatacenterConfig, authEnabled bool) { - enableRemoteJmxAccess(dcConfig) + enableRemoteJmxAccess(dcConfig, authEnabled) if authEnabled && !dcConfig.ExternalSecrets { cassandra.AddCqlUser(reaperTemplate.CassandraUserSecretRef, dcConfig, DefaultUserSecretName(dcConfig.Cluster)) } } -// By default, the Cassandra process will be started with LOCAL_JMX=yes, see cassandra-env.sh. This means that the -// Cassandra process will only be accessible with JMX from localhost. However, Reaper needs remote JMX access, so we -// need to change that to LOCAL_JMX=no here. Note that this change has implications on authentication that were handled -// already in pkg/cassandra/auth.go. -func enableRemoteJmxAccess(dcConfig *cassandra.DatacenterConfig) { +func enableRemoteJmxAccess(dcConfig *cassandra.DatacenterConfig, authEnabled bool) { cassandra.UpdateCassandraContainer(&dcConfig.PodTemplateSpec, func(c *corev1.Container) { + + // By default, the Cassandra process will be started with LOCAL_JMX=yes, see cassandra-env.sh. This means that + // the Cassandra process will only be accessible with JMX from localhost. Reaper needs remote JMX access, so we + // need to change that to LOCAL_JMX=no here. c.Env = append(c.Env, corev1.EnvVar{Name: "LOCAL_JMX", Value: "no"}) + + // However, setting LOCAL_JMX=no also enables JMX authentication. If auth is disabled on the K8ssandraCluster, + // we don't want that so rectify it with a system property: + if !authEnabled { + if !utils.SliceContains(dcConfig.CassandraConfig.JvmOptions.AdditionalOptions, jmxAuthDisabledOption) { + dcConfig.CassandraConfig.JvmOptions.AdditionalOptions = append( + []string{jmxAuthDisabledOption}, + dcConfig.CassandraConfig.JvmOptions.AdditionalOptions..., + ) + } + } }) } diff --git a/pkg/reaper/datacenter_test.go b/pkg/reaper/datacenter_test.go index fed133da4..f60394092 100644 --- a/pkg/reaper/datacenter_test.go +++ b/pkg/reaper/datacenter_test.go @@ -69,6 +69,11 @@ func TestAddReaperSettingsToDcConfig(t *testing.T) { }}, }, }, + CassandraConfig: api.CassandraConfig{ + JvmOptions: api.JvmOptions{ + AdditionalOptions: []string{"-Dcom.sun.management.jmxremote.authenticate=false"}, + }, + }, }, client.ObjectKey{Namespace: "ns1", Name: "k8c"}, false, @@ -175,6 +180,11 @@ func TestAddReaperSettingsToDcConfig(t *testing.T) { }, }, }, + CassandraConfig: api.CassandraConfig{ + JvmOptions: api.JvmOptions{ + AdditionalOptions: []string{"-Dcom.sun.management.jmxremote.authenticate=false"}, + }, + }, }, client.ObjectKey{Namespace: "ns1", Name: "k8c"}, false,