From 802272db8357d200e998451366d95f10af7a7fdb Mon Sep 17 00:00:00 2001
From: James Telfer <792299+jamestelfer@users.noreply.github.com>
Date: Tue, 30 Jul 2024 23:07:35 +1000
Subject: [PATCH 1/2] fix: fail silently for unsupported actions

Credential storage makes little sense for a plugin based on environment variables.

Failure to store or delete is written as a warning, but will not fail the Docker process.
---
 helper/env.go      | 10 ++++------
 helper/env_test.go | 10 +++++-----
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/helper/env.go b/helper/env.go
index 0c7152f..632ab3b 100644
--- a/helper/env.go
+++ b/helper/env.go
@@ -30,18 +30,16 @@ func (e EnvHelper) Get(serverURL string) (string, string, error) {
 	logger.Info("Get", "user", user, "err", err)
 
 	return user, password, err
-
 }
 
 func (e EnvHelper) Add(creds *credentials.Credentials) error {
-
-	slog.Info("", "action", "add", "serverURL", creds.ServerURL, "username", creds.Username)
-	return ErrNotImplemented
+	slog.Warn("Saving credentials is not supported by docker-credential-env", "action", "add", "serverURL", creds.ServerURL, "username", creds.Username)
+	return nil
 }
 
 func (e EnvHelper) Delete(serverURL string) error {
-	slog.Info("", "action", "delete", "serverURL", serverURL)
-	return ErrNotImplemented
+	slog.Warn("Deleting credentials is not supported by docker-credential-env", "action", "delete", "serverURL", serverURL)
+	return nil
 }
 
 func (e EnvHelper) List() (map[string]string, error) {
diff --git a/helper/env_test.go b/helper/env_test.go
index 4baee8b..869fa86 100644
--- a/helper/env_test.go
+++ b/helper/env_test.go
@@ -31,19 +31,19 @@ func TestEnvHelper_Get_Failure(t *testing.T) {
 	assert.Empty(t, password)
 }
 
-func TestEnvHelper_Add(t *testing.T) {
+func TestEnvHelper_Add_FailsSilently(t *testing.T) {
 	helper := EnvHelper{}
 	err := helper.Add(&credentials.Credentials{})
-	assert.ErrorIs(t, err, ErrNotImplemented)
+	assert.NoError(t, err)
 }
 
-func TestEnvHelper_Delete(t *testing.T) {
+func TestEnvHelper_Delete_FailsSilently(t *testing.T) {
 	helper := EnvHelper{}
 	err := helper.Delete("foo")
-	assert.ErrorIs(t, err, ErrNotImplemented)
+	assert.NoError(t, err)
 }
 
-func TestEnvHelper_List(t *testing.T) {
+func TestEnvHelper_List_NotImplemented(t *testing.T) {
 	helper := EnvHelper{}
 	_, err := helper.List()
 	assert.ErrorIs(t, err, ErrNotImplemented)

From e1a3945590574f7069d7f4296fb03ef2e8025c2a Mon Sep 17 00:00:00 2001
From: James Telfer <792299+jamestelfer@users.noreply.github.com>
Date: Tue, 30 Jul 2024 23:33:44 +1000
Subject: [PATCH 2/2] fix: allow credentials discovery to be optional

Allow the user to configure the helper such that failure to find the required environment variables results in an empty username and password being returned instead of causing the helper to fail.
---
 helper/env.go      | 19 +++++++++++++++----
 helper/env_test.go | 11 +++++++++++
 main.go            |  4 +++-
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/helper/env.go b/helper/env.go
index 632ab3b..10882b3 100644
--- a/helper/env.go
+++ b/helper/env.go
@@ -19,15 +19,27 @@ var nonAlphanumericPattern = regexp.MustCompile(`[^a-zA-Z0-9]`)
 var _ credentials.Helper = EnvHelper{}
 
 type EnvHelper struct {
+	CredentialsOptional bool
 }
 
+// Get retrieves the credentials for the server URL from the process
+// environment.
 func (e EnvHelper) Get(serverURL string) (string, string, error) {
 	logger := slog.Default().With("action", "get", "serverURL", serverURL)
 
-	// FIXME: allow this to return empty on failure if so configured
 	user, password, err := credentialsForServer(serverURL)
 
-	logger.Info("Get", "user", user, "err", err)
+	if err != nil {
+		if e.CredentialsOptional {
+			logger.Warn("Ignoring failed credential lookup, unset DOCKER_CREDENTIALS_ENV_OPTIONAL if this should cause Docker to fail", "err", err)
+			return "", "", nil
+		}
+
+		logger.Error("Failed to retrieve credentials, set DOCKER_CREDENTIALS_ENV_OPTIONAL to ignore this error.", "err", err)
+		return "", "", err
+	}
+
+	logger.Info("Got credentials from environment", "user", user, "err", err)
 
 	return user, password, err
 }
@@ -43,8 +55,7 @@ func (e EnvHelper) Delete(serverURL string) error {
 }
 
 func (e EnvHelper) List() (map[string]string, error) {
-	slog.Info("", "action", "list")
-
+	slog.Error("List action not implemented", "action", "list")
 	return nil, ErrNotImplemented
 }
 
diff --git a/helper/env_test.go b/helper/env_test.go
index 869fa86..8acf9f1 100644
--- a/helper/env_test.go
+++ b/helper/env_test.go
@@ -21,6 +21,17 @@ func TestEnvHelper_Get_Success(t *testing.T) {
 	assert.Equal(t, "testpassword", password)
 }
 
+func TestEnvHelper_Get_CredentialsOptional_Success(t *testing.T) {
+
+	helper := EnvHelper{CredentialsOptional: true}
+
+	user, password, err := helper.Get("example.com")
+
+	assert.NoError(t, err)
+	assert.Empty(t, user)
+	assert.Empty(t, password)
+}
+
 func TestEnvHelper_Get_Failure(t *testing.T) {
 	helper := EnvHelper{}
 
diff --git a/main.go b/main.go
index d6bfea7..872101d 100644
--- a/main.go
+++ b/main.go
@@ -21,7 +21,9 @@ func main() {
 
 	configureLogging()
 
-	credentials.Serve(helper.EnvHelper{})
+	credentialsOptional := os.Getenv("DOCKER_CREDENTIALS_ENV_OPTIONAL") == "true"
+
+	credentials.Serve(helper.EnvHelper{CredentialsOptional: credentialsOptional})
 }
 
 func configureLogging() {