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.

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

Adds ability for user to define additional tags for images #1388

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions cmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@
}
}

// validate tags
if cmd.Tag != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

cmd.Tag is initialised as []string{} so won't be nil, I think it would be better to check len(cmd.Tag) > 0

err = image.ValidateTags(cmd.Tag)

if err != nil {
return fmt.Errorf("cannot build image, %v", err)

Check failure on line 62 in cmd/build.go

View workflow job for this annotation

GitHub Actions / lint

non-wrapping format verb for fmt.Errorf. Use `%w` to format errors (errorlint)
}
}

// create a temporary workspace
exists := workspace2.Exists(ctx, devPodConfig, args, "", log.Default)
sshConfigFile, err := os.CreateTemp("", "devpodssh.config")
Expand Down Expand Up @@ -112,6 +121,7 @@
buildCmd.Flags().BoolVar(&cmd.SkipDelete, "skip-delete", false, "If true will not delete the workspace after building it")
buildCmd.Flags().StringVar(&cmd.Machine, "machine", "", "The machine to use for this workspace. The machine needs to exist beforehand or the command will fail. If the workspace already exists, this option has no effect")
buildCmd.Flags().StringVar(&cmd.Repository, "repository", "", "The repository to push to")
buildCmd.Flags().StringSliceVar(&cmd.Tag, "tag", []string{}, "Image Tag(s) in the form of a comma separated list --tag latest,arm64 or multiple flags --tag latest --tag arm64")
buildCmd.Flags().StringSliceVar(&cmd.Platform, "platform", []string{}, "Set target platform for build")
buildCmd.Flags().BoolVar(&cmd.SkipPush, "skip-push", false, "If true will not push the image to the repository, useful for testing")
buildCmd.Flags().Var(&cmd.GitCloneStrategy, "git-clone-strategy", "The git clone strategy DevPod uses to checkout git based workspaces. Can be full (default), blobless, treeless or shallow")
Expand Down
4 changes: 4 additions & 0 deletions pkg/devcontainer/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func (r *runner) build(
ImageName: overrideBuildImageName,
PrebuildHash: imageTag,
RegistryCache: options.RegistryCache,
Tags: options.Tag,
}, nil
}

Expand Down Expand Up @@ -135,6 +136,7 @@ func (r *runner) extendImage(
ImageMetadata: extendedBuildInfo.MetadataConfig,
ImageName: imageBase,
RegistryCache: options.RegistryCache,
Tags: options.Tag,
}, nil
}

Expand Down Expand Up @@ -322,6 +324,7 @@ func (r *runner) buildImage(
ImageName: prebuildImage,
PrebuildHash: prebuildHash,
RegistryCache: options.RegistryCache,
Tags: options.Tag,
}, nil
} else if err != nil {
r.Log.Debugf("Error trying to find prebuild image %s: %v", prebuildImage, err)
Expand Down Expand Up @@ -385,6 +388,7 @@ func dockerlessFallback(
User: buildInfo.User,
},
RegistryCache: options.RegistryCache,
Tags: options.Tag,
}, nil
}

Expand Down
1 change: 1 addition & 0 deletions pkg/devcontainer/config/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type BuildInfo struct {
ImageName string
PrebuildHash string
RegistryCache string
Tags []string

Dockerless *BuildInfoDockerless
}
Expand Down
25 changes: 21 additions & 4 deletions pkg/devcontainer/prebuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"fmt"
"os"
"path/filepath"
"strings"

"github.com/loft-sh/devpod/pkg/devcontainer/config"
"github.com/loft-sh/devpod/pkg/driver"
Expand Down Expand Up @@ -70,7 +71,6 @@
}

if isDockerComposeConfig(substitutedConfig.Config) {
r.Log.Debug("Tagging image prebuild=%s buildInfo=%s", prebuildImage, buildInfo.ImageName)
err = dockerDriver.TagDevContainer(ctx, buildInfo.ImageName, prebuildImage)
if err != nil {
return "", errors.Wrap(err, "tag image")
Expand All @@ -87,10 +87,27 @@
)
}

// Setup all image tags (prebuild and any user defined tags)
ImageRefs := []string{prebuildImage}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: imageRefs over ImageRefs


imageRepoName := strings.Split(prebuildImage, ":")
if buildInfo.Tags != nil {
for _, tag := range buildInfo.Tags {
ImageRefs = append(ImageRefs, imageRepoName[0]+":"+tag)
}
}

