From fd7a3f23d2205aef0fccb31f04493f8744390920 Mon Sep 17 00:00:00 2001 From: Vanessasaurus <814322+vsoch@users.noreply.github.com> Date: Tue, 21 Jun 2022 08:44:45 -0600 Subject: [PATCH] feat: add DOCKER_HOST environment (#875) * start of work to add DOCKER_HOST environment this seems to be the basic to add support for a custom docker host as an envar or command line option. Some questions I have: 1. how/where to test 2. should the default value be set to something instead of empty string? Signed-off-by: vsoch * adding support for bare DOCKER_ envars, and DOCKER_HOST tests Signed-off-by: vsoch * trying to fix linting Signed-off-by: vsoch * more linting interesting I did not see this output locally - I must have a different development environment than what is run in the tests! Signed-off-by: vsoch * fixes from Dave review! Signed-off-by: vsoch * fix parsing of docker host flag value Signed-off-by: vsoch * typo in docker test and we do not need custom lookup for docker host flag Signed-off-by: vsoch * undefined out Signed-off-by: vsoch * simplify build command Signed-off-by: vsoch * simplify build command Signed-off-by: vsoch * try to fix docker build - scratch adding an empty file Signed-off-by: vsoch * attempting to fix tests Signed-off-by: vsoch * try triggering ci again? Signed-off-by: vsoch Co-authored-by: vsoch --- CHANGELOG.md | 2 + cmd/internal/cli/action_flags.go | 1 + cmd/internal/cli/build.go | 1 + cmd/internal/cli/build_linux.go | 1 + cmd/internal/cli/pull.go | 1 + cmd/internal/cli/push.go | 1 + cmd/internal/cli/singularity.go | 42 ++++-- e2e/docker/docker.go | 135 ++++++++++++++++++ .../pkg/build/sources/conveyorPacker_oci.go | 2 + pkg/build/types/bundle.go | 2 + pkg/cmdline/flag.go | 22 ++- pkg/sylog/sylog.go | 2 +- 12 files changed, 196 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1895177110..9c0cc1b9f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ specified instance, running within a cgroup. - Add `--sparse` flag to `overlay create` command to allow generation of a sparse ext3 overlay image. +- Support for `DOCKER_HOST` parsing when using `docker-daemon://` +- `DOCKER_USERNAME` and `DOCKER_PASSWORD` supported without `SINGULARITY_` prefix. - The `--no-mount` flag now accepts the value `bind-paths` to disable mounting of all `bind path` entries in `singularity.conf. diff --git a/cmd/internal/cli/action_flags.go b/cmd/internal/cli/action_flags.go index f4fcbca8e0..514ec4c969 100644 --- a/cmd/internal/cli/action_flags.go +++ b/cmd/internal/cli/action_flags.go @@ -861,6 +861,7 @@ func init() { cmdManager.RegisterFlagForCmd(&commonNoHTTPSFlag, actionsInstanceCmd...) cmdManager.RegisterFlagForCmd(&commonOldNoHTTPSFlag, actionsInstanceCmd...) cmdManager.RegisterFlagForCmd(&dockerLoginFlag, actionsInstanceCmd...) + cmdManager.RegisterFlagForCmd(&dockerHostFlag, actionsInstanceCmd...) cmdManager.RegisterFlagForCmd(&dockerPasswordFlag, actionsInstanceCmd...) cmdManager.RegisterFlagForCmd(&dockerUsernameFlag, actionsInstanceCmd...) cmdManager.RegisterFlagForCmd(&actionEnvFlag, actionsInstanceCmd...) diff --git a/cmd/internal/cli/build.go b/cmd/internal/cli/build.go index 6387e852aa..1db1fadeec 100644 --- a/cmd/internal/cli/build.go +++ b/cmd/internal/cli/build.go @@ -300,6 +300,7 @@ func init() { cmdManager.RegisterFlagForCmd(&commonNoHTTPSFlag, buildCmd) cmdManager.RegisterFlagForCmd(&commonTmpDirFlag, buildCmd) + cmdManager.RegisterFlagForCmd(&dockerHostFlag, buildCmd) cmdManager.RegisterFlagForCmd(&dockerUsernameFlag, buildCmd) cmdManager.RegisterFlagForCmd(&dockerPasswordFlag, buildCmd) cmdManager.RegisterFlagForCmd(&dockerLoginFlag, buildCmd) diff --git a/cmd/internal/cli/build_linux.go b/cmd/internal/cli/build_linux.go index 10a287e2d8..b42d95a81b 100644 --- a/cmd/internal/cli/build_linux.go +++ b/cmd/internal/cli/build_linux.go @@ -393,6 +393,7 @@ func runBuildLocal(ctx context.Context, cmd *cobra.Command, dst, spec string) { LibraryAuthToken: authToken, KeyServerOpts: ko, DockerAuthConfig: authConf, + DockerDaemonHost: dockerHost, EncryptionKeyInfo: keyInfo, FixPerms: buildArgs.fixPerms, SandboxTarget: sandboxTarget, diff --git a/cmd/internal/cli/pull.go b/cmd/internal/cli/pull.go index e80d57b041..03df4a967f 100644 --- a/cmd/internal/cli/pull.go +++ b/cmd/internal/cli/pull.go @@ -141,6 +141,7 @@ func init() { cmdManager.RegisterFlagForCmd(&pullDisableCacheFlag, PullCmd) cmdManager.RegisterFlagForCmd(&pullDirFlag, PullCmd) + cmdManager.RegisterFlagForCmd(&dockerHostFlag, PullCmd) cmdManager.RegisterFlagForCmd(&dockerUsernameFlag, PullCmd) cmdManager.RegisterFlagForCmd(&dockerPasswordFlag, PullCmd) cmdManager.RegisterFlagForCmd(&dockerLoginFlag, PullCmd) diff --git a/cmd/internal/cli/push.go b/cmd/internal/cli/push.go index fcee671805..70f1be8bfe 100644 --- a/cmd/internal/cli/push.go +++ b/cmd/internal/cli/push.go @@ -69,6 +69,7 @@ func init() { cmdManager.RegisterFlagForCmd(&pushAllowUnsignedFlag, PushCmd) cmdManager.RegisterFlagForCmd(&pushDescriptionFlag, PushCmd) + cmdManager.RegisterFlagForCmd(&dockerHostFlag, PushCmd) cmdManager.RegisterFlagForCmd(&dockerUsernameFlag, PushCmd) cmdManager.RegisterFlagForCmd(&dockerPasswordFlag, PushCmd) }) diff --git a/cmd/internal/cli/singularity.go b/cmd/internal/cli/singularity.go index 90826104b8..20fdf63f7a 100644 --- a/cmd/internal/cli/singularity.go +++ b/cmd/internal/cli/singularity.go @@ -50,6 +50,7 @@ var currentRemoteEndpoint *endpoint.Config var ( dockerAuthConfig ocitypes.DockerAuthConfig dockerLogin bool + dockerHost string encryptionPEMPath string promptForPassphrase bool @@ -124,24 +125,26 @@ var singVerboseFlag = cmdline.Flag{ // --docker-username var dockerUsernameFlag = cmdline.Flag{ - ID: "dockerUsernameFlag", - Value: &dockerAuthConfig.Username, - DefaultValue: "", - Name: "docker-username", - Usage: "specify a username for docker authentication", - Hidden: true, - EnvKeys: []string{"DOCKER_USERNAME"}, + ID: "dockerUsernameFlag", + Value: &dockerAuthConfig.Username, + DefaultValue: "", + Name: "docker-username", + Usage: "specify a username for docker authentication", + Hidden: true, + EnvKeys: []string{"DOCKER_USERNAME"}, + WithoutPrefix: true, } // --docker-password var dockerPasswordFlag = cmdline.Flag{ - ID: "dockerPasswordFlag", - Value: &dockerAuthConfig.Password, - DefaultValue: "", - Name: "docker-password", - Usage: "specify a password for docker authentication", - Hidden: true, - EnvKeys: []string{"DOCKER_PASSWORD"}, + ID: "dockerPasswordFlag", + Value: &dockerAuthConfig.Password, + DefaultValue: "", + Name: "docker-password", + Usage: "specify a password for docker authentication", + Hidden: true, + EnvKeys: []string{"DOCKER_PASSWORD"}, + WithoutPrefix: true, } // --docker-login @@ -154,6 +157,17 @@ var dockerLoginFlag = cmdline.Flag{ EnvKeys: []string{"DOCKER_LOGIN"}, } +// --docker-host +var dockerHostFlag = cmdline.Flag{ + ID: "dockerHostFlag", + Value: &dockerHost, + DefaultValue: "", + Name: "docker-host", + Usage: "specify a custom Docker daemon host", + EnvKeys: []string{"DOCKER_HOST"}, + WithoutPrefix: true, +} + // --passphrase var commonPromptForPassphraseFlag = cmdline.Flag{ ID: "commonPromptForPassphraseFlag", diff --git a/e2e/docker/docker.go b/e2e/docker/docker.go index fb126f7511..b74d8bd5b8 100644 --- a/e2e/docker/docker.go +++ b/e2e/docker/docker.go @@ -8,10 +8,12 @@ package docker import ( "fmt" "os" + "os/exec" "path/filepath" "strings" "testing" + dockerclient "github.com/docker/docker/client" "github.com/pkg/errors" "github.com/sylabs/singularity/e2e/internal/e2e" "github.com/sylabs/singularity/e2e/internal/testhelper" @@ -118,6 +120,138 @@ func (c ctx) testDockerPulls(t *testing.T) { } } +// Testing DOCKER_ host support (only if docker available) +func (c ctx) testDockerHost(t *testing.T) { + require.Command(t, "docker") + + // Write a temporary "empty" Dockerfile (from scratch) + tmpPath, err := fs.MakeTmpDir(c.env.TestDir, "docker-", 0o755) + err = errors.Wrapf(err, "creating temporary directory in %q for docker host test", c.env.TestDir) + if err != nil { + t.Fatalf("failed to create temporary directory: %+v", err) + } + defer os.RemoveAll(tmpPath) + + // Here is the Dockerfile, and a temporary image path + dockerfile := filepath.Join(tmpPath, "Dockerfile") + emptyfile := filepath.Join(tmpPath, "file") + tmpImage := filepath.Join(tmpPath, "scratch-tmp.sif") + + // Write Dockerfile and empty file to file + dockerfileContent := []byte("FROM scratch\nCOPY file /mrbigglesworth") + err = os.WriteFile(dockerfile, dockerfileContent, 0o644) + if err != nil { + t.Fatalf("failed to create temporary Dockerfile: %+v", err) + } + + fileContent := []byte("") + err = os.WriteFile(emptyfile, fileContent, 0o644) + if err != nil { + t.Fatalf("failed to create empty file: %+v", err) + } + + dockerURI := "dinosaur/test-image:latest" + pullURI := "docker-daemon:" + dockerURI + + // Invoke docker build to build an empty scratch image in the docker daemon. + // Use os/exec because easier to generate a command with a working directory + e2e.Privileged(func(t *testing.T) { + cmd := exec.Command("docker", "build", "-t", dockerURI, ".") + cmd.Dir = tmpPath + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("Unexpected error while running command.\n%s: %s", err, string(out)) + } + }) + + tests := []struct { + name string + envarName string + envarValue string + exit int + }{ + // Unset docker host should use default and succeed + { + name: "singularityDockerHostEmpty", + envarName: "SINGULARITY_DOCKER_HOST", + envarValue: "", + exit: 0, + }, + { + name: "dockerHostEmpty", + envarName: "DOCKER_HOST", + envarValue: "", + exit: 0, + }, + + // bad Docker host should fail + { + name: "singularityDockerHostInvalid", + envarName: "SINGULARITY_DOCKER_HOST", + envarValue: "tcp://192.168.59.103:oops", + exit: 255, + }, + { + name: "dockerHostInvalid", + envarName: "DOCKER_HOST", + envarValue: "tcp://192.168.59.103:oops", + exit: 255, + }, + + // Set to default should succeed + // The default host varies based on OS, so we use dockerclient default + { + name: "singularityDockerHostValid", + envarName: "SINGULARITY_DOCKER_HOST", + envarValue: dockerclient.DefaultDockerHost, + exit: 0, + }, + { + name: "dockerHostValid", + envarName: "DOCKER_HOST", + envarValue: dockerclient.DefaultDockerHost, + exit: 0, + }, + } + + for _, tt := range tests { + // Export variable to environment if it's defined + if tt.envarValue != "" { + e2e.Privileged(func(t *testing.T) { + c.env.RunSingularity( + t, + e2e.WithEnv(append(os.Environ(), tt.envarValue)), + e2e.AsSubtest(tt.name), + e2e.WithProfile(e2e.UserProfile), + e2e.WithCommand("pull"), + e2e.WithArgs(tmpImage, pullURI), + e2e.ExpectExit(tt.exit), + ) + }) + } else { + e2e.Privileged(func(t *testing.T) { + c.env.RunSingularity( + t, + e2e.AsSubtest(tt.name), + e2e.WithProfile(e2e.UserProfile), + e2e.WithCommand("pull"), + e2e.WithArgs(tmpImage, pullURI), + e2e.ExpectExit(tt.exit), + ) + }) + } + } + + // Clean up docker image + e2e.Privileged(func(t *testing.T) { + cmd := exec.Command("docker", "rmi", dockerURI) + _, err = cmd.Output() + if err != nil { + t.Fatalf("Unexpected error while cleaning up docker image.\n%s", err) + } + }) +} + // AUFS sanity tests func (c ctx) testDockerAUFS(t *testing.T) { imageDir, cleanup := e2e.MakeTempDir(t, c.env.TestDir, "aufs-", "") @@ -731,6 +865,7 @@ func E2ETests(env e2e.TestEnv) testhelper.Tests { return testhelper.Tests{ "AUFS": c.testDockerAUFS, "def file": c.testDockerDefFile, + "docker host": c.testDockerHost, "permissions": c.testDockerPermissions, "pulls": c.testDockerPulls, "registry": c.testDockerRegistry, diff --git a/internal/pkg/build/sources/conveyorPacker_oci.go b/internal/pkg/build/sources/conveyorPacker_oci.go index 3453721871..d81ce20da2 100644 --- a/internal/pkg/build/sources/conveyorPacker_oci.go +++ b/internal/pkg/build/sources/conveyorPacker_oci.go @@ -144,11 +144,13 @@ func (cp *OCIConveyorPacker) Get(ctx context.Context, b *sytypes.Bundle) (err er cp.sysCtx = &types.SystemContext{ OCIInsecureSkipTLSVerify: cp.b.Opts.NoHTTPS, DockerAuthConfig: cp.b.Opts.DockerAuthConfig, + DockerDaemonHost: cp.b.Opts.DockerDaemonHost, OSChoice: "linux", AuthFilePath: syfs.DockerConf(), DockerRegistryUserAgent: useragent.Value(), BigFilesTemporaryDir: b.TmpDir, } + if cp.b.Opts.NoHTTPS { cp.sysCtx.DockerInsecureSkipTLSVerify = types.NewOptionalBool(true) } diff --git a/pkg/build/types/bundle.go b/pkg/build/types/bundle.go index a39c97b21a..985210ff74 100644 --- a/pkg/build/types/bundle.go +++ b/pkg/build/types/bundle.go @@ -49,6 +49,8 @@ type Options struct { KeyServerOpts []scskeyclient.Option // contains docker credentials if specified. DockerAuthConfig *ocitypes.DockerAuthConfig + // Custom docker Daemon host + DockerDaemonHost string // EncryptionKeyInfo specifies the key used for filesystem // encryption if applicable. // A nil value indicates encryption should not occur. diff --git a/pkg/cmdline/flag.go b/pkg/cmdline/flag.go index ca255075cf..6b2deed36b 100644 --- a/pkg/cmdline/flag.go +++ b/pkg/cmdline/flag.go @@ -27,6 +27,8 @@ type Flag struct { Required bool EnvKeys []string EnvHandler EnvHandler + // Export envar also without prefix + WithoutPrefix bool // When Value is a []String: // If true, will use pFlag StringArrayVar(P) type, where values are not split on comma. // If false, will use pFlag StringSliceVar(P) type, where a single value is split on commas. @@ -52,6 +54,11 @@ func (m *flagManager) setFlagOptions(flag *Flag, cmd *cobra.Command) { if len(flag.EnvKeys) > 0 { cmd.Flags().SetAnnotation(flag.Name, "envkey", flag.EnvKeys) + + // Environment flags can also be exported without a prefix (e.g. DOCKER_*) + if flag.WithoutPrefix { + cmd.Flags().SetAnnotation(flag.Name, "withoutPrefix", []string{"true"}) + } } if flag.Deprecated != "" { cmd.Flags().MarkDeprecated(flag.Name, flag.Deprecated) @@ -202,9 +209,22 @@ func (m *flagManager) updateCmdFlagFromEnv(cmd *cobra.Command, prefix string) er return } for _, key := range envKeys { + + // First priority goes to prefixed variable val, set := os.LookupEnv(prefix + key) if !set { - continue + + // Determine if environment keys should be looked for without prefix + // This annotation just needs to be present, period + _, withoutPrefix := flag.Annotations["withoutPrefix"] + if !withoutPrefix { + continue + } + // Second try - looking for the same without prefix! + val, set = os.LookupEnv(key) + if !set { + continue + } } if mflag.EnvHandler != nil { if err := mflag.EnvHandler(flag, val); err != nil { diff --git a/pkg/sylog/sylog.go b/pkg/sylog/sylog.go index 2b7b62c510..f5e2c902a5 100644 --- a/pkg/sylog/sylog.go +++ b/pkg/sylog/sylog.go @@ -28,7 +28,7 @@ var messageColors = map[messageLevel]string{ var ( noColorLevel messageLevel = 90 - loggerLevel messageLevel = InfoLevel + loggerLevel = InfoLevel ) var logWriter = (io.Writer)(os.Stderr)