diff --git a/CHANGELOG/CHANGELOG-1.22.md b/CHANGELOG/CHANGELOG-1.22.md index 58ab84827..be379bfd1 100644 --- a/CHANGELOG/CHANGELOG-1.22.md +++ b/CHANGELOG/CHANGELOG-1.22.md @@ -14,3 +14,5 @@ Changelog for the K8ssandra Operator, new PRs should update the `unreleased` sec When cutting a new release, update the `unreleased` heading to the tag being generated and date, like `## vX.Y.Z - YYYY-MM-DD` and create a new placeholder section for `unreleased` entries. ## unreleased + +* [BUGFIX] [#1489](https://github.com/k8ssandra/k8ssandra-operator/issues/1489) Fix reconciliation of Medusa with config without credentials file. diff --git a/controllers/k8ssandra/k8ssandracluster_controller_test.go b/controllers/k8ssandra/k8ssandracluster_controller_test.go index cad05d642..6401fcc2d 100644 --- a/controllers/k8ssandra/k8ssandracluster_controller_test.go +++ b/controllers/k8ssandra/k8ssandracluster_controller_test.go @@ -102,6 +102,7 @@ func TestK8ssandraCluster(t *testing.T) { t.Run("createMultiDcClusterWithControlPlaneReaper", testEnv.ControllerTest(ctx, createMultiDcClusterWithControlPlaneReaper)) t.Run("CreateMultiDcClusterWithMedusa", testEnv.ControllerTest(ctx, createMultiDcClusterWithMedusa)) t.Run("CreateSingleDcClusterWithMedusaConfigRef", testEnv.ControllerTest(ctx, createSingleDcClusterWithMedusaConfigRef)) + t.Run("CreateSingleDcClusterWithoutStorageCredentials", testEnv.ControllerTest(ctx, createSingleDcClusterWithoutStorageCredentials)) t.Run("CreateSingleDcClusterWithManagementApiSecured", testEnv.ControllerTest(ctx, createSingleDcClusterWithManagementApiSecured)) t.Run("CreatingSingleDcClusterWithoutPrefixInClusterSpecFail", testEnv.ControllerTest(ctx, creatingSingleDcClusterWithoutPrefixInClusterSpecFails)) t.Run("CreateMultiDcClusterWithReplicatedSecrets", testEnv.ControllerTest(ctx, createMultiDcClusterWithReplicatedSecrets)) diff --git a/controllers/k8ssandra/medusa_reconciler.go b/controllers/k8ssandra/medusa_reconciler.go index 357c1b52f..8f6d95d6c 100644 --- a/controllers/k8ssandra/medusa_reconciler.go +++ b/controllers/k8ssandra/medusa_reconciler.go @@ -10,9 +10,7 @@ import ( "github.com/adutra/goalesce" "github.com/go-logr/logr" api "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" - k8ssandraapi "github.com/k8ssandra/k8ssandra-operator/apis/k8ssandra/v1alpha1" medusaapi "github.com/k8ssandra/k8ssandra-operator/apis/medusa/v1alpha1" - replication "github.com/k8ssandra/k8ssandra-operator/apis/replication/v1alpha1" cassandra "github.com/k8ssandra/k8ssandra-operator/pkg/cassandra" "github.com/k8ssandra/k8ssandra-operator/pkg/labels" medusa "github.com/k8ssandra/k8ssandra-operator/pkg/medusa" @@ -20,11 +18,8 @@ import ( "github.com/k8ssandra/k8ssandra-operator/pkg/result" "github.com/k8ssandra/k8ssandra-operator/pkg/secret" "github.com/k8ssandra/k8ssandra-operator/pkg/utils" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) const ( @@ -60,15 +55,10 @@ func (r *K8ssandraClusterReconciler) reconcileMedusa( } } - if medusaSpec.StorageProperties.StorageSecretRef.Name == "" { - medusaSpec.StorageProperties.StorageSecretRef = corev1.LocalObjectReference{Name: ""} - if medusaSpec.StorageProperties.CredentialsType == "role-based" && medusaSpec.StorageProperties.StorageProvider == "s3" { - // It's okay for the secret ref to be unset - medusaSpec.StorageProperties.StorageSecretRef = corev1.LocalObjectReference{Name: ""} - } else { - return result.Error(fmt.Errorf("medusa storage secret is not defined for storage provider %s", medusaSpec.StorageProperties.StorageProvider)) - } + if err := r.validateStorageCredentials(medusaSpec); err != nil { + return result.Error(err) } + if res := r.reconcileMedusaConfigMap(ctx, remoteClient, kc, dcConfig, logger, dcNamespace); res.Completed() { return res } @@ -168,12 +158,7 @@ func (r *K8ssandraClusterReconciler) reconcileMedusaSecrets( logger.Error(err, "Failed to reconcile Medusa CQL user secret") return result.Error(err) } - - if res := r.reconcileRemoteBucketSecretsDeprecated(ctx, r.ClientCache.GetLocalClient(), kc, logger); res.Completed() { - return res - } } - logger.Info("Medusa user secrets successfully reconciled") return result.Continue() } @@ -234,98 +219,25 @@ func (r *K8ssandraClusterReconciler) mergeStorageProperties( logger.Error(err, "failed to merge MedusaConfiguration StorageProperties") return result.Error(err) } - // medusaapi.MedusaConfiguration comes with a storage corev1.Secret containing the credentials to access the storage - // we make a copy of that secret for each cluster/dc, and then point to it with a corev1.LocalObjectReference - // when we do the copy, we name the secret as - - // here we need to update the reference to point to that copied secret - if desiredKc.Namespace != medusaSpec.MedusaConfigurationRef.Name { - // Deprecated: when we remove the ability to reference a non-namespace-local MedusaConfig in v 1.17, - // this if statement should be eliminated. - mergedProperties.StorageSecretRef.Name = fmt.Sprintf("%s-%s", desiredKc.Name, mergedProperties.StorageSecretRef.Name) - } // imagine an else here which just contains `mergedProperties.StorageSecretRef.Name = mergedProperties.StorageSecretRef.Name`, since this is a no-op. + + // only touch storage secret names when we actually need them (which we don't for role-based credentials) + if mergedProperties.CredentialsType != medusa.CredentialsTypeRoleBased { + // medusaapi.MedusaConfiguration comes with a storage corev1.Secret containing the credentials to access the storage + // we make a copy of that secret for each cluster/dc, and then point to it with a corev1.LocalObjectReference + // when we do the copy, we name the secret as - + // here we need to update the reference to point to that copied secret + if desiredKc.Namespace != medusaSpec.MedusaConfigurationRef.Name { + // Deprecated: when we remove the ability to reference a non-namespace-local MedusaConfig in v 1.17, + // this if statement should be eliminated. + mergedProperties.StorageSecretRef.Name = fmt.Sprintf("%s-%s", desiredKc.Name, mergedProperties.StorageSecretRef.Name) + } // imagine an else here which just contains `mergedProperties.StorageSecretRef.Name = mergedProperties.StorageSecretRef.Name`, since this is a no-op. + } // copy the merged properties back into the cluster mergedProperties.DeepCopyInto(&desiredKc.Spec.Medusa.StorageProperties) return result.Continue() } -// Deprecated: This code path can be removed at version 1.17, as MedusaConfigs should now always be namespace-local to the K8ssandraCluster referencing them. At that point, we no longer -// need this code, because it is mainly concerned with copying the bucket secrets into the K8ssandraCluster's namespace. -func (r *K8ssandraClusterReconciler) reconcileRemoteBucketSecretsDeprecated( - ctx context.Context, - c client.Client, - kc *api.K8ssandraCluster, - logger logr.Logger, -) result.ReconcileResult { - logger.Info("Reconciling Medusa bucket secrets") - medusaSpec := kc.Spec.Medusa - - // there is nothing to reconcile if we're not using Medusa configuration reference - if medusaSpec == nil || medusaSpec.MedusaConfigurationRef.Name == "" { - logger.Info("MedusaConfigurationRef is not set, skipping bucket secret reconciliation") - return result.Continue() - } - - // This is the deprecated code path. Moving forward we will use a replicated secret with a prefix, but we will remove this code path after v1.17. - // fetch the referenced configuration - medusaConfigName := medusaSpec.MedusaConfigurationRef.Name - medusaConfigNamespace := utils.FirstNonEmptyString(medusaSpec.MedusaConfigurationRef.Namespace, kc.Namespace) - medusaConfigKey := types.NamespacedName{Namespace: medusaConfigNamespace, Name: medusaConfigName} - medusaConfig := &medusaapi.MedusaConfiguration{} - if err := c.Get(ctx, medusaConfigKey, medusaConfig); err != nil { - logger.Error(err, fmt.Sprintf("could not get MedusaConfiguration %s/%s", medusaConfigNamespace, medusaConfigName)) - return result.Error(err) - } - - repSecret := replication.ReplicatedSecret{ - ObjectMeta: metav1.ObjectMeta{ - Name: kc.GetClusterIdHash() + "-" + medusaConfig.Spec.StorageProperties.StorageSecretRef.Name, - Namespace: medusaConfigNamespace, - Labels: map[string]string{ - k8ssandraapi.K8ssandraClusterNameLabel: kc.Name, - k8ssandraapi.K8ssandraClusterNamespaceLabel: kc.Namespace, - }, - Annotations: map[string]string{ - k8ssandraapi.PurposeAnnotation: "This replicated secret is designed for the old codepath in the Medusa reconciler within the k8ssandra cluster controller. In this deprecated path, a MedusaConfig could reside in a different namespace to the K8ssandraCluster. This ReplicatedSecret ensures that the bucket secret is copied into the K8ssandraCluster's namespace. This codepath will be removed in v1.17.", - }, - }, - Spec: replication.ReplicatedSecretSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - medusaapi.MedusaStorageSecretIdentifierLabel: utils.HashNameNamespace( - medusaConfig.Spec.StorageProperties.StorageSecretRef.Name, - medusaConfigNamespace), - }, - }, - ReplicationTargets: []replication.ReplicationTarget{ - { - Namespace: kc.Namespace, - TargetPrefix: kc.Name + "-", - DropLabels: []string{ - medusaapi.MedusaStorageSecretIdentifierLabel, - }, - AddLabels: map[string]string{ - k8ssandraapi.K8ssandraClusterNameLabel: kc.Name, - k8ssandraapi.K8ssandraClusterNamespaceLabel: kc.Namespace, - k8ssandraapi.ReplicatedByLabel: k8ssandraapi.ReplicatedByLabelValue, - }, - }, - }, - }, - } - if err := controllerutil.SetControllerReference(kc, &repSecret, r.Scheme); err != nil { - return result.Error(err) - } - if err := controllerutil.SetOwnerReference(kc, &repSecret, r.Scheme); err != nil { - return result.Error(err) - } - // TODO: this should also have finalizer logic included in the k8ssandraCluster finalizer to remove the replicated secret if it is no longer being used. - // TODO: this should probably have a finalizer on it too so that the replicatedSecret cannot be deleted. - - return reconciliation.ReconcileObject(ctx, c, r.DefaultDelay, repSecret) - -} - func (r *K8ssandraClusterReconciler) getOperatorNamespace() string { operatorNamespace, found := os.LookupEnv(operatorNamespaceEnvVar) if !found { @@ -333,3 +245,20 @@ func (r *K8ssandraClusterReconciler) getOperatorNamespace() string { } return operatorNamespace } + +func (r *K8ssandraClusterReconciler) validateStorageCredentials(medusaSpec *medusaapi.MedusaClusterTemplate) error { + + // we must specify either storage secret or role-based credentials + if medusaSpec.StorageProperties.StorageSecretRef.Name == "" && medusaSpec.StorageProperties.CredentialsType != medusa.CredentialsTypeRoleBased { + return fmt.Errorf("must specify either a storge secret or use role-based credentials") + } + + // if a storage secret is set, we error if role-based credentials are set too + if medusaSpec.StorageProperties.StorageSecretRef.Name != "" { + if medusaSpec.StorageProperties.CredentialsType == medusa.CredentialsTypeRoleBased { + return fmt.Errorf("cannot specify both a storage secret and role-based credentials: %s", medusaSpec.StorageProperties.StorageSecretRef.Name) + } + } + + return nil +} diff --git a/controllers/k8ssandra/medusa_reconciler_test.go b/controllers/k8ssandra/medusa_reconciler_test.go index cdfc5ade5..c4a645f6a 100644 --- a/controllers/k8ssandra/medusa_reconciler_test.go +++ b/controllers/k8ssandra/medusa_reconciler_test.go @@ -3,6 +3,7 @@ package k8ssandra import ( "context" "fmt" + "github.com/k8ssandra/k8ssandra-operator/pkg/medusa" "strings" "testing" @@ -34,6 +35,8 @@ const ( prefixFromClusterSpec = "prefix-from-cluster-spec" defaultConcurrentTransfers = 1 concurrentTransfersFromMedusaConfig = 2 + keyFilePresent = true + keyFileAbsent = false ) func dcTemplate(dcName string, dataPlaneContext string) api.CassandraDatacenterTemplate { @@ -171,7 +174,7 @@ func createMultiDcClusterWithMedusa(t *testing.T, ctx context.Context, f *framew t.Log("verify the config map exists and has the contents from the MedusaConfiguration object") defaultPrefix := kc.Spec.Medusa.StorageProperties.Prefix - verifyConfigMap(require, ctx, f, namespace, defaultPrefix, defaultConcurrentTransfers) + verifyConfigMap(require, ctx, f, namespace, defaultPrefix, defaultConcurrentTransfers, keyFilePresent) t.Log("update datacenter status to scaling up") err = f.PatchDatacenterStatus(ctx, dc1Key, func(dc *cassdcapi.CassandraDatacenter) { @@ -395,10 +398,51 @@ func createSingleDcClusterWithMedusaConfigRef(t *testing.T, ctx context.Context, verifyReplicatedSecretReconciled(ctx, t, f, kc) t.Log("verify the config map exists and has the contents from the MedusaConfiguration object") - verifyConfigMap(require, ctx, f, namespace, prefixFromClusterSpec, concurrentTransfersFromMedusaConfig) + verifyConfigMap(require, ctx, f, namespace, prefixFromClusterSpec, concurrentTransfersFromMedusaConfig, keyFilePresent) } -func verifyConfigMap(r *require.Assertions, ctx context.Context, f *framework.Framework, namespace string, expectedPrefix string, expectedConcurrentTransfers int) { +func createSingleDcClusterWithoutStorageCredentials(t *testing.T, ctx context.Context, f *framework.Framework, namespace string) { + require := require.New(t) + + t.Log("Creating Medusa Configuration object") + medusaConfig := MedusaConfig(medusaConfigName, namespace) + medusaConfig.Spec.StorageProperties.CredentialsType = medusa.CredentialsTypeRoleBased + medusaConfigKey := controlPlaneContextKey(f, medusaConfig, f.ControlPlaneContext) + err := f.Create(ctx, medusaConfigKey, medusaConfig) + + require.NoError(err, "failed to create Medusa Configuration") + require.Eventually(f.MedusaConfigExists(ctx, f.ControlPlaneContext, medusaConfigKey), timeout, interval) + + kc := &api.K8ssandraCluster{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: k8ssandraClusterName, + }, + Spec: api.K8ssandraClusterSpec{ + Cassandra: &api.CassandraClusterTemplate{ + Datacenters: []api.CassandraDatacenterTemplate{ + dcTemplate("dc1", f.DataPlaneContexts[0]), + }, + }, + Medusa: medusaTemplateWithConfigRefWithPrefix(medusaConfigName, namespace, prefixFromClusterSpec), + }, + } + kc.Spec.Medusa.StorageProperties.StorageSecretRef.Name = "" + require.NotNil(kc.Spec.Medusa.MedusaConfigurationRef) + require.Equal(medusaConfigName, kc.Spec.Medusa.MedusaConfigurationRef.Name) + + t.Log("Creating k8ssandracluster with Medusa and a config ref") + err = f.Client.Create(ctx, kc) + require.NoError(err, "failed to create K8ssandraCluster") + + // even though we're not creating a storage secret, we still need this to happen so that the Medusa config map gets created + verifyReplicatedSecretReconciled(ctx, t, f, kc) + + t.Log("verify the config map exists and has the contents from the MedusaConfiguration object") + verifyConfigMap(require, ctx, f, namespace, prefixFromClusterSpec, concurrentTransfersFromMedusaConfig, keyFileAbsent) +} + +func verifyConfigMap(r *require.Assertions, ctx context.Context, f *framework.Framework, namespace string, expectedPrefix string, expectedConcurrentTransfers int, keyFilePresent bool) { configMapName := fmt.Sprintf("%s-medusa", k8ssandraClusterName) configMapKey := framework.ClusterKey{NamespacedName: types.NamespacedName{Namespace: namespace, Name: configMapName}, K8sContext: f.DataPlaneContexts[0]} configMap := &corev1.ConfigMap{} @@ -409,7 +453,8 @@ func verifyConfigMap(r *require.Assertions, ctx context.Context, f *framework.Fr } prefixCorrect := strings.Contains(configMap.Data["medusa.ini"], fmt.Sprintf("prefix = %s", expectedPrefix)) concurrentTransfersCorrect := strings.Contains(configMap.Data["medusa.ini"], fmt.Sprintf("concurrent_transfers = %d", expectedConcurrentTransfers)) - return prefixCorrect && concurrentTransfersCorrect + keyFileCorrect := strings.Contains(configMap.Data["medusa.ini"], "key_file = /etc/medusa-secrets/credentials") == keyFilePresent + return prefixCorrect && concurrentTransfersCorrect && keyFileCorrect }, timeout, interval, "Medusa ConfigMap doesn't have the right content") } diff --git a/pkg/medusa/reconcile.go b/pkg/medusa/reconcile.go index 7107b8744..71703b4b9 100644 --- a/pkg/medusa/reconcile.go +++ b/pkg/medusa/reconcile.go @@ -43,6 +43,8 @@ const ( MedusaBackupsVolumeName = "medusa-backups" MedusaBackupsMountPath = "/mnt/backups" serviceAccountNameEnvVar = "SERVICE_ACCOUNT_NAME" + + CredentialsTypeRoleBased = "role-based" ) var (