Skip to content

Commit 147ba65

Browse files
mafredrimafredri
committed
fix: always set DOCKER_CONFIG unless manually set and no base64 config provided
Fixes #392
1 parent 08bdb8d commit 147ba65

File tree

2 files changed

+120
-33
lines changed

2 files changed

+120
-33
lines changed

‎envbuilder.go

+57-28
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,13 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro
154154

155155
opts.Logger(log.LevelInfo, "%s %s - Build development environments from repositories in a container", newColor(color.Bold).Sprintf("envbuilder"), buildinfo.Version())
156156

157-
cleanupDockerConfigJSON, err := initDockerConfigJSON(opts.Logger, workingDir, opts.DockerConfigBase64)
157+
cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Logger, workingDir, opts.DockerConfigBase64)
158158
if err != nil {
159159
return err
160160
}
161161
defer func() {
162-
if err := cleanupDockerConfigJSON(); err != nil {
163-
opts.Logger(log.LevelError, "failed to cleanup docker config JSON: %w", err)
162+
if err := cleanupDockerConfigOverride(); err != nil {
163+
opts.Logger(log.LevelError, "failed to cleanup docker config override: %w", err)
164164
}
165165
}() // best effort
166166

@@ -771,7 +771,7 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro
771771
}
772772

773773
// Remove the Docker config secret file!
774-
if err := cleanupDockerConfigJSON(); err != nil {
774+
if err := cleanupDockerConfigOverride(); err != nil {
775775
return err
776776
}
777777

@@ -978,13 +978,13 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error)
978978

979979
opts.Logger(log.LevelInfo, "%s %s - Build development environments from repositories in a container", newColor(color.Bold).Sprintf("envbuilder"), buildinfo.Version())
980980

981-
cleanupDockerConfigJSON, err := initDockerConfigJSON(opts.Logger, workingDir, opts.DockerConfigBase64)
981+
cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Logger, workingDir, opts.DockerConfigBase64)
982982
if err != nil {
983983
return nil, err
984984
}
985985
defer func() {
986-
if err := cleanupDockerConfigJSON(); err != nil {
987-
opts.Logger(log.LevelError, "failed to cleanup docker config JSON: %w", err)
986+
if err := cleanupDockerConfigOverride(); err != nil {
987+
opts.Logger(log.LevelError, "failed to cleanup docker config override: %w", err)
988988
}
989989
}() // best effort
990990

@@ -1315,7 +1315,7 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error)
13151315
options.UnsetEnv()
13161316

