Skip to content

Commit 4cccbd6

Browse files
dgageotdgageot
authored
Pass a context to DefaultAuthHelper.GetAllConfigs() (#4760)
* Pass a context to DefaultAuthHelper.GetAllConfigs() Signed-off-by: David Gageot <[email protected]> * Add tests Signed-off-by: David Gageot <[email protected]>
1 parent 36284dd commit 4cccbd6

File tree

5 files changed

+82
-28
lines changed

5 files changed

+82
-28
lines changed

‎pkg/skaffold/build/cache/retrieve_test.go

+6-2
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,12 @@ func (b *mockBuilder) BuildAndTest(ctx context.Context, out io.Writer, tags tag.
8585

8686
type stubAuth struct{}
8787

88-
func (t stubAuth) GetAuthConfig(string) (types.AuthConfig, error) { return types.AuthConfig{}, nil }
89-
func (t stubAuth) GetAllAuthConfigs() (map[string]types.AuthConfig, error) { return nil, nil }
88+
func (t stubAuth) GetAuthConfig(string) (types.AuthConfig, error) {
89+
return types.AuthConfig{}, nil
90+
}
91+
func (t stubAuth) GetAllAuthConfigs(context.Context) (map[string]types.AuthConfig, error) {
92+
return nil, nil
93+
}
9094

9195
func TestCacheBuildLocal(t *testing.T) {
9296
testutil.Run(t, "", func(t *testutil.T) {

‎pkg/skaffold/build/local/local_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@ type testAuthHelper struct{}
4343
func (t testAuthHelper) GetAuthConfig(string) (types.AuthConfig, error) {
4444
return types.AuthConfig{}, nil
4545
}
46-
func (t testAuthHelper) GetAllAuthConfigs() (map[string]types.AuthConfig, error) { return nil, nil }
46+
func (t testAuthHelper) GetAllAuthConfigs(context.Context) (map[string]types.AuthConfig, error) {
47+
return nil, nil
48+
}
4749

4850
func TestLocalRun(t *testing.T) {
4951
tests := []struct {

‎pkg/skaffold/docker/auth.go

+26-3
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func init() {
5757
// Ideally this shouldn't be public, but the LocalBuilder needs to use it.
5858
type AuthConfigHelper interface {
5959
GetAuthConfig(registry string) (types.AuthConfig, error)
60-
GetAllAuthConfigs() (map[string]types.AuthConfig, error)
60+
GetAllAuthConfigs(ctx context.Context) (map[string]types.AuthConfig, error)
6161
}
6262

6363
type credsHelper struct{}
@@ -73,7 +73,7 @@ func loadDockerConfig() (*configfile.ConfigFile, error) {
7373
return cf, nil
7474
}
7575

76-
func (credsHelper) GetAuthConfig(registry string) (types.AuthConfig, error) {
76+
func (h credsHelper) GetAuthConfig(registry string) (types.AuthConfig, error) {
7777
cf, err := loadDockerConfig()
7878
if err != nil {
7979
return types.AuthConfig{}, err
@@ -87,7 +87,30 @@ func (credsHelper) GetAuthConfig(registry string) (types.AuthConfig, error) {
8787
return types.AuthConfig(auth), nil
8888
}
8989

90-
func (credsHelper) GetAllAuthConfigs() (map[string]types.AuthConfig, error) {
90+
// GetAllAuthConfigs retrieves all the auth configs.
91+
// Because this can take a long time, we make sure it can be interrupted by the user.
92+
func (h credsHelper) GetAllAuthConfigs(ctx context.Context) (map[string]types.AuthConfig, error) {
93+
type result struct {
94+
configs map[string]types.AuthConfig
95+
err error
96+
}
97+
98+
auth := make(chan result)
99+
100+
go func() {
101+
configs, err := h.doGetAllAuthConfigs()
102+
auth <- result{configs, err}
103+
}()
104+
105+
select {
106+
case <-ctx.Done():
107+
return nil, ctx.Err()
108+
case r := <-auth:
109+
return r.configs, r.err
110+
}
111+
}
112+
113+
func (h credsHelper) doGetAllAuthConfigs() (map[string]types.AuthConfig, error) {
91114
cf, err := loadDockerConfig()
92115
if err != nil {
93116
return nil, err

‎pkg/skaffold/docker/auth_test.go

+44-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package docker
1919
import (
2020
"context"
2121
"fmt"
22+
"runtime"
2223
"testing"
2324

2425
"github.com/docker/docker/api/types"
@@ -44,11 +45,52 @@ var allAuthConfig = map[string]types.AuthConfig{
4445
func (t testAuthHelper) GetAuthConfig(string) (types.AuthConfig, error) {
4546
return gcrAuthConfig, t.getAuthConfigErr
4647
}
47-
48-
func (t testAuthHelper) GetAllAuthConfigs() (map[string]types.AuthConfig, error) {
48+
func (t testAuthHelper) GetAllAuthConfigs(context.Context) (map[string]types.AuthConfig, error) {
4949
return allAuthConfig, t.getAllAuthConfigsErr
5050
}
5151

52+
func TestGetAllAuthConfigs(t *testing.T) {
53+
testutil.Run(t, "auto-configure gcr.io", func(t *testutil.T) {
54+
if runtime.GOOS == "windows" {
55+
t.Skip("test doesn't work on windows")
56+
}
57+
58+
tmpDir := t.NewTempDir().
59+
Write("config.json", `{"credHelpers":{"my.registry":"helper"}}`).
60+
Write("docker-credential-gcloud", `#!/bin/sh
61+
read server
62+
echo "{\"Username\":\"<token>\",\"Secret\":\"TOKEN_$server\"}"`).
63+
Write("docker-credential-helper", `#!/bin/sh
64+
read server
65+
echo "{\"Username\":\"<token>\",\"Secret\":\"TOKEN_$server\"}"`)
66+
t.Override(&configDir, tmpDir.Root())
67+
t.SetEnvs(map[string]string{"PATH": tmpDir.Root()})
68+
69+
auth, err := DefaultAuthHelper.GetAllAuthConfigs(context.Background())
70+
71+
t.CheckNoError(err)
72+
t.CheckDeepEqual(map[string]types.AuthConfig{
73+
"asia.gcr.io": {IdentityToken: "TOKEN_asia.gcr.io"},
74+
"eu.gcr.io": {IdentityToken: "TOKEN_eu.gcr.io"},
75+
"gcr.io": {IdentityToken: "TOKEN_gcr.io"},
76+
"my.registry": {IdentityToken: "TOKEN_my.registry"},
77+
"marketplace.gcr.io": {IdentityToken: "TOKEN_marketplace.gcr.io"},
78+
"staging-k8s.gcr.io": {IdentityToken: "TOKEN_staging-k8s.gcr.io"},
79+
"us.gcr.io": {IdentityToken: "TOKEN_us.gcr.io"},
80+
}, auth)
81+
})
82+
83+
testutil.Run(t, "invalid config.json", func(t *testutil.T) {
84+
tmpDir := t.NewTempDir().Write("config.json", "invalid json")
85+
t.Override(&configDir, tmpDir.Root())
86+
87+
auth, err := DefaultAuthHelper.GetAllAuthConfigs(context.Background())
88+
89+
t.CheckError(true, err)
90+
t.CheckEmpty(auth)
91+
})
92+
}
93+
5294
func TestGetEncodedRegistryAuth(t *testing.T) {
5395
tests := []struct {
5496
description string

‎pkg/skaffold/docker/image.go

+3-20
Original file line numberDiff line numberDiff line change
@@ -156,25 +156,6 @@ func (l *localDaemon) ConfigFile(ctx context.Context, image string) (*v1.ConfigF
156156
return cfg, nil
157157
}
158158

159-
func authConfig(ctx context.Context) map[string]types.AuthConfig {
160-
auth := make(chan map[string]types.AuthConfig)
161-
162-
go func() {
163-
// Like `docker build`, we ignore the errors
164-
// See https://github.com/docker/cli/blob/75c1bb1f33d7cedbaf48404597d5bf9818199480/cli/command/image/build.go#L364
165-
authConfigs, _ := DefaultAuthHelper.GetAllAuthConfigs()
166-
auth <- authConfigs
167-
}()
168-
169-
// Because this can take a long time, we make sure it can be interrupted by the user.
170-
select {
171-
case <-ctx.Done():
172-
return nil
173-
case r := <-auth:
174-
return r
175-
}
176-
}
177-
178159
// Build performs a docker build and returns the imageID.
179160
func (l *localDaemon) Build(ctx context.Context, out io.Writer, workspace string, a *latest.DockerArtifact, ref string, mode config.RunMode) (string, error) {
180161
logrus.Debugf("Running docker build: context: %s, dockerfile: %s", workspace, a.DockerfilePath)
@@ -184,7 +165,9 @@ func (l *localDaemon) Build(ctx context.Context, out io.Writer, workspace string
184165
return "", fmt.Errorf("unable to evaluate build args: %w", err)
185166
}
186167

187-
authConfigs := authConfig(ctx)
168+
// Like `docker build`, we ignore the errors
169+
// See https://github.com/docker/cli/blob/75c1bb1f33d7cedbaf48404597d5bf9818199480/cli/command/image/build.go#L364
170+
authConfigs, _ := DefaultAuthHelper.GetAllAuthConfigs(ctx)
188171

189172
buildCtx, buildCtxWriter := io.Pipe()
190173
go func() {

0 commit comments

Comments
 (0)