Skip to content

Commit 07371e0

Browse files
chmeliikchmeliik
committed
image build: refactor annotation/label merging
Turn mergedLabels and mergedAnnotations into attributes of the Build struct and call mergeDefaultLabelsAndAnnotations() outside of the buildImage() method. Also rename to processLabelsAndAnnotations, because the method does more than just merge in the defaults now. The main reason to do this is that the merged arrays of labels and annotations will be needed earlier, outside the buildImage method, for injecting the labels.json file. Signed-off-by: Adam Cmiel <acmiel@redhat.com>
1 parent 3d583bf commit 07371e0

File tree

2 files changed

+40
-37
lines changed

2 files changed

+40
-37
lines changed

‎pkg/commands/build.go‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,8 @@ type Build struct {
214214

215215
containerfilePath string
216216
buildahSecrets []cliWrappers.BuildahSecret
217+
mergedLabels []string
218+
mergedAnnotations []string
217219
}
218220

219221
func NewBuild(cmd *cobra.Command, extraArgs []string) (*Build, error) {
@@ -264,6 +266,10 @@ func (c *Build) Run() error {
264266
return err
265267
}
266268

269+
if err := c.processLabelsAndAnnotations(); err != nil {
270+
return err
271+
}
272+
267273
if err := c.setSecretArgs(); err != nil {
268274
return err
269275
}
@@ -633,13 +639,13 @@ func processKeyValueEnvs(args []string) map[string]string {
633639
//
634640
// In addition to the OCI annotations (and labels), if AddLegacyLabels is enabled,
635641
// adds labels based on https://github.com/projectatomic/ContainerApplicationGenericLabels.
636-
func (c *Build) mergeDefaultLabelsAndAnnotations() ([]string, []string, error) {
642+
func (c *Build) processLabelsAndAnnotations() error {
637643
var defaultLabels []string
638644
var defaultAnnotations []string
639645

640646
buildTimeStr, err := c.getBuildTimeRFC3339()
641647
if err != nil {
642-
return nil, nil, fmt.Errorf("determining build timestamp: %w", err)
648+
return fmt.Errorf("determining build timestamp: %w", err)
643649
}
644650
ociCreated := "org.opencontainers.image.created=" + buildTimeStr
645651
defaultAnnotations = append(defaultAnnotations, ociCreated)
@@ -691,15 +697,17 @@ func (c *Build) mergeDefaultLabelsAndAnnotations() ([]string, []string, error) {
691697
if c.Params.AnnotationsFile != "" {
692698
fileAnnotations, err := parseAnnotationsFile(c.Params.AnnotationsFile)
693699
if err != nil {
694-
return nil, nil, fmt.Errorf("parsing annotations file: %w", err)
700+
return fmt.Errorf("parsing annotations file: %w", err)
695701
}
696702
// --annotations-file takes precedence over defaults
697703
mergedAnnotations = append(mergedAnnotations, fileAnnotations...)
698704
}
699705
// --annotations take precedence over --annotations-file
700706
mergedAnnotations = append(mergedAnnotations, c.Params.Annotations...)
701707

702-
return mergedLabels, mergedAnnotations, nil
708+
c.mergedLabels = mergedLabels
709+
c.mergedAnnotations = mergedAnnotations
710+
return nil
703711
}
704712

705713
func (c *Build) getBuildTimeRFC3339() (string, error) {
@@ -753,11 +761,6 @@ func goArchToArchitectureLabel(goarch string) string {
753761
func (c *Build) buildImage() error {
754762
l.Logger.Info("Building container image...")
755763

756-
mergedLabels, mergedAnnotations, err := c.mergeDefaultLabelsAndAnnotations()
757-
if err != nil {
758-
return err
759-
}
760-
761764
originalCwd, err := os.Getwd()
762765
if err != nil {
763766
return err
@@ -775,8 +778,8 @@ func (c *Build) buildImage() error {
775778
BuildArgs: c.Params.BuildArgs,
776779
BuildArgsFile: c.Params.BuildArgsFile,
777780
Envs: c.Params.Envs,
778-
Labels: mergedLabels,
779-
Annotations: mergedAnnotations,
781+
Labels: c.mergedLabels,
782+
Annotations: c.mergedAnnotations,
780783
SourceDateEpoch: c.Params.SourceDateEpoch,
781784
RewriteTimestamp: c.Params.RewriteTimestamp,
782785
ExtraArgs: c.Params.ExtraArgs,

‎pkg/commands/build_test.go‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,7 +1092,7 @@ func Test_goArchToArchitectureLabel(t *testing.T) {
10921092
}
10931093
}
10941094

1095-
func Test_Build_mergeDefaultLabelsAndAnnotations(t *testing.T) {
1095+
func Test_Build_processLabelsAndAnnotations(t *testing.T) {
10961096
g := NewWithT(t)
10971097

10981098
t.Run("should add default labels and annotations with provided values", func(t *testing.T) {
@@ -1104,15 +1104,15 @@ func Test_Build_mergeDefaultLabelsAndAnnotations(t *testing.T) {
11041104
},
11051105
}
11061106

1107-
labels, annotations, err := c.mergeDefaultLabelsAndAnnotations()
1107+
err := c.processLabelsAndAnnotations()
11081108
g.Expect(err).ToNot(HaveOccurred())
11091109

1110-
g.Expect(labels).To(Equal([]string{
1110+
g.Expect(c.mergedLabels).To(Equal([]string{
11111111
"org.opencontainers.image.created=2026-01-01T00:00:00Z",
11121112
"org.opencontainers.image.source=https://github.com/org/repo",
11131113
"org.opencontainers.image.revision=abc123",
11141114
}))
1115-
g.Expect(annotations).To(Equal([]string{
1115+
g.Expect(c.mergedAnnotations).To(Equal([]string{
11161116
"org.opencontainers.image.created=2026-01-01T00:00:00Z",
11171117
"org.opencontainers.image.source=https://github.com/org/repo",
11181118
"org.opencontainers.image.revision=abc123",
@@ -1124,15 +1124,15 @@ func Test_Build_mergeDefaultLabelsAndAnnotations(t *testing.T) {
11241124
Params: &BuildParams{},
11251125
}
11261126

1127-
labels, annotations, err := c.mergeDefaultLabelsAndAnnotations()
1127+
err := c.processLabelsAndAnnotations()
11281128
g.Expect(err).ToNot(HaveOccurred())
11291129

1130-
g.Expect(labels).To(ConsistOf(
1130+
g.Expect(c.mergedLabels).To(ConsistOf(
11311131
MatchRegexp(`^org.opencontainers.image.created=.+Z$`),
11321132
))
1133-
g.Expect(annotations).To(Equal(labels))
1133+
g.Expect(c.mergedAnnotations).To(Equal(c.mergedLabels))
11341134

1135-
imageCreated := labels[0]
1135+
imageCreated := c.mergedLabels[0]
11361136

11371137
_, rfc3339time, _ := strings.Cut(imageCreated, "=")
11381138
timestamp, err := time.Parse(time.RFC3339, rfc3339time)
@@ -1158,17 +1158,17 @@ func Test_Build_mergeDefaultLabelsAndAnnotations(t *testing.T) {
11581158
},
11591159
}
11601160

1161-
labels, annotations, err := c.mergeDefaultLabelsAndAnnotations()
1161+
err := c.processLabelsAndAnnotations()
11621162
g.Expect(err).ToNot(HaveOccurred())
11631163

1164-
g.Expect(labels).To(Equal([]string{
1164+
g.Expect(c.mergedLabels).To(Equal([]string{
11651165
"org.opencontainers.image.created=2026-01-01T00:00:00Z",
11661166
"org.opencontainers.image.source=https://github.com/org/repo",
11671167
"org.opencontainers.image.revision=abc123",
11681168
"some-label=foo",
11691169
"org.opencontainers.image.revision=main",
11701170
}))
1171-
g.Expect(annotations).To(Equal([]string{
1171+
g.Expect(c.mergedAnnotations).To(Equal([]string{
11721172
"org.opencontainers.image.created=2026-01-01T00:00:00Z",
11731173
"org.opencontainers.image.source=https://github.com/org/repo",
11741174
"org.opencontainers.image.revision=abc123",
@@ -1188,11 +1188,11 @@ func Test_Build_mergeDefaultLabelsAndAnnotations(t *testing.T) {
11881188
},
11891189
}
11901190

1191-
labels, annotations, err := c.mergeDefaultLabelsAndAnnotations()
1191+
err := c.processLabelsAndAnnotations()
11921192
g.Expect(err).ToNot(HaveOccurred())
11931193

11941194
arch := goArchToArchitectureLabel(runtime.GOARCH)
1195-
g.Expect(labels).To(Equal([]string{
1195+
g.Expect(c.mergedLabels).To(Equal([]string{
11961196
"org.opencontainers.image.created=2026-01-01T00:00:00Z",
11971197
"org.opencontainers.image.source=https://github.com/org/repo",
11981198
"org.opencontainers.image.revision=abc123",
@@ -1203,7 +1203,7 @@ func Test_Build_mergeDefaultLabelsAndAnnotations(t *testing.T) {
12031203
"vcs-type=git",
12041204
}))
12051205
// Should be added *only* as labels, not as annotations
1206-
g.Expect(annotations).To(Equal([]string{
1206+
g.Expect(c.mergedAnnotations).To(Equal([]string{
12071207
"org.opencontainers.image.created=2026-01-01T00:00:00Z",
12081208
"org.opencontainers.image.source=https://github.com/org/repo",
12091209
"org.opencontainers.image.revision=abc123",
@@ -1218,14 +1218,14 @@ func Test_Build_mergeDefaultLabelsAndAnnotations(t *testing.T) {
12181218
},
12191219
}
12201220

1221-
labels, annotations, err := c.mergeDefaultLabelsAndAnnotations()
1221+
err := c.processLabelsAndAnnotations()
12221222
g.Expect(err).ToNot(HaveOccurred())
12231223

1224-
g.Expect(labels).To(ContainElements(
1224+
g.Expect(c.mergedLabels).To(ContainElements(
12251225
"org.opencontainers.image.created=2026-01-01T00:00:00Z",
12261226
"build-date=2026-01-01T00:00:00Z",
12271227
))
1228-
g.Expect(annotations).To(ContainElements(
1228+
g.Expect(c.mergedAnnotations).To(ContainElements(
12291229
"org.opencontainers.image.created=2026-01-01T00:00:00Z",
12301230
))
12311231
})
@@ -1237,11 +1237,11 @@ func Test_Build_mergeDefaultLabelsAndAnnotations(t *testing.T) {
12371237
},
12381238
}
12391239

1240-
labels, annotations, err := c.mergeDefaultLabelsAndAnnotations()
1240+
err := c.processLabelsAndAnnotations()
12411241
g.Expect(err).ToNot(HaveOccurred())
12421242

1243-
g.Expect(labels).To(ContainElement("quay.expires-after=2w"))
1244-
g.Expect(annotations).ToNot(ContainElement("quay.expires-after=2w"))
1243+
g.Expect(c.mergedLabels).To(ContainElement("quay.expires-after=2w"))
1244+
g.Expect(c.mergedAnnotations).ToNot(ContainElement("quay.expires-after=2w"))
12451245
})
12461246

12471247
t.Run("should return error for invalid legacy-build-timestamp", func(t *testing.T) {
@@ -1251,7 +1251,7 @@ func Test_Build_mergeDefaultLabelsAndAnnotations(t *testing.T) {
12511251
},
12521252
}
12531253

1254-
_, _, err := c.mergeDefaultLabelsAndAnnotations()
1254+
err := c.processLabelsAndAnnotations()
12551255
g.Expect(err).To(HaveOccurred())
12561256
g.Expect(err.Error()).To(ContainSubstring("determining build timestamp: parsing legacy-build-timestamp:"))
12571257
})
@@ -1263,7 +1263,7 @@ func Test_Build_mergeDefaultLabelsAndAnnotations(t *testing.T) {
12631263
},
12641264
}
12651265

1266-
_, _, err := c.mergeDefaultLabelsAndAnnotations()
1266+
err := c.processLabelsAndAnnotations()
12671267
g.Expect(err).To(HaveOccurred())
12681268
g.Expect(err.Error()).To(ContainSubstring("determining build timestamp: parsing source-date-epoch:"))
12691269
})
@@ -1289,9 +1289,9 @@ with.hash.char=this comment # is not a comment
12891289
},
12901290
}
12911291

1292-
_, annotations, err := c.mergeDefaultLabelsAndAnnotations()
1292+
err := c.processLabelsAndAnnotations()
12931293
g.Expect(err).ToNot(HaveOccurred())
1294-
g.Expect(annotations).To(Equal([]string{
1294+
g.Expect(c.mergedAnnotations).To(Equal([]string{
12951295
// always added
12961296
"org.opencontainers.image.created=2026-01-01T00:00:00Z",
12971297
// from file, sorted alphabetically
@@ -1308,7 +1308,7 @@ with.hash.char=this comment # is not a comment
13081308
},
13091309
}
13101310

1311-
_, _, err := c.mergeDefaultLabelsAndAnnotations()
1311+
err := c.processLabelsAndAnnotations()
13121312
g.Expect(err).To(HaveOccurred())
13131313
g.Expect(err.Error()).To(MatchRegexp("parsing annotations file: .* /nonexistent/annotations.cfg"))
13141314
})
@@ -1325,7 +1325,7 @@ with.hash.char=this comment # is not a comment
13251325
},
13261326
}
13271327

1328-
_, _, err := c.mergeDefaultLabelsAndAnnotations()
1328+
err := c.processLabelsAndAnnotations()
13291329
g.Expect(err).To(HaveOccurred())
13301330
g.Expect(err.Error()).To(MatchRegexp("parsing annotations file: .*annotations.cfg:1: expected arg=value"))
13311331
})

0 commit comments

Comments
 (0)