Skip to content

Commit 0874e0d

Browse files
authored
Warn when Helm credentials are set but helmRepoURLRegex is empty (#5316) (#5334)
When Helm credentials are configured in an Auth object but helmRepoURLRegex is empty, addRemoteCharts silently drops the credentials. Log a Warn-level message so operators know credentials are not forwarded and how to fix it by setting spec.helmRepoURLRegex.
1 parent 2c9dfa8 commit 0874e0d

2 files changed

Lines changed: 79 additions & 0 deletions

File tree

internal/bundlereader/resources.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"sync"
1212

1313
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
14+
"github.com/sirupsen/logrus"
1415
"golang.org/x/sync/errgroup"
1516
"golang.org/x/sync/semaphore"
1617

@@ -206,13 +207,18 @@ func mergeGenericMap(first, second *fleet.GenericMap) *fleet.GenericMap {
206207
// For every chart that is not on disk, create a directory struct that contains the charts URL as path.
207208
// This adds one directory per HelmOption.
208209
func addRemoteCharts(directories []directory, base string, charts []*fleet.HelmOptions, auth Auth, helmRepoURLRegex string) ([]directory, error) {
210+
warnedOnce := false
209211
for _, chart := range charts {
210212
if _, err := os.Stat(filepath.Join(base, chart.Chart)); os.IsNotExist(err) || chart.Repo != "" {
211213
shouldAddAuthToRequest, err := shouldAddAuthToRequest(helmRepoURLRegex, chart.Repo, chart.Chart)
212214
if err != nil {
213215
return nil, fmt.Errorf("failed to add auth to request for %s: %w", downloadChartError(*chart), err)
214216
}
215217
if !shouldAddAuthToRequest {
218+
if !warnedOnce && helmRepoURLRegex == "" && (auth.Username != "" || auth.Password != "" || len(auth.SSHPrivateKey) > 0) {
219+
logrus.Warn("helmRepoURLRegex is empty: Helm credentials will not be forwarded to any repository; set spec.helmRepoURLRegex to enable credential forwarding")
220+
warnedOnce = true
221+
}
216222
// Only clear credentials; preserve transport settings (BasicHTTP, CABundle, InsecureSkipVerify)
217223
auth.Username = ""
218224
auth.Password = ""

internal/bundlereader/resources_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"bytes"
55
"testing"
66

7+
"github.com/sirupsen/logrus"
8+
79
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
810

911
"github.com/stretchr/testify/assert"
@@ -206,3 +208,74 @@ func TestAddRemoteChartsStripsCredentials(t *testing.T) {
206208
assert.True(t, got.BasicHTTP, "BasicHTTP must be preserved when stripping credentials")
207209
assert.True(t, got.InsecureSkipVerify, "InsecureSkipVerify must be preserved when stripping credentials")
208210
}
211+
212+
func TestAddRemoteChartsWarnsMissingRegex(t *testing.T) {
213+
remoteChart := []*fleet.HelmOptions{{Chart: "/nonexistent/chart"}}
214+
215+
tests := []struct {
216+
name string
217+
charts []*fleet.HelmOptions
218+
auth Auth
219+
regex string
220+
wantWarning bool
221+
}{
222+
{
223+
name: "credentials set, regex empty — warn",
224+
charts: remoteChart,
225+
auth: Auth{Username: "user", Password: "secret"},
226+
regex: "",
227+
wantWarning: true,
228+
},
229+
{
230+
name: "SSH key set, regex empty — warn",
231+
charts: remoteChart,
232+
auth: Auth{SSHPrivateKey: []byte("key")},
233+
regex: "",
234+
wantWarning: true,
235+
},
236+
{
237+
name: "no credentials, regex empty — no warn",
238+
charts: remoteChart,
239+
auth: Auth{BasicHTTP: true},
240+
regex: "",
241+
wantWarning: false,
242+
},
243+
{
244+
name: "credentials set, regex provided — no warn",
245+
charts: remoteChart,
246+
auth: Auth{Username: "user", Password: "secret"},
247+
regex: "https://charts\\.example\\.com.*",
248+
wantWarning: false,
249+
},
250+
{
251+
name: "credentials set, regex empty, no charts — no warn",
252+
charts: nil,
253+
auth: Auth{Username: "user", Password: "secret"},
254+
regex: "",
255+
wantWarning: false,
256+
},
257+
}
258+
259+
for _, tt := range tests {
260+
t.Run(tt.name, func(t *testing.T) {
261+
var buf bytes.Buffer
262+
oldOut := logrus.StandardLogger().Out
263+
oldLevel := logrus.GetLevel()
264+
logrus.SetOutput(&buf)
265+
logrus.SetLevel(logrus.WarnLevel)
266+
t.Cleanup(func() {
267+
logrus.SetOutput(oldOut)
268+
logrus.SetLevel(oldLevel)
269+
})
270+
271+
_, err := addRemoteCharts(nil, t.TempDir(), tt.charts, tt.auth, tt.regex)
272+
require.NoError(t, err)
273+
274+
if tt.wantWarning {
275+
assert.Contains(t, buf.String(), "helmRepoURLRegex")
276+
} else {
277+
assert.NotContains(t, buf.String(), "helmRepoURLRegex")
278+
}
279+
})
280+
}
281+
}

0 commit comments

Comments
 (0)