Skip to content

Commit

Permalink
OSD-23318: Allow hypershift system account to make changes to IDMS/ICSP
Browse files Browse the repository at this point in the history
  • Loading branch information
samanthajayasinghe committed Jun 16, 2024
1 parent b1e1f9b commit 0cc99b8
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 8 deletions.
42 changes: 42 additions & 0 deletions pkg/webhooks/imagecontentpolicies/imagecontentpolicies.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package imagecontentpolicies
import (
"net/http"
"regexp"
"slices"

"github.com/go-logr/logr"
configv1 "github.com/openshift/api/config/v1"
Expand All @@ -22,6 +23,15 @@ 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 (
allowedGroups = []string{
"system:serviceaccounts:openshift-backplane-srep",
}
)

type ImageContentPoliciesWebhook struct {
Expand All @@ -42,6 +52,11 @@ func (w *ImageContentPoliciesWebhook) Authorized(request admission.Request) admi
return admission.Errored(http.StatusBadRequest, err)
}

// Allow system account to change IDMS,ITMS and ICSP for HCP hosted cluster
if w.HypershiftEnabled() && isAllowedUserGroup(request) {
return utils.WebhookResponse(request, true, "")
}

switch request.RequestKind.Kind {
case "ImageDigestMirrorSet":
idms := configv1.ImageDigestMirrorSet{}
Expand All @@ -50,6 +65,11 @@ func (w *ImageContentPoliciesWebhook) Authorized(request admission.Request) admi
return admission.Errored(http.StatusBadRequest, err)
}

// Allow HCP to mirror ECR repos
if w.HypershiftEnabled() && authorizeHCPImageDigestMirrorSet(idms) {
return utils.WebhookResponse(request, true, "")
}

if !authorizeImageDigestMirrorSet(idms) {
w.log.Info("denying ImageDigestMirrorSet", "name", idms.Name)
return utils.WebhookResponse(request, false, WebhookDoc)
Expand Down Expand Up @@ -185,6 +205,18 @@ func authorizeImageDigestMirrorSet(idms configv1.ImageDigestMirrorSet) bool {
return true
}

// authorizeHCPImageDigestMirrorSet should allow an ImageDigestMirrorSet that matches an authorized mirror list
func authorizeHCPImageDigestMirrorSet(idms configv1.ImageDigestMirrorSet) bool {
authorizedECRMirrorsRe := regexp.MustCompile(authorizedECRMirrors)
for _, mirror := range idms.Spec.ImageDigestMirrors {
if authorizedECRMirrorsRe.Match([]byte(mirror.Source)) {
return true
}
}

return false
}

// authorizeImageTagMirrorSet should reject an ImageTagMirrorSet that matches an unauthorized mirror list
func authorizeImageTagMirrorSet(itms configv1.ImageTagMirrorSet) bool {
unauthorizedRepositoryMirrorsRe := regexp.MustCompile(unauthorizedRepositoryMirrors)
Expand All @@ -208,3 +240,13 @@ func authorizeImageContentSourcePolicy(icsp operatorv1alpha1.ImageContentSourceP

return true
}

// isAllowedUserGroup checks if the user or group is allowed to perform the action
func isAllowedUserGroup(request admission.Request) bool {
for _, group := range allowedGroups {
if slices.Contains(request.UserInfo.Groups, group) {
return true
}
}
return false
}
65 changes: 57 additions & 8 deletions pkg/webhooks/imagecontentpolicies/imagecontentpolicies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -380,6 +382,16 @@ func TestImageContentPolicy(t *testing.T) {
gvr: idmsgvr,
allowed: false,
},
{
name: "allowed-creation-idms-for-ecr",
op: admissionv1.Create,
obj: &runtime.RawExtension{
Raw: []byte(fmt.Sprintf(rawImageDigestMirrorSet, "aws_account_id.dkr.ecr.region.amazonaws.com")),
},
gvk: idmsgvk,
gvr: idmsgvr,
allowed: true,
},
{
name: "allowed-creation-itms",
op: admissionv1.Create,
Expand Down Expand Up @@ -472,12 +484,49 @@ func TestImageContentPolicy(t *testing.T) {
gvr: icspgvr,
allowed: false,
},
{
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)
}
Expand Down

0 comments on commit 0cc99b8

Please sign in to comment.