// tag the image
for _, imageRef := range ImageRefs {
err = dockerDriver.TagDevContainer(ctx, prebuildImage, imageRef)

Check failure on line 102 in pkg/devcontainer/prebuild.go

View workflow job for this annotation

GitHub Actions / lint

ineffectual assignment to err (ineffassign)
Copy link
Contributor

Choose a reason for hiding this comment

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

need to check err is not nil or log here

}

// push the image to the registry
err = dockerDriver.PushDevContainer(ctx, prebuildImage)
if err != nil {
return "", errors.Wrap(err, "push image")
for _, imageRef := range ImageRefs {
err = dockerDriver.PushDevContainer(ctx, imageRef)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this could be a one liner with err check

if err != nil {
return "", errors.Wrap(err, "push image")
}
}

return prebuildImage, nil
Expand Down
2 changes: 2 additions & 0 deletions pkg/driver/docker/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func (d *dockerDriver) BuildDevContainer(
ImageName: imageName,
PrebuildHash: prebuildHash,
RegistryCache: options.RegistryCache,
Tags: options.Tag,
}, nil
} else if err != nil {
d.Log.Debugf("Error trying to find local image %s: %v", imageName, err)
Expand Down Expand Up @@ -114,6 +115,7 @@ func (d *dockerDriver) BuildDevContainer(
ImageName: imageName,
PrebuildHash: prebuildHash,
RegistryCache: options.RegistryCache,
Tags: options.Tag,
}, nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/driver/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (d *dockerDriver) PushDevContainer(ctx context.Context, image string) error
}

func (d *dockerDriver) TagDevContainer(ctx context.Context, image, tag string) error {
// push image
// Tag image
writer := d.Log.Writer(logrus.InfoLevel, false)
defer writer.Close()

Expand All @@ -127,7 +127,7 @@ func (d *dockerDriver) TagDevContainer(ctx context.Context, image, tag string) e
d.Log.Debugf("Running docker command: %s %s", d.Docker.DockerCommand, strings.Join(args, " "))
err := d.Docker.Run(ctx, args, nil, writer, writer)
if err != nil {
return errors.Wrap(err, "push image")
return errors.Wrap(err, "tag image")
}

return nil
Expand Down
33 changes: 31 additions & 2 deletions pkg/image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@
import (
"context"
"fmt"
"net/http"

"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/pkg/errors"
"net/http"
"regexp"
)

var (
dockerTagRegexp = regexp.MustCompile(`^[\w][\w.-]*$`)
DockerTagMaxSize = 128
)

func GetImage(ctx context.Context, image string) (v1.Image, error) {
Expand Down Expand Up @@ -58,3 +63,27 @@

return configFile, img, nil
}

func ValidateTags(tags []string) error {
for _, tag := range tags {
if !IsValidDockerTag(tag) {
return fmt.Errorf(`%q is not a valid docker tag

Check failure on line 70 in pkg/image/image.go

View workflow job for this annotation

GitHub Actions / lint

ST1005: error strings should not end with punctuation or newlines (stylecheck)

- a tag name must be valid ASCII and may contain lowercase and uppercase letters, digits, underscores, periods and dashes;
- a tag name may not start with a period or a dash and may contain a maximum of 128 characters.`, tag)
}
}
return nil
}

func IsValidDockerTag(tag string) bool {
if shouldNotBeSlugged(tag, dockerTagRegexp, DockerTagMaxSize) {

Check failure on line 80 in pkg/image/image.go

View workflow job for this annotation

GitHub Actions / lint

S1008: should use 'return shouldNotBeSlugged(tag, dockerTagRegexp, DockerTagMaxSize)' instead of 'if shouldNotBeSlugged(tag, dockerTagRegexp, DockerTagMaxSize) { return true }; return false' (gosimple)
return true
}

return false
}

func shouldNotBeSlugged(data string, regexp *regexp.Regexp, maxSize int) bool {
return len(data) == 0 || regexp.Match([]byte(data)) && len(data) <= maxSize
}
1 change: 1 addition & 0 deletions pkg/provider/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ type CLIOptions struct {
Repository string `json:"repository,omitempty"`
SkipPush bool `json:"skipPush,omitempty"`
Platform []string `json:"platform,omitempty"`
Tag []string `json:"tag,omitempty"`

ForceBuild bool `json:"forceBuild,omitempty"`
ForceDockerless bool `json:"forceDockerless,omitempty"`
Expand Down
Loading