Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for podman image digests #5154

Open
wants to merge 4 commits into
base: mainline
Choose a base branch
from
Open

Add support for podman image digests #5154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fc2b9bd
Add support for podman image digests
mjrlee Aug 3, 2023
faa10e7
Use string type and constant for default engine
mjrlee Feb 13, 2024
ab6b666
Merge branch 'mainline' into podman
mjrlee Feb 24, 2024
5373e1e
Merge branch 'mainline' into podman
huanjani Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions internal/pkg/cli/deploy/workload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@
CacheFrom: buildArgs.CacheFrom,
Target: aws.StringValue(buildArgs.Target),
Platform: mf.ContainerPlatform(),
Engine: aws.StringValue(buildArgs.Engine),

Check failure on line 596 in internal/pkg/cli/deploy/workload.go

View workflow job for this annotation

GitHub Actions / binsize 

cannot use buildArgs.Engine (variable of type string) as *string value in argument to aws.StringValue

Check failure on line 596 in internal/pkg/cli/deploy/workload.go

View workflow job for this annotation

GitHub Actions / build (compile-linux) 

cannot use buildArgs.Engine (variable of type string) as *string value in argument to aws.StringValue

Check failure on line 596 in internal/pkg/cli/deploy/workload.go

View workflow job for this annotation

GitHub Actions / build (compile-windows) 

cannot use buildArgs.Engine (variable of type string) as *string value in argument to aws.StringValue

Check failure on line 596 in internal/pkg/cli/deploy/workload.go

View workflow job for this annotation

GitHub Actions / staticcheck 

cannot use buildArgs.Engine (variable of type string) as *string value in argument to aws.StringValue) (typecheck)

Check failure on line 596 in internal/pkg/cli/deploy/workload.go

View workflow job for this annotation

GitHub Actions / staticcheck 

cannot use buildArgs.Engine (variable of type string) as *string value in argument to aws.StringValue) (typecheck)

Check failure on line 596 in internal/pkg/cli/deploy/workload.go

View workflow job for this annotation

GitHub Actions / staticcheck 

cannot use buildArgs.Engine (variable of type string) as *string value in argument to aws.StringValue (typecheck)

Check failure on line 596 in internal/pkg/cli/deploy/workload.go

View workflow job for this annotation

GitHub Actions / build (compile-darwin) 

cannot use buildArgs.Engine (variable of type string) as *string value in argument to aws.StringValue

Check failure on line 596 in internal/pkg/cli/deploy/workload.go

View workflow job for this annotation

GitHub Actions / test 

cannot use buildArgs.Engine (variable of type string) as *string value in argument to aws.StringValue
Tags: tags,
Labels: labels,
}
Expand Down
21 changes: 20 additions & 1 deletion internal/pkg/docker/dockerengine/dockerengine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ type BuildArguments struct {
Platform string // Optional. OS/Arch to pass to `docker build`.
Args map[string]string // Optional. Build args to pass via `--build-arg` flags. Equivalent to ARG directives in dockerfile.
Labels map[string]string // Required. Set metadata for an image.
Engine string // Optional. Currently supported options are "docker" and "podman". Defaults to "docker".
}

// RunOptions holds the options for running a Docker container.
Expand Down Expand Up @@ -232,12 +233,22 @@ func (c DockerCmdClient) Exec(ctx context.Context, container string, out io.Writ
}

