Add hvsock service config annotation

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>

internal/annotations/annotations.go

@@ -1,12 +1,35 @@
-// This package contains annotations that are not exposed to end users and mainly for
-// testing and debugging purposes.
+// This package contains annotations that are not exposed to end users and are either:
+//  1. intended for testing and debugging purposes; or
+//  2. rely on undocumented Windows APIs that are subject to change.
 //
 // Do not rely on these annotations to customize production workload behavior.
 package annotations
 
 // uVM specific annotations
 
 const (
+	// UVMHyperVSocketConfigPrefix is the prefix of an annotation to map a [hyper-v socket] service GUID
+	// to a JSON-encoded string of its [configuration].
+	//
+	// The service GUID should be part of the annotation.
+	// For example:
+	//
+	// 	"io.microsoft.virtualmachine.hv-socket.service-table.00000000-0000-0000-0000-000000000000" =
+	// 		"{\"AllowWildcardBinds\": true, \"BindSecurityDescriptor\": \"D:P(A;;FA;;;WD)\"}"
+	//
+	// If multiple annotations with the same GUID are present, then it is undefined which configuration will
+	// take precedence.
+	//
+	// For LCOW, it is preferred to use [ExtraVSockPorts], as vsock ports specified there will take precedence.
+	//
+	// # Warning
+	//
+	// Setting the configuration for special services (e.g., the GCS) can cause catastrophic failures.
+	//
+	// [hyper-v socket]: https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/user-guide/make-integration-service
+	// [configuration]: https://learn.microsoft.com/en-us/virtualization/api/hcs/schemareference#HvSocketServiceConfig
+	UVMHyperVSocketConfigPrefix = "io.microsoft.virtualmachine.hv-socket.service-table."
+
 	// AdditionalRegistryValues specifies additional registry keys and their values to set in the WCOW UVM.
 	// The format is a JSON-encoded string of an array containing [HCS RegistryValue] objects.
 	//

internal/oci/annotations.go

@@ -4,13 +4,15 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
+	"slices"
 	"strconv"
 	"strings"
 
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
-	"golang.org/x/exp/slices"
 
+	"github.com/Microsoft/go-winio/pkg/guid"
 	iannotations "github.com/Microsoft/hcsshim/internal/annotations"
 	hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2"
 	"github.com/Microsoft/hcsshim/internal/log"
@@ -66,6 +68,7 @@ func ParseAnnotationsDisableGMSA(ctx context.Context, s *specs.Spec) bool {
 }
 
 // parseAdditionalRegistryValues extracts the additional registry values to set from annotations.
+//
 // Like the [parseAnnotation*] functions, this logs errors but does not return them.
 func parseAdditionalRegistryValues(ctx context.Context, a map[string]string) []hcsschema.RegistryValue {
 	// rather than have users deal with nil vs []hcsschema.RegistryValue as returns, always
@@ -81,7 +84,7 @@ func parseAdditionalRegistryValues(ctx context.Context, a map[string]string) []h
 
 	t := []hcsschema.RegistryValue{}
 	if err := json.Unmarshal([]byte(v), &t); err != nil {
-		logAnnotationParseError(ctx, k, v, "JSON string", err)
+		logAnnotationValueParseError(ctx, k, v, fmt.Sprintf("%T", t), err)
 		return []hcsschema.RegistryValue{}
 	}
 
@@ -182,20 +185,63 @@ func parseAdditionalRegistryValues(ctx context.Context, a map[string]string) []h
 	return slices.Clip(rvs)
 }
 
+// parseHVSocketServiceTable extracts any additional Hyper-V socket service configurations from annotations.
+//
+// Like the [parseAnnotation*] functions, this logs errors but does not return them.
+func parseHVSocketServiceTable(ctx context.Context, a map[string]string) map[string]hcsschema.HvSocketServiceConfig {
+	sc := make(map[string]hcsschema.HvSocketServiceConfig)
+	// TODO(go1.23) use range over functions to implement a functional `filter | map $ a`
+	for k, v := range a {
+		sGUID, found := strings.CutPrefix(k, iannotations.UVMHyperVSocketConfigPrefix)
+		if !found {
+			continue
+		}
+
+		entry := log.G(ctx).WithFields(logrus.Fields{
+			logfields.OCIAnnotation: k,
+			logfields.Value:         v,
+			"guid":                  sGUID,
+		})
+
+		g, err := guid.FromString(sGUID)
+		if err != nil {
+			entry.WithError(err).Warn("invalid GUID string for Hyper-V socket service configuration annotation")
+			continue
+		}
+		sGUID = g.String() // overwrite the GUID string to standardize format (capitalization)
+
+		conf := hcsschema.HvSocketServiceConfig{}
+		if err := json.Unmarshal([]byte(v), &conf); err != nil {
+			logAnnotationValueParseError(ctx, k, v, fmt.Sprintf("%T", conf), err)
+			continue
+		}
+
+		if _, found := sc[sGUID]; found {
+			entry.WithFields(logrus.Fields{
+				"guid": sGUID,
+			}).Warn("overwritting existing Hyper-V socket service configuration")
+		}
+
+		if entry.Logger.IsLevelEnabled(logrus.TraceLevel) {
+			entry.WithField("configuration", log.Format(ctx, conf)).Trace("found Hyper-V socket service configuration annotation")
+		}
+		sc[sGUID] = conf
+	}
+
+	return sc
+}
+
 // general annotation parsing
 
 // ParseAnnotationsBool searches `a` for `key` and if found verifies that the
 // value is `true` or `false` in any case. If `key` is not found returns `def`.
 func ParseAnnotationsBool(ctx context.Context, a map[string]string, key string, def bool) bool {
 	if v, ok := a[key]; ok {
-		switch strings.ToLower(v) {
-		case "true":
-			return true
-		case "false":
-			return false
-		default:
-			logAnnotationParseError(ctx, key, v, logfields.Bool, nil)
+		b, err := strconv.ParseBool(v)
+		if err == nil {
+			return b
 		}
+		logAnnotationValueParseError(ctx, key, v, logfields.Bool, err)
 	}
 	return def
 }
@@ -206,17 +252,11 @@ func ParseAnnotationsBool(ctx context.Context, a map[string]string, key string,
 // the value they point at.
 func ParseAnnotationsNullableBool(ctx context.Context, a map[string]string, key string) *bool {
 	if v, ok := a[key]; ok {
-		switch strings.ToLower(v) {
-		case "true":
-			_bool := true
-			return &_bool
-		case "false":
-			_bool := false
-			return &_bool
-		default:
-			err := errors.New("boolean fields must be 'true', 'false', or not set")
-			logAnnotationParseError(ctx, key, v, logfields.Bool, err)
+		b, err := strconv.ParseBool(v)
+		if err == nil {
+			return &b
 		}
+		logAnnotationValueParseError(ctx, key, v, logfields.Bool, err)
 	}
 	return nil
 }
@@ -230,7 +270,7 @@ func ParseAnnotationsInt32(ctx context.Context, a map[string]string, key string,
 			v := int32(countu)
 			return v
 		}
-		logAnnotationParseError(ctx, key, v, logfields.Int32, err)
+		logAnnotationValueParseError(ctx, key, v, logfields.Int32, err)
 	}
 	return def
 }
@@ -244,7 +284,7 @@ func ParseAnnotationsUint32(ctx context.Context, a map[string]string, key string
 			v := uint32(countu)
 			return v
 		}
-		logAnnotationParseError(ctx, key, v, logfields.Uint32, err)
+		logAnnotationValueParseError(ctx, key, v, logfields.Uint32, err)
 	}
 	return def
 }
