diff --git a/controllers/workspace/devworkspace_controller.go b/controllers/workspace/devworkspace_controller.go index 7c280b15a..6ac3b99ce 100644 --- a/controllers/workspace/devworkspace_controller.go +++ b/controllers/workspace/devworkspace_controller.go @@ -700,69 +700,6 @@ func (r *DevWorkspaceReconciler) getWorkspaceId(ctx context.Context, workspace * } } -// Mapping the pod to the devworkspace -func dwRelatedPodsHandler(obj client.Object) []reconcile.Request { - labels := obj.GetLabels() - if _, ok := labels[constants.DevWorkspaceNameLabel]; !ok { - return []reconcile.Request{} - } - - //If the dewworkspace label does not exist, do no reconcile - if _, ok := labels[constants.DevWorkspaceIDLabel]; !ok { - return []reconcile.Request{} - } - - return []reconcile.Request{ - { - NamespacedName: types.NamespacedName{ - Name: labels[constants.DevWorkspaceNameLabel], - Namespace: obj.GetNamespace(), - }, - }, - } -} - -func (r *DevWorkspaceReconciler) dwPVCHandler(obj client.Object) []reconcile.Request { - // Check if PVC is owned by a DevWorkspace (per-workspace storage case) - for _, ownerref := range obj.GetOwnerReferences() { - if ownerref.Kind != "DevWorkspace" { - continue - } - return []reconcile.Request{ - { - NamespacedName: types.NamespacedName{ - Name: ownerref.Name, - Namespace: obj.GetNamespace(), - }, - }, - } - } - - // TODO: Label PVCs used for workspace storage so that they can be cleaned up if non-default name is used. - // Otherwise, check if common PVC is deleted to make sure all DevWorkspaces see it happen - if obj.GetName() != wkspConfig.GetGlobalConfig().Workspace.PVCName || obj.GetDeletionTimestamp() == nil { - // We're looking for a deleted common PVC - return []reconcile.Request{} - } - dwList := &dw.DevWorkspaceList{} - if err := r.Client.List(context.Background(), dwList, &client.ListOptions{Namespace: obj.GetNamespace()}); err != nil { - return []reconcile.Request{} - } - var reconciles []reconcile.Request - for _, workspace := range dwList.Items { - storageType := workspace.Spec.Template.Attributes.GetString(constants.DevWorkspaceStorageTypeAttribute, nil) - if storageType == constants.CommonStorageClassType || storageType == constants.PerUserStorageClassType || storageType == "" { - reconciles = append(reconciles, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: workspace.GetName(), - Namespace: workspace.GetNamespace(), - }, - }) - } - } - return reconciles -} - func (r *DevWorkspaceReconciler) SetupWithManager(mgr ctrl.Manager) error { setupHttpClients() @@ -775,7 +712,8 @@ func (r *DevWorkspaceReconciler) SetupWithManager(mgr ctrl.Manager) error { return []reconcile.Request{} } - var configWatcher builder.WatchesOption = builder.WithPredicates(wkspConfig.Predicates()) + configWatcher := builder.WithPredicates(wkspConfig.Predicates()) + automountWatcher := builder.WithPredicates(automountPredicates) // TODO: Set up indexing https://book.kubebuilder.io/cronjob-tutorial/controller-implementation.html#setup return ctrl.NewControllerManagedBy(mgr). @@ -793,8 +731,11 @@ func (r *DevWorkspaceReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&corev1.ServiceAccount{}). Watches(&source.Kind{Type: &corev1.Pod{}}, handler.EnqueueRequestsFromMapFunc(dwRelatedPodsHandler)). Watches(&source.Kind{Type: &corev1.PersistentVolumeClaim{}}, handler.EnqueueRequestsFromMapFunc(r.dwPVCHandler)). + Watches(&source.Kind{Type: &corev1.Secret{}}, handler.EnqueueRequestsFromMapFunc(r.runningWorkspacesHandler), automountWatcher). + Watches(&source.Kind{Type: &corev1.ConfigMap{}}, handler.EnqueueRequestsFromMapFunc(r.runningWorkspacesHandler), automountWatcher). + Watches(&source.Kind{Type: &corev1.PersistentVolumeClaim{}}, handler.EnqueueRequestsFromMapFunc(r.runningWorkspacesHandler), automountWatcher). Watches(&source.Kind{Type: &controllerv1alpha1.DevWorkspaceOperatorConfig{}}, handler.EnqueueRequestsFromMapFunc(emptyMapper), configWatcher). - WithEventFilter(predicates). + WithEventFilter(devworkspacePredicates). WithEventFilter(podPredicates). Complete(r) } diff --git a/controllers/workspace/devworkspace_controller_test.go b/controllers/workspace/devworkspace_controller_test.go index 331cd1c0a..6b072ed39 100644 --- a/controllers/workspace/devworkspace_controller_test.go +++ b/controllers/workspace/devworkspace_controller_test.go @@ -353,7 +353,6 @@ var _ = Describe("DevWorkspace Controller", func() { }, }, }) - createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") }) AfterEach(func() { @@ -362,6 +361,7 @@ var _ = Describe("DevWorkspace Controller", func() { }) It("Mounts image pull secrets to the DevWorkspace Deployment", func() { + createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") devworkspace := getExistingDevWorkspace(devWorkspaceName) workspaceID := devworkspace.Status.DevWorkspaceId @@ -394,6 +394,7 @@ var _ = Describe("DevWorkspace Controller", func() { }) It("Manages git credentials for DevWorkspace", func() { + createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") devworkspace := getExistingDevWorkspace(devWorkspaceName) workspaceID := devworkspace.Status.DevWorkspaceId @@ -456,6 +457,7 @@ var _ = Describe("DevWorkspace Controller", func() { }) It("Automounts secrets and configmaps volumes", func() { + createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") devworkspace := getExistingDevWorkspace(devWorkspaceName) workspaceID := devworkspace.Status.DevWorkspaceId @@ -518,6 +520,7 @@ var _ = Describe("DevWorkspace Controller", func() { }) It("Automounts secrets and configmaps env vars", func() { + createDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") devworkspace := getExistingDevWorkspace(devWorkspaceName) workspaceID := devworkspace.Status.DevWorkspaceId @@ -558,6 +561,45 @@ var _ = Describe("DevWorkspace Controller", func() { Expect(container.EnvFrom).Should(ContainElements(expectedEnvFromSources), "Automounted env sources should be added to containers") } }) + + It("Detects changes to automount resources and reconciles", func() { + // NOTE: timeout for this test is reduced, as eventually DWO will reconcile the workspace by coincidence and notice + // the automount secret. + createStartedDevWorkspace(devWorkspaceName, "test-devworkspace.yaml") + devworkspace := getExistingDevWorkspace(devWorkspaceName) + workspaceID := devworkspace.Status.DevWorkspaceId + + mergedSecretNN := namespacedName(constants.GitCredentialsMergedSecretName, testNamespace) + mergedSecret := &corev1.Secret{} + Expect(k8sClient.Get(ctx, mergedSecretNN, mergedSecret)).Error() + + By("Creating git-credential secret") + secret := generateSecret("git-credential-secret", corev1.SecretTypeOpaque) + secret.Labels[constants.DevWorkspaceGitCredentialLabel] = "true" + secret.Data["credentials"] = []byte("https://test:token@github.com") + createObject(secret) + defer deleteObject(secret) + + By("Checking that merged credentials secret is created") + Eventually(func() error { + return k8sClient.Get(ctx, mergedSecretNN, mergedSecret) + }, 1*time.Second, interval).Should(Succeed(), "Merged credentials secret is created") + + By("Checking that workspace deployment mounts merged credentials secret") + Eventually(func() error { + deploy := &appsv1.Deployment{} + deployNN := namespacedName(common.DeploymentName(workspaceID), testNamespace) + if err := k8sClient.Get(ctx, deployNN, deploy); err != nil { + return err + } + for _, volume := range deploy.Spec.Template.Spec.Volumes { + if volume.Secret != nil && volume.Secret.SecretName == constants.GitCredentialsMergedSecretName { + return nil + } + } + return fmt.Errorf("Secret not found in volumes") + }, 1*time.Second, interval).Should(Succeed(), "Merged credentials secret is added to deployment") + }) }) Context("Stopping DevWorkspaces", func() { diff --git a/controllers/workspace/eventhandlers.go b/controllers/workspace/eventhandlers.go new file mode 100644 index 000000000..74a73313c --- /dev/null +++ b/controllers/workspace/eventhandlers.go @@ -0,0 +1,108 @@ +// Copyright (c) 2019-2023 Red Hat, Inc. +// 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, +// 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. + +package controllers + +import ( + "context" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + wkspConfig "github.com/devfile/devworkspace-operator/pkg/config" + "github.com/devfile/devworkspace-operator/pkg/constants" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// Mapping the pod to the devworkspace +func dwRelatedPodsHandler(obj client.Object) []reconcile.Request { + labels := obj.GetLabels() + if _, ok := labels[constants.DevWorkspaceNameLabel]; !ok { + return []reconcile.Request{} + } + + //If the dewworkspace label does not exist, do no reconcile + if _, ok := labels[constants.DevWorkspaceIDLabel]; !ok { + return []reconcile.Request{} + } + + return []reconcile.Request{ + { + NamespacedName: types.NamespacedName{ + Name: labels[constants.DevWorkspaceNameLabel], + Namespace: obj.GetNamespace(), + }, + }, + } +} + +func (r *DevWorkspaceReconciler) dwPVCHandler(obj client.Object) []reconcile.Request { + // Check if PVC is owned by a DevWorkspace (per-workspace storage case) + for _, ownerref := range obj.GetOwnerReferences() { + if ownerref.Kind != "DevWorkspace" { + continue + } + return []reconcile.Request{ + { + NamespacedName: types.NamespacedName{ + Name: ownerref.Name, + Namespace: obj.GetNamespace(), + }, + }, + } + } + + // TODO: Label PVCs used for workspace storage so that they can be cleaned up if non-default name is used. + // Otherwise, check if common PVC is deleted to make sure all DevWorkspaces see it happen + if obj.GetName() != wkspConfig.GetGlobalConfig().Workspace.PVCName || obj.GetDeletionTimestamp() == nil { + // We're looking for a deleted common PVC + return []reconcile.Request{} + } + dwList := &dw.DevWorkspaceList{} + if err := r.Client.List(context.Background(), dwList, &client.ListOptions{Namespace: obj.GetNamespace()}); err != nil { + return []reconcile.Request{} + } + var reconciles []reconcile.Request + for _, workspace := range dwList.Items { + storageType := workspace.Spec.Template.Attributes.GetString(constants.DevWorkspaceStorageTypeAttribute, nil) + if storageType == constants.CommonStorageClassType || storageType == constants.PerUserStorageClassType || storageType == "" { + reconciles = append(reconciles, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: workspace.GetName(), + Namespace: workspace.GetNamespace(), + }, + }) + } + } + return reconciles +} + +func (r *DevWorkspaceReconciler) runningWorkspacesHandler(obj client.Object) []reconcile.Request { + dwList := &dw.DevWorkspaceList{} + if err := r.Client.List(context.Background(), dwList, &client.ListOptions{Namespace: obj.GetNamespace()}); err != nil { + return []reconcile.Request{} + } + var reconciles []reconcile.Request + for _, workspace := range dwList.Items { + // Queue reconciles for any started workspaces to make sure they pick up new object + if workspace.Spec.Started { + reconciles = append(reconciles, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: workspace.GetName(), + Namespace: workspace.GetNamespace(), + }, + }) + } + } + return reconciles +} diff --git a/controllers/workspace/predicates.go b/controllers/workspace/predicates.go index 22d923c26..d81f7234e 100644 --- a/controllers/workspace/predicates.go +++ b/controllers/workspace/predicates.go @@ -21,14 +21,15 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" ) -// predicates filters incoming events to avoid unnecessary reconciles to failed workspaces. +// devworkspacePredicates filters incoming events to avoid unnecessary reconciles to failed workspaces. // If a workspace failed and its spec is changed, we trigger reconciles to allow for fixing // issues in the workspace spec. -var predicates = predicate.Funcs{ +var devworkspacePredicates = predicate.Funcs{ CreateFunc: func(_ event.CreateEvent) bool { return true }, DeleteFunc: func(_ event.DeleteEvent) bool { return true }, UpdateFunc: func(ev event.UpdateEvent) bool { @@ -84,3 +85,30 @@ var podPredicates = predicate.Funcs{ return true }, } + +var automountPredicates = predicate.Funcs{ + CreateFunc: func(ev event.CreateEvent) bool { + return objectIsAutomountResource(ev.Object) + }, + DeleteFunc: func(ev event.DeleteEvent) bool { + return objectIsAutomountResource(ev.Object) + }, + UpdateFunc: func(ev event.UpdateEvent) bool { + return objectIsAutomountResource(ev.ObjectNew) + }, + GenericFunc: func(_ event.GenericEvent) bool { return false }, +} + +func objectIsAutomountResource(obj client.Object) bool { + labels := obj.GetLabels() + switch { + case labels[constants.DevWorkspaceMountLabel] == "true", + labels[constants.DevWorkspaceGitCredentialLabel] == "true", + labels[constants.DevWorkspaceGitTLSLabel] == "true", + labels[constants.DevWorkspacePullSecretLabel] == "true": + return true + default: + return false + } + +} diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index 162491609..0d72b15bd 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -58,6 +58,15 @@ const ( // If the git host is not defined then the certificate will be used for all http repositories. DevWorkspaceGitTLSLabel = "controller.devfile.io/git-tls-credential" + // GitCredentialsConfigMapName is the name used for the configmap that stores the Git configuration for workspaces + // in a given namespace. It is used when e.g. adding Git credentials via secret + GitCredentialsConfigMapName = "devworkspace-gitconfig" + + // GitCredentialsMergedSecretName is the name for the merged Git credentials secret that is mounted to workspaces + // when Git credentials are defined. This secret combines the values of any secrets labelled + // "controller.devfile.io/git-credential" + GitCredentialsMergedSecretName = "devworkspace-merged-git-credentials" + // DevWorkspaceMountPathAnnotation is the annotation key to store the mount path for the secret or configmap. // If no mount path is provided, configmaps will be mounted at /etc/config/, secrets will // be mounted at /etc/secret/, and persistent volume claims will be mounted to /tmp/ diff --git a/pkg/provision/automount/gitconfig.go b/pkg/provision/automount/gitconfig.go index 85b8ff036..e738ac55a 100644 --- a/pkg/provision/automount/gitconfig.go +++ b/pkg/provision/automount/gitconfig.go @@ -116,7 +116,7 @@ func getGitResources(api sync.ClusterAPI, namespace string) (credentialSecrets [ func cleanupGitConfig(api sync.ClusterAPI, namespace string) error { secretNN := types.NamespacedName{ - Name: gitCredentialsSecretName, + Name: constants.GitCredentialsMergedSecretName, Namespace: namespace, } tlsSecret := &corev1.Secret{} @@ -134,7 +134,7 @@ func cleanupGitConfig(api sync.ClusterAPI, namespace string) error { } configmapNN := types.NamespacedName{ - Name: gitCredentialsConfigMapName, + Name: constants.GitCredentialsConfigMapName, Namespace: namespace, } credentialsConfigMap := &corev1.ConfigMap{} diff --git a/pkg/provision/automount/templates.go b/pkg/provision/automount/templates.go index d490dc159..e15ba131c 100644 --- a/pkg/provision/automount/templates.go +++ b/pkg/provision/automount/templates.go @@ -28,10 +28,8 @@ const gitTLSCertificateKey = "certificate" const gitConfigName = "gitconfig" const gitConfigLocation = "/etc/" + gitConfigName -const gitCredentialsConfigMapName = "devworkspace-gitconfig" const gitCredentialsSecretKey = "credentials" -const gitCredentialsSecretName = "devworkspace-merged-git-credentials" // gitLFSConfig is the default configuration that gets provisioned when git-lfs // is installed. It needs to be included in the overridden gitconfig to avoid @@ -95,7 +93,7 @@ func constructGitConfig(namespace, credentialMountPath string, certificatesConfi gitConfigMap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: gitCredentialsConfigMapName, + Name: constants.GitCredentialsConfigMapName, Namespace: namespace, Labels: map[string]string{ "app.kubernetes.io/defaultName": "git-config-secret", @@ -122,7 +120,7 @@ func mergeGitCredentials(namespace string, credentialSecrets []corev1.Secret) (* } mergedCredentials := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: gitCredentialsSecretName, + Name: constants.GitCredentialsMergedSecretName, Namespace: namespace, Labels: map[string]string{ "app.kubernetes.io/defaultName": "git-config-secret",