Skip to content

Commit 45e0564

Browse files
authored
Surface HelmRepoURLRegex hint in bundle errors (#5367)
* Surface HelmRepoURLRegex hint in bundle errors Track when Helm credentials are stripped because `helmRepoURLRegex` is empty and carry that marker on the chart directory metadata. Append an explicit guidance hint to content-loading errors for those directories so the status message shown in Rancher UI explains how to fix the configuration.
1 parent f23383d commit 45e0564

4 files changed

Lines changed: 64 additions & 7 deletions

File tree

internal/bundlereader/loaddirectory.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package bundlereader
33
import (
44
"bufio"
55
"context"
6+
"errors"
67
"fmt"
78
"io"
89
"io/fs"
@@ -23,6 +24,8 @@ import (
2324
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
2425
)
2526

27+
const helmRepoURLRegexUIHint = "helmRepoURLRegex is empty, so Helm credentials were not forwarded; set spec.helmRepoURLRegex to allow credential forwarding"
28+
2629
// ignoreTree represents a tree of ignored paths (read from .fleetignore files), each node being a directory.
2730
// It provides a means for ignored paths to be propagated down the tree, but not between subdirectories of a same
2831
// directory.
@@ -166,7 +169,7 @@ func loadDirectory(ctx context.Context, opts loadOpts, dir directory) ([]fleet.B
166169

167170
files, err := GetContent(ctx, dir.base, dir.source, dir.version, dir.auth, opts.disableDepsUpdate, opts.ignoreApplyConfigs)
168171
if err != nil {
169-
return nil, err
172+
return nil, maybeAddHelmRepoURLRegexHint(err, dir)
170173
}
171174

172175
for name, data := range files {
@@ -190,6 +193,19 @@ func loadDirectory(ctx context.Context, opts loadOpts, dir directory) ([]fleet.B
190193
return resources, nil
191194
}
192195

196+
func maybeAddHelmRepoURLRegexHint(err error, dir directory) error {
197+
if err == nil {
198+
return nil
199+
}
200+
if !dir.strippedCreds {
201+
return err
202+
}
203+
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
204+
return err
205+
}
206+
return fmt.Errorf("%w: %s", err, helmRepoURLRegexUIHint)
207+
}
208+
193209
// GetContent uses fetchToDir (and Helm for OCI) to read the files from directories and servers.
194210
func GetContent(ctx context.Context, base, source, version string, auth Auth, disableDepsUpdate bool, ignoreApplyConfigs []string) (map[string][]byte, error) {
195211
temp, err := os.MkdirTemp("", "fleet")

internal/bundlereader/loaddirectory_internal_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,47 @@
11
package bundlereader
22

33
import (
4+
"context"
45
"encoding/base64"
6+
"errors"
57
"net/url"
68
"testing"
79

810
"github.com/stretchr/testify/assert"
911
"github.com/stretchr/testify/require"
1012
)
1113

14+
func TestMaybeAddHelmRepoURLRegexHint(t *testing.T) {
15+
t.Run("adds hint when credentials were stripped for empty regex", func(t *testing.T) {
16+
err := errors.New("unauthorized")
17+
got := maybeAddHelmRepoURLRegexHint(err, directory{strippedCreds: true})
18+
require.Error(t, got)
19+
require.ErrorIs(t, got, err)
20+
assert.Contains(t, got.Error(), helmRepoURLRegexUIHint)
21+
})
22+
23+
t.Run("returns nil for nil error", func(t *testing.T) {
24+
got := maybeAddHelmRepoURLRegexHint(nil, directory{strippedCreds: true})
25+
assert.NoError(t, got)
26+
})
27+
28+
t.Run("keeps original error when marker is not set", func(t *testing.T) {
29+
err := errors.New("unauthorized")
30+
got := maybeAddHelmRepoURLRegexHint(err, directory{strippedCreds: false})
31+
assert.Equal(t, err, got)
32+
})
33+
34+
t.Run("keeps context cancellation errors unchanged", func(t *testing.T) {
35+
got := maybeAddHelmRepoURLRegexHint(context.Canceled, directory{strippedCreds: true})
36+
assert.Equal(t, context.Canceled, got)
37+
})
38+
39+
t.Run("keeps context deadline errors unchanged", func(t *testing.T) {
40+
got := maybeAddHelmRepoURLRegexHint(context.DeadlineExceeded, directory{strippedCreds: true})
41+
assert.Equal(t, context.DeadlineExceeded, got)
42+
})
43+
}
44+
1245
func TestSplitForcedScheme(t *testing.T) {
1346
tests := []struct {
1447
name string

internal/bundlereader/resources.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,8 @@ type directory struct {
148148
version string
149149
// auth is the auth to use for the chart URL
150150
auth Auth
151+
// indicates auth was stripped because helmRepoURLRegex was empty
152+
strippedCreds bool
151153
}
152154

153155
func addDirectory(base, customDir, defaultDir string) ([]directory, error) {
@@ -222,8 +224,12 @@ func addRemoteCharts(ctx context.Context, directories []directory, base string,
222224
return nil, fmt.Errorf("failed to add auth to request for %s: %w", downloadChartError(*chart), err)
223225
}
224226
auth := auth // loop-scoped variable
227+
strippedCredentialsForEmptyRegex := false
225228
if !shouldAddAuthToRequest {
226-
if !warnedOnce && helmRepoURLRegex == "" && (auth.Username != "" || auth.Password != "" || len(auth.SSHPrivateKey) > 0) {
229+
if helmRepoURLRegex == "" && (auth.Username != "" || auth.Password != "" || len(auth.SSHPrivateKey) > 0) {
230+
strippedCredentialsForEmptyRegex = true
231+
}
232+
if !warnedOnce && strippedCredentialsForEmptyRegex {
227233
logrus.Warn("helmRepoURLRegex is empty: Helm credentials will not be forwarded to any repository; set spec.helmRepoURLRegex to enable credential forwarding")
228234
warnedOnce = true
229235
}
@@ -239,11 +245,12 @@ func addRemoteCharts(ctx context.Context, directories []directory, base string,
239245
}
240246

241247
directories = append(directories, directory{
242-
prefix: checksum(chart),
243-
base: base,
244-
source: chartURL,
245-
auth: auth,
246-
version: chart.Version,
248+
prefix: checksum(chart),
249+
base: base,
250+
source: chartURL,
251+
auth: auth,
252+
version: chart.Version,
253+
strippedCreds: strippedCredentialsForEmptyRegex,
247254
})
248255
}
249256
}

internal/bundlereader/resources_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ func TestAddRemoteChartsStripsCredentials(t *testing.T) {
208208
assert.Nil(t, got.SSHPrivateKey, "SSHPrivateKey must be stripped when helmRepoURLRegex is empty")
209209
assert.True(t, got.BasicHTTP, "BasicHTTP must be preserved when stripping credentials")
210210
assert.True(t, got.InsecureSkipVerify, "InsecureSkipVerify must be preserved when stripping credentials")
211+
assert.True(t, dirs[0].strippedCreds, "directory should be marked when credentials are stripped due to empty helmRepoURLRegex")
211212
}
212213

213214
func TestAddRemoteChartsWarnsMissingRegex(t *testing.T) {

0 commit comments

Comments
 (0)