Lift configfile (#757)

* Don't store the RegistryPuller's StorageClient

There's no point in storing it, as it's never reused.

* Lift configfile.ConfigFile out of CredentialsStore

Rather than assuming that users of CredentialStore will know to call Init
after instantiation, the CredentialStore's dependency on a Docker *ConfigFile
is made explicit by requiring it to be passed to the constructor. Now we need
never worry that a user will forget to call Init, and it removes the awkward
pattern of New-then-Init.

pkg/artifacts/registry.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"path/filepath"
 
+	"github.com/docker/cli/cli/config"
 	"github.com/go-logr/logr"
 
 	"github.com/aws/eks-anywhere-packages/pkg/registry"
@@ -16,8 +17,7 @@ var certFile = filepath.Join(configPath, "ca.crt")
 // RegistryPuller handles pulling OCI artifacts from an OCI registry
 // (i.e. bundles)
 type RegistryPuller struct {
-	storageClient registry.StorageClient
-	log           logr.Logger
+	log logr.Logger
 }
 
 var _ Puller = (*RegistryPuller)(nil)
@@ -40,19 +40,18 @@ func (p *RegistryPuller) Pull(ctx context.Context, ref string) ([]byte, error) {
 		p.log.Error(err, "problem getting certificate file", "filename", certFile)
 	}
 
-	credentialStore := registry.NewCredentialStore()
-	credentialStore.SetDirectory(configPath)
-	err = credentialStore.Init()
+	configFile, err := config.Load("")
 	if err != nil {
 		return nil, err
 	}
+	store := registry.NewDockerCredentialStore(configFile)
 
-	sc := registry.NewStorageContext(art.Registry, credentialStore, certificates, false)
-	p.storageClient = registry.NewOCIRegistry(sc)
-	err = p.storageClient.Init()
+	sc := registry.NewStorageContext(art.Registry, store, certificates, false)
+	client := registry.NewOCIRegistry(sc)
+	err = client.Init()
 	if err != nil {
 		return nil, err
 	}
 
-	return registry.PullBytes(ctx, p.storageClient, *art)
+	return registry.PullBytes(ctx, client, *art)
 }

pkg/registry/client_test.go

@@ -5,7 +5,9 @@ import (
 	"crypto/x509"
 	"testing"
 
+	"github.com/docker/cli/cli/config"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/aws/eks-anywhere-packages/pkg/registry"
 )
@@ -18,13 +20,11 @@ var (
 		Digest:     "sha256:6efe21500abbfbb6b3e37b80dd5dea0b11a0d1b145e84298fee5d7784a77e967",
 		Tag:        "0.2.22-eks-a-24",
 	}
