Merge pull request #2522 from treuherz/annotation-per-type

Make multi-type annotation settings match docs

util/buildflags/export.go

@@ -81,42 +81,65 @@ 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+)$`)
+
+	// type followed by optional platform specifier in square brackets
+	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, "=")
 		if !ok {
 			return nil, errors.Errorf("invalid annotation %q, expected key=value", inp)
 		}
 
-		groups := annotationRegexp.FindStringSubmatch(k)
-		if groups == nil {
-			return nil, errors.Errorf("invalid annotation format, expected <type>:<key>=<value>, got %q", inp)
-		}
+		types, key, ok := strings.Cut(k, ":")
+		if !ok {
+			// no types specified, swap Cut outputs
+			key = types
 
-		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)
+			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(types, ",")
+		for _, typeAndPlatform := range typesSplit {
+			groups := annotationTypeRegexp.FindStringSubmatch(typeAndPlatform)
+			if groups == nil {
+				return nil, errors.Errorf(
+					"invalid annotation type %q, expected type and optional platform in square brackets",
+					typeAndPlatform)
 			}
-			ociPlatform = &p
-		}
 
-		ak := exptypes.AnnotationKey{
-			Type:     typ,
-			Platform: ociPlatform,
-			Key:      key,
+			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)
+			}
+
+			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
 		}
-		annotations[ak] = v
+
 	}
 	return annotations, nil
 }

util/buildflags/export_test.go

@@ -0,0 +1,119 @@
+package buildflags
+
+import (
+	"cmp"
+	"slices"
+	"testing"
+
+	"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, err := ParseAnnotations(test.in)
+			if test.wantErr != "" {
+				require.ErrorContains(t, err, test.wantErr)
+			} else {
+				require.NoError(t, err)
+			}
+
+			// Can't compare maps with pointer in their keys, need to extract and sort the map entries
+			wantKVs := entries(test.want)
+			gotKVs := entries(got)
+
+			assert.Equal(t, wantKVs, gotKVs)
+		})
+	}
+}
+
+type kv struct {
+	Key exptypes.AnnotationKey
+	Val string
+}
+
+func entries(in map[exptypes.AnnotationKey]string) []kv {
+	var out []kv
+	for k, v := range in {
+		out = append(out, kv{k, v})
+	}
+
+	sortFunc := func(a, b kv) int { return cmp.Compare(a.Key.String(), b.Key.String()) }
+	slices.SortFunc(out, sortFunc)
+
+	return out
+}