// Push pushes the images with the specified tags and ecr repository URI, and returns the image digest on success.
func (c DockerCmdClient) Push(ctx context.Context, uri string, w io.Writer, tags ...string) (digest string, err error) {
func (c DockerCmdClient) Push(ctx context.Context, uri string, engine string, w io.Writer, tags ...string) (digest string, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i feel like we should have a different function for podman push (same signature as Push it's just with a different name), so that we don't need to change the function signature and also the push function is quite different when the engine type is podman.

Copy link
Author

@mjrlee mjrlee Feb 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did consider this, but it seems like there's still a lot of shared functionality that would be a pain to update in multiple places.

The alternative could be to build a new type PodmanCmdClient that implements ContainerLoginBuildPusher by just calling the existing methods - but again that seemed to add more complexity than it removed.

images := []string{}
for _, tag := range tags {
images = append(images, imageName(uri, tag))
}
var args []string
// Podman image digests are based on the compressed image, so need to be gathered from podman.
var digestFile *os.File
if engine == "podman" {
digestFile, err = os.CreateTemp("", "copilot-digest")
if err != nil {
return "", fmt.Errorf("create temp file for digest: %w", err)
}
defer os.Remove(digestFile.Name())
args = append(args, "--digestfile", digestFile.Name())
}
if ci, _ := c.lookupEnv("CI"); ci == "true" {
args = append(args, "--quiet")
}
Expand All @@ -247,6 +258,14 @@ func (c DockerCmdClient) Push(ctx context.Context, uri string, w io.Writer, tags
return "", fmt.Errorf("docker push %s: %w", img, err)
}
}
if engine == "podman" {
digest, err := os.ReadFile(digestFile.Name())
if err != nil {
return "", fmt.Errorf("read digest file: %w", err)
}
return string(digest), nil
}

buf := new(strings.Builder)
// The container image will have the same digest regardless of the associated tag.
// Pick the first tag and get the image's digest.
Expand Down
11 changes: 6 additions & 5 deletions internal/pkg/docker/dockerengine/dockerengine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func TestDockerCommand_Build(t *testing.T) {
args map[string]string
target string
cacheFrom []string
engine string
envVars map[string]string
labels map[string]string
setupMocks func(controller *gomock.Controller)
Expand Down Expand Up @@ -313,7 +314,7 @@ func TestDockerCommand_Push(t *testing.T) {
lookupEnv: emptyLookupEnv,
}
buf := new(strings.Builder)
digest, err := cmd.Push(ctx, "aws_account_id.dkr.ecr.region.amazonaws.com/my-web-app", buf, "latest", "g123bfc")
digest, err := cmd.Push(ctx, "aws_account_id.dkr.ecr.region.amazonaws.com/my-web-app", "docker", buf, "latest", "g123bfc")

// THEN
require.NoError(t, err)
Expand Down Expand Up @@ -343,7 +344,7 @@ func TestDockerCommand_Push(t *testing.T) {
},
}
buf := new(strings.Builder)
digest, err := cmd.Push(context.Background(), "aws_account_id.dkr.ecr.region.amazonaws.com/my-web-app", buf, "latest")
digest, err := cmd.Push(context.Background(), "aws_account_id.dkr.ecr.region.amazonaws.com/my-web-app", "docker", buf, "latest")

// THEN
require.NoError(t, err)
Expand All @@ -362,7 +363,7 @@ func TestDockerCommand_Push(t *testing.T) {
lookupEnv: emptyLookupEnv,
}
buf := new(strings.Builder)
_, err := cmd.Push(ctx, "uri", buf, "latest")
_, err := cmd.Push(ctx, "uri", "docker", buf, "latest")

// THEN
require.EqualError(t, err, "docker push uri:latest: some error")
Expand All @@ -381,7 +382,7 @@ func TestDockerCommand_Push(t *testing.T) {
lookupEnv: emptyLookupEnv,
}
buf := new(strings.Builder)
_, err := cmd.Push(ctx, "uri", buf, "latest")
_, err := cmd.Push(ctx, "uri", "docker", buf, "latest")

// THEN
require.EqualError(t, err, "inspect image digest for uri: some error")
Expand All @@ -406,7 +407,7 @@ func TestDockerCommand_Push(t *testing.T) {
lookupEnv: emptyLookupEnv,
}
buf := new(strings.Builder)
_, err := cmd.Push(ctx, "aws_account_id.dkr.ecr.region.amazonaws.com/my-web-app", buf, "latest", "g123bfc")
_, err := cmd.Push(ctx, "aws_account_id.dkr.ecr.region.amazonaws.com/my-web-app", "docker", buf, "latest", "g123bfc")

// THEN
require.EqualError(t, err, "parse the digest from the repo digest ''")
Expand Down
11 changes: 11 additions & 0 deletions internal/pkg/manifest/workload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

const (
defaultDockerfileName = "Dockerfile"
defaultEngine = "docker"
)

// SQS Queue field options.
Expand Down Expand Up @@ -178,6 +179,7 @@ func (i *ImageLocationOrBuild) BuildConfig(rootDirectory string) *DockerBuildArg
Args: i.args(),
Target: i.target(),
CacheFrom: i.cacheFrom(),
Engine: i.engine(),
}
}

Expand Down Expand Up @@ -238,6 +240,14 @@ func (i *ImageLocationOrBuild) cacheFrom() []string {
return i.Build.BuildArgs.CacheFrom
}

// engine returns the engine to use for building the image if it exists, otherwise the defaultEngine
func (i *ImageLocationOrBuild) engine() string {
if i.Build.BuildArgs.Engine != "" {
return i.Build.BuildArgs.Engine
}
return defaultEngine
}

// ImageOverride holds fields that override Dockerfile image defaults.
type ImageOverride struct {
EntryPoint EntryPointOverride `yaml:"entrypoint"`
Expand Down Expand Up @@ -398,6 +408,7 @@ type DockerBuildArgs struct {
Args map[string]string `yaml:"args,omitempty"`
Target *string `yaml:"target,omitempty"`
CacheFrom []string `yaml:"cache_from,omitempty"`
Engine string `yaml:"engine,omitempty"`
}

func (b *DockerBuildArgs) isEmpty() bool {
Expand Down
5 changes: 3 additions & 2 deletions internal/pkg/repository/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.

Check failure on line 1 in internal/pkg/repository/repository.go

View workflow job for this annotation

GitHub Actions / staticcheck 

: # github.com/aws/copilot-cli/internal/pkg/repository [github.com/aws/copilot-cli/internal/pkg/repository.test]
// SPDX-License-Identifier: Apache-2.0

// Package repository provides support for building and pushing images to a repository.
Expand All @@ -18,7 +18,7 @@
type ContainerLoginBuildPusher interface {
Build(ctx context.Context, args *dockerengine.BuildArguments, w io.Writer) error
Login(uri, username, password string) error
Push(ctx context.Context, uri string, w io.Writer, tags ...string) (digest string, err error)
Push(ctx context.Context, uri string, engine string, w io.Writer, tags ...string) (digest string, err error)
IsEcrCredentialHelperEnabled(uri string) bool
}

Expand Down Expand Up @@ -77,10 +77,11 @@
return "", fmt.Errorf("build Dockerfile at %s: %w", args.Dockerfile, err)
}

digest, err = r.docker.Push(ctx, args.URI, w, args.Tags...)
digest, err = r.docker.Push(ctx, args.URI, args.Engine, w, args.Tags...)
if err != nil {
return "", fmt.Errorf("push to repo %s: %w", r.name, err)
}

return digest, nil
}

Expand Down
Loading