From 1883ed4a73c4b8b595938e8939b52f0ab87d9a55 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Sat, 26 Jun 2021 16:36:31 -0300 Subject: [PATCH] refactor: preparing for other docker implementations (#2314) * wip: podman Signed-off-by: Carlos A Becker * refactor: preparing for other docker implementations Signed-off-by: Carlos A Becker * refactor: preparing for other docker implementations Signed-off-by: Carlos A Becker * fix: log Signed-off-by: Carlos A Becker * fix: use buildx Signed-off-by: Carlos A Becker * test: cover Signed-off-by: Carlos A Becker * fix: lint Signed-off-by: Carlos A Becker --- .goreleaser.yml | 4 +- internal/pipe/docker/api.go | 43 ++++++++++++++++ internal/pipe/docker/api_docker.go | 60 ++++++++++++++++++++++ internal/pipe/docker/docker.go | 77 ++++++++++++++--------------- internal/pipe/docker/docker_test.go | 57 +++++++++++---------- internal/pipe/docker/manifest.go | 66 +++++-------------------- pkg/config/config.go | 3 +- www/docs/customization/docker.md | 9 ++-- www/docs/deprecations.md | 24 +++++++++ 9 files changed, 212 insertions(+), 131 deletions(-) create mode 100644 internal/pipe/docker/api.go create mode 100644 internal/pipe/docker/api_docker.go diff --git a/.goreleaser.yml b/.goreleaser.yml index af231775d..c915e3acf 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -39,7 +39,7 @@ dockers: - 'goreleaser/goreleaser:{{ .Tag }}-amd64' - 'ghcr.io/goreleaser/goreleaser:{{ .Tag }}-amd64' dockerfile: Dockerfile - use_buildx: true + use: buildx build_flag_templates: - "--pull" - "--label=org.opencontainers.image.created={{.Date}}" @@ -54,7 +54,7 @@ dockers: - 'goreleaser/goreleaser:{{ .Tag }}-arm64' - 'ghcr.io/goreleaser/goreleaser:{{ .Tag }}-arm64' dockerfile: Dockerfile - use_buildx: true + use: buildx build_flag_templates: - "--pull" - "--label=org.opencontainers.image.created={{.Date}}" diff --git a/internal/pipe/docker/api.go b/internal/pipe/docker/api.go new file mode 100644 index 000000000..0c25f8a91 --- /dev/null +++ b/internal/pipe/docker/api.go @@ -0,0 +1,43 @@ +package docker + +import ( + "context" + "os/exec" + + "github.com/apex/log" + "github.com/goreleaser/goreleaser/pkg/config" +) + +// imager is something that can build and push docker images. +type imager interface { + Build(ctx context.Context, root string, images, flags []string) error + Push(ctx context.Context, image string) error +} + +// manifester is something that can create and push docker manifests. +type manifester interface { + Create(ctx context.Context, manifest string, images, flags []string) error + Push(ctx context.Context, manifest string, flags []string) error +} + +func newManifester(manifest config.DockerManifest) manifester { + return dockerManifester{} +} + +func newImager(docker config.Docker) imager { + return dockerImager{ + buildx: docker.Use == useBuildx, + } +} + +// nolint: unparam +func runCommand(ctx context.Context, dir, binary string, args ...string) error { + /* #nosec */ + cmd := exec.CommandContext(ctx, binary, args...) + cmd.Dir = dir + log := log.WithField("cmd", cmd.Args).WithField("cwd", cmd.Dir) + log.Debug("running") + out, err := cmd.CombinedOutput() + log.Debug(string(out)) + return err +} diff --git a/internal/pipe/docker/api_docker.go b/internal/pipe/docker/api_docker.go new file mode 100644 index 000000000..40b9df17a --- /dev/null +++ b/internal/pipe/docker/api_docker.go @@ -0,0 +1,60 @@ +package docker + +import ( + "context" + "fmt" +) + +type dockerManifester struct{} + +func (m dockerManifester) Create(ctx context.Context, manifest string, images, flags []string) error { + _ = runCommand(ctx, ".", "docker", "manifest", "rm", manifest) + + args := []string{"manifest", "create", manifest} + args = append(args, images...) + args = append(args, flags...) + + if err := runCommand(ctx, ".", "docker", args...); err != nil { + return fmt.Errorf("failed to create %s: %w", manifest, err) + } + return nil +} + +func (m dockerManifester) Push(ctx context.Context, manifest string, flags []string) error { + args := []string{"manifest", "push", manifest} + args = append(args, flags...) + if err := runCommand(ctx, ".", "docker", args...); err != nil { + return fmt.Errorf("failed to push %s: %w", manifest, err) + } + return nil +} + +type dockerImager struct { + buildx bool +} + +func (i dockerImager) Push(ctx context.Context, image string) error { + if err := runCommand(ctx, ".", "docker", "push", image); err != nil { + return fmt.Errorf("failed to push %s: %w", image, err) + } + return nil +} + +func (i dockerImager) Build(ctx context.Context, root string, images, flags []string) error { + if err := runCommand(ctx, root, "docker", i.buildCommand(images, flags)...); err != nil { + return fmt.Errorf("failed to build %s: %w", images[0], err) + } + return nil +} + +func (i dockerImager) buildCommand(images, flags []string) []string { + base := []string{"build", "."} + if i.buildx { + base = []string{"buildx", "build", ".", "--load"} + } + for _, image := range images { + base = append(base, "-t", image) + } + base = append(base, flags...) + return base +} diff --git a/internal/pipe/docker/docker.go b/internal/pipe/docker/docker.go index df2cf3a11..a52d663cd 100644 --- a/internal/pipe/docker/docker.go +++ b/internal/pipe/docker/docker.go @@ -5,7 +5,6 @@ import ( "fmt" "io/ioutil" "os" - "os/exec" "path/filepath" "strings" @@ -19,8 +18,14 @@ import ( "github.com/goreleaser/goreleaser/pkg/context" ) -// ErrNoDocker is shown when docker cannot be found in $PATH. -var ErrNoDocker = errors.New("docker not present in $PATH") +const ( + dockerConfigExtra = "DockerConfig" + + useBuildx = "buildx" + useDocker = "docker" +) + +var validDockerUses = []string{useBuildx, useDocker} // Pipe for docker. type Pipe struct{} @@ -50,6 +55,18 @@ func (Pipe) Default(ctx *context.Context) error { deprecate.Notice(ctx, "docker.builds") docker.IDs = append(docker.IDs, docker.Builds...) } + if docker.Buildx { + deprecate.Notice(ctx, "docker.use_buildx") + if docker.Use == "" { + docker.Use = useBuildx + } + } + if docker.Use == "" { + docker.Use = useDocker + } + if !isValidUse(docker.Use) { + return fmt.Errorf("docker: invalid use: %s, valid options are %v", docker.Use, validDockerUses) + } for _, f := range docker.Files { if f == "." || strings.HasPrefix(f, ctx.Config.Dist) { return fmt.Errorf("invalid docker.files: can't be . or inside dist folder: %s", f) @@ -59,15 +76,20 @@ func (Pipe) Default(ctx *context.Context) error { return nil } +func isValidUse(use string) bool { + for _, s := range validDockerUses { + if s == use { + return true + } + } + return false +} + // Run the pipe. func (Pipe) Run(ctx *context.Context) error { if len(ctx.Config.Dockers) == 0 || len(ctx.Config.Dockers[0].ImageTemplates) == 0 { return pipe.ErrSkipDisabledPipe } - _, err := exec.LookPath("docker") - if err != nil { - return ErrNoDocker - } return doRun(ctx) } @@ -145,7 +167,7 @@ func process(ctx *context.Context, docker config.Docker, artifacts []*artifact.A return err } - if err := dockerBuild(ctx, tmp, images, buildFlags, docker.Buildx); err != nil { + if err := newImager(docker).Build(ctx, tmp, images, buildFlags); err != nil { return err } @@ -166,6 +188,9 @@ func process(ctx *context.Context, docker config.Docker, artifacts []*artifact.A Goarch: docker.Goarch, Goos: docker.Goos, Goarm: docker.Goarm, + Extra: map[string]interface{}{ + dockerConfigExtra: docker, + }, }) } return nil @@ -229,42 +254,12 @@ func link(src, dest string) error { }) } -func dockerBuild(ctx *context.Context, root string, images, flags []string, buildx bool) error { - log.WithField("image", images[0]).WithField("buildx", buildx).Info("building docker image") - /* #nosec */ - cmd := exec.CommandContext(ctx, "docker", buildCommand(buildx, images, flags)...) - cmd.Dir = root - log.WithField("cmd", cmd.Args).WithField("cwd", cmd.Dir).Debug("running") - out, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("failed to build docker image: %s: \n%s: %w", images[0], string(out), err) - } - log.Debugf("docker build output: \n%s", string(out)) - return nil -} - -func buildCommand(buildx bool, images, flags []string) []string { - base := []string{"build", "."} - if buildx { - base = []string{"buildx", "build", ".", "--load"} - } - for _, image := range images { - base = append(base, "-t", image) - } - base = append(base, flags...) - return base -} - func dockerPush(ctx *context.Context, image *artifact.Artifact) error { log.WithField("image", image.Name).Info("pushing docker image") - /* #nosec */ - cmd := exec.CommandContext(ctx, "docker", "push", image.Name) - log.WithField("cmd", cmd.Args).Debug("running") - out, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("failed to push docker image: \n%s: %w", string(out), err) + docker := image.Extra[dockerConfigExtra].(config.Docker) + if err := newImager(docker).Push(ctx, image.Name); err != nil { + return err } - log.Debugf("docker push output: \n%s", string(out)) ctx.Artifacts.Add(&artifact.Artifact{ Type: artifact.DockerImage, Name: image.Name, diff --git a/internal/pipe/docker/docker_test.go b/internal/pipe/docker/docker_test.go index 35f474d74..121eafa96 100644 --- a/internal/pipe/docker/docker_test.go +++ b/internal/pipe/docker/docker_test.go @@ -344,7 +344,7 @@ func TestRunPipe(t *testing.T) { expect: []string{registry + "goreleaser/test_multiarch_fail:latest-arm64v8"}, assertError: shouldNotErr, pubAssertError: shouldNotErr, - manifestAssertError: shouldErr("failed to create docker manifest: localhost:5000/goreleaser/test_multiarch_fail:test"), + manifestAssertError: shouldErr("failed to create localhost:5000/goreleaser/test_multiarch_fail:test"), assertImageLabels: noLabels, }, "multiarch manifest template error": { @@ -525,7 +525,7 @@ func TestRunPipe(t *testing.T) { }, expect: []string{}, assertImageLabels: noLabels, - assertError: shouldErr(`goreleaser/test_run_pipe_template_UPPERCASE:v1.0.0" for "-t, --tag" flag: invalid reference format: repository name must be lowercase`), + assertError: shouldErr(`failed to build localhost:5000/goreleaser/test_run_pipe_template_UPPERCASE:v1.0.0`), pubAssertError: shouldNotErr, manifestAssertError: shouldNotErr, }, @@ -687,7 +687,7 @@ func TestRunPipe(t *testing.T) { registry + "goreleaser/one_img_error_with_skip_push:true", }, assertImageLabels: noLabels, - assertError: shouldErr("failed to build docker image"), + assertError: shouldErr("failed to build localhost:5000/goreleaser/one_img_error_with_skip_push:false"), }, "valid_no_latest": { dockers: []config.Docker{ @@ -745,7 +745,7 @@ func TestRunPipe(t *testing.T) { }, }, assertImageLabels: noLabels, - assertError: shouldErr("unknown flag: --bad-flag"), + assertError: shouldErr("failed to build localhost:5000/goreleaser/test_build_args:latest"), }, "bad_dockerfile": { dockers: []config.Docker{ @@ -759,7 +759,7 @@ func TestRunPipe(t *testing.T) { }, }, assertImageLabels: noLabels, - assertError: shouldErr("pull access denied"), + assertError: shouldErr("failed to build localhost:5000/goreleaser/bad_dockerfile:latest"), }, "tag_template_error": { dockers: []config.Docker{ @@ -860,7 +860,7 @@ func TestRunPipe(t *testing.T) { }, assertImageLabels: noLabels, assertError: shouldNotErr, - pubAssertError: shouldErr(`requested access to the resource is denied`), + pubAssertError: shouldErr(`failed to push docker.io/nope:latest`), manifestAssertError: shouldNotErr, }, "dockerfile_doesnt_exist": { @@ -1107,7 +1107,10 @@ func TestBuildCommand(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.expect, buildCommand(tt.buildx, images, tt.flags)) + imager := dockerImager{ + buildx: tt.buildx, + } + require.Equal(t, tt.expect, imager.buildCommand(images, tt.flags)) }) } } @@ -1130,25 +1133,6 @@ func TestNoDockerWithoutImageName(t *testing.T) { })))) } -func TestDockerNotInPath(t *testing.T) { - path := os.Getenv("PATH") - defer func() { - require.NoError(t, os.Setenv("PATH", path)) - }() - require.NoError(t, os.Setenv("PATH", "")) - ctx := &context.Context{ - Version: "1.0.0", - Config: config.Project{ - Dockers: []config.Docker{ - { - ImageTemplates: []string{"a/b"}, - }, - }, - }, - } - require.EqualError(t, Pipe{}.Run(ctx), ErrNoDocker.Error()) -} - func TestDefault(t *testing.T) { ctx := &context.Context{ Config: config.Project{ @@ -1158,15 +1142,34 @@ func TestDefault(t *testing.T) { Builds: []string{"foo"}, Binaries: []string{"aaa"}, }, + { + Use: useBuildx, + }, }, }, } require.NoError(t, Pipe{}.Default(ctx)) - require.Len(t, ctx.Config.Dockers, 1) + require.Len(t, ctx.Config.Dockers, 2) docker := ctx.Config.Dockers[0] require.Equal(t, "linux", docker.Goos) require.Equal(t, "amd64", docker.Goarch) require.Equal(t, []string{"aa", "foo"}, docker.IDs) + require.Equal(t, useDocker, docker.Use) + docker = ctx.Config.Dockers[1] + require.Equal(t, useBuildx, docker.Use) +} + +func TestDefaultInvalidUse(t *testing.T) { + ctx := &context.Context{ + Config: config.Project{ + Dockers: []config.Docker{ + { + Use: "something", + }, + }, + }, + } + require.EqualError(t, Pipe{}.Default(ctx), `docker: invalid use: something, valid options are [buildx docker]`) } func TestDefaultDockerfile(t *testing.T) { diff --git a/internal/pipe/docker/manifest.go b/internal/pipe/docker/manifest.go index 4387f65fb..53a52b4d5 100644 --- a/internal/pipe/docker/manifest.go +++ b/internal/pipe/docker/manifest.go @@ -1,8 +1,6 @@ package docker import ( - "fmt" - "os/exec" "strings" "github.com/apex/log" @@ -34,21 +32,25 @@ func (ManifestPipe) Publish(ctx *context.Context) error { if strings.TrimSpace(manifest.SkipPush) == "true" { return pipe.Skip("docker_manifest.skip_push is set") } + if strings.TrimSpace(manifest.SkipPush) == "auto" && ctx.Semver.Prerelease != "" { return pipe.Skip("prerelease detected with 'auto' push, skipping docker manifest") } + name, err := manifestName(ctx, manifest) if err != nil { return err } - if err := dockerManifestRm(ctx, name); err != nil { - return err - } + images, err := manifestImages(ctx, manifest) if err != nil { return err } - if err := dockerManifestCreate(ctx, name, images, manifest.CreateFlags); err != nil { + + manifester := newManifester(manifest) + + log.WithField("manifest", manifest).WithField("images", images).Info("creating docker manifest") + if err := manifester.Create(ctx, name, images, manifest.CreateFlags); err != nil { return err } ctx.Artifacts.Add(&artifact.Artifact{ @@ -56,7 +58,9 @@ func (ManifestPipe) Publish(ctx *context.Context) error { Name: name, Path: name, }) - return dockerManifestPush(ctx, name, manifest.PushFlags) + + log.WithField("manifest", manifest).Info("pushing docker manifest") + return manifester.Push(ctx, name, manifest.PushFlags) }) } return g.Wait() @@ -87,51 +91,3 @@ func manifestImages(ctx *context.Context, manifest config.DockerManifest) ([]str } return imgs, nil } - -func dockerManifestRm(ctx *context.Context, manifest string) error { - log.WithField("manifest", manifest).Info("removing local docker manifest") - /* #nosec */ - cmd := exec.CommandContext(ctx, "docker", "manifest", "rm", manifest) - log.WithField("cmd", cmd.Args).Debug("running") - out, err := cmd.CombinedOutput() - if err != nil { - if strings.HasPrefix(string(out), "No such manifest: ") { - // ignore "no such manifest" error, is the state we want in the end... - return nil - } - return fmt.Errorf("failed to remove local docker manifest: %s: \n%s: %w", manifest, string(out), err) - } - log.Debugf("docker manifest rm output: \n%s", string(out)) - return nil -} - -func dockerManifestCreate(ctx *context.Context, manifest string, images, flags []string) error { - log.WithField("manifest", manifest).WithField("images", images).Info("creating docker manifest") - args := []string{"manifest", "create", manifest} - args = append(args, images...) - args = append(args, flags...) - /* #nosec */ - cmd := exec.CommandContext(ctx, "docker", args...) - log.WithField("cmd", cmd.Args).Debug("running") - out, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("failed to create docker manifest: %s: \n%s: %w", manifest, string(out), err) - } - log.Debugf("docker manifest output: \n%s", string(out)) - return nil -} - -func dockerManifestPush(ctx *context.Context, manifest string, flags []string) error { - log.WithField("manifest", manifest).Info("pushing docker manifest") - args := []string{"manifest", "push", manifest} - args = append(args, flags...) - /* #nosec */ - cmd := exec.CommandContext(ctx, "docker", args...) - log.WithField("cmd", cmd.Args).Debug("running") - out, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("failed to push docker manifest: %s: \n%s: %w", manifest, string(out), err) - } - log.Debugf("docker manifest output: \n%s", string(out)) - return nil -} diff --git a/pkg/config/config.go b/pkg/config/config.go index 94548ab26..a866a0fc7 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -519,7 +519,8 @@ type Docker struct { SkipPush string `yaml:"skip_push,omitempty"` Files []string `yaml:"extra_files,omitempty"` BuildFlagTemplates []string `yaml:"build_flag_templates,omitempty"` - Buildx bool `yaml:"use_buildx,omitempty"` + Buildx bool `yaml:"use_buildx,omitempty"` // deprecated: use Use instead + Use string `yaml:"use,omitempty"` } // DockerManifest config. diff --git a/www/docs/customization/docker.md b/www/docs/customization/docker.md index 18495fd6b..62ae26fac 100644 --- a/www/docs/customization/docker.md +++ b/www/docs/customization/docker.md @@ -82,11 +82,10 @@ dockers: # Path to the Dockerfile (from the project root). dockerfile: Dockerfile - # Whether to use `docker buildx build` instead of `docker build`. - # You probably want to set it to true when using flags like `--platform`. - # If true, will also add `--load` to the build flags. - # Defaults to false. - use_buildx: true + # Set the "backend" for the docker pipe. + # Valid options are: docker, buildx + # Defaults to docker. + use: docker # Template of the docker build flags. build_flag_templates: diff --git a/www/docs/deprecations.md b/www/docs/deprecations.md index 3266d7b40..0674459e0 100644 --- a/www/docs/deprecations.md +++ b/www/docs/deprecations.md @@ -15,6 +15,30 @@ goreleaser check ## Active deprecation notices + +### docker.use_buildx + +> since 2021-06-26 (v0.172.0) + +`use_buildx` is deprecated in favor of the more generalist `use`, since now it also allow other options in the future: + +Change this: + +=== "Before" + ```yaml + dockers: + - + use_buildx: true + ``` + +=== "After" + ```yaml + dockers: + - + use: buildx + ``` + + ### Skipping SemVer Validations > since 2021-02-28 (v0.158.0)