Attempt 2: Check for Podman's auth in DefaultKeychain (#1185)

* Revert "Revert "Check for Podman's auth.json in DefaultKeychain (#1181)" (#1184)"

This reverts commit bce5496.

* Attempt 2: Check for Podman's auth in DefaultKeychain

* go mod download in k8schain to pick up the new dep

* remove some debug logging

go.mod

@@ -16,6 +16,7 @@ require (
 	github.com/google/go-cmp v0.5.6
 	github.com/gorilla/mux v1.8.0 // indirect
 	github.com/kr/text v0.2.0 // indirect
+	github.com/mitchellh/go-homedir v1.1.0
 	github.com/morikuni/aec v1.0.0 // indirect
 	github.com/opencontainers/image-spec v1.0.2-0.20210730191737-8e42a01fb1b7
 	github.com/spf13/cobra v1.2.1

pkg/authn/keychain.go

@@ -16,11 +16,14 @@ package authn
 
 import (
 	"os"
+	"path/filepath"
 	"sync"
 
 	"github.com/docker/cli/cli/config"
+	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/cli/config/types"
 	"github.com/google/go-containerregistry/pkg/name"
+	"github.com/mitchellh/go-homedir"
 )
 
 // Resource represents a registry or repository that can be authenticated against.
@@ -62,9 +65,52 @@ const (
 func (dk *defaultKeychain) Resolve(target Resource) (Authenticator, error) {
 	dk.mu.Lock()
 	defer dk.mu.Unlock()
-	cf, err := config.Load(os.Getenv("DOCKER_CONFIG"))
-	if err != nil {
-		return nil, err
+
+	// Podman users may have their container registry auth configured in a
+	// different location, that Docker packages aren't aware of.
+	// If the Docker config file isn't found, we'll fallback to look where
+	// Podman configures it, and parse that as a Docker auth config instead.
+
+	// First, check $HOME/.docker/config.json
+	foundDockerConfig := false
+	home, err := homedir.Dir()
+	if err == nil {
+		if _, err := os.Stat(filepath.Join(home, ".docker/config.json")); err == nil {
+			foundDockerConfig = true
+		}
+	}
+	// If $HOME/.docker/config.json isn't found, check $DOCKER_CONFIG (if set)
+	if !foundDockerConfig && os.Getenv("DOCKER_CONFIG") != "" {
+		if _, err := os.Stat(filepath.Join(os.Getenv("DOCKER_CONFIG"), "config.json")); err == nil {
+			foundDockerConfig = true
+		}
+	}
+	// If either of those locations are found, load it using Docker's
+	// config.Load, which may fail if the config can't be parsed.
+	//
+	// If neither was found, look for Podman's auth at
+	// $XDG_RUNTIME_DIR/containers/auth.json and attempt to load it as a
+	// Docker config.
+	//
+	// If neither are found, fallback to Anonymous.
+	var cf *configfile.ConfigFile
+	if foundDockerConfig {
+		cf, err = config.Load(os.Getenv("DOCKER_CONFIG"))
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		f, err := os.Open(filepath.Join(os.Getenv("XDG_RUNTIME_DIR"), "containers/auth.json"))
+		if os.IsNotExist(err) {
+			return Anonymous, nil
+		} else if err != nil {
+			return nil, err
+		}
+		defer f.Close()
+		cf, err = config.LoadFromReader(f)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	// See:

pkg/authn/keychain_test.go

@@ -18,6 +18,7 @@ import (
 	"encoding/base64"
 	"fmt"
 	"io/ioutil"
+	"log"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -32,6 +33,20 @@ var (
 	defaultRegistry, _ = name.NewRegistry(name.DefaultRegistry, name.WeakValidation)
 )
 
+func TestMain(m *testing.M) {
+	// Set $HOME to a temp empty dir, to ensure $HOME/.docker/config.json
+	// isn't unexpectedly found.
+	tmp, err := ioutil.TempDir("", "keychain_test_home")
+	if err != nil {
+		log.Fatal(err)
+	}
+	os.Setenv("HOME", tmp)
+	os.Exit(func() int {
+		defer os.RemoveAll(tmp)
+		return m.Run()
+	}())
+}
+
 // setupConfigDir sets up an isolated configDir() for this test.
 func setupConfigDir(t *testing.T) string {
 	tmpdir := os.Getenv("TEST_TMPDIR")
@@ -43,8 +58,9 @@ func setupConfigDir(t *testing.T) string {
 		}
 	}
 
-	fresh = fresh + 1
-	p := fmt.Sprintf("%s/%d", tmpdir, fresh)
+	fresh++
+	p := filepath.Join(tmpdir, fmt.Sprintf("%d", fresh))
+	t.Logf("DOCKER_CONFIG=%s", p)
 	os.Setenv("DOCKER_CONFIG", p)
 	if err := os.Mkdir(p, 0777); err != nil {
 		t.Fatalf("mkdir %q: %v", p, err)
@@ -77,33 +93,130 @@ func TestNoConfig(t *testing.T) {
 	}
 }
 
+func TestPodmanConfig(t *testing.T) {
+	tmpdir := os.Getenv("TEST_TMPDIR")
+	if tmpdir == "" {
+		var err error
+		tmpdir, err = ioutil.TempDir("", "keychain_test")
+		if err != nil {
+			t.Fatalf("creating temp dir: %v", err)
+		}
+	}
+	fresh++
+	p := filepath.Join(tmpdir, fmt.Sprintf("%d", fresh))
+	os.Setenv("XDG_RUNTIME_DIR", p)
+	os.Unsetenv("DOCKER_CONFIG")
+	if err := os.MkdirAll(filepath.Join(p, "containers"), 0777); err != nil {
+		t.Fatalf("mkdir %s/containers: %v", p, err)
+	}
+	cfg := filepath.Join(p, "containers/auth.json")
+	content := fmt.Sprintf(`{"auths": {"test.io": {"auth": %q}}}`, encode("foo", "bar"))
+	if err := ioutil.WriteFile(cfg, []byte(content), 0600); err != nil {
+		t.Fatalf("write %q: %v", cfg, err)
+	}
+
+	// At first, $DOCKER_CONFIG is unset and $HOME/.docker/config.json isn't
+	// found, but Podman auth is configured. This should return Podman's
+	// auth.
+	auth, err := DefaultKeychain.Resolve(testRegistry)
+	if err != nil {
+		t.Fatalf("Resolve() = %v", err)
+	}
+	got, err := auth.Authorization()
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := &AuthConfig{
+		Username: "foo",
+		Password: "bar",
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %+v, want %+v", got, want)
+	}
+
+	// Now, configure $HOME/.docker/config.json, which should override
+	// Podman auth and be used.
+	if err := os.MkdirAll(filepath.Join(os.Getenv("HOME"), ".docker"), 0777); err != nil {
+		t.Fatalf("mkdir $HOME/.docker: %v", err)
+	}
+	cfg = filepath.Join(os.Getenv("HOME"), ".docker/config.json")
+	content = fmt.Sprintf(`{"auths": {"test.io": {"auth": %q}}}`, encode("home-foo", "home-bar"))
+	if err := ioutil.WriteFile(cfg, []byte(content), 0600); err != nil {
+		t.Fatalf("write %q: %v", cfg, err)
+	}
+	auth, err = DefaultKeychain.Resolve(testRegistry)
+	if err != nil {
+		t.Fatalf("Resolve() = %v", err)
+	}
+	got, err = auth.Authorization()
+	if err != nil {
+		t.Fatal(err)
+	}
+	want = &AuthConfig{
+		Username: "home-foo",
+		Password: "home-bar",
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %+v, want %+v", got, want)
+	}
+
+	// Then, configure DOCKER_CONFIG with a valid config file with different
+	// auth configured.
+	// This demonstrates that DOCKER_CONFIG is preferred over Podman auth
+	// and $HOME/.docker/config.json.
+	content = fmt.Sprintf(`{"auths": {"test.io": {"auth": %q}}}`, encode("another-foo", "another-bar"))
+	cd := setupConfigFile(t, content)
+	defer os.RemoveAll(filepath.Dir(cd))
+
+	auth, err = DefaultKeychain.Resolve(testRegistry)
+	if err != nil {
+		t.Fatalf("Resolve() = %v", err)
+	}
+	got, err = auth.Authorization()
+	if err != nil {
+		t.Fatal(err)
+	}
+	want = &AuthConfig{
+		Username: "another-foo",
+		Password: "another-bar",
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("got %+v, want %+v", got, want)
+	}
+}
+
 func encode(user, pass string) string {
 	delimited := fmt.Sprintf("%s:%s", user, pass)
 	return base64.StdEncoding.EncodeToString([]byte(delimited))
 }
 
 func TestVariousPaths(t *testing.T) {
 	tests := []struct {
+		desc    string
 		content string
 		wantErr bool
 		target  name.Registry
 		cfg     *AuthConfig
 	}{{
+		desc:    "invalid config file",
 		target:  testRegistry,
 		content: `}{`,
 		wantErr: true,
 	}, {
+		desc:    "creds store does not exist",
 		target:  testRegistry,
 		content: `{"credsStore":"#definitely-does-not-exist"}`,
 		wantErr: true,
 	}, {
+		desc:    "valid config file",
 		target:  testRegistry,
 		content: fmt.Sprintf(`{"auths": {"test.io": {"auth": %q}}}`, encode("foo", "bar")),
 		cfg: &AuthConfig{
 			Username: "foo",
 			Password: "bar",
 		},
 	}, {
+		desc:    "valid config file; default registry",
 		target:  defaultRegistry,
 		content: fmt.Sprintf(`{"auths": {"%s": {"auth": %q}}}`, DefaultAuthKey, encode("foo", "bar")),
 		cfg: &AuthConfig{
@@ -113,29 +226,31 @@ func TestVariousPaths(t *testing.T) {
 	}}
 
 	for _, test := range tests {
-		cd := setupConfigFile(t, test.content)
-		// For some reason, these tempdirs don't get cleaned up.
-		defer os.RemoveAll(filepath.Dir(cd))
-
-		auth, err := DefaultKeychain.Resolve(test.target)
-		if test.wantErr {
-			if err == nil {
-				t.Fatal("wanted err, got nil")
-			} else if err != nil {
-				// success
-				continue
+		t.Run(test.desc, func(t *testing.T) {
+			cd := setupConfigFile(t, test.content)
+			// For some reason, these tempdirs don't get cleaned up.
+			defer os.RemoveAll(filepath.Dir(cd))
+
+			auth, err := DefaultKeychain.Resolve(test.target)
+			if test.wantErr {
+				if err == nil {
+					t.Fatal("wanted err, got nil")
+				} else if err != nil {
+					// success
+					return
+				}
+			}
+			if err != nil {
+				t.Fatalf("wanted nil, got err: %v", err)
+			}
+			cfg, err := auth.Authorization()
+			if err != nil {
+				t.Fatal(err)
 			}
-		}
-		if err != nil {
-			t.Fatalf("wanted nil, got err: %v", err)
-		}
-		cfg, err := auth.Authorization()
-		if err != nil {
-			t.Fatal(err)
-		}
 
-		if !reflect.DeepEqual(cfg, test.cfg) {
-			t.Errorf("got %+v, want %+v", cfg, test.cfg)
-		}
+			if !reflect.DeepEqual(cfg, test.cfg) {
+				t.Errorf("got %+v, want %+v", cfg, test.cfg)
+			}
+		})
 	}
 }