From 00d660f0522ed42d78e543d7ee192aac0d29f5b4 Mon Sep 17 00:00:00 2001 From: Olivier Michallat Date: Fri, 15 Mar 2024 15:47:03 -0700 Subject: [PATCH] Fix name override issues - reject cluster creation if a DC has an invalid name - always use sanitized DC name override when naming secondary resources - use cluster name override for metrics agent configMap Fixes #1138 Fixes #1141 --- .../k8ssandra/v1alpha1/k8ssandracluster_webhook.go | 11 ++++++++++- .../v1alpha1/k8ssandracluster_webhook_test.go | 14 ++++++++++++++ .../k8ssandra/cassandra_telemetry_reconciler.go | 2 +- controllers/reaper/vector_test.go | 2 +- controllers/stargate/vector_test.go | 2 +- pkg/reaper/vector.go | 2 +- pkg/stargate/config.go | 2 +- pkg/stargate/deployments.go | 4 ++-- pkg/stargate/vector.go | 2 +- .../cassandra_agent/cassandra_agent_config.go | 8 ++++++-- 10 files changed, 38 insertions(+), 11 deletions(-) diff --git a/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go b/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go index 97e4b1b30..a0b0c2321 100644 --- a/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go +++ b/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook.go @@ -18,6 +18,8 @@ package v1alpha1 import ( "fmt" + "k8s.io/apimachinery/pkg/util/validation" + "strings" "github.com/Masterminds/semver/v3" "github.com/k8ssandra/k8ssandra-operator/pkg/clientcache" @@ -69,8 +71,15 @@ func (r *K8ssandraCluster) ValidateCreate() error { func (r *K8ssandraCluster) validateK8ssandraCluster() error { hasClusterStorageConfig := r.Spec.Cassandra.DatacenterOptions.StorageConfig != nil - // Verify given k8s-contexts are correct for _, dc := range r.Spec.Cassandra.Datacenters { + dns1035Errs := validation.IsDNS1035Label(dc.Meta.Name) + if len(dns1035Errs) > 0 { + return fmt.Errorf( + "invalid DC name (you might want to use datacenterName to override the name used in Cassandra): %s", + strings.Join(dns1035Errs, ", ")) + } + + // Verify given k8s-context is correct _, err := clientCache.GetRemoteClient(dc.K8sContext) if err != nil { // No client found for this context name, reject diff --git a/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go b/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go index 1f86cce89..aed36c9ed 100644 --- a/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go +++ b/apis/k8ssandra/v1alpha1/k8ssandracluster_webhook_test.go @@ -147,6 +147,7 @@ func TestWebhook(t *testing.T) { t.Run("NumTokensValidationInUpdate", testNumTokensInUpdate) t.Run("StsNameTooLong", testStsNameTooLong) t.Run("MedusaPrefixMissing", testMedusaPrefixMissing) + t.Run("InvalidDcName", testInvalidDcName) } func testContextValidation(t *testing.T) { @@ -219,6 +220,7 @@ func testStorageConfigValidation(t *testing.T) { required.NoError(err) cluster.Spec.Cassandra.Datacenters = append(cluster.Spec.Cassandra.Datacenters, CassandraDatacenterTemplate{ + Meta: EmbeddedObjectMeta{Name: "dc2"}, K8sContext: "envtest", Size: 1, }) @@ -376,6 +378,7 @@ func createClusterObjWithCassandraConfig(name, namespace string) *K8ssandraClust Datacenters: []CassandraDatacenterTemplate{ { + Meta: EmbeddedObjectMeta{Name: "dc1"}, K8sContext: "envtest", Size: 1, }, @@ -451,3 +454,14 @@ func testMedusaPrefixMissing(t *testing.T) { err = k8sClient.Create(ctx, clusterWithPrefix) required.NoError(err) } + +func testInvalidDcName(t *testing.T) { + required := require.New(t) + createNamespace(required, "ns") + + clusterWithBadDcName := createMinimalClusterObj("bad-dc-name", "ns") + clusterWithBadDcName.Spec.Cassandra.Datacenters[0].Meta.Name = "DC1" + err := k8sClient.Create(ctx, clusterWithBadDcName) + required.Error(err) + required.Contains(err.Error(), "invalid DC name") +} diff --git a/controllers/k8ssandra/cassandra_telemetry_reconciler.go b/controllers/k8ssandra/cassandra_telemetry_reconciler.go index 7067ac5d6..1de74796f 100644 --- a/controllers/k8ssandra/cassandra_telemetry_reconciler.go +++ b/controllers/k8ssandra/cassandra_telemetry_reconciler.go @@ -37,7 +37,7 @@ func (r *K8ssandraClusterReconciler) reconcileCassandraDCTelemetry( cfg := telemetry.PrometheusResourcer{ MonitoringTargetNS: actualDc.Namespace, MonitoringTargetName: actualDc.Name, - ServiceMonitorName: kc.SanitizedName() + "-" + actualDc.DatacenterName() + "-" + "cass-servicemonitor", + ServiceMonitorName: cassdcapi.CleanupForKubernetes(kc.CassClusterName() + "-" + actualDc.DatacenterName() + "-" + "cass-servicemonitor"), Logger: logger, CommonLabels: mustLabels(kc.Name, kc.Namespace, actualDc.DatacenterName(), commonLabels), } diff --git a/controllers/reaper/vector_test.go b/controllers/reaper/vector_test.go index 440084c07..cb61f7253 100644 --- a/controllers/reaper/vector_test.go +++ b/controllers/reaper/vector_test.go @@ -26,6 +26,6 @@ func TestBuildVectorAgentConfigMap(t *testing.T) { vectorToml := "Test" vectorConfigMap := reaperpkg.CreateVectorConfigMap("k8ssandra-operator", vectorToml, test.NewCassandraDatacenter("testDc", "k8ssandra-operator")) assert.Equal(t, vectorToml, vectorConfigMap.Data["vector.toml"]) - assert.Equal(t, "test-cluster-testDc-reaper-vector", vectorConfigMap.Name) + assert.Equal(t, "test-cluster-testdc-reaper-vector", vectorConfigMap.Name) assert.Equal(t, "k8ssandra-operator", vectorConfigMap.Namespace) } diff --git a/controllers/stargate/vector_test.go b/controllers/stargate/vector_test.go index e18fbe183..1070b720d 100644 --- a/controllers/stargate/vector_test.go +++ b/controllers/stargate/vector_test.go @@ -26,6 +26,6 @@ func TestBuildVectorAgentConfigMap(t *testing.T) { vectorToml := "Test" vectorConfigMap := stargatepkg.CreateVectorConfigMap("k8ssandra-operator", vectorToml, test.NewCassandraDatacenter("testDc", "k8ssandra-operator")) assert.Equal(t, vectorToml, vectorConfigMap.Data["vector.toml"]) - assert.Equal(t, "test-cluster-testDc-stargate-vector", vectorConfigMap.Name) + assert.Equal(t, "test-cluster-testdc-stargate-vector", vectorConfigMap.Name) assert.Equal(t, "k8ssandra-operator", vectorConfigMap.Namespace) } diff --git a/pkg/reaper/vector.go b/pkg/reaper/vector.go index f55d98afa..dd9da7e13 100644 --- a/pkg/reaper/vector.go +++ b/pkg/reaper/vector.go @@ -27,7 +27,7 @@ const ( // VectorAgentConfigMapName generates a ConfigMap name based on // the K8s sanitized cluster name and datacenter name. func VectorAgentConfigMapName(clusterName, dcName string) string { - return fmt.Sprintf("%s-%s-%s", cassdcapi.CleanupForKubernetes(clusterName), dcName, vectorConfigMap) + return fmt.Sprintf("%s-%s-%s", cassdcapi.CleanupForKubernetes(clusterName), cassdcapi.CleanupForKubernetes(dcName), vectorConfigMap) } func CreateVectorConfigMap(namespace, vectorToml string, dc cassdcapi.CassandraDatacenter) *corev1.ConfigMap { diff --git a/pkg/stargate/config.go b/pkg/stargate/config.go index 226f32fde..60fed798f 100644 --- a/pkg/stargate/config.go +++ b/pkg/stargate/config.go @@ -42,7 +42,7 @@ func FilterConfig(config map[string]interface{}, retainedSettings []string) map[ func CreateStargateConfigMap(namespace, cassandraYaml, stargateCqlYaml string, dc cassdcapi.CassandraDatacenter) *corev1.ConfigMap { return &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: GeneratedConfigMapName(dc.Spec.ClusterName, dc.DatacenterName()), + Name: GeneratedConfigMapName(dc.Spec.ClusterName, dc.Name), Namespace: namespace, Labels: utils.MergeMap( map[string]string{ diff --git a/pkg/stargate/deployments.go b/pkg/stargate/deployments.go index cf7464e40..06a65b81d 100644 --- a/pkg/stargate/deployments.go +++ b/pkg/stargate/deployments.go @@ -351,7 +351,7 @@ func computeVolumes(template *api.StargateTemplate, dc *cassdcapi.CassandraDatac VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ - Name: GeneratedConfigMapName(dc.Spec.ClusterName, dc.DatacenterName()), + Name: GeneratedConfigMapName(dc.Spec.ClusterName, dc.Name), }, }, }, @@ -513,5 +513,5 @@ func createPodMeta(stargate *api.Stargate, deploymentName string) meta.Tags { } func GeneratedConfigMapName(clusterName, dcName string) string { - return fmt.Sprintf("%s-%s-%s", cassdcapi.CleanupForKubernetes(clusterName), dcName, cassandraConfigMap) + return fmt.Sprintf("%s-%s-%s", cassdcapi.CleanupForKubernetes(clusterName), cassdcapi.CleanupForKubernetes(dcName), cassandraConfigMap) } diff --git a/pkg/stargate/vector.go b/pkg/stargate/vector.go index f65e15332..d06a7f7a5 100644 --- a/pkg/stargate/vector.go +++ b/pkg/stargate/vector.go @@ -27,7 +27,7 @@ const ( // VectorAgentConfigMapName generates a ConfigMap name based on // the K8s sanitized cluster name and datacenter name. func VectorAgentConfigMapName(clusterName, dcName string) string { - return fmt.Sprintf("%s-%s-%s", cassdcapi.CleanupForKubernetes(clusterName), dcName, vectorConfigMap) + return fmt.Sprintf("%s-%s-%s", cassdcapi.CleanupForKubernetes(clusterName), cassdcapi.CleanupForKubernetes(dcName), vectorConfigMap) } func CreateVectorConfigMap(namespace, vectorToml string, dc cassdcapi.CassandraDatacenter) *corev1.ConfigMap { diff --git a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go index 963b88190..c69062d80 100644 --- a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go +++ b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go @@ -135,13 +135,17 @@ func (c Configurator) GetTelemetryAgentConfigMap() (*corev1.ConfigMap, error) { cm := corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Namespace: c.DcNamespace, - Name: c.Kluster.Name + "-" + c.DcName + "-metrics-agent-config", + Name: c.configMapName(), }, Data: map[string]string{filepath.Base(agentConfigLocation): string(yamlData)}, } return &cm, nil } +func (c Configurator) configMapName() string { + return cassdcapi.CleanupForKubernetes(c.Kluster.CassClusterName() + "-" + c.DcName + "-metrics-agent-config") +} + func (c Configurator) ReconcileTelemetryAgentConfig(dc *cassdcapi.CassandraDatacenter) result.ReconcileResult { //Reconcile the agent's ConfigMap desiredCm, err := c.GetTelemetryAgentConfigMap() @@ -182,7 +186,7 @@ func (c Configurator) AddVolumeSource(dc *cassdcapi.CassandraDatacenter) error { }, }, LocalObjectReference: corev1.LocalObjectReference{ - Name: c.Kluster.Name + "-" + c.DcName + "-metrics-agent-config", + Name: c.configMapName(), }, }, },