Repository: rancher/fleet
Pull request #2311 (2024-04-09 10:17:49)

Catch OCI version errors and return them
Refers to #1606 

The module we use would panic if version wasn't set or invalid.


With this change, an explicit version is not required for OCI charts. The module will reach out to the registry and fetch the tags and will resolve them to a version.
 
```
		// If empty, try to get the highest available tag
		// If exact version, try to find it
		// If semver constraint string, try to find a match
```
https://github.com/helm/helm/blob/release-3.14/pkg/downloader/chart_downloader.go#L144-L175



Patch:
diff --git a/.github/workflows/release-against-charts.yml b/.github/workflows/release-against-charts.yml
index 348aea9c..96592aa1 100644
--- a/.github/workflows/release-against-charts.yml
+++ b/.github/workflows/release-against-charts.yml
@@ -9,7 +9,7 @@ on:
       charts_ref:
         description: "Submit PR against the following rancher/charts branch (e.g. dev-v2.7)"
         required: true
-        default: "dev-v2.8"
+        default: "dev-v2.9"
       prev_fleet:
         description: "Previous Fleet version (e.g. 0.6.0-rc.3)"
         required: true
@@ -58,7 +58,7 @@ jobs:
         with:
           token: ${{secrets.PUSH_TO_FORKS_SUBMIT_PRS}}
           push-to-fork: rancherbot/charts
-          title: 'Update Fleet to v${{github.event.inputs.new_fleet}}'
+          title: '[${{github.event.inputs.charts_ref}}] fleet ${{github.event.inputs.new_chart}}+up${{github.event.inputs.new_fleet}} update'
           body: |
             Update Fleet to v${{github.event.inputs.new_fleet}}
 
diff --git a/.github/workflows/release-against-rancher.yml b/.github/workflows/release-against-rancher.yml
index 26ce5144..88c5a8f2 100644
--- a/.github/workflows/release-against-rancher.yml
+++ b/.github/workflows/release-against-rancher.yml
@@ -60,7 +60,7 @@ jobs:
         with:
           token: ${{secrets.PUSH_TO_FORKS_SUBMIT_PRS}}
           push-to-fork: rancherbot/rancher
-          title: ' Update Fleet to v${{github.event.inputs.new_fleet}}'
+          title: '[${{github.event.inputs.rancher_ref}}] fleet ${{github.event.inputs.new_chart}}+up${{github.event.inputs.new_fleet}} update'
           body: |
             Update Fleet to v${{github.event.inputs.new_fleet}}
 
diff --git a/.github/workflows/release-fleet.yml b/.github/workflows/release-fleet.yml
index 01ad7dc9..ea7dd972 100644
--- a/.github/workflows/release-fleet.yml
+++ b/.github/workflows/release-fleet.yml
@@ -39,11 +39,13 @@ jobs:
           args: --timeout=10m --config=.golangci.json
 
       - name: Run FOSSA scan
+        if: contains(github.ref, 'rc') != true
         uses: fossas/fossa-action@v1.3.3
         with:
           api-key: ${{ secrets.FOSSA_API_KEY }}
 
       - name: Run FOSSA tests
+        if: contains(github.ref, 'rc') != true
         uses: fossas/fossa-action@v1.3.3
         with:
           api-key: ${{ secrets.FOSSA_API_KEY }}
diff --git a/internal/bundlereader/loaddirectory.go b/internal/bundlereader/loaddirectory.go
index 38eeacda..3944de9e 100644
--- a/internal/bundlereader/loaddirectory.go
+++ b/internal/bundlereader/loaddirectory.go
@@ -17,7 +17,6 @@ import (
 	"unicode/utf8"
 
 	"github.com/hashicorp/go-getter"
-	"github.com/pkg/errors"
 	"github.com/rancher/fleet/internal/content"
 	"github.com/rancher/fleet/internal/helmupdater"
 	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
@@ -300,7 +299,7 @@ func GetContent(ctx context.Context, base, source, version string, auth Auth, di
 		return nil
 	})
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to read %s relative to %s", orgSource, base)
+		return nil, fmt.Errorf("failed to read %s relative to %s: %w", orgSource, base, err)
 	}
 
 	return files, nil
@@ -316,6 +315,11 @@ func downloadOCIChart(name, version, path string, auth Auth) (string, error) {
 		return "", err
 	}
 
