don't fetch tags when exact version is used in HelmRepository

Taking this shortcut has two benefits:

1. It allows charts to be fetched from AWS's public container registry
   at public.ecr.aws
2. It makes reconciling a HelmChart faster by skipping one or more
   potentially expensive API calls to the registry.

I adapted the unit tests to the new behavior that the
OCIChartRepository doesn't fail anymore for the case where a specific
chart version has been requested that doesn't actually exist in the
registry.

refs #845

Signed-off-by: Max Jonas Werner <[email protected]>
Signed-off-by: Sunny <[email protected]>

Author: makkes

internal/helm/chart/builder_remote_test.go

@@ -64,10 +64,14 @@ func (m *mockRegistryClient) Logout(url string, opts ...registry.LogoutOption) e
 type mockIndexChartGetter struct {
 	IndexResponse []byte
 	ChartResponse []byte
+	ErrorResponse error
 	requestedURL  string
 }
 
 func (g *mockIndexChartGetter) Get(u string, _ ...helmgetter.Option) (*bytes.Buffer, error) {
+	if g.ErrorResponse != nil {
+		return nil, g.ErrorResponse
+	}
 	g.requestedURL = u
 	r := g.ChartResponse
 	if strings.HasSuffix(u, "index.yaml") {
@@ -248,6 +252,15 @@ func TestRemoteBuilder_BuildFromOCIChatRepository(t *testing.T) {
 			RegistryClient: registryClient,
 		}
 	}
+	mockRepoWithoutChart := func() *repository.OCIChartRepository {
+		return &repository.OCIChartRepository{
+			URL: *u,
+			Client: &mockIndexChartGetter{
+				ErrorResponse: fmt.Errorf("chart doesn't exist"),
+			},
+			RegistryClient: registryClient,
+		}
+	}
 
 	tests := []struct {
 		name         string
@@ -278,8 +291,8 @@ func TestRemoteBuilder_BuildFromOCIChatRepository(t *testing.T) {
 		{
 			name:       "chart version not in repository",
 			reference:  RemoteReference{Name: "grafana", Version: "1.1.1"},
-			repository: mockRepo(),
-			wantErr:    "failed to get chart version for remote reference",
+			repository: mockRepoWithoutChart(),
+			wantErr:    "failed to download chart for remote reference",
 		},
 		{
 			name:       "invalid version metadata",
@@ -334,7 +347,7 @@ func TestRemoteBuilder_BuildFromOCIChatRepository(t *testing.T) {
 			cb, err := b.Build(context.TODO(), tt.reference, targetPath, tt.buildOpts)
 
 			if tt.wantErr != "" {
-				g.Expect(err).To(HaveOccurred())
+				g.Expect(err).To(HaveOccurred(), "expected error '%s'", tt.wantErr)
 				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
 				g.Expect(cb).To(BeZero())
 				return

internal/helm/repository/oci_chart_repository.go

@@ -124,6 +124,22 @@ func (r *OCIChartRepository) Get(name, ver string) (*repo.ChartVersion, error) {
 	// Either in an index file or from a registry.
 	cpURL := r.URL
 	cpURL.Path = path.Join(cpURL.Path, name)
+
+	// if ver is a valid semver version, take a shortcut here so we don't need to list all tags which can be an
+	// expensive operation.
+	if _, err := version.ParseVersion(ver); err == nil {
+		return &repo.ChartVersion{
+			URLs: []string{fmt.Sprintf("%s:%s", cpURL.String(), ver)},
+			Metadata: &chart.Metadata{
+				Name:    name,
+				Version: ver,
+			},
+		}, nil
+	}
+
+	// ver doesn't denote a concrete version so we interpret it as a semver range and try to find the best-matching
+	// version from the list of tags in the registry.
+
 	cvs, err := r.getTags(cpURL.String())
 	if err != nil {
 		return nil, err

internal/helm/repository/oci_chart_repository_test.go

@@ -118,67 +118,76 @@ func TestOCIChartRepository_Get(t *testing.T) {
 	testURL := "oci://localhost:5000/my_repo"
 
 	testCases := []struct {
-		name        string
-		url         string
-		version     string
-		expected    string
-		expectedErr string
+		name           string
+		registryClient RegistryClient
+		url            string
+		version        string
+		expected       string
+		expectedErr    string
 	}{
 		{
-			name:     "should return latest stable version",
-			version:  "",
-			url:      testURL,
-			expected: "1.0.0",
+			name:           "should return latest stable version",
+			registryClient: registryClient,
+			version:        "",
+			url:            testURL,
+			expected:       "1.0.0",
 		},
 		{
-			name:     "should return latest stable version (asterisk)",
-			version:  "*",
-			url:      testURL,
-			expected: "1.0.0",
+			name:           "should return latest stable version (asterisk)",
+			registryClient: registryClient,
+			version:        "*",
+			url:            testURL,
+			expected:       "1.0.0",
 		},
 		{
-			name:     "should return latest stable version (semver range)",
-			version:  ">=0.1.5",
-			url:      testURL,
-			expected: "1.0.0",
+			name:           "should return latest stable version (semver range)",
+			registryClient: registryClient,
+			version:        ">=0.1.5",
+			url:            testURL,
+			expected:       "1.0.0",
 		},
 		{
-			name:     "should return 0.2.0 (semver range)",
-			version:  "0.2.x",
-			url:      testURL,
-			expected: "0.2.0",
+			name:           "should return 0.2.0 (semver range)",
+			registryClient: registryClient,
+			version:        "0.2.x",
+			url:            testURL,
+			expected:       "0.2.0",
 		},
 		{
-			name:     "should return a perfect match",
-			version:  "0.1.0",
-			url:      testURL,
-			expected: "0.1.0",
+			name:           "should return a perfect match",
+			registryClient: nil,
+			version:        "0.1.0",
+			url:            testURL,
+			expected:       "0.1.0",
 		},
 		{
-			name:     "should return 0.10.0",
-			version:  "0.*",
-			url:      testURL,
-			expected: "0.10.0",
+			name:           "should return 0.10.0",
+			registryClient: registryClient,
+			version:        "0.*",
+			url:            testURL,
+			expected:       "0.10.0",
 		},
 		{
-			name:        "should an error for unfunfilled range",
-			version:     ">2.0.0",
-			url:         testURL,
-			expectedErr: "could not locate a version matching provided version string >2.0.0",
+			name:           "should an error for unfulfilled range",
+			registryClient: registryClient,
+			version:        ">2.0.0",
+			url:            testURL,
+			expectedErr:    "could not locate a version matching provided version string >2.0.0",
 		},
 		{
-			name:     "shouldn't error out with trailing slash",
-			version:  "",
-			url:      "oci://localhost:5000/my_repo/",
-			expected: "1.0.0",
+			name:           "shouldn't error out with trailing slash",
+			registryClient: registryClient,
+			version:        "",
+			url:            "oci://localhost:5000/my_repo/",
+			expected:       "1.0.0",
 		},
 	}
 
 	for _, tc := range testCases {
 
 		t.Run(tc.name, func(t *testing.T) {
 			g := NewWithT(t)
-			r, err := NewOCIChartRepository(tc.url, WithOCIRegistryClient(registryClient), WithOCIGetter(providers))
+			r, err := NewOCIChartRepository(tc.url, WithOCIRegistryClient(tc.registryClient), WithOCIGetter(providers))
 			g.Expect(err).ToNot(HaveOccurred())
 			g.Expect(r).ToNot(BeNil())