From 5202416385547fe82b9009deb1ede6cba081d01b Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Wed, 4 Aug 2021 12:44:33 -0400 Subject: [PATCH 1/5] Don't set image.base.name if base is specified by digest --- pkg/build/gobuild.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/build/gobuild.go b/pkg/build/gobuild.go index 7de7280c96..633148f145 100644 --- a/pkg/build/gobuild.go +++ b/pkg/build/gobuild.go @@ -779,9 +779,13 @@ func (g *gobuild) buildOne(ctx context.Context, refStr string, baseRef name.Refe return nil, err } + var baseName string + if _, ok := baseRef.(name.Tag); ok { + baseName = baseRef.Name() + } withApp = mutate.Annotations(withApp, map[string]string{ specsv1.AnnotationBaseImageDigest: baseDigest.String(), - specsv1.AnnotationBaseImageName: baseRef.Name(), + specsv1.AnnotationBaseImageName: baseName, }).(v1.Image) // Start from a copy of the base image's config file, and set @@ -928,8 +932,12 @@ func (g *gobuild) buildAll(ctx context.Context, ref string, baseRef name.Referen // Annotate the index with base image information, if the index is an OCI image index. // (Docker manifest lists don't support annotations) if baseType == types.OCIImageIndex { + var baseName string + if _, ok := baseRef.(name.Tag); ok { + baseName = baseRef.Name() + } idx = mutate.Annotations(idx, map[string]string{ - specsv1.AnnotationBaseImageName: baseRef.Name(), + specsv1.AnnotationBaseImageName: baseName, specsv1.AnnotationBaseImageDigest: baseDigest.String(), }).(v1.ImageIndex) } From a85cfeb2041e2635735bf35eca59ab82eed8ad14 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Wed, 4 Aug 2021 15:00:20 -0400 Subject: [PATCH 2/5] don't set empty annotation --- pkg/build/gobuild.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/pkg/build/gobuild.go b/pkg/build/gobuild.go index 633148f145..05d746e3e5 100644 --- a/pkg/build/gobuild.go +++ b/pkg/build/gobuild.go @@ -779,14 +779,13 @@ func (g *gobuild) buildOne(ctx context.Context, refStr string, baseRef name.Refe return nil, err } - var baseName string + anns := map[string]string{ + specsv1.AnnotationBaseImageDigest: baseDigest.String(), + } if _, ok := baseRef.(name.Tag); ok { - baseName = baseRef.Name() + anns[specsv1.AnnotationBaseImageName] = baseRef.Name() } - withApp = mutate.Annotations(withApp, map[string]string{ - specsv1.AnnotationBaseImageDigest: baseDigest.String(), - specsv1.AnnotationBaseImageName: baseName, - }).(v1.Image) + withApp = mutate.Annotations(withApp, anns).(v1.Image) // Start from a copy of the base image's config file, and set // the entrypoint to our app. @@ -932,14 +931,13 @@ func (g *gobuild) buildAll(ctx context.Context, ref string, baseRef name.Referen // Annotate the index with base image information, if the index is an OCI image index. // (Docker manifest lists don't support annotations) if baseType == types.OCIImageIndex { - var baseName string + anns := map[string]string{ + specsv1.AnnotationBaseImageDigest: baseDigest.String(), + } if _, ok := baseRef.(name.Tag); ok { - baseName = baseRef.Name() + anns[specsv1.AnnotationBaseImageName] = baseRef.Name() } - idx = mutate.Annotations(idx, map[string]string{ - specsv1.AnnotationBaseImageName: baseName, - specsv1.AnnotationBaseImageDigest: baseDigest.String(), - }).(v1.ImageIndex) + idx = mutate.Annotations(idx, anns).(v1.ImageIndex) } return idx, nil From 86c81958d9bc95dab9615c60fa9eb0a6d203feaf Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Wed, 4 Aug 2021 15:42:00 -0400 Subject: [PATCH 3/5] annotate Results, not Images and Indexes separately --- pkg/build/gobuild.go | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/pkg/build/gobuild.go b/pkg/build/gobuild.go index 05d746e3e5..d686f51e05 100644 --- a/pkg/build/gobuild.go +++ b/pkg/build/gobuild.go @@ -779,14 +779,6 @@ func (g *gobuild) buildOne(ctx context.Context, refStr string, baseRef name.Refe return nil, err } - anns := map[string]string{ - specsv1.AnnotationBaseImageDigest: baseDigest.String(), - } - if _, ok := baseRef.(name.Tag); ok { - anns[specsv1.AnnotationBaseImageName] = baseRef.Name() - } - withApp = mutate.Annotations(withApp, anns).(v1.Image) - // Start from a copy of the base image's config file, and set // the entrypoint to our app. cfg, err := withApp.ConfigFile() @@ -866,22 +858,40 @@ func (g *gobuild) Build(ctx context.Context, s string) (Result, error) { return nil, err } + var res Result switch mt { case types.OCIImageIndex, types.DockerManifestList: baseIndex, ok := base.(v1.ImageIndex) if !ok { return nil, fmt.Errorf("failed to interpret base as index: %v", base) } - return g.buildAll(ctx, s, baseRef, baseDigest, baseIndex) + res, err = g.buildAll(ctx, s, baseRef, baseDigest, baseIndex) case types.OCIManifestSchema1, types.DockerManifestSchema2: baseImage, ok := base.(v1.Image) if !ok { return nil, fmt.Errorf("failed to interpret base as image: %v", base) } - return g.buildOne(ctx, s, baseRef, baseDigest, baseImage, nil) + res, err = g.buildOne(ctx, s, baseRef, baseDigest, baseImage, nil) default: return nil, fmt.Errorf("base image media type: %s", mt) } + if err != nil { + return nil, err + } + + // Annotate the image or index with base image information. + // (Docker manifest lists don't support annotations) + if mt != types.DockerManifestList { + anns := map[string]string{ + specsv1.AnnotationBaseImageDigest: baseDigest.String(), + } + if _, ok := baseRef.(name.Tag); ok { + anns[specsv1.AnnotationBaseImageName] = baseRef.Name() + } + res = mutate.Annotations(res, anns).(Result) + } + + return res, nil } // TODO(#192): Do these in parallel? @@ -928,18 +938,6 @@ func (g *gobuild) buildAll(ctx context.Context, ref string, baseRef name.Referen } idx := mutate.IndexMediaType(mutate.AppendManifests(empty.Index, adds...), baseType) - // Annotate the index with base image information, if the index is an OCI image index. - // (Docker manifest lists don't support annotations) - if baseType == types.OCIImageIndex { - anns := map[string]string{ - specsv1.AnnotationBaseImageDigest: baseDigest.String(), - } - if _, ok := baseRef.(name.Tag); ok { - anns[specsv1.AnnotationBaseImageName] = baseRef.Name() - } - idx = mutate.Annotations(idx, anns).(v1.ImageIndex) - } - return idx, nil } From 7479ef6470ee27f2f16adc4d00e4b9ff483c25b1 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Wed, 4 Aug 2021 15:45:50 -0400 Subject: [PATCH 4/5] moar cleanup --- pkg/build/gobuild.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/build/gobuild.go b/pkg/build/gobuild.go index d686f51e05..07017ec6c7 100644 --- a/pkg/build/gobuild.go +++ b/pkg/build/gobuild.go @@ -702,7 +702,7 @@ func (g *gobuild) configForImportPath(ip string) Config { return config } -func (g *gobuild) buildOne(ctx context.Context, refStr string, baseRef name.Reference, baseDigest v1.Hash, base v1.Image, platform *v1.Platform) (v1.Image, error) { +func (g *gobuild) buildOne(ctx context.Context, refStr string, base v1.Image, platform *v1.Platform) (v1.Image, error) { ref := newRef(refStr) cf, err := base.ConfigFile() @@ -865,13 +865,13 @@ func (g *gobuild) Build(ctx context.Context, s string) (Result, error) { if !ok { return nil, fmt.Errorf("failed to interpret base as index: %v", base) } - res, err = g.buildAll(ctx, s, baseRef, baseDigest, baseIndex) + res, err = g.buildAll(ctx, s, baseIndex) case types.OCIManifestSchema1, types.DockerManifestSchema2: baseImage, ok := base.(v1.Image) if !ok { return nil, fmt.Errorf("failed to interpret base as image: %v", base) } - res, err = g.buildOne(ctx, s, baseRef, baseDigest, baseImage, nil) + res, err = g.buildOne(ctx, s, baseImage, nil) default: return nil, fmt.Errorf("base image media type: %s", mt) } @@ -895,7 +895,7 @@ func (g *gobuild) Build(ctx context.Context, s string) (Result, error) { } // TODO(#192): Do these in parallel? -func (g *gobuild) buildAll(ctx context.Context, ref string, baseRef name.Reference, baseDigest v1.Hash, baseIndex v1.ImageIndex) (v1.ImageIndex, error) { +func (g *gobuild) buildAll(ctx context.Context, ref string, baseIndex v1.ImageIndex) (v1.ImageIndex, error) { im, err := baseIndex.IndexManifest() if err != nil { return nil, err @@ -917,7 +917,7 @@ func (g *gobuild) buildAll(ctx context.Context, ref string, baseRef name.Referen if err != nil { return nil, err } - img, err := g.buildOne(ctx, ref, baseRef, baseDigest, baseImage, desc.Platform) + img, err := g.buildOne(ctx, ref, baseImage, desc.Platform) if err != nil { return nil, err } From 2fcb7f36acc291a30b68dfabcb43744cd9560744 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Thu, 5 Aug 2021 09:40:55 -0400 Subject: [PATCH 5/5] skip annotations check for images in indexes, these won't be annotated anymore --- pkg/build/gobuild_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/build/gobuild_test.go b/pkg/build/gobuild_test.go index a8b7eef147..8f8211f573 100644 --- a/pkg/build/gobuild_test.go +++ b/pkg/build/gobuild_test.go @@ -343,7 +343,7 @@ func TestGoBuildNoKoData(t *testing.T) { }) } -func validateImage(t *testing.T, img v1.Image, baseLayers int64, creationTime v1.Time) { +func validateImage(t *testing.T, img v1.Image, baseLayers int64, creationTime v1.Time, checkAnnotations bool) { t.Helper() ls, err := img.Layers() @@ -483,6 +483,9 @@ func validateImage(t *testing.T, img v1.Image, baseLayers int64, creationTime v1 }) t.Run("check annotations", func(t *testing.T) { + if !checkAnnotations { + t.Skip("skipping annotations check") + } mf, err := img.Manifest() if err != nil { t.Fatalf("Manifest() = %v", err) @@ -540,7 +543,7 @@ func TestGoBuild(t *testing.T) { t.Fatalf("Build() not an image: %v", result) } - validateImage(t, img, baseLayers, creationTime) + validateImage(t, img, baseLayers, creationTime, true) // Check that rebuilding the image again results in the same image digest. t.Run("check determinism", func(t *testing.T) { @@ -622,7 +625,7 @@ func TestGoBuildIndex(t *testing.T) { if err != nil { t.Fatalf("idx.Image(%s) = %v", desc.Digest, err) } - validateImage(t, img, baseLayers, creationTime) + validateImage(t, img, baseLayers, creationTime, false) } if want, got := images, int64(len(im.Manifests)); want != got {