diff --git a/cmd/apprepository-controller/controller.go b/cmd/apprepository-controller/controller.go index 127b6951bfa..06e8135431d 100644 --- a/cmd/apprepository-controller/controller.go +++ b/cmd/apprepository-controller/controller.go @@ -25,6 +25,7 @@ import ( appreposcheme "github.com/kubeapps/kubeapps/cmd/apprepository-controller/pkg/client/clientset/versioned/scheme" informers "github.com/kubeapps/kubeapps/cmd/apprepository-controller/pkg/client/informers/externalversions" listers "github.com/kubeapps/kubeapps/cmd/apprepository-controller/pkg/client/listers/apprepository/v1alpha1" + "github.com/kubeapps/kubeapps/pkg/kube" log "github.com/sirupsen/logrus" batchv1 "k8s.io/api/batch/v1" batchv1beta1 "k8s.io/api/batch/v1beta1" @@ -426,7 +427,7 @@ func newCronJob(apprepo *apprepov1alpha1.AppRepository, kubeappsNamespace string // https://github.com/kubernetes/kubernetes/issues/54870 ConcurrencyPolicy: "Replace", JobTemplate: batchv1beta1.JobTemplateSpec{ - Spec: syncJobSpec(apprepo), + Spec: syncJobSpec(apprepo, kubeappsNamespace), }, }, } @@ -440,12 +441,12 @@ func newSyncJob(apprepo *apprepov1alpha1.AppRepository, kubeappsNamespace string GenerateName: cronJobName(apprepo) + "-", OwnerReferences: ownerReferencesForAppRepo(apprepo, kubeappsNamespace), }, - Spec: syncJobSpec(apprepo), + Spec: syncJobSpec(apprepo, kubeappsNamespace), } } // jobSpec returns a batchv1.JobSpec for running the chart-repo sync job -func syncJobSpec(apprepo *apprepov1alpha1.AppRepository) batchv1.JobSpec { +func syncJobSpec(apprepo *apprepov1alpha1.AppRepository, kubeappsNamespace string) batchv1.JobSpec { volumes := []corev1.Volume{} volumeMounts := []corev1.VolumeMount{} if apprepo.Spec.Auth.CustomCA != nil { @@ -453,7 +454,7 @@ func syncJobSpec(apprepo *apprepov1alpha1.AppRepository) batchv1.JobSpec { Name: apprepo.Spec.Auth.CustomCA.SecretKeyRef.Name, VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ - SecretName: apprepo.Spec.Auth.CustomCA.SecretKeyRef.Name, + SecretName: secretKeyRefForRepo(apprepo.Spec.Auth.CustomCA.SecretKeyRef, apprepo, kubeappsNamespace).Name, Items: []corev1.KeyToPath{ {Key: apprepo.Spec.Auth.CustomCA.SecretKeyRef.Key, Path: "ca.crt"}, }, @@ -487,7 +488,7 @@ func syncJobSpec(apprepo *apprepov1alpha1.AppRepository) batchv1.JobSpec { podTemplateSpec.Spec.Containers[0].Image = repoSyncImage podTemplateSpec.Spec.Containers[0].Command = []string{repoSyncCommand} podTemplateSpec.Spec.Containers[0].Args = apprepoSyncJobArgs(apprepo) - podTemplateSpec.Spec.Containers[0].Env = append(podTemplateSpec.Spec.Containers[0].Env, apprepoSyncJobEnvVars(apprepo)...) + podTemplateSpec.Spec.Containers[0].Env = append(podTemplateSpec.Spec.Containers[0].Env, apprepoSyncJobEnvVars(apprepo, kubeappsNamespace)...) podTemplateSpec.Spec.Containers[0].VolumeMounts = append(podTemplateSpec.Spec.Containers[0].VolumeMounts, volumeMounts...) // Add volumes podTemplateSpec.Spec.Volumes = append(podTemplateSpec.Spec.Volumes, volumes...) @@ -570,7 +571,7 @@ func apprepoSyncJobArgs(apprepo *apprepov1alpha1.AppRepository) []string { } // apprepoSyncJobEnvVars returns a list of env variables for the sync container -func apprepoSyncJobEnvVars(apprepo *apprepov1alpha1.AppRepository) []corev1.EnvVar { +func apprepoSyncJobEnvVars(apprepo *apprepov1alpha1.AppRepository, kubeappsNamespace string) []corev1.EnvVar { var envVars []corev1.EnvVar envVars = append(envVars, corev1.EnvVar{ Name: "DB_PASSWORD", @@ -585,13 +586,25 @@ func apprepoSyncJobEnvVars(apprepo *apprepov1alpha1.AppRepository) []corev1.EnvV envVars = append(envVars, corev1.EnvVar{ Name: "AUTHORIZATION_HEADER", ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &apprepo.Spec.Auth.Header.SecretKeyRef, + SecretKeyRef: secretKeyRefForRepo(apprepo.Spec.Auth.Header.SecretKeyRef, apprepo, kubeappsNamespace), }, }) } return envVars } +// secretKeyRefForRepo returns a secret key ref with a name depending on whether +// this repo is in the kubeapps namespace or not. If the repo is not in the +// kubeapps namespace, then the secret will have been copied from another namespace +// into the kubeapps namespace and have a slightly different name. +func secretKeyRefForRepo(keyRef corev1.SecretKeySelector, apprepo *apprepov1alpha1.AppRepository, kubeappsNamespace string) *corev1.SecretKeySelector { + if apprepo.ObjectMeta.Namespace == kubeappsNamespace { + return &keyRef + } + keyRef.LocalObjectReference.Name = kube.KubeappsSecretNameForRepo(apprepo.ObjectMeta.Name, apprepo.ObjectMeta.Namespace) + return &keyRef +} + // apprepoCleanupJobArgs returns a list of args for the repo cleanup container func apprepoCleanupJobArgs(repoName string) []string { return append([]string{ diff --git a/cmd/apprepository-controller/controller_test.go b/cmd/apprepository-controller/controller_test.go index 5b6a634b0c0..b2c5f805b4b 100644 --- a/cmd/apprepository-controller/controller_test.go +++ b/cmd/apprepository-controller/controller_test.go @@ -205,6 +205,92 @@ func Test_newCronJob(t *testing.T) { "kubeapps/v2.3", "*/20 * * * *", }, + { + "a cronjob for an app repo in another namespace references the repo secret in kubeapps", + &apprepov1alpha1.AppRepository{ + TypeMeta: metav1.TypeMeta{ + Kind: "AppRepository", + APIVersion: "kubeapps.com/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-charts-in-otherns", + Namespace: "otherns", + Labels: map[string]string{ + "name": "my-charts", + "created-by": "kubeapps", + }, + }, + Spec: apprepov1alpha1.AppRepositorySpec{ + Type: "helm", + URL: "https://charts.acme.com/my-charts", + Auth: apprepov1alpha1.AppRepositoryAuth{ + Header: &apprepov1alpha1.AppRepositoryAuthHeader{ + SecretKeyRef: corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "apprepo-my-charts-in-otherns"}, Key: "AuthorizationHeader"}}, + }, + }, + }, + batchv1beta1.CronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: "apprepo-otherns-sync-my-charts-in-otherns", + Labels: map[string]string{ + LabelRepoName: "my-charts-in-otherns", + LabelRepoNamespace: "otherns", + }, + }, + Spec: batchv1beta1.CronJobSpec{ + Schedule: "*/20 * * * *", + ConcurrencyPolicy: "Replace", + JobTemplate: batchv1beta1.JobTemplateSpec{ + Spec: batchv1.JobSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + LabelRepoName: "my-charts-in-otherns", + LabelRepoNamespace: "otherns", + }, + }, + Spec: corev1.PodSpec{ + RestartPolicy: "OnFailure", + Containers: []corev1.Container{ + { + Name: "sync", + Image: repoSyncImage, + Command: []string{"/chart-repo"}, + Args: []string{ + "sync", + "--database-type=mongodb", + "--database-url=mongodb.kubeapps", + "--database-user=admin", + "--database-name=assets", + "--user-agent-comment=kubeapps/v2.3", + "my-charts-in-otherns", + "https://charts.acme.com/my-charts", + }, + Env: []corev1.EnvVar{ + { + Name: "DB_PASSWORD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "mongodb"}, Key: "mongodb-root-password"}}, + }, + { + Name: "AUTHORIZATION_HEADER", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{LocalObjectReference: corev1.LocalObjectReference{Name: "otherns-apprepo-my-charts-in-otherns"}, Key: "AuthorizationHeader"}}, + }, + }, + VolumeMounts: nil, + }, + }, + Volumes: nil, + }, + }, + }, + }, + }, + }, + "kubeapps/v2.3", + "*/20 * * * *", + }, } for _, tt := range tests { diff --git a/pkg/kube/kube_handler.go b/pkg/kube/kube_handler.go index 0f525499037..aa0d14b6cf9 100644 --- a/pkg/kube/kube_handler.go +++ b/pkg/kube/kube_handler.go @@ -246,7 +246,7 @@ func (a *userHandler) CreateAppRepository(appRepoBody io.ReadCloser, requestName // See the relevant section of the design doc for details: // https://docs.google.com/document/d/1YEeKC6nPLoq4oaxs9v8_UsmxrRfWxB6KCyqrh2-Q8x0/edit?ts=5e2adf87#heading=h.kilvd2vii0w if requestNamespace != a.kubeappsNamespace { - repoSecret.ObjectMeta.Name = kubeappsSecretNameForRepo(appRepo.ObjectMeta.Name, appRepo.ObjectMeta.Namespace) + repoSecret.ObjectMeta.Name = KubeappsSecretNameForRepo(appRepo.ObjectMeta.Name, appRepo.ObjectMeta.Namespace) repoSecret.ObjectMeta.OwnerReferences = nil _, err = a.svcClientset.CoreV1().Secrets(a.kubeappsNamespace).Create(repoSecret) if err != nil { @@ -264,10 +264,7 @@ func (a *userHandler) DeleteAppRepository(repoName, repoNamespace string) error return err } hasCredentials := appRepo.Spec.Auth.Header != nil || appRepo.Spec.Auth.CustomCA != nil - var propagationPolicy metav1.DeletionPropagation = "Foreground" - err = a.clientset.KubeappsV1alpha1().AppRepositories(repoNamespace).Delete(repoName, &metav1.DeleteOptions{ - PropagationPolicy: &propagationPolicy, - }) + err = a.clientset.KubeappsV1alpha1().AppRepositories(repoNamespace).Delete(repoName, &metav1.DeleteOptions{}) if err != nil { return err } @@ -276,7 +273,7 @@ func (a *userHandler) DeleteAppRepository(repoName, repoNamespace string) error // the repository credentials kept in the kubeapps namespace (the repo credentials in the actual // namespace should be deleted when the owning app repo is deleted). if hasCredentials && repoNamespace != a.kubeappsNamespace { - err = a.clientset.CoreV1().Secrets(a.kubeappsNamespace).Delete(kubeappsSecretNameForRepo(repoName, repoNamespace), &metav1.DeleteOptions{}) + err = a.clientset.CoreV1().Secrets(a.kubeappsNamespace).Delete(KubeappsSecretNameForRepo(repoName, repoNamespace), &metav1.DeleteOptions{}) } return err } @@ -366,9 +363,9 @@ func secretNameForRepo(repoName string) string { return fmt.Sprintf("apprepo-%s", repoName) } -// kubeappsSecretNameForRepo returns a name suitable for recording a copy of +// KubeappsSecretNameForRepo returns a name suitable for recording a copy of // a per-namespace repository secret in the kubeapps namespace. -func kubeappsSecretNameForRepo(repoName, namespace string) string { +func KubeappsSecretNameForRepo(repoName, namespace string) string { return fmt.Sprintf("%s-%s", namespace, secretNameForRepo(repoName)) } diff --git a/pkg/kube/kube_handler_test.go b/pkg/kube/kube_handler_test.go index bdf63dfc097..362591e0832 100644 --- a/pkg/kube/kube_handler_test.go +++ b/pkg/kube/kube_handler_test.go @@ -85,7 +85,7 @@ func makeSecretsForRepos(reposPerNamespace map[string][]repoStub, kubeappsNamesp if namespace != kubeappsNamespace { var appRepo runtime.Object = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: kubeappsSecretNameForRepo(repoStub.name, namespace), + Name: KubeappsSecretNameForRepo(repoStub.name, namespace), Namespace: kubeappsNamespace, }, } @@ -236,7 +236,7 @@ func TestAppRepositoryCreate(t *testing.T) { // Verify the copy of the repo secret in in kubeapps is // also stored if this is a per-namespace app repository. - kubeappsSecretName := kubeappsSecretNameForRepo(expectedAppRepo.ObjectMeta.Name, expectedAppRepo.ObjectMeta.Namespace) + kubeappsSecretName := KubeappsSecretNameForRepo(expectedAppRepo.ObjectMeta.Name, expectedAppRepo.ObjectMeta.Namespace) expectedSecret.ObjectMeta.Name = kubeappsSecretName expectedSecret.ObjectMeta.Namespace = tc.kubeappsNamespace // The owner ref cannot be present for the copy in the kubeapps namespace. @@ -332,7 +332,7 @@ func TestDeleteAppRepository(t *testing.T) { // because the fake client does not handle finalizers but verified in real life. // Ensure any copy of the repo credentials has been deleted from the kubeapps namespace. - _, err = cs.CoreV1().Secrets(kubeappsNamespace).Get(kubeappsSecretNameForRepo(tc.repoName, tc.requestNamespace), metav1.GetOptions{}) + _, err = cs.CoreV1().Secrets(kubeappsNamespace).Get(KubeappsSecretNameForRepo(tc.repoName, tc.requestNamespace), metav1.GetOptions{}) if got, want := errorCodeForK8sError(t, err), 404; got != want { t.Errorf("got: %d, want: %d", got, want) }