From b00001d8ac6bccd75c05e6a6a2041deb8f93bad1 Mon Sep 17 00:00:00 2001 From: Eli Treuherz Date: Mon, 10 Jun 2024 19:56:40 +0100 Subject: [PATCH] Make multi-type annotation settings match docs The Docker docs in multiple places describe passing an annotation at the command line like "index,manifest:com.example.name=my-cool-image", and say that this will result in the annotation being applied to both the index and the manifest. It doesn't seem like this was actually implemented, and instead it just results in an annotation key with "index,manifest:" at the beginning being applied to the manifest. This change splits the part of the key before the colon by comma, and creates an annotation for each type/platform given, so the implementation should now match the docs. Signed-off-by: Eli Treuherz --- go.mod | 2 +- util/buildflags/export.go | 53 +++++++++------ util/buildflags/export_test.go | 118 +++++++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 20 deletions(-) create mode 100644 util/buildflags/export_test.go diff --git a/go.mod b/go.mod index 7244a24e..14442e0b 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/gofrs/flock v0.8.1 github.com/gogo/protobuf v1.3.2 github.com/golang/protobuf v1.5.4 + github.com/google/go-cmp v0.6.0 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/google/uuid v1.6.0 github.com/hashicorp/go-cty-funcs v0.0.0-20230405223818-a090f58aa992 @@ -98,7 +99,6 @@ require ( github.com/go-viper/mapstructure/v2 v2.0.0 // indirect github.com/gogo/googleapis v1.4.1 // indirect github.com/google/gnostic-models v0.6.8 // indirect - github.com/google/go-cmp v0.6.0 // indirect github.com/google/gofuzz v1.2.0 // indirect github.com/gorilla/mux v1.8.0 // indirect github.com/gorilla/websocket v1.5.0 // indirect diff --git a/util/buildflags/export.go b/util/buildflags/export.go index fb66d2a6..a9a2ffc6 100644 --- a/util/buildflags/export.go +++ b/util/buildflags/export.go @@ -81,7 +81,8 @@ func ParseExports(inp []string) ([]*controllerapi.ExportEntry, error) { func ParseAnnotations(inp []string) (map[exptypes.AnnotationKey]string, error) { // TODO: use buildkit's annotation parser once it supports setting custom prefix and ":" separator - annotationRegexp := regexp.MustCompile(`^(?:([a-z-]+)(?:\[([A-Za-z0-9_/-]+)\])?:)?(\S+)$`) + annotationRegexp := regexp.MustCompile(`^((?:[a-z-]+(?:\[[A-Za-z0-9_/-]+\])?)(?:,[a-z-]+(?:\[[A-Za-z0-9_/-]+\])?)*:)?(\S+)$`) + annotationTypeRegexp := regexp.MustCompile(`^([a-z-]+)(?:\[([A-Za-z0-9_/-]+)\])?$`) annotations := make(map[exptypes.AnnotationKey]string) for _, inp := range inp { k, v, ok := strings.Cut(inp, "=") @@ -94,29 +95,43 @@ func ParseAnnotations(inp []string) (map[exptypes.AnnotationKey]string, error) { return nil, errors.Errorf("invalid annotation format, expected :=, got %q", inp) } - typ, platform, key := groups[1], groups[2], groups[3] - switch typ { - case "": - case exptypes.AnnotationIndex, exptypes.AnnotationIndexDescriptor, exptypes.AnnotationManifest, exptypes.AnnotationManifestDescriptor: - default: - return nil, errors.Errorf("unknown annotation type %q", typ) + types, key := groups[1], groups[2] + + if types == "" { + ak := exptypes.AnnotationKey{Key: key} + annotations[ak] = v + continue } - var ociPlatform *ocispecs.Platform - if platform != "" { - p, err := platforms.Parse(platform) - if err != nil { - return nil, errors.Wrapf(err, "invalid platform %q", platform) + typesSplit := strings.Split(strings.TrimSuffix(types, ":"), ",") + for _, typeAndPlatform := range typesSplit { + groups := annotationTypeRegexp.FindStringSubmatch(typeAndPlatform) + typ, platform := groups[1], groups[2] + + switch typ { + case "": + case exptypes.AnnotationIndex, exptypes.AnnotationIndexDescriptor, exptypes.AnnotationManifest, exptypes.AnnotationManifestDescriptor: + default: + return nil, errors.Errorf("unknown annotation type %q", typ) } - ociPlatform = &p + + var ociPlatform *ocispecs.Platform + if platform != "" { + p, err := platforms.Parse(platform) + if err != nil { + return nil, errors.Wrapf(err, "invalid platform %q", platform) + } + ociPlatform = &p + } + + ak := exptypes.AnnotationKey{ + Type: typ, + Platform: ociPlatform, + Key: key, + } + annotations[ak] = v } - ak := exptypes.AnnotationKey{ - Type: typ, - Platform: ociPlatform, - Key: key, - } - annotations[ak] = v } return annotations, nil } diff --git a/util/buildflags/export_test.go b/util/buildflags/export_test.go new file mode 100644 index 00000000..b39ab130 --- /dev/null +++ b/util/buildflags/export_test.go @@ -0,0 +1,118 @@ +package buildflags + +import ( + "cmp" + "slices" + "testing" + + gocmp "github.com/google/go-cmp/cmp" + "github.com/moby/buildkit/exporter/containerimage/exptypes" + ocispecs "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseAnnotations(t *testing.T) { + tests := []struct { + name string + in []string + want map[exptypes.AnnotationKey]string + wantErr string + }{ + { + name: "basic", + in: []string{"a=b"}, + want: map[exptypes.AnnotationKey]string{ + {Key: "a"}: "b", + }, + }, + { + name: "reverse-DNS key", + in: []string{"com.example=a"}, + want: map[exptypes.AnnotationKey]string{ + {Key: "com.example"}: "a", + }, + }, + { + name: "specify type", + in: []string{"manifest:com.example=a"}, + want: map[exptypes.AnnotationKey]string{ + {Type: "manifest", Key: "com.example"}: "a", + }, + }, + { + name: "specify bad type", + in: []string{"bad:com.example=a"}, + wantErr: "unknown annotation type", + }, + { + name: "specify type and platform", + in: []string{"manifest[plat/form]:com.example=a"}, + want: map[exptypes.AnnotationKey]string{ + { + Type: "manifest", + Platform: &ocispecs.Platform{ + OS: "plat", + Architecture: "form", + }, + Key: "com.example", + }: "a", + }, + }, + { + name: "specify multiple types", + in: []string{"index,manifest:com.example=a"}, + want: map[exptypes.AnnotationKey]string{ + {Type: "index", Key: "com.example"}: "a", + {Type: "manifest", Key: "com.example"}: "a", + }, + }, + { + name: "specify multiple types and platform", + in: []string{"index,manifest[plat/form]:com.example=a"}, + want: map[exptypes.AnnotationKey]string{ + {Type: "index", Key: "com.example"}: "a", + { + Type: "manifest", + Platform: &ocispecs.Platform{ + OS: "plat", + Architecture: "form", + }, + Key: "com.example", + }: "a", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, gotErr := ParseAnnotations(test.in) + if test.wantErr != "" { + require.ErrorContains(t, gotErr, test.wantErr) + } else { + assert.NoError(t, gotErr) + } + + // Can't compare maps with pointer in their keys, need to extract and sort the map entries + type kv struct { + Key exptypes.AnnotationKey + Val string + } + var wantKVs, gotKVs []kv + for k, v := range test.want { + wantKVs = append(wantKVs, kv{k, v}) + } + for k, v := range got { + gotKVs = append(gotKVs, kv{k, v}) + } + + sortFunc := func(a, b kv) int { return cmp.Compare(a.Key.String(), b.Key.String()) } + slices.SortFunc(wantKVs, sortFunc) + slices.SortFunc(gotKVs, sortFunc) + + if diff := gocmp.Diff(wantKVs, gotKVs); diff != "" { + t.Error(diff) + } + }) + } +}