Skip to content

Commit

Permalink
Use correct secret in kubeapps namespace when syncing an app repo (#1525
Browse files Browse the repository at this point in the history 
)

* Use correct secret in kubeapps namespace when syncing an app repo from another namespace.

* Improve test name

* Update secret key ref for volume.
  • Loading branch information
@absoludity
absoludity authored Feb 19, 2020
1 parent 4ab27c4 commit b772c3b
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 18 deletions.
27 changes: 20 additions & 7 deletions cmd/apprepository-controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
},
},
}
Expand All @@ -440,20 +441,20 @@ 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 {
volumes = append(volumes, corev1.Volume{
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"},
},
Expand Down Expand Up @@ -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...)
Expand Down Expand Up @@ -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",
Expand All @@ -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{
Expand Down
86 changes: 86 additions & 0 deletions cmd/apprepository-controller/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 5 additions & 8 deletions pkg/kube/kube_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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))
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/kube/kube_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit b772c3b

Please sign in to comment.