Merge pull request #31115 from banjoh/em/fix-username-password

fix: use username and password if provided
3 weeks ago · 4c615a505d
parent 892a5cbfd0 4aa2240750
commit 4c615a505d
2 changed files with 146 additions and 47 deletions
--- a/pkg/cmd/pull_test.go
+++ b/pkg/cmd/pull_test.go
@ -268,6 +268,78 @@ func TestPullCmd(t *testing.T) {
 	}
 }

+// runPullTests is a helper function to run pull command tests with common logic
+func runPullTests(t *testing.T, tests []struct {
+	name         string
+	args         string
+	existFile    string
+	existDir     string
+	wantError    bool
+	wantErrorMsg string
+	expectFile   string
+	expectDir    bool
+}, outdir string, additionalFlags string) {
+	t.Helper()
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd := fmt.Sprintf("pull %s -d '%s' --repository-config %s --repository-cache %s --registry-config %s %s",
+				tt.args,
+				outdir,
+				filepath.Join(outdir, "repositories.yaml"),
+				outdir,
+				filepath.Join(outdir, "config.json"),
+				additionalFlags,
+			)
+			// Create file or Dir before helm pull --untar, see: https://github.com/helm/helm/issues/7182
+			if tt.existFile != "" {
+				file := filepath.Join(outdir, tt.existFile)
+				_, err := os.Create(file)
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+			if tt.existDir != "" {
+				file := filepath.Join(outdir, tt.existDir)
+				err := os.Mkdir(file, 0755)
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+			_, _, err := executeActionCommand(cmd)
+			if err != nil {
+				if tt.wantError {
+					if tt.wantErrorMsg != "" && tt.wantErrorMsg == err.Error() {
+						t.Fatalf("Actual error %s, not equal to expected error %s", err, tt.wantErrorMsg)
+					}
+					return
+				}
+				t.Fatalf("%q reported error: %s", tt.name, err)
+			}
+
+			ef := filepath.Join(outdir, tt.expectFile)
+			fi, err := os.Stat(ef)
+			if err != nil {
+				t.Errorf("%q: expected a file at %s. %s", tt.name, ef, err)
+			}
+			if fi.IsDir() != tt.expectDir {
+				t.Errorf("%q: expected directory=%t, but it's not.", tt.name, tt.expectDir)
+			}
+		})
+	}
+}
+
+// buildOCIURL is a helper function to build OCI URLs with credentials
+func buildOCIURL(registryURL, chartName, version, username, password string) string {
+	baseURL := fmt.Sprintf("oci://%s/u/ocitestuser/%s", registryURL, chartName)
+	if version != "" {
+		baseURL += fmt.Sprintf(" --version %s", version)
+	}
+	if username != "" && password != "" {
+		baseURL += fmt.Sprintf(" --username %s --password %s", username, password)
+	}
+	return baseURL
+}
+
 func TestPullWithCredentialsCmd(t *testing.T) {
 	srv := repotest.NewTempServer(
 		t,
@ -323,52 +395,7 @@ func TestPullWithCredentialsCmd(t *testing.T) {
 		},
 	}

-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			outdir := srv.Root()
-			cmd := fmt.Sprintf("pull %s -d '%s' --repository-config %s --repository-cache %s --registry-config %s",
-				tt.args,
-				outdir,
-				filepath.Join(outdir, "repositories.yaml"),
-				outdir,
-				filepath.Join(outdir, "config.json"),
-			)
-			// Create file or Dir before helm pull --untar, see: https://github.com/helm/helm/issues/7182
-			if tt.existFile != "" {
-				file := filepath.Join(outdir, tt.existFile)
-				_, err := os.Create(file)
-				if err != nil {
-					t.Fatal(err)
-				}
-			}
-			if tt.existDir != "" {
-				file := filepath.Join(outdir, tt.existDir)
-				err := os.Mkdir(file, 0755)
-				if err != nil {
-					t.Fatal(err)
-				}
-			}
-			_, _, err := executeActionCommand(cmd)
-			if err != nil {
-				if tt.wantError {
-					if tt.wantErrorMsg != "" && tt.wantErrorMsg == err.Error() {
-						t.Fatalf("Actual error %s, not equal to expected error %s", err, tt.wantErrorMsg)
-					}
-					return
-				}
-				t.Fatalf("%q reported error: %s", tt.name, err)
-			}
-
-			ef := filepath.Join(outdir, tt.expectFile)
-			fi, err := os.Stat(ef)
-			if err != nil {
-				t.Errorf("%q: expected a file at %s. %s", tt.name, ef, err)
-			}
-			if fi.IsDir() != tt.expectDir {
-				t.Errorf("%q: expected directory=%t, but it's not.", tt.name, tt.expectDir)
-			}
-		})
-	}
+	runPullTests(t, tests, srv.Root(), "")
 }

 func TestPullVersionCompletion(t *testing.T) {
@ -401,6 +428,72 @@ func TestPullVersionCompletion(t *testing.T) {
 	runTestCmd(t, tests)
 }

+func TestPullWithCredentialsCmdOCIRegistry(t *testing.T) {
+	srv := repotest.NewTempServer(
+		t,
+		repotest.WithChartSourceGlob("testdata/testcharts/*.tgz*"),
+	)
+	defer srv.Stop()
+
+	ociSrv, err := repotest.NewOCIServer(t, srv.Root())
+	if err != nil {
+		t.Fatal(err)
+	}
+	ociSrv.Run(t)
+
+	if err := srv.LinkIndices(); err != nil {
+		t.Fatal(err)
+	}
+
+	// all flags will get "-d outdir" appended.
+	tests := []struct {
+		name         string
+		args         string
+		existFile    string
+		existDir     string
+		wantError    bool
+		wantErrorMsg string
+		expectFile   string
+		expectDir    bool
+	}{
+		{
+			name:       "OCI Chart fetch with credentials",
+			args:       buildOCIURL(ociSrv.RegistryURL, "oci-dependent-chart", "0.1.0", ociSrv.TestUsername, ociSrv.TestPassword),
+			expectFile: "./oci-dependent-chart-0.1.0.tgz",
+		},
+		{
+			name:       "OCI Chart fetch with credentials and untar",
+			args:       buildOCIURL(ociSrv.RegistryURL, "oci-dependent-chart", "0.1.0", ociSrv.TestUsername, ociSrv.TestPassword) + " --untar",
+			expectFile: "./oci-dependent-chart",
+			expectDir:  true,
+		},
+		{
+			name:       "OCI Chart fetch with credentials and untardir",
+			args:       buildOCIURL(ociSrv.RegistryURL, "oci-dependent-chart", "0.1.0", ociSrv.TestUsername, ociSrv.TestPassword) + " --untar --untardir ocitest-credentials",
+			expectFile: "./ocitest-credentials",
+			expectDir:  true,
+		},
+		{
+			name:      "Fail fetching OCI chart with wrong credentials",
+			args:      buildOCIURL(ociSrv.RegistryURL, "oci-dependent-chart", "0.1.0", "wronguser", "wrongpass"),
+			wantError: true,
+		},
+		{
+			name:      "Fail fetching non-existent OCI chart with credentials",
+			args:      buildOCIURL(ociSrv.RegistryURL, "nosuchthing", "0.1.0", ociSrv.TestUsername, ociSrv.TestPassword),
+			wantError: true,
+		},
+		{
+			name:         "Fail fetching OCI chart without version specified",
+			args:         buildOCIURL(ociSrv.RegistryURL, "nosuchthing", "", ociSrv.TestUsername, ociSrv.TestPassword),
+			wantErrorMsg: "Error: --version flag is explicitly required for OCI registries",
+			wantError:    true,
+		},
+	}
+
+	runPullTests(t, tests, srv.Root(), "--plain-http")
+}
+
 func TestPullFileCompletion(t *testing.T) {
 	checkFileCompletion(t, "pull", false)
 	checkFileCompletion(t, "pull repo/chart", false)
--- a/pkg/registry/client.go
+++ b/pkg/registry/client.go
@ -128,7 +128,13 @@ func NewClient(options ...ClientOption) (*Client, error) {
 		}
 		authorizer.SetUserAgent(version.GetUserAgent())

-		authorizer.Credential = credentials.Credential(client.credentialsStore)
+		if client.username != "" && client.password != "" {
+			authorizer.Credential = func(_ context.Context, _ string) (auth.Credential, error) {
+				return auth.Credential{Username: client.username, Password: client.password}, nil
+			}
+		} else {
+			authorizer.Credential = credentials.Credential(client.credentialsStore)
+		}

 		if client.enableCache {
 			authorizer.Cache = auth.NewCache()