Skip to content

Commit b7592ab

Browse files
authored
Merge pull request #1372 from dgageot/tag-options-by-value
Pass tag options by value
2 parents f54473e + d5b4ee8 commit b7592ab

15 files changed

+41
-69
lines changed

pkg/skaffold/build/gcb/cloud_build.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,10 @@ watch:
149149
builtTag := fmt.Sprintf("%s@%s", artifact.ImageName, imageID)
150150
logrus.Infof("Image built at %s", builtTag)
151151

152-
newTag, err := tagger.GenerateFullyQualifiedImageName(artifact.Workspace, &tag.Options{
152+
newTag, err := tagger.GenerateFullyQualifiedImageName(artifact.Workspace, tag.Options{
153153
ImageName: artifact.ImageName,
154154
Digest: imageID,
155155
})
156-
157156
if err != nil {
158157
return "", errors.Wrap(err, "generating tag")
159158
}

pkg/skaffold/build/kaniko/kaniko.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, tagger tag.T
4949
return "", errors.Wrap(err, "getting digest")
5050
}
5151

52-
tag, err := tagger.GenerateFullyQualifiedImageName(artifact.Workspace, &tag.Options{
52+
tag, err := tagger.GenerateFullyQualifiedImageName(artifact.Workspace, tag.Options{
5353
ImageName: artifact.ImageName,
5454
Digest: digest,
5555
})

pkg/skaffold/build/local/local.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, tagger tag.T
6464
return tag, nil
6565
}
6666

67-
tag, err := tagger.GenerateFullyQualifiedImageName(artifact.Workspace, &tag.Options{
67+
tag, err := tagger.GenerateFullyQualifiedImageName(artifact.Workspace, tag.Options{
6868
ImageName: artifact.ImageName,
6969
Digest: digest,
7070
})

pkg/skaffold/build/local/local_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type FakeTagger struct {
3838
Err error
3939
}
4040

41-
func (f *FakeTagger) GenerateFullyQualifiedImageName(workingDir string, tagOpts *tag.Options) (string, error) {
41+
func (f *FakeTagger) GenerateFullyQualifiedImageName(workingDir string, tagOpts tag.Options) (string, error) {
4242
return f.Out, f.Err
4343
}
4444

pkg/skaffold/build/tag/custom.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,7 @@ func (c *CustomTag) Labels() map[string]string {
3434
}
3535

3636
// GenerateFullyQualifiedImageName tags an image with the custom tag
37-
func (c *CustomTag) GenerateFullyQualifiedImageName(workingDir string, opts *Options) (string, error) {
38-
if opts == nil {
39-
return "", errors.New("tag options not provided")
40-
}
41-
37+
func (c *CustomTag) GenerateFullyQualifiedImageName(workingDir string, opts Options) (string, error) {
4238
tag := c.Tag
4339
if tag == "" {
4440
return "", errors.New("custom tag not provided")

pkg/skaffold/build/tag/custom_test.go

+6-9
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,13 @@ import (
2323
)
2424

2525
func TestCustomTag_GenerateFullyQualifiedImageName(t *testing.T) {
26-
opts := &Options{
27-
ImageName: "test",
28-
Digest: "sha256:12345abcde",
26+
c := &CustomTag{
27+
Tag: "1.2.3-beta",
2928
}
3029

31-
expectedTag := "1.2.3-beta"
30+
tag, err := c.GenerateFullyQualifiedImageName(".", Options{
31+
ImageName: "test",
32+
})
3233

33-
c := &CustomTag{
34-
Tag: expectedTag,
35-
}
36-
tag, err := c.GenerateFullyQualifiedImageName(".", opts)
37-
testutil.CheckErrorAndDeepEqual(t, false, err, "test:"+expectedTag, tag)
34+
testutil.CheckErrorAndDeepEqual(t, false, err, "test:1.2.3-beta", tag)
3835
}

pkg/skaffold/build/tag/date_time.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,7 @@ func (tagger *dateTimeTagger) Labels() map[string]string {
4949
}
5050

5151
// GenerateFullyQualifiedImageName tags an image with the supplied image name and the current timestamp
52-
func (tagger *dateTimeTagger) GenerateFullyQualifiedImageName(workingDir string, opts *Options) (string, error) {
53-
if opts == nil {
54-
return "", fmt.Errorf("tag options not provided")
55-
}
56-
52+
func (tagger *dateTimeTagger) GenerateFullyQualifiedImageName(workingDir string, opts Options) (string, error) {
5753
format := tagTime
5854
if len(tagger.Format) > 0 {
5955
format = tagger.Format

pkg/skaffold/build/tag/date_time_test.go

+11-22
Original file line numberDiff line numberDiff line change
@@ -32,56 +32,45 @@ func TestDateTime_GenerateFullyQualifiedImageName(t *testing.T) {
3232
format string
3333
buildTime time.Time
3434
timezone string
35-
opts *Options
35+
imageName string
3636
digest string
3737
image string
3838
want string
39-
shouldErr bool
4039
}{
4140
{
4241
description: "default formatter",
4342
buildTime: aLocalTimeStamp,
44-
opts: &Options{
45-
ImageName: "test_image",
46-
},
47-
want: "test_image:2015-03-07_11-06-39.123_" + localZone,
43+
imageName: "test_image",
44+
want: "test_image:2015-03-07_11-06-39.123_" + localZone,
4845
},
4946
{
5047
description: "user provided timezone",
5148
buildTime: time.Unix(1234, 123456789),
5249
timezone: "UTC",
53-
opts: &Options{
54-
ImageName: "test_image",
55-
},
56-
want: "test_image:1970-01-01_00-20-34.123_UTC",
50+
imageName: "test_image",
51+
want: "test_image:1970-01-01_00-20-34.123_UTC",
5752
},
5853
{
5954
description: "user provided format",
6055
buildTime: aLocalTimeStamp,
6156
format: "2006-01-02",
62-
opts: &Options{
63-
ImageName: "test_image",
64-
},
65-
want: "test_image:2015-03-07",
66-
},
67-
{
68-
description: "error no tag opts",
69-
shouldErr: true,
57+
imageName: "test_image",
58+
want: "test_image:2015-03-07",
7059
},
7160
}
7261

7362
for _, test := range tests {
7463
t.Run(test.description, func(t *testing.T) {
75-
7664
c := &dateTimeTagger{
7765
Format: test.format,
7866
TimeZone: test.timezone,
7967
timeFn: func() time.Time { return test.buildTime },
8068
}
81-
tag, err := c.GenerateFullyQualifiedImageName(".", test.opts)
69+
tag, err := c.GenerateFullyQualifiedImageName(".", Options{
70+
ImageName: test.imageName,
71+
})
8272

83-
testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.want, tag)
73+
testutil.CheckErrorAndDeepEqual(t, false, err, test.want, tag)
8474
})
8575
}
86-
8776
}

pkg/skaffold/build/tag/env_template.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func (t *envTemplateTagger) Labels() map[string]string {
4848
}
4949

5050
// GenerateFullyQualifiedImageName tags an image with the custom tag
51-
func (t *envTemplateTagger) GenerateFullyQualifiedImageName(workingDir string, opts *Options) (string, error) {
51+
func (t *envTemplateTagger) GenerateFullyQualifiedImageName(workingDir string, opts Options) (string, error) {
5252
customMap := CreateEnvVarMap(opts.ImageName, opts.Digest)
5353
return util.ExecuteEnvTemplate(t.Template, customMap)
5454
}

pkg/skaffold/build/tag/env_template_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,15 @@ func TestEnvTemplateTagger_GenerateFullyQualifiedImageName(t *testing.T) {
2828
tests := []struct {
2929
name string
3030
template string
31-
opts *Options
31+
opts Options
3232
env []string
3333
want string
3434
shouldErr bool
3535
}{
3636
{
3737
name: "empty env",
3838
template: "{{.IMAGE_NAME}}:{{.DIGEST}}",
39-
opts: &Options{
39+
opts: Options{
4040
ImageName: "foo",
4141
Digest: "bar",
4242
},
@@ -46,7 +46,7 @@ func TestEnvTemplateTagger_GenerateFullyQualifiedImageName(t *testing.T) {
4646
name: "env",
4747
template: "{{.FOO}}-{{.BAZ}}:latest",
4848
env: []string{"FOO=BAR", "BAZ=BAT"},
49-
opts: &Options{
49+
opts: Options{
5050
ImageName: "foo",
5151
Digest: "bar",
5252
},
@@ -56,7 +56,7 @@ func TestEnvTemplateTagger_GenerateFullyQualifiedImageName(t *testing.T) {
5656
name: "opts precedence",
5757
template: "{{.IMAGE_NAME}}-{{.FROM_ENV}}:latest",
5858
env: []string{"FROM_ENV=FOO", "IMAGE_NAME=BAT"},
59-
opts: &Options{
59+
opts: Options{
6060
ImageName: "image_name",
6161
Digest: "bar",
6262
},
@@ -65,7 +65,7 @@ func TestEnvTemplateTagger_GenerateFullyQualifiedImageName(t *testing.T) {
6565
{
6666
name: "digest algo hex",
6767
template: "{{.IMAGE_NAME}}:{{.DIGEST_ALGO}}-{{.DIGEST_HEX}}",
68-
opts: &Options{
68+
opts: Options{
6969
ImageName: "foo",
7070
Digest: "sha256:abcd",
7171
},

pkg/skaffold/build/tag/git_commit.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func (c *GitCommit) Labels() map[string]string {
3939
}
4040

4141
// GenerateFullyQualifiedImageName tags an image with the supplied image name and the git commit.
42-
func (c *GitCommit) GenerateFullyQualifiedImageName(workingDir string, opts *Options) (string, error) {
42+
func (c *GitCommit) GenerateFullyQualifiedImageName(workingDir string, opts Options) (string, error) {
4343
hash, err := runGit(workingDir, "rev-parse", "--short", "HEAD")
4444
if err != nil {
4545
return fallbackOnDigest(opts, err), nil
@@ -72,23 +72,23 @@ func runGit(workingDir string, arg ...string) (string, error) {
7272
return string(bytes.TrimSpace(out)), nil
7373
}
7474

75-
func commitOrTag(currentTag string, tag string, opts *Options) string {
75+
func commitOrTag(currentTag string, tag string, opts Options) string {
7676
if len(tag) > 0 {
7777
currentTag = tag
7878
}
7979

8080
return fmt.Sprintf("%s:%s", opts.ImageName, currentTag)
8181
}
8282

83-
func shortDigest(opts *Options) string {
83+
func shortDigest(opts Options) string {
8484
return strings.TrimPrefix(opts.Digest, "sha256:")[0:7]
8585
}
8686

87-
func dirtyTag(currentTag string, opts *Options) string {
87+
func dirtyTag(currentTag string, opts Options) string {
8888
return fmt.Sprintf("%s:%s-dirty-%s", opts.ImageName, currentTag, shortDigest(opts))
8989
}
9090

91-
func fallbackOnDigest(opts *Options, err error) string {
91+
func fallbackOnDigest(opts Options, err error) string {
9292
logrus.Warnln("Using digest instead of git commit:", err)
9393

9494
return fmt.Sprintf("%s:dirty-%s", opts.ImageName, shortDigest(opts))

pkg/skaffold/build/tag/git_commit_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ func TestGitCommit_GenerateFullyQualifiedImageName(t *testing.T) {
221221
tt.createGitRepo(tmpDir.Root())
222222
workspace := tmpDir.Path(tt.subDir)
223223

224-
opts := &Options{
224+
opts := Options{
225225
ImageName: "test",
226226
Digest: "sha256:ababababababababababa",
227227
}

pkg/skaffold/build/tag/sha256.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"strings"
2222

2323
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
24-
"github.com/pkg/errors"
2524
)
2625

2726
// ChecksumTagger tags an image by the sha256 of the image tarball
@@ -35,11 +34,7 @@ func (c *ChecksumTagger) Labels() map[string]string {
3534
}
3635

3736
// GenerateFullyQualifiedImageName tags an image with the supplied image name and the sha256 checksum of the image
38-
func (c *ChecksumTagger) GenerateFullyQualifiedImageName(workingDir string, opts *Options) (string, error) {
39-
if opts == nil {
40-
return "", errors.New("tag options not provided")
41-
}
42-
37+
func (c *ChecksumTagger) GenerateFullyQualifiedImageName(workingDir string, opts Options) (string, error) {
4338
digest := opts.Digest
4439
sha256 := strings.TrimPrefix(opts.Digest, "sha256:")
4540
if sha256 == digest {

pkg/skaffold/build/tag/sha256_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
func TestGenerateFullyQualifiedImageName(t *testing.T) {
2626
var tests = []struct {
2727
description string
28-
opts *Options
28+
opts Options
2929
digest string
3030
image string
3131
expected string
@@ -34,23 +34,23 @@ func TestGenerateFullyQualifiedImageName(t *testing.T) {
3434
}{
3535
{
3636
description: "no error",
37-
opts: &Options{
37+
opts: Options{
3838
ImageName: "test",
3939
Digest: "sha256:12345abcde",
4040
},
4141
expected: "test:12345abcde",
4242
},
4343
{
4444
description: "wrong digest format",
45-
opts: &Options{
45+
opts: Options{
4646
ImageName: "test",
4747
Digest: "wrong:digest:format",
4848
},
4949
shouldErr: true,
5050
},
5151
{
5252
description: "wrong digest format no colon",
53-
opts: &Options{
53+
opts: Options{
5454
ImageName: "test",
5555
Digest: "sha256",
5656
},

pkg/skaffold/build/tag/tag.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ package tag
2020
type Tagger interface {
2121
Labels() map[string]string
2222

23-
GenerateFullyQualifiedImageName(workingDir string, tagOpts *Options) (string, error)
23+
GenerateFullyQualifiedImageName(workingDir string, tagOpts Options) (string, error)
2424
}
2525

2626
type Options struct {

0 commit comments

Comments
 (0)