13171317
// Remove the Docker config secret file!
1318-
if err := cleanupDockerConfigJSON(); err != nil {
1318+
if err := cleanupDockerConfigOverride(); err != nil {
13191319
return nil, err
13201320
}
13211321

@@ -1627,55 +1627,84 @@ func parseMagicImageFile(fs billy.Filesystem, path string, v any) error {
16271627
return nil
16281628
}
16291629

1630-
func initDockerConfigJSON(logf log.Func, workingDir workingdir.WorkingDir, dockerConfigBase64 string) (func() error, error) {
1630+
func initDockerConfigOverride(logf log.Func, workingDir workingdir.WorkingDir, dockerConfigBase64 string) (func() error, error) {
16311631
var cleanupOnce sync.Once
1632-
noop := func() error { return nil }
1632+
1633+
var (
1634+
oldDockerConfig = os.Getenv("DOCKER_CONFIG")
1635+
newDockerConfig = workingDir.Path()
1636+
cfgPath = workingDir.Join("config.json")
1637+
restoreEnv = func() error { return nil } // noop.
1638+
)
1639+
if dockerConfigBase64 != "" || (dockerConfigBase64 == "" && oldDockerConfig == "") {
1640+
err := os.Setenv("DOCKER_CONFIG", newDockerConfig)
1641+
if err != nil {
1642+
logf(log.LevelError, "Failed to set DOCKER_CONFIG: %s", err)
1643+
return nil, fmt.Errorf("set DOCKER_CONFIG: %w", err)
1644+
}
1645+
logf(log.LevelInfo, "Set DOCKER_CONFIG to %s", newDockerConfig)
1646+
1647+
restoreEnv = func() error {
1648+
// Restore the old DOCKER_CONFIG value.
1649+
if oldDockerConfig == "" {
1650+
err := os.Unsetenv("DOCKER_CONFIG")
1651+
if err != nil {
1652+
err = fmt.Errorf("unset DOCKER_CONFIG: %w", err)
1653+
}
1654+
return err
1655+
}
1656+
err := os.Setenv("DOCKER_CONFIG", oldDockerConfig)
1657+
if err != nil {
1658+
return fmt.Errorf("restore DOCKER_CONFIG: %w", err)
1659+
}
1660+
logf(log.LevelInfo, "Restored DOCKER_CONFIG to %s", oldDockerConfig)
1661+
return nil
1662+
}
1663+
} else {
1664+
logf(log.LevelInfo, "Using existing DOCKER_CONFIG set to %s", oldDockerConfig)
1665+
}
1666+
16331667
if dockerConfigBase64 == "" {
1634-
return noop, nil
1668+
return restoreEnv, nil
16351669
}
1636-
cfgPath := workingDir.Join("config.json")
1670+
16371671
decoded, err := base64.StdEncoding.DecodeString(dockerConfigBase64)
16381672
if err != nil {
1639-
return noop, fmt.Errorf("decode docker config: %w", err)
1673+
return restoreEnv, fmt.Errorf("decode docker config: %w", err)
16401674
}
16411675
var configFile DockerConfig
16421676
decoded, err = hujson.Standardize(decoded)
16431677
if err != nil {
1644-
return noop, fmt.Errorf("humanize json for docker config: %w", err)
1678+
return restoreEnv, fmt.Errorf("humanize json for docker config: %w", err)
16451679
}
16461680
err = json.Unmarshal(decoded, &configFile)
16471681
if err != nil {
1648-
return noop, fmt.Errorf("parse docker config: %w", err)
1682+
return restoreEnv, fmt.Errorf("parse docker config: %w", err)
16491683
}
16501684
for k := range configFile.AuthConfigs {
16511685
logf(log.LevelInfo, "Docker config contains auth for registry %q", k)
16521686
}
16531687
err = os.WriteFile(cfgPath, decoded, 0o644)
16541688
if err != nil {
1655-
return noop, fmt.Errorf("write docker config: %w", err)
1689+
return restoreEnv, fmt.Errorf("write docker config: %w", err)
16561690
}
16571691
logf(log.LevelInfo, "Wrote Docker config JSON to %s", cfgPath)
1658-
oldDockerConfig := os.Getenv("DOCKER_CONFIG")
1659-
_ = os.Setenv("DOCKER_CONFIG", workingDir.Path())
1660-
newDockerConfig := os.Getenv("DOCKER_CONFIG")
1661-
logf(log.LevelInfo, "Set DOCKER_CONFIG to %s", newDockerConfig)
1662-
cleanup := func() error {
1692+
1693+
return func() error {
16631694
var cleanupErr error
16641695
cleanupOnce.Do(func() {
1665-
// Restore the old DOCKER_CONFIG value.
1666-
os.Setenv("DOCKER_CONFIG", oldDockerConfig)
1667-
logf(log.LevelInfo, "Restored DOCKER_CONFIG to %s", oldDockerConfig)
1696+
cleanupErr = restoreEnv()
16681697
// Remove the Docker config secret file!
1669-
if cleanupErr = os.Remove(cfgPath); err != nil {
1698+
if err := os.Remove(cfgPath); err != nil {
16701699
if !errors.Is(err, fs.ErrNotExist) {
1671-
cleanupErr = fmt.Errorf("remove docker config: %w", cleanupErr)
1700+
err = errors.Join(err, fmt.Errorf("remove docker config: %w", err))
16721701
}
16731702
logf(log.LevelError, "Failed to remove the Docker config secret file: %s", cleanupErr)
1703+
cleanupErr = errors.Join(cleanupErr, err)
16741704
}
16751705
})
16761706
return cleanupErr
1677-
}
1678-
return cleanup, err
1707+
}, nil
16791708
}
16801709

16811710
// Allows quick testing of layer caching using a local directory!

‎integration/integration_test.go

+63-5
Original file line numberDiff line numberDiff line change
@@ -552,13 +552,20 @@ func TestBuildFromDockerfile(t *testing.T) {
552552
"Dockerfile": "FROM " + testImageAlpine,
553553
},
554554
})
555-
ctr, err := runEnvbuilder(t, runOpts{env: []string{
556-
envbuilderEnv("GIT_URL", srv.URL),
557-
envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"),
558-
envbuilderEnv("DOCKER_CONFIG_BASE64", base64.StdEncoding.EncodeToString([]byte(`{"experimental": "enabled"}`))),
559-
}})
555+
logbuf := new(bytes.Buffer)
556+
ctr, err := runEnvbuilder(t, runOpts{
557+
env: []string{
558+
envbuilderEnv("GIT_URL", srv.URL),
559+
envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"),
560+
envbuilderEnv("DOCKER_CONFIG_BASE64", base64.StdEncoding.EncodeToString([]byte(`{"experimental": "enabled"}`))),
561+
"DOCKER_CONFIG=/config", // Ignored, because we're setting DOCKER_CONFIG_BASE64.
562+
},
563+
logbuf: logbuf,
564+
})
560565
require.NoError(t, err)
561566

567+
require.Contains(t, logbuf.String(), "Set DOCKER_CONFIG to /.envbuilder")
568+
562569
output := execContainer(t, ctr, "echo hello")
563570
require.Equal(t, "hello", strings.TrimSpace(output))
564571

@@ -568,6 +575,52 @@ func TestBuildFromDockerfile(t *testing.T) {
568575
require.Contains(t, output, "No such file or directory")
569576
}
570577

578+
func TestBuildDockerConfigPathFromEnv(t *testing.T) {
579+
// Ensures that a Git repository with a Dockerfile is cloned and built.
580+
srv := gittest.CreateGitServer(t, gittest.Options{
581+
Files: map[string]string{
582+
"Dockerfile": "FROM " + testImageAlpine,
583+
},
584+
})
585+
dir := t.TempDir()
586+
err := os.WriteFile(filepath.Join(dir, "config.json"), []byte(`{"experimental": "enabled"}`), 0o644)
587+
require.NoError(t, err)
588+
589+
logbuf := new(bytes.Buffer)
590+
_, err = runEnvbuilder(t, runOpts{
591+
env: []string{
592+
envbuilderEnv("GIT_URL", srv.URL),
593+
envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"),
594+
"DOCKER_CONFIG=/config",
595+
},
596+
binds: []string{fmt.Sprintf("%s:/config:ro", dir)},
597+
logbuf: logbuf,
598+
})
599+
require.NoError(t, err)
600+
601+
require.Contains(t, logbuf.String(), "Using existing DOCKER_CONFIG set to /config")
602+
}
603+
604+
func TestBuildDockerConfigDefaultPath(t *testing.T) {
605+
// Ensures that a Git repository with a Dockerfile is cloned and built.
606+
srv := gittest.CreateGitServer(t, gittest.Options{
607+
Files: map[string]string{
608+
"Dockerfile": "FROM " + testImageAlpine,
609+
},
610+
})
611+
logbuf := new(bytes.Buffer)
612+
_, err := runEnvbuilder(t, runOpts{
613+
env: []string{
614+
envbuilderEnv("GIT_URL", srv.URL),
615+
envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"),
616+
},
617+
logbuf: logbuf,
618+
})
619+
require.NoError(t, err)
620+
621+
require.Contains(t, logbuf.String(), "Set DOCKER_CONFIG to /.envbuilder")
622+
}
623+
571624
func TestBuildPrintBuildOutput(t *testing.T) {
572625
// Ensures that a Git repository with a Dockerfile is cloned and built.
573626
srv := gittest.CreateGitServer(t, gittest.Options{
@@ -2296,6 +2349,7 @@ type runOpts struct {
22962349
binds []string
22972350
env []string
22982351
volumes map[string]string
2352+
logbuf *bytes.Buffer
22992353
}
23002354

23012355
// runEnvbuilder starts the envbuilder container with the given environment
@@ -2340,6 +2394,7 @@ func runEnvbuilder(t *testing.T, opts runOpts) (string, error) {
23402394
testContainerLabel: "true",
23412395
},
23422396
}, &container.HostConfig{
2397+
CapAdd: []string{"SYS_ADMIN"}, // For remounting.
23432398
NetworkMode: container.NetworkMode("host"),
23442399
Binds: opts.binds,
23452400
Mounts: mounts,
@@ -2357,6 +2412,9 @@ func runEnvbuilder(t *testing.T, opts runOpts) (string, error) {
23572412
logChan, errChan := streamContainerLogs(t, cli, ctr.ID)
23582413
go func() {
23592414
for log := range logChan {
2415+
if opts.logbuf != nil {
2416+
opts.logbuf.WriteString(log)
2417+
}
23602418
if strings.HasPrefix(log, "=== Running init command") {
23612419
errChan <- nil
23622420
return

0 commit comments

Comments
 (0)