@@ -257,15 +297,15 @@ func ParseAnnotationsUint64(ctx context.Context, a map[string]string, key string
 		if err == nil {
 			return countu
 		}
-		logAnnotationParseError(ctx, key, v, logfields.Uint64, err)
+		logAnnotationValueParseError(ctx, key, v, logfields.Uint64, err)
 	}
 	return def
 }
 
-// ParseAnnotationCommaSeparated searches `annotations` for `annotation` corresponding to a
-// list of comma separated strings
-func ParseAnnotationCommaSeparatedUint32(ctx context.Context, annotations map[string]string, annotation string, def []uint32) []uint32 {
-	cs, ok := annotations[annotation]
+// ParseAnnotationCommaSeparated searches `a` for `annotation` corresponding to a
+// list of comma separated strings.
+func ParseAnnotationCommaSeparatedUint32(_ context.Context, a map[string]string, key string, def []uint32) []uint32 {
+	cs, ok := a[key]
 	if !ok || cs == "" {
 		return def
 	}
@@ -289,18 +329,18 @@ func ParseAnnotationsString(a map[string]string, key string, def string) string
 	return def
 }
 
-// ParseAnnotationCommaSeparated searches `annotations` for `annotation` corresponding to a
-// list of comma separated strings
-func ParseAnnotationCommaSeparated(annotation string, annotations map[string]string) []string {
-	cs, ok := annotations[annotation]
+// ParseAnnotationCommaSeparated searches `a` for `key` corresponding to a
+// list of comma separated strings.
+func ParseAnnotationCommaSeparated(key string, a map[string]string) []string {
+	cs, ok := a[key]
 	if !ok || cs == "" {
 		return nil
 	}
 	results := strings.Split(cs, ",")
 	return results
 }
 
-func logAnnotationParseError(ctx context.Context, k, v, et string, err error) {
+func logAnnotationValueParseError(ctx context.Context, k, v, et string, err error) {
 	entry := log.G(ctx).WithFields(logrus.Fields{
 		logfields.OCIAnnotation: k,
 		logfields.Value:         v,
@@ -309,5 +349,5 @@ func logAnnotationParseError(ctx context.Context, k, v, et string, err error) {
 	if err != nil {
 		entry = entry.WithError(err)
 	}
-	entry.Warning("annotation could not be parsed")
+	entry.Warning("annotation value could not be parsed")
 }

internal/oci/annotations_test.go

@@ -1,9 +1,12 @@
 package oci
 
 import (
+	"bytes"
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
+	"maps"
 	"strings"
 	"testing"
 
@@ -51,6 +54,8 @@ func TestProccessAnnotations_Expansion(t *testing.T) {
 	}
 
 	for _, tt := range tests {
+		tt := tt // TODO (go1.22) remove this since loop aliasing is avoided (https://go.dev/ref/spec#Go_1.22)
+
 		// test correct expansion
 		for _, v := range []string{"true", "false"} {
 			t.Run(tt.name+"_disable_unsafe_"+v, func(subtest *testing.T) {
@@ -225,7 +230,9 @@ func TestParseAdditionalRegistryValues(t *testing.T) {
 			t.Logf("registry values:\n%s", tt.give)
 			v := strings.ReplaceAll(tt.give, "\n", "")
 			rvs := parseAdditionalRegistryValues(ctx, map[string]string{
-				iannotations.AdditionalRegistryValues: v,
+				"some-random-annotation":                                "random",
+				"not-microsoft.virtualmachine.wcow.additional-reg-keys": "this is fake",
+				iannotations.AdditionalRegistryValues:                   v,
 			})
 			want := tt.want
 			if want == nil {
@@ -237,3 +244,112 @@ func TestParseAdditionalRegistryValues(t *testing.T) {
 		})
 	}
 }
+
+func TestParseHVSocketServiceTable(t *testing.T) {
+	ctx := context.Background()
+
+	toString := func(t *testing.T, v hcsschema.HvSocketServiceConfig) string {
+		t.Helper()
+
+		buf := &bytes.Buffer{}
+		enc := json.NewEncoder(buf)
+		enc.SetEscapeHTML(false)
+		enc.SetIndent("", "")
+
+		if err := enc.Encode(v); err != nil {
+			t.Fatalf("encode %v to JSON: %v", v, err)
+		}
+
+		return strings.TrimSpace(buf.String())
+	}
+
+	g1 := "0b52781f-b24d-5685-ddf6-69830ed40ec3"
+	g2 := "00000000-0000-0000-0000-000000000000"
+
+	defaultConfig := hcsschema.HvSocketServiceConfig{
+		AllowWildcardBinds:     true,
+		BindSecurityDescriptor: "D:P(A;;FA;;;WD)",
+	}
+	defaultConfigStr := toString(t, defaultConfig)
+
+	disabledConfig := hcsschema.HvSocketServiceConfig{
+		Disabled: true,
+	}
+	disabledConfigStr := toString(t, disabledConfig)
+
+	for _, tt := range []struct {
+		name string
+		give map[string]string
+		want map[string]hcsschema.HvSocketServiceConfig
+	}{
+		{
+			name: "empty",
+		},
+		{
+			name: "single",
+			give: map[string]string{
+				iannotations.UVMHyperVSocketConfigPrefix + g1: defaultConfigStr,
+			},
+			want: map[string]hcsschema.HvSocketServiceConfig{
+				g1: defaultConfig,
+			},
+		},
+		{
+			name: "invalid guid",
+			give: map[string]string{
+				iannotations.UVMHyperVSocketConfigPrefix + "not-a-guid": defaultConfigStr,
+			},
+		},
+		{
+			name: "invalid config",
+			give: map[string]string{
+				iannotations.UVMHyperVSocketConfigPrefix + g1: `["not", "a", "valid", "config"]`,
+			},
+		},
+		{
+			name: "override",
+			give: map[string]string{
+				iannotations.UVMHyperVSocketConfigPrefix + g1:                  defaultConfigStr,
+				iannotations.UVMHyperVSocketConfigPrefix + strings.ToUpper(g1): defaultConfigStr,
+			},
+			want: map[string]hcsschema.HvSocketServiceConfig{
+				g1: defaultConfig,
+			},
+		},
+		{
+			name: "multiple",
+			give: map[string]string{
+				iannotations.UVMHyperVSocketConfigPrefix + strings.ToUpper(g1): defaultConfigStr,
+				iannotations.UVMHyperVSocketConfigPrefix + g2:                  disabledConfigStr,
+
+				iannotations.UVMHyperVSocketConfigPrefix + g1:           `["not", "a", "valid", "config"]`,
+				iannotations.UVMHyperVSocketConfigPrefix + "not-a-guid": defaultConfigStr,
+				"also.not-a-guid": disabledConfigStr,
+			},
+			want: map[string]hcsschema.HvSocketServiceConfig{
+				g1: defaultConfig,
+				g2: disabledConfig,
+			},
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			annots := map[string]string{
+				"some-random-annotation":                               "random",
+				"io.microsoft.virtualmachine.hv-socket.service-table":  "should be ignored",
+				"not-microsoft.virtualmachine.hv-socket.service-table": "this is fake",
+			}
+			maps.Copy(annots, tt.give)
+			t.Logf("annotations:\n%v", annots)
+
+			rvs := parseHVSocketServiceTable(ctx, annots)
+			t.Logf("got %v", rvs)
+			want := tt.want
+			if want == nil {
+				want = map[string]hcsschema.HvSocketServiceConfig{}
+			}
+			if diff := cmp.Diff(want, rvs); diff != "" {
+				t.Fatal(diff)
+			}
+		})
+	}
+}