Skip to content

Commit

Permalink
[repository] fixes and enhancements for repository update support (#5763
Browse files Browse the repository at this point in the history 
)

### Description of the change

This PR contains changes for two purposes:
 - improve feature parity between plugins
 - enable update support in UI

Feature parity:
 - flux: description support
 - flux: support changing to/from auth provider 
 - carvel: description support
 - carvel: support changing auth type 

Bug fixes to support update support in UI:
 - auth updates were not possible
 - fixed conditions for fields enablement
- form fields data were sent incorrectly as a group, instead of only
sending what applied
 - errors handling 'undefined' values
 - fixed which auth types are valid per plugin+type combination
 - fixed issue with helm plugins image pull secrets

helm/flux bug fixes:
- fixed handling of redacted fields leading to secrets not being updated
as expected or secret data being overwritten with 'redacted' as value.

(note: the PR has many small changes, but i was fixing issues as i was
testing, hence the list)


### Benefits

With the changes, users should see more commonality between plugin
features, as well as a more robust UI experience regarding repository
creation/updates.


### Applicable issues

fixes issues #5745, #5746 and #5747


### Additional information

The issue 1 in # 5746 could not be reproduce with the latest bits, so no
change were made for it.
  • Loading branch information
@dlaloue-vmware
dlaloue-vmware authored Dec 15, 2022
1 parent 8ae4a75 commit ed020ef
Show file tree
Hide file tree
Showing 21 changed files with 1,898 additions and 915 deletions.
150 changes: 136 additions & 14 deletions cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/global_vars_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package main

import (
"encoding/json"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/k8sutils"
"os"
"time"

Expand Down Expand Up @@ -1123,6 +1124,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{k8sutils.AnnotationDescriptionKey: "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"},
Expand Down Expand Up @@ -1531,6 +1549,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"),
}
Expand Down Expand Up @@ -3518,6 +3545,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 = 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_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"),
}
Expand Down Expand Up @@ -3792,26 +3849,80 @@ 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,
},
}

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,
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,
Expand All @@ -3838,6 +3949,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{
Expand Down
72 changes: 51 additions & 21 deletions cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"encoding/gob"
"fmt"
"github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/k8sutils"
"regexp"
"strings"
"time"
Expand Down Expand Up @@ -240,6 +241,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 == "" {
Expand All @@ -251,7 +253,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
Expand All @@ -269,9 +272,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
Expand Down Expand Up @@ -336,9 +345,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: k8sutils.GetDescription(&repo.ObjectMeta),
// 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,
Expand Down Expand Up @@ -390,9 +398,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: k8sutils.GetDescription(&repo.ObjectMeta),
// 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,
Expand All @@ -406,7 +413,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 {
Expand All @@ -421,18 +428,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
k8sutils.SetDescription(&repo.ObjectMeta, 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
Expand All @@ -442,14 +447,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
}
Expand All @@ -462,7 +466,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
Expand Down Expand Up @@ -887,6 +913,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,
Expand Down Expand Up @@ -914,6 +941,9 @@ func newFluxHelmRepo(
if typ == sourcev1.HelmRepositoryTypeOCI {
fluxRepo.Spec.Type = sourcev1.HelmRepositoryTypeOCI
}
if desc != "" {
k8sutils.SetDescription(&fluxRepo.ObjectMeta, desc)
}
if secret != nil {
fluxRepo.Spec.SecretRef = &fluxmeta.LocalObjectReference{
Name: secret.Name,
Expand Down
Loading

0 comments on commit ed020ef

Please sign in to comment.