OSD-23318: Allow hypershift system account to make changes to IDMS/ICSP

openshift · Jun 6, 2024 · f59e5c0 · f59e5c0
1 parent 81099c8
commit f59e5c0
Show file tree

Hide file tree

Showing 2 changed files with 123 additions and 9 deletions.
diff --git a/pkg/webhooks/imagecontentpolicies/imagecontentpolicies.go b/pkg/webhooks/imagecontentpolicies/imagecontentpolicies.go
@@ -1,8 +1,10 @@
 package imagecontentpolicies
 
 import (
+	"fmt"
 	"net/http"
 	"regexp"
+	"slices"
 
 	"github.com/go-logr/logr"
 	configv1 "github.com/openshift/api/config/v1"
@@ -22,6 +24,18 @@ const (
 	// Only registry.redhat.io exactly is blocked, while all other contained regexes
 	// follow a similar pattern, i.e. rejecting quay.io or quay.io/.*
 	unauthorizedRepositoryMirrors = `(^registry\.redhat\.io$|^quay\.io(/.*)?$|^registry\.access\.redhat\.com(/.*)?)`
+
+	//Allow Hypershift hosted clusters to mirror ECR
+	authorizedECRMirrors = `(^(.*).dkr.ecr.(.*).amazonaws.com)`
+)
+
+var (
+	allowedUsers = []string{
+		"backplane-cluster-admin",
+	}
+	allowedGroups = []string{
+		"system:serviceaccounts:openshift-backplane-srep",
+	}
 )
 
 type ImageContentPoliciesWebhook struct {
@@ -42,6 +56,11 @@ func (w *ImageContentPoliciesWebhook) Authorized(request admission.Request) admi
 		return admission.Errored(http.StatusBadRequest, err)
 	}
 
+	// Allow system account to change IDMS,ITMS and ICSP
+	if isAllowedUserGroup(request) {
+		return utils.WebhookResponse(request, true, "")
+	}
+
 	switch request.RequestKind.Kind {
 	case "ImageDigestMirrorSet":
 		idms := configv1.ImageDigestMirrorSet{}
@@ -72,6 +91,11 @@ func (w *ImageContentPoliciesWebhook) Authorized(request admission.Request) admi
 			return admission.Errored(http.StatusBadRequest, err)
 		}
 
+		// Allow HCP to mirror ECR repos
+		if w.HypershiftEnabled() && authorizeHCPImageContentSourcePolicy(icsp) {
+			return utils.WebhookResponse(request, true, "")
+		}
+
 		if !authorizeImageContentSourcePolicy(icsp) {
 			w.log.Info("denying ImageContentSourcePolicy", "name", icsp.Name)
 			return utils.WebhookResponse(request, false, WebhookDoc)
@@ -162,7 +186,11 @@ func (w *ImageContentPoliciesWebhook) Doc() string {
 }
 
 func (w *ImageContentPoliciesWebhook) SyncSetLabelSelector() metav1.LabelSelector {
-	return utils.DefaultLabelSelector()
+	return metav1.LabelSelector{
+		MatchLabels: map[string]string{
+			"api.openshift.com/managed": "true",
+		},
+	}
 }
 
 func (w *ImageContentPoliciesWebhook) ClassicEnabled() bool {
@@ -208,3 +236,29 @@ func authorizeImageContentSourcePolicy(icsp operatorv1alpha1.ImageContentSourceP
 
 	return true
 }
+
+// authorizeHCPImageContentSourcePolicy should allow an ImageContentSourcePolicy that matches an authorized mirror list
+func authorizeHCPImageContentSourcePolicy(icsp operatorv1alpha1.ImageContentSourcePolicy) bool {
+	authorizedECRMirrorsRe := regexp.MustCompile(authorizedECRMirrors)
+	for _, mirror := range icsp.Spec.RepositoryDigestMirrors {
+		if authorizedECRMirrorsRe.Match([]byte(mirror.Source)) {
+			return true
+		}
+	}
+
+	return false
+}
+
+// isAllowedUserGroup checks if the user or group is allowed to perform the action
+func isAllowedUserGroup(request admission.Request) bool {
+	fmt.Print(request.UserInfo)
+	if slices.Contains(allowedUsers, request.UserInfo.Username) {
+		return true
+	}
+	for _, group := range allowedGroups {
+		if slices.Contains(request.UserInfo.Groups, group) {
+			return true
+		}
+	}
+	return false
+}
diff --git a/pkg/webhooks/imagecontentpolicies/imagecontentpolicies_test.go b/pkg/webhooks/imagecontentpolicies/imagecontentpolicies_test.go
@@ -326,13 +326,15 @@ func TestImageContentPolicy(t *testing.T) {
 		Resource: "imagecontentsourcepolicies",
 	}
 	tests := []struct {
-		name    string
-		op      admissionv1.Operation
-		gvk     metav1.GroupVersionKind
-		gvr     metav1.GroupVersionResource
-		obj     *runtime.RawExtension
-		oldObj  *runtime.RawExtension
-		allowed bool
+		name       string
+		op         admissionv1.Operation
+		gvk        metav1.GroupVersionKind
+		gvr        metav1.GroupVersionResource
+		obj        *runtime.RawExtension
+		oldObj     *runtime.RawExtension
+		username   string
+		userGroups []string
+		allowed    bool
 	}{
 		{
 			name: "allowed-creation-idms",
@@ -472,12 +474,70 @@ func TestImageContentPolicy(t *testing.T) {
 			gvr:     icspgvr,
 			allowed: false,
 		},
+		{
+			name: "allowed-creation-icsp-for-ecr",
+			op:   admissionv1.Create,
+			obj: &runtime.RawExtension{
+				Raw: []byte(fmt.Sprintf(rawImageContentSourcePolicy, "aws_account_id.dkr.ecr.region.amazonaws.com")),
+			},
+			gvk:     icspgvk,
+			gvr:     icspgvr,
+			allowed: true,
+		},
+		{
+			name: "authorized-users-can-create-icsp",
+			op:   admissionv1.Create,
+			obj: &runtime.RawExtension{
+				Raw: []byte(fmt.Sprintf(rawImageContentSourcePolicy, "quay.io")),
+			},
+			gvk:      icspgvk,
+			gvr:      icspgvr,
+			username: "backplane-cluster-admin",
+			allowed:  true,
+		},
+		{
+			name: "unauthorized-users-can-not-create-icsp",
+			op:   admissionv1.Create,
+			obj: &runtime.RawExtension{
+				Raw: []byte(fmt.Sprintf(rawImageContentSourcePolicy, "quay.io")),
+			},
+			gvk:      icspgvk,
+			gvr:      icspgvr,
+			username: "no-auth-user",
+			allowed:  false,
+		},
+		{
+			name: "authorized-service-account-can-create-icsp",
+			op:   admissionv1.Create,
+			obj: &runtime.RawExtension{
+				Raw: []byte(fmt.Sprintf(rawImageContentSourcePolicy, "quay.io")),
+			},
+			gvk:        icspgvk,
+			gvr:        icspgvr,
+			userGroups: []string{"system:serviceaccounts:openshift-backplane-srep"},
+			allowed:    true,
+		},
+		{
+			name: "unauthorized-service-account-can-not-create-icsp",
+			op:   admissionv1.Create,
+			obj: &runtime.RawExtension{
+				Raw: []byte(fmt.Sprintf(rawImageContentSourcePolicy, "quay.io")),
+			},
+			gvk:        icspgvk,
+			gvr:        icspgvr,
+			userGroups: []string{"system:unauthenticated"},
+			allowed:    false,
+		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			hook := NewWebhook()
-			req, err := testutils.CreateHTTPRequest(hook.GetURI(), test.name, test.gvk, test.gvr, test.op, "", []string{}, "", test.obj, test.oldObj)
+			req, err := testutils.CreateHTTPRequest(
+				hook.GetURI(), test.name, test.gvk, test.gvr,
+				test.op, test.username, test.userGroups, "",
+				test.obj, test.oldObj,
+			)
 			if err != nil {
 				t.Errorf("failed to create test HTTP request: %v", err)
 			}