Skip to content

Commit

Permalink
Use the OCI Catalog when syncing charts. (#6692)
Browse files Browse the repository at this point in the history
### Description of the change

This PR updates the sync code to also try the OCI Catalog service for
app repositories where the repos aren't listed (after trying the VAC
index).

### Benefits

Finishes the work of #6263, although, as we've discussed, to benefit
from this (ie. be able to add the public bitnami OCI repo), we need to
add a couple more features, since currently it uses the existing OCI
Distribution API which *requires* authentication. We'd like to ensure
people don't need authentication to use a public OCI namespace, such as
the bitnami catalog.

### Possible drawbacks


### Applicable issues

<!-- Enter any applicable Issues here (You can reference an issue using
#) -->

- fixes #6263

### Additional information

<!-- If there's anything else that's important and relevant to your pull
request, mention that information here.-->

Signed-off-by: Michael Nelson <minelson@vmware.com>
  • Loading branch information
absoludity committed Aug 24, 2023
1 parent 27c0fac commit 86dc7c9
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 33 deletions.
6 changes: 4 additions & 2 deletions cmd/asset-syncer/server/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package server

import (
"context"
"encoding/json"
"fmt"
"time"
Expand All @@ -20,6 +21,7 @@ func Sync(serveOpts Config, version string, args []string) error {
return fmt.Errorf("need exactly three arguments: [REPO NAME] [REPO URL] [REPO TYPE] (got %v)", len(args))
}

ctx := context.Background()
dbConfig := dbutils.Config{URL: serveOpts.DatabaseURL, Database: serveOpts.DatabaseName, Username: serveOpts.DatabaseUser, Password: serveOpts.DatabasePassword}
globalPackagingNamespace := serveOpts.GlobalPackagingNamespace
manager, err := newManager(dbConfig, globalPackagingNamespace)
Expand Down Expand Up @@ -66,7 +68,7 @@ func Sync(serveOpts Config, version string, args []string) error {
return fmt.Errorf("error: %v", err)
}
repo := repoIface.AppRepository()
checksum, err := repoIface.Checksum()
checksum, err := repoIface.Checksum(ctx)
if err != nil {
return fmt.Errorf("error: %v", err)
}
Expand All @@ -90,7 +92,7 @@ func Sync(serveOpts Config, version string, args []string) error {
}

for _, fetchLatestOnly := range fetchLatestOnlySlice {
charts, err := repoIface.Charts(fetchLatestOnly)
charts, err := repoIface.Charts(ctx, fetchLatestOnly)
if err != nil {
return fmt.Errorf("error: %v", err)
}
Expand Down
67 changes: 46 additions & 21 deletions cmd/asset-syncer/server/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,10 @@ func getSha256(src []byte) (string, error) {

// ChartCatalog defines the methods to retrieve information from the given repository
type ChartCatalog interface {
Checksum() (string, error)
Checksum(ctx context.Context) (string, error)
AppRepository() *models.AppRepositoryInternal
FilterIndex()
Charts(fetchLatestOnly bool) ([]models.Chart, error)
Charts(ctx context.Context, fetchLatestOnly bool) ([]models.Chart, error)
FetchFiles(cv models.ChartVersion, userAgent string, passCredentials bool) (map[string]string, error)
}

Expand All @@ -140,7 +140,7 @@ type HelmRepo struct {
}

// Checksum returns the sha256 of the repo
func (r *HelmRepo) Checksum() (string, error) {
func (r *HelmRepo) Checksum(ctx context.Context) (string, error) {
return getSha256(r.content)
}

Expand Down Expand Up @@ -251,7 +251,7 @@ func filterCharts(charts []models.Chart, filterRule *apprepov1alpha1.FilterRuleS
}

// Charts retrieve the list of charts exposed in the repo
func (r *HelmRepo) Charts(fetchLatestOnly bool) ([]models.Chart, error) {
func (r *HelmRepo) Charts(ctx context.Context, fetchLatestOnly bool) ([]models.Chart, error) {
repo := &models.AppRepository{
Namespace: r.Namespace,
Name: r.Name,
Expand Down Expand Up @@ -331,7 +331,7 @@ type ociAPI interface {
TagList(appName, userAgent string) (*TagList, error)
IsHelmChart(appName, tag, userAgent string) (bool, error)
CatalogAvailable(ctx context.Context, userAgent string) (bool, error)
Catalog(userAgent string) ([]string, error)
Catalog(ctx context.Context, userAgent string) ([]string, error)
}

// OciAPIClient enables basic interactions with an OCI registry
Expand Down Expand Up @@ -393,8 +393,6 @@ func (o *OciAPIClient) IsHelmChart(appName, tag, userAgent string) (bool, error)
// https://docs.vmware.com/en/VMware-Application-Catalog/services/main/GUID-using-consume-metadata.html#method-2-obtain-metadata-from-the-oci-registry-10
// although examples have "chart-index" rather than "index" as the artifact
// name.
// In the future, this should check the oci-catalog service for possible
// catalogs.
func (o *OciAPIClient) CatalogAvailable(ctx context.Context, userAgent string) (bool, error) {
manifest, err := o.catalogManifest(userAgent)
if err == nil {
Expand Down Expand Up @@ -451,16 +449,7 @@ func (o *OciAPIClient) catalogManifest(userAgent string) (*OCIManifest, error) {
return &manifest, nil
}

// Catalog returns the list of repositories in the (namespaced) registry
// when discoverable.
func (o *OciAPIClient) Catalog(userAgent string) ([]string, error) {
// TODO(minelson): all Kubeapps interactions with OCI registries should
// be updated to use the oras go lib.
manifest, err := o.catalogManifest(userAgent)
if err != nil {
return nil, err
}

func (o *OciAPIClient) getVACReposForManifest(manifest *OCIManifest, userAgent string) ([]string, error) {
if len(manifest.Layers) != 1 || manifest.Layers[0].MediaType != "application/vnd.vmware.charts.index.layer.v1+json" {
log.Errorf("Unexpected layer in index manifest: %v", manifest)
return nil, fmt.Errorf("unexpected layer in index manifest")
Expand Down Expand Up @@ -492,6 +481,42 @@ func (o *OciAPIClient) Catalog(userAgent string) ([]string, error) {
return repos, nil
}

// Catalog returns the list of repositories in the (namespaced) registry
// when discoverable.
func (o *OciAPIClient) Catalog(ctx context.Context, userAgent string) ([]string, error) {
// TODO(minelson): all Kubeapps interactions with OCI registries should
// be updated to use the oras go lib.
manifest, err := o.catalogManifest(userAgent)
if err == nil {
return o.getVACReposForManifest(manifest, userAgent)
}
if o.GrpcClient != nil {
log.Infof("Unable to find VAC index: %+v. Attempting OCI-Catalog")
repos_stream, err := o.GrpcClient.ListRepositoriesForRegistry(ctx, &ocicatalog.ListRepositoriesForRegistryRequest{
Registry: o.RegistryNamespaceUrl.Host,
Namespace: o.RegistryNamespaceUrl.Path,
ContentTypes: []string{ocicatalog_client.CONTENT_TYPE_HELM},
})
if err != nil {
return nil, fmt.Errorf("error querying OCI catalog for repos: %+v", err)
}

repos := []string{}
for {
repo, err := repos_stream.Recv()
if err != nil {
if err == io.EOF {
return repos, nil
}
return nil, fmt.Errorf("error receiving OCI Repositories: %+v", err)
}
repos = append(repos, repo.Name)
}
} else {
return nil, err
}
}

func tagCheckerWorker(o ociAPI, tagJobs <-chan checkTagJob, resultChan chan checkTagResult) {
for j := range tagJobs {
isHelmChart, err := o.IsHelmChart(j.AppName, j.Tag, GetUserAgent("", ""))
Expand All @@ -502,11 +527,11 @@ func tagCheckerWorker(o ociAPI, tagJobs <-chan checkTagJob, resultChan chan chec
// Checksum returns the sha256 of the repo by concatenating tags for
// 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) {
func (r *OCIRegistry) Checksum(ctx context.Context) (string, error) {
r.tags = map[string]TagList{}

if len(r.repositories) == 0 {
repos, err := r.ociCli.Catalog("")
repos, err := r.ociCli.Catalog(ctx, "")
if err != nil {
return "", err
}
Expand Down Expand Up @@ -666,15 +691,15 @@ func (r *OCIRegistry) FilterIndex() {
}

// Charts retrieve the list of charts exposed in the repo
func (r *OCIRegistry) Charts(fetchLatestOnly bool) ([]models.Chart, error) {
func (r *OCIRegistry) Charts(ctx context.Context, fetchLatestOnly bool) ([]models.Chart, error) {
result := map[string]*models.Chart{}
repoURL, err := parseRepoURL(r.AppRepositoryInternal.URL)
if err != nil {
return nil, err
}

if len(r.repositories) == 0 {
repos, err := r.ociCli.Catalog("")
repos, err := r.ociCli.Catalog(ctx, "")
if err != nil {
return nil, err
}
Expand Down
111 changes: 101 additions & 10 deletions cmd/asset-syncer/server/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
apprepov1alpha1 "github.com/vmware-tanzu/kubeapps/cmd/apprepository-controller/pkg/apis/apprepository/v1alpha1"
ocicatalog "github.com/vmware-tanzu/kubeapps/cmd/oci-catalog/gen/catalog/v1alpha1"
"github.com/vmware-tanzu/kubeapps/pkg/chart/models"
"github.com/vmware-tanzu/kubeapps/pkg/dbutils"
"github.com/vmware-tanzu/kubeapps/pkg/helm"
helmfake "github.com/vmware-tanzu/kubeapps/pkg/helm/fake"
helmtest "github.com/vmware-tanzu/kubeapps/pkg/helm/test"
httpclient "github.com/vmware-tanzu/kubeapps/pkg/http-client"
"github.com/vmware-tanzu/kubeapps/pkg/ocicatalog_client"
"github.com/vmware-tanzu/kubeapps/pkg/ocicatalog_client/ocicatalog_clienttest"
tartest "github.com/vmware-tanzu/kubeapps/pkg/tarutil/test"
"helm.sh/helm/v3/pkg/chart"
log "k8s.io/klog/v2"
Expand Down Expand Up @@ -602,7 +605,7 @@ type fakeRepo struct {
chartFiles models.ChartFiles
}

func (r *fakeRepo) Checksum() (string, error) {
func (r *fakeRepo) Checksum(ctx context.Context) (string, error) {
return "checksum", nil
}

Expand All @@ -614,7 +617,7 @@ func (r *fakeRepo) FilterIndex() {
// no-op
}

func (r *fakeRepo) Charts(shallow bool) ([]models.Chart, error) {
func (r *fakeRepo) Charts(ctx context.Context, shallow bool) ([]models.Chart, error) {
return r.charts, nil
}

Expand Down Expand Up @@ -904,6 +907,62 @@ func Test_ociAPICli(t *testing.T) {
}
})

t.Run("CatalogAvailable - returns true if oci-catalog responds", func(t *testing.T) {
grpcAddr, grpcDouble, closer := ocicatalog_clienttest.SetupTestDouble(t)
defer closer()
grpcDouble.Repositories = []*ocicatalog.Repository{
{
Name: "apache",
},
}
grpcClient, closer, err := ocicatalog_client.NewClient(grpcAddr)
if err != nil {
t.Fatalf("%+v", err)
}
defer closer()

apiCli := &OciAPIClient{
RegistryNamespaceUrl: urlWithNamespace,
HttpClient: &badHTTPClient{},
GrpcClient: grpcClient,
}

got, err := apiCli.CatalogAvailable(context.Background(), "my-user-agent")
if err != nil {
t.Fatalf("%+v", err)
}

if got, want := got, true; got != want {
t.Errorf("got: %t, want: %t", got, want)
}
})

t.Run("CatalogAvailable - returns false if oci-catalog responds with zero repos", func(t *testing.T) {
grpcAddr, grpcDouble, closer := ocicatalog_clienttest.SetupTestDouble(t)
defer closer()
grpcDouble.Repositories = []*ocicatalog.Repository{}
grpcClient, closer, err := ocicatalog_client.NewClient(grpcAddr)
if err != nil {
t.Fatalf("%+v", err)
}
defer closer()

apiCli := &OciAPIClient{
RegistryNamespaceUrl: urlWithNamespace,
HttpClient: &badHTTPClient{},
GrpcClient: grpcClient,
}

got, err := apiCli.CatalogAvailable(context.Background(), "my-user-agent")
if err != nil {
t.Fatalf("%+v", err)
}

if got, want := got, false; got != want {
t.Errorf("got: %t, want: %t", got, want)
}
})

t.Run("CatalogAvailable - returns false on any other", func(t *testing.T) {
apiCli := &OciAPIClient{
RegistryNamespaceUrl: urlWithNamespace,
Expand Down Expand Up @@ -931,7 +990,39 @@ func Test_ociAPICli(t *testing.T) {
},
}

got, err := apiCli.Catalog("my-user-agent")
got, err := apiCli.Catalog(context.Background(), "my-user-agent")
if err != nil {
t.Fatal(err)
}

if got, want := got, []string{"common", "redis"}; !cmp.Equal(got, want) {
t.Errorf("got: %s, want: %s", got, want)
}
})

t.Run("Catalog - successful request via oci-catalog", func(t *testing.T) {
grpcAddr, grpcDouble, closer := ocicatalog_clienttest.SetupTestDouble(t)
defer closer()
grpcDouble.Repositories = []*ocicatalog.Repository{
{
Name: "common",
},
{
Name: "redis",
},
}
grpcClient, closer, err := ocicatalog_client.NewClient(grpcAddr)
if err != nil {
t.Fatalf("%+v", err)
}
defer closer()
apiCli := &OciAPIClient{
RegistryNamespaceUrl: urlWithNamespace,
HttpClient: &badHTTPClient{},
GrpcClient: grpcClient,
}

got, err := apiCli.Catalog(context.Background(), "my-user-agent")
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -959,7 +1050,7 @@ func (o *fakeOCIAPICli) CatalogAvailable(ctx context.Context, userAgent string)
return false, nil
}

func (o *fakeOCIAPICli) Catalog(userAgent string) ([]string, error) {
func (o *fakeOCIAPICli) Catalog(ctx context.Context, userAgent string) ([]string, error) {
return nil, nil
}

Expand All @@ -973,15 +1064,15 @@ func Test_OCIRegistry(t *testing.T) {

t.Run("Checksum - failed request", func(t *testing.T) {
repo.ociCli = &fakeOCIAPICli{err: fmt.Errorf("request failed")}
_, err := repo.Checksum()
_, err := repo.Checksum(context.Background())
assert.Error(t, fmt.Errorf("request failed"), err)
})

t.Run("Checksum - success", func(t *testing.T) {
repo.ociCli = &fakeOCIAPICli{
tagList: &TagList{Name: "test/apache", Tags: []string{"1.0.0", "1.1.0"}},
}
checksum, err := repo.Checksum()
checksum, err := repo.Checksum(context.Background())
assert.NoError(t, err)
assert.Equal(t, checksum, "b1b1ae17ddc8f83606acb8a175025a264e8634bb174b6e6a5799bdb5d20eaa58", "checksum")
})
Expand All @@ -996,7 +1087,7 @@ func Test_OCIRegistry(t *testing.T) {
tagList: &TagList{Name: "test/apache", Tags: []string{"1.0.0", "1.1.0"}},
},
}
_, err := emptyRepo.Checksum()
_, err := emptyRepo.Checksum(context.Background())
assert.NoError(t, err)
assert.Equal(t, emptyRepo.tags, map[string]TagList{
"apache": {Name: "test/apache", Tags: []string{"1.0.0", "1.1.0"}},
Expand Down Expand Up @@ -1364,7 +1455,7 @@ version: 1.0.0
},
},
}
charts, err := chartsRepo.Charts(tt.shallow)
charts, err := chartsRepo.Charts(context.Background(), tt.shallow)
assert.NoError(t, err)
if !cmp.Equal(charts, tt.expected) {
t.Errorf("Unexpected result %v", cmp.Diff(tt.expected, charts))
Expand Down Expand Up @@ -1410,7 +1501,7 @@ version: 1.0.0
},
},
}
charts, err := chartsRepo.Charts(true)
charts, err := chartsRepo.Charts(context.Background(), true)
assert.NoError(t, err)
if len(charts) != 1 && charts[0].Name != "common" {
t.Errorf("got: %+v", charts)
Expand Down Expand Up @@ -1703,7 +1794,7 @@ func TestHelmRepoAppliesUnescape(t *testing.T) {
AppRepositoryInternal: repo,
}
t.Run("Helm repo applies unescaping to chart data", func(t *testing.T) {
charts, _ := helmRepo.Charts(false)
charts, _ := helmRepo.Charts(context.Background(), false)
if !cmp.Equal(charts, expectedCharts) {
t.Errorf("Unexpected result: %v", cmp.Diff(charts, expectedCharts))
}
Expand Down

0 comments on commit 86dc7c9

Please sign in to comment.