From fcbf0c9c71911d2a7c83217e58757153171d7ca1 Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Wed, 22 Jun 2022 23:16:51 -0700 Subject: [PATCH 01/11] incremental --- .../fluxv2/packages/v1alpha1/common/utils.go | 6 +- .../v1alpha1/docker_reg_v2_repo_lister.go | 104 +++++++++++++++++ .../v1alpha1/github_oci_repo_lister.go | 110 ------------------ .../fluxv2/packages/v1alpha1/oci_repo.go | 66 ++++++----- go.mod | 5 + go.sum | 4 + 6 files changed, 155 insertions(+), 140 deletions(-) create mode 100644 cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go delete mode 100644 cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/github_oci_repo_lister.go diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common/utils.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common/utils.go index f0a0ee099f0..2a0190f67b3 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common/utils.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common/utils.go @@ -38,7 +38,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" log "k8s.io/klog/v2" - registryauth "oras.land/oras-go/pkg/registry/remote/auth" + orasregistryauthv2 "oras.land/oras-go/v2/registry/remote/auth" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -309,7 +309,7 @@ func tlsClientConfigFromSecret(secret apiv1.Secret, options *HttpClientOptions) // complete Docker configuration. If both, "username" and "password" are // empty, a nil LoginOption and a nil error will be returned. // ref https://github.com/fluxcd/source-controller/blob/main/internal/helm/registry/auth.go -func OCIRegistryCredentialFromSecret(registryURL string, secret apiv1.Secret) (*registryauth.Credential, error) { +func OCIRegistryCredentialFromSecret(registryURL string, secret apiv1.Secret) (*orasregistryauthv2.Credential, error) { var username, password string if secret.Type == apiv1.SecretTypeDockerConfigJson { dockerCfg, err := config.LoadFromReader(bytes.NewReader(secret.Data[apiv1.DockerConfigJsonKey])) @@ -349,7 +349,7 @@ func OCIRegistryCredentialFromSecret(registryURL string, secret apiv1.Secret) (* pwdRedacted = pwdRedacted[0:3] + "..." } log.Infof("-OCIRegistryCredentialFromSecret: username: [%s], password: [%s]", username, pwdRedacted) - return ®istryauth.Credential{ + return &orasregistryauthv2.Credential{ Username: username, Password: password, }, nil diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go new file mode 100644 index 00000000000..9f80508f9c7 --- /dev/null +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go @@ -0,0 +1,104 @@ +// Copyright 2021-2022 the Kubeapps contributors. +// SPDX-License-Identifier: Apache-2.0 +package main + +import ( + "context" + "errors" + "fmt" + "strings" + + log "k8s.io/klog/v2" + + "helm.sh/helm/v3/pkg/registry" + + // ORAS => OCI Registry AS Storage + orasregistryv2 "oras.land/oras-go/v2/registry" + orasregistryremotev2 "oras.land/oras-go/v2/registry/remote" + orasregistryauthv2 "oras.land/oras-go/v2/registry/remote/auth" +) + +// This flavor of OCI lister Works with respect to those OCI registry vendors that implement +// Docker Registry API V2 or OCI Distribution Specification. For example, GitHub (ghcr.io) +// References: +// - https://docs.docker.com/registry/spec/api/#base +// - https://github.com/opencontainers/distribution-spec/blob/main/spec.md#api +func NewDockerRegistryApiV2RepositoryLister() OCIRepositoryLister { + return &dockerRegistryApiV2RepositoryLister{} +} + +type dockerRegistryApiV2RepositoryLister struct { +} + +// ref https://github.com/distribution/distribution/blob/main/docs/spec/api.md#api-version-check +// also https://github.com/oras-project/oras-go/blob/14422086e418/registry/remote/registry.go +func (l *dockerRegistryApiV2RepositoryLister) IsApplicableFor(ociRegistry *OCIRegistry) (bool, error) { + log.Infof("+IsApplicableFor(%s)", ociRegistry.url.String()) + + orasRegistry, err := newRemoteOrasRegistry(ociRegistry) + if err != nil { + return false, err + } else { + ping := "OK" + err = orasRegistry.Ping(context.Background()) + if err != nil { + ping = fmt.Sprintf("%v", err) + } + log.Infof("ORAS Registry [%s] Ping: %s", ociRegistry.url.String(), ping) + return err == nil, err + } +} + +// ref https://github.com/distribution/distribution/blob/main/docs/spec/api.md#listing-repositories +func (l *dockerRegistryApiV2RepositoryLister) ListRepositoryNames(ociRegistry *OCIRegistry) ([]string, error) { + log.Infof("+ListRepositoryNames()") + + orasRegistry, err := newRemoteOrasRegistry(ociRegistry) + if err != nil { + return nil, err + } else { + // this is the way to stop the loop in + // https://github.com/oras-project/oras-go/blob/14422086e41897a44cb706726e687d39dc728805/registry/remote/registry.go#L112 + done := errors.New("(done) backstop") + + fn := func(repos []string) error { + log.Infof("orasRegistry.Repositories fn: %s", repos) + return done + } + // TODO need to append + // "?last=" + orasRegistry.Reference.Repository + // to req.Query so we don't start at the beggining of the alphabet + + // impl refs: + // 1. https://github.com/oras-project/oras-go/blob/14422086e418/registry/remote/registry.go + // 2. https://github.com/oras-project/oras-go/blob/14422086e41897a44cb706726e687d39dc728805/registry/remote/url.go#L43 + err = orasRegistry.Repositories(context.Background(), fn) + log.Infof("ORAS Repositories returned: %v", err) + if err != nil && err != done { + return nil, err + } + //repositoryList := []string{} + //return repositoryList, nil + } + + // OLD + return []string{"stefanprodan/charts/podinfo"}, nil +} + +func newRemoteOrasRegistry(ociRegistry *OCIRegistry) (*orasregistryremotev2.Registry, error) { + ref := strings.TrimPrefix(ociRegistry.url.String(), fmt.Sprintf("%s://", registry.OCIScheme)) + parsedRef, err := orasregistryv2.ParseReference(ref) + if err != nil { + return nil, err + } + orasRegistry, err := orasregistryremotev2.NewRegistry(parsedRef.Registry) + if err != nil { + return nil, err + } + orasRegistry.Client = &orasregistryauthv2.Client{ + Header: orasregistryauthv2.DefaultClient.Header.Clone(), + Cache: orasregistryauthv2.DefaultCache, + Credential: ociRegistry.registryCredentialFn, + } + return orasRegistry, nil +} diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/github_oci_repo_lister.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/github_oci_repo_lister.go deleted file mode 100644 index 8534b20ca7b..00000000000 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/github_oci_repo_lister.go +++ /dev/null @@ -1,110 +0,0 @@ -// Copyright 2021-2022 the Kubeapps contributors. -// SPDX-License-Identifier: Apache-2.0 -package main - -import ( - "fmt" - "io/ioutil" - "net/http" - "strings" - - "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common" - log "k8s.io/klog/v2" - - "helm.sh/helm/v3/pkg/registry" - orascontext "oras.land/oras-go/pkg/context" - orasregistry "oras.land/oras-go/pkg/registry" - registryremote "oras.land/oras-go/pkg/registry/remote" - registryauth "oras.land/oras-go/pkg/registry/remote/auth" -) - -func NewGitHubRepositoryLister() OCIRepositoryLister { - return &gitHubRepositoryLister{} -} - -type gitHubRepositoryLister struct { -} - -// ref https://github.com/distribution/distribution/blob/main/docs/spec/api.md#api-version-check -func (l *gitHubRepositoryLister) IsApplicableFor(ociRegistry *OCIRegistry) (bool, error) { - log.Infof("+IsApplicableFor(%s)", ociRegistry.url.String()) - - // ref https://github.com/helm/helm/blob/657850e44b880cca43d0606ebf5a54eb75362c3f/pkg/registry/client.go#L55 - registryAuthorizer := ®istryauth.Client{ - Header: http.Header{"User-Agent": {common.UserAgentString()}}, - Cache: registryauth.DefaultCache, - Credential: ociRegistry.registryCredentialFn, - } - - // given ref like this - // ghcr.io/stefanprodan/charts/podinfo - // will return - // { - // "Registry": "ghcr.io", - // "Repository": "stefanprodan/charts/podinfo", - // "Reference": "" - // } - ref := strings.TrimPrefix(ociRegistry.url.String(), fmt.Sprintf("%s://", registry.OCIScheme)) - log.Infof("ref: [%s]", ref) - - parsedRef, err := orasregistry.ParseReference(ref) - if err != nil { - return false, err - } - log.Infof("parsed reference: [%s]", common.PrettyPrint(parsedRef)) - - ociRepo := registryremote.Repository{ - Reference: parsedRef, - Client: registryAuthorizer, - } - - // build the base endpoint of the remote registry. - // Format: :///v2/ - url := fmt.Sprintf("%s://%s/v2/", "https", ociRepo.Reference.Host()) - req, err := http.NewRequestWithContext(orascontext.Background(), http.MethodGet, url, nil) - if err != nil { - return false, err - } - - resp, err := ociRepo.Client.Do(req) - if err != nil { - return false, err - } - defer resp.Body.Close() - - _, err = ioutil.ReadAll(resp.Body) - if err != nil { - return false, err - } - - if resp.StatusCode == http.StatusOK { - // based on the presence of this header Docker-Distribution-Api-Version:[registry/2.0] - // conclude this is a case for GitHubRepositoryLister, e.g. - // +HTTP GET request: - // URL: https://ghcr.io/v2/ - // -HTTP GET response: code: 200 OK - // headers: - // map[ - // Content-Length:[0] Content-Type:[application/json] - // Date:[Sun, 19 Jun 2022 05:08:18 GMT] - // Docker-Distribution-Api-Version:[registry/2.0] - // X-Github-Request-Id:[C4E4:2F9A:3069FD:914D65:62AEAF42] - // ] - - val, ok := resp.Header["Docker-Distribution-Api-Version"] - if ok && len(val) == 1 && val[0] == "registry/2.0" { - log.Infof("-isApplicableFor(): yes") - return true, nil - } - } else { - log.Infof("isApplicableFor(): HTTP GET (%s) returned status [%d]", url, resp.StatusCode) - } - return false, nil -} - -// ref https://github.com/distribution/distribution/blob/main/docs/spec/api.md#listing-repositories -func (l *gitHubRepositoryLister) ListRepositoryNames() ([]string, error) { - log.Infof("+ListRepositoryNames()") - // TODO (gfichtenholt) fix me - return []string{"podinfo"}, nil -} diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go index 0232fe6832c..d0e58edc3e8 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go @@ -43,7 +43,7 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" // OCI Registry As a Storage (ORAS) - registryauth "oras.land/oras-go/pkg/registry/remote/auth" + orasregistryauthv2 "oras.land/oras-go/v2/registry/remote/auth" ) // RegistryClient is an interface for interacting with OCI registries @@ -60,7 +60,7 @@ type RegistryClient interface { // registry repository name lister applies, and then executes specific logic type OCIRepositoryLister interface { IsApplicableFor(*OCIRegistry) (bool, error) - ListRepositoryNames() ([]string, error) + ListRepositoryNames(ociRegistry *OCIRegistry) ([]string, error) } // OCIRegistry represents a Helm chart repository, and the configuration @@ -93,7 +93,7 @@ type OCIRegistry struct { // to configure an OCIRegistry. type OCIRegistryOption func(*OCIRegistry) error -type OCIRegistryCredentialFn func(ctx context.Context, reg string) (registryauth.Credential, error) +type OCIRegistryCredentialFn func(ctx context.Context, reg string) (orasregistryauthv2.Credential, error) var ( helmGetters = getter.Providers{ @@ -110,7 +110,7 @@ var ( // TODO: make this thing extensible so code coming from other plugs/modules // can register new repository listers builtInRepoListers = []OCIRepositoryLister{ - NewGitHubRepositoryLister(), + NewDockerRegistryApiV2RepositoryLister(), // TODO } ) @@ -185,7 +185,7 @@ func (r *OCIRegistry) listRepositoryNames() ([]string, error) { return nil, status.Errorf(codes.Internal, "No repository lister found for OCI registry with url: [%s]", &r.url) } - return r.repositoryLister.ListRepositoryNames() + return r.repositoryLister.ListRepositoryNames(r) } // Get returns the ChartVersion for the given name, the version is expected @@ -193,7 +193,7 @@ func (r *OCIRegistry) listRepositoryNames() ([]string, error) { // stable version will be returned and prerelease versions will be ignored. // adapted from https://github.com/helm/helm/blob/49819b4ef782e80b0c7f78c30bd76b51ebb56dc8/pkg/downloader/chart_downloader.go#L162 func (r *OCIRegistry) getChartVersion(name, ver string) (*repo.ChartVersion, error) { - log.Infof("+getChartVersion(%s,%s", name, ver) + log.Infof("+getChartVersion(%s,%s)", name, ver) // Find chart versions matching the given name. // Either in an index file or from a registry. ref := fmt.Sprintf("%s/%s", r.url.String(), name) @@ -382,6 +382,11 @@ func (s *repoEventSink) onAddOciRepo(repo sourcev1.HelmRepository) ([]byte, bool charts := []models.Chart{} for _, appName := range appNames { + appName, err := ociRegistry.shortRepoName(appName) + if err != nil { + return nil, false, err + } + log.Infof("==========>: app name: [%s]", appName) chartVersion, err := ociRegistry.getChartVersion(appName, "") @@ -529,12 +534,17 @@ type TagList struct { // all repositories within the registry and returning the sha256. // Caveat: Mutated image tags won't be detected as new func (r *OCIRegistry) checksum() (string, error) { + log.Infof("+checksum") appNames, err := r.listRepositoryNames() if err != nil { return "", err } tags := map[string]TagList{} for _, appName := range appNames { + appName, err := r.shortRepoName(appName) + if err != nil { + return "", err + } ref := fmt.Sprintf("%s/%s", r.url.String(), appName) tagss, err := r.getTags(ref) if err != nil { @@ -551,13 +561,24 @@ func (r *OCIRegistry) checksum() (string, error) { return common.GetSha256(content) } +func (r *OCIRegistry) shortRepoName(fullRepoName string) (string, error) { + expectedPrefix := strings.TrimLeft(r.url.Path, "/") + "/" + if strings.HasPrefix(fullRepoName, expectedPrefix) { + return fullRepoName[len(expectedPrefix):], nil + } else { + err := status.Errorf(codes.Internal, + "Unexpected repository name: expected prefix: [%s], actual name: [%s]", + expectedPrefix, fullRepoName) + return "", err + } +} + func (s *repoEventSink) newOCIRegistryAndLogin(repo sourcev1.HelmRepository) (*OCIRegistry, error) { // Construct the Getter options from the HelmRepository data loginOpts, getterOpts, cred, err := s.clientOptionsForRepo(repo) if err != nil { return nil, status.Errorf(codes.Internal, "failed to create registry client options: %v", err) } - log.Infof("=====================> loginOpts: [%v], getterOpts: [%v], cred: %t", len(loginOpts), len(getterOpts), cred != nil) // Create registry client and login if needed. registryClient, file, err := newRegistryClient(loginOpts != nil) @@ -565,31 +586,22 @@ func (s *repoEventSink) newOCIRegistryAndLogin(repo sourcev1.HelmRepository) (*O return nil, status.Errorf(codes.Internal, "failed to create registry client: %v", err) } if file != "" { - /* - defer func() { - byteArray, err := ioutil.ReadFile(file) - if err != nil { - log.Infof("Failed to read temporary credentials file [%s]: %v", file, err) - } - - // Convert []byte to string and print to screen - log.Infof("about to remove temporary credentials file [%s] content\n[%s]", - file, string(byteArray)) - - if err := os.Remove(file); err != nil { - log.Infof("Failed to delete temporary credentials file: %v", err) - } - }() - */ + defer func() { + if err := os.Remove(file); err != nil { + log.Infof("Failed to delete temporary credentials file: %v", err) + } + log.Infof("Removed temporary credentials file: [%s]", file) + }() } - registryCredentialFn := func(ctx context.Context, reg string) (registryauth.Credential, error) { + registryCredentialFn := func(ctx context.Context, reg string) (orasregistryauthv2.Credential, error) { if cred != nil { return *cred, nil } else { - return registryauth.EmptyCredential, nil + return orasregistryauthv2.EmptyCredential, nil } } + // a little bit misleading, since repo.Spec.URL is really an OCI Registry URL, // which may contain zero or more "helm repositories", such as // oci://demo.goharbor.io/test-oci-1, which may contain repositories "repo-1", "repo2", etc @@ -615,11 +627,11 @@ func (s *repoEventSink) newOCIRegistryAndLogin(repo sourcev1.HelmRepository) (*O return ociRegistry, nil } -func (s *repoEventSink) clientOptionsForRepo(repo sourcev1.HelmRepository) ([]registry.LoginOption, []getter.Option, *registryauth.Credential, error) { +func (s *repoEventSink) clientOptionsForRepo(repo sourcev1.HelmRepository) ([]registry.LoginOption, []getter.Option, *orasregistryauthv2.Credential, error) { log.Infof("+clientOptionsForRepo()") var loginOpts []registry.LoginOption - var cred *registryauth.Credential + var cred *orasregistryauthv2.Credential getterOpts := []getter.Option{ getter.WithURL(repo.Spec.URL), getter.WithTimeout(repo.Spec.Timeout.Duration), diff --git a/go.mod b/go.mod index 0c4be214906..404f0c45bcd 100644 --- a/go.mod +++ b/go.mod @@ -98,6 +98,11 @@ require ( sigs.k8s.io/yaml v1.3.0 ) +require ( + github.com/oras-project/artifacts-spec v1.0.0-draft.1.1 // indirect + oras.land/oras-go/v2 v2.0.0-20220621073716-14422086e418 // indirect +) + require ( github.com/Azure/go-ansiterm v0.0.0-20210617225240-d185dfc1b5a1 // indirect github.com/BurntSushi/toml v1.1.0 // indirect diff --git a/go.sum b/go.sum index af8753cf6d6..9270887db7f 100644 --- a/go.sum +++ b/go.sum @@ -1130,6 +1130,8 @@ github.com/openzipkin-contrib/zipkin-go-opentracing v0.4.5/go.mod h1:/wsWhb9smxS github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJc5AZX7/PBEpw= github.com/openzipkin/zipkin-go v0.2.1/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4= github.com/openzipkin/zipkin-go v0.2.2/go.mod h1:NaW6tEwdmWMaCDZzg8sh+IBNOxHMPnhQw8ySjnjRyN4= +github.com/oras-project/artifacts-spec v1.0.0-draft.1.1 h1:2YMUDyDH0glYA4gNG/zEg9HNVzgGX8kr/NBLR9AQkLQ= +github.com/oras-project/artifacts-spec v1.0.0-draft.1.1/go.mod h1:Xch2aLzSwtkhbFFN6LUzTfLtukYvMMdXJ4oZ8O7BOdc= github.com/pact-foundation/pact-go v1.0.4/go.mod h1:uExwJY4kCzNPcHRj+hCR/HBbOOIwwtUjcrb0b5/5kLM= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= @@ -2206,6 +2208,8 @@ nhooyr.io/websocket v1.8.7 h1:usjR2uOr/zjjkVMy0lW+PPohFok7PCow5sDjLgX4P4g= nhooyr.io/websocket v1.8.7/go.mod h1:B70DZP8IakI65RVQ51MsWP/8jndNma26DVA/nFSCgW0= oras.land/oras-go v1.1.1 h1:gI00ftziRivKXaw1BdMeEoIA4uBgga33iVlOsEwefFs= oras.land/oras-go v1.1.1/go.mod h1:n2TE1ummt9MUyprGhT+Q7kGZUF4kVUpYysPFxeV2IpQ= +oras.land/oras-go/v2 v2.0.0-20220621073716-14422086e418 h1:eFXe4uXwdAQA9LLsaAqyscEB99ngao9xTVoPMYQoaSI= +oras.land/oras-go/v2 v2.0.0-20220621073716-14422086e418/go.mod h1:0IQiLwHUJuMs0+QYGavaeQWw5FD4ABD/RP5YamXT/sc= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/pdf v0.1.1/go.mod h1:n8OzWcQ6Sp37PL01nO98y4iUCRdTGarVfzxY20ICaU4= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= From c17652e5fa1c6b072e57245158940b09e472aff8 Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Wed, 22 Jun 2022 23:49:27 -0700 Subject: [PATCH 02/11] incremental --- .../packages/v1alpha1/chart_integration_test.go | 15 ++++++++++++++- .../v1alpha1/docker_reg_v2_repo_lister.go | 5 ++++- .../packages/v1alpha1/global_vars_test.go | 17 +++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go index 12edc35ceda..df5f4331206 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go @@ -12,7 +12,10 @@ import ( "time" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" corev1 "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/core/packages/v1alpha1" + plugins "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/core/plugins/v1alpha1" "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common" "golang.org/x/sync/semaphore" "google.golang.org/grpc/codes" @@ -579,5 +582,15 @@ func TestKindClusterGetAvailablePackageSummariesForOCI(t *testing.T) { t.Fatalf("%v", err) } - t.Logf("=======> %s", common.PrettyPrint(resp)) + opt1 := cmpopts.IgnoreUnexported( + corev1.GetAvailablePackageSummariesResponse{}, + corev1.AvailablePackageSummary{}, + corev1.AvailablePackageReference{}, + corev1.Context{}, + plugins.Plugin{}, + corev1.PackageAppVersion{}) + opt2 := cmpopts.SortSlices(lessAvailablePackageFunc) + if got, want := resp, oci_stefanprodan_podinfo_available_summaries(repoName.Name); !cmp.Equal(got, want, opt1, opt2) { + t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, opt1, opt2)) + } } diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go index 9f80508f9c7..49e120d9a6e 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go @@ -65,7 +65,10 @@ func (l *dockerRegistryApiV2RepositoryLister) ListRepositoryNames(ociRegistry *O log.Infof("orasRegistry.Repositories fn: %s", repos) return done } - // TODO need to append + + // see https://github.com/vmware-tanzu/kubeapps/pull/4932#issuecomment-1164004999 + // and https://github.com/oras-project/oras-go/issues/196 + // TODO (gfichtenholt) need to append // "?last=" + orasRegistry.Reference.Repository // to req.Query so we don't start at the beggining of the alphabet 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 273c3ae8dc7..ce181360436 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 @@ -3123,4 +3123,21 @@ var ( delete_repo_req_6 = &corev1.DeletePackageRepositoryRequest{ PackageRepoRef: repoRefInReq("my-podinfo-4", "TBD"), } + + oci_stefanprodan_podinfo_available_summaries = func(name string) *corev1.GetAvailablePackageSummariesResponse { + return &corev1.GetAvailablePackageSummariesResponse{ + AvailablePackageSummaries: []*corev1.AvailablePackageSummary{ + { + Name: "podinfo", + AvailablePackageRef: availableRef(name+"/podinfo", "default"), + LatestVersion: &corev1.PackageAppVersion{ + PkgVersion: "6.1.6", + }, + DisplayName: "podinfo", + ShortDescription: "Podinfo Helm chart for Kubernetes", + Categories: []string{""}, + }, + }, + } + } ) From 03cf27a1d9fb8fd2f0573855928245bb9329c982 Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Sat, 25 Jun 2022 20:55:42 -0700 Subject: [PATCH 03/11] incremental --- .../templates/kubeappsapis/rbac_fluxv2.yaml | 12 ++ .../v1alpha1/chart_integration_test.go | 16 +- .../fluxv2/packages/v1alpha1/oci_repo.go | 174 +++++++++--------- .../plugins/fluxv2/packages/v1alpha1/repo.go | 8 +- 4 files changed, 126 insertions(+), 84 deletions(-) diff --git a/chart/kubeapps/templates/kubeappsapis/rbac_fluxv2.yaml b/chart/kubeapps/templates/kubeappsapis/rbac_fluxv2.yaml index 190937bcf5e..030fcd0ec7f 100644 --- a/chart/kubeapps/templates/kubeappsapis/rbac_fluxv2.yaml +++ b/chart/kubeapps/templates/kubeappsapis/rbac_fluxv2.yaml @@ -20,6 +20,18 @@ rules: - apiGroups: ["apiextensions.k8s.io"] resources: ["customresourcedefinitions"] verbs: ["get", "list"] + # Temp hack to avoid + # Failed to read secret for repo due to: rpc error: code = PermissionDenied desc = Forbidden + # to get the secret 'helm-podinfo' due to 'secrets "helm-podinfo" is forbidden: + # User "system:serviceaccount:kubeapps:kubeapps-internal-kubeappsapis" cannot get resource + # "secrets" in API group "" in the namespace "default"' + # see discussion in https://github.com/vmware-tanzu/kubeapps/pull/4932#issuecomment-1161243049 + - apiGroups: + - "" + resources: + - secrets + verbs: + - get --- apiVersion: {{ include "common.capabilities.rbac.apiVersion" . }} kind: ClusterRoleBinding diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go index df5f4331206..3432a495a4d 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go @@ -517,7 +517,7 @@ func TestKindClusterRepoAndChartRBAC(t *testing.T) { } } -func TestKindClusterGetAvailablePackageSummariesForOCI(t *testing.T) { +func TestKindClusterGetAvailablePackageSummariesAndVersionsForOCI(t *testing.T) { fluxPluginClient, fluxPluginReposClient, err := checkEnv(t) if err != nil { t.Fatal(err) @@ -593,4 +593,18 @@ func TestKindClusterGetAvailablePackageSummariesForOCI(t *testing.T) { if got, want := resp, oci_stefanprodan_podinfo_available_summaries(repoName.Name); !cmp.Equal(got, want, opt1, opt2) { t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, opt1, opt2)) } + + resp2, err := fluxPluginClient.GetAvailablePackageVersions( + grpcContext, &corev1.GetAvailablePackageVersionsRequest{ + AvailablePackageRef: &corev1.AvailablePackageReference{ + Context: &corev1.Context{ + Namespace: "default", + }, + Identifier: repoName.Name + "/podinfo", + }, + }) + if err != nil { + t.Fatalf("%v", err) + } + t.Logf("=============> %s", common.PrettyPrint(resp2)) } diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go index d0e58edc3e8..415503a14a6 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go @@ -188,23 +188,12 @@ func (r *OCIRegistry) listRepositoryNames() ([]string, error) { return r.repositoryLister.ListRepositoryNames(r) } -// Get returns the ChartVersion for the given name, the version is expected +// pickChartVersionFrom returns the ChartVersion for the given name, the version is expected // to be a semver.Constraints compatible string. If version is empty, the latest // stable version will be returned and prerelease versions will be ignored. // adapted from https://github.com/helm/helm/blob/49819b4ef782e80b0c7f78c30bd76b51ebb56dc8/pkg/downloader/chart_downloader.go#L162 -func (r *OCIRegistry) getChartVersion(name, ver string) (*repo.ChartVersion, error) { - log.Infof("+getChartVersion(%s,%s)", name, ver) - // Find chart versions matching the given name. - // Either in an index file or from a registry. - ref := fmt.Sprintf("%s/%s", r.url.String(), name) - cvs, err := r.getTags(ref) - if err != nil { - return nil, err - } - - if len(cvs) == 0 { - return nil, status.Errorf(codes.Internal, "unable to locate any tags in provided repository: %s", name) - } +func (r *OCIRegistry) pickChartVersionFrom(name, ver string, cvs []string) (*repo.ChartVersion, error) { + log.Infof("+pickChartVersionFrom(%s,%s,%s)", name, ver, cvs) // Determine if version provided // If empty, try to get the highest available tag @@ -223,14 +212,18 @@ func (r *OCIRegistry) getChartVersion(name, ver string) (*repo.ChartVersion, err // This function shall be called for OCI registries only // It assumes that the ref has been validated to be an OCI reference. func (r *OCIRegistry) getTags(ref string) ([]string, error) { + log.Infof("+getTags(%s)", ref) + defer log.Infof("-getTags(%s)", ref) + ref = strings.TrimPrefix(ref, fmt.Sprintf("%s://", registry.OCIScheme)) + log.Infof("getTags: about to call .Tags(%s)", ref) tags, err := r.registryClient.Tags(ref) + log.Infof("getTags: done with call .Tags(%s): %s %v", ref, tags, err) if err != nil { return nil, err } - log.Infof("getTags(%s): %s %v", ref, tags, err) if len(tags) == 0 { return nil, fmt.Errorf("unable to locate any tags in provided repository: %s", ref) } @@ -381,85 +374,99 @@ func (s *repoEventSink) onAddOciRepo(repo sourcev1.HelmRepository) ([]byte, bool } charts := []models.Chart{} - for _, appName := range appNames { - appName, err := ociRegistry.shortRepoName(appName) + for _, fullAppName := range appNames { + appName, err := ociRegistry.shortRepoName(fullAppName) if err != nil { return nil, false, err } - log.Infof("==========>: app name: [%s]", appName) - - chartVersion, err := ociRegistry.getChartVersion(appName, "") - if err != nil { - return nil, false, status.Errorf(codes.Internal, "%v", err) - } - log.Infof("==========>: chart version: %s", common.PrettyPrint(chartVersion)) - - chartBuffer, err := ociRegistry.downloadChart(chartVersion) - if err != nil { - return nil, false, status.Errorf(codes.Internal, "%v", err) - } - log.Infof("==========>: chart buffer: [%d] bytes", chartBuffer.Len()) - // Encode repository names to store them in the database. encodedAppName := url.PathEscape(appName) - - // not sure yet why flux untars into a temp directory chartID := path.Join(repo.Name, encodedAppName) - files, err := tarutil.FetchChartDetailFromTarball( - bytes.NewReader(chartBuffer.Bytes()), chartID) - if err != nil { - return nil, false, status.Errorf(codes.Internal, "%v", err) - } - log.Infof("==========>: files: [%d]", len(files)) + log.Infof("==========>: app name: [%s], chartID: [%s]", appName, chartID) - chartYaml, ok := files[models.ChartYamlKey] - // TODO (gfichtenholt): if there is no chart yaml (is that even possible?), - // fall back to chart info from repo index.yaml - if !ok || chartYaml == "" { - return nil, false, status.Errorf(codes.Internal, "No chart manifest found for chart [%s]", chartID) - } - var chartMetadata chart.Metadata - err = yaml.Unmarshal([]byte(chartYaml), &chartMetadata) + ref := fmt.Sprintf("%s/%s", ociRegistry.url.String(), appName) + allTags, err := ociRegistry.getTags(ref) if err != nil { return nil, false, err } - log.Infof("==========>: chart metadata: %s", common.PrettyPrint(chartMetadata)) + var mc *models.Chart + // TODO (gfichtenholt) this loop needs to be implemented using + // multiple concurrent readers + for _, tag := range allTags { + chartVersion, err := ociRegistry.pickChartVersionFrom(appName, tag, allTags) + if err != nil { + return nil, false, status.Errorf(codes.Internal, "%v", err) + } + log.Infof("==========>: chart version: %s", common.PrettyPrint(chartVersion)) - maintainers := []chart.Maintainer{} - for _, maintainer := range chartMetadata.Maintainers { - maintainers = append(maintainers, *maintainer) - } + chartBuffer, err := ociRegistry.downloadChart(chartVersion) + if err != nil { + return nil, false, status.Errorf(codes.Internal, "%v", err) + } + log.Infof("==========>: chart buffer: [%d] bytes", chartBuffer.Len()) - modelsChartVersion := models.ChartVersion{ - Version: chartVersion.Version, - AppVersion: chartVersion.AppVersion, - Created: chartVersion.Created, - Digest: chartVersion.Digest, - URLs: chartVersion.URLs, - Readme: files[models.ReadmeKey], - Values: files[models.ValuesKey], - Schema: files[models.SchemaKey], - } + // not sure yet why flux untars into a temp directory + files, err := tarutil.FetchChartDetailFromTarball( + bytes.NewReader(chartBuffer.Bytes()), chartID) + if err != nil { + return nil, false, status.Errorf(codes.Internal, "%v", err) + } + + log.Infof("==========>: files: [%d]", len(files)) + + chartYaml, ok := files[models.ChartYamlKey] + // TODO (gfichtenholt): if there is no chart yaml (is that even possible?), + // fall back to chart info from repo index.yaml + if !ok || chartYaml == "" { + return nil, false, status.Errorf(codes.Internal, "No chart manifest found for chart [%s]", chartID) + } + var chartMetadata chart.Metadata + err = yaml.Unmarshal([]byte(chartYaml), &chartMetadata) + if err != nil { + return nil, false, err + } - chart := models.Chart{ - ID: path.Join(repo.Name, encodedAppName), - Name: encodedAppName, - Repo: chartRepo, - Description: chartMetadata.Description, - Home: chartMetadata.Home, - Keywords: chartMetadata.Keywords, - Maintainers: maintainers, - Sources: chartMetadata.Sources, - Icon: chartMetadata.Icon, - Category: chartMetadata.Annotations["category"], - ChartVersions: []models.ChartVersion{ - modelsChartVersion, - }, + log.Infof("==========>: chart metadata: %s", common.PrettyPrint(chartMetadata)) + + maintainers := []chart.Maintainer{} + for _, maintainer := range chartMetadata.Maintainers { + maintainers = append(maintainers, *maintainer) + } + + mcv := models.ChartVersion{ + Version: chartVersion.Version, + AppVersion: chartVersion.AppVersion, + Created: chartVersion.Created, + Digest: chartVersion.Digest, + URLs: chartVersion.URLs, + Readme: files[models.ReadmeKey], + Values: files[models.ValuesKey], + Schema: files[models.SchemaKey], + } + + if mc == nil { + mc = &models.Chart{ + ID: chartID, + Name: encodedAppName, + Repo: chartRepo, + Description: chartMetadata.Description, + Home: chartMetadata.Home, + Keywords: chartMetadata.Keywords, + Maintainers: maintainers, + Sources: chartMetadata.Sources, + Icon: chartMetadata.Icon, + Category: chartMetadata.Annotations["category"], + ChartVersions: []models.ChartVersion{}, + } + } + mc.ChartVersions = append(mc.ChartVersions, mcv) + } + if mc != nil { + charts = append(charts, *mc) } - charts = append(charts, chart) } checksum, err := ociRegistry.checksum() @@ -534,23 +541,26 @@ type TagList struct { // all repositories within the registry and returning the sha256. // Caveat: Mutated image tags won't be detected as new func (r *OCIRegistry) checksum() (string, error) { - log.Infof("+checksum") + log.Infof("+checksum()") + defer log.Infof("-checksum()") appNames, err := r.listRepositoryNames() if err != nil { return "", err } tags := map[string]TagList{} - for _, appName := range appNames { - appName, err := r.shortRepoName(appName) + for _, fullAppName := range appNames { + appName, err := r.shortRepoName(fullAppName) if err != nil { return "", err } ref := fmt.Sprintf("%s/%s", r.url.String(), appName) - tagss, err := r.getTags(ref) + tagz, err := r.getTags(ref) if err != nil { return "", err } - tags[appName] = TagList{Name: appName, Tags: tagss} + + //ref := fmt.Sprintf("%s/%s", r.url.String(), fullAppName) + tags[appName] = TagList{Name: appName, Tags: tagz} } content, err := json.Marshal(tags) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go index ef9831d890c..4e4cf4f1a5c 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go @@ -807,7 +807,7 @@ func (s *repoEventSink) indexOneRepo(repo sourcev1.HelmRepository) ([]models.Cha // this is potentially a very expensive operation for large repos like 'bitnami' // shallow = true => 8-9 sec - // shallow = false => 12-13 sec, so deep copy adds 50% to cost, but we need it to + // shallow = false => 12-13 sec, so deep copy adds 50% to cost, but we need it // for GetAvailablePackageVersions() charts, err := helm.ChartsFromIndex(byteArray, modelRepo, false) if err != nil { @@ -818,11 +818,17 @@ func (s *repoEventSink) indexOneRepo(repo sourcev1.HelmRepository) ([]models.Cha msg := fmt.Sprintf("-indexOneRepo: [%s], indexed [%d] packages in [%d] ms", repo.Name, len(charts), duration.Milliseconds()) if len(charts) > 0 { log.Info(msg) + log.Infof("%s", common.PrettyPrint(charts)) } else { // this is kind of a red flag - an index with 0 charts, most likely contents of index.yaml is // messed up and didn't parse successfully but the helm library didn't raise an error log.Warning(msg) } + // note that we are returning an array of model.Chart, each of which has an + // array of model.ChartVersions, which in turn, only has those fields initialized that + // can be read from index.yaml. Fields like readme, schema, values are empty at this point. + // They do not get stored in the repo cache. They get stored in the chart cache + // in a .tgz file return charts, nil } From 31c0ed731241883d79f806b0cdc9dd7dddd5f1d0 Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Sat, 25 Jun 2022 22:20:11 -0700 Subject: [PATCH 04/11] incremental --- .../v1alpha1/chart_integration_test.go | 10 +- .../packages/v1alpha1/global_vars_test.go | 8 ++ .../fluxv2/packages/v1alpha1/oci_repo.go | 125 ++++++++++-------- 3 files changed, 85 insertions(+), 58 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go index 3432a495a4d..d8aaf699204 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go @@ -606,5 +606,13 @@ func TestKindClusterGetAvailablePackageSummariesAndVersionsForOCI(t *testing.T) if err != nil { t.Fatalf("%v", err) } - t.Logf("=============> %s", common.PrettyPrint(resp2)) + if err != nil { + t.Fatal(err) + } + opts := cmpopts.IgnoreUnexported( + corev1.GetAvailablePackageVersionsResponse{}, + corev1.PackageAppVersion{}) + if got, want := resp2, expected_versions_stefanprodan_podinfo; !cmp.Equal(want, got, opts) { + t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, opts)) + } } 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 ce181360436..32de7b467fd 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 @@ -1950,6 +1950,14 @@ var ( }, } + expected_versions_stefanprodan_podinfo = &corev1.GetAvailablePackageVersionsResponse{ + PackageAppVersions: []*corev1.PackageAppVersion{ + {PkgVersion: "6.1.6"}, + {PkgVersion: "6.1.5"}, + {PkgVersion: "6.1.4"}, + }, + } + create_package_simple_req = &corev1.CreateInstalledPackageRequest{ AvailablePackageRef: availableRef("podinfo/podinfo", "namespace-1"), Name: "my-podinfo", diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go index 415503a14a6..e00ae5f7a5f 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go @@ -392,7 +392,40 @@ func (s *repoEventSink) onAddOciRepo(repo sourcev1.HelmRepository) ([]byte, bool return nil, false, err } - var mc *models.Chart + // to be consistent with how we support helm http repos + // the chart fields like Desciption, home, sources come from the + // most recent chart version + // ref https://github.com/vmware-tanzu/kubeapps/blob/11c87926d6cd798af72875d01437d15ae8d85b9a/pkg/helm/index.go#L30 + log.Infof("==========>: most recent chart version: %s", allTags[0]) + latestChartVersion, err := ociRegistry.pickChartVersionFrom(appName, allTags[0], allTags) + if err != nil { + return nil, false, status.Errorf(codes.Internal, "%v", err) + } + + latestChartMetadata, err := s.getChartMetadata(ociRegistry, chartID, latestChartVersion) + if err != nil { + return nil, false, err + } + + maintainers := []chart.Maintainer{} + for _, maintainer := range latestChartMetadata.Maintainers { + maintainers = append(maintainers, *maintainer) + } + + mc := models.Chart{ + ID: chartID, + Name: encodedAppName, + Repo: chartRepo, + Description: latestChartMetadata.Description, + Home: latestChartMetadata.Home, + Keywords: latestChartMetadata.Keywords, + Maintainers: maintainers, + Sources: latestChartMetadata.Sources, + Icon: latestChartMetadata.Icon, + Category: latestChartMetadata.Annotations["category"], + ChartVersions: []models.ChartVersion{}, + } + // TODO (gfichtenholt) this loop needs to be implemented using // multiple concurrent readers for _, tag := range allTags { @@ -402,71 +435,16 @@ func (s *repoEventSink) onAddOciRepo(repo sourcev1.HelmRepository) ([]byte, bool } log.Infof("==========>: chart version: %s", common.PrettyPrint(chartVersion)) - chartBuffer, err := ociRegistry.downloadChart(chartVersion) - if err != nil { - return nil, false, status.Errorf(codes.Internal, "%v", err) - } - log.Infof("==========>: chart buffer: [%d] bytes", chartBuffer.Len()) - - // not sure yet why flux untars into a temp directory - files, err := tarutil.FetchChartDetailFromTarball( - bytes.NewReader(chartBuffer.Bytes()), chartID) - if err != nil { - return nil, false, status.Errorf(codes.Internal, "%v", err) - } - - log.Infof("==========>: files: [%d]", len(files)) - - chartYaml, ok := files[models.ChartYamlKey] - // TODO (gfichtenholt): if there is no chart yaml (is that even possible?), - // fall back to chart info from repo index.yaml - if !ok || chartYaml == "" { - return nil, false, status.Errorf(codes.Internal, "No chart manifest found for chart [%s]", chartID) - } - var chartMetadata chart.Metadata - err = yaml.Unmarshal([]byte(chartYaml), &chartMetadata) - if err != nil { - return nil, false, err - } - - log.Infof("==========>: chart metadata: %s", common.PrettyPrint(chartMetadata)) - - maintainers := []chart.Maintainer{} - for _, maintainer := range chartMetadata.Maintainers { - maintainers = append(maintainers, *maintainer) - } - mcv := models.ChartVersion{ Version: chartVersion.Version, AppVersion: chartVersion.AppVersion, Created: chartVersion.Created, Digest: chartVersion.Digest, URLs: chartVersion.URLs, - Readme: files[models.ReadmeKey], - Values: files[models.ValuesKey], - Schema: files[models.SchemaKey], - } - - if mc == nil { - mc = &models.Chart{ - ID: chartID, - Name: encodedAppName, - Repo: chartRepo, - Description: chartMetadata.Description, - Home: chartMetadata.Home, - Keywords: chartMetadata.Keywords, - Maintainers: maintainers, - Sources: chartMetadata.Sources, - Icon: chartMetadata.Icon, - Category: chartMetadata.Annotations["category"], - ChartVersions: []models.ChartVersion{}, - } } mc.ChartVersions = append(mc.ChartVersions, mcv) } - if mc != nil { - charts = append(charts, *mc) - } + charts = append(charts, mc) } checksum, err := ociRegistry.checksum() @@ -670,3 +648,36 @@ func (s *repoEventSink) clientOptionsForRepo(repo sourcev1.HelmRepository) ([]re } return loginOpts, getterOpts, cred, nil } + +func (s *repoEventSink) getChartMetadata(ociRegistry *OCIRegistry, chartID string, chartVersion *repo.ChartVersion) (*chart.Metadata, error) { + log.Infof("getChartMetadata(%s, %s)", chartID, chartVersion.Metadata.Version) + + chartBuffer, err := ociRegistry.downloadChart(chartVersion) + if err != nil { + return nil, status.Errorf(codes.Internal, "%v", err) + } + log.Infof("==========>: chart buffer: [%d] bytes", chartBuffer.Len()) + + // not sure yet why flux untars into a temp directory + files, err := tarutil.FetchChartDetailFromTarball( + bytes.NewReader(chartBuffer.Bytes()), chartID) + if err != nil { + return nil, status.Errorf(codes.Internal, "%v", err) + } + + log.Infof("==========>: files: [%d]", len(files)) + + chartYaml, ok := files[models.ChartYamlKey] + // TODO (gfichtenholt): if there is no chart yaml (is that even possible?), + // fall back to chart info from repo index.yaml + if !ok || chartYaml == "" { + return nil, status.Errorf(codes.Internal, "No chart manifest found for chart [%s]", chartID) + } + var chartMetadata chart.Metadata + err = yaml.Unmarshal([]byte(chartYaml), &chartMetadata) + if err != nil { + return nil, err + } + log.Infof("==========>: chart metadata: %s", common.PrettyPrint(chartMetadata)) + return &chartMetadata, nil +} From 94d6eb14efa64d74f0f499fcf8e2e5a4f5ac1e65 Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Sun, 26 Jun 2022 21:07:10 -0700 Subject: [PATCH 05/11] incremental --- .../packages/v1alpha1/cache/chart_cache.go | 61 ++++++++----------- .../plugins/fluxv2/packages/v1alpha1/chart.go | 28 ++++++++- .../v1alpha1/chart_integration_test.go | 34 +++++++++-- .../fluxv2/packages/v1alpha1/chart_test.go | 9 ++- .../packages/v1alpha1/global_vars_test.go | 22 ++++++- .../fluxv2/packages/v1alpha1/oci_repo.go | 48 ++++++++++++--- .../plugins/fluxv2/packages/v1alpha1/repo.go | 2 +- 7 files changed, 148 insertions(+), 56 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache/chart_cache.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache/chart_cache.go index 35d11f7d31e..f939ee76a6c 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache/chart_cache.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache/chart_cache.go @@ -7,7 +7,6 @@ import ( "bytes" "encoding/gob" "fmt" - "io/ioutil" "net/url" "os" "reflect" @@ -19,7 +18,6 @@ import ( "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common" "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/pkgutils" "github.com/vmware-tanzu/kubeapps/pkg/chart/models" - httpclient "github.com/vmware-tanzu/kubeapps/pkg/http-client" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "k8s.io/apimachinery/pkg/types" @@ -73,18 +71,20 @@ type ChartCache struct { resyncCh chan int } +type DownloadChartFn func(chartID, chartUrl, chartVersion string) ([]byte, error) + // chartCacheStoreEntry is what we'll be storing in the processing store // note that url and delete fields are mutually exclusive, you must either: // - set url to non-empty string or // - deleted flag to true // setting both for a given entry does not make sense type chartCacheStoreEntry struct { - namespace string - id string - version string - url string - clientOptions *common.HttpClientOptions - deleted bool + namespace string + id string + version string + url string + downloadFn DownloadChartFn + deleted bool } func NewChartCache(name string, redisCli *redis.Client, stopCh <-chan struct{}) (*ChartCache, error) { @@ -118,7 +118,7 @@ func NewChartCache(name string, redisCli *redis.Client, stopCh <-chan struct{}) // this func will enqueue work items into chart work queue and return. // the charts will be synced worker threads running in the background -func (c *ChartCache) SyncCharts(charts []models.Chart, clientOptions *common.HttpClientOptions) error { +func (c *ChartCache) SyncCharts(charts []models.Chart, downloadFn DownloadChartFn) error { log.Infof("+SyncCharts()") totalToSync := 0 defer func() { @@ -163,12 +163,12 @@ func (c *ChartCache) SyncCharts(charts []models.Chart, clientOptions *common.Htt } entry := chartCacheStoreEntry{ - namespace: chart.Repo.Namespace, - id: chart.ID, - version: chart.ChartVersions[0].Version, - url: u.String(), - clientOptions: clientOptions, - deleted: false, + namespace: chart.Repo.Namespace, + id: chart.ID, + version: chart.ChartVersions[0].Version, + url: u.String(), + downloadFn: downloadFn, + deleted: false, } if key, err := chartCacheKeyFunc(entry); err != nil { log.Errorf("Failed to get key for chart due to %+v", err) @@ -393,7 +393,7 @@ func (c *ChartCache) syncHandler(workerName, key string) error { return nil } } - byteArray, err := ChartCacheComputeValue(chart.id, chart.url, chart.version, chart.clientOptions) + byteArray, err := ChartCacheComputeValue(chart.id, chart.url, chart.version, chart.downloadFn) if err != nil { return err } @@ -454,7 +454,7 @@ func (c *ChartCache) FetchForOne(key string) ([]byte, error) { • otherwise return the bytes stored in the chart cache for the given entry */ -func (c *ChartCache) GetForOne(key string, chart *models.Chart, clientOptions *common.HttpClientOptions) ([]byte, error) { +func (c *ChartCache) GetForOne(key string, chart *models.Chart, downloadFn DownloadChartFn) ([]byte, error) { // TODO (gfichtenholt) it'd be nice to get rid of all arguments except for the key, similar to that of // NamespacedResourceWatcherCache.GetForOne() log.Infof("+GetForOne(%s)", key) @@ -478,11 +478,11 @@ func (c *ChartCache) GetForOne(key string, chart *models.Chart, clientOptions *c log.Warningf("chart: [%s], version: [%s] has no URLs", chart.ID, v.Version) } else { entry = &chartCacheStoreEntry{ - namespace: namespace, - id: chartID, - version: v.Version, - url: v.URLs[0], - clientOptions: clientOptions, + namespace: namespace, + id: chartID, + version: v.Version, + url: v.URLs[0], + downloadFn: downloadFn, } } break @@ -599,21 +599,8 @@ func chartCacheKeyFor(namespace, chartID, chartVersion string) (string, error) { } // FYI: The work queue is able to retry transient HTTP errors -func ChartCacheComputeValue(chartID, chartUrl, chartVersion string, clientOptions *common.HttpClientOptions) ([]byte, error) { - client, headers, err := common.NewHttpClientAndHeaders(clientOptions) - if err != nil { - return nil, err - } - - reader, _, err := httpclient.GetStream(chartUrl, client, headers) - if reader != nil { - defer reader.Close() - } - if err != nil { - return nil, err - } - - chartTgz, err := ioutil.ReadAll(reader) +func ChartCacheComputeValue(chartID, chartUrl, chartVersion string, downloadFn DownloadChartFn) ([]byte, error) { + chartTgz, err := downloadFn(chartID, chartUrl, chartVersion) if err != nil { return nil, err } diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go index 91cb82be9f7..b971d614b64 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "fmt" + "io/ioutil" "reflect" "strings" @@ -16,6 +17,7 @@ import ( "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/pkgutils" "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/statuserror" "github.com/vmware-tanzu/kubeapps/pkg/chart/models" + httpclient "github.com/vmware-tanzu/kubeapps/pkg/http-client" "github.com/vmware-tanzu/kubeapps/pkg/tarutil" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -85,7 +87,7 @@ func (s *Server) availableChartDetail(ctx context.Context, packageRef *corev1.Av return nil, err } else if opts, err := s.clientOptionsForRepo(ctx, repoName); err != nil { return nil, err - } else if byteArray, err = s.chartCache.GetForOne(key, chartModel, opts); err != nil { + } else if byteArray, err = s.chartCache.GetForOne(key, chartModel, downloadChartViaHttpFn(opts)); err != nil { return nil, err } else if byteArray == nil { return nil, status.Errorf(codes.Internal, "failed to load details for chart [%s]", chartModel.ID) @@ -286,3 +288,27 @@ func availablePackageDetailFromChartDetail(chartID string, chartDetail map[strin // is not included in the tarball return pkg, nil } + +func downloadChartViaHttpFn(options *common.HttpClientOptions) func(chartID, chartUrl, chartVersion string) ([]byte, error) { + return func(chartID, chartUrl, chartVersion string) ([]byte, error) { + client, headers, err := common.NewHttpClientAndHeaders(options) + if err != nil { + return nil, err + } + + reader, _, err := httpclient.GetStream(chartUrl, client, headers) + if reader != nil { + defer reader.Close() + } + if err != nil { + return nil, err + } + + chartTgz, err := ioutil.ReadAll(reader) + if err != nil { + return nil, err + } + + return chartTgz, nil + } +} diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go index d8aaf699204..e7eba604fec 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go @@ -517,7 +517,7 @@ func TestKindClusterRepoAndChartRBAC(t *testing.T) { } } -func TestKindClusterGetAvailablePackageSummariesAndVersionsForOCI(t *testing.T) { +func TestKindClusterAvailablePackageEndpointsForOCI(t *testing.T) { fluxPluginClient, fluxPluginReposClient, err := checkEnv(t) if err != nil { t.Fatal(err) @@ -573,7 +573,7 @@ func TestKindClusterGetAvailablePackageSummariesAndVersionsForOCI(t *testing.T) t.Fatal(err) } - grpcContext, cancel = context.WithTimeout(grpcContext, 90*time.Second) + grpcContext, cancel = context.WithTimeout(grpcContext, defaultContextTimeout) defer cancel() resp, err := fluxPluginClient.GetAvailablePackageSummaries( grpcContext, @@ -590,10 +590,12 @@ func TestKindClusterGetAvailablePackageSummariesAndVersionsForOCI(t *testing.T) plugins.Plugin{}, corev1.PackageAppVersion{}) opt2 := cmpopts.SortSlices(lessAvailablePackageFunc) - if got, want := resp, oci_stefanprodan_podinfo_available_summaries(repoName.Name); !cmp.Equal(got, want, opt1, opt2) { + if got, want := resp, expected_oci_stefanprodan_podinfo_available_summaries(repoName.Name); !cmp.Equal(got, want, opt1, opt2) { t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, opt1, opt2)) } + grpcContext, cancel = context.WithTimeout(grpcContext, defaultContextTimeout) + defer cancel() resp2, err := fluxPluginClient.GetAvailablePackageVersions( grpcContext, &corev1.GetAvailablePackageVersionsRequest{ AvailablePackageRef: &corev1.AvailablePackageReference{ @@ -603,9 +605,6 @@ func TestKindClusterGetAvailablePackageSummariesAndVersionsForOCI(t *testing.T) Identifier: repoName.Name + "/podinfo", }, }) - if err != nil { - t.Fatalf("%v", err) - } if err != nil { t.Fatal(err) } @@ -615,4 +614,27 @@ func TestKindClusterGetAvailablePackageSummariesAndVersionsForOCI(t *testing.T) if got, want := resp2, expected_versions_stefanprodan_podinfo; !cmp.Equal(want, got, opts) { t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got, opts)) } + + grpcContext, cancel = context.WithTimeout(grpcContext, defaultContextTimeout) + defer cancel() + resp3, err := fluxPluginClient.GetAvailablePackageDetail( + grpcContext, + &corev1.GetAvailablePackageDetailRequest{ + AvailablePackageRef: &corev1.AvailablePackageReference{ + Context: &corev1.Context{ + Namespace: "default", + }, + Identifier: repoName.Name + "/podinfo", + }, + }) + if err != nil { + t.Fatal(err) + } + + compareActualVsExpectedAvailablePackageDetail( + t, + resp3.AvailablePackageDetail, + expected_detail_oci_stefanprodan_podinfo(repoName.Name).AvailablePackageDetail) + + // TODO try a few older versions } diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_test.go index 466db6a749f..e8cbf66b817 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_test.go @@ -754,7 +754,8 @@ func TestChartCacheResyncNotIdle(t *testing.T) { t.Fatalf("%+v", err) } if i == 0 { - chartBytes, err = cache.ChartCacheComputeValue(chartID, ts.URL, chartVersion, opts) + fn := downloadChartViaHttpFn(opts) + chartBytes, err = cache.ChartCacheComputeValue(chartID, ts.URL, chartVersion, fn) if err != nil { t.Fatalf("%+v", err) } @@ -973,7 +974,8 @@ func (cs *repoEventSink) redisMockSetValueForChart(mock redismock.ClientMock, ke if err != nil { return err } - byteArray, err := cache.ChartCacheComputeValue(chartID, url, version, opts) + fn := downloadChartViaHttpFn(opts) + byteArray, err := cache.ChartCacheComputeValue(chartID, url, version, fn) if err != nil { return fmt.Errorf("chartCacheComputeValue failed due to: %+v", err) } @@ -994,7 +996,8 @@ func redisMockExpectGetFromChartCache(mock redismock.ClientMock, key, url string if err != nil { return err } - bytes, err := cache.ChartCacheComputeValue(chartID, url, version, opts) + fn := downloadChartViaHttpFn(opts) + bytes, err := cache.ChartCacheComputeValue(chartID, url, version, fn) if err != nil { return err } 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 32de7b467fd..65d3b83c7b6 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 @@ -3132,7 +3132,7 @@ var ( PackageRepoRef: repoRefInReq("my-podinfo-4", "TBD"), } - oci_stefanprodan_podinfo_available_summaries = func(name string) *corev1.GetAvailablePackageSummariesResponse { + expected_oci_stefanprodan_podinfo_available_summaries = func(name string) *corev1.GetAvailablePackageSummariesResponse { return &corev1.GetAvailablePackageSummariesResponse{ AvailablePackageSummaries: []*corev1.AvailablePackageSummary{ { @@ -3148,4 +3148,24 @@ var ( }, } } + + expected_detail_oci_stefanprodan_podinfo = func(name string) *corev1.GetAvailablePackageDetailResponse { + return &corev1.GetAvailablePackageDetailResponse{ + AvailablePackageDetail: &corev1.AvailablePackageDetail{ + AvailablePackageRef: availableRef(name+"/podinfo", "default"), + Name: "podinfo", + Version: pkgAppVersion("6.1.6"), + RepoUrl: "oci://ghcr.io/stefanprodan/charts", + HomeUrl: "https://github.com/stefanprodan/podinfo", + DisplayName: "podinfo", + ShortDescription: "Podinfo Helm chart for Kubernetes", + SourceUrls: []string{"https://github.com/stefanprodan/podinfo"}, + Maintainers: []*corev1.Maintainer{ + {Name: "stefanprodan", Email: "stefanprodan@users.noreply.github.com"}, + }, + Readme: "Podinfo is a tiny web application made with Go", + DefaultValues: "Default values for podinfo.\n\nreplicaCount: 1\n", + }, + } + } ) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go index e00ae5f7a5f..30c934d6725 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go @@ -35,6 +35,7 @@ import ( "github.com/Masterminds/semver/v3" "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common" "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common/transport" + "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/pkgutils" "github.com/vmware-tanzu/kubeapps/pkg/chart/models" "github.com/vmware-tanzu/kubeapps/pkg/tarutil" log "k8s.io/klog/v2" @@ -402,7 +403,7 @@ func (s *repoEventSink) onAddOciRepo(repo sourcev1.HelmRepository) ([]byte, bool return nil, false, status.Errorf(codes.Internal, "%v", err) } - latestChartMetadata, err := s.getChartMetadata(ociRegistry, chartID, latestChartVersion) + latestChartMetadata, err := s.getOCIChartMetadata(ociRegistry, chartID, latestChartVersion) if err != nil { return nil, false, err } @@ -463,6 +464,14 @@ func (s *repoEventSink) onAddOciRepo(repo sourcev1.HelmRepository) ([]byte, bool if err = enc.Encode(cacheEntryValue); err != nil { return nil, false, err } + + if s.chartCache != nil { + fn := s.downloadOCIChartFn(ociRegistry) + if err = s.chartCache.SyncCharts(charts, fn); err != nil { + return nil, false, err + } + } + return buf.Bytes(), true, nil } @@ -497,6 +506,7 @@ func (s *repoEventSink) onModifyOciRepo(key string, oldValue interface{}, repo s } if cacheEntry.Checksum != newChecksum { + // TODO (gfichtenholt) return nil, false, status.Errorf(codes.Unimplemented, "TODO") } else { // skip because the content did not change @@ -649,18 +659,25 @@ func (s *repoEventSink) clientOptionsForRepo(repo sourcev1.HelmRepository) ([]re return loginOpts, getterOpts, cred, nil } -func (s *repoEventSink) getChartMetadata(ociRegistry *OCIRegistry, chartID string, chartVersion *repo.ChartVersion) (*chart.Metadata, error) { - log.Infof("getChartMetadata(%s, %s)", chartID, chartVersion.Metadata.Version) - +func (s *repoEventSink) getOCIChartTarball(ociRegistry *OCIRegistry, chartID string, chartVersion *repo.ChartVersion) ([]byte, error) { chartBuffer, err := ociRegistry.downloadChart(chartVersion) if err != nil { return nil, status.Errorf(codes.Internal, "%v", err) } - log.Infof("==========>: chart buffer: [%d] bytes", chartBuffer.Len()) + return chartBuffer.Bytes(), nil +} + +func (s *repoEventSink) getOCIChartMetadata(ociRegistry *OCIRegistry, chartID string, chartVersion *repo.ChartVersion) (*chart.Metadata, error) { + log.Infof("getOCIChartMetadata(%s, %s)", chartID, chartVersion.Metadata.Version) + + chartTarball, err := s.getOCIChartTarball(ociRegistry, chartID, chartVersion) + if err != nil { + return nil, status.Errorf(codes.Internal, "%v", err) + } + log.Infof("==========>: chart .tgz: [%d] bytes", len(chartTarball)) // not sure yet why flux untars into a temp directory - files, err := tarutil.FetchChartDetailFromTarball( - bytes.NewReader(chartBuffer.Bytes()), chartID) + files, err := tarutil.FetchChartDetailFromTarball(bytes.NewReader(chartTarball), chartID) if err != nil { return nil, status.Errorf(codes.Internal, "%v", err) } @@ -681,3 +698,20 @@ func (s *repoEventSink) getChartMetadata(ociRegistry *OCIRegistry, chartID strin log.Infof("==========>: chart metadata: %s", common.PrettyPrint(chartMetadata)) return &chartMetadata, nil } + +func (s *repoEventSink) downloadOCIChartFn(ociRegistry *OCIRegistry) func(chartID, chartUrl, chartVersion string) ([]byte, error) { + return func(chartID, chartUrl, chartVersion string) ([]byte, error) { + _, chartName, err := pkgutils.SplitPackageIdentifier(chartID) + if err != nil { + return nil, err + } + cv := &repo.ChartVersion{ + URLs: []string{chartUrl}, + Metadata: &chart.Metadata{ + Name: chartName, + Version: chartVersion, + }, + } + return s.getOCIChartTarball(ociRegistry, chartID, cv) + } +} diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go index 4e4cf4f1a5c..4cbe5f75659 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go @@ -763,7 +763,7 @@ func (s *repoEventSink) indexAndEncode(checksum string, repo sourcev1.HelmReposi // resource "secrets" in API group "" in the namespace "default" // So we still finish the indexing of the repo but skip the charts log.Errorf("Failed to read secret for repo due to: %+v", err) - } else if err = s.chartCache.SyncCharts(charts, opts); err != nil { + } else if err = s.chartCache.SyncCharts(charts, downloadChartViaHttpFn(opts)); err != nil { return nil, false, err } } From a3a31f794a3afcf1e59fe2e777e6b622bf75760e Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Tue, 28 Jun 2022 01:39:25 -0700 Subject: [PATCH 06/11] incremental --- .../plugins/fluxv2/packages/v1alpha1/common/utils.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common/utils.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common/utils.go index 2a0190f67b3..9b2ee30f7fa 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common/utils.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common/utils.go @@ -15,6 +15,7 @@ import ( "net/url" "os" "reflect" + "runtime" "strconv" "strings" "sync" @@ -484,3 +485,11 @@ func GetSha256(src []byte) (string, error) { } return fmt.Sprintf("%x", h.Sum(nil)), nil } + +// https://stackoverflow.com/questions/28712397/put-stack-trace-to-string-variable +func GetStackTrace() string { + // adjust buffer size to be larger than expected stack + b := make([]byte, 2048) + n := runtime.Stack(b, false) + return string(b[:n]) +} From f23d31086d5ab632cb9f7dba5b8206a01e0729fc Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Tue, 28 Jun 2022 01:41:40 -0700 Subject: [PATCH 07/11] incremental --- .../v1alpha1/chart_integration_test.go | 23 ++++++++++++++++++- .../packages/v1alpha1/global_vars_test.go | 20 ++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go index e7eba604fec..1476991b2ed 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_integration_test.go @@ -636,5 +636,26 @@ func TestKindClusterAvailablePackageEndpointsForOCI(t *testing.T) { resp3.AvailablePackageDetail, expected_detail_oci_stefanprodan_podinfo(repoName.Name).AvailablePackageDetail) - // TODO try a few older versions + // try a few older versions + grpcContext, cancel = context.WithTimeout(grpcContext, defaultContextTimeout) + defer cancel() + resp4, err := fluxPluginClient.GetAvailablePackageDetail( + grpcContext, + &corev1.GetAvailablePackageDetailRequest{ + AvailablePackageRef: &corev1.AvailablePackageReference{ + Context: &corev1.Context{ + Namespace: "default", + }, + Identifier: repoName.Name + "/podinfo", + }, + PkgVersion: "6.1.5", + }) + if err != nil { + t.Fatal(err) + } + + compareActualVsExpectedAvailablePackageDetail( + t, + resp4.AvailablePackageDetail, + expected_detail_oci_stefanprodan_podinfo_2(repoName.Name).AvailablePackageDetail) } 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 65d3b83c7b6..3dc4113d600 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 @@ -3168,4 +3168,24 @@ var ( }, } } + + expected_detail_oci_stefanprodan_podinfo_2 = func(name string) *corev1.GetAvailablePackageDetailResponse { + return &corev1.GetAvailablePackageDetailResponse{ + AvailablePackageDetail: &corev1.AvailablePackageDetail{ + AvailablePackageRef: availableRef(name+"/podinfo", "default"), + Name: "podinfo", + Version: pkgAppVersion("6.1.5"), + RepoUrl: "oci://ghcr.io/stefanprodan/charts", + HomeUrl: "https://github.com/stefanprodan/podinfo", + DisplayName: "podinfo", + ShortDescription: "Podinfo Helm chart for Kubernetes", + SourceUrls: []string{"https://github.com/stefanprodan/podinfo"}, + Maintainers: []*corev1.Maintainer{ + {Name: "stefanprodan", Email: "stefanprodan@users.noreply.github.com"}, + }, + Readme: "Podinfo is a tiny web application made with Go", + DefaultValues: "Default values for podinfo.\n\nreplicaCount: 1\n", + }, + } + } ) From 0eef938dd63ac4dc170d8ec163a7e635e0d98b9e Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Tue, 28 Jun 2022 01:50:17 -0700 Subject: [PATCH 08/11] incremental --- .../fluxv2/packages/v1alpha1/repo_test.go | 12 ++++++---- .../fluxv2/packages/v1alpha1/server.go | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) 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 087cb915aa2..c207d4eb7b3 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go @@ -2285,11 +2285,15 @@ func redisMockSetValueForRepo(mock redismock.ClientMock, key string, newValue, o mock.ExpectInfo("memory").SetVal("used_memory_rss_human:NA\r\nmaxmemory_human:NA") } -func (s *Server) redisKeyValueForRepo(r sourcev1.HelmRepository) (key string, byteArray []byte, err error) { - sink := repoEventSink{ - clientGetter: s.newBackgroundClientGetter(), - chartCache: nil, +func (s *Server) newRepoEventSinkNoCache() repoEventSink { + cg := func(ctx context.Context) (clientgetter.ClientInterfaces, error) { + return s.clientGetter(ctx, s.kubeappsCluster) } + return repoEventSink{clientGetter: cg} +} + +func (s *Server) redisKeyValueForRepo(r sourcev1.HelmRepository) (key string, byteArray []byte, err error) { + sink := s.newRepoEventSinkNoCache() return sink.redisKeyValueForRepo(r) } diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go index 31b50b18fae..fc02a1e93ef 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go @@ -675,3 +675,27 @@ func (s *Server) hasAccessToNamespace(ctx context.Context, gvr schema.GroupVersi func GetPluginDetail() *plugins.Plugin { return common.GetPluginDetail() } + +// makes the server look like a repo event sink. Facilitates code reuse between +// use cases when something happens in background as a result of a watch event, +// aka an "out-of-band" interaction and use cases when the user wants something +// done explicitly, aka "in-band" interaction +func (s *Server) newRepoEventSink() repoEventSink { + cg := func(ctx context.Context) (clientgetter.ClientInterfaces, error) { + return s.clientGetter(ctx, s.kubeappsCluster) + } + + // notice a bit of inconsistency here, we are using s.clientGetter + // (i.e. the context of the incoming request) to read the secret + // as opposed to s.repoCache.clientGetter (which uses the context of + // User "system:serviceaccount:kubeapps:kubeapps-internal-kubeappsapis") + // which is what is used when the repo is being processed/indexed. + // I don't think it's necessarily a bad thing if the incoming user's RBAC + // settings are more permissive than that of the default RBAC for + // kubeapps-internal-kubeappsapis account. If we don't like that behavior, + // I can easily switch to BackgroundClientGetter here + return repoEventSink{ + clientGetter: cg, + chartCache: s.chartCache, + } +} From df8618e833c308e164345971b799a65d7728a12b Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Tue, 28 Jun 2022 01:55:07 -0700 Subject: [PATCH 09/11] incremental --- .../plugins/fluxv2/packages/v1alpha1/chart.go | 30 ++++++++--- .../fluxv2/packages/v1alpha1/chart_test.go | 5 +- .../fluxv2/packages/v1alpha1/oci_repo.go | 54 ++++++++++++------- .../plugins/fluxv2/packages/v1alpha1/repo.go | 24 +++------ .../fluxv2/packages/v1alpha1/server.go | 47 +++++++--------- 5 files changed, 86 insertions(+), 74 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go index b971d614b64..0c007153d08 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go @@ -13,6 +13,7 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" corev1 "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/gen/core/packages/v1alpha1" + "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/cache" "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/common" "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/pkgutils" "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/statuserror" @@ -83,13 +84,30 @@ func (s *Server) availableChartDetail(ctx context.Context, packageRef *corev1.Av chartVersion = chartModel.ChartVersions[0].Version } - if key, err := s.chartCache.KeyFor(repoName.Namespace, chartID, chartVersion); err != nil { - return nil, err - } else if opts, err := s.clientOptionsForRepo(ctx, repoName); err != nil { + var key string + if key, err = s.chartCache.KeyFor(repoName.Namespace, chartID, chartVersion); err != nil { return nil, err - } else if byteArray, err = s.chartCache.GetForOne(key, chartModel, downloadChartViaHttpFn(opts)); err != nil { + } + + var fn cache.DownloadChartFn + if chartModel.Repo.Type == "oci" { + if ociRegistry, err := s.newOCIRegistryAndLoginWithRepo(ctx, repoName); err != nil { + return nil, err + } else { + fn = downloadOCIChartFn(ociRegistry) + } + } else { + if opts, err := s.httpClientOptionsForRepo(ctx, repoName); err != nil { + return nil, err + } else { + fn = downloadChartViaHttpFn(opts) + } + } + if byteArray, err = s.chartCache.GetForOne(key, chartModel, fn); err != nil { return nil, err - } else if byteArray == nil { + } + + if byteArray == nil { return nil, status.Errorf(codes.Internal, "failed to load details for chart [%s]", chartModel.ID) } } @@ -240,7 +258,7 @@ func availablePackageDetailFromChartDetail(chartID string, chartDetail map[strin // TODO (gfichtenholt): if there is no chart yaml (is that even possible?), // fall back to chart info from repo index.yaml if !ok || chartYaml == "" { - return nil, status.Errorf(codes.Internal, "No chart manifest found for chart [%s]", chartID) + return nil, status.Errorf(codes.Internal, "No chart manifest found for chart: [%s]", chartID) } var chartMetadata chart.Metadata err := yaml.Unmarshal([]byte(chartYaml), &chartMetadata) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_test.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_test.go index e8cbf66b817..eb70347358d 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart_test.go @@ -962,10 +962,7 @@ func newChart(name, namespace string, spec *sourcev1.HelmChartSpec, status *sour } func (s *Server) redisMockSetValueForChart(mock redismock.ClientMock, key, url string, opts *common.HttpClientOptions) error { - sink := repoEventSink{ - clientGetter: s.newBackgroundClientGetter(), - chartCache: s.chartCache, - } + sink := s.newRepoEventSink() return sink.redisMockSetValueForChart(mock, key, url, opts) } diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go index 30c934d6725..27f7b20f22a 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/oci_repo.go @@ -38,6 +38,7 @@ import ( "github.com/vmware-tanzu/kubeapps/cmd/kubeapps-apis/plugins/pkg/pkgutils" "github.com/vmware-tanzu/kubeapps/pkg/chart/models" "github.com/vmware-tanzu/kubeapps/pkg/tarutil" + "k8s.io/apimachinery/pkg/types" log "k8s.io/klog/v2" "github.com/fluxcd/pkg/version" @@ -352,11 +353,13 @@ func newRegistryClient(isLogin bool) (*registry.Client, string, error) { // OCI Helm repository, which defines a source, does not produce an Artifact // ref https://fluxcd.io/docs/components/source/helmrepositories/#helm-oci-repository + +// TODO: this function is way too long. Break it up func (s *repoEventSink) onAddOciRepo(repo sourcev1.HelmRepository) ([]byte, bool, error) { log.Infof("+onAddOciRepo(%s)", common.PrettyPrint(repo)) defer log.Infof("-onAddOciRepo") - ociRegistry, err := s.newOCIRegistryAndLogin(repo) + ociRegistry, err := s.newOCIRegistryAndLoginWithRepo(context.Background(), repo) if err != nil { return nil, false, err } @@ -403,7 +406,7 @@ func (s *repoEventSink) onAddOciRepo(repo sourcev1.HelmRepository) ([]byte, bool return nil, false, status.Errorf(codes.Internal, "%v", err) } - latestChartMetadata, err := s.getOCIChartMetadata(ociRegistry, chartID, latestChartVersion) + latestChartMetadata, err := getOCIChartMetadata(ociRegistry, chartID, latestChartVersion) if err != nil { return nil, false, err } @@ -427,8 +430,6 @@ func (s *repoEventSink) onAddOciRepo(repo sourcev1.HelmRepository) ([]byte, bool ChartVersions: []models.ChartVersion{}, } - // TODO (gfichtenholt) this loop needs to be implemented using - // multiple concurrent readers for _, tag := range allTags { chartVersion, err := ociRegistry.pickChartVersionFrom(appName, tag, allTags) if err != nil { @@ -466,7 +467,7 @@ func (s *repoEventSink) onAddOciRepo(repo sourcev1.HelmRepository) ([]byte, bool } if s.chartCache != nil { - fn := s.downloadOCIChartFn(ociRegistry) + fn := downloadOCIChartFn(ociRegistry) if err = s.chartCache.SyncCharts(charts, fn); err != nil { return nil, false, err } @@ -495,7 +496,7 @@ func (s *repoEventSink) onModifyOciRepo(key string, oldValue interface{}, repo s key, cacheEntryUntyped) } - ociRegistry, err := s.newOCIRegistryAndLogin(repo) + ociRegistry, err := s.newOCIRegistryAndLoginWithRepo(context.Background(), repo) if err != nil { return nil, false, err } @@ -571,13 +572,25 @@ func (r *OCIRegistry) shortRepoName(fullRepoName string) (string, error) { } } -func (s *repoEventSink) newOCIRegistryAndLogin(repo sourcev1.HelmRepository) (*OCIRegistry, error) { - // Construct the Getter options from the HelmRepository data - loginOpts, getterOpts, cred, err := s.clientOptionsForRepo(repo) +func (s *Server) newOCIRegistryAndLoginWithRepo(ctx context.Context, repoName types.NamespacedName) (*OCIRegistry, error) { + repo, err := s.getRepoInCluster(ctx, repoName) if err != nil { - return nil, status.Errorf(codes.Internal, "failed to create registry client options: %v", err) + return nil, err + } else { + sink := s.newRepoEventSink() + return sink.newOCIRegistryAndLoginWithRepo(ctx, *repo) } +} + +func (s *repoEventSink) newOCIRegistryAndLoginWithRepo(ctx context.Context, repo sourcev1.HelmRepository) (*OCIRegistry, error) { + if loginOpts, getterOpts, cred, err := s.ociClientOptionsForRepo(ctx, repo); err != nil { + return nil, status.Errorf(codes.Internal, "failed to create registry client: %v", err) + } else { + return s.newOCIRegistryAndLoginWithOptions(repo.Spec.URL, loginOpts, getterOpts, cred) + } +} +func (s *repoEventSink) newOCIRegistryAndLoginWithOptions(registryURL string, loginOpts []registry.LoginOption, getterOpts []getter.Option, cred *orasregistryauthv2.Credential) (*OCIRegistry, error) { // Create registry client and login if needed. registryClient, file, err := newRegistryClient(loginOpts != nil) if err != nil { @@ -605,13 +618,13 @@ func (s *repoEventSink) newOCIRegistryAndLogin(repo sourcev1.HelmRepository) (*O // oci://demo.goharbor.io/test-oci-1, which may contain repositories "repo-1", "repo2", etc ociRegistry, err := newOCIRegistry( - repo.Spec.URL, + registryURL, withOCIGetter(helmGetters), withOCIGetterOptions(getterOpts), withOCIRegistryClient(registryClient), withRegistryCredentialFn(registryCredentialFn)) if err != nil { - return nil, status.Errorf(codes.Internal, "failed to parse URL '%s': %v", repo.Spec.URL, err) + return nil, status.Errorf(codes.Internal, "failed to parse URL '%s': %v", registryURL, err) } // Attempt to login to the registry if credentials are provided. @@ -625,8 +638,9 @@ func (s *repoEventSink) newOCIRegistryAndLogin(repo sourcev1.HelmRepository) (*O return ociRegistry, nil } -func (s *repoEventSink) clientOptionsForRepo(repo sourcev1.HelmRepository) ([]registry.LoginOption, []getter.Option, *orasregistryauthv2.Credential, error) { - log.Infof("+clientOptionsForRepo()") +func (s *repoEventSink) ociClientOptionsForRepo(ctx context.Context, repo sourcev1.HelmRepository) ([]registry.LoginOption, []getter.Option, *orasregistryauthv2.Credential, error) { + log.Infof("+ociClientOptionsForRepo(%s)", common.PrettyPrint(repo)) + log.Infof("-ociClientOptionsForRepo") var loginOpts []registry.LoginOption var cred *orasregistryauthv2.Credential @@ -636,7 +650,7 @@ func (s *repoEventSink) clientOptionsForRepo(repo sourcev1.HelmRepository) ([]re getter.WithPassCredentialsAll(repo.Spec.PassCredentials), } - secret, err := s.getRepoSecret(context.Background(), repo) + secret, err := s.getRepoSecret(ctx, repo) if err != nil { return nil, nil, nil, err } else if secret != nil { @@ -659,7 +673,7 @@ func (s *repoEventSink) clientOptionsForRepo(repo sourcev1.HelmRepository) ([]re return loginOpts, getterOpts, cred, nil } -func (s *repoEventSink) getOCIChartTarball(ociRegistry *OCIRegistry, chartID string, chartVersion *repo.ChartVersion) ([]byte, error) { +func getOCIChartTarball(ociRegistry *OCIRegistry, chartID string, chartVersion *repo.ChartVersion) ([]byte, error) { chartBuffer, err := ociRegistry.downloadChart(chartVersion) if err != nil { return nil, status.Errorf(codes.Internal, "%v", err) @@ -667,10 +681,10 @@ func (s *repoEventSink) getOCIChartTarball(ociRegistry *OCIRegistry, chartID str return chartBuffer.Bytes(), nil } -func (s *repoEventSink) getOCIChartMetadata(ociRegistry *OCIRegistry, chartID string, chartVersion *repo.ChartVersion) (*chart.Metadata, error) { +func getOCIChartMetadata(ociRegistry *OCIRegistry, chartID string, chartVersion *repo.ChartVersion) (*chart.Metadata, error) { log.Infof("getOCIChartMetadata(%s, %s)", chartID, chartVersion.Metadata.Version) - chartTarball, err := s.getOCIChartTarball(ociRegistry, chartID, chartVersion) + chartTarball, err := getOCIChartTarball(ociRegistry, chartID, chartVersion) if err != nil { return nil, status.Errorf(codes.Internal, "%v", err) } @@ -699,7 +713,7 @@ func (s *repoEventSink) getOCIChartMetadata(ociRegistry *OCIRegistry, chartID st return &chartMetadata, nil } -func (s *repoEventSink) downloadOCIChartFn(ociRegistry *OCIRegistry) func(chartID, chartUrl, chartVersion string) ([]byte, error) { +func downloadOCIChartFn(ociRegistry *OCIRegistry) func(chartID, chartUrl, chartVersion string) ([]byte, error) { return func(chartID, chartUrl, chartVersion string) ([]byte, error) { _, chartName, err := pkgutils.SplitPackageIdentifier(chartID) if err != nil { @@ -712,6 +726,6 @@ func (s *repoEventSink) downloadOCIChartFn(ociRegistry *OCIRegistry) func(chartI Version: chartVersion, }, } - return s.getOCIChartTarball(ociRegistry, chartID, cv) + return getOCIChartTarball(ociRegistry, chartID, cv) } } diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go index 4cbe5f75659..023ec1e9dad 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo.go @@ -180,24 +180,12 @@ func (s *Server) getChartsForRepos(ctx context.Context, match []string) (map[str return chartsTyped, nil } -func (s *Server) clientOptionsForRepo(ctx context.Context, repoName types.NamespacedName) (*common.HttpClientOptions, error) { +func (s *Server) httpClientOptionsForRepo(ctx context.Context, repoName types.NamespacedName) (*common.HttpClientOptions, error) { repo, err := s.getRepoInCluster(ctx, repoName) if err != nil { return nil, err } - // notice a bit of inconsistency here, we are using s.clientGetter - // (i.e. the context of the incoming request) to read the secret - // as opposed to s.repoCache.clientGetter (which uses the context of - // User "system:serviceaccount:kubeapps:kubeapps-internal-kubeappsapis") - // which is what is used when the repo is being processed/indexed. - // I don't think it's necessarily a bad thing if the incoming user's RBAC - // settings are more permissive than that of the default RBAC for - // kubeapps-internal-kubeappsapis account. If we don't like that behavior, - // I can easily switch to BackgroundClientGetter here - sink := repoEventSink{ - clientGetter: s.newBackgroundClientGetter(), - chartCache: s.chartCache, - } + sink := s.newRepoEventSink() return sink.httpClientOptionsForRepo(ctx, *repo) } @@ -763,8 +751,11 @@ func (s *repoEventSink) indexAndEncode(checksum string, repo sourcev1.HelmReposi // resource "secrets" in API group "" in the namespace "default" // So we still finish the indexing of the repo but skip the charts log.Errorf("Failed to read secret for repo due to: %+v", err) - } else if err = s.chartCache.SyncCharts(charts, downloadChartViaHttpFn(opts)); err != nil { - return nil, false, err + } else { + fn := downloadChartViaHttpFn(opts) + if err = s.chartCache.SyncCharts(charts, fn); err != nil { + return nil, false, err + } } } return buf.Bytes(), true, nil @@ -818,7 +809,6 @@ func (s *repoEventSink) indexOneRepo(repo sourcev1.HelmRepository) ([]models.Cha msg := fmt.Sprintf("-indexOneRepo: [%s], indexed [%d] packages in [%d] ms", repo.Name, len(charts), duration.Milliseconds()) if len(charts) > 0 { log.Info(msg) - log.Infof("%s", common.PrettyPrint(charts)) } else { // this is kind of a red flag - an index with 0 charts, most likely contents of index.yaml is // messed up and didn't parse successfully but the helm library didn't raise an error diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go index fc02a1e93ef..ce563e9088f 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/server.go @@ -630,11 +630,28 @@ func (s *Server) SetUserManagedSecrets(ctx context.Context, request *v1alpha1.Se }, nil } -// convenience func mostly used by unit tests -func (s *Server) newBackgroundClientGetter() clientgetter.BackgroundClientGetterFunc { - return func(ctx context.Context) (clientgetter.ClientInterfaces, error) { +// makes the server look like a repo event sink. Facilitates code reuse between +// use cases when something happens in background as a result of a watch event, +// aka an "out-of-band" interaction and use cases when the user wants something +// done explicitly, aka "in-band" interaction +func (s *Server) newRepoEventSink() repoEventSink { + cg := func(ctx context.Context) (clientgetter.ClientInterfaces, error) { return s.clientGetter(ctx, s.kubeappsCluster) } + + // notice a bit of inconsistency here, we are using s.clientGetter + // (i.e. the context of the incoming request) to read the secret + // as opposed to s.repoCache.clientGetter (which uses the context of + // User "system:serviceaccount:kubeapps:kubeapps-internal-kubeappsapis") + // which is what is used when the repo is being processed/indexed. + // I don't think it's necessarily a bad thing if the incoming user's RBAC + // settings are more permissive than that of the default RBAC for + // kubeapps-internal-kubeappsapis account. If we don't like that behavior, + // I can easily switch to BackgroundClientGetter here + return repoEventSink{ + clientGetter: cg, + chartCache: s.chartCache, + } } func (s *Server) getClient(ctx context.Context, namespace string) (ctrlclient.Client, error) { @@ -675,27 +692,3 @@ func (s *Server) hasAccessToNamespace(ctx context.Context, gvr schema.GroupVersi func GetPluginDetail() *plugins.Plugin { return common.GetPluginDetail() } - -// makes the server look like a repo event sink. Facilitates code reuse between -// use cases when something happens in background as a result of a watch event, -// aka an "out-of-band" interaction and use cases when the user wants something -// done explicitly, aka "in-band" interaction -func (s *Server) newRepoEventSink() repoEventSink { - cg := func(ctx context.Context) (clientgetter.ClientInterfaces, error) { - return s.clientGetter(ctx, s.kubeappsCluster) - } - - // notice a bit of inconsistency here, we are using s.clientGetter - // (i.e. the context of the incoming request) to read the secret - // as opposed to s.repoCache.clientGetter (which uses the context of - // User "system:serviceaccount:kubeapps:kubeapps-internal-kubeappsapis") - // which is what is used when the repo is being processed/indexed. - // I don't think it's necessarily a bad thing if the incoming user's RBAC - // settings are more permissive than that of the default RBAC for - // kubeapps-internal-kubeappsapis account. If we don't like that behavior, - // I can easily switch to BackgroundClientGetter here - return repoEventSink{ - clientGetter: cg, - chartCache: s.chartCache, - } -} From 00c9c66f3e3dc9ea89159c1a11a0bb583d3ba611 Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Tue, 28 Jun 2022 02:19:33 -0700 Subject: [PATCH 10/11] incremental --- .../plugins/fluxv2/packages/v1alpha1/chart.go | 1 + .../plugins/fluxv2/packages/v1alpha1/repo_test.go | 10 +++------- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go index 0c007153d08..1e5e9dafbc8 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/chart.go @@ -40,6 +40,7 @@ func (s *Server) getChartInCluster(ctx context.Context, key types.NamespacedName return &chartObj, nil } +// TODO (gfichtenholt) this func is too long. Break it up func (s *Server) availableChartDetail(ctx context.Context, packageRef *corev1.AvailablePackageReference, chartVersion string) (*corev1.AvailablePackageDetail, error) { log.Infof("+availableChartDetail(%s, %s)", packageRef, chartVersion) 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 c207d4eb7b3..aeee835e204 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/repo_test.go @@ -2285,16 +2285,12 @@ func redisMockSetValueForRepo(mock redismock.ClientMock, key string, newValue, o mock.ExpectInfo("memory").SetVal("used_memory_rss_human:NA\r\nmaxmemory_human:NA") } -func (s *Server) newRepoEventSinkNoCache() repoEventSink { +func (s *Server) redisKeyValueForRepo(r sourcev1.HelmRepository) (key string, byteArray []byte, err error) { cg := func(ctx context.Context) (clientgetter.ClientInterfaces, error) { return s.clientGetter(ctx, s.kubeappsCluster) } - return repoEventSink{clientGetter: cg} -} - -func (s *Server) redisKeyValueForRepo(r sourcev1.HelmRepository) (key string, byteArray []byte, err error) { - sink := s.newRepoEventSinkNoCache() - return sink.redisKeyValueForRepo(r) + sinkNoChartCache := repoEventSink{clientGetter: cg} + return sinkNoChartCache.redisKeyValueForRepo(r) } func (sink *repoEventSink) redisKeyValueForRepo(r sourcev1.HelmRepository) (key string, byteArray []byte, err error) { From b554af3a36a5e42ca13c5faf37119fd94f2717ea Mon Sep 17 00:00:00 2001 From: gfichtenholt Date: Tue, 28 Jun 2022 22:17:26 -0700 Subject: [PATCH 11/11] Michael's comments --- .../fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go index 49e120d9a6e..2ef0503d539 100644 --- a/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go +++ b/cmd/kubeapps-apis/plugins/fluxv2/packages/v1alpha1/docker_reg_v2_repo_lister.go @@ -73,7 +73,7 @@ func (l *dockerRegistryApiV2RepositoryLister) ListRepositoryNames(ociRegistry *O // to req.Query so we don't start at the beggining of the alphabet // impl refs: - // 1. https://github.com/oras-project/oras-go/blob/14422086e418/registry/remote/registry.go + // 1. https://github.com/oras-project/oras-go/blob/14422086e41897a44cb706726e687d39dc728805/registry/remote/registry.go#L105 // 2. https://github.com/oras-project/oras-go/blob/14422086e41897a44cb706726e687d39dc728805/registry/remote/url.go#L43 err = orasRegistry.Repositories(context.Background(), fn) log.Infof("ORAS Repositories returned: %v", err)