Merge pull request #30932 from mattfarina/fix-empty-registry-config-file

Handle an empty registry config file.

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)
+			}
+		})
+	}
+}

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
 	}