diff --git a/cmd/helm/registry_login_test.go b/cmd/helm/registry_login_test.go index 517fe08e1..10485f4cf 100644 --- a/cmd/helm/registry_login_test.go +++ b/cmd/helm/registry_login_test.go @@ -17,9 +17,73 @@ limitations under the License. package main import ( + "fmt" + "os" + "path/filepath" "testing" + + "helm.sh/helm/v3/pkg/repo/repotest" ) func TestRegistryLoginFileCompletion(t *testing.T) { checkFileCompletion(t, "registry login", false) } + +func TestRegistryLogin(t *testing.T) { + srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*") + if err != nil { + t.Fatal(err) + } + defer srv.Stop() + + ociSrv, err := repotest.NewOCIServer(t, srv.Root()) + if err != nil { + t.Fatal(err) + } + ociSrv.Run(t) + + // Creating an empty json file to use as an empty registry configuration file. Used for + // testing the handling of this situation. + tmpDir := t.TempDir() + emptyFile, err := os.Create(filepath.Join(tmpDir, "empty.json")) + if err != nil { + t.Fatal(err) + } + emptyFile.Close() + + tests := []struct { + name string + cmd string + wantError bool + wantErrorMsg string + }{ + { + name: "Normal login", + cmd: fmt.Sprintf("registry login %s --username %s --password %s", ociSrv.RegistryURL, ociSrv.TestUsername, ociSrv.TestPassword), + }, + { + name: "Failed login", + cmd: fmt.Sprintf("registry login %s --username foo --password bar", ociSrv.RegistryURL), + wantError: true, + }, + { + name: "Normal login with empty registry file", + cmd: fmt.Sprintf("registry login %s --username %s --password %s --registry-config %s", ociSrv.RegistryURL, ociSrv.TestUsername, ociSrv.TestPassword, emptyFile.Name()), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, err := executeActionCommand(tt.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) + } + }) + } +} diff --git a/pkg/registry/client.go b/pkg/registry/client.go index ea197e6f7..f172aaca8 100644 --- a/pkg/registry/client.go +++ b/pkg/registry/client.go @@ -81,6 +81,9 @@ type ( httpClient *http.Client plainHTTP bool err error // pass any errors from the ClientOption functions + + // credentialsFileTemp captures if the empty file / EOF work around is being used. + credentialsFileTemp bool } // ClientOption allows specifying various settings configurable by the user for overriding the defaults @@ -115,11 +118,26 @@ func NewClient(options ...ClientOption) (*Client, error) { } store, err := credentials.NewStore(client.credentialsFile, storeOptions) if err != nil { - return nil, err + // If the file exists and is empty there will be an EOF error. This error is not wrapped so + // a check with errors.Is will not work. The only way to capture it is an EOF error is + // with string parsing. + // This handling passes no file location which will cause NewStore to invoke its + // fault tolerance for a file not existing. A bool records this bypass so that if the + // credential store needs to be written to it this work around can be handled. See the + // Login method for more details. + if strings.Contains(err.Error(), "invalid config format: EOF") { + var err2 error + store, err2 = credentials.NewStore("", storeOptions) + if err2 != nil { + return nil, err + } + client.credentialsFileTemp = true + } else { + return nil, err + } } dockerStore, err := credentials.NewStoreFromDocker(storeOptions) if err != nil { - // should only fail if user home directory can't be determined client.credentialsStore = store } else { // use Helm credentials with fallback to Docker @@ -284,6 +302,31 @@ func (c *Client) Login(host string, options ...LoginOption) error { } key := credentials.ServerAddressFromRegistry(host) + + // The credentialsStore loader does not handle empty files. So, there is a workaround. + // This can be removed when the credentials loader can handle empty files. + // When Helm catches an empty file error it causes the loader to trigger its fault + // tolerance for a file not existing and records it with a bool. If that bool is set and the + // file needs to be written, the file needs to be put into a usable state and loaded + // properly. + // See the NewClient function for the bypass setup. + if c.credentialsFileTemp { + err = os.WriteFile(c.credentialsFile, []byte("{}"), 0600) + if err != nil { + return err + } + storeOptions := credentials.StoreOptions{ + AllowPlaintextPut: true, + DetectDefaultNativeStore: true, + } + store, err := credentials.NewStore(c.credentialsFile, storeOptions) + if err != nil { + return err + } + c.credentialsStore = store + c.credentialsFileTemp = false + } + if err := c.credentialsStore.Put(ctx, key, cred); err != nil { return err }