+	registryClient, err = registry.NewClient()
+	if err != nil {
+		return "", err
+	}
+
 	// Helm does not support direct authentication for private OCI registries when a chart is downloaded
 	// so it is necessary to login before via Helm which stores the registry token in a configuration
 	// file on the system
@@ -325,11 +329,6 @@ func downloadOCIChart(name, version, path string, auth Auth) (string, error) {
 			addr = fmt.Sprintf("%s:%s", addr, port)
 		}
 
-		registryClient, err = registry.NewClient()
-		if err != nil {
-			return "", err
-		}
-
 		err = registryClient.Login(
 			addr,
 			registry.LoginOptInsecure(false),
@@ -348,7 +347,7 @@ func downloadOCIChart(name, version, path string, auth Auth) (string, error) {
 
 	saved, _, err := c.DownloadTo(name, version, path)
 	if err != nil {
-		return "", fmt.Errorf("Helm chart download: %v", err)
+		return "", fmt.Errorf("helm chart download: %w", err)
 	}
 
 	// Logout to remove the token configuration file from the system again


Test patch:
diff --git a/internal/bundlereader/loaddirectory_test.go b/internal/bundlereader/loaddirectory_test.go
index 4f88968e..b459f0df 100644
--- a/internal/bundlereader/loaddirectory_test.go
+++ b/internal/bundlereader/loaddirectory_test.go
@@ -344,6 +344,91 @@ func TestGetContent(t *testing.T) {
 	}
 }
 
+func TestGetContentOCI(t *testing.T) {
+	cases := []struct {
+		name    string
+		source  string
+		version string
+
+		result      []string
+		expectedErr string
+	}{
+		// Note: These tests rely on external hosts and DNS resolution.
+		// We could just test the helm registry client is initialized
+		// correctly, however for now these tests document different
+		// scenarios nicely.
+		{
+			name:   "OCI URL without version",
+			source: "oci://ghcr.io/rancher/fleet-test-configmap-chart",
+			result: []string{
+				"fleet-test-configmap-chart/Chart.yaml",
+				"fleet-test-configmap-chart/values.yaml",
+				"fleet-test-configmap-chart/templates/configmap.yaml",
+			},
+		},
+		{
+			name:    "OCI URL with version",
+			source:  "oci://ghcr.io/rancher/fleet-test-configmap-chart",
+			version: "0.1.0",
+			result: []string{
+				"fleet-test-configmap-chart/Chart.yaml",
+				"fleet-test-configmap-chart/values.yaml",
+				"fleet-test-configmap-chart/templates/configmap.yaml",
+			},
+		},
+		{
+			name:        "OCI URL with invalid version",
+			source:      "oci://ghcr.io/rancher/fleet-test-configmap-chart",
+			version:     "latest",
+			expectedErr: "helm chart download: improper constraint: latest",
+		},
+		{
+			name:        "Non-existing OCI URL without version",
+			source:      "oci://non-existing-hostname/charts/chart",
+			expectedErr: "dial tcp: lookup non-existing-hostname",
+		},
+		{
+			name:        "Non-existing OCI URL with invalid version",
+			source:      "oci://non-existing-hostname/charts/chart",
+			version:     "latest",
+			expectedErr: "dial tcp: lookup non-existing-hostname",
+		},
+		{
+			name:        "Invalid OCI URL which includes version too",
+			source:      "oci://non-existing-hostname/charts/chart:1.0",
+			version:     "1.0",
+			expectedErr: "helm chart download: invalid_reference: invalid tag",
+		},
+		{
+			name:        "Non-existing OCI URL with valid semver",
+			source:      "oci://non-existing-hostname/charts/chart",
+			version:     "1.0",
+			expectedErr: "helm chart download: failed to do request",
+		},
+	}
+
+	assert := assert.New(t)
+
+	base, err := os.MkdirTemp("", "test-fleet")
+	require.NoError(t, err)
+
+	defer os.RemoveAll(base)
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			result, err := bundlereader.GetContent(context.Background(), base, c.source, c.version, bundlereader.Auth{}, false)
+			if c.expectedErr == "" {
+				assert.NoError(err)
+				for k := range result {
+					assert.Contains(c.result, k)
+				}
+			} else {
+				assert.ErrorContains(err, c.expectedErr, c.name)
+			}
+		})
+	}
+}
+
 // createDirStruct generates and populates a directory structure which root is node, placing it at basePath.
 func createDirStruct(t *testing.T, basePath string, node fsNode) string {
 	path := filepath.Join(basePath, node.name)