Use default keychain to resolve OCI credentials (#8274)

The ECR credential helper stopped working for one of our on-prem users
after they switched from podman to OCI. This is because `oci.Resolve()`
doesn't respect credential helpers.

This PR enables the default keychain in `oci.Resolve()` (behind a flag)
so that credential helpers will work. The default keychain respects
commonly used container auth config files, including
`.docker/config.json` and podman's
`$XDG_RUNTIME_DIR/containers/auth.json`.

The customer was specifically trying to use
`~/.config/containers/auth.json` which doesn't quite work after this PR,
but will work if we either patch
google/go-containerregistry#2052 or if the
customer switches to `~/.docker/config.json` instead (that file is
generally the least common denominator and is supported by docker,
podman, and now oci after this PR).

The plan is to either enable this new flag by default (but make sure
it's disabled for our cloud executors), or just enable it in the helm
charts.

Author: bduffany

enterprise/server/util/oci/BUILD

@@ -32,6 +32,8 @@ go_test(
         ":oci",
         "//enterprise/server/remote_execution/platform",
         "//proto:registry_go_proto",
+        "//server/testutil/testenviron",
+        "//server/testutil/testfs",
         "//server/testutil/testregistry",
         "//server/util/proto",
         "//server/util/status",

enterprise/server/util/oci/oci.go

@@ -24,8 +24,9 @@ import (
 )
 
 var (
-	registries = flag.Slice("executor.container_registries", []Registry{}, "")
-	mirrors    = flag.Slice("executor.container_registry_mirrors", []MirrorConfig{}, "")
+	registries             = flag.Slice("executor.container_registries", []Registry{}, "")
+	mirrors                = flag.Slice("executor.container_registry_mirrors", []MirrorConfig{}, "")
+	defaultKeychainEnabled = flag.Bool("executor.container_registry_default_keychain_enabled", false, "Enable the default container registry keychain, respecting both docker configs and podman configs.")
 )
 
 type MirrorConfig struct {
@@ -76,7 +77,7 @@ func CredentialsFromProto(creds *rgpb.Credentials) (Credentials, error) {
 // Extracts the container registry Credentials from the provided platform
 // properties, falling back to credentials specified in
 // --executor.container_registries if the platform properties credentials are
-// absent.
+// absent, then falling back to the default keychain (docker/podman config JSON)
 func CredentialsFromProperties(props *platform.Properties) (Credentials, error) {
 	imageRef := props.ContainerImage
 	if imageRef == "" {
@@ -92,10 +93,6 @@ func CredentialsFromProperties(props *platform.Properties) (Credentials, error)
 
 	// If no credentials were provided, fallback to any specified by
 	// --executor.container_registries.
-	if len(*registries) == 0 {
-		return Credentials{}, nil
-	}
-
 	ref, err := reference.ParseNormalizedNamed(imageRef)
 	if err != nil {
 		log.Debugf("Failed to parse image ref %q: %s", imageRef, err)
@@ -113,9 +110,44 @@ func CredentialsFromProperties(props *platform.Properties) (Credentials, error)
 		}
 	}
 
+	// No matching registries were found in the executor config. Fall back to
+	// the default keychain.
+	if *defaultKeychainEnabled {
+		return resolveWithDefaultKeychain(ref)
+	}
+
 	return Credentials{}, nil
 }
 
+// Reads the auth configuration from a set of commonly supported config file
+// locations such as ~/.docker/config.json or
+// $XDG_RUNTIME_DIR/containers/auth.json, and returns any configured
+// credentials, possibly by invoking a credential helper if applicable.
+func resolveWithDefaultKeychain(ref reference.Named) (Credentials, error) {
+	// TODO: parse the errors below and if they're 403/401 errors then return
+	// Unauthenticated/PermissionDenied
+	ctrRef, err := ctrname.ParseReference(ref.String())
+	if err != nil {
+		log.Debugf("Failed to parse image ref %q: %s", ref.String(), err)
+		return Credentials{}, nil
+	}
+	authenticator, err := authn.DefaultKeychain.Resolve(ctrRef.Context())
+	if err != nil {
+		return Credentials{}, status.UnavailableErrorf("resolve default keychain: %s", err)
+	}
+	authConfig, err := authenticator.Authorization()
+	if err != nil {
+		return Credentials{}, status.UnavailableErrorf("authorize via default keychain: %s", err)
+	}
+	if authConfig == nil {
+		return Credentials{}, nil
+	}
+	return Credentials{
+		Username: authConfig.Username,
+		Password: authConfig.Password,
+	}, nil
+}
+
 func credentials(username, password string) (Credentials, error) {
 	if username == "" && password != "" {
 		return Credentials{}, status.InvalidArgumentError(

enterprise/server/util/oci/oci_test.go

@@ -6,12 +6,16 @@ import (
 	"context"
 	"io"
 	"net/http"
+	"os"
+	"path/filepath"
 	"regexp"
 	"runtime"
 	"testing"
 
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/remote_execution/platform"
 	"github.com/buildbuddy-io/buildbuddy/enterprise/server/util/oci"
+	"github.com/buildbuddy-io/buildbuddy/server/testutil/testenviron"
+	"github.com/buildbuddy-io/buildbuddy/server/testutil/testfs"
 	"github.com/buildbuddy-io/buildbuddy/server/testutil/testregistry"
 	"github.com/buildbuddy-io/buildbuddy/server/util/proto"
 	"github.com/buildbuddy-io/buildbuddy/server/util/status"
@@ -135,6 +139,62 @@ func TestCredentialsFromProperties(t *testing.T) {
 	}
 }
 
+func TestCredentialsFromProperties_DefaultKeychain(t *testing.T) {
+	tmp := testfs.MakeTempDir(t)
+	testfs.WriteAllFileContents(t, tmp, map[string]string{
+		// Write a credential helper that always returns the creds
+		// test-user:test-pass
+		"bin/docker-credential-test": `#!/usr/bin/env sh
+			if [ "$1" = "get" ]; then
+				echo '{"ServerURL":"","Username":"test-user","Secret":"test-pass"}'
+				exit 0
+			else
+				echo "Not implemented"
+				exit 1
+			fi
+		`,
+		// Set up docker-credential-test as the credential helper for
+		// "testregistry.io". Note the "docker-credential-" prefix is omitted.
+		".docker/config.json": `{"credHelpers": {"testregistry.io": "test"}}`,
+	})
+	testfs.MakeExecutable(t, tmp, "bin/docker-credential-test")
+	// The default keychain reads config.json from the path specified in the
+	// DOCKER_CONFIG environment variable, if set. Point that to our directory.
+	testenviron.Set(t, "DOCKER_CONFIG", filepath.Join(tmp, ".docker"))
+	// Add our directory to PATH so the default keychain can find our credential
+	// helper binary.
+	testenviron.Set(t, "PATH", filepath.Join(tmp, "bin")+":"+os.Getenv("PATH"))
+
+	for _, test := range []struct {
+		Name                   string
+		DefaultKeychainEnabled bool
+		ExpectedCredentials    oci.Credentials
+	}{
+		{
+			Name:                   "invokes credential helper if default keychain enabled",
+			DefaultKeychainEnabled: true,
+			ExpectedCredentials:    oci.Credentials{Username: "test-user", Password: "test-pass"},
+		},
+		{
+			Name:                   "returns empty credentials if default keychain disabled",
+			DefaultKeychainEnabled: false,
+			ExpectedCredentials:    oci.Credentials{},
+		},
+	} {
+		t.Run(test.Name, func(t *testing.T) {
+			flags.Set(t, "executor.container_registry_default_keychain_enabled", test.DefaultKeychainEnabled)
+
+			creds, err := oci.CredentialsFromProperties(&platform.Properties{
+				ContainerImage: "testregistry.io/test-repo/test-img",
+			})
+			require.NoError(t, err)
+
+			require.Equal(t, test.ExpectedCredentials, creds)
+		})
+	}
+
+}
+
 func creds(username, password string) oci.Credentials {
 	return oci.Credentials{Username: username, Password: password}
 }