Modify the annotation expansion to take target values for the expande…

…d annotations

* Add host process containers annotation to the list of unsafe operations

Signed-off-by: Kathryn Baldauf <kabaldau@microsoft.com>

internal/oci/annotations.go

@@ -36,10 +36,15 @@ func ProcessAnnotations(ctx context.Context, s *specs.Spec) (err error) {
 	// expand annotations
 	for key, exps := range annotations.AnnotationExpansions {
 		// check if annotation is present
-		if val, ok := s.Annotations[key]; ok {
+		if expVal, ok := s.Annotations[key]; ok {
+			// If the annotation is a bool and is set to false, we will treat that as
+			// equivalent to disabling the expansion.
+			if strings.ToLower(expVal) == "false" {
+				continue
+			}
 			// ideally, some normalization would occur here (ie, "True" -> "true")
 			// but strings may be case-sensitive
-			for _, k := range exps {
+			for k, val := range exps {
 				if v, ok := s.Annotations[k]; ok && val != v {
 					err = ErrAnnotationExpansionConflict
 					log.G(ctx).WithFields(logrus.Fields{

internal/oci/annotations_test.go

@@ -55,24 +55,41 @@ func TestProccessAnnotations_Expansion(t *testing.T) {
 
 	for _, tt := range tests {
 		// test correct expansion
-		for _, v := range []string{"true", "false"} {
-			t.Run(tt.name+"_disable_unsafe_"+v, func(subtest *testing.T) {
-				tt.spec.Annotations = map[string]string{
-					annotations.DisableUnsafeOperations: v,
-				}
+		t.Run(tt.name+"_disable_unsafe", func(subtest *testing.T) {
+			tt.spec.Annotations = map[string]string{
+				annotations.DisableUnsafeOperations: "true",
+			}
 
-				err := ProcessAnnotations(ctx, &tt.spec)
-				if err != nil {
-					subtest.Fatalf("could not update spec from options: %v", err)
+			err := ProcessAnnotations(ctx, &tt.spec)
+			if err != nil {
+				subtest.Fatalf("could not update spec from options: %v", err)
+			}
+			for k := range annotations.AnnotationExpansions[annotations.DisableUnsafeOperations] {
+				expected := annotations.AnnotationExpansions[annotations.DisableUnsafeOperations][k]
+				if vv := tt.spec.Annotations[k]; vv != expected {
+					subtest.Fatalf("annotation %q was incorrectly expanded to %q, expected %q", k, vv, expected)
 				}
+			}
+		})
 
-				for _, k := range annotations.AnnotationExpansions[annotations.DisableUnsafeOperations] {
-					if vv := tt.spec.Annotations[k]; vv != v {
-						subtest.Fatalf("annotation %q was incorrectly expanded to %q, expected %q", k, vv, v)
-					}
-				}
-			})
-		}
+		// test when disable unsafe operations is set to false
+		t.Run(tt.name+"_disable_unsafe_off", func(subtest *testing.T) {
+			tt.spec.Annotations = map[string]string{
+				annotations.DisableUnsafeOperations: "false",
+				annotations.HostProcessContainer:    "true",
+			}
+
+			err := ProcessAnnotations(ctx, &tt.spec)
+			if err != nil {
+				subtest.Fatalf("could not update spec from options: %v", err)
+			}
+
+			// When disable unsafe operations is set to true, annotations.HostProcessContainer, check
+			// that we can set annotations.HostProcessContainer when disable unsafe operations is off.
+			if vv, ok := tt.spec.Annotations[annotations.HostProcessContainer]; ok && vv != "true" {
+				subtest.Fatalf("annotation %q was incorrectly expanded to %q, expected true", annotations.WCOWDisableGMSA, vv)
+			}
+		})
 
 		// test errors raised on conflict
 		t.Run(tt.name+"_disable_unsafe_error", func(subtest *testing.T) {

pkg/annotations/annotations.go

@@ -314,13 +314,13 @@ const (
 )
 
 // AnnotationExpansions maps annotations that will be expanded into an array of
-// other annotations. The expanded annotations will have the same value as the
-// original. It is an error for the expansions to already exist and have a value
-// that differs from the original.
-var AnnotationExpansions = map[string][]string{
+// other annotations and their values. It is an error for the expansions to already
+// exist and have a value that differs from the value set here.
+var AnnotationExpansions = map[string]map[string]string{
 	DisableUnsafeOperations: {
-		WCOWDisableGMSA,
-		DisableWritableFileShares,
-		VSMBNoDirectMap,
+		WCOWDisableGMSA:           "true",
+		DisableWritableFileShares: "true",
+		VSMBNoDirectMap:           "true",
+		HostProcessContainer:      "false",
 	},
 }