From 75a419a2f877985949f7fa7b1a2a6bc3deee1ec0 Mon Sep 17 00:00:00 2001 From: Dimitri Laloue Date: Wed, 30 Nov 2022 13:09:03 -0800 Subject: [PATCH 1/8] fixes --- .../packages/v1alpha1/repositories_auth.go | 381 +++++++++++----- .../packages/v1alpha1/repositories_test.go | 409 +++++++++++++----- .../helm/packages/v1alpha1/test_util_test.go | 12 +- 3 files changed, 589 insertions(+), 213 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_auth.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_auth.go index a865a166c0c..4c0fe51caa0 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_auth.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_auth.go @@ -69,7 +69,7 @@ func handleAuthSecretForCreate( secret, err := validateUserManagedRepoSecret(ctx, typedClient, repoName.Namespace, tlsConfig, auth) return secret, false, err } else if hasCaData || hasAuthData { - secret, _, err := newSecretFromTlsConfigAndAuth(repoName.Name, tlsConfig, auth) + secret, _, err := newSecretFromTlsConfigAndAuth(repoName.Name, nil, tlsConfig, auth) return secret, true, err } else { return nil, false, nil @@ -90,7 +90,7 @@ func handleImagesPullSecretForCreate( secret, err := validateDockerImagePullSecret(ctx, typedClient, repoName.Namespace, customDetail.ImagesPullSecret.GetSecretRef()) return secret, false, err } else if hasData { - secret, _, err := newDockerImagePullSecret(repoName.Name, customDetail.ImagesPullSecret.GetCredentials()) + secret, _, err := newDockerImagePullSecret(repoName.Name, nil, customDetail.ImagesPullSecret.GetCredentials()) return secret, true, err } else { return nil, false, nil @@ -122,29 +122,40 @@ func handleAuthSecretForUpdate( } } - // handle user managed secret + // create/get secret if hasCaRef || hasAuthRef { + // handle user managed secret updatedSecret, err := validateUserManagedRepoSecret(ctx, typedClient, repo.GetNamespace(), tlsConfig, auth) return updatedSecret, false, true, err - } - // handle kubeapps managed secret - updatedSecret, isSameSecret, err := newSecretFromTlsConfigAndAuth(repo.GetName(), tlsConfig, auth) - if err != nil { - return nil, true, false, err - } else if isSameSecret { - // Do nothing if repo auth data came redacted - return nil, true, false, nil + } else if hasCaData || hasAuthData { + // handle kubeapps managed secret + updatedSecret, isSameSecret, err := newSecretFromTlsConfigAndAuth(repo.GetName(), secret, tlsConfig, auth) + if err != nil { + return nil, true, false, err + } else if isSameSecret { + // Do nothing if repo auth data came fully redacted + return nil, true, false, nil + } else { + // secret has changed, we try to delete any existing secret + if secret != nil { + secretsInterface := typedClient.CoreV1().Secrets(secret.GetNamespace()) + if err := deleteSecret(ctx, secretsInterface, secret.GetName()); err != nil { + log.Errorf("Error deleting existing secret: [%s] due to %v", err) + } + } + return updatedSecret, true, true, nil + } + } else { - // either we have no secret, or it has changed. in both cases, we try to delete any existing secret + // no auth, delete existing secret if necessary if secret != nil { secretsInterface := typedClient.CoreV1().Secrets(secret.GetNamespace()) if err := deleteSecret(ctx, secretsInterface, secret.GetName()); err != nil { log.Errorf("Error deleting existing secret: [%s] due to %v", err) } } - - return updatedSecret, true, true, nil + return nil, false, true, nil } } @@ -172,120 +183,190 @@ func handleImagesPullSecretForUpdate( } } - // handle user managed secret + // create/get secret if hasRef { + // handle user managed secret updatedSecret, err := validateDockerImagePullSecret(ctx, typedClient, repo.GetNamespace(), imagesPullSecrets.GetSecretRef()) return updatedSecret, false, true, err - } - // handle kubeapps managed secret - updatedSecret, isSameSecret, err := newDockerImagePullSecret(repo.GetName(), imagesPullSecrets.GetCredentials()) - if err != nil { - return nil, true, false, err - } else if isSameSecret { - // Do nothing if repo credential data came redacted - return nil, true, false, nil + } else if hasData { + // handle kubeapps managed secret + updatedSecret, isSameSecret, err := newDockerImagePullSecret(repo.GetName(), secret, imagesPullSecrets.GetCredentials()) + if err != nil { + return nil, true, false, err + } else if isSameSecret { + // Do nothing if repo auth data came fully redacted + return nil, true, false, nil + } else { + // secret has changed, we try to delete any existing secret + if secret != nil { + secretsInterface := typedClient.CoreV1().Secrets(secret.GetNamespace()) + if err := deleteSecret(ctx, secretsInterface, secret.GetName()); err != nil { + log.Errorf("Error deleting existing secret: [%s] due to %v", err) + } + } + return updatedSecret, true, true, nil + } + } else { - // either we have no secret, or it has changed. in both cases, we try to delete any existing secret + // no image pull secret, delete existing secret if necessary if secret != nil { secretsInterface := typedClient.CoreV1().Secrets(secret.GetNamespace()) if err := deleteSecret(ctx, secretsInterface, secret.GetName()); err != nil { log.Errorf("Error deleting existing secret: [%s] due to %v", err) } } - - return updatedSecret, true, true, nil + return nil, false, true, nil } } // this func is only used with kubeapps-managed secrets func newSecretFromTlsConfigAndAuth(repoName string, + existingSecret *k8scorev1.Secret, tlsConfig *corev1.PackageRepositoryTlsConfig, auth *corev1.PackageRepositoryAuth) (secret *k8scorev1.Secret, isSameSecret bool, err error) { - if tlsConfig != nil { - if tlsConfig.GetSecretRef() != nil { - return nil, false, status.Errorf(codes.InvalidArgument, "SecretRef may not be used with kubeapps managed secrets") - } + + var hadSecretCa, hadSecretHeader, hadSecretDocker bool + if existingSecret != nil { + _, hadSecretCa = existingSecret.Data[SecretCaKey] + _, hadSecretHeader = existingSecret.Data[SecretAuthHeaderKey] + _, hadSecretDocker = existingSecret.Data[DockerConfigJsonKey] + } + + isSameSecret = true + secret = newLocalOpaqueSecret(repoName) + + if tlsConfig != nil && tlsConfig.GetCertAuthority() != "" { caCert := tlsConfig.GetCertAuthority() if caCert == RedactedString { - isSameSecret = true - } else if caCert != "" { - secret = newLocalOpaqueSecret(repoName) + if hadSecretCa { + secret.Data[SecretCaKey] = existingSecret.Data[SecretCaKey] + } else { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, unexpected REDACTED content") + } + } else { secret.Data[SecretCaKey] = []byte(caCert) + isSameSecret = false + } + } else { + if hadSecretCa { + isSameSecret = false } } + if auth != nil { - if auth.GetSecretRef() != nil { - return nil, false, status.Errorf(codes.InvalidArgument, "SecretRef may not be used with kubeapps managed secrets") - } - if secret == nil { - secret = newLocalOpaqueSecret(repoName) - } switch auth.Type { case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH: - if unp := auth.GetUsernamePassword(); unp != nil { - if unp.Username == "" || unp.Password == "" { - return nil, false, status.Errorf(codes.InvalidArgument, "Wrong combination of username and password") - } else if unp.Username == RedactedString && unp.Password == RedactedString { - isSameSecret = true - } else { - authString := fmt.Sprintf("%s:%s", unp.Username, unp.Password) - authHeader := fmt.Sprintf("Basic %s", base64.StdEncoding.EncodeToString([]byte(authString))) - secret.Data[SecretAuthHeaderKey] = []byte(authHeader) + unp := auth.GetUsernamePassword() + if unp == nil || unp.GetUsername() == "" || unp.GetPassword() == "" { + return nil, false, status.Errorf(codes.InvalidArgument, "Username/Password configuration is missing") + } + if (unp.GetUsername() == RedactedString || unp.GetPassword() == RedactedString) && !hadSecretHeader { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, unexpected REDACTED content") + } + + if unp.GetUsername() == RedactedString && unp.GetPassword() == RedactedString { + secret.Data[SecretAuthHeaderKey] = existingSecret.Data[SecretAuthHeaderKey] + } else if unp.GetUsername() == RedactedString || unp.GetPassword() == RedactedString { + username, password, ok := decodeBasicAuth(string(existingSecret.Data[SecretAuthHeaderKey])) + if !ok { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, the existing repository does not have username/password authentication") + } + if unp.GetUsername() != RedactedString { + username = unp.GetUsername() } + if unp.GetPassword() != RedactedString { + password = unp.GetPassword() + } + isSameSecret = false + secret.Data[SecretAuthHeaderKey] = []byte(encodeBasicAuth(username, password)) } else { - return nil, false, status.Errorf(codes.Internal, "Username/Password configuration is missing") + isSameSecret = false + secret.Data[SecretAuthHeaderKey] = []byte(encodeBasicAuth(unp.GetUsername(), unp.GetPassword())) } case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER: - if token := auth.GetHeader(); token != "" { - if token == RedactedString { - isSameSecret = true + token := auth.GetHeader() + if token == "" { + return nil, false, status.Errorf(codes.InvalidArgument, "Bearer token is missing") + } + + if token == RedactedString { + if hadSecretHeader { + secret.Data[SecretAuthHeaderKey] = existingSecret.Data[SecretAuthHeaderKey] } else { - secret.Data[SecretAuthHeaderKey] = []byte("Bearer " + strings.TrimPrefix(token, "Bearer ")) + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, unexpected REDACTED content") } } else { - return nil, false, status.Errorf(codes.InvalidArgument, "Bearer token is missing") + isSameSecret = false + secret.Data[SecretAuthHeaderKey] = []byte(encodeBearerAuth(token)) } case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_AUTHORIZATION_HEADER: - if authHeaderValue := auth.GetHeader(); authHeaderValue != "" { - if authHeaderValue == RedactedString { - isSameSecret = true + header := auth.GetHeader() + if header == "" { + return nil, false, status.Errorf(codes.InvalidArgument, "Authentication header value is missing") + } + + if header == RedactedString { + if hadSecretHeader { + secret.Data[SecretAuthHeaderKey] = existingSecret.Data[SecretAuthHeaderKey] } else { - secret.Data[SecretAuthHeaderKey] = []byte(authHeaderValue) + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, unexpected REDACTED content") } } else { - return nil, false, status.Errorf(codes.InvalidArgument, "Authentication header value is missing") + isSameSecret = false + secret.Data[SecretAuthHeaderKey] = []byte(header) } case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON: - if dockerCreds := auth.GetDockerCreds(); dockerCreds != nil { - if dockerCreds.Password == RedactedString { - isSameSecret = true + creds := auth.GetDockerCreds() + if creds == nil || creds.Server == "" || creds.Username == "" || creds.Password == "" { + return nil, false, status.Errorf(codes.InvalidArgument, "Docker credentials are missing") + } + if (creds.Server == RedactedString || creds.Username == RedactedString || creds.Password == RedactedString || creds.Email == RedactedString) && !hadSecretDocker { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, unexpected REDACTED content") + } + + secret.Type = k8scorev1.SecretTypeDockerConfigJson + if creds.Server == RedactedString && creds.Username == RedactedString && creds.Password == RedactedString && creds.Email == RedactedString { + secret.Data[DockerConfigJsonKey] = existingSecret.Data[DockerConfigJsonKey] + } else if creds.Server == RedactedString || creds.Username == RedactedString || creds.Password == RedactedString || creds.Email == RedactedString { + newcreds, err := decodeDockerAuth(existingSecret.Data[DockerConfigJsonKey]) + if err != nil { + return nil, false, status.Errorf(codes.Internal, "Invalid configuration, the existing repository does not have valid docker authentication") + } + + if creds.Server != RedactedString { + newcreds.Server = creds.Server + } + if creds.Username != RedactedString { + newcreds.Username = creds.Username + } + if creds.Password != RedactedString { + newcreds.Password = creds.Password + } + if creds.Email != RedactedString { + newcreds.Email = creds.Email + } + + isSameSecret = false + if configjson, err := encodeDockerAuth(newcreds); err != nil { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid Docker credentials") } else { - secret.Type = k8scorev1.SecretTypeDockerConfigJson - dockerConfig := &credentialprovider.DockerConfigJSON{ - Auths: map[string]credentialprovider.DockerConfigEntry{ - dockerCreds.Server: { - Username: dockerCreds.Username, - Password: dockerCreds.Password, - Email: dockerCreds.Email, - }, - }, - } - dockerConfigJson, err := json.Marshal(dockerConfig) - if err != nil { - return nil, false, status.Errorf(codes.InvalidArgument, "Docker credentials are wrong") - } - secret.Data[DockerConfigJsonKey] = dockerConfigJson + secret.Data[DockerConfigJsonKey] = configjson } } else { - return nil, false, status.Errorf(codes.InvalidArgument, "Docker credentials are missing") + isSameSecret = false + if configjson, err := encodeDockerAuth(creds); err != nil { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid Docker credentials") + } else { + secret.Data[DockerConfigJsonKey] = configjson + } } - case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_TLS: - return nil, false, status.Errorf(codes.Unimplemented, "Package repository authentication type %q is not supported", auth.Type) - case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_UNSPECIFIED: - return nil, true, nil default: - return nil, false, status.Errorf(codes.Internal, "Unexpected package repository authentication type: %q", auth.Type) + return nil, false, status.Errorf(codes.InvalidArgument, "Package repository authentication type %q is not supported", auth.Type) + } + } else { + if hadSecretHeader || hadSecretDocker { + isSameSecret = false } } return secret, isSameSecret, nil @@ -394,41 +475,63 @@ func imagesPullSecretName(repoName string) string { return fmt.Sprintf("pullsecret-%s", repoName) } -func newDockerImagePullSecret(repoName string, credentials *corev1.DockerCredentials) (secret *k8scorev1.Secret, isSameSecret bool, err error) { - if credentials != nil { - if credentials.Server == "" || credentials.Username == "" || credentials.Password == "" || credentials.Email == "" { - return nil, false, status.Errorf(codes.InvalidArgument, "Images pull secret Docker credentials are wrong") +func newDockerImagePullSecret(repoName string, existingSecret *k8scorev1.Secret, creds *corev1.DockerCredentials) (secret *k8scorev1.Secret, isSameSecret bool, err error) { + if creds.Server == "" || creds.Username == "" || creds.Password == "" { + return nil, false, status.Errorf(codes.InvalidArgument, "Docker credentials are missing") + } + if (creds.Server == RedactedString || creds.Username == RedactedString || creds.Password == RedactedString || creds.Email == RedactedString) && existingSecret == nil { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, unexpected REDACTED content") + } + + secret = &k8scorev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: imagesPullSecretName(repoName), + Annotations: map[string]string{Annotation_ManagedBy_Key: Annotation_ManagedBy_Value}, + }, + Type: k8scorev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{}, + } + + if creds.Server == RedactedString && creds.Username == RedactedString && creds.Password == RedactedString && creds.Email == RedactedString { + // same + return nil, true, nil + + } else if creds.Server == RedactedString || creds.Username == RedactedString || creds.Password == RedactedString || creds.Email == RedactedString { + // merge + newcreds, err := decodeDockerAuth(existingSecret.Data[DockerConfigJsonKey]) + if err != nil { + return nil, false, status.Errorf(codes.Internal, "Invalid configuration, the existing repository does not have valid docker authentication") } - secret = &k8scorev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: imagesPullSecretName(repoName), - Annotations: map[string]string{Annotation_ManagedBy_Key: Annotation_ManagedBy_Value}, - }, - Type: k8scorev1.SecretTypeDockerConfigJson, - Data: map[string][]byte{}, + if creds.Server != RedactedString { + newcreds.Server = creds.Server + } + if creds.Username != RedactedString { + newcreds.Username = creds.Username + } + if creds.Password != RedactedString { + newcreds.Password = creds.Password + } + if creds.Email != RedactedString { + newcreds.Email = creds.Email } - if credentials.Password == RedactedString { - isSameSecret = true + if configjson, err := encodeDockerAuth(newcreds); err != nil { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid Docker credentials") } else { - dockerConfig := &credentialprovider.DockerConfigJSON{ - Auths: map[string]credentialprovider.DockerConfigEntry{ - credentials.Server: { - Username: credentials.Username, - Password: credentials.Password, - Email: credentials.Email, - }, - }, - } - dockerConfigJson, err := json.Marshal(dockerConfig) - if err != nil { - return nil, false, status.Errorf(codes.InvalidArgument, "Docker credentials are malformed") - } - secret.Data[k8scorev1.DockerConfigJsonKey] = dockerConfigJson + secret.Data[DockerConfigJsonKey] = configjson + return secret, false, nil + } + + } else { + // new + if configjson, err := encodeDockerAuth(creds); err != nil { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid Docker credentials") + } else { + secret.Data[DockerConfigJsonKey] = configjson + return secret, false, nil } } - return secret, isSameSecret, nil } func deleteSecret(ctx context.Context, secretsInterface v1.SecretInterface, secretName string) error { @@ -700,3 +803,59 @@ func isSecretKubeappsManaged(repo *apprepov1alpha1.AppRepository, secret *k8scor } return true } + +// utilities for secrets encoding/decocing + +func encodeBasicAuth(username, password string) string { + auth := fmt.Sprintf("%s:%s", username, password) + auth = fmt.Sprintf("Basic %s", base64.StdEncoding.EncodeToString([]byte(auth))) + return auth +} + +func decodeBasicAuth(auth string) (username string, password string, ok bool) { + if strings.HasPrefix(auth, "Basic ") { + auth = strings.TrimPrefix(auth, "Basic ") + } else { + return "", "", false + } + if bytes, err := base64.StdEncoding.DecodeString(auth); err != nil { + return "", "", false + } else { + auth = string(bytes) + } + return strings.Cut(auth, ":") +} + +func encodeBearerAuth(token string) string { + return "Bearer " + strings.TrimPrefix(token, "Bearer ") +} + +func encodeDockerAuth(credentials *corev1.DockerCredentials) ([]byte, error) { + config := &credentialprovider.DockerConfigJSON{ + Auths: map[string]credentialprovider.DockerConfigEntry{ + credentials.Server: { + Username: credentials.Username, + Password: credentials.Password, + Email: credentials.Email, + }, + }, + } + return json.Marshal(config) +} + +func decodeDockerAuth(dockerjson []byte) (*corev1.DockerCredentials, error) { + config := &credentialprovider.DockerConfigJSON{} + if err := json.Unmarshal(dockerjson, config); err != nil { + return nil, err + } + for server, entry := range config.Auths { + docker := &corev1.DockerCredentials{ + Server: server, + Username: entry.Username, + Password: entry.Password, + Email: entry.Email, + } + return docker, nil + } + return nil, fmt.Errorf("invalid dockerconfig, no Auths entries were found") +} diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go index 0d368bba44d..817e9208b95 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go @@ -89,7 +89,7 @@ var repo3 = &appRepov1alpha1.AppRepository{ Description: "description 3", Auth: appRepov1alpha1.AppRepositoryAuth{ Header: &appRepov1alpha1.AppRepositoryAuthHeader{ - SecretKeyRef: apiv1.SecretKeySelector{LocalObjectReference: apiv1.LocalObjectReference{Name: helm.SecretNameForRepo("repo-3")}, Key: "AuthorizationHeader"}, + SecretKeyRef: apiv1.SecretKeySelector{LocalObjectReference: apiv1.LocalObjectReference{Name: helm.SecretNameForRepo("repo-3")}, Key: "authorizationHeader"}, }, }, }, @@ -142,6 +142,55 @@ var repo5 = &appRepov1alpha1.AppRepository{ }, } +var repo6 = &appRepov1alpha1.AppRepository{ + TypeMeta: metav1.TypeMeta{ + APIVersion: appReposAPIVersion, + Kind: AppRepositoryKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "repo-6", + Namespace: "ns-6", + ResourceVersion: "1", + }, + Spec: appRepov1alpha1.AppRepositorySpec{ + URL: "https://test-repo6", + Type: "helm", + Auth: appRepov1alpha1.AppRepositoryAuth{ + Header: &appRepov1alpha1.AppRepositoryAuthHeader{ + SecretKeyRef: apiv1.SecretKeySelector{LocalObjectReference: apiv1.LocalObjectReference{Name: helm.SecretNameForRepo("repo-6")}, Key: "authorizationHeader"}, + }, + CustomCA: &appRepov1alpha1.AppRepositoryCustomCA{ + SecretKeyRef: apiv1.SecretKeySelector{LocalObjectReference: apiv1.LocalObjectReference{Name: helm.SecretNameForRepo("repo-6")}, Key: "ca.crt"}, + }, + }, + }, +} + +var repo7 = &appRepov1alpha1.AppRepository{ + TypeMeta: metav1.TypeMeta{ + APIVersion: appReposAPIVersion, + Kind: AppRepositoryKind, + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "repo-7", + Namespace: "ns-7", + ResourceVersion: "1", + }, + Spec: appRepov1alpha1.AppRepositorySpec{ + URL: "https://test-repo7", + Type: "helm", + Auth: appRepov1alpha1.AppRepositoryAuth{ + Header: &appRepov1alpha1.AppRepositoryAuthHeader{ + SecretKeyRef: apiv1.SecretKeySelector{LocalObjectReference: apiv1.LocalObjectReference{Name: helm.SecretNameForRepo("repo-7")}, Key: "authorizationHeader"}, + }, + CustomCA: &appRepov1alpha1.AppRepositoryCustomCA{ + SecretKeyRef: apiv1.SecretKeySelector{LocalObjectReference: apiv1.LocalObjectReference{Name: helm.SecretNameForRepo("repo-7")}, Key: "ca.crt"}, + }, + }, + DockerRegistrySecrets: []string{imagesPullSecretName("repo-7")}, + }, +} + var repo1Summary = &corev1.PackageRepositorySummary{ PackageRepoRef: repoRef("repo-1", KubeappsCluster, "ns-1"), Name: "repo-1", @@ -194,16 +243,18 @@ func TestAddPackageRepository(t *testing.T) { } testCases := []struct { - name string - request *corev1.AddPackageRepositoryRequest - expectedResponse *corev1.AddPackageRepositoryResponse - expectedRepo *appRepov1alpha1.AppRepository - statusCode codes.Code - existingSecret *apiv1.Secret - expectedCreatedSecret *apiv1.Secret - userManagedSecrets bool - repoClientGetter newRepoClient - expectedGlobalSecret *apiv1.Secret + name string + request *corev1.AddPackageRepositoryRequest + expectedResponse *corev1.AddPackageRepositoryResponse + expectedRepo *appRepov1alpha1.AppRepository + statusCode codes.Code + existingAuthSecret *apiv1.Secret + existingDockerSecret *apiv1.Secret + expectedAuthCreatedSecret *apiv1.Secret + expectedDockerCreatedSecret *apiv1.Secret + userManagedSecrets bool + repoClientGetter newRepoClient + expectedGlobalSecret *apiv1.Secret }{ { name: "returns error if no namespace is provided", @@ -255,19 +306,19 @@ func TestAddPackageRepository(t *testing.T) { }, // CUSTOM CA AUTH { - name: "package repository with tls cert authority", - request: addRepoReqTLSCA(ca), - expectedResponse: addRepoExpectedResp, - expectedRepo: &addRepoWithTLSCA, - expectedCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newTlsSecret("apprepo-bar", "foo", 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, + name: "package repository with tls cert authority", + request: addRepoReqTLSCA(ca), + expectedResponse: addRepoExpectedResp, + expectedRepo: &addRepoWithTLSCA, + expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newTlsSecret("apprepo-bar", "foo", 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, }, { name: "package repository with secret key reference", request: addRepoReqTLSSecretRef, userManagedSecrets: true, - existingSecret: newTlsSecret("secret-1", "foo", nil, nil, ca), + existingAuthSecret: newTlsSecret("secret-1", "foo", nil, nil, ca), expectedResponse: addRepoExpectedResp, expectedRepo: &addRepoTLSSecret, expectedGlobalSecret: newTlsSecret("foo-apprepo-bar", kubeappsNamespace, nil, nil, ca), @@ -281,13 +332,13 @@ func TestAddPackageRepository(t *testing.T) { }, // BASIC AUTH { - name: "[kubeapps managed secrets] package repository with basic auth and pass_credentials flag", - request: addRepoReqBasicAuth("baz", "zot"), - expectedResponse: addRepoExpectedResp, - expectedRepo: addRepoAuthHeaderPassCredentials("foo"), - expectedCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newBasicAuthSecret("apprepo-bar", "foo", "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", + request: addRepoReqBasicAuth("baz", "zot"), + expectedResponse: addRepoExpectedResp, + expectedRepo: addRepoAuthHeaderPassCredentials("foo"), + expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newBasicAuthSecret("apprepo-bar", "foo", "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 copies secret to kubeapps ns", @@ -308,11 +359,11 @@ func TestAddPackageRepository(t *testing.T) { PassCredentials: true, }, }, - expectedResponse: addRepoExpectedGlobalResp, - expectedRepo: addRepoAuthHeaderPassCredentials(globalPackagingNamespace), - expectedCreatedSecret: setSecretAnnotations(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, + expectedResponse: addRepoExpectedGlobalResp, + expectedRepo: addRepoAuthHeaderPassCredentials(globalPackagingNamespace), + expectedAuthCreatedSecret: setSecretAnnotations(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, }, { name: "package repository with wrong basic auth", @@ -323,7 +374,7 @@ func TestAddPackageRepository(t *testing.T) { name: "[user managed secrets] package repository basic auth with existing secret", request: addRepoReqAuthWithSecret(corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, "foo", "secret-basic"), userManagedSecrets: true, - existingSecret: newBasicAuthSecret("secret-basic", "foo", "baz-user", "zot-pwd"), + existingAuthSecret: newBasicAuthSecret("secret-basic", "foo", "baz-user", "zot-pwd"), expectedResponse: addRepoExpectedResp, expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "secret-basic"), expectedGlobalSecret: newBasicAuthSecret("foo-apprepo-bar", kubeappsNamespace, "baz-user", "zot-pwd"), @@ -347,7 +398,7 @@ func TestAddPackageRepository(t *testing.T) { }, }, userManagedSecrets: true, - existingSecret: newBasicAuthSecret("secret-basic", globalPackagingNamespace, "baz-user", "zot-pwd"), + existingAuthSecret: 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"), @@ -355,22 +406,22 @@ func TestAddPackageRepository(t *testing.T) { }, // BEARER TOKEN { - name: "package repository with bearer token w/o prefix", - request: addRepoReqBearerToken("the-token", false), - expectedResponse: addRepoExpectedResp, - expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"), - expectedCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "Bearer the-token"))), - expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"), - statusCode: codes.OK, + name: "package repository with bearer token w/o prefix", + request: addRepoReqBearerToken("the-token", false), + expectedResponse: addRepoExpectedResp, + expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"), + expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "Bearer the-token"))), + expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"), + statusCode: codes.OK, }, { - name: "package repository with bearer token w/ prefix", - request: addRepoReqBearerToken("the-token", true), - expectedResponse: addRepoExpectedResp, - expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"), - expectedCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "Bearer the-token"))), - expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"), - statusCode: codes.OK, + name: "package repository with bearer token w/ prefix", + request: addRepoReqBearerToken("the-token", true), + expectedResponse: addRepoExpectedResp, + expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"), + expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "Bearer the-token"))), + expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"), + statusCode: codes.OK, }, { name: "package repository with no bearer token", @@ -381,7 +432,7 @@ func TestAddPackageRepository(t *testing.T) { name: "package repository bearer token with secret (user managed secrets)", request: addRepoReqAuthWithSecret(corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, "foo", "secret-bearer"), userManagedSecrets: true, - existingSecret: newAuthTokenSecret("secret-bearer", "foo", "Bearer the-token"), + existingAuthSecret: newAuthTokenSecret("secret-bearer", "foo", "Bearer the-token"), expectedResponse: addRepoExpectedResp, expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "secret-bearer"), expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"), @@ -389,19 +440,19 @@ func TestAddPackageRepository(t *testing.T) { }, // CUSTOM AUTH { - name: "package repository with custom auth", - request: addRepoReqCustomAuth, - expectedResponse: addRepoExpectedResp, - expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"), - expectedCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "foobarzot"))), - expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "foobarzot"), - statusCode: codes.OK, + name: "package repository with custom auth", + request: addRepoReqCustomAuth, + expectedResponse: addRepoExpectedResp, + expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"), + expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "foobarzot"))), + expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "foobarzot"), + statusCode: codes.OK, }, { name: "package repository custom auth with existing secret (user managed secrets)", request: addRepoReqAuthWithSecret(corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_AUTHORIZATION_HEADER, "foo", "secret-custom"), userManagedSecrets: true, - existingSecret: newBasicAuthSecret("secret-custom", "foo", "baz", "zot"), + existingAuthSecret: newBasicAuthSecret("secret-custom", "foo", "baz", "zot"), expectedResponse: addRepoExpectedResp, expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "secret-custom"), expectedGlobalSecret: newBasicAuthSecret("foo-apprepo-bar", kubeappsNamespace, "baz", "zot"), @@ -425,7 +476,7 @@ func TestAddPackageRepository(t *testing.T) { }, }, userManagedSecrets: true, - existingSecret: newBasicAuthSecret("secret-custom", globalPackagingNamespace, "baz", "zot"), + existingAuthSecret: newBasicAuthSecret("secret-custom", globalPackagingNamespace, "baz", "zot"), expectedResponse: addRepoExpectedGlobalResp, expectedRepo: addRepoAuthHeaderWithSecretRef(globalPackagingNamespace, "secret-custom"), expectedGlobalSecret: newBasicAuthSecret("kubeapps-repos-global-apprepo-bar", kubeappsNamespace, "baz", "zot"), @@ -442,7 +493,7 @@ func TestAddPackageRepository(t *testing.T) { }), expectedResponse: addRepoExpectedResp, expectedRepo: addRepoAuthDocker("apprepo-bar"), - expectedCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", + expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthDockerSecret("apprepo-bar", "foo", dockerAuthJson("https://docker-server", "the-user", "the-password", "foo@bar.com", "dGhlLXVzZXI6dGhlLXBhc3N3b3Jk")))), expectedGlobalSecret: newAuthDockerSecret("foo-apprepo-bar", kubeappsNamespace, @@ -453,7 +504,7 @@ func TestAddPackageRepository(t *testing.T) { name: "package repository with Docker auth (user managed secrets)", request: addRepoReqAuthWithSecret(corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON, "foo", "secret-docker"), userManagedSecrets: true, - existingSecret: newAuthDockerSecret("secret-docker", "foo", + existingAuthSecret: newAuthDockerSecret("secret-docker", "foo", dockerAuthJson("https://docker-server", "the-user", "the-password", "foo@bar.com", "dGhlLXVzZXI6dGhlLXBhc3N3b3Jk")), expectedResponse: addRepoExpectedResp, expectedRepo: addRepoAuthDocker("secret-docker"), @@ -483,7 +534,7 @@ func TestAddPackageRepository(t *testing.T) { { name: "package repository with reference to malformed secret", request: addRepoReqAuthWithSecret(corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, "foo", "secret-basic"), - existingSecret: newTlsSecret("secret-basic", "foo", nil, nil, nil), // Creates empty secret + existingAuthSecret: newTlsSecret("secret-basic", "foo", nil, nil, nil), // Creates empty secret userManagedSecrets: true, statusCode: codes.Internal, }, @@ -542,7 +593,7 @@ func TestAddPackageRepository(t *testing.T) { }), expectedResponse: addRepoExpectedResp, userManagedSecrets: true, - existingSecret: newAuthDockerSecret("secret-docker", "foo", + existingDockerSecret: newAuthDockerSecret("secret-docker", "foo", dockerAuthJson("https://docker-server", "the-user", "the-password", "foo@bar.com", "dGhlLXVzZXI6dGhlLXBhc3N3b3Jk")), expectedRepo: &appRepov1alpha1.AppRepository{ TypeMeta: metav1.TypeMeta{ @@ -588,7 +639,7 @@ func TestAddPackageRepository(t *testing.T) { }), expectedResponse: addRepoExpectedResp, userManagedSecrets: true, - existingSecret: newAuthTokenSecret("secret-docker", "foo", ""), + existingAuthSecret: newAuthTokenSecret("secret-docker", "foo", ""), statusCode: codes.InvalidArgument, }, { @@ -602,7 +653,7 @@ func TestAddPackageRepository(t *testing.T) { }), expectedResponse: addRepoExpectedResp, userManagedSecrets: true, - existingSecret: &apiv1.Secret{ + existingDockerSecret: &apiv1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "secret-docker", Namespace: "foo", @@ -645,7 +696,7 @@ func TestAddPackageRepository(t *testing.T) { DockerRegistrySecrets: []string{"pullsecret-bar"}, }, }, - expectedCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", + expectedDockerCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthDockerSecret("pullsecret-bar", "foo", dockerAuthJson("https://myfooserver.com", "username", "password", "foo@bar.com", "dXNlcm5hbWU6cGFzc3dvcmQ=")))), statusCode: codes.OK, @@ -666,7 +717,7 @@ func TestAddPackageRepository(t *testing.T) { OciRepositories: []string{"oci-repo-1", "oci-repo-2"}, }), expectedResponse: addRepoExpectedResp, - existingSecret: newAuthDockerSecret("pullsecret-bar", "foo", + existingDockerSecret: newAuthDockerSecret("pullsecret-bar", "foo", dockerAuthJson("https://myfooserver.com", "username", "password", "foo@bar.com", "dXNlcm5hbWU6cGFzc3dvcmQ=")), statusCode: codes.AlreadyExists, }, @@ -675,8 +726,11 @@ func TestAddPackageRepository(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { var secrets []k8sruntime.Object - if tc.existingSecret != nil { - secrets = append(secrets, tc.existingSecret) + if tc.existingAuthSecret != nil { + secrets = append(secrets, tc.existingAuthSecret) + } + if tc.existingDockerSecret != nil { + secrets = append(secrets, tc.existingDockerSecret) } s := newServerWithSecretsAndRepos(t, secrets, nil, nil) if tc.repoClientGetter != nil { @@ -726,7 +780,7 @@ func TestAddPackageRepository(t *testing.T) { if err = ctrlClient.Get(ctx, nsname, &actualRepo); err != nil { t.Fatal(err) } else { - checkRepoSecrets(s, t, tc.userManagedSecrets, &actualRepo, tc.expectedRepo, tc.expectedCreatedSecret, tc.expectedGlobalSecret) + checkRepoSecrets(s, t, tc.userManagedSecrets, &actualRepo, tc.expectedRepo, tc.expectedAuthCreatedSecret, tc.expectedDockerCreatedSecret, tc.expectedGlobalSecret) } } }) @@ -1136,7 +1190,7 @@ func TestUpdatePackageRepository(t *testing.T) { } } - ca, _, _ := getCertsForTesting(t) + ca, pub, _ := getCertsForTesting(t) testCases := []struct { name string @@ -1144,8 +1198,10 @@ func TestUpdatePackageRepository(t *testing.T) { expectedRepoCustomizer func(repository appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository expectedStatusCode codes.Code expectedRef *corev1.PackageRepositoryReference - existingSecret *apiv1.Secret - expectedSecret *apiv1.Secret + existingAuthSecret *apiv1.Secret + existingDockerSecret *apiv1.Secret + expectedAuthSecret *apiv1.Secret + expectedDockerSecret *apiv1.Secret userManagedSecrets bool expectedGlobalSecret *apiv1.Secret }{ @@ -1156,7 +1212,8 @@ func TestUpdatePackageRepository(t *testing.T) { return request }, expectedStatusCode: codes.InvalidArgument, - }, { + }, + { name: "repository not found", requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { request.PackageRepoRef.Context = &corev1.Context{Cluster: "other", Namespace: globalPackagingNamespace} @@ -1220,13 +1277,13 @@ func TestUpdatePackageRepository(t *testing.T) { return &repository }, expectedRef: defaultRef, - expectedSecret: setSecretAnnotations(setSecretOwnerRef("repo-1", newTlsSecret("apprepo-repo-1", "ns-1", nil, nil, ca))), + expectedAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-1", newTlsSecret("apprepo-repo-1", "ns-1", nil, nil, ca))), expectedGlobalSecret: newTlsSecret("ns-1-apprepo-repo-1", kubeappsNamespace, nil, nil, ca), expectedStatusCode: codes.OK, }, { - name: "update removing tsl config", - existingSecret: newTlsSecret(helm.SecretNameForRepo("repo-4"), "ns-4", nil, nil, ca), + name: "update removing tsl config", + existingAuthSecret: newTlsSecret(helm.SecretNameForRepo("repo-4"), "ns-4", nil, nil, ca), requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { request.PackageRepoRef = &corev1.PackageRepositoryReference{ Plugin: &pluginDetail, @@ -1279,7 +1336,7 @@ func TestUpdatePackageRepository(t *testing.T) { return &repository }, expectedRef: defaultRef, - expectedSecret: setSecretAnnotations(setSecretOwnerRef("repo-1", newAuthTokenSecret("apprepo-repo-1", "ns-1", "Bearer foobarzot"))), + expectedAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-1", newAuthTokenSecret("apprepo-repo-1", "ns-1", "Bearer foobarzot"))), expectedGlobalSecret: newAuthTokenSecret("ns-1-apprepo-repo-1", kubeappsNamespace, "Bearer foobarzot"), expectedStatusCode: codes.OK, }, @@ -1297,7 +1354,7 @@ func TestUpdatePackageRepository(t *testing.T) { return request }, userManagedSecrets: true, - existingSecret: newAuthTokenSecret("my-own-secret", "ns-1", "Bearer foobarzot"), + existingAuthSecret: newAuthTokenSecret("my-own-secret", "ns-1", "Bearer foobarzot"), expectedRepoCustomizer: func(repository appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository { repository.ResourceVersion = "2" repository.Spec.Auth = appRepov1alpha1.AppRepositoryAuth{ @@ -1317,8 +1374,8 @@ func TestUpdatePackageRepository(t *testing.T) { expectedStatusCode: codes.OK, }, { - name: "update removing auth", - existingSecret: newAuthTokenSecret(helm.SecretNameForRepo("repo-3"), globalPackagingNamespace, "token-value"), + name: "update removing auth", + existingAuthSecret: newAuthTokenSecret(helm.SecretNameForRepo("repo-3"), globalPackagingNamespace, "token-value"), requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { request.PackageRepoRef = &corev1.PackageRepositoryReference{ Plugin: &pluginDetail, @@ -1401,7 +1458,7 @@ func TestUpdatePackageRepository(t *testing.T) { repository.Spec.DockerRegistrySecrets = []string{"pullsecret-repo-1"} return &repository }, - expectedSecret: setSecretAnnotations(setSecretOwnerRef("repo-1", + expectedDockerSecret: setSecretAnnotations(setSecretOwnerRef("repo-1", newAuthDockerSecret("pullsecret-repo-1", "ns-1", dockerAuthJson("https://myfooserver.com", "username", "password", "foo@bar.com", "dXNlcm5hbWU6cGFzc3dvcmQ=")))), expectedStatusCode: codes.OK, @@ -1430,7 +1487,7 @@ func TestUpdatePackageRepository(t *testing.T) { }) return request }, - existingSecret: newAuthDockerSecret(imagesPullSecretName("repo-5"), "ns-5", + existingDockerSecret: newAuthDockerSecret(imagesPullSecretName("repo-5"), "ns-5", dockerAuthJson("https://myfooserver.com", "username", "password", "foo@bar.com", "dXNlcm5hbWU6cGFzc3dvcmQ=")), expectedRepoCustomizer: func(repository appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository { repository.Name = "repo-5" @@ -1441,6 +1498,8 @@ func TestUpdatePackageRepository(t *testing.T) { repository.Spec.DockerRegistrySecrets = []string{imagesPullSecretName("repo-5")} return &repository }, + expectedDockerSecret: newAuthDockerSecret(imagesPullSecretName("repo-5"), "ns-5", + dockerAuthJson("https://myfooserver.com", "username", "password", "foo@bar.com", "dXNlcm5hbWU6cGFzc3dvcmQ=")), expectedRef: &corev1.PackageRepositoryReference{ Plugin: &pluginDetail, Context: &corev1.Context{Namespace: "ns-5", Cluster: KubeappsCluster}, @@ -1469,7 +1528,7 @@ func TestUpdatePackageRepository(t *testing.T) { return &repository }, userManagedSecrets: true, - existingSecret: newAuthDockerSecret("test-pull-secret", "ns-1", + existingDockerSecret: newAuthDockerSecret("test-pull-secret", "ns-1", dockerAuthJson("https://docker-server", "the-user", "the-password", "foo@bar.com", "dGhlLXVzZXI6dGhlLXBhc3N3b3Jk")), expectedStatusCode: codes.OK, expectedRef: defaultRef, @@ -1486,7 +1545,7 @@ func TestUpdatePackageRepository(t *testing.T) { request.CustomDetail = toProtoBufAny(&v1alpha1.HelmPackageRepositoryCustomDetail{}) return request }, - existingSecret: newAuthDockerSecret("pullsecret-repo-5", "ns-5", + existingDockerSecret: newAuthDockerSecret("pullsecret-repo-5", "ns-5", dockerAuthJson("https://myfooserver.com", "username", "password", "foo@bar.com", "dXNlcm5hbWU6cGFzc3dvcmQ=")), expectedStatusCode: codes.OK, expectedRepoCustomizer: func(repository appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository { @@ -1504,9 +1563,147 @@ func TestUpdatePackageRepository(t *testing.T) { Identifier: "repo-5", }, }, + { + name: "[issue 5746] secret updates ignored if not all credentials are provided - auth updates", + existingAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-6", + addTlsToSecret(newBasicAuthSecret("apprepo-repo-6", "ns-6", "foo", "bar"), nil, nil, ca))), + + requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { + request.PackageRepoRef = &corev1.PackageRepositoryReference{ + Plugin: &pluginDetail, + Context: &corev1.Context{Namespace: "ns-6", Cluster: KubeappsCluster}, + Identifier: "repo-6", + } + request.Url = repo6.Spec.URL + request.Auth = &corev1.PackageRepositoryAuth{ + Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, + PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_UsernamePassword{ + UsernamePassword: &corev1.UsernamePassword{ + Username: RedactedString, + Password: "zot", + }, + }, + } + request.TlsConfig = &corev1.PackageRepositoryTlsConfig{ + PackageRepoTlsConfigOneOf: &corev1.PackageRepositoryTlsConfig_CertAuthority{ + CertAuthority: RedactedString, + }, + } + return request + }, + + expectedStatusCode: codes.OK, + expectedAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-6", + addTlsToSecret(newBasicAuthSecret("apprepo-repo-6", "ns-6", "foo", "zot"), nil, nil, ca))), + expectedRef: &corev1.PackageRepositoryReference{ + Plugin: &pluginDetail, + Context: &corev1.Context{Namespace: "ns-6", Cluster: KubeappsCluster}, + Identifier: "repo-6", + }, + expectedRepoCustomizer: func(repository appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository { + repository.ResourceVersion = "2" + repository.Namespace = repo6.Namespace + repository.Name = repo6.Name + repository.Spec = repo6.Spec + return &repository + }, + expectedGlobalSecret: addTlsToSecret(newBasicAuthSecret("ns-6-apprepo-repo-6", kubeappsNamespace, "foo", "zot"), nil, nil, ca), + }, + { + name: "[issue 5746] secret updates ignored if not all credentials are provided - tls updates", + existingAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-6", + addTlsToSecret(newBasicAuthSecret("apprepo-repo-6", "ns-6", "foo", "bar"), nil, nil, ca))), + + requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { + request.PackageRepoRef = &corev1.PackageRepositoryReference{ + Plugin: &pluginDetail, + Context: &corev1.Context{Namespace: "ns-6", Cluster: KubeappsCluster}, + Identifier: "repo-6", + } + request.Url = repo6.Spec.URL + request.Auth = &corev1.PackageRepositoryAuth{ + Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, + PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_UsernamePassword{ + UsernamePassword: &corev1.UsernamePassword{ + Username: RedactedString, + Password: RedactedString, + }, + }, + } + request.TlsConfig = &corev1.PackageRepositoryTlsConfig{ + PackageRepoTlsConfigOneOf: &corev1.PackageRepositoryTlsConfig_CertAuthority{ + CertAuthority: string(pub), + }, + } + return request + }, + + expectedStatusCode: codes.OK, + expectedAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-6", + addTlsToSecret(newBasicAuthSecret("apprepo-repo-6", "ns-6", "foo", "bar"), nil, nil, pub))), + expectedRef: &corev1.PackageRepositoryReference{ + Plugin: &pluginDetail, + Context: &corev1.Context{Namespace: "ns-6", Cluster: KubeappsCluster}, + Identifier: "repo-6", + }, + expectedRepoCustomizer: func(repository appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository { + repository.ResourceVersion = "2" + repository.Namespace = repo6.Namespace + repository.Name = repo6.Name + repository.Spec = repo6.Spec + return &repository + }, + expectedGlobalSecret: addTlsToSecret(newBasicAuthSecret("ns-6-apprepo-repo-6", kubeappsNamespace, "foo", "bar"), nil, nil, pub), + }, + { + name: "[issue 5746] secret updates ignored if not all credentials are provided - image pull secrets updates", + existingDockerSecret: setSecretAnnotations(setSecretOwnerRef("repo-5", + newAuthDockerSecret("pullsecret-repo-5", "ns-5", + dockerAuthJson("https://myfooserver.com", "username", "password", "foo@bar.com", "dXNlcm5hbWU6cGFzc3dvcmQ=")))), + + requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { + request.PackageRepoRef = &corev1.PackageRepositoryReference{ + Plugin: &pluginDetail, + Context: &corev1.Context{Namespace: "ns-5", Cluster: KubeappsCluster}, + Identifier: "repo-5", + } + request.Url = repo5.Spec.URL + request.Description = repo5.Spec.Description + request.CustomDetail = toProtoBufAny(&v1alpha1.HelmPackageRepositoryCustomDetail{ + ImagesPullSecret: &v1alpha1.ImagesPullSecret{ + DockerRegistryCredentialOneOf: &v1alpha1.ImagesPullSecret_Credentials{ + Credentials: &corev1.DockerCredentials{ + Server: RedactedString, + Username: RedactedString, + Password: RedactedString, + Email: "newemail", + }, + }, + }, + }) + return request + }, + + expectedStatusCode: codes.OK, + expectedDockerSecret: setSecretAnnotations(setSecretOwnerRef("repo-5", + newAuthDockerSecret("pullsecret-repo-5", "ns-5", + dockerAuthJson("https://myfooserver.com", "username", "password", "newemail", "dXNlcm5hbWU6cGFzc3dvcmQ=")))), + expectedRef: &corev1.PackageRepositoryReference{ + Plugin: &pluginDetail, + Context: &corev1.Context{Namespace: "ns-5", Cluster: KubeappsCluster}, + Identifier: "repo-5", + }, + expectedRepoCustomizer: func(repository appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository { + repository.ResourceVersion = "2" + repository.Namespace = repo5.Namespace + repository.Name = repo5.Name + repository.Spec = repo5.Spec + return &repository + }, + }, } - repos := []*appRepov1alpha1.AppRepository{repo1, repo3, repo4, repo5} + repos := []*appRepov1alpha1.AppRepository{repo1, repo3, repo4, repo5, repo6, repo7} for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { @@ -1517,8 +1714,11 @@ func TestUpdatePackageRepository(t *testing.T) { } var secrets []k8sruntime.Object - if tc.existingSecret != nil { - secrets = append(secrets, tc.existingSecret) + if tc.existingAuthSecret != nil { + secrets = append(secrets, tc.existingAuthSecret) + } + if tc.existingDockerSecret != nil { + secrets = append(secrets, tc.existingDockerSecret) } s := newServerWithSecretsAndRepos(t, secrets, unstructuredObjects, repos) @@ -1568,7 +1768,7 @@ func TestUpdatePackageRepository(t *testing.T) { t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, opts)) } - checkRepoSecrets(s, t, tc.userManagedSecrets, appRepo, expectedRepository, tc.expectedSecret, tc.expectedGlobalSecret) + checkRepoSecrets(s, t, tc.userManagedSecrets, appRepo, expectedRepository, tc.expectedAuthSecret, tc.expectedDockerSecret, tc.expectedGlobalSecret) }) } } @@ -1847,10 +2047,11 @@ func TestGetPackageRepositoryPermissions(t *testing.T) { func checkRepoSecrets(s *Server, t *testing.T, userManagedSecrets bool, actualRepo *appRepov1alpha1.AppRepository, expectedRepo *appRepov1alpha1.AppRepository, - expectedSecret *apiv1.Secret, expectedGlobalSecret *apiv1.Secret) { + expectedAuthSecret *apiv1.Secret, expectedDockerSecret *apiv1.Secret, + expectedGlobalSecret *apiv1.Secret) { ctx := context.Background() if userManagedSecrets { - if expectedSecret != nil { + if expectedAuthSecret != nil || expectedDockerSecret != nil { t.Fatalf("Error: unexpected state") } if got, want := actualRepo, expectedRepo; !cmp.Equal(want, got) { @@ -1861,21 +2062,18 @@ func checkRepoSecrets(s *Server, t *testing.T, userManagedSecrets bool, t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got)) } - if expectedSecret != nil { - if actualRepo.Spec.Auth.Header == nil && actualRepo.Spec.Auth.CustomCA == nil && len(actualRepo.Spec.DockerRegistrySecrets) == 0 { - t.Errorf("Error: Repository secrets were expected but auth header, CA and imagePullSecret are empty") + if expectedAuthSecret != nil { + if actualRepo.Spec.Auth.Header == nil && actualRepo.Spec.Auth.CustomCA == nil { + t.Errorf("Error: Repository auth secret was expected but auth header and CA are empty") } typedClient, err := s.clientGetter.Typed(ctx, s.kubeappsCluster) if err != nil { t.Fatal(err) } if actualRepo.Spec.Auth.Header != nil { - checkSecrets(t, ctx, typedClient, actualRepo.Namespace, actualRepo.Spec.Auth.Header.SecretKeyRef.Name, expectedSecret) + checkSecrets(t, ctx, typedClient, actualRepo.Namespace, actualRepo.Spec.Auth.Header.SecretKeyRef.Name, expectedAuthSecret) } else if actualRepo.Spec.Auth.CustomCA != nil { - checkSecrets(t, ctx, typedClient, actualRepo.Namespace, actualRepo.Spec.Auth.CustomCA.SecretKeyRef.Name, expectedSecret) - } else { - // Docker image pull secret check - checkSecrets(t, ctx, typedClient, actualRepo.Namespace, actualRepo.Spec.DockerRegistrySecrets[0], expectedSecret) + checkSecrets(t, ctx, typedClient, actualRepo.Namespace, actualRepo.Spec.Auth.CustomCA.SecretKeyRef.Name, expectedAuthSecret) } } else if actualRepo.Spec.Auth.Header != nil { t.Fatalf("Expected no secret, but found Header: [%v]", actualRepo.Spec.Auth.Header.SecretKeyRef) @@ -1884,8 +2082,23 @@ func checkRepoSecrets(s *Server, t *testing.T, userManagedSecrets bool, } else if expectedRepo.Spec.Auth.Header != nil { t.Fatalf("Error: unexpected state") } + + if expectedDockerSecret != nil { + if len(actualRepo.Spec.DockerRegistrySecrets) == 0 { + t.Errorf("Error: Repository docker secret was expected but imagePullSecrets are empty") + } + typedClient, err := s.clientGetter.Typed(ctx, s.kubeappsCluster) + if err != nil { + t.Fatal(err) + } + checkSecrets(t, ctx, typedClient, actualRepo.Namespace, actualRepo.Spec.DockerRegistrySecrets[0], expectedDockerSecret) + } else if len(actualRepo.Spec.DockerRegistrySecrets) > 0 { + t.Fatalf("Expected no secret, but found image pull secrets: [%v]", actualRepo.Spec.DockerRegistrySecrets[0]) + } else if len(expectedRepo.Spec.DockerRegistrySecrets) > 0 { + t.Fatalf("Error: unexpected state") + } } - checkGlobalSecret(s, t, expectedRepo, expectedGlobalSecret, expectedSecret != nil || expectedRepo.Spec.Auth.Header != nil || expectedRepo.Spec.Auth.CustomCA != nil) + checkGlobalSecret(s, t, expectedRepo, expectedGlobalSecret, expectedAuthSecret != nil || expectedRepo.Spec.Auth.Header != nil || expectedRepo.Spec.Auth.CustomCA != nil) } func checkGlobalSecret(s *Server, t *testing.T, expectedRepo *appRepov1alpha1.AppRepository, expectedGlobalSecret *apiv1.Secret, checkNoGlobalSecret bool) { diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/test_util_test.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/test_util_test.go index d8953c4c601..307cab2ff5a 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/test_util_test.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/test_util_test.go @@ -142,16 +142,20 @@ func newTlsSecret(name, namespace string, pub, priv, ca []byte) *apiv1.Secret { Type: apiv1.SecretTypeOpaque, Data: map[string][]byte{}, } + return addTlsToSecret(s, pub, priv, ca) +} + +func addTlsToSecret(secret *apiv1.Secret, pub, priv, ca []byte) *apiv1.Secret { if pub != nil { - s.Data["certFile"] = pub + secret.Data["certFile"] = pub } if priv != nil { - s.Data["keyFile"] = priv + secret.Data["keyFile"] = priv } if ca != nil { - s.Data["ca.crt"] = ca + secret.Data["ca.crt"] = ca } - return s + return secret } func newRepoHttpClient(responses map[string]*http.Response) newRepoClient { From d3f3c63c67b6b482676d9ded64c5318749810c07 Mon Sep 17 00:00:00 2001 From: Dimitri Laloue Date: Thu, 1 Dec 2022 18:16:05 -0800 Subject: [PATCH 2/8] fixes for flux --- .../packages/v1alpha1/global_vars_test.go | 109 ++++++- .../fluxv2/packages/v1alpha1/repo_auth.go | 284 +++++++++++------- .../fluxv2/packages/v1alpha1/repo_test.go | 132 ++++++-- .../packages/v1alpha1/globals_vars_test.go | 9 +- .../packages/v1alpha1/repositories_auth.go | 58 ++-- .../packages/v1alpha1/repositories_test.go | 15 +- 6 files changed, 423 insertions(+), 184 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go index c1b235400ed..89df2c29997 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go @@ -3518,6 +3518,36 @@ var ( Auth: secret_1_auth, } + update_repo_req_22 = &corev1.UpdatePackageRepositoryRequest{ + PackageRepoRef: repoRefInReq("repo-1", "namespace-1"), + Url: "http://url.com", + Auth: basic_auth(redactedString, "doe"), + } + + update_repo_req_23 = &corev1.UpdatePackageRepositoryRequest{ + PackageRepoRef: repoRefInReq("repo-1", "namespace-1"), + Url: "http://url.com", + TlsConfig: tls_config_redacted, + Auth: basic_auth("john", "doe"), + } + + update_repo_req_24 = &corev1.UpdatePackageRepositoryRequest{ + PackageRepoRef: repoRefInReq("repo-1", "namespace-1"), + Url: "http://url.com", + Auth: basic_auth("john", "doe"), + } + + update_repo_req_25 = func(ca []byte) *corev1.UpdatePackageRepositoryRequest { + return &corev1.UpdatePackageRepositoryRequest{ + PackageRepoRef: repoRefInReq("repo-1", "namespace-1"), + Url: "http://url.com", + TlsConfig: tls_config(ca), + Auth: &corev1.PackageRepositoryAuth{ + Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_UNSPECIFIED, + }, + } + } + update_repo_resp_1 = &corev1.UpdatePackageRepositoryResponse{ PackageRepoRef: repoRef("repo-1", "namespace-1"), } @@ -3792,26 +3822,66 @@ var ( }, } - foo_bar_auth = &corev1.PackageRepositoryAuth{ - Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, - PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_UsernamePassword{ - UsernamePassword: &corev1.UsernamePassword{ - Username: "foo", - Password: "bar", - }, + update_repo_detail_18 = &corev1.GetPackageRepositoryDetailResponse{ + Detail: &corev1.PackageRepositoryDetail{ + PackageRepoRef: get_repo_detail_package_resp_ref, + Name: "repo-1", + Description: "", + NamespaceScoped: true, + Type: "helm", + Url: "http://url.com", + Interval: "10m", + Auth: foo_bar_auth_redacted, + Status: repo_status_pending, }, } - foo_bar_auth_redacted = &corev1.PackageRepositoryAuth{ - Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, - PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_UsernamePassword{ - UsernamePassword: &corev1.UsernamePassword{ - Username: redactedString, - Password: redactedString, - }, + update_repo_detail_19 = &corev1.GetPackageRepositoryDetailResponse{ + Detail: &corev1.PackageRepositoryDetail{ + PackageRepoRef: get_repo_detail_package_resp_ref, + Name: "repo-1", + Description: "", + NamespaceScoped: true, + Type: "helm", + Url: "http://url.com", + Interval: "10m", + TlsConfig: tls_config_redacted, + Auth: foo_bar_auth_redacted, + Status: repo_status_pending, }, } + update_repo_detail_20 = &corev1.GetPackageRepositoryDetailResponse{ + Detail: &corev1.PackageRepositoryDetail{ + PackageRepoRef: get_repo_detail_package_resp_ref, + Name: "repo-1", + Description: "", + NamespaceScoped: true, + Type: "helm", + Url: "http://url.com", + Interval: "10m", + TlsConfig: tls_config_redacted, + Auth: &corev1.PackageRepositoryAuth{PassCredentials: false}, + Status: repo_status_pending, + }, + } + + basic_auth = func(username, password string) *corev1.PackageRepositoryAuth { + return &corev1.PackageRepositoryAuth{ + Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, + PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_UsernamePassword{ + UsernamePassword: &corev1.UsernamePassword{ + Username: username, + Password: password, + }, + }, + } + } + + foo_bar_auth = basic_auth("foo", "bar") + + foo_bar_auth_redacted = basic_auth(redactedString, redactedString) + github_auth = func(ghUser, ghToken string) *corev1.PackageRepositoryAuth { return &corev1.PackageRepositoryAuth{ Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, @@ -3838,6 +3908,17 @@ var ( tls_auth_redacted = tls_auth([]byte(redactedString), []byte(redactedString)) + tls_config = func(ca []byte) *corev1.PackageRepositoryTlsConfig { + return &corev1.PackageRepositoryTlsConfig{ + InsecureSkipVerify: false, + PackageRepoTlsConfigOneOf: &corev1.PackageRepositoryTlsConfig_CertAuthority{ + CertAuthority: string(ca), + }, + } + } + + tls_config_redacted = tls_config([]byte(redactedString)) + secret_1_auth = &corev1.PackageRepositoryAuth{ Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_SecretRef{ diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_auth.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_auth.go index 3811584e4d2..526d2044b15 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_auth.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_auth.go @@ -6,6 +6,7 @@ package main import ( "context" "encoding/json" + "fmt" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" corev1 "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/core/packages/v1alpha1" @@ -51,7 +52,7 @@ func (s *Server) handleRepoSecretForCreate( return secret, false, err } else if hasCaData || hasAuthData { // kubeapps managed - secret, _, err := newSecretFromTlsConfigAndAuth(repoName, repoType, tlsConfig, auth) + secret, _, err := newSecretFromTlsConfigAndAuth(repoName, repoType, nil, tlsConfig, auth) if err != nil { return nil, false, err } @@ -113,33 +114,44 @@ func (s *Server) handleRepoSecretForUpdate( repoName := types.NamespacedName{Name: repo.Name, Namespace: repo.Namespace} repoType := repo.Spec.Type - // handle user managed secret + // create/get secret if hasCaRef || hasAuthRef { + // handle user managed secret updatedSecret, err := s.validateUserManagedRepoSecret(ctx, repoName, repoType, newTlsConfig, newAuth) return updatedSecret, false, true, err - } - // handle kubeapps managed secret - var isSameSecret bool - updatedSecret, isSameSecret, err = newSecretFromTlsConfigAndAuth(repoName, repoType, newTlsConfig, newAuth) - if err != nil { - return nil, true, false, err - } else if isSameSecret { - // Do nothing if repo auth data came redacted - return nil, true, false, nil + } else if hasCaData || hasAuthData { + // handle kubeapps managed secret + updatedSecret, isSameSecret, err := newSecretFromTlsConfigAndAuth(repoName, repoType, existingSecret, newTlsConfig, newAuth) + if err != nil { + return nil, true, false, err + } else if isSameSecret { + // Do nothing if repo auth data came fully redacted + return nil, true, false, nil + } else { + // secret has changed, we try to delete any existing secret + if existingSecret != nil { + if err = secretInterface.Delete(ctx, existingSecret.Name, metav1.DeleteOptions{}); err != nil { + log.Errorf("Error deleting existing secret: [%s] due to %v", err) + } + } + // and we recreate the updated one + if updatedSecret != nil { + if updatedSecret, err = typedClient.CoreV1().Secrets(repoName.Namespace).Create(ctx, updatedSecret, metav1.CreateOptions{}); err != nil { + return nil, false, false, statuserror.FromK8sError("create", "secret", updatedSecret.GetGenerateName(), err) + } + } + return updatedSecret, true, true, nil + } + } else { - // either we have no secret, or it has changed. in both cases, we try to delete any existing secret + // no auth, delete existing secret if necessary if existingSecret != nil { if err = secretInterface.Delete(ctx, existingSecret.Name, metav1.DeleteOptions{}); err != nil { log.Errorf("Error deleting existing secret: [%s] due to %v", err) } } - if updatedSecret != nil { - if updatedSecret, err = typedClient.CoreV1().Secrets(repoName.Namespace).Create(ctx, updatedSecret, metav1.CreateOptions{}); err != nil { - return nil, false, false, statuserror.FromK8sError("create", "secret", updatedSecret.GetGenerateName(), err) - } - } - return updatedSecret, true, true, nil + return nil, false, true, nil } } @@ -150,23 +162,12 @@ func (s *Server) validateUserManagedRepoSecret( tlsConfig *corev1.PackageRepositoryTlsConfig, auth *corev1.PackageRepositoryAuth) (*apiv1.Secret, error) { var secretRefTls, secretRefAuth string - if tlsConfig != nil { - if tlsConfig.GetCertAuthority() != "" { - return nil, status.Errorf(codes.InvalidArgument, "Secret Ref must be used with user managed secrets") - } else if tlsConfig.GetSecretRef().GetName() != "" { - secretRefTls = tlsConfig.GetSecretRef().GetName() - } - } - if auth != nil { - if auth.GetDockerCreds() != nil || - auth.GetHeader() != "" || - auth.GetTlsCertKey() != nil || - auth.GetUsernamePassword() != nil { - return nil, status.Errorf(codes.InvalidArgument, "Secret Ref must be used with user managed secrets") - } else if auth.GetSecretRef().GetName() != "" { - secretRefAuth = auth.GetSecretRef().GetName() - } + if tlsConfig != nil && tlsConfig.GetSecretRef() != nil { + secretRefTls = tlsConfig.GetSecretRef().GetName() + } + if auth != nil && auth.GetSecretRef() != nil { + secretRefAuth = auth.GetSecretRef().GetName() } var secretRef string @@ -318,96 +319,149 @@ func (s *Server) getRepoTlsConfigAndAuth(ctx context.Context, repo sourcev1.Helm // this func is only used with kubeapps-managed secrets func newSecretFromTlsConfigAndAuth(repoName types.NamespacedName, repoType string, + existingSecret *apiv1.Secret, tlsConfig *corev1.PackageRepositoryTlsConfig, auth *corev1.PackageRepositoryAuth) (secret *apiv1.Secret, isSameSecret bool, err error) { - if tlsConfig != nil { - if tlsConfig.GetSecretRef() != nil { - return nil, false, status.Errorf(codes.InvalidArgument, "SecretRef may not be used with kubeapps managed secrets") - } + + var hadSecretTlsCa, hadSecretTlsCert, hadSecretTlsKey, hadSecretUsername, hadSecretPassword, hadSecretDocker bool + if existingSecret != nil { + _, hadSecretTlsCa = existingSecret.Data["caFile"] + _, hadSecretTlsCert = existingSecret.Data["certFile"] + _, hadSecretTlsKey = existingSecret.Data["keyFile"] + _, hadSecretUsername = existingSecret.Data["username"] + _, hadSecretPassword = existingSecret.Data["password"] + _, hadSecretDocker = existingSecret.Data[apiv1.DockerConfigJsonKey] + } + + isSameSecret = true + secret = newLocalOpaqueSecret(repoName) + + if tlsConfig != nil && tlsConfig.GetCertAuthority() != "" { caCert := tlsConfig.GetCertAuthority() if caCert == redactedString { - isSameSecret = true - } else if caCert != "" { - secret = newLocalOpaqueSecret(repoName) - secret.Data["caFile"] = []byte(caCert) - } - } - if auth != nil { - if auth.GetSecretRef() != nil { - return nil, false, status.Errorf(codes.InvalidArgument, "SecretRef may not be used with kubeapps managed secrets") - } - if secret == nil { - if auth.Type == corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON { - secret = newLocalDockerConfigJsonSecret(repoName) + if hadSecretTlsCa { + secret.Data["caFile"] = existingSecret.Data["caFile"] } else { - secret = newLocalOpaqueSecret(repoName) + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, unexpected REDACTED content") } + } else { + secret.Data["caFile"] = []byte(caCert) + isSameSecret = false + } + } else { + if hadSecretTlsCa { + isSameSecret = false } + } + + if auth != nil && auth.Type != corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_UNSPECIFIED { switch auth.Type { case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH: - if unp := auth.GetUsernamePassword(); unp != nil { - if unp.Username == redactedString && unp.Password == redactedString { - isSameSecret = true - } else { + unp := auth.GetUsernamePassword() + if unp == nil || unp.Username == "" || unp.Password == "" { + return nil, false, status.Errorf(codes.InvalidArgument, "Username/Password configuration is missing") + } + if (unp.Username == redactedString && !hadSecretUsername) || (unp.Password == redactedString && !hadSecretPassword) { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, unexpected REDACTED content") + } + + if existingSecret != nil { + secret.Data["username"] = existingSecret.Data["username"] + secret.Data["password"] = existingSecret.Data["password"] + } + if unp.Username != redactedString || unp.Password != redactedString { + isSameSecret = false + if unp.Username != redactedString { secret.Data["username"] = []byte(unp.Username) + } + if unp.Password != redactedString { secret.Data["password"] = []byte(unp.Password) } - } else { - return nil, false, status.Errorf(codes.Internal, "Username/Password configuration is missing") } case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_TLS: if repoType == sourcev1.HelmRepositoryTypeOCI { // ref https://fluxcd.io/flux/components/source/helmrepositories/#tls-authentication // Note: TLS authentication is not yet supported by OCI Helm repositories. return nil, false, status.Errorf(codes.Internal, "Package repository authentication type %q is not supported for OCI repositories", auth.Type) - } else { - if ck := auth.GetTlsCertKey(); ck != nil { - if ck.Cert == redactedString && ck.Key == redactedString { - isSameSecret = true - } else { - secret.Data["certFile"] = []byte(ck.Cert) - secret.Data["keyFile"] = []byte(ck.Key) - } - } else { - return nil, false, status.Errorf(codes.Internal, "TLS Cert/Key configuration is missing") - } } + ck := auth.GetTlsCertKey() + if ck == nil || ck.Cert == "" || ck.Key == "" { + return nil, false, status.Errorf(codes.InvalidArgument, "TLS Cert/Key configuration is missing") + } + if (ck.Cert == redactedString && !hadSecretTlsCert) || (ck.Key == redactedString && !hadSecretTlsKey) { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, unexpected REDACTED content") + } + + if existingSecret != nil { + secret.Data["certFile"] = existingSecret.Data["certFile"] + secret.Data["keyFile"] = existingSecret.Data["keyFile"] + } + if ck.Cert != redactedString || ck.Key != redactedString { + isSameSecret = false + if ck.Cert != redactedString { + secret.Data["certFile"] = []byte(ck.Cert) + } + if ck.Key != redactedString { + secret.Data["keyFile"] = []byte(ck.Key) + } + } case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON: - if repoType == sourcev1.HelmRepositoryTypeOCI { - if dc := auth.GetDockerCreds(); dc != nil { - if dc.Password == redactedString { - isSameSecret = true - } else { - secret.Type = apiv1.SecretTypeDockerConfigJson - dockerConfig := &credentialprovider.DockerConfigJSON{ - Auths: map[string]credentialprovider.DockerConfigEntry{ - dc.Server: { - Username: dc.Username, - Password: dc.Password, - Email: dc.Email, - }, - }, - } - dockerConfigJson, err := json.Marshal(dockerConfig) - if err != nil { - return nil, false, status.Errorf(codes.InvalidArgument, "Docker credentials are wrong") - } - secret.Data[apiv1.DockerConfigJsonKey] = dockerConfigJson - } + if repoType != sourcev1.HelmRepositoryTypeOCI { + return nil, false, status.Errorf(codes.Internal, "Unsupported package repository authentication type: %q", auth.Type) + } + + creds := auth.GetDockerCreds() + if creds == nil || creds.Server == "" || creds.Username == "" || creds.Password == "" { + return nil, false, status.Errorf(codes.InvalidArgument, "Docker credentials are missing") + } + if (creds.Server == redactedString || creds.Username == redactedString || creds.Password == redactedString || creds.Email == redactedString) && !hadSecretDocker { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid configuration, unexpected REDACTED content") + } + + secret.Type = apiv1.SecretTypeDockerConfigJson + if creds.Server == redactedString && creds.Username == redactedString && creds.Password == redactedString && creds.Email == redactedString { + secret.Data[apiv1.DockerConfigJsonKey] = existingSecret.Data[apiv1.DockerConfigJsonKey] + } else if creds.Server == redactedString || creds.Username == redactedString || creds.Password == redactedString || creds.Email == redactedString { + newcreds, err := decodeDockerAuth(existingSecret.Data[apiv1.DockerConfigJsonKey]) + if err != nil { + return nil, false, status.Errorf(codes.Internal, "Invalid configuration, the existing repository does not have valid docker authentication") + } + + if creds.Server != redactedString { + newcreds.Server = creds.Server + } + if creds.Username != redactedString { + newcreds.Username = creds.Username + } + if creds.Password != redactedString { + newcreds.Password = creds.Password + } + if creds.Email != redactedString { + newcreds.Email = creds.Email + } + + isSameSecret = false + if configjson, err := encodeDockerAuth(newcreds); err != nil { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid Docker credentials") } else { - return nil, false, status.Errorf(codes.Internal, "Docker credentials configuration is missing") + secret.Data[apiv1.DockerConfigJsonKey] = configjson } } else { - return nil, false, status.Errorf(codes.Internal, "Unsupported package repository authentication type: %q", auth.Type) + isSameSecret = false + if configjson, err := encodeDockerAuth(creds); err != nil { + return nil, false, status.Errorf(codes.InvalidArgument, "Invalid Docker credentials") + } else { + secret.Data[apiv1.DockerConfigJsonKey] = configjson + } } - case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, - corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_AUTHORIZATION_HEADER: - return nil, false, status.Errorf(codes.Internal, "Package repository authentication type %q is not supported", auth.Type) - case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_UNSPECIFIED: - return nil, true, nil default: - return nil, false, status.Errorf(codes.Internal, "Unsupported package repository authentication type: %q", auth.Type) + return nil, false, status.Errorf(codes.InvalidArgument, "Package repository authentication type %q is not supported", auth.Type) + } + } else { + // no authentication, check if it was removed + if hadSecretTlsCert || hadSecretTlsKey || hadSecretUsername || hadSecretPassword || hadSecretDocker { + isSameSecret = false } } return secret, isSameSecret, nil @@ -533,16 +587,32 @@ func newLocalOpaqueSecret(ownerRepo types.NamespacedName) *apiv1.Secret { } } -// "Local" in the sense of no namespace is specified -func newLocalDockerConfigJsonSecret(ownerRepo types.NamespacedName) *apiv1.Secret { - return &apiv1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: ownerRepo.Name + "-", - Annotations: map[string]string{ - annotationManagedByKey: annotationManagedByValue, +func encodeDockerAuth(credentials *corev1.DockerCredentials) ([]byte, error) { + config := &credentialprovider.DockerConfigJSON{ + Auths: map[string]credentialprovider.DockerConfigEntry{ + credentials.Server: { + Username: credentials.Username, + Password: credentials.Password, + Email: credentials.Email, }, }, - Type: apiv1.SecretTypeDockerConfigJson, - Data: map[string][]byte{}, } + return json.Marshal(config) +} + +func decodeDockerAuth(dockerjson []byte) (*corev1.DockerCredentials, error) { + config := &credentialprovider.DockerConfigJSON{} + if err := json.Unmarshal(dockerjson, config); err != nil { + return nil, err + } + for server, entry := range config.Auths { + docker := &corev1.DockerCredentials{ + Server: server, + Username: entry.Username, + Password: entry.Password, + Email: entry.Email, + } + return docker, nil + } + return nil, fmt.Errorf("invalid dockerconfig, no Auths entries were found") } diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go index 7b4fd2bcc5c..340e33c3fde 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go @@ -1287,12 +1287,12 @@ func TestAddPackageRepository(t *testing.T) { { name: "errors for package repository with bearer token", request: add_repo_req_10, - statusCode: codes.Internal, + statusCode: codes.InvalidArgument, }, { name: "errors for package repository with custom auth token", request: add_repo_req_11, - statusCode: codes.Internal, + statusCode: codes.InvalidArgument, }, { name: "package repository with docker config JSON authentication", @@ -1851,18 +1851,19 @@ func TestGetPackageRepositorySummaries(t *testing.T) { func TestUpdatePackageRepository(t *testing.T) { ca, pub, priv := getCertsForTesting(t) testCases := []struct { - name string - request *corev1.UpdatePackageRepositoryRequest - repoIndex string - repoName string - repoNamespace string - oldRepoSecret *apiv1.Secret - newRepoSecret *apiv1.Secret - pending bool - expectedStatusCode codes.Code - expectedResponse *corev1.UpdatePackageRepositoryResponse - expectedDetail *corev1.GetPackageRepositoryDetailResponse - userManagedSecrets bool + name string + request *corev1.UpdatePackageRepositoryRequest + repoIndex string + repoName string + repoNamespace string + oldRepoSecret *apiv1.Secret + newRepoSecret *apiv1.Secret + expectedCreatedSecret *apiv1.Secret + pending bool + expectedStatusCode codes.Code + expectedResponse *corev1.UpdatePackageRepositoryResponse + expectedDetail *corev1.GetPackageRepositoryDetailResponse + userManagedSecrets bool }{ { name: "update repository url", @@ -2007,12 +2008,17 @@ func TestUpdatePackageRepository(t *testing.T) { oldRepoSecret: setSecretManagedByKubeapps(setSecretOwnerRef( "repo-1", newBasicAuthSecret(types.NamespacedName{ - Name: "secret-1", + Name: "repo-1", Namespace: "namespace-1"}, "foo", "bar"))), request: update_repo_req_16, expectedStatusCode: codes.OK, expectedResponse: update_repo_resp_1, expectedDetail: update_repo_detail_15, + expectedCreatedSecret: setSecretManagedByKubeapps(setSecretOwnerRef( + "repo-1", + newBasicAuthSecret(types.NamespacedName{ + Name: "repo-1", + Namespace: "namespace-1"}, "foo", "bar"))), }, { name: "returns error when mix referenced secrets and user provided secret data", @@ -2047,6 +2053,61 @@ func TestUpdatePackageRepository(t *testing.T) { request: update_repo_req_21, expectedStatusCode: codes.InvalidArgument, }, + { + name: "issue5747 - update auth password: username was incorrectly overriden to redacted string", + repoIndex: testYaml("valid-index.yaml"), + repoName: "repo-1", + repoNamespace: "namespace-1", + oldRepoSecret: setSecretManagedByKubeapps(setSecretOwnerRef( + "repo-1", + newBasicAuthSecret(types.NamespacedName{ + Name: "repo-1", + Namespace: "namespace-1"}, "foo", "bar"))), + request: update_repo_req_22, + expectedStatusCode: codes.OK, + expectedResponse: update_repo_resp_1, + expectedDetail: update_repo_detail_18, + expectedCreatedSecret: setSecretManagedByKubeapps(setSecretOwnerRef( + "repo-1", + newBasicAuthSecret(types.NamespacedName{ + Name: "repo-1", + Namespace: "namespace-1"}, "foo", "doe"))), + }, + { + name: "issue5747 - update basic auth but not tls ca: basic auth updates are ignored", + repoIndex: testYaml("valid-index.yaml"), + repoName: "repo-1", + repoNamespace: "namespace-1", + oldRepoSecret: setSecretManagedByKubeapps(setSecretOwnerRef( + "repo-1", + newBasicAuthTlsSecret(types.NamespacedName{ + Name: "repo-1", + Namespace: "namespace-1"}, "foo", "bar", nil, nil, ca))), + request: update_repo_req_23, + expectedStatusCode: codes.OK, + expectedResponse: update_repo_resp_1, + expectedDetail: update_repo_detail_19, + expectedCreatedSecret: setSecretManagedByKubeapps(setSecretOwnerRef( + "repo-1", + newBasicAuthTlsSecret(types.NamespacedName{ + Name: "repo-1", + Namespace: "namespace-1"}, "john", "doe", nil, nil, ca))), + }, + { + name: "issue5747 - starts with no auth/tls, adding tls is being ignored", + repoIndex: testYaml("valid-index.yaml"), + repoName: "repo-1", + repoNamespace: "namespace-1", + request: update_repo_req_25(ca), + expectedStatusCode: codes.OK, + expectedResponse: update_repo_resp_1, + expectedDetail: update_repo_detail_20, + expectedCreatedSecret: setSecretManagedByKubeapps(setSecretOwnerRef( + "repo-1", + newTlsSecret(types.NamespacedName{ + Name: "repo-1", + Namespace: "namespace-1"}, nil, nil, ca))), + }, } for _, tc := range testCases { @@ -2135,12 +2196,43 @@ func TestUpdatePackageRepository(t *testing.T) { comparePackageRepositoryDetail(t, actualDetail, tc.expectedDetail) } - if !tc.userManagedSecrets && tc.oldRepoSecret != nil && actualDetail.Detail.Auth.Type == corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_UNSPECIFIED { - // check the secret's been deleted - if typedClient, err := s.clientGetter.Typed(ctx, s.kubeappsCluster); err != nil { + // ensures the secret has been created/updated correctly + if !tc.userManagedSecrets && (tc.oldRepoSecret != nil || tc.expectedCreatedSecret != nil) { + typedClient, err := s.clientGetter.Typed(ctx, s.kubeappsCluster) + if err != nil { t.Fatal(err) - } else if _, err = typedClient.CoreV1().Secrets(tc.repoNamespace).Get(ctx, tc.oldRepoSecret.Name, metav1.GetOptions{}); err == nil { - t.Fatalf("Expected secret [%q] to have been deleted", tc.oldRepoSecret.Name) + } + ctrlClient, err := s.clientGetter.ControllerRuntime(ctx, s.kubeappsCluster) + if err != nil { + t.Fatal(err) + } + + // check the secret has been deleted + if tc.oldRepoSecret != nil && tc.expectedCreatedSecret == nil { + if _, err = typedClient.CoreV1().Secrets(tc.repoNamespace).Get(ctx, tc.oldRepoSecret.Name, metav1.GetOptions{}); err == nil { + t.Fatalf("Expected secret [%q] to have been deleted", tc.oldRepoSecret.Name) + } + } + + // check the created/updated secret + if tc.expectedCreatedSecret != nil { + var actualRepo sourcev1.HelmRepository + if err = ctrlClient.Get(ctx, types.NamespacedName{Namespace: tc.repoNamespace, Name: tc.repoName}, &actualRepo); err != nil { + t.Fatal(err) + } + if actualRepo.Spec.SecretRef == nil { + t.Fatalf("Expected repo to have a secret ref, none found") + } + + opt2 := cmpopts.IgnoreFields(metav1.ObjectMeta{}, "Name", "GenerateName") + if secret, err := typedClient.CoreV1().Secrets(tc.repoNamespace).Get(ctx, actualRepo.Spec.SecretRef.Name, metav1.GetOptions{}); err != nil { + t.Fatal(err) + } else if got, want := secret, tc.expectedCreatedSecret; !cmp.Equal(want, got, opt2) { + t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, opt2)) + } else if !strings.HasPrefix(secret.Name, tc.expectedCreatedSecret.Name) { + t.Errorf("Secret Name [%s] was expected to start with [%s]", secret.Name, tc.expectedCreatedSecret.Name) + } + } } }) diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/globals_vars_test.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/globals_vars_test.go index 82533b4ab4e..3d8d2dda460 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/globals_vars_test.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/globals_vars_test.go @@ -359,12 +359,7 @@ var ( }, } - addRepoReqBearerToken = func(token string, withPrefix bool) *corev1.AddPackageRepositoryRequest { - prefixedToken := token - if withPrefix { - prefixedToken = "Bearer " + token - } - + addRepoReqBearerToken = func(token string) *corev1.AddPackageRepositoryRequest { return &corev1.AddPackageRepositoryRequest{ Name: "bar", Context: &corev1.Context{Namespace: "foo", Cluster: KubeappsCluster}, @@ -374,7 +369,7 @@ var ( Auth: &corev1.PackageRepositoryAuth{ Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_Header{ - Header: prefixedToken, + Header: token, }, }, } diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_auth.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_auth.go index 4c0fe51caa0..53019faf7af 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_auth.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_auth.go @@ -254,7 +254,7 @@ func newSecretFromTlsConfigAndAuth(repoName string, } } - if auth != nil { + if auth != nil && auth.Type != corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_UNSPECIFIED { switch auth.Type { case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH: unp := auth.GetUsernamePassword() @@ -365,6 +365,7 @@ func newSecretFromTlsConfigAndAuth(repoName string, return nil, false, status.Errorf(codes.InvalidArgument, "Package repository authentication type %q is not supported", auth.Type) } } else { + // no authentication, check if it was removed if hadSecretHeader || hadSecretDocker { isSameSecret = false } @@ -575,23 +576,12 @@ func validateUserManagedRepoSecret( tlsConfig *corev1.PackageRepositoryTlsConfig, auth *corev1.PackageRepositoryAuth) (*k8scorev1.Secret, error) { var secretRefTls, secretRefAuth string - if tlsConfig != nil { - if tlsConfig.GetCertAuthority() != "" { - return nil, status.Errorf(codes.InvalidArgument, "Secret Ref must be used with user managed secrets") - } else if tlsConfig.GetSecretRef().GetName() != "" { - secretRefTls = tlsConfig.GetSecretRef().GetName() - } - } - if auth != nil { - if auth.GetDockerCreds() != nil || - auth.GetHeader() != "" || - auth.GetTlsCertKey() != nil || - auth.GetUsernamePassword() != nil { - return nil, status.Errorf(codes.InvalidArgument, "Secret Ref must be used with user managed secrets") - } else if auth.GetSecretRef().GetName() != "" { - secretRefAuth = auth.GetSecretRef().GetName() - } + if tlsConfig != nil && tlsConfig.GetSecretRef() != nil { + secretRefTls = tlsConfig.GetSecretRef().GetName() + } + if auth != nil && auth.GetSecretRef() != nil { + secretRefAuth = auth.GetSecretRef().GetName() } var secretRef string @@ -620,18 +610,30 @@ func validateUserManagedRepoSecret( } if secretRefAuth != "" { switch auth.Type { - case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, - corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_AUTHORIZATION_HEADER, - corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH: - if secret.Data[SecretAuthHeaderKey] == nil { + case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH: + if data := secret.Data[SecretAuthHeaderKey]; data == nil { + return nil, status.Errorf(codes.Internal, "Specified secret [%s] missing key '%s'", secretRef, SecretAuthHeaderKey) + } else if _, _, ok := decodeBasicAuth(string(data)); !ok { + return nil, status.Errorf(codes.Internal, "Specified secret [%s] does not represent a valid Basic Auth secret'", secretRef) + } + case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER: + if data := secret.Data[SecretAuthHeaderKey]; data == nil { + return nil, status.Errorf(codes.Internal, "Specified secret [%s] missing key '%s'", secretRef, SecretAuthHeaderKey) + } else if _, ok := decodeBearerAuth(string(data)); !ok { + return nil, status.Errorf(codes.Internal, "Specified secret [%s] does not represent a valid Bearer Auth secret'", secretRef) + } + case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_AUTHORIZATION_HEADER: + if data := secret.Data[SecretAuthHeaderKey]; data == nil { return nil, status.Errorf(codes.Internal, "Specified secret [%s] missing key '%s'", secretRef, SecretAuthHeaderKey) } case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON: - if secret.Data[DockerConfigJsonKey] == nil { + if secret.Type != k8scorev1.SecretTypeDockerConfigJson { + return nil, status.Errorf(codes.Internal, "Specified secret [%s] does not have expected dockerconfig type", secretRef) + } else if _, ok := secret.Data[k8scorev1.DockerConfigJsonKey]; !ok { return nil, status.Errorf(codes.Internal, "Specified secret [%s] missing key '%s'", secretRef, DockerConfigJsonKey) } default: - return nil, status.Errorf(codes.Internal, "Package repository authentication type %q is not supported", auth.Type) + return nil, status.Errorf(codes.InvalidArgument, "Package repository authentication type %q is not supported", auth.Type) } } } @@ -827,7 +829,15 @@ func decodeBasicAuth(auth string) (username string, password string, ok bool) { } func encodeBearerAuth(token string) string { - return "Bearer " + strings.TrimPrefix(token, "Bearer ") + return "Bearer " + token +} + +func decodeBearerAuth(auth string) (token string, ok bool) { + if strings.HasPrefix(auth, "Bearer ") { + return strings.TrimPrefix(auth, "Bearer "), true + } else { + return "", false + } } func encodeDockerAuth(credentials *corev1.DockerCredentials) ([]byte, error) { diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go index 817e9208b95..7a0b21a5732 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go @@ -406,17 +406,8 @@ func TestAddPackageRepository(t *testing.T) { }, // BEARER TOKEN { - name: "package repository with bearer token w/o prefix", - request: addRepoReqBearerToken("the-token", false), - expectedResponse: addRepoExpectedResp, - expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"), - expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "Bearer the-token"))), - expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"), - statusCode: codes.OK, - }, - { - name: "package repository with bearer token w/ prefix", - request: addRepoReqBearerToken("the-token", true), + name: "package repository with bearer token", + request: addRepoReqBearerToken("the-token"), expectedResponse: addRepoExpectedResp, expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"), expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "Bearer the-token"))), @@ -425,7 +416,7 @@ func TestAddPackageRepository(t *testing.T) { }, { name: "package repository with no bearer token", - request: addRepoReqBearerToken("", false), + request: addRepoReqBearerToken(""), statusCode: codes.InvalidArgument, }, { From 7332d4fb43777e3dfccc259bd89f31a51f6bd040 Mon Sep 17 00:00:00 2001 From: Dimitri Laloue Date: Fri, 2 Dec 2022 10:05:39 -0800 Subject: [PATCH 3/8] carvel repos now allow changing auth type --- .../packages/v1alpha1/global_vars_test.go | 8 +- .../fluxv2/packages/v1alpha1/repo_test.go | 2 +- .../packages/v1alpha1/repositories_test.go | 112 ++++++- .../helm/packages/v1alpha1/test_util_test.go | 12 +- .../packages/v1alpha1/server_data_adapters.go | 48 +-- .../packages/v1alpha1/server_test.go | 310 +++++++++--------- 6 files changed, 279 insertions(+), 213 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go index 89df2c29997..14983ade251 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go @@ -3531,13 +3531,7 @@ var ( Auth: basic_auth("john", "doe"), } - update_repo_req_24 = &corev1.UpdatePackageRepositoryRequest{ - PackageRepoRef: repoRefInReq("repo-1", "namespace-1"), - Url: "http://url.com", - Auth: basic_auth("john", "doe"), - } - - update_repo_req_25 = func(ca []byte) *corev1.UpdatePackageRepositoryRequest { + update_repo_req_24 = func(ca []byte) *corev1.UpdatePackageRepositoryRequest { return &corev1.UpdatePackageRepositoryRequest{ PackageRepoRef: repoRefInReq("repo-1", "namespace-1"), Url: "http://url.com", diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go index 340e33c3fde..b8c159e4898 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go @@ -2098,7 +2098,7 @@ func TestUpdatePackageRepository(t *testing.T) { repoIndex: testYaml("valid-index.yaml"), repoName: "repo-1", repoNamespace: "namespace-1", - request: update_repo_req_25(ca), + request: update_repo_req_24(ca), expectedStatusCode: codes.OK, expectedResponse: update_repo_resp_1, expectedDetail: update_repo_detail_20, diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go index 7a0b21a5732..a564111d5b4 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/repositories_test.go @@ -183,11 +183,7 @@ var repo7 = &appRepov1alpha1.AppRepository{ Header: &appRepov1alpha1.AppRepositoryAuthHeader{ SecretKeyRef: apiv1.SecretKeySelector{LocalObjectReference: apiv1.LocalObjectReference{Name: helm.SecretNameForRepo("repo-7")}, Key: "authorizationHeader"}, }, - CustomCA: &appRepov1alpha1.AppRepositoryCustomCA{ - SecretKeyRef: apiv1.SecretKeySelector{LocalObjectReference: apiv1.LocalObjectReference{Name: helm.SecretNameForRepo("repo-7")}, Key: "ca.crt"}, - }, }, - DockerRegistrySecrets: []string{imagesPullSecretName("repo-7")}, }, } @@ -410,8 +406,8 @@ func TestAddPackageRepository(t *testing.T) { request: addRepoReqBearerToken("the-token"), expectedResponse: addRepoExpectedResp, expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"), - expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "Bearer the-token"))), - expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"), + expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newBearerAuthSecret("apprepo-bar", "foo", "the-token"))), + expectedGlobalSecret: newBearerAuthSecret("foo-apprepo-bar", kubeappsNamespace, "the-token"), statusCode: codes.OK, }, { @@ -423,10 +419,10 @@ func TestAddPackageRepository(t *testing.T) { name: "package repository bearer token with secret (user managed secrets)", request: addRepoReqAuthWithSecret(corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, "foo", "secret-bearer"), userManagedSecrets: true, - existingAuthSecret: newAuthTokenSecret("secret-bearer", "foo", "Bearer the-token"), + existingAuthSecret: newBearerAuthSecret("secret-bearer", "foo", "the-token"), expectedResponse: addRepoExpectedResp, expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "secret-bearer"), - expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "Bearer the-token"), + expectedGlobalSecret: newBearerAuthSecret("foo-apprepo-bar", kubeappsNamespace, "the-token"), statusCode: codes.OK, }, // CUSTOM AUTH @@ -435,8 +431,8 @@ func TestAddPackageRepository(t *testing.T) { request: addRepoReqCustomAuth, expectedResponse: addRepoExpectedResp, expectedRepo: addRepoAuthHeaderWithSecretRef("foo", "apprepo-bar"), - expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newAuthTokenSecret("apprepo-bar", "foo", "foobarzot"))), - expectedGlobalSecret: newAuthTokenSecret("foo-apprepo-bar", kubeappsNamespace, "foobarzot"), + expectedAuthCreatedSecret: setSecretAnnotations(setSecretOwnerRef("bar", newHeaderAuthSecret("apprepo-bar", "foo", "foobarzot"))), + expectedGlobalSecret: newHeaderAuthSecret("foo-apprepo-bar", kubeappsNamespace, "foobarzot"), statusCode: codes.OK, }, { @@ -630,7 +626,7 @@ func TestAddPackageRepository(t *testing.T) { }), expectedResponse: addRepoExpectedResp, userManagedSecrets: true, - existingAuthSecret: newAuthTokenSecret("secret-docker", "foo", ""), + existingAuthSecret: newHeaderAuthSecret("secret-docker", "foo", ""), statusCode: codes.InvalidArgument, }, { @@ -1327,8 +1323,8 @@ func TestUpdatePackageRepository(t *testing.T) { return &repository }, expectedRef: defaultRef, - expectedAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-1", newAuthTokenSecret("apprepo-repo-1", "ns-1", "Bearer foobarzot"))), - expectedGlobalSecret: newAuthTokenSecret("ns-1-apprepo-repo-1", kubeappsNamespace, "Bearer foobarzot"), + expectedAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-1", newBearerAuthSecret("apprepo-repo-1", "ns-1", "foobarzot"))), + expectedGlobalSecret: newBearerAuthSecret("ns-1-apprepo-repo-1", kubeappsNamespace, "foobarzot"), expectedStatusCode: codes.OK, }, { @@ -1345,7 +1341,7 @@ func TestUpdatePackageRepository(t *testing.T) { return request }, userManagedSecrets: true, - existingAuthSecret: newAuthTokenSecret("my-own-secret", "ns-1", "Bearer foobarzot"), + existingAuthSecret: newBearerAuthSecret("my-own-secret", "ns-1", "foobarzot"), expectedRepoCustomizer: func(repository appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository { repository.ResourceVersion = "2" repository.Spec.Auth = appRepov1alpha1.AppRepositoryAuth{ @@ -1361,12 +1357,12 @@ func TestUpdatePackageRepository(t *testing.T) { return &repository }, expectedRef: defaultRef, - expectedGlobalSecret: newAuthTokenSecret("ns-1-apprepo-repo-1", kubeappsNamespace, "Bearer foobarzot"), + expectedGlobalSecret: newBearerAuthSecret("ns-1-apprepo-repo-1", kubeappsNamespace, "foobarzot"), expectedStatusCode: codes.OK, }, { name: "update removing auth", - existingAuthSecret: newAuthTokenSecret(helm.SecretNameForRepo("repo-3"), globalPackagingNamespace, "token-value"), + existingAuthSecret: newBearerAuthSecret(helm.SecretNameForRepo("repo-3"), globalPackagingNamespace, "token-value"), requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { request.PackageRepoRef = &corev1.PackageRepositoryReference{ Plugin: &pluginDetail, @@ -1554,6 +1550,90 @@ func TestUpdatePackageRepository(t *testing.T) { Identifier: "repo-5", }, }, + { + name: "[kubeapps managed secrets] update repo auth basic to token", + existingAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-7", + newBasicAuthSecret("apprepo-repo-7", "ns-7", "foo", "bar"))), + + requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { + request.PackageRepoRef = &corev1.PackageRepositoryReference{ + Plugin: &pluginDetail, + Context: &corev1.Context{Namespace: "ns-7", Cluster: KubeappsCluster}, + Identifier: "repo-7", + } + request.Url = repo7.Spec.URL + request.Auth = &corev1.PackageRepositoryAuth{ + Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, + PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_Header{ + Header: "zot", + }, + } + return request + }, + + expectedStatusCode: codes.OK, + expectedRepoCustomizer: func(repository appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository { + repository.ResourceVersion = "2" + repository.Namespace = "ns-7" + repository.Name = "repo-7" + repository.Spec = repo7.Spec + return &repository + }, + expectedRef: &corev1.PackageRepositoryReference{ + Plugin: &pluginDetail, + Context: &corev1.Context{Namespace: "ns-7", Cluster: KubeappsCluster}, + Identifier: "repo-7", + }, + expectedAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-7", + newBearerAuthSecret("apprepo-repo-7", "ns-7", "zot"))), + expectedGlobalSecret: newBearerAuthSecret("ns-7-apprepo-repo-7", kubeappsNamespace, "zot"), + }, + { + name: "[kubeapps managed secrets] update repo auth basic to docker", + existingAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-7", + newBasicAuthSecret("apprepo-repo-7", "ns-7", "foo", "bar"))), + + requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { + request.PackageRepoRef = &corev1.PackageRepositoryReference{ + Plugin: &pluginDetail, + Context: &corev1.Context{Namespace: "ns-7", Cluster: KubeappsCluster}, + Identifier: "repo-7", + } + request.Url = repo7.Spec.URL + request.Auth = &corev1.PackageRepositoryAuth{ + Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON, + PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_DockerCreds{ + DockerCreds: &corev1.DockerCredentials{ + Server: "sample.com", + Username: "foo", + Password: "bar", + Email: "user@sample.com", + }, + }, + } + return request + }, + + expectedStatusCode: codes.OK, + expectedRepoCustomizer: func(repository appRepov1alpha1.AppRepository) *appRepov1alpha1.AppRepository { + repository.ResourceVersion = "2" + repository.Namespace = "ns-7" + repository.Name = "repo-7" + repository.Spec = repo7.Spec + repository.Spec.Auth.Header.SecretKeyRef.Key = DockerConfigJsonKey + return &repository + }, + expectedRef: &corev1.PackageRepositoryReference{ + Plugin: &pluginDetail, + Context: &corev1.Context{Namespace: "ns-7", Cluster: KubeappsCluster}, + Identifier: "repo-7", + }, + expectedAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-7", + newAuthDockerSecret("apprepo-repo-7", "ns-7", + dockerAuthJson("sample.com", "foo", "bar", "user@sample.com", "Zm9vOmJhcg==")))), + expectedGlobalSecret: newAuthDockerSecret("ns-7-apprepo-repo-7", kubeappsNamespace, + dockerAuthJson("sample.com", "foo", "bar", "user@sample.com", "Zm9vOmJhcg==")), + }, { name: "[issue 5746] secret updates ignored if not all credentials are provided - auth updates", existingAuthSecret: setSecretAnnotations(setSecretOwnerRef("repo-6", diff --git a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/test_util_test.go b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/test_util_test.go index 307cab2ff5a..624a2a89ec2 100644 --- a/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/test_util_test.go +++ b/cmd/kubeapps-apis/plugins/helm/packages/v1alpha1/test_util_test.go @@ -73,15 +73,17 @@ func lessPackageRepositorySummaryFunc(p1, p2 *corev1.PackageRepositorySummary) b return p1.Name < p2.Name } -// ref: https://kubernetes.io/docs/concepts/configuration/secret/#basic-authentication-secret func newBasicAuthSecret(name, namespace, username, password string) *apiv1.Secret { authString := fmt.Sprintf("%s:%s", username, password) authHeader := fmt.Sprintf("Basic %s", base64.StdEncoding.EncodeToString([]byte(authString))) - return newAuthTokenSecret(name, namespace, authHeader) + return newHeaderAuthSecret(name, namespace, authHeader) } -// ref: https://kubernetes.io/docs/concepts/configuration/secret/#basic-authentication-secret -func newAuthTokenSecret(name, namespace, token string) *apiv1.Secret { +func newBearerAuthSecret(name, namespace, token string) *apiv1.Secret { + return newHeaderAuthSecret(name, namespace, "Bearer "+token) +} + +func newHeaderAuthSecret(name, namespace, header string) *apiv1.Secret { return &apiv1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -89,7 +91,7 @@ func newAuthTokenSecret(name, namespace, token string) *apiv1.Secret { }, Type: apiv1.SecretTypeOpaque, Data: map[string][]byte{ - "authorizationHeader": []byte(token), + "authorizationHeader": []byte(header), }, } } diff --git a/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_adapters.go b/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_adapters.go index 091846fd2ef..3d590d381e5 100644 --- a/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_adapters.go +++ b/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_adapters.go @@ -915,28 +915,6 @@ func (s *Server) validatePackageRepositoryAuth(ctx context.Context, cluster, nam } } - // validate the type is not changed - if pkgRepository != nil && pkgSecret != nil && isPluginManaged(pkgRepository, pkgSecret) { - switch auth.Type { - case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH: - if !isBasicAuth(pkgSecret) { - return status.Errorf(codes.InvalidArgument, "auth type cannot be changed") - } - case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_SSH: - if !isSshAuth(pkgSecret) { - return status.Errorf(codes.InvalidArgument, "auth type cannot be changed") - } - case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON: - if !isDockerAuth(pkgSecret) { - return status.Errorf(codes.InvalidArgument, "auth type cannot be changed") - } - case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER: - if !isBearerAuth(pkgSecret) { - return status.Errorf(codes.InvalidArgument, "auth type cannot be changed") - } - } - } - // validate referenced secret matches type if auth.GetSecretRef() != nil { name := auth.GetSecretRef().Name @@ -974,14 +952,14 @@ func (s *Server) validatePackageRepositoryAuth(ctx context.Context, cluster, nam // validate auth data // ensures the expected credential struct is provided - // for new auth, credentials can't have Redacted content + // for new auth or new auth type, credentials can't have Redacted content switch auth.Type { case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH: up := auth.GetUsernamePassword() if up == nil || up.Username == "" || up.Password == "" { return status.Errorf(codes.InvalidArgument, "missing basic auth credentials") } - if pkgSecret == nil { + if pkgSecret == nil || !isBasicAuth(pkgSecret) { if up.Username == Redacted || up.Password == Redacted { return status.Errorf(codes.InvalidArgument, "invalid auth, unexpected REDACTED content") } @@ -991,7 +969,7 @@ func (s *Server) validatePackageRepositoryAuth(ctx context.Context, cluster, nam if ssh == nil || ssh.PrivateKey == "" { return status.Errorf(codes.InvalidArgument, "missing SSH auth credentials") } - if pkgSecret == nil { + if pkgSecret == nil || !isSshAuth(pkgSecret) { if ssh.PrivateKey == Redacted || ssh.KnownHosts == Redacted { return status.Errorf(codes.InvalidArgument, "invalid auth, unexpected REDACTED content") } @@ -1001,7 +979,7 @@ func (s *Server) validatePackageRepositoryAuth(ctx context.Context, cluster, nam if docker == nil || docker.Username == "" || docker.Password == "" || docker.Server == "" { return status.Errorf(codes.InvalidArgument, "missing Docker Config auth credentials") } - if pkgSecret == nil { + if pkgSecret == nil || !isDockerAuth(pkgSecret) { if docker.Username == Redacted || docker.Password == Redacted || docker.Server == Redacted || docker.Email == Redacted { return status.Errorf(codes.InvalidArgument, "invalid auth, unexpected REDACTED content") } @@ -1011,7 +989,7 @@ func (s *Server) validatePackageRepositoryAuth(ctx context.Context, cluster, nam if token == "" { return status.Errorf(codes.InvalidArgument, "missing Token auth credentials") } - if pkgSecret == nil { + if pkgSecret == nil || !isBearerAuth(pkgSecret) { if token == Redacted { return status.Errorf(codes.InvalidArgument, "invalid auth, unexpected REDACTED content") } @@ -1059,11 +1037,9 @@ func (s *Server) buildPkgRepositorySecretCreate(namespace, name string, auth *co case corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER: pkgSecret.Type = k8scorev1.SecretTypeOpaque - if token := auth.GetHeader(); token != "" { - pkgSecret.StringData[BearerAuthToken] = "Bearer " + strings.TrimPrefix(token, "Bearer ") - } else { - return nil, status.Errorf(codes.InvalidArgument, "Bearer token is missing") - } + + token := auth.GetHeader() + pkgSecret.StringData[BearerAuthToken] = token } return pkgSecret, nil @@ -1098,8 +1074,10 @@ func (s *Server) buildPkgRepositorySecretUpdate(pkgSecret *k8scorev1.Secret, aut if ssh.PrivateKey == Redacted && ssh.KnownHosts == Redacted { return false, nil } - if ssh.KnownHosts != Redacted { + if ssh.PrivateKey != Redacted { pkgSecret.StringData[k8scorev1.SSHAuthPrivateKey] = ssh.PrivateKey + } + if ssh.KnownHosts != Redacted { pkgSecret.StringData[SSHAuthKnownHosts] = ssh.KnownHosts } } else { @@ -1154,12 +1132,12 @@ func (s *Server) buildPkgRepositorySecretUpdate(pkgSecret *k8scorev1.Secret, aut if token == Redacted { return false, nil } else { - pkgSecret.StringData[BearerAuthToken] = "Bearer " + strings.TrimPrefix(token, "Bearer ") + pkgSecret.StringData[BearerAuthToken] = token } } else { pkgSecret.Type = k8scorev1.SecretTypeOpaque pkgSecret.Data = nil - pkgSecret.StringData[BearerAuthToken] = "Bearer " + strings.TrimPrefix(token, "Bearer ") + pkgSecret.StringData[BearerAuthToken] = token } } diff --git a/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go b/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go index 5811b9057ce..e2c91543ce5 100644 --- a/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go +++ b/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go @@ -7070,37 +7070,7 @@ func TestAddPackageRepository(t *testing.T) { }, }, { - name: "create with auth (plugin managed, bearer auth w/ Bearer prefix)", - requestCustomizer: func(request *corev1.AddPackageRepositoryRequest) *corev1.AddPackageRepositoryRequest { - request.Auth = &corev1.PackageRepositoryAuth{ - Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, - PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_Header{ - Header: "Bearer foo", - }, - } - return request - }, - repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { - repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{} // the name will be empty as the fake client does not handle generating names - return repository - }, - expectedStatusCode: codes.OK, - expectedRef: defaultRef, - customChecks: func(t *testing.T, s *Server) { - secret, err := s.getSecret(context.Background(), defaultGlobalContext.Cluster, demoGlobalPackagingNamespace, "") - if err != nil { - t.Fatalf("error fetching newly created secret:%+v", err) - } - if !isPluginManaged(defaultRepository(), secret) { - t.Errorf("annotations and ownership was not properly set: %+v", secret) - } - if secret.Type != k8scorev1.SecretTypeOpaque || secret.StringData[BearerAuthToken] != "Bearer foo" { - t.Errorf("secret data was not properly constructed: %+v", secret) - } - }, - }, - { - name: "create with auth (plugin managed, bearer auth w/o Bearer prefix)", + name: "create with auth (plugin managed, bearer auth)", requestCustomizer: func(request *corev1.AddPackageRepositoryRequest) *corev1.AddPackageRepositoryRequest { request.Auth = &corev1.PackageRepositoryAuth{ Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, @@ -7124,7 +7094,7 @@ func TestAddPackageRepository(t *testing.T) { if !isPluginManaged(defaultRepository(), secret) { t.Errorf("annotations and ownership was not properly set: %+v", secret) } - if secret.Type != k8scorev1.SecretTypeOpaque || secret.StringData[BearerAuthToken] != "Bearer foo" { + if secret.Type != k8scorev1.SecretTypeOpaque || secret.StringData[BearerAuthToken] != "foo" { t.Errorf("secret data was not properly constructed: %+v", secret) } }, @@ -7249,7 +7219,7 @@ func TestUpdatePackageRepository(t *testing.T) { defaultRepository := func() *packagingv1alpha1.PackageRepository { return &packagingv1alpha1.PackageRepository{ TypeMeta: defaultTypeMeta, - ObjectMeta: metav1.ObjectMeta{Name: "globalrepo", Namespace: defaultGlobalContext.Namespace}, + ObjectMeta: metav1.ObjectMeta{Name: "globalrepo", Namespace: defaultGlobalContext.Namespace, UID: "globalrepo"}, Spec: packagingv1alpha1.PackageRepositorySpec{ SyncPeriod: &metav1.Duration{Duration: time.Duration(24) * time.Hour}, Fetch: &packagingv1alpha1.PackageRepositoryFetch{ @@ -7262,25 +7232,43 @@ func TestUpdatePackageRepository(t *testing.T) { } } - defaultSecret := func() *k8scorev1.Secret { - return &k8scorev1.Secret{ + defaultSecret := func(name string, managed bool) *k8scorev1.Secret { + secret := &k8scorev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Namespace: defaultGlobalContext.Namespace, - Name: "my-secret", - Annotations: map[string]string{Annotation_ManagedBy_Key: Annotation_ManagedBy_Value}, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: defaultTypeMeta.APIVersion, - Kind: defaultTypeMeta.Kind, - Name: "globalrepo", - UID: "globalrepo", - Controller: func() *bool { v := true; return &v }(), - }, - }, + Namespace: defaultGlobalContext.Namespace, + Name: name, }, Type: k8scorev1.SecretTypeOpaque, Data: map[string][]byte{}, } + if managed { + secret.ObjectMeta.Annotations = map[string]string{Annotation_ManagedBy_Key: Annotation_ManagedBy_Value} + secret.ObjectMeta.OwnerReferences = []metav1.OwnerReference{ + { + APIVersion: defaultTypeMeta.APIVersion, + Kind: defaultTypeMeta.Kind, + Name: "globalrepo", + UID: "globalrepo", + Controller: func() *bool { v := true; return &v }(), + }, + } + } + return secret + } + basicAuthSecret := func(secret *k8scorev1.Secret, username string, password string) *k8scorev1.Secret { + secret.Type = k8scorev1.SecretTypeBasicAuth + secret.Data = map[string][]byte{k8scorev1.BasicAuthUsernameKey: []byte(username), k8scorev1.BasicAuthPasswordKey: []byte(password)} + return secret + } + tokenAuthSecret := func(secret *k8scorev1.Secret, token string) *k8scorev1.Secret { + secret.Type = k8scorev1.SecretTypeOpaque + secret.Data = map[string][]byte{BearerAuthToken: []byte(token)} + return secret + } + dockerAuthSecret := func(secret *k8scorev1.Secret, dockerconfig string) *k8scorev1.Secret { + secret.Type = k8scorev1.SecretTypeDockerConfigJson + secret.Data = map[string][]byte{k8scorev1.DockerConfigJsonKey: []byte(dockerconfig)} + return secret } testCases := []struct { @@ -7346,8 +7334,10 @@ func TestUpdatePackageRepository(t *testing.T) { expectedStatusString: "Auth Type is incompatible", }, { - name: "validate auth (mode incompatibility)", - existingTypedObjects: []k8sruntime.Object{defaultSecret()}, + name: "validate auth (mode incompatibility)", + existingTypedObjects: []k8sruntime.Object{ + basicAuthSecret(defaultSecret("my-secret", false), "foo", "bar"), + }, initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{ Name: "my-secret", @@ -7396,10 +7386,7 @@ func TestUpdatePackageRepository(t *testing.T) { { name: "validate auth (user managed, secret is incompatible)", existingTypedObjects: []k8sruntime.Object{ - &k8scorev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: defaultGlobalContext.Namespace, Name: "my-secret"}, - Data: map[string][]byte{k8scorev1.BasicAuthUsernameKey: []byte("foo"), k8scorev1.BasicAuthPasswordKey: []byte("bar")}, - }, + basicAuthSecret(defaultSecret("my-secret", false), "foo", "bar"), }, requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { request.Auth = &corev1.PackageRepositoryAuth{ @@ -7444,36 +7431,6 @@ func TestUpdatePackageRepository(t *testing.T) { expectedStatusCode: codes.InvalidArgument, expectedStatusString: "unexpected REDACTED", }, - { - name: "validate (plugin managed, type changed)", - existingTypedObjects: []k8sruntime.Object{ - func() *k8scorev1.Secret { - s := defaultSecret() - s.Type = k8scorev1.SecretTypeBasicAuth - s.Data[k8scorev1.BasicAuthUsernameKey] = []byte("foo") - s.Data[k8scorev1.BasicAuthPasswordKey] = []byte("bar") - return s - }(), - }, - initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { - repository.ObjectMeta.UID = "globalrepo" - repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{ - Name: "my-secret", - } - return repository - }, - requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { - request.Auth = &corev1.PackageRepositoryAuth{ - Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, - PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_Header{ - Header: "eXYZ", - }, - } - return request - }, - expectedStatusCode: codes.InvalidArgument, - expectedStatusString: "type cannot be changed", - }, { name: "validate not found", requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { @@ -7748,10 +7705,7 @@ func TestUpdatePackageRepository(t *testing.T) { { name: "updated with auth (user managed, added)", existingTypedObjects: []k8sruntime.Object{ - &k8scorev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: defaultGlobalContext.Namespace, Name: "my-secret"}, - Data: map[string][]byte{k8scorev1.BasicAuthUsernameKey: []byte("foo"), k8scorev1.BasicAuthPasswordKey: []byte("bar")}, - }, + basicAuthSecret(defaultSecret("my-secret", false), "foo", "bar"), }, requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { request.Auth = &corev1.PackageRepositoryAuth{ @@ -7776,14 +7730,8 @@ func TestUpdatePackageRepository(t *testing.T) { { name: "updated with auth (user managed, updated)", existingTypedObjects: []k8sruntime.Object{ - &k8scorev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: defaultGlobalContext.Namespace, Name: "my-secret"}, - Data: map[string][]byte{k8scorev1.BasicAuthUsernameKey: []byte("foo"), k8scorev1.BasicAuthPasswordKey: []byte("bar")}, - }, - &k8scorev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: defaultGlobalContext.Namespace, Name: "my-secret-2"}, - Data: map[string][]byte{k8scorev1.DockerConfigJsonKey: []byte("{}")}, - }, + basicAuthSecret(defaultSecret("my-secret", false), "foo", "bar"), + dockerAuthSecret(defaultSecret("my-secret-2", false), "{}"), }, initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{ @@ -7814,10 +7762,7 @@ func TestUpdatePackageRepository(t *testing.T) { { name: "updated with auth (user managed, removed)", existingTypedObjects: []k8sruntime.Object{ - &k8scorev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Namespace: defaultGlobalContext.Namespace, Name: "my-secret"}, - Data: map[string][]byte{k8scorev1.BasicAuthUsernameKey: []byte("foo"), k8scorev1.BasicAuthPasswordKey: []byte("bar")}, - }, + basicAuthSecret(defaultSecret("my-secret", false), "foo", "bar"), }, initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{ @@ -7866,37 +7811,7 @@ func TestUpdatePackageRepository(t *testing.T) { }, }, { - name: "updated with auth (plugin managed, bearer auth w/ Bearer prefix)", - requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { - request.Auth = &corev1.PackageRepositoryAuth{ - Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, - PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_Header{ - Header: "Bearer foo", - }, - } - return request - }, - repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { - repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{} // the name will be empty as the fake client does not handle generating names - return repository - }, - expectedStatusCode: codes.OK, - expectedRef: defaultRef, - customChecks: func(t *testing.T, s *Server) { - secret, err := s.getSecret(context.Background(), defaultGlobalContext.Cluster, demoGlobalPackagingNamespace, "") - if err != nil { - t.Fatalf("error fetching newly created secret:%+v", err) - } - if !isPluginManaged(defaultRepository(), secret) { - t.Errorf("annotations and ownership was not properly set: %+v", secret) - } - if secret.Type != k8scorev1.SecretTypeOpaque || secret.StringData[BearerAuthToken] != "Bearer foo" { - t.Errorf("secret data was not properly constructed: %+v", secret) - } - }, - }, - { - name: "updated with auth (plugin managed, bearer auth w/o Bearer prefix)", + name: "updated with auth (plugin managed, bearer auth)", requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { request.Auth = &corev1.PackageRepositoryAuth{ Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, @@ -7920,14 +7835,16 @@ func TestUpdatePackageRepository(t *testing.T) { if !isPluginManaged(defaultRepository(), secret) { t.Errorf("annotations and ownership was not properly set: %+v", secret) } - if secret.Type != k8scorev1.SecretTypeOpaque || secret.StringData[BearerAuthToken] != "Bearer foo" { + if secret.Type != k8scorev1.SecretTypeOpaque || secret.StringData[BearerAuthToken] != "foo" { t.Errorf("secret data was not properly constructed: %+v", secret) } }, }, { - name: "updated with auth (plugin managed, removed)", - existingTypedObjects: []k8sruntime.Object{defaultSecret()}, + name: "updated with auth (plugin managed, removed)", + existingTypedObjects: []k8sruntime.Object{ + basicAuthSecret(defaultSecret("my-secret", true), "foo", "bar"), + }, initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{ Name: "my-secret", @@ -7944,16 +7861,9 @@ func TestUpdatePackageRepository(t *testing.T) { { name: "updated with auth (plugin managed, update unchanged)", existingTypedObjects: []k8sruntime.Object{ - func() *k8scorev1.Secret { - s := defaultSecret() - s.Type = k8scorev1.SecretTypeBasicAuth - s.Data[k8scorev1.BasicAuthUsernameKey] = []byte("foo") - s.Data[k8scorev1.BasicAuthPasswordKey] = []byte("bar") - return s - }(), + basicAuthSecret(defaultSecret("my-secret", true), "foo", "bar"), }, initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { - repository.ObjectMeta.UID = "globalrepo" repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{ Name: "my-secret", } @@ -7987,16 +7897,9 @@ func TestUpdatePackageRepository(t *testing.T) { { name: "updated with auth (plugin managed, update some changes)", existingTypedObjects: []k8sruntime.Object{ - func() *k8scorev1.Secret { - s := defaultSecret() - s.Type = k8scorev1.SecretTypeBasicAuth - s.Data[k8scorev1.BasicAuthUsernameKey] = []byte("foo") - s.Data[k8scorev1.BasicAuthPasswordKey] = []byte("bar2") - return s - }(), + basicAuthSecret(defaultSecret("my-secret", true), "foo", "bar"), }, initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { - repository.ObjectMeta.UID = "globalrepo" repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{ Name: "my-secret", } @@ -8026,6 +7929,115 @@ func TestUpdatePackageRepository(t *testing.T) { } }, }, + { + name: "updated with new auth type (plugin managed, update basic to token)", + existingTypedObjects: []k8sruntime.Object{ + basicAuthSecret(defaultSecret("my-secret", true), "foo", "bar"), + }, + initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { + repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{ + Name: "my-secret", + } + return repository + }, + requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { + request.Auth = &corev1.PackageRepositoryAuth{ + Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, + PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_Header{ + Header: "zot", + }, + } + return request + }, + expectedStatusCode: codes.OK, + expectedRef: defaultRef, + customChecks: func(t *testing.T, s *Server) { + secret, err := s.getSecret(context.Background(), defaultGlobalContext.Cluster, demoGlobalPackagingNamespace, "my-secret") + if err != nil { + t.Fatalf("error fetching secret:%+v", err) + } + if secret.Type != k8scorev1.SecretTypeOpaque || secret.StringData[BearerAuthToken] != "zot" { + t.Errorf("secret data not as expected: %+v", secret) + } + }, + }, + { + name: "updated with new auth type (plugin managed, invalid use of redacted)", + existingTypedObjects: []k8sruntime.Object{ + tokenAuthSecret(defaultSecret("my-secret", true), "zot"), + }, + initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { + repository.Spec.Fetch.ImgpkgBundle.SecretRef = &kappctrlv1alpha1.AppFetchLocalRef{ + Name: "my-secret", + } + return repository + }, + requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { + request.Auth = &corev1.PackageRepositoryAuth{ + Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, + PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_UsernamePassword{ + UsernamePassword: &corev1.UsernamePassword{ + Username: Redacted, + Password: "bar", + }, + }, + } + return request + }, + expectedStatusCode: codes.InvalidArgument, + expectedStatusString: "unexpected REDACTED content", + }, + { + name: "updated with new auth type (plugin managed, update basic to ssh, git spec)", + existingTypedObjects: []k8sruntime.Object{ + basicAuthSecret(defaultSecret("my-secret", true), "foo", "bar"), + }, + initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { + repository.Spec.Fetch = &packagingv1alpha1.PackageRepositoryFetch{ + Git: &kappctrlv1alpha1.AppFetchGit{ + URL: "http://github.com/repo-1/main", + SecretRef: &kappctrlv1alpha1.AppFetchLocalRef{ + Name: "my-secret", + }, + }, + } + return repository + }, + requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { + request.Url = "http://github.com/repo-1/main" + request.Auth = &corev1.PackageRepositoryAuth{ + Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_SSH, + PackageRepoAuthOneOf: &corev1.PackageRepositoryAuth_SshCreds{ + SshCreds: &corev1.SshCredentials{ + PrivateKey: "ssh-key", + }, + }, + } + return request + }, + repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { + repository.Spec.Fetch = &packagingv1alpha1.PackageRepositoryFetch{ + Git: &kappctrlv1alpha1.AppFetchGit{ + URL: "http://github.com/repo-1/main", + SecretRef: &kappctrlv1alpha1.AppFetchLocalRef{ + Name: "my-secret", + }, + }, + } + return repository + }, + expectedStatusCode: codes.OK, + expectedRef: defaultRef, + customChecks: func(t *testing.T, s *Server) { + secret, err := s.getSecret(context.Background(), defaultGlobalContext.Cluster, demoGlobalPackagingNamespace, "my-secret") + if err != nil { + t.Fatalf("error fetching secret:%+v", err) + } + if secret.Type != k8scorev1.SecretTypeSSHAuth || secret.StringData[k8scorev1.SSHAuthPrivateKey] != "ssh-key" { + t.Errorf("secret data not as expected: %+v", secret) + } + }, + }, } for _, tc := range testCases { From 5faa8bf40fa35cb63eacf8f587467a8d2b07646b Mon Sep 17 00:00:00 2001 From: Dimitri Laloue Date: Fri, 2 Dec 2022 11:26:48 -0800 Subject: [PATCH 4/8] added description support for carvel repositories --- .../packages/v1alpha1/server_data_adapters.go | 16 ++- .../packages/v1alpha1/server_test.go | 104 +++++++++++++++--- .../packages/v1alpha1/server_utils.go | 18 ++- 3 files changed, 115 insertions(+), 23 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_adapters.go b/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_adapters.go index 3d590d381e5..cd46bc594a4 100644 --- a/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_adapters.go +++ b/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_data_adapters.go @@ -35,6 +35,8 @@ const ( Redacted = "REDACTED" + Annotation_Description_Key = "kubeapps.dev/description" + Annotation_ManagedBy_Key = "kubeapps.dev/managed-by" Annotation_ManagedBy_Value = "plugin:kapp-controller" @@ -436,6 +438,7 @@ func (s *Server) buildPackageRepositorySummary(pkgRepository *packagingv1alpha1. Identifier: pkgRepository.Name, }, Name: pkgRepository.Name, + Description: getDescription(pkgRepository), NamespaceScoped: s.pluginConfig.globalPackagingNamespace != pkgRepository.Namespace, RequiresAuth: repositorySecretRef(pkgRepository) != nil, } @@ -487,6 +490,7 @@ func (s *Server) buildPackageRepository(pkgRepository *packagingv1alpha1.Package Identifier: pkgRepository.Name, }, Name: pkgRepository.Name, + Description: getDescription(pkgRepository), NamespaceScoped: s.pluginConfig.globalPackagingNamespace != pkgRepository.Namespace, } @@ -649,6 +653,9 @@ func (s *Server) buildPkgRepositoryCreate(request *corev1.AddPackageRepositoryRe } repository.Spec = s.buildPkgRepositorySpec(request.Type, request.Interval, request.Url, request.Auth, pkgSecret, details) + // description + setDescription(repository, request.Description) + return repository, nil } @@ -677,6 +684,9 @@ func (s *Server) buildPkgRepositoryUpdate(request *corev1.UpdatePackageRepositor // repository repository.Spec = s.buildPkgRepositorySpec(rptype, request.Interval, request.Url, request.Auth, pkgSecret, details) + // description + setDescription(repository, request.Description) + return repository, nil } @@ -758,9 +768,6 @@ func (s *Server) validatePackageRepositoryCreate(ctx context.Context, cluster st namespace = s.pluginConfig.globalPackagingNamespace } - if request.Description != "" { - return status.Errorf(codes.InvalidArgument, "Description is not supported") - } if request.TlsConfig != nil { return status.Errorf(codes.InvalidArgument, "TLS Config is not supported") } @@ -820,9 +827,6 @@ func (s *Server) validatePackageRepositoryUpdate(ctx context.Context, cluster st return status.Errorf(codes.Internal, "the package repository has a fetch directive that is not supported") } - if request.Description != "" { - return status.Errorf(codes.InvalidArgument, "Description is not supported") - } if request.TlsConfig != nil { return status.Errorf(codes.InvalidArgument, "TLS Config is not supported") } diff --git a/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go b/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go index e2c91543ce5..df9e71d65ca 100644 --- a/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go +++ b/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_test.go @@ -6574,14 +6574,6 @@ func TestAddPackageRepository(t *testing.T) { }, expectedStatusCode: codes.InvalidArgument, }, - { - name: "validate desc", - requestCustomizer: func(request *corev1.AddPackageRepositoryRequest) *corev1.AddPackageRepositoryRequest { - request.Description = "some description" - return request - }, - expectedStatusCode: codes.InvalidArgument, - }, { name: "validate scope", requestCustomizer: func(request *corev1.AddPackageRepositoryRequest) *corev1.AddPackageRepositoryRequest { @@ -6813,6 +6805,19 @@ func TestAddPackageRepository(t *testing.T) { expectedStatusCode: codes.InvalidArgument, expectedStatusString: "unexpected REDACTED", }, + { + name: "create with description", + requestCustomizer: func(request *corev1.AddPackageRepositoryRequest) *corev1.AddPackageRepositoryRequest { + request.Description = "repository description" + return request + }, + repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { + repository.Annotations = map[string]string{Annotation_Description_Key: "repository description"} + return repository + }, + expectedStatusCode: codes.OK, + expectedRef: defaultRef, + }, { name: "create with no interval", requestCustomizer: func(request *corev1.AddPackageRepositoryRequest) *corev1.AddPackageRepositoryRequest { @@ -7298,14 +7303,6 @@ func TestUpdatePackageRepository(t *testing.T) { }, expectedStatusCode: codes.InvalidArgument, }, - { - name: "validate desc", - requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { - request.Description = "some description" - return request - }, - expectedStatusCode: codes.InvalidArgument, - }, { name: "validate url", requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { @@ -7479,6 +7476,40 @@ func TestUpdatePackageRepository(t *testing.T) { expectedStatusCode: codes.FailedPrecondition, expectedStatusString: "not in a stable state", }, + { + name: "update with new description", + initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { + repository.Annotations = map[string]string{Annotation_Description_Key: "initial description"} + return repository + }, + requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { + request.Description = "updated description" + return request + }, + repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { + repository.Annotations = map[string]string{Annotation_Description_Key: "updated description"} + return repository + }, + expectedStatusCode: codes.OK, + expectedRef: defaultRef, + }, + { + name: "update remove description", + initialCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { + repository.Annotations = map[string]string{Annotation_Description_Key: "initial description"} + return repository + }, + requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { + request.Description = "" + return request + }, + repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { + repository.Annotations = nil + return repository + }, + expectedStatusCode: codes.OK, + expectedRef: defaultRef, + }, { name: "update with no interval", requestCustomizer: func(request *corev1.UpdatePackageRepositoryRequest) *corev1.UpdatePackageRepositoryRequest { @@ -8377,6 +8408,18 @@ func TestGetPackageRepositoryDetail(t *testing.T) { }, expectedStatusCode: codes.OK, }, + { + name: "check description", + repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { + repository.Annotations = map[string]string{Annotation_Description_Key: "repository description"} + return repository + }, + responseCustomizer: func(response *corev1.GetPackageRepositoryDetailResponse) *corev1.GetPackageRepositoryDetailResponse { + response.Detail.Description = "repository description" + return response + }, + expectedStatusCode: codes.OK, + }, { name: "check interval (none)", repositoryCustomizer: func(repository *packagingv1alpha1.PackageRepository) *packagingv1alpha1.PackageRepository { @@ -9084,6 +9127,35 @@ func TestGetPackageRepositorySummaries(t *testing.T) { RequiresAuth: true, }, }, + { + name: "test with description", + existingObjects: []k8sruntime.Object{ + &packagingv1alpha1.PackageRepository{ + TypeMeta: defaultTypeMeta, + ObjectMeta: metav1.ObjectMeta{Name: "globalrepo", Namespace: demoGlobalPackagingNamespace, Annotations: map[string]string{Annotation_Description_Key: "repository summary description"}}, + Spec: packagingv1alpha1.PackageRepositorySpec{ + Fetch: &packagingv1alpha1.PackageRepositoryFetch{ + ImgpkgBundle: &kappctrlv1alpha1.AppFetchImgpkgBundle{ + Image: "projects.registry.example.com/repo-1/main@sha256:abcd", + }, + }, + }, + Status: packagingv1alpha1.PackageRepositoryStatus{}, + }, + }, + expectedResponse: &corev1.PackageRepositorySummary{ + PackageRepoRef: &corev1.PackageRepositoryReference{ + Context: defaultGlobalContext, + Plugin: &pluginDetail, + Identifier: "globalrepo", + }, + Name: "globalrepo", + Description: "repository summary description", + Type: Type_ImgPkgBundle, + Url: "projects.registry.example.com/repo-1/main@sha256:abcd", + RequiresAuth: false, + }, + }, } for _, tc := range testCases { diff --git a/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_utils.go b/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_utils.go index 61841886791..388715bc112 100644 --- a/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_utils.go +++ b/cmd/kubeapps-apis/plugins/kapp_controller/packages/v1alpha1/server_utils.go @@ -18,10 +18,10 @@ import ( "k8s.io/client-go/rest" "github.com/Masterminds/semver/v3" - kappcmdcore "github.com/vmware-tanzu/carvel-kapp/pkg/kapp/cmd/core" kappctrlv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" packagingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/packaging/v1alpha1" datapackagingv1alpha1 "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver/apis/datapackaging/v1alpha1" + kappcmdcore "github.com/vmware-tanzu/carvel-kapp/pkg/kapp/cmd/core" vendirversions "github.com/vmware-tanzu/carvel-vendir/pkg/vendir/versions/v1alpha1" corev1 "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/core/packages/v1alpha1" kappcorev1 "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/plugins/kapp_controller/packages/v1alpha1" @@ -490,6 +490,22 @@ func toPkgVersionSelection(version *kappcorev1.VersionSelection) *vendirversions return pkgversion } +// description +func setDescription(pkgRepository *packagingv1alpha1.PackageRepository, description string) { + if description != "" { + if pkgRepository.Annotations == nil { + pkgRepository.Annotations = make(map[string]string) + } + pkgRepository.Annotations[Annotation_Description_Key] = description + } else { + delete(pkgRepository.Annotations, Annotation_Description_Key) + } +} + +func getDescription(pkgRepository *packagingv1alpha1.PackageRepository) string { + return pkgRepository.Annotations[Annotation_Description_Key] +} + // secret state func repositorySecretRef(pkgRepository *packagingv1alpha1.PackageRepository) *kappctrlv1alpha1.AppFetchLocalRef { From e35dd3d44c523ba77ef647a7c82ed0cd2937df1e Mon Sep 17 00:00:00 2001 From: Dimitri Laloue Date: Mon, 5 Dec 2022 13:21:50 -0800 Subject: [PATCH 5/8] ui fixes related to flux support, plus flux support enhancements --- .../packages/v1alpha1/global_vars_test.go | 46 ++++ .../plugins/fluxv2/packages/v1alpha1/repo.go | 90 ++++++-- .../fluxv2/packages/v1alpha1/repo_test.go | 26 +++ .../fluxv2/packages/v1alpha1/server.go | 2 +- .../Config/PkgRepoList/PkgRepoForm.test.tsx | 1 + .../Config/PkgRepoList/PkgRepoForm.tsx | 216 +++++++----------- .../src/shared/PackageRepositoriesService.ts | 160 +++++++------ dashboard/src/shared/types.ts | 1 + dashboard/src/shared/utils.ts | 21 +- 9 files changed, 335 insertions(+), 228 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go index 14983ade251..70cbd847e83 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go @@ -1123,6 +1123,23 @@ var ( }, } + add_repo_8 = sourcev1.HelmRepository{ + TypeMeta: metav1.TypeMeta{ + Kind: sourcev1.HelmRepositoryKind, + APIVersion: sourcev1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Namespace: "foo", + ResourceVersion: "1", + Annotations: map[string]string{Annotation_Description_Key: "repo desc"}, + }, + Spec: sourcev1.HelmRepositorySpec{ + URL: "http://example.com", + Interval: metav1.Duration{Duration: 10 * time.Minute}, + }, + } + add_repo_req_1 = &corev1.AddPackageRepositoryRequest{ Name: "bar", Context: &corev1.Context{Namespace: "foo"}, @@ -1531,6 +1548,15 @@ var ( }, } + add_repo_req_31 = &corev1.AddPackageRepositoryRequest{ + Name: "bar", + Context: &corev1.Context{Namespace: "foo"}, + Type: "helm", + NamespaceScoped: true, + Url: "http://example.com", + Description: "repo desc", + } + add_repo_expected_resp = &corev1.AddPackageRepositoryResponse{ PackageRepoRef: repoRef("bar", "foo"), } @@ -3542,6 +3568,12 @@ var ( } } + update_repo_req_25 = &corev1.UpdatePackageRepositoryRequest{ + PackageRepoRef: repoRefInReq("repo-1", "namespace-1"), + Url: "http://url.com", + Description: "test desc", + } + update_repo_resp_1 = &corev1.UpdatePackageRepositoryResponse{ PackageRepoRef: repoRef("repo-1", "namespace-1"), } @@ -3860,6 +3892,20 @@ var ( }, } + update_repo_detail_21 = &corev1.GetPackageRepositoryDetailResponse{ + Detail: &corev1.PackageRepositoryDetail{ + PackageRepoRef: get_repo_detail_package_resp_ref, + Name: "repo-1", + Description: "test desc", + NamespaceScoped: true, + Type: "helm", + Url: "http://url.com", + Interval: "10m", + Auth: &corev1.PackageRepositoryAuth{PassCredentials: false}, + Status: repo_status_pending, + }, + } + basic_auth = func(username, password string) *corev1.PackageRepositoryAuth { return &corev1.PackageRepositoryAuth{ Type: corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go index 013cfae359c..50192c94e42 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go @@ -40,6 +40,9 @@ const ( // see docs at https://fluxcd.io/docs/components/source/ and // https://fluxcd.io/docs/components/helm/api/ fluxHelmRepositories = "helmrepositories" + + // description support + Annotation_Description_Key = "kubeapps.dev/description" ) var ( @@ -240,6 +243,7 @@ func (s *Server) newRepo(ctx context.Context, request *corev1.AddPackageReposito return nil, status.Errorf(codes.Unimplemented, "repository type [%s] not supported", typ) } + description := request.GetDescription() url := request.GetUrl() tlsConfig := request.GetTlsConfig() if url == "" { @@ -251,7 +255,8 @@ func (s *Server) newRepo(ctx context.Context, request *corev1.AddPackageReposito name := types.NamespacedName{Name: request.Name, Namespace: request.Context.Namespace} auth := request.GetAuth() - // Get or validate secret resource for auth, not yet stored in K8s + + // Get or validate secret resource for auth (stored in K8s in this method) secret, isSecretKubeappsManaged, err := s.handleRepoSecretForCreate(ctx, name, typ, tlsConfig, auth) if err != nil { return nil, err @@ -269,9 +274,15 @@ func (s *Server) newRepo(ctx context.Context, request *corev1.AddPackageReposito return nil, status.Errorf(codes.InvalidArgument, "customDetail could not be parsed due to: %v", err) } provider = customDetail.Provider + + if provider != "" && provider != "generic" { + if auth != nil && auth.Type != corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_UNSPECIFIED { + return nil, status.Errorf(codes.InvalidArgument, "Auth provider cannot be configured in combination with another auth method") + } + } } - if fluxRepo, err := newFluxHelmRepo(name, typ, url, interval, secret, passCredentials, provider); err != nil { + if fluxRepo, err := newFluxHelmRepo(name, description, typ, url, interval, secret, passCredentials, provider); err != nil { return nil, err } else if client, err := s.getClient(ctx, name.Namespace); err != nil { return nil, err @@ -336,9 +347,8 @@ func (s *Server) repoDetail(ctx context.Context, repoRef *corev1.PackageReposito Identifier: repo.Name, Plugin: GetPluginDetail(), }, - Name: repo.Name, - // TODO (gfichtenholt) Flux HelmRepository CR doesn't have a designated field for description - Description: "", + Name: repo.Name, + Description: getDescription(repo), // flux repositories are now considered to be namespaced, to support the most common cases. // see discussion at https://github.com/vmware-tanzu/kubeapps/issues/5542 NamespaceScoped: true, @@ -390,9 +400,8 @@ func (s *Server) repoSummaries(ctx context.Context, ns string) ([]*corev1.Packag Identifier: repo.Name, Plugin: GetPluginDetail(), }, - Name: repo.Name, - // TODO (gfichtenholt) Flux HelmRepository CR doesn't have a designated field for description - Description: "", + Name: repo.Name, + Description: getDescription(&repo), // flux repositories are now considered to be namespaced, to support the most common cases. // see discussion at https://github.com/vmware-tanzu/kubeapps/issues/5542 NamespaceScoped: true, @@ -406,7 +415,7 @@ func (s *Server) repoSummaries(ctx context.Context, ns string) ([]*corev1.Packag return summaries, nil } -func (s *Server) updateRepo(ctx context.Context, repoRef *corev1.PackageRepositoryReference, url string, interval string, tlsConfig *corev1.PackageRepositoryTlsConfig, auth *corev1.PackageRepositoryAuth) (*corev1.PackageRepositoryReference, error) { +func (s *Server) updateRepo(ctx context.Context, repoRef *corev1.PackageRepositoryReference, request *corev1.UpdatePackageRepositoryRequest) (*corev1.PackageRepositoryReference, error) { key := types.NamespacedName{Namespace: repoRef.GetContext().GetNamespace(), Name: repoRef.GetIdentifier()} repo, err := s.getRepoInCluster(ctx, key) if err != nil { @@ -421,18 +430,16 @@ func (s *Server) updateRepo(ctx context.Context, repoRef *corev1.PackageReposito return nil, status.Errorf(codes.Internal, "updates to repositories pending reconciliation are not supported") } - if url == "" { + if request.Url == "" { return nil, status.Errorf(codes.InvalidArgument, "repository url may not be empty") } - repo.Spec.URL = url + repo.Spec.URL = request.Url - // flux does not grok repository description yet - // the only field in customDetail is "provider" and I don't see the need to - // have the user update that. Its not like one repository is going to move from - // GCP to AWS. + // description now supported via annotation + setDescription(repo, request.Description) - if interval != "" { - if duration, err := pkgutils.ToDuration(interval); err != nil { + if request.Interval != "" { + if duration, err := pkgutils.ToDuration(request.Interval); err != nil { return nil, status.Errorf(codes.InvalidArgument, "interval is invalid: %v", err) } else { repo.Spec.Interval = *duration @@ -442,14 +449,13 @@ func (s *Server) updateRepo(ctx context.Context, repoRef *corev1.PackageReposito repo.Spec.Interval = defaultPollInterval } - if tlsConfig != nil && tlsConfig.InsecureSkipVerify { + if request.TlsConfig != nil && request.TlsConfig.InsecureSkipVerify { // ref https://github.com/fluxcd/source-controller/issues/807 return nil, status.Errorf(codes.InvalidArgument, "TLS flag insecureSkipVerify is not supported") } // validate and get updated (or newly created) secret - secret, isKubeappsManagedSecret, isSecretUpdated, err := - s.handleRepoSecretForUpdate(ctx, repo, tlsConfig, auth) + secret, isKubeappsManagedSecret, isSecretUpdated, err := s.handleRepoSecretForUpdate(ctx, repo, request.TlsConfig, request.Auth) if err != nil { return nil, err } @@ -462,7 +468,29 @@ func (s *Server) updateRepo(ctx context.Context, repoRef *corev1.PackageReposito } } - repo.Spec.PassCredentials = auth != nil && auth.PassCredentials + repo.Spec.PassCredentials = request.Auth != nil && request.Auth.PassCredentials + + // Get Flux-specific values + if request.CustomDetail != nil { + customDetail := &v1alpha1.FluxPackageRepositoryCustomDetail{} + if err := request.CustomDetail.UnmarshalTo(customDetail); err != nil { + return nil, status.Errorf(codes.InvalidArgument, "customDetail could not be parsed due to: %v", err) + } + provider := customDetail.Provider + + // following fixes for issue5746, the provider is allowed to be configured on update if not previously configured + if provider != "" && provider != "generic" { + if request.Auth != nil && request.Auth.Type != corev1.PackageRepositoryAuth_PACKAGE_REPOSITORY_AUTH_TYPE_UNSPECIFIED { + return nil, status.Errorf(codes.InvalidArgument, "Auth provider cannot be configured in combination with another auth method") + } + if repo.Spec.Provider != "" && repo.Spec.Provider != "generic" && repo.Spec.Provider != provider { + return nil, status.Errorf(codes.InvalidArgument, "Auth provider cannot be changed.") + } + repo.Spec.Provider = provider + } else { + repo.Spec.Provider = "" + } + } // get rid of the status field, since now there will be a new reconciliation // process and the current status no longer applies. metadata and spec I want @@ -887,6 +915,7 @@ func checkRepoGeneration(repo sourcev1.HelmRepository) bool { // ref https://fluxcd.io/docs/components/source/helmrepositories/ func newFluxHelmRepo( targetName types.NamespacedName, + desc string, typ string, url string, interval string, @@ -914,6 +943,9 @@ func newFluxHelmRepo( if typ == sourcev1.HelmRepositoryTypeOCI { fluxRepo.Spec.Type = sourcev1.HelmRepositoryTypeOCI } + if desc != "" { + setDescription(fluxRepo, desc) + } if secret != nil { fluxRepo.Spec.SecretRef = &fluxmeta.LocalObjectReference{ Name: secret.Name, @@ -927,3 +959,19 @@ func newFluxHelmRepo( } return fluxRepo, nil } + +// description +func setDescription(fluxRepo *sourcev1.HelmRepository, description string) { + if description != "" { + if fluxRepo.Annotations == nil { + fluxRepo.Annotations = make(map[string]string) + } + fluxRepo.Annotations[Annotation_Description_Key] = description + } else { + delete(fluxRepo.Annotations, Annotation_Description_Key) + } +} + +func getDescription(fluxRepo *sourcev1.HelmRepository) string { + return fluxRepo.Annotations[Annotation_Description_Key] +} diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go index b8c159e4898..0cbfa3ed28a 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go @@ -1352,6 +1352,13 @@ func TestAddPackageRepository(t *testing.T) { request: add_repo_req_30, statusCode: codes.InvalidArgument, }, + { + name: "simple repo with description", + request: add_repo_req_31, + expectedResponse: add_repo_expected_resp, + expectedRepo: &add_repo_8, + statusCode: codes.OK, + }, } for _, tc := range testCases { @@ -1616,6 +1623,15 @@ func TestGetPackageRepositoryDetail(t *testing.T) { expectedStatusCode: codes.OK, expectedResponse: get_repo_detail_resp_10a, }, + { + name: "get package repository detail description", + repoIndex: testYaml("valid-index.yaml"), + repoName: "repo-1", + repoNamespace: "namespace-1", + request: get_repo_detail_req_1, + expectedStatusCode: codes.OK, + expectedResponse: get_repo_detail_resp_1, + }, } for _, tc := range testCases { @@ -2108,6 +2124,16 @@ func TestUpdatePackageRepository(t *testing.T) { Name: "repo-1", Namespace: "namespace-1"}, nil, nil, ca))), }, + { + name: "update description", + repoIndex: testYaml("valid-index.yaml"), + repoName: "repo-1", + repoNamespace: "namespace-1", + request: update_repo_req_25, + expectedStatusCode: codes.OK, + expectedResponse: update_repo_resp_1, + expectedDetail: update_repo_detail_21, + }, } for _, tc := range testCases { diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go index 78e40512380..e269ff8b430 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go @@ -611,7 +611,7 @@ func (s *Server) UpdatePackageRepository(ctx context.Context, request *corev1.Up cluster) } - if responseRef, err := s.updateRepo(ctx, repoRef, request.Url, request.Interval, request.TlsConfig, request.Auth); err != nil { + if responseRef, err := s.updateRepo(ctx, repoRef, request); err != nil { return nil, err } else { return &corev1.UpdatePackageRepositoryResponse{ diff --git a/dashboard/src/components/Config/PkgRepoList/PkgRepoForm.test.tsx b/dashboard/src/components/Config/PkgRepoList/PkgRepoForm.test.tsx index 0b8edb308e2..14dac0ccac1 100644 --- a/dashboard/src/components/Config/PkgRepoList/PkgRepoForm.test.tsx +++ b/dashboard/src/components/Config/PkgRepoList/PkgRepoForm.test.tsx @@ -44,6 +44,7 @@ const pkgRepoFormData = { authHeader: "", authMethod: PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_UNSPECIFIED, + isUserManaged: false, basicAuth: { password: "", username: "", diff --git a/dashboard/src/components/Config/PkgRepoList/PkgRepoForm.tsx b/dashboard/src/components/Config/PkgRepoList/PkgRepoForm.tsx index 66a5f027b06..79821a3b53c 100644 --- a/dashboard/src/components/Config/PkgRepoList/PkgRepoForm.tsx +++ b/dashboard/src/components/Config/PkgRepoList/PkgRepoForm.tsx @@ -222,6 +222,7 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { setSecretAuthName(repo.auth?.secretRef?.name || ""); setSecretTLSName(repo.tlsConfig?.secretRef?.name || ""); setIsUserManagedSecret(!!repo.auth?.secretRef?.name); + setIsUserManagedCASecret(!!repo.tlsConfig?.secretRef?.name); // setting custom details for the Helm plugin if (repo.packageRepoRef?.plugin?.name === PluginNames.PACKAGES_HELM) { @@ -320,7 +321,7 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { finalHeader = authCustomHeader; break; case PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_BEARER: - finalHeader = `Bearer ${bearerToken}`; + finalHeader = bearerToken; break; } @@ -346,6 +347,7 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { const request = { authHeader: !isUserManagedSecret ? finalHeader : "", authMethod, + isUserManaged: isUserManagedSecret, basicAuth: { password: !isUserManagedSecret ? basicPassword : "", username: !isUserManagedSecret ? basicUser : "", @@ -393,19 +395,25 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { filterRule: filter, imagesPullSecret: { // if using the same credentials toggle is set, use the repo auth's creds instead - secretRef: isUserManagedPSSecret - ? useSameAuthCreds - ? secretAuthName - : secretPSName - : "", - credentials: !isUserManagedPSSecret - ? { - email: useSameAuthCreds ? secretEmail : pullSecretEmail, - username: useSameAuthCreds ? secretUser : pullSecretUser, - password: useSameAuthCreds ? secretPassword : pullSecretPassword, - server: useSameAuthCreds ? secretServer : pullSecretServer, - } - : undefined, + secretRef: + helmPSAuthMethod === + PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON && + isUserManagedPSSecret + ? useSameAuthCreds + ? secretAuthName + : secretPSName + : "", + credentials: + helmPSAuthMethod === + PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON && + !isUserManagedPSSecret + ? { + email: useSameAuthCreds ? secretEmail : pullSecretEmail, + username: useSameAuthCreds ? secretUser : pullSecretUser, + password: useSameAuthCreds ? secretPassword : pullSecretPassword, + server: useSameAuthCreds ? secretServer : pullSecretServer, + } + : undefined, }, } as HelmPackageRepositoryCustomDetail; break; @@ -744,9 +752,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { placeholder="Description of the repository" value={description || ""} onChange={handleDescriptionChange} - disabled={( - [PluginNames.PACKAGES_FLUX, PluginNames.PACKAGES_KAPP] as string[] - ).includes(plugin?.name)} /> {/* TODO(agamez): these plugin selectors should be loaded @@ -941,27 +946,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { required={plugin?.name === PluginNames.PACKAGES_KAPP} /> - - - - + + + + )} @@ -1055,7 +1058,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_UNSPECIFIED } onChange={handleAuthRadioButtonChange} - disabled={!!repo.auth?.type} /> @@ -1071,13 +1073,7 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { showAuthProviderDetails } onChange={handleFluxAuthProviderAuthChange} - disabled={ - !( - plugin?.name === PluginNames.PACKAGES_FLUX && - type === "oci" && - !repo.name - ) - } + disabled={!(plugin?.name === PluginNames.PACKAGES_FLUX && type === "oci")} /> @@ -1098,7 +1094,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { } onChange={handleAuthRadioButtonChange} disabled={ - !!repo.auth?.type || !getSupportedPackageRepositoryAuthTypes(plugin, type).includes( PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH, ) @@ -1123,7 +1118,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { } onChange={handleAuthRadioButtonChange} disabled={ - !!repo.auth?.type || !getSupportedPackageRepositoryAuthTypes(plugin, type).includes( PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_BEARER, ) @@ -1150,7 +1144,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { } onChange={handleAuthRadioButtonChange} disabled={ - !!repo.auth?.type || !getSupportedPackageRepositoryAuthTypes(plugin, type).includes( PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON, ) @@ -1177,7 +1170,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { } onChange={handleAuthRadioButtonChange} disabled={ - !!repo.auth?.type || !getSupportedPackageRepositoryAuthTypes(plugin, type).includes( PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_AUTHORIZATION_HEADER, ) @@ -1204,7 +1196,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { } onChange={handleAuthRadioButtonChange} disabled={ - !!repo.auth?.type || !getSupportedPackageRepositoryAuthTypes(plugin, type).includes( PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_SSH, ) @@ -1231,7 +1222,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { } onChange={handleAuthRadioButtonChange} disabled={ - !!repo.auth?.type || !getSupportedPackageRepositoryAuthTypes(plugin, type).includes( PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_TLS, ) @@ -1258,7 +1248,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { } onChange={handleAuthRadioButtonChange} disabled={ - !!repo.auth?.type || !getSupportedPackageRepositoryAuthTypes(plugin, type).includes( PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_OPAQUE, ) @@ -1296,7 +1285,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH } - disabled={!!repo.auth?.type} />
@@ -1312,7 +1300,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_BASIC_AUTH } - disabled={!!repo.auth?.type} /> @@ -1345,7 +1332,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_BEARER } - disabled={!!repo.auth?.type} /> @@ -1377,7 +1363,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON } - disabled={!!repo.auth?.type} />
@@ -1392,7 +1377,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON } - disabled={!!repo.auth?.type} />
@@ -1408,7 +1392,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_DOCKER_CONFIG_JSON } - disabled={!!repo.auth?.type} />
@@ -1419,7 +1402,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { value={secretEmail || ""} onChange={handleAuthSecretEmailChange} placeholder="user@example.com" - disabled={!!repo.auth?.type} /> @@ -1454,7 +1436,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_AUTHORIZATION_HEADER } - disabled={!!repo.auth?.type} /> @@ -1489,7 +1470,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_SSH } - disabled={!!repo.auth?.type} />
@@ -1509,7 +1489,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_SSH } - disabled={!!repo.auth?.type} /> @@ -1544,7 +1523,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_TLS } - disabled={!!repo.auth?.type} />
@@ -1562,7 +1540,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_TLS } - disabled={!!repo.auth?.type} /> @@ -1597,7 +1574,6 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { authMethod === PackageRepositoryAuth_PackageRepositoryAuthType.PACKAGE_REPOSITORY_AUTH_TYPE_OPAQUE } - disabled={!!repo.auth?.type} /> @@ -1613,7 +1589,13 @@ export function PkgRepoForm(props: IPkgRepoFormProps) { value={authProvider} onChange={handleFluxAuthProviderChange} required={true} - disabled={selectedPkgRepo !== undefined} + disabled={ + !!repo.name && + ["aws", "azure", "gcp"].includes( + (repo.customDetail as Partial) + ?.provider || "", + ) + } >