-	credentialStore = registry.NewCredentialStore()
-	certificates    = &x509.CertPool{}
-	registryContext = registry.NewStorageContext("localhost", credentialStore, certificates, false)
+	certificates = &x509.CertPool{}
 )
 
 func TestOCIRegistryClient_Init(t *testing.T) {
-	sut := registry.NewOCIRegistry(registryContext)
+	sut := registry.NewOCIRegistry(newStorageContext(t, ""))
 
 	err := sut.Init()
 	assert.NoError(t, err)
@@ -35,7 +35,7 @@ func TestOCIRegistryClient_Init(t *testing.T) {
 }
 
 func TestOCIRegistryClient_Destination(t *testing.T) {
-	sut := registry.NewOCIRegistry(registryContext)
+	sut := registry.NewOCIRegistry(newStorageContext(t, ""))
 	destination := sut.Destination(image)
 	assert.Equal(t, "localhost/eks-anywhere/eks-anywhere-packages@sha256:6efe21500abbfbb6b3e37b80dd5dea0b11a0d1b145e84298fee5d7784a77e967", destination)
 	sut.SetProject("project/")
@@ -44,7 +44,7 @@ func TestOCIRegistryClient_Destination(t *testing.T) {
 }
 
 func TestOCIRegistryClient_GetStorage(t *testing.T) {
-	sut := registry.NewOCIRegistry(registryContext)
+	sut := registry.NewOCIRegistry(newStorageContext(t, ""))
 	assert.NoError(t, sut.Init())
 	_, err := sut.GetStorage(context.Background(), image)
 	assert.NoError(t, err)
@@ -57,3 +57,10 @@ func TestOCIRegistryClient_GetStorage(t *testing.T) {
 	_, err = sut.GetStorage(context.Background(), bogusImage)
 	assert.EqualError(t, err, "error creating repository !@#$: invalid reference: invalid repository")
 }
+
+func newStorageContext(t *testing.T, dir string) registry.StorageContext {
+	configFile, err := config.Load(dir)
+	require.NoError(t, err)
+	store := registry.NewDockerCredentialStore(configFile)
+	return registry.NewStorageContext("localhost", store, certificates, false)
+}

pkg/registry/credentials.go

@@ -1,44 +1,28 @@
 package registry
 
 import (
-	"github.com/docker/cli/cli/config"
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/docker/cli/cli/config/credentials"
 	"oras.land/oras-go/v2/registry/remote/auth"
 )
 
-// CredentialStore for registry credentials such as ~/.docker/config.json.
-type CredentialStore struct {
-	directory  string
+// DockerCredentialStore for Docker registry credentials, like ~/.docker/config.json.
+type DockerCredentialStore struct {
 	configFile *configfile.ConfigFile
 }
 
-// NewCredentialStore create a credential store.
-func NewCredentialStore() *CredentialStore {
-	return &CredentialStore{
-		directory: config.Dir(),
+// NewDockerCredentialStore creates a DockerCredentialStore.
+func NewDockerCredentialStore(configFile *configfile.ConfigFile) *DockerCredentialStore {
+	if !configFile.ContainsAuth() {
+		configFile.CredentialsStore = credentials.DetectDefaultStore(configFile.CredentialsStore)
 	}
-}
-
-// SetDirectory override default directory.
-func (cs *CredentialStore) SetDirectory(directory string) {
-	cs.directory = directory
-}
-
-// Init initialize a credential store.
-func (cs *CredentialStore) Init() (err error) {
-	cs.configFile, err = config.Load(cs.directory)
-	if err != nil {
-		return err
-	}
-	if !cs.configFile.ContainsAuth() {
-		cs.configFile.CredentialsStore = credentials.DetectDefaultStore(cs.configFile.CredentialsStore)
+	return &DockerCredentialStore{
+		configFile: configFile,
 	}
-	return nil
 }
 
 // Credential get an authentication credential for a given registry.
-func (cs *CredentialStore) Credential(registry string) (auth.Credential, error) {
+func (cs *DockerCredentialStore) Credential(registry string) (auth.Credential, error) {
 	authConf, err := cs.configFile.GetCredentialsStore(registry).Get(registry)
 	if err != nil {
 		return auth.EmptyCredential, err

pkg/registry/credentials_test.go

@@ -3,53 +3,54 @@ package registry_test
 import (
 	"testing"
 
+	"github.com/docker/cli/cli/config"
+	"github.com/docker/cli/cli/config/configfile"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"oras.land/oras-go/v2/registry/remote/auth"
 
 	"github.com/aws/eks-anywhere-packages/pkg/registry"
 )
 
-func TestCredentialStore_Init(t *testing.T) {
-	credentialStore := registry.NewCredentialStore()
-	credentialStore.SetDirectory("testdata")
-
-	err := credentialStore.Init()
-	assert.NoError(t, err)
+func TestDockerCredentialStore(t *testing.T) {
+	configFile := newConfigFile(t, "testdata")
+	credentialStore := registry.NewDockerCredentialStore(configFile)
 
 	result, err := credentialStore.Credential("localhost")
-	assert.NoError(t, err)
-	assert.Equal(t, "user", result.Username)
-	assert.Equal(t, "pass", result.Password)
-	assert.Equal(t, "", result.AccessToken)
-	assert.Equal(t, "", result.RefreshToken)
+	require.NoError(t, err)
+	assertAuthEqual(t, auth.Credential{Username: "user", Password: "pass"}, result)
 
 	result, err = credentialStore.Credential("harbor.eksa.demo:30003")
-	assert.NoError(t, err)
-	assert.Equal(t, "captain", result.Username)
-	assert.Equal(t, "haddock", result.Password)
-	assert.Equal(t, "", result.AccessToken)
-	assert.Equal(t, "", result.RefreshToken)
+	require.NoError(t, err)
+	assertAuthEqual(t, auth.Credential{Username: "captain", Password: "haddock"}, result)
 
 	result, err = credentialStore.Credential("bogus")
-	assert.NoError(t, err)
-	assert.Equal(t, "", result.Username)
-	assert.Equal(t, "", result.Password)
-	assert.Equal(t, "", result.AccessToken)
-	assert.Equal(t, "", result.RefreshToken)
+	require.NoError(t, err)
+	assertAuthEqual(t, auth.EmptyCredential, result)
 
 	result, err = credentialStore.Credential("5551212.dkr.ecr.us-west-2.amazonaws.com")
 	// This is a generic error, so using errors.Is won't work, and this is as
 	// much of the string as we can reliably match against in a cross-platform
 	// fashion. Until they change it, then everything will break.
-	assert.ErrorContains(t, err, "error getting credentials - err")
-	assert.Equal(t, "", result.Username)
-	assert.Equal(t, "", result.Password)
-	assert.Equal(t, "", result.AccessToken)
-	assert.Equal(t, "", result.RefreshToken)
+	require.ErrorContains(t, err, "error getting credentials - err")
+	assertAuthEqual(t, auth.EmptyCredential, result)
 }
 
 func TestCredentialStore_InitEmpty(t *testing.T) {
-	credentialStore := registry.NewCredentialStore()
-	credentialStore.SetDirectory("testdata/empty")
-	err := credentialStore.Init()
-	assert.NoError(t, err)
+	registry.NewDockerCredentialStore(newConfigFile(t, "testdata/empty"))
+}
+
+func newConfigFile(t *testing.T, dir string) *configfile.ConfigFile {
+	t.Helper()
+	configFile, err := config.Load(dir)
+	require.NoError(t, err)
+	return configFile
+}
+
+func assertAuthEqual(t *testing.T, expected, got auth.Credential) {
+	t.Helper()
+	assert.Equal(t, expected.Username, got.Username)
+	assert.Equal(t, expected.Password, got.Password)
+	assert.Equal(t, expected.AccessToken, got.AccessToken)
+	assert.Equal(t, expected.RefreshToken, got.RefreshToken)
 }

pkg/registry/storage.go

@@ -12,13 +12,13 @@ import (
 type StorageContext struct {
 	host            string
 	project         string
-	credentialStore *CredentialStore
+	credentialStore *DockerCredentialStore
 	certificates    *x509.CertPool
 	insecure        bool
 }
 
 // NewStorageContext create registry context.
-func NewStorageContext(host string, credentialStore *CredentialStore, certificates *x509.CertPool, insecure bool) StorageContext {
+func NewStorageContext(host string, credentialStore *DockerCredentialStore, certificates *x509.CertPool, insecure bool) StorageContext {
 	return StorageContext{
 		host:            host,
 		credentialStore: credentialStore,