diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 783b570a..60aa0269 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -18,6 +18,15 @@ rules: - patch - update - watch + - apiGroups: + - "" + resources: + - configmaps + verbs: + - delete + - get + - list + - watch - apiGroups: - "" resources: @@ -28,16 +37,15 @@ rules: - apiGroups: - "" resources: - - services - - configmaps - secrets + - services verbs: - - get - - patch - - update - create - delete + - get - list + - patch + - update - watch - apiGroups: - perses.dev @@ -143,4 +151,3 @@ rules: - get - patch - update - diff --git a/controllers/perses/perses_controller.go b/controllers/perses/perses_controller.go index 00c118fb..3f787e42 100644 --- a/controllers/perses/perses_controller.go +++ b/controllers/perses/perses_controller.go @@ -79,6 +79,8 @@ var log = logger.WithField("module", "perses_controller") // +kubebuilder:rbac:groups=core,resources=events,verbs=create;patch // +kubebuilder:rbac:groups=apps,resources=deployments;statefulsets,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch +// +kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;delete func (r *PersesReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { start := time.Now() objKey := req.String() @@ -118,7 +120,8 @@ func (r *PersesReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr r.validateVolumes, r.reconcileProvisioning, r.reconcileService, - r.reconcileConfigMap, + r.reconcileConfigSecret, + r.cleanupOldConfigMap, r.reconcileDeployment, r.reconcileStatefulSet, r.setStatusToComplete, @@ -488,7 +491,7 @@ func (r *PersesReconciler) SetupWithManager(mgr ctrl.Manager) error { For(&v1alpha2.Perses{}). Owns(&appsv1.Deployment{}). Owns(&appsv1.StatefulSet{}). - Owns(&corev1.ConfigMap{}). + Owns(&corev1.Secret{}). Owns(&corev1.Service{}). Watches( &corev1.Secret{}, diff --git a/controllers/perses/configmap_controller.go b/controllers/perses/secret_controller.go similarity index 50% rename from controllers/perses/configmap_controller.go rename to controllers/perses/secret_controller.go index 152297e3..5aeb420a 100644 --- a/controllers/perses/configmap_controller.go +++ b/controllers/perses/secret_controller.go @@ -1,12 +1,12 @@ // Copyright The Perses Authors -// Licensed under the Apache License, Version 2.0 (the \"License\"); +// Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // // http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an \"AS IS\" BASIS, +// distributed under the License is distributed on an "AS IS" BASIS, // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. @@ -33,64 +33,63 @@ import ( "github.com/perses/perses-operator/internal/subreconciler" ) -var cmlog = logger.WithField("module", "configmap_controller") +var secretlog = logger.WithField("module", "secret_controller") -func (r *PersesReconciler) reconcileConfigMap(ctx context.Context, req ctrl.Request) (*ctrl.Result, error) { +func (r *PersesReconciler) reconcileConfigSecret(ctx context.Context, req ctrl.Request) (*ctrl.Result, error) { perses, ok := persesFromContext(ctx) if !ok { - cmlog.Error("perses not found in context") + secretlog.Error("perses not found in context") return subreconciler.RequeueWithError(fmt.Errorf("perses not found in context")) } configName := common.GetConfigName(perses.Name) - found := &corev1.ConfigMap{} + found := &corev1.Secret{} if err := r.Get(ctx, types.NamespacedName{Name: configName, Namespace: perses.Namespace}, found); err != nil { if !apierrors.IsNotFound(err) { - cmlog.WithError(err).Error("Failed to get ConfigMap") + secretlog.WithError(err).Error("Failed to get config Secret") return subreconciler.RequeueWithError(err) } - cm, err2 := r.createPersesConfigMap(perses) + sec, err2 := r.createPersesConfigSecret(perses) if err2 != nil { - cmlog.WithError(err2).Error("Failed to define new ConfigMap resource for perses") + secretlog.WithError(err2).Error("Failed to define new config Secret resource for perses") meta.SetStatusCondition(&perses.Status.Conditions, metav1.Condition{Type: common.TypeAvailablePerses, Status: metav1.ConditionFalse, Reason: "Reconciling", - Message: fmt.Sprintf("Failed to create ConfigMap for the custom resource (%s): (%s)", perses.Name, err2)}) + Message: fmt.Sprintf("Failed to create config Secret for the custom resource (%s): (%s)", perses.Name, err2)}) if err = r.Status().Update(ctx, perses); err != nil { - cmlog.WithError(err).Error("Failed to update perses status") + secretlog.WithError(err).Error("Failed to update perses status") return subreconciler.RequeueWithError(err) } return subreconciler.RequeueWithError(err2) } - cmlog.Infof("Creating a new ConfigMap: ConfigMap.Namespace %s ConfigMap.Name %s", cm.Namespace, cm.Name) - if err = r.Create(ctx, cm); err != nil { - cmlog.WithError(err).Errorf("Failed to create new ConfigMap: ConfigMap.Namespace %s ConfigMap.Name %s", cm.Namespace, cm.Name) + secretlog.Infof("Creating a new config Secret: Secret.Namespace %s Secret.Name %s", sec.Namespace, sec.Name) + if err = r.Create(ctx, sec); err != nil { + secretlog.WithError(err).Errorf("Failed to create new config Secret: Secret.Namespace %s Secret.Name %s", sec.Namespace, sec.Name) return subreconciler.RequeueWithError(err) } return subreconciler.ContinueReconciling() } - cm, err := r.createPersesConfigMap(perses) + sec, err := r.createPersesConfigSecret(perses) if err != nil { - cmlog.WithError(err).Error("Failed to define new ConfigMap resource for perses") + secretlog.WithError(err).Error("Failed to define new config Secret resource for perses") return subreconciler.RequeueWithError(err) } - // call update with dry run to fill out fields that are also returned via the k8s api - if err := r.Update(ctx, cm, client.DryRunAll); err != nil { - cmlog.WithError(err).Error("Failed to update ConfigMap with dry run") + if err := r.Update(ctx, sec, client.DryRunAll); err != nil { + secretlog.WithError(err).Error("Failed to update config Secret with dry run") return subreconciler.RequeueWithError(err) } - if configMapNeedsUpdate(found, cm, configName, perses) { - if err := r.Update(ctx, cm); err != nil { - cmlog.WithError(err).Error("Failed to update ConfigMap") + if configSecretNeedsUpdate(found, sec, configName, perses) { + if err := r.Update(ctx, sec); err != nil { + secretlog.WithError(err).Error("Failed to update config Secret") return subreconciler.RequeueWithError(err) } } @@ -98,7 +97,7 @@ func (r *PersesReconciler) reconcileConfigMap(ctx context.Context, req ctrl.Requ return subreconciler.ContinueReconciling() } -func configMapNeedsUpdate(existing, updated *corev1.ConfigMap, name string, perses *v1alpha2.Perses) bool { +func configSecretNeedsUpdate(existing, updated *corev1.Secret, name string, perses *v1alpha2.Perses) bool { if existing == nil && updated == nil { return false } @@ -113,7 +112,6 @@ func configMapNeedsUpdate(existing, updated *corev1.ConfigMap, name string, pers return true } - // check for differences only in the labels that are set by the operator labels := common.LabelsForPerses(name, perses) for k := range labels { if existing.Labels[k] != updated.Labels[k] { @@ -124,7 +122,7 @@ func configMapNeedsUpdate(existing, updated *corev1.ConfigMap, name string, pers return false } -func (r *PersesReconciler) createPersesConfigMap(perses *v1alpha2.Perses) (*corev1.ConfigMap, error) { +func (r *PersesReconciler) createPersesConfigSecret(perses *v1alpha2.Perses) (*corev1.Secret, error) { configName := common.GetConfigName(perses.Name) ls := common.LabelsForPerses(configName, perses) @@ -134,29 +132,53 @@ func (r *PersesReconciler) createPersesConfigMap(perses *v1alpha2.Perses) (*core } persesConfig, err := yaml.Marshal(perses.Spec.Config.Config) - if err != nil { - cmlog.WithError(err).Errorf("Failed to marshal configmap data: ConfigMap.Namespace %s ConfigMap.Name %s", perses.Namespace, configName) + secretlog.WithError(err).Errorf("Failed to marshal config data: Secret.Namespace %s Secret.Name %s", perses.Namespace, configName) return nil, err } - configData := map[string]string{ - "config.yaml": string(persesConfig), - } - - cm := &corev1.ConfigMap{ + sec := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: configName, Namespace: perses.Namespace, Annotations: annotations, Labels: ls, }, - Data: configData, + Data: map[string][]byte{ + "config.yaml": persesConfig, + }, } - // Set Perses instance as the owner and controller - if err := ctrl.SetControllerReference(perses, cm, r.Scheme); err != nil { + if err := ctrl.SetControllerReference(perses, sec, r.Scheme); err != nil { return nil, err } - return cm, nil + return sec, nil +} + +// cleanupOldConfigMap deletes any leftover ConfigMap from before the migration +// to Secret-based config storage. This ensures a clean upgrade path. +func (r *PersesReconciler) cleanupOldConfigMap(ctx context.Context, req ctrl.Request) (*ctrl.Result, error) { + perses, ok := persesFromContext(ctx) + if !ok { + secretlog.Error("perses not found in context") + return subreconciler.RequeueWithError(fmt.Errorf("perses not found in context")) + } + + configName := common.GetConfigName(perses.Name) + oldCM := &corev1.ConfigMap{} + if err := r.Get(ctx, types.NamespacedName{Name: configName, Namespace: perses.Namespace}, oldCM); err != nil { + if apierrors.IsNotFound(err) { + return subreconciler.ContinueReconciling() + } + secretlog.WithError(err).Error("Failed to check for old ConfigMap") + return subreconciler.RequeueWithError(err) + } + + secretlog.Infof("Cleaning up old ConfigMap: %s/%s (migrated to Secret)", oldCM.Namespace, oldCM.Name) + if err := r.Delete(ctx, oldCM); err != nil && !apierrors.IsNotFound(err) { + secretlog.WithError(err).Error("Failed to delete old ConfigMap") + return subreconciler.RequeueWithError(err) + } + + return subreconciler.ContinueReconciling() } diff --git a/controllers/perses_controller_test.go b/controllers/perses_controller_test.go index ada77812..b68b6af5 100644 --- a/controllers/perses_controller_test.go +++ b/controllers/perses_controller_test.go @@ -51,7 +51,7 @@ var _ = Describe("Perses controller", func() { It("should successfully reconcile a custom resource for Perses", func() { typeNamespaceName := types.NamespacedName{Name: PersesName, Namespace: persesNamespace} - configMapNamespaceName := types.NamespacedName{Name: common.GetConfigName(PersesName), Namespace: persesNamespace} + configSecretNamespaceName := types.NamespacedName{Name: common.GetConfigName(PersesName), Namespace: persesNamespace} By("Creating the custom resource for the Kind Perses") perses := &persesv1alpha2.Perses{} @@ -159,10 +159,10 @@ var _ = Describe("Perses controller", func() { return err }, time.Minute, time.Second).Should(Succeed()) - By("Checking if ConfigMap was successfully created in the reconciliation") + By("Checking if config Secret was successfully created in the reconciliation") Eventually(func() error { - found := &corev1.ConfigMap{} - return k8sClient.Get(ctx, configMapNamespaceName, found) + found := &corev1.Secret{} + return k8sClient.Get(ctx, configSecretNamespaceName, found) }, time.Minute*3, time.Second).Should(Succeed()) By("Checking if StatefulSet was successfully created in the reconciliation") @@ -262,10 +262,10 @@ var _ = Describe("Perses controller", func() { return k8sClient.Get(ctx, types.NamespacedName{Name: persesServiceName, Namespace: persesNamespace}, found) }, time.Minute, time.Second).Should(Succeed()) - By("Checking if ConfigMap was successfully deleted in the reconciliation") + By("Checking if config Secret was successfully deleted in the reconciliation") Eventually(func() error { - found := &corev1.ConfigMap{} - return k8sClient.Get(ctx, configMapNamespaceName, found) + found := &corev1.Secret{} + return k8sClient.Get(ctx, configSecretNamespaceName, found) }, time.Minute, time.Second).Should(Succeed()) By("Checking the latest Status Condition added to the Perses instance") @@ -286,7 +286,7 @@ var _ = Describe("Perses controller", func() { It("should successfully reconcile a custom resource for Perses with storage", func() { PersesStorageName := "perses-test-with-storage" typeNamespaceName := types.NamespacedName{Name: PersesStorageName, Namespace: persesNamespace} - configMapNamespaceName := types.NamespacedName{Name: common.GetConfigName(PersesStorageName), Namespace: persesNamespace} + configSecretNamespaceName := types.NamespacedName{Name: common.GetConfigName(PersesStorageName), Namespace: persesNamespace} By("Creating the custom resource for the Kind Perses with storage") perses := &persesv1alpha2.Perses{} @@ -405,10 +405,10 @@ var _ = Describe("Perses controller", func() { return k8sClient.Get(ctx, typeNamespaceName, found) }, time.Minute, time.Second).Should(Succeed()) - By("Checking if ConfigMap was successfully deleted in the reconciliation") + By("Checking if config Secret was successfully deleted in the reconciliation") Eventually(func() error { - found := &corev1.ConfigMap{} - return k8sClient.Get(ctx, configMapNamespaceName, found) + found := &corev1.Secret{} + return k8sClient.Get(ctx, configSecretNamespaceName, found) }, time.Minute, time.Second).Should(Succeed()) By("Checking the latest Status Condition added to the Perses instance") @@ -1028,5 +1028,83 @@ var _ = Describe("Perses controller", func() { Expect(k8sClient.Get(ctx, typeNamespaceName, persesToDelete)).To(Succeed()) Expect(k8sClient.Delete(ctx, persesToDelete)).To(Succeed()) }) + + It("should clean up old ConfigMap when migrating to Secret-based config", func() { + const migrationName = "perses-migration-test" + typeNamespaceName := types.NamespacedName{Name: migrationName, Namespace: persesNamespace} + configName := common.GetConfigName(migrationName) + configNamespaceName := types.NamespacedName{Name: configName, Namespace: persesNamespace} + + By("Creating a Perses CR") + perses := &persesv1alpha2.Perses{ + ObjectMeta: metav1.ObjectMeta{ + Name: migrationName, + Namespace: persesNamespace, + }, + Spec: persesv1alpha2.PersesSpec{ + Image: ptr.To(persesImage), + Config: persesv1alpha2.PersesConfig{ + Config: persesconfig.Config{ + Database: persesconfig.Database{ + File: &persesconfig.File{ + Folder: "/etc/perses/storage", + }, + }, + }, + }, + Storage: &persesv1alpha2.StorageConfiguration{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + } + Expect(k8sClient.Create(ctx, perses)).To(Succeed()) + + By("Simulating a pre-existing ConfigMap from before the migration") + oldCM := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: configName, + Namespace: persesNamespace, + }, + Data: map[string]string{ + "config.yaml": "database:\n file:\n folder: /etc/perses/storage\n", + }, + } + Expect(k8sClient.Create(ctx, oldCM)).To(Succeed()) + + By("Verifying the old ConfigMap exists") + Eventually(func() error { + return k8sClient.Get(ctx, configNamespaceName, &corev1.ConfigMap{}) + }, time.Second*10, time.Millisecond*250).Should(Succeed()) + + persesReconciler := &persescontroller.PersesReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + By("Reconciling to trigger the migration") + Eventually(func() error { + _, err := persesReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespaceName, + }) + return err + }, time.Second*10, time.Millisecond*250).Should(Succeed()) + + By("Checking that a config Secret was created") + Eventually(func() error { + return k8sClient.Get(ctx, configNamespaceName, &corev1.Secret{}) + }, time.Minute, time.Second).Should(Succeed()) + + By("Checking that the old ConfigMap was deleted") + Eventually(func() bool { + err := k8sClient.Get(ctx, configNamespaceName, &corev1.ConfigMap{}) + return errors.IsNotFound(err) + }, time.Minute, time.Second).Should(BeTrue(), "Old ConfigMap should be deleted after migration") + + By("Deleting the custom resource") + persesToDelete := &persesv1alpha2.Perses{} + Expect(k8sClient.Get(ctx, typeNamespaceName, persesToDelete)).To(Succeed()) + Expect(k8sClient.Delete(ctx, persesToDelete)).To(Succeed()) + }) + }) }) diff --git a/docs/dev.md b/docs/dev.md index 836d3d5c..5f3f5179 100644 --- a/docs/dev.md +++ b/docs/dev.md @@ -14,7 +14,7 @@ This project follows the Kubernetes [Operator pattern](https://kubernetes.io/doc Each `Perses` CR instance manages the following resources: -* A ConfigMap holding the Perses server configuration +* A Secret holding the Perses server configuration (sensitive fields like database credentials and OAuth secrets are preserved) * A Deployment (SQL storage) or StatefulSet (file-based storage) running the Perses server * A Service for in-cluster API access diff --git a/internal/perses/common/volumes.go b/internal/perses/common/volumes.go index f1f192cf..86255ab3 100644 --- a/internal/perses/common/volumes.go +++ b/internal/perses/common/volumes.go @@ -35,10 +35,8 @@ func GetVolumes(perses *v1alpha2.Perses) []corev1.Volume { { Name: configVolumeName, VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: configName, - }, + Secret: &corev1.SecretVolumeSource{ + SecretName: configName, DefaultMode: ptr.To[int32](defaultFileMode), }, }, diff --git a/internal/perses/common/volumes_test.go b/internal/perses/common/volumes_test.go index 8acc1a7d..9632a5b1 100644 --- a/internal/perses/common/volumes_test.go +++ b/internal/perses/common/volumes_test.go @@ -37,6 +37,8 @@ var _ = Describe("GetVolumes", func() { func(volumes []corev1.Volume) { Expect(volumes).To(HaveLen(2)) Expect(volumes[0].Name).To(Equal(configVolumeName)) + Expect(volumes[0].VolumeSource.Secret).NotTo(BeNil()) + Expect(volumes[0].VolumeSource.Secret.SecretName).To(Equal("test-config")) Expect(volumes[1].Name).To(Equal(pluginsVolumeName)) Expect(volumes[1].VolumeSource.EmptyDir).NotTo(BeNil()) }, diff --git a/jsonnet/examples/role.yaml b/jsonnet/examples/role.yaml index 7bb468f9..32ff3ac1 100644 --- a/jsonnet/examples/role.yaml +++ b/jsonnet/examples/role.yaml @@ -23,6 +23,15 @@ rules: - patch - update - watch +- apiGroups: + - "" + resources: + - configmaps + verbs: + - delete + - get + - list + - watch - apiGroups: - "" resources: @@ -33,16 +42,15 @@ rules: - apiGroups: - "" resources: - - services - - configmaps - secrets + - services verbs: - - get - - patch - - update - create - delete + - get - list + - patch + - update - watch - apiGroups: - perses.dev diff --git a/test/e2e/global-datasource/00-assert.yaml b/test/e2e/global-datasource/00-assert.yaml index f938b465..45182160 100644 --- a/test/e2e/global-datasource/00-assert.yaml +++ b/test/e2e/global-datasource/00-assert.yaml @@ -12,7 +12,7 @@ status: reason: Reconciled --- apiVersion: v1 -kind: ConfigMap +kind: Secret metadata: name: perses-gds-test-config --- diff --git a/test/e2e/multi-instance-sync/00-assert.yaml b/test/e2e/multi-instance-sync/00-assert.yaml index 91fc9825..6591f5fe 100644 --- a/test/e2e/multi-instance-sync/00-assert.yaml +++ b/test/e2e/multi-instance-sync/00-assert.yaml @@ -12,7 +12,7 @@ status: reason: Reconciled --- apiVersion: v1 -kind: ConfigMap +kind: Secret metadata: name: perses-instance-1-config --- diff --git a/test/e2e/perses-deployment/00-assert.yaml b/test/e2e/perses-deployment/00-assert.yaml index 28e53003..76eb878b 100644 --- a/test/e2e/perses-deployment/00-assert.yaml +++ b/test/e2e/perses-deployment/00-assert.yaml @@ -12,7 +12,7 @@ status: reason: Reconciled --- apiVersion: v1 -kind: ConfigMap +kind: Secret metadata: name: perses-sql-test-config --- diff --git a/test/e2e/perses-emptydir/00-assert.yaml b/test/e2e/perses-emptydir/00-assert.yaml index 1bb7429a..3b9ee82c 100644 --- a/test/e2e/perses-emptydir/00-assert.yaml +++ b/test/e2e/perses-emptydir/00-assert.yaml @@ -12,7 +12,7 @@ status: reason: Reconciled --- apiVersion: v1 -kind: ConfigMap +kind: Secret metadata: name: perses-emptydir-test-config --- diff --git a/test/e2e/perses-statefulset/00-assert.yaml b/test/e2e/perses-statefulset/00-assert.yaml index 3e7e72c7..b8208bd0 100644 --- a/test/e2e/perses-statefulset/00-assert.yaml +++ b/test/e2e/perses-statefulset/00-assert.yaml @@ -12,7 +12,7 @@ status: reason: Reconciled --- apiVersion: v1 -kind: ConfigMap +kind: Secret metadata: name: perses-e2e-test-config --- diff --git a/test/e2e/perses-volumes/00-assert.yaml b/test/e2e/perses-volumes/00-assert.yaml index 7f896123..db2bfbe6 100644 --- a/test/e2e/perses-volumes/00-assert.yaml +++ b/test/e2e/perses-volumes/00-assert.yaml @@ -12,7 +12,7 @@ status: reason: Reconciled --- apiVersion: v1 -kind: ConfigMap +kind: Secret metadata: name: perses-volumes-test-config ---