Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Helm plugin] Changed repo secrets copied now to Kubeapps namespace #5183

Merged
merged 2 commits into from
Aug 9, 2022
Merged

[Helm plugin] Changed repo secrets copied now to Kubeapps namespace #5183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b651150
Changed repo secrets copied to Kubeapps namespace
Aug 9, 2022
69597cd
Added deletion of syncer secret
Aug 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/globals_vars_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ var (
},
ObjectMeta: metav1.ObjectMeta{
Name: "bar",
Namespace: "kubeapps",
Namespace: globalPackagingNamespace,
ResourceVersion: "1",
},
Spec: v1alpha1.AppRepositorySpec{
Expand All @@ -259,7 +259,7 @@ var (

addRepoReqGlobal = &corev1.AddPackageRepositoryRequest{
Name: "bar",
Context: &corev1.Context{Namespace: "kubeapps", Cluster: KubeappsCluster},
Context: &corev1.Context{Namespace: globalPackagingNamespace, Cluster: KubeappsCluster},
Type: "helm",
Url: "http://example.com",
NamespaceScoped: false,
Expand Down
26 changes: 20 additions & 6 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,11 @@ func (s *Server) newRepo(ctx context.Context, repo *HelmRepository) (*corev1.Pac
return nil, err
}
}
// Copy secret to global namespace if needed. See issue #5129.
if repo.name.Namespace != s.globalPackagingNamespace && secret != nil {
if err = s.copyRepositorySecret(typedClient, secret, repo.name); err != nil {
// Copy secret to the namespace of asset syncer if needed. See issue #5129.
if repo.name.Namespace != s.kubeappsNamespace && secret != nil {
// Target namespace must be the same as the asset syncer job,
// otherwise the job won't be able to access the secret
if _, err = s.copyRepositorySecretToNamespace(typedClient, s.kubeappsNamespace, secret, repo.name); err != nil {
return nil, err
}
}
Expand Down Expand Up @@ -327,9 +329,11 @@ func (s *Server) updateRepo(ctx context.Context, repo *HelmRepository) (*corev1.
return nil, err
}
}
// Copy secret to global namespace if needed. See issue #5129.
if repo.name.Namespace != s.globalPackagingNamespace && secret != nil {
if err = s.copyRepositorySecret(typedClient, secret, repo.name); err != nil {
// Copy secret to the namespace of asset syncer if needed. See issue #5129.
if repo.name.Namespace != s.kubeappsNamespace && secret != nil {
// Target namespace must be the same as the asset syncer job,
// otherwise the job won't be able to access the secret
if _, err = s.copyRepositorySecretToNamespace(typedClient, s.kubeappsNamespace, secret, repo.name); err != nil {
return nil, err
}
}
Expand Down Expand Up @@ -532,6 +536,16 @@ func (s *Server) deleteRepo(ctx context.Context, cluster string, repoRef *corev1
if err = client.Delete(ctx, repo); err != nil {
return statuserror.FromK8sError("delete", AppRepositoryKind, repoRef.Identifier, err)
} else {
// Cross-namespace owner references are disallowed by design.
// We need to explicitly delete the repo secret from the namespace of the asset syncer.
typedClient, err := s.clientGetter.Typed(ctx, cluster)
if err != nil {
return err
}
namespacedSecretName := namespacedSecretNameForRepo(repoRef.Identifier, repoRef.Context.Namespace)
if deleteErr := s.deleteRepositorySecretFromNamespace(typedClient, s.kubeappsNamespace, namespacedSecretName); deleteErr != nil {
return statuserror.FromK8sError("delete", "Secret", namespacedSecretName, deleteErr)
}
return nil
}
}
19 changes: 13 additions & 6 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,21 +286,28 @@ func deleteSecret(ctx context.Context, secretsInterface v1.SecretInterface, secr
return nil
}

func (s *Server) copyRepositorySecret(typedClient kubernetes.Interface, secret *k8scorev1.Secret, repoName types.NamespacedName) error {
targetNamespace := s.globalPackagingNamespace
globalSecret := &k8scorev1.Secret{
func (s *Server) copyRepositorySecretToNamespace(typedClient kubernetes.Interface, targetNamespace string, secret *k8scorev1.Secret, repoName types.NamespacedName) (copiedSecret *k8scorev1.Secret, err error) {
newSecret := &k8scorev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: namespacedSecretNameForRepo(repoName.Name, repoName.Namespace),
Namespace: targetNamespace,
},
Type: secret.Type,
Data: secret.Data,
}
_, err := typedClient.CoreV1().Secrets(targetNamespace).Create(context.TODO(), globalSecret, metav1.CreateOptions{})
copiedSecret, err = typedClient.CoreV1().Secrets(targetNamespace).Create(context.TODO(), newSecret, metav1.CreateOptions{})
if err != nil && k8sErrors.IsAlreadyExists(err) {
_, err = typedClient.CoreV1().Secrets(targetNamespace).Update(context.TODO(), globalSecret, metav1.UpdateOptions{})
copiedSecret, err = typedClient.CoreV1().Secrets(targetNamespace).Update(context.TODO(), newSecret, metav1.UpdateOptions{})
}
return err
return copiedSecret, err
}

func (s *Server) deleteRepositorySecretFromNamespace(typedClient kubernetes.Interface, targetNamespace, secretName string) error {
secretsInterface := typedClient.CoreV1().Secrets(targetNamespace)
if deleteErr := deleteSecret(context.TODO(), secretsInterface, secretName); deleteErr != nil {
return deleteErr
}
return nil
}

func updateKubeappsManagedImagesPullSecret(ctx context.Context, typedClient kubernetes.Interface,
Expand Down
110 changes: 76 additions & 34 deletions cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func TestAddPackageRepository(t *testing.T) {
expectedResponse: addRepoExpectedResp,
expectedRepo: &addRepoWithTLSCA,
expectedCreatedSecret: setSecretOwnerRef("bar", newTlsSecret("apprepo-bar", "foo", nil, nil, ca)),
expectedGlobalSecret: newTlsSecret("foo-apprepo-bar", globalPackagingNamespace, nil, nil, ca),
expectedGlobalSecret: newTlsSecret("foo-apprepo-bar", kubeappsNamespace, nil, nil, ca), // Global secrets must reside in the Kubeapps (asset syncer) namespace
statusCode: codes.OK,
},
{
Expand All @@ -276,7 +276,7 @@ func TestAddPackageRepository(t *testing.T) {
existingSecret: newTlsSecret("secret-1", "foo", nil, nil, ca),
expectedResponse: addRepoExpectedResp,
expectedRepo: &addRepoTLSSecret,
expectedGlobalSecret: newTlsSecret("foo-apprepo-bar", globalPackagingNamespace, nil, nil, ca),
expectedGlobalSecret: newTlsSecret("foo-apprepo-bar", kubeappsNamespace, nil, nil, ca),
statusCode: codes.OK,
},
{
Expand All @@ -297,14 +297,14 @@ func TestAddPackageRepository(t *testing.T) {
expectedResponse: addRepoExpectedResp,
expectedRepo: addRepoAuthHeaderPassCredentials("foo"),
expectedCreatedSecret: setSecretOwnerRef("bar", newBasicAuthSecret("apprepo-bar", "foo", "baz", "zot")),
expectedGlobalSecret: newBasicAuthSecret("foo-apprepo-bar", globalPackagingNamespace, "baz", "zot"),
expectedGlobalSecret: newBasicAuthSecret("foo-apprepo-bar", kubeappsNamespace, "baz", "zot"),
statusCode: codes.OK,
},
{
name: "[kubeapps managed secrets] package repository with basic auth and pass_credentials flag in global namespace",
name: "[kubeapps managed secrets] package repository with basic auth and pass_credentials flag in global namespace copies secret to kubeapps ns",
request: &corev1.AddPackageRepositoryRequest{
Name: "bar",
Context: &corev1.Context{Namespace: globalPackagingNamespace, Cluster: KubeappsCluster},
Context: &corev1.Context{Cluster: KubeappsCluster, Namespace: globalPackagingNamespace},
Type: "helm",
Url: "http://example.com",
NamespaceScoped: false,
Expand All @@ -322,6 +322,7 @@ func TestAddPackageRepository(t *testing.T) {
expectedResponse: addRepoExpectedGlobalResp,
expectedRepo: addRepoAuthHeaderPassCredentials(globalPackagingNamespace),
expectedCreatedSecret: setSecretOwnerRef("bar", newBasicAuthSecret("apprepo-bar", globalPackagingNamespace, "the-user", "the-pwd")),
expectedGlobalSecret: newBasicAuthSecret("kubeapps-repos-global-apprepo-bar", kubeappsNamespace, "the-user", "the-pwd"),
statusCode: codes.OK,
},
{
Expand All @@ -342,11 +343,11 @@ func TestAddPackageRepository(t *testing.T) {
existingSecret: newBasicAuthSecret("secret-basic", "foo", "baz-user", "zot-pwd"),
expectedResponse: addRepoExpectedResp,
expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "secret-basic"),
expectedGlobalSecret: newBasicAuthSecret("foo-apprepo-bar", globalPackagingNamespace, "baz-user", "zot-pwd"),
expectedGlobalSecret: newBasicAuthSecret("foo-apprepo-bar", kubeappsNamespace, "baz-user", "zot-pwd"),
statusCode: codes.OK,
},
{
name: "[user managed secrets] add repository to global namespace does not create global secret",
name: "[user managed secrets] add repository to global namespace creates secret in kubeapps namespace for syncer",
request: &corev1.AddPackageRepositoryRequest{
Name: "bar",
Context: &corev1.Context{Namespace: globalPackagingNamespace, Cluster: KubeappsCluster},
Expand All @@ -362,11 +363,12 @@ func TestAddPackageRepository(t *testing.T) {
},
},
},
userManagedSecrets: true,
existingSecret: newBasicAuthSecret("secret-basic", globalPackagingNamespace, "baz-user", "zot-pwd"),
expectedResponse: addRepoExpectedGlobalResp,
expectedRepo: addRepoAuthHeaderWithSecretRef(globalPackagingNamespace, "secret-basic"),
statusCode: codes.OK,
userManagedSecrets: true,
existingSecret: newBasicAuthSecret("secret-basic", globalPackagingNamespace, "baz-user", "zot-pwd"),
expectedResponse: addRepoExpectedGlobalResp,
expectedRepo: addRepoAuthHeaderWithSecretRef(globalPackagingNamespace, "secret-basic"),
expectedGlobalSecret: newBasicAuthSecret("kubeapps-repos-global-apprepo-bar", kubeappsNamespace, "baz-user", "zot-pwd"),
statusCode: codes.OK,
},
{
name: "package repository basic auth with existing secret (kubeapps managed secrets)",
Expand All @@ -380,7 +382,7 @@ func TestAddPackageRepository(t *testing.T) {
expectedResponse: addRepoExpectedResp,
expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"),
expectedCreatedSecret: setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "Bearer the-token")),
expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", globalPackagingNamespace, "Bearer the-token"),
expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"),
statusCode: codes.OK,
},
{
Expand All @@ -389,7 +391,7 @@ func TestAddPackageRepository(t *testing.T) {
expectedResponse: addRepoExpectedResp,
expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"),
expectedCreatedSecret: setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "Bearer the-token")),
expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", globalPackagingNamespace, "Bearer the-token"),
expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"),
statusCode: codes.OK,
},
{
Expand All @@ -404,7 +406,7 @@ func TestAddPackageRepository(t *testing.T) {
existingSecret: newAuthTokenSecret("secret-bearer", "foo", "Bearer the-token"),
expectedResponse: addRepoExpectedResp,
expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "secret-bearer"),
expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", globalPackagingNamespace, "Bearer the-token"),
expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"),
statusCode: codes.OK,
},
{
Expand All @@ -425,7 +427,7 @@ func TestAddPackageRepository(t *testing.T) {
expectedResponse: addRepoExpectedResp,
expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"),
expectedCreatedSecret: setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "foobarzot")),
expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", globalPackagingNamespace, "foobarzot"),
expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "foobarzot"),
statusCode: codes.OK,
},
{
Expand All @@ -435,7 +437,7 @@ func TestAddPackageRepository(t *testing.T) {
existingSecret: newBasicAuthSecret("secret-custom", "foo", "baz", "zot"),
expectedResponse: addRepoExpectedResp,
expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "secret-custom"),
expectedGlobalSecret: newBasicAuthSecret("foo-apprepo-bar", globalPackagingNamespace, "baz", "zot"),
expectedGlobalSecret: newBasicAuthSecret("foo-apprepo-bar", kubeappsNamespace, "baz", "zot"),
statusCode: codes.OK,
},
{
Expand All @@ -455,11 +457,12 @@ func TestAddPackageRepository(t *testing.T) {
},
},
},
userManagedSecrets: true,
existingSecret: newBasicAuthSecret("secret-custom", globalPackagingNamespace, "baz", "zot"),
expectedResponse: addRepoExpectedGlobalResp,
expectedRepo: addRepoAuthHeaderWithSecretRef(globalPackagingNamespace, "secret-custom"),
statusCode: codes.OK,
userManagedSecrets: true,
existingSecret: newBasicAuthSecret("secret-custom", globalPackagingNamespace, "baz", "zot"),
expectedResponse: addRepoExpectedGlobalResp,
expectedRepo: addRepoAuthHeaderWithSecretRef(globalPackagingNamespace, "secret-custom"),
expectedGlobalSecret: newBasicAuthSecret("kubeapps-repos-global-apprepo-bar", kubeappsNamespace, "baz", "zot"),
statusCode: codes.OK,
},
// DOCKER AUTH
{
Expand All @@ -475,7 +478,7 @@ func TestAddPackageRepository(t *testing.T) {
expectedCreatedSecret: setSecretOwnerRef("bar",
newAuthDockerSecret("apprepo-bar", "foo",
dockerAuthJson("https://docker-server", "the-user", "the-password", "foo@bar.com", "dGhlLXVzZXI6dGhlLXBhc3N3b3Jk"))),
expectedGlobalSecret: newAuthDockerSecret("foo-apprepo-bar", globalPackagingNamespace,
expectedGlobalSecret: newAuthDockerSecret("foo-apprepo-bar", kubeappsNamespace,
dockerAuthJson("https://docker-server", "the-user", "the-password", "foo@bar.com", "dGhlLXVzZXI6dGhlLXBhc3N3b3Jk")),
statusCode: codes.OK,
},
Expand All @@ -487,7 +490,7 @@ func TestAddPackageRepository(t *testing.T) {
dockerAuthJson("https://docker-server", "the-user", "the-password", "foo@bar.com", "dGhlLXVzZXI6dGhlLXBhc3N3b3Jk")),
expectedResponse: addRepoExpectedResp,
expectedRepo: addRepoAuthDocker("secret-docker"),
expectedGlobalSecret: newAuthDockerSecret("foo-apprepo-bar", globalPackagingNamespace,
expectedGlobalSecret: newAuthDockerSecret("foo-apprepo-bar", kubeappsNamespace,
dockerAuthJson("https://docker-server", "the-user", "the-password", "foo@bar.com", "dGhlLXVzZXI6dGhlLXBhc3N3b3Jk")),
statusCode: codes.OK,
},
Expand Down Expand Up @@ -1183,7 +1186,7 @@ func TestUpdatePackageRepository(t *testing.T) {
},
expectedRef: defaultRef,
expectedSecret: setSecretOwnerRef("repo-1", newTlsSecret("apprepo-repo-1", "ns-1", nil, nil, ca)),
expectedGlobalSecret: newTlsSecret("ns-1-apprepo-repo-1", globalPackagingNamespace, nil, nil, ca),
expectedGlobalSecret: newTlsSecret("ns-1-apprepo-repo-1", kubeappsNamespace, nil, nil, ca),
expectedStatusCode: codes.OK,
},
{
Expand Down Expand Up @@ -1242,7 +1245,7 @@ func TestUpdatePackageRepository(t *testing.T) {
},
expectedRef: defaultRef,
expectedSecret: setSecretOwnerRef("repo-1", newAuthTokenSecret("apprepo-repo-1", "ns-1", "Bearer foobarzot")),
expectedGlobalSecret: newAuthTokenSecret("ns-1-apprepo-repo-1", globalPackagingNamespace, "Bearer foobarzot"),
expectedGlobalSecret: newAuthTokenSecret("ns-1-apprepo-repo-1", kubeappsNamespace, "Bearer foobarzot"),
expectedStatusCode: codes.OK,
},
{
Expand Down Expand Up @@ -1275,7 +1278,7 @@ func TestUpdatePackageRepository(t *testing.T) {
return &repository
},
expectedRef: defaultRef,
expectedGlobalSecret: newAuthTokenSecret("ns-1-apprepo-repo-1", globalPackagingNamespace, "Bearer foobarzot"),
expectedGlobalSecret: newAuthTokenSecret("ns-1-apprepo-repo-1", kubeappsNamespace, "Bearer foobarzot"),
expectedStatusCode: codes.OK,
},
{
Expand Down Expand Up @@ -1568,10 +1571,11 @@ func TestDeletePackageRepository(t *testing.T) {
repos := []*appRepov1alpha1.AppRepository{repo1}

testCases := []struct {
name string
existingObjects []k8sruntime.Object
request *corev1.DeletePackageRepositoryRequest
expectedStatusCode codes.Code
name string
existingObjects []k8sruntime.Object
request *corev1.DeletePackageRepositoryRequest
expectedStatusCode codes.Code
expectedNonExistingSecrets []metav1.ObjectMeta
}{
{
name: "no context provided",
Expand Down Expand Up @@ -1605,6 +1609,27 @@ func TestDeletePackageRepository(t *testing.T) {
},
expectedStatusCode: codes.NotFound,
},
{
name: "delete - deletes associated secrets",
request: &corev1.DeletePackageRepositoryRequest{
PackageRepoRef: &corev1.PackageRepositoryReference{
Plugin: plugin,
Context: &corev1.Context{Namespace: "ns-1", Cluster: KubeappsCluster},
Identifier: "repo-1",
},
},
expectedStatusCode: codes.OK,
expectedNonExistingSecrets: []metav1.ObjectMeta{
{
Name: "apprepo-repo-1",
Namespace: "ns-1",
},
{
Name: "ns-1-apprepo-repo-1",
Namespace: kubeappsNamespace,
},
},
},
}

for _, tc := range testCases {
Expand All @@ -1625,6 +1650,23 @@ func TestDeletePackageRepository(t *testing.T) {
} else if got != codes.OK {
return
}

if tc.expectedNonExistingSecrets != nil {
ctx := context.Background()
typedClient, err := s.clientGetter.Typed(ctx, s.kubeappsCluster)
if err != nil {
t.Fatal(err)
}
for _, deletedSecret := range tc.expectedNonExistingSecrets {
secret, err := typedClient.CoreV1().Secrets(deletedSecret.Namespace).Get(ctx, deletedSecret.Name, metav1.GetOptions{})
if err != nil && !k8sErrors.IsNotFound(err) {
t.Fatal(err)
}
if secret != nil {
t.Fatalf("found existing secret '%s' in namespace '%s'", deletedSecret.Name, deletedSecret.Namespace)
}
}
}
})
}
}
Expand Down Expand Up @@ -1681,7 +1723,7 @@ func checkGlobalSecret(s *Server, t *testing.T, expectedRepo *appRepov1alpha1.Ap
repoGlobalSecretName := fmt.Sprintf("%s-apprepo-%s", expectedRepo.Namespace, expectedRepo.Name)
if expectedGlobalSecret != nil {
// Check for copied secret to global namespace
actualGlobalSecret, err := typedClient.CoreV1().Secrets(s.globalPackagingNamespace).Get(ctx, repoGlobalSecretName, metav1.GetOptions{})
actualGlobalSecret, err := typedClient.CoreV1().Secrets(s.kubeappsNamespace).Get(ctx, repoGlobalSecretName, metav1.GetOptions{})
if err != nil {
t.Fatal(err)
}
Expand All @@ -1690,12 +1732,12 @@ func checkGlobalSecret(s *Server, t *testing.T, expectedRepo *appRepov1alpha1.Ap
}
} else if checkNoGlobalSecret {
// Check that global secret does not exist
secret, err := typedClient.CoreV1().Secrets(s.globalPackagingNamespace).Get(ctx, repoGlobalSecretName, metav1.GetOptions{})
secret, err := typedClient.CoreV1().Secrets(s.kubeappsNamespace).Get(ctx, repoGlobalSecretName, metav1.GetOptions{})
if err != nil && !k8sErrors.IsNotFound(err) {
t.Fatal(err)
}
if secret != nil {
t.Errorf("global secret was found")
t.Errorf("global secret was found: %v", secret)
}
}
}
Expand Down