From 34648fd39f11ae76b42a9aaea6d283a3861ddc40 Mon Sep 17 00:00:00 2001 From: Jeff Banks Date: Tue, 14 Jun 2022 08:50:03 -0500 Subject: [PATCH] Fix: label values not validated (#355) (#339) Includes: * Fix: label values not validated (#355) * Support for CreatedByLabelValue; clean labels in api * Smoke test updates * Co-authored-by: Alexander Dejanovski --- CHANGELOG.md | 1 + .../v1beta1/cassandradatacenter_types.go | 30 +++--- .../v1beta1/cassandradatacenter_webhook.go | 6 +- ...dra.datastax.com_cassandradatacenters.yaml | 98 +++++++++---------- .../control.k8ssandra.io_cassandratasks.yaml | 3 +- config/rbac/role.yaml | 2 - config/webhook/manifests.yaml | 1 - pkg/oplabels/labels.go | 7 +- pkg/oplabels/labels_test.go | 64 ++++++++++++ pkg/reconciliation/construct_service_test.go | 9 +- .../construct_statefulset_test.go | 5 +- pkg/reconciliation/reconcile_fql.go | 1 - pkg/reconciliation/reconcile_racks_test.go | 34 +++++++ pkg/reconciliation/secrets_test.go | 1 + tests/testdata/smoke-test-oss.yaml | 2 +- 15 files changed, 188 insertions(+), 76 deletions(-) create mode 100644 pkg/oplabels/labels_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ac63ba19..aea33758 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Changelog for Cass Operator, new PRs should update the `main / unreleased` secti ``` ## unreleased +* [BUGFIX] [#355](https://github.com/k8ssandra/cass-operator/issues/335) Cleanse label values derived from cluster name, which can contain illegal chars. Include app.kubernetes.io/created-by label. ## v1.11.0 diff --git a/apis/cassandra/v1beta1/cassandradatacenter_types.go b/apis/cassandra/v1beta1/cassandradatacenter_types.go index e7edebf9..dc4337df 100644 --- a/apis/cassandra/v1beta1/cassandradatacenter_types.go +++ b/apis/cassandra/v1beta1/cassandradatacenter_types.go @@ -32,7 +32,6 @@ const ( // RackLabel is the operator's label for the rack name RackLabel = "cassandra.datastax.com/rack" - // RackLabel is the operator's label for the rack name CassOperatorProgressLabel = "cassandra.datastax.com/operator-progress" // PromMetricsLabel is a service label that can be selected for prometheus metrics scraping @@ -53,19 +52,16 @@ const ( // CassandraDatacenter is deleted. DecommissionOnDeleteAnnotation = "cassandra.datastax.com/decommission-on-delete" - // CassNodeState CassNodeState = "cassandra.datastax.com/node-state" - // Progress states for status ProgressUpdating ProgressState = "Updating" ProgressReady ProgressState = "Ready" - // Default port numbers DefaultNativePort = 9042 DefaultInternodePort = 7000 ) -// This type exists so there's no chance of pushing random strings to our progress status +// ProgressState - this type exists so there's no chance of pushing random strings to our progress status type ProgressState string type CassandraUser struct { @@ -238,7 +234,7 @@ type NodePortConfig struct { InternodeSSL int `json:"internodeSSL,omitempty"` } -// Is the NodePort service enabled? +// IsNodePortEnabled is the NodePort service enabled? func (dc *CassandraDatacenter) IsNodePortEnabled() bool { return dc.Spec.Networking != nil && dc.Spec.Networking.NodePort != nil } @@ -254,7 +250,7 @@ type DseWorkloads struct { SearchEnabled bool `json:"searchEnabled,omitempty"` } -// StorageConfig defines additional storage configurations +// AdditionalVolumes StorageConfig defines additional storage configurations type AdditionalVolumes struct { // Mount path into cassandra container MountPath string `json:"mountPath"` @@ -533,20 +529,31 @@ func (dc *CassandraDatacenter) SetCondition(condition DatacenterCondition) { // GetDatacenterLabels ... func (dc *CassandraDatacenter) GetDatacenterLabels() map[string]string { labels := dc.GetClusterLabels() - labels[DatacenterLabel] = dc.Name + labels[DatacenterLabel] = CleanLabelValue(dc.Name) return labels } // GetClusterLabels returns a new map with the cluster label key and cluster name value func (dc *CassandraDatacenter) GetClusterLabels() map[string]string { return map[string]string{ - ClusterLabel: dc.Spec.ClusterName, + ClusterLabel: CleanLabelValue(dc.Spec.ClusterName), } } // apimachinery validation does not expose this, copied here const dns1035LabelFmt string = "[a-z]([-a-z0-9]*[a-z0-9])?" +var whitelistRegex = regexp.MustCompile(`(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?`) + +// CleanLabelValue a valid label must be an empty string or consist of alphanumeric characters, +// '-', '_' or '.', and must start and end with an alphanumeric. +// Note: we apply a prefix of "cassandra-" to the cluster name value used as label name. +// As such, empty string isn't a valid case. +func CleanLabelValue(value string) string { + regexpResult := whitelistRegex.FindAllString(strings.Replace(value, " ", "", -1), -1) + return strings.Join(regexpResult, "") +} + func CleanupForKubernetes(input string) string { if len(validation.IsDNS1035Label(input)) > 0 { r := regexp.MustCompile(dns1035LabelFmt) @@ -653,8 +660,7 @@ func (dc *CassandraDatacenter) GetConfigAsJSON(config []byte) (string, error) { } // Combine the model values with the user-specified values - - modelParsed, err := gabs.ParseJSON([]byte(modelBytes)) + modelParsed, err := gabs.ParseJSON(modelBytes) if err != nil { return "", errors.Wrap(err, "Model information for CassandraDatacenter resource was not properly configured") } @@ -673,6 +679,7 @@ func (dc *CassandraDatacenter) GetConfigAsJSON(config []byte) (string, error) { return modelParsed.String(), nil } +// GetNodePortNativePort // Gets the defined CQL port for NodePort. // 0 will be returned if NodePort is not configured. // The SSL port will be returned if it is defined, @@ -691,6 +698,7 @@ func (dc *CassandraDatacenter) GetNodePortNativePort() int { } } +// GetNodePortInternodePort // Gets the defined internode/broadcast port for NodePort. // 0 will be returned if NodePort is not configured. // The SSL port will be returned if it is defined, diff --git a/apis/cassandra/v1beta1/cassandradatacenter_webhook.go b/apis/cassandra/v1beta1/cassandradatacenter_webhook.go index 5db3e114..8248ae86 100644 --- a/apis/cassandra/v1beta1/cassandradatacenter_webhook.go +++ b/apis/cassandra/v1beta1/cassandradatacenter_webhook.go @@ -33,9 +33,9 @@ import ( var log = logf.Log.WithName("api") -func (r *CassandraDatacenter) SetupWebhookWithManager(mgr ctrl.Manager) error { +func (dc *CassandraDatacenter) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). - For(r). + For(dc). Complete() } @@ -45,7 +45,7 @@ func (r *CassandraDatacenter) SetupWebhookWithManager(mgr ctrl.Manager) error { var _ webhook.Defaulter = &CassandraDatacenter{} // Default implements webhook.Defaulter so a webhook will be registered for the type -func (r *CassandraDatacenter) Default() { +func (dc *CassandraDatacenter) Default() { // No mutations at this point } diff --git a/config/crd/bases/cassandra.datastax.com_cassandradatacenters.yaml b/config/crd/bases/cassandra.datastax.com_cassandradatacenters.yaml index 4884de07..d233e12f 100644 --- a/config/crd/bases/cassandra.datastax.com_cassandradatacenters.yaml +++ b/config/crd/bases/cassandra.datastax.com_cassandradatacenters.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.7.0 + controller-gen.kubebuilder.io/version: v0.8.0 creationTimestamp: null name: cassandradatacenters.cassandra.datastax.com spec: @@ -188,12 +187,12 @@ spec: description: "ConfigSecret is the name of a secret that contains configuration for Cassandra. The secret is expected to have a property named config whose value should be a JSON formatted string that should look like - this: \n config: |- { \"cassandra-yaml\": { \"read_request_timeout_in_ms\": - 10000 }, \"jmv-options\": { \"max_heap_size\": - 1024M } } \n ConfigSecret is mutually exclusive with - Config. ConfigSecret takes precedence and will be used exclusively - if both properties are set. The operator sets a watch such that - an update to the secret will trigger an update of the StatefulSets." + this: \n config: |- { \"cassandra-yaml\": { \"read_request_timeout_in_ms\": + 10000 }, \"jmv-options\": { \"max_heap_size\": 1024M } } \n ConfigSecret + is mutually exclusive with Config. ConfigSecret takes precedence + and will be used exclusively if both properties are set. The operator + sets a watch such that an update to the secret will trigger an update + of the StatefulSets." type: string disableSystemLoggerSidecar: description: Configuration for disabling the simple log tailing sidecar @@ -5688,13 +5687,13 @@ spec: with a pod if it doesn''t satisfy the spread constraint. - DoNotSchedule (default) tells the scheduler not to schedule it. - ScheduleAnyway tells the scheduler - to schedule the pod in any location, but giving - higher precedence to topologies that would help reduce - the skew. A constraint is considered "Unsatisfiable" - for an incoming pod if and only if every possible - node assignment for that pod would violate "MaxSkew" - on some topology. For example, in a 3-zone cluster, - MaxSkew is set to 1, and pods with the same labelSelector + to schedule the pod in any location, but giving higher + precedence to topologies that would help reduce the + skew. A constraint is considered "Unsatisfiable" for + an incoming pod if and only if every possible node + assignment for that pod would violate "MaxSkew" on + some topology. For example, in a 3-zone cluster, MaxSkew + is set to 1, and pods with the same labelSelector spread as 3/1/1: | zone1 | zone2 | zone3 | | P P P | P | P | If WhenUnsatisfiable is set to DoNotSchedule, incoming pod can only be scheduled to zone2(zone3) @@ -6131,13 +6130,12 @@ spec: when the pod is removed. \n Use this if: a) the volume is only needed while the pod runs, b) features of normal volumes like restoring from snapshot or capacity - \ tracking are needed, c) the storage driver is - specified through a storage class, and d) the storage - driver supports dynamic volume provisioning through - \ a PersistentVolumeClaim (see EphemeralVolumeSource - for more information on the connection between - this volume type and PersistentVolumeClaim). \n - Use PersistentVolumeClaim or one of the vendor-specific + tracking are needed, c) the storage driver is specified + through a storage class, and d) the storage driver + supports dynamic volume provisioning through a PersistentVolumeClaim + (see EphemeralVolumeSource for more information on + the connection between this volume type and PersistentVolumeClaim). + \n Use PersistentVolumeClaim or one of the vendor-specific APIs for volumes that persist for longer than the lifecycle of an individual pod. \n Use CSI for light-weight local ephemeral volumes if the CSI driver is meant @@ -6262,14 +6260,15 @@ spec: are two important differences between DataSource and DataSourceRef: * While DataSource only allows two specific types - of objects, DataSourceRef allows any - non-core object, as well as PersistentVolumeClaim + of objects, DataSourceRef allows any non-core + object, as well as PersistentVolumeClaim objects. * While DataSource ignores disallowed - values (dropping them), DataSourceRef preserves - all values, and generates an error if - a disallowed value is specified. (Alpha) - Using this field requires the AnyVolumeDataSource - feature gate to be enabled.' + values (dropping them), DataSourceRef + preserves all values, and generates an + error if a disallowed value is specified. + (Alpha) Using this field requires the + AnyVolumeDataSource feature gate to be + enabled.' properties: apiGroup: description: APIGroup is the group for @@ -7440,7 +7439,8 @@ spec: properties: additionalVolumes: items: - description: StorageConfig defines additional storage configurations + description: AdditionalVolumes StorageConfig defines additional + storage configurations properties: mountPath: description: Mount path into cassandra container @@ -7503,11 +7503,11 @@ spec: if one of them is empty and the other is non-empty. There are two important differences between DataSource and DataSourceRef: * While DataSource only allows - two specific types of objects, DataSourceRef allows + two specific types of objects, DataSourceRef allows any non-core object, as well as PersistentVolumeClaim objects. * While DataSource ignores disallowed values - (dropping them), DataSourceRef preserves all values, - and generates an error if a disallowed value is specified. + (dropping them), DataSourceRef preserves all values, + and generates an error if a disallowed value is specified. (Alpha) Using this field requires the AnyVolumeDataSource feature gate to be enabled.' properties: @@ -7680,10 +7680,10 @@ spec: if one of them is empty and the other is non-empty. There are two important differences between DataSource and DataSourceRef: * While DataSource only allows two specific types of objects, - DataSourceRef allows any non-core object, as well as PersistentVolumeClaim + DataSourceRef allows any non-core object, as well as PersistentVolumeClaim objects. * While DataSource ignores disallowed values (dropping - them), DataSourceRef preserves all values, and generates - an error if a disallowed value is specified. (Alpha) Using + them), DataSourceRef preserves all values, and generates + an error if a disallowed value is specified. (Alpha) Using this field requires the AnyVolumeDataSource feature gate to be enabled.' properties: @@ -7958,23 +7958,23 @@ spec: description: 'ObjectReference contains enough information to let you inspect or modify the referred object. --- New uses of this type are discouraged because of difficulty describing its usage - when embedded in APIs. 1. Ignored fields. It includes many fields + when embedded in APIs. 1. Ignored fields. It includes many fields which are not generally honored. For instance, ResourceVersion - and FieldPath are both very rarely valid in actual usage. 2. - Invalid usage help. It is impossible to add specific help for - individual usage. In most embedded usages, there are particular restrictions + and FieldPath are both very rarely valid in actual usage. 2. Invalid + usage help. It is impossible to add specific help for individual + usage. In most embedded usages, there are particular restrictions like, "must refer only to types A and B" or "UID not honored" - or "name must be restricted". Those cannot be well described - when embedded. 3. Inconsistent validation. Because the usages - are different, the validation rules are different by usage, which - makes it hard for users to predict what will happen. 4. The fields + or "name must be restricted". Those cannot be well described when + embedded. 3. Inconsistent validation. Because the usages are + different, the validation rules are different by usage, which + makes it hard for users to predict what will happen. 4. The fields are both imprecise and overly precise. Kind is not a precise - mapping to a URL. This can produce ambiguity during interpretation + mapping to a URL. This can produce ambiguity during interpretation and require a REST mapping. In most cases, the dependency is - on the group,resource tuple and the version of the actual - struct is irrelevant. 5. We cannot easily change it. Because - this type is embedded in many locations, updates to this type will - affect numerous schemas. Don''t make new APIs embed an underspecified + on the group,resource tuple and the version of the actual struct + is irrelevant. 5. We cannot easily change it. Because this type + is embedded in many locations, updates to this type will affect + numerous schemas. Don''t make new APIs embed an underspecified API type they do not control. Instead of using this type, create a locally provided and used type that is well-focused on your reference. For example, ServiceReferences for admission registration: diff --git a/config/crd/bases/control.k8ssandra.io_cassandratasks.yaml b/config/crd/bases/control.k8ssandra.io_cassandratasks.yaml index e8bf4f43..85116cc1 100644 --- a/config/crd/bases/control.k8ssandra.io_cassandratasks.yaml +++ b/config/crd/bases/control.k8ssandra.io_cassandratasks.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.7.0 + controller-gen.kubebuilder.io/version: v0.8.0 creationTimestamp: null name: cassandratasks.control.k8ssandra.io spec: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index f335c40b..8ee7de45 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -1,4 +1,3 @@ - --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole @@ -15,7 +14,6 @@ rules: - get - list - watch - --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml index 625f0710..84794538 100644 --- a/config/webhook/manifests.yaml +++ b/config/webhook/manifests.yaml @@ -1,4 +1,3 @@ - --- apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration diff --git a/pkg/oplabels/labels.go b/pkg/oplabels/labels.go index 33feb550..ea4a1e04 100644 --- a/pkg/oplabels/labels.go +++ b/pkg/oplabels/labels.go @@ -5,7 +5,6 @@ package oplabels import ( "fmt" - api "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" ) @@ -18,15 +17,15 @@ const ( InstanceLabel = "app.kubernetes.io/instance" VersionLabel = "app.kubernetes.io/version" CreatedByLabel = "app.kubernetes.io/created-by" + CreatedByLabelValue = "cassandradatacenter_controller" ) func AddOperatorLabels(m map[string]string, dc *api.CassandraDatacenter) { m[ManagedByLabel] = ManagedByLabelValue m[NameLabel] = NameLabelValue m[VersionLabel] = dc.Spec.ServerVersion - - instanceName := fmt.Sprintf("cassandra-%s", dc.Spec.ClusterName) - m[InstanceLabel] = instanceName + m[InstanceLabel] = fmt.Sprintf("cassandra-%s", api.CleanLabelValue(dc.Spec.ClusterName)) + m[CreatedByLabel] = CreatedByLabelValue if len(dc.Spec.AdditionalLabels) != 0 { for key, value := range dc.Spec.AdditionalLabels { diff --git a/pkg/oplabels/labels_test.go b/pkg/oplabels/labels_test.go new file mode 100644 index 00000000..3af2601e --- /dev/null +++ b/pkg/oplabels/labels_test.go @@ -0,0 +1,64 @@ +package oplabels + +import ( + . "github.com/k8ssandra/cass-operator/apis/cassandra/v1beta1" + "github.com/stretchr/testify/require" + "regexp" + "strings" + "testing" +) + +/** +A valid label must be an empty string or consist of alphanumeric characters, +'-', '_' or '.', and must start and end with an alphanumeric. +*/ +func TestLabelValueClean(t *testing.T) { + + var cleaned = CleanLabelValue("TestCluster") + require.EqualValues(t, "TestCluster", cleaned, + "expect label name to not be cleaned as no need") + + cleaned = CleanLabelValue("Test Cluster") + require.EqualValues(t, "TestCluster", cleaned, + "expect space to be cleaned") + + cleaned = CleanLabelValue("+!*(-_)cor @#$%^&rect_ LABEL.name-1=<>_?,.") + require.EqualValues(t, "correct_LABEL.name-1", cleaned, + "expect label name w/ outlier chars to be cleaned") + + cleaned = CleanLabelValue("cor!@#$%^&*()rect__ LABEL.name") + require.EqualValues(t, "correct__LABEL.name", cleaned, + "expect label name w/ inside chars to be cleaned") + + cleaned = CleanLabelValue("correct") + require.EqualValues(t, "correct", cleaned, + "expect label name without chars to be correct") + + cleaned = CleanLabelValue("") + require.EqualValues(t, "", cleaned, + "expect label name as empty without chars to be correct") + + cleaned = CleanLabelValue("-_!@#$%-^&*.()<>?._-&*.") + require.EqualValues(t, "", cleaned, + "expect label name as empty as contains all bad chars") + + cleaned = CleanLabelValue("-_!@#$%-^&*.()<>?._-&*.X") + require.EqualValues(t, "X", cleaned, + "expect label name as last char only") + + cleaned = CleanLabelValue("Y-_!@#$%-^&*.()<>?._-&*.") + require.EqualValues(t, "Y", cleaned, + "expect label name as first char only") +} + +func TestWhitelistRegex(t *testing.T) { + + var whitelistRegex = regexp.MustCompile(`(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?`) + var unclean = "+!*(-_)cor @#$%^&rect _ LABEL.name-1=<>_?,." + var regexpResult = whitelistRegex.FindAllString(strings.Replace(unclean, " ", "", -1), -1) + require.EqualValues(t, "correct_LABEL.name-1", strings.Join(regexpResult, "")) + + unclean = "+!*(-_)cor @#$ %^&re _c-t-.LABEL.name-1=<>_?,." + regexpResult = whitelistRegex.FindAllString(strings.Replace(unclean, " ", "", -1), -1) + require.EqualValues(t, "corre_c-t-.LABEL.name-1", strings.Join(regexpResult, "")) +} diff --git a/pkg/reconciliation/construct_service_test.go b/pkg/reconciliation/construct_service_test.go index 9847a6f0..1e06bec9 100644 --- a/pkg/reconciliation/construct_service_test.go +++ b/pkg/reconciliation/construct_service_test.go @@ -5,10 +5,10 @@ package reconciliation import ( "fmt" + "github.com/k8ssandra/cass-operator/pkg/oplabels" "reflect" "testing" - "github.com/k8ssandra/cass-operator/pkg/oplabels" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -48,6 +48,7 @@ func TestCassandraDatacenter_allPodsServiceLabels(t *testing.T) { oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, dc.Spec.ClusterName), oplabels.NameLabel: oplabels.NameLabelValue, oplabels.VersionLabel: "4.0.1", + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, api.ClusterLabel: "bob", api.DatacenterLabel: "dc1", api.PromMetricsLabel: "true", @@ -120,6 +121,7 @@ func TestLabelsWithNewSeedServiceForCassandraDatacenter(t *testing.T) { oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, dc.Spec.ClusterName), oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.VersionLabel: "4.0.1", api.ClusterLabel: "piclem", "Add": "label", @@ -178,6 +180,7 @@ func TestLabelsWithNewNodePortServiceForCassandraDatacenter(t *testing.T) { oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, dc.Spec.ClusterName), oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.VersionLabel: "4.0.1", api.DatacenterLabel: "dc1", api.ClusterLabel: "piclem", @@ -237,6 +240,7 @@ func TestLabelsWithNewAllPodsServiceForCassandraDatacenter(t *testing.T) { oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, dc.Spec.ClusterName), oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.VersionLabel: "4.0.1", api.DatacenterLabel: "dc1", api.ClusterLabel: "piclem", @@ -297,6 +301,7 @@ func TestLabelsWithNewServiceForCassandraDatacenter(t *testing.T) { oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, dc.Spec.ClusterName), oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.VersionLabel: "4.0.1", api.DatacenterLabel: "dc1", api.ClusterLabel: "piclem", @@ -356,6 +361,7 @@ func TestLabelsWithNewAdditionalSeedServiceForCassandraDatacenter(t *testing.T) oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, dc.Spec.ClusterName), oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.VersionLabel: "4.0.1", api.DatacenterLabel: "dc1", api.ClusterLabel: "piclem", @@ -388,6 +394,7 @@ func TestAddingAdditionalLabels(t *testing.T) { oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, dc.Spec.ClusterName), oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.VersionLabel: "4.0.1", api.DatacenterLabel: "dc1", api.ClusterLabel: "piclem", diff --git a/pkg/reconciliation/construct_statefulset_test.go b/pkg/reconciliation/construct_statefulset_test.go index 58c23e8a..63507efd 100644 --- a/pkg/reconciliation/construct_statefulset_test.go +++ b/pkg/reconciliation/construct_statefulset_test.go @@ -86,6 +86,7 @@ func Test_newStatefulSetForCassandraDatacenter_additionalLabels(t *testing.T) { oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, dc.Spec.ClusterName), oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.VersionLabel: "4.0.1", api.DatacenterLabel: "dc1", api.ClusterLabel: "piclem", @@ -97,6 +98,7 @@ func Test_newStatefulSetForCassandraDatacenter_additionalLabels(t *testing.T) { oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, dc.Spec.ClusterName), oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.VersionLabel: "4.0.1", api.DatacenterLabel: "dc1", api.ClusterLabel: "piclem", @@ -105,7 +107,8 @@ func Test_newStatefulSetForCassandraDatacenter_additionalLabels(t *testing.T) { "Add": "label", } - statefulset, newStatefulSetForCassandraDatacenterError := newStatefulSetForCassandraDatacenter(nil, "rack1", dc, 1, false) + statefulset, newStatefulSetForCassandraDatacenterError := newStatefulSetForCassandraDatacenter(nil, + "rack1", dc, 1, false) assert.NoError(t, newStatefulSetForCassandraDatacenterError, "should not have gotten error when creating the new statefulset") diff --git a/pkg/reconciliation/reconcile_fql.go b/pkg/reconciliation/reconcile_fql.go index cb838ac6..63c0dc42 100644 --- a/pkg/reconciliation/reconcile_fql.go +++ b/pkg/reconciliation/reconcile_fql.go @@ -2,7 +2,6 @@ package reconciliation import ( "errors" - "github.com/k8ssandra/cass-operator/pkg/httphelper" "github.com/k8ssandra/cass-operator/pkg/internal/result" ) diff --git a/pkg/reconciliation/reconcile_racks_test.go b/pkg/reconciliation/reconcile_racks_test.go index 1453bce1..387a5899 100644 --- a/pkg/reconciliation/reconcile_racks_test.go +++ b/pkg/reconciliation/reconcile_racks_test.go @@ -63,10 +63,37 @@ func Test_validateLabelsForCluster(t *testing.T) { api.ClusterLabel: "exampleCluster", oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-exampleCluster", oplabels.NameLabelValue), oplabels.VersionLabel: "4.0.1", }, }, { + name: "Cluster name with spaces", + args: args{ + resourceLabels: make(map[string]string), + rc: &ReconciliationContext{ + Datacenter: &api.CassandraDatacenter{ + ObjectMeta: metav1.ObjectMeta{ + Name: "exampleDC", + }, + Spec: api.CassandraDatacenterSpec{ + ClusterName: "Example Cluster", + ServerVersion: "4.0.1", + }, + }, + }, + }, + want: true, + wantLabels: map[string]string{ + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, + api.ClusterLabel: "ExampleCluster", + oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, + oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.InstanceLabel: fmt.Sprintf("%s-ExampleCluster", oplabels.NameLabelValue), + oplabels.VersionLabel: "4.0.1", + }, + }, + { name: "Nil labels", args: args{ resourceLabels: nil, @@ -87,6 +114,7 @@ func Test_validateLabelsForCluster(t *testing.T) { api.ClusterLabel: "exampleCluster", oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-exampleCluster", oplabels.NameLabelValue), oplabels.VersionLabel: "4.0.1", }, @@ -98,6 +126,7 @@ func Test_validateLabelsForCluster(t *testing.T) { api.ClusterLabel: "exampleCluster", oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-exampleCluster", oplabels.NameLabelValue), oplabels.VersionLabel: "4.0.1", }, @@ -118,6 +147,7 @@ func Test_validateLabelsForCluster(t *testing.T) { api.ClusterLabel: "exampleCluster", oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-exampleCluster", oplabels.NameLabelValue), oplabels.VersionLabel: "4.0.1", }, @@ -145,6 +175,7 @@ func Test_validateLabelsForCluster(t *testing.T) { api.ClusterLabel: "exampleCluster", oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-exampleCluster", oplabels.NameLabelValue), oplabels.VersionLabel: "6.8.13", }, @@ -1235,6 +1266,7 @@ func Test_shouldUpdateLabelsForRackResource(t *testing.T) { api.RackLabel: rackName, oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, clusterName), oplabels.VersionLabel: "4.0.1", } @@ -1319,6 +1351,7 @@ func Test_shouldUpdateLabelsForRackResource(t *testing.T) { api.RackLabel: rackName, oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, clusterName), oplabels.VersionLabel: "4.0.1", }, @@ -1336,6 +1369,7 @@ func Test_shouldUpdateLabelsForRackResource(t *testing.T) { api.RackLabel: rackName, oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.InstanceLabel: fmt.Sprintf("%s-%s", oplabels.NameLabelValue, clusterName), oplabels.VersionLabel: "4.0.1", "foo": "bar", diff --git a/pkg/reconciliation/secrets_test.go b/pkg/reconciliation/secrets_test.go index d045388e..39e813d6 100644 --- a/pkg/reconciliation/secrets_test.go +++ b/pkg/reconciliation/secrets_test.go @@ -54,6 +54,7 @@ func Test_buildDefaultSuperuserSecret(t *testing.T) { oplabels.InstanceLabel: "cassandra-exampleCluster", oplabels.ManagedByLabel: oplabels.ManagedByLabelValue, oplabels.NameLabel: oplabels.NameLabelValue, + oplabels.CreatedByLabel: oplabels.CreatedByLabelValue, oplabels.VersionLabel: "", "piclem": "add", } diff --git a/tests/testdata/smoke-test-oss.yaml b/tests/testdata/smoke-test-oss.yaml index 9570efdf..4ced7f59 100644 --- a/tests/testdata/smoke-test-oss.yaml +++ b/tests/testdata/smoke-test-oss.yaml @@ -3,7 +3,7 @@ kind: CassandraDatacenter metadata: name: dc2 spec: - clusterName: cluster2 + clusterName: cluster 2 serverType: cassandra serverVersion: "4.0.0" managementApiAuth: