image: mutate group admission attributes to ensure grouped resources are captured

Author: mfojtik

pkg/api/latest/latest.go

@@ -55,6 +55,16 @@ func OriginLegacyKind(gvk unversioned.GroupVersionKind) bool {
 	return OriginKind(gvk) && gvk.Group == ""
 }
 
+// IsOriginAPIGroup returns true if the provided group name belongs to Origin API.
+func IsOriginAPIGroup(groupName string) bool {
+	for _, v := range Versions {
+		if v.Group == groupName {
+			return true
+		}
+	}
+	return false
+}
+
 // IsKindInAnyOriginGroup returns true if OpenShift owns the kind described in any apiVersion.
 // TODO: this may not work once we divide builds/deployments/images into their own API groups
 func IsKindInAnyOriginGroup(kind string) bool {

pkg/image/admission/imagepolicy/imagepolicy.go

@@ -16,6 +16,7 @@ import (
 	clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	"k8s.io/kubernetes/pkg/util/sets"
 
+	"github.com/openshift/origin/pkg/api/latest"
 	"github.com/openshift/origin/pkg/api/meta"
 	"github.com/openshift/origin/pkg/client"
 	oadmission "github.com/openshift/origin/pkg/cmd/server/admission"
@@ -124,6 +125,34 @@ func (a *imagePolicyPlugin) Validate() error {
 	return nil
 }
 
+// mutateAttributesToLegacyResources mutates the admission attributes in a way where the
+// Origin API groups are converted to "legacy" or "core" group.
+// This provides a backward compatibility with existing configurations and also closes the
+// hole where clients might bypass the admission by using API group endpoint and API group
+// resource instead of legacy one.
+func mutateAttributesToLegacyResources(attr admission.Attributes) admission.Attributes {
+	resource := attr.GetResource()
+	if len(resource.Group) > 0 && latest.IsOriginAPIGroup(resource.Group) {
+		resource.Group = ""
+	}
+	kind := attr.GetKind()
+	if len(kind.Group) > 0 && latest.IsOriginAPIGroup(kind.Group) {
+		kind.Group = ""
+	}
+	attrs := admission.NewAttributesRecord(
+		attr.GetObject(),
+		attr.GetOldObject(),
+		attr.GetKind(),
+		attr.GetNamespace(),
+		attr.GetName(),
+		resource,
+		attr.GetSubresource(),
+		attr.GetOperation(),
+		attr.GetUserInfo(),
+	)
+	return attrs
+}
+
 // Admit attempts to apply the image policy to the incoming resource.
 func (a *imagePolicyPlugin) Admit(attr admission.Attributes) error {
 	switch attr.GetOperation() {
@@ -138,27 +167,31 @@ func (a *imagePolicyPlugin) Admit(attr admission.Attributes) error {
 		return nil
 	}
 
-	gr := attr.GetResource().GroupResource()
+	newAttr := mutateAttributesToLegacyResources(attr)
+
+	// This will convert any non-legacy Origin resource to a legacy resource, so specifying
+	// a 'builds.build.openshift.io' is converted to 'builds'.
+	gr := newAttr.GetResource().GroupResource()
 	if !a.accepter.Covers(gr) {
 		return nil
 	}
 
-	m, err := meta.GetImageReferenceMutator(attr.GetObject())
+	m, err := meta.GetImageReferenceMutator(newAttr.GetObject())
 	if err != nil {
-		return apierrs.NewForbidden(gr, attr.GetName(), fmt.Errorf("unable to apply image policy against objects of type %T: %v", attr.GetObject(), err))
+		return apierrs.NewForbidden(gr, newAttr.GetName(), fmt.Errorf("unable to apply image policy against objects of type %T: %v", newAttr.GetObject(), err))
 	}
 
 	// load exclusion rules from the namespace cache
 	var excluded sets.String
-	if ns := attr.GetNamespace(); len(ns) > 0 {
+	if ns := newAttr.GetNamespace(); len(ns) > 0 {
 		if ns, err := a.projectCache.GetNamespace(ns); err == nil {
 			if value := ns.Annotations[api.IgnorePolicyRulesAnnotation]; len(value) > 0 {
 				excluded = sets.NewString(strings.Split(value, ",")...)
 			}
 		}
 	}
 
-	if err := accept(a.accepter, a.config.ResolveImages, a.resolver, m, attr, excluded); err != nil {
+	if err := accept(a.accepter, a.config.ResolveImages, a.resolver, m, newAttr, excluded); err != nil {
 		return err
 	}
 

pkg/image/admission/imagepolicy/imagepolicy_test.go

@@ -577,6 +577,42 @@ func TestAdmissionResolveImages(t *testing.T) {
 				},
 			},
 		},
+		// resolves builds.build.openshift.io with image stream tags, uses the image DockerImageReference with SHA set.
+		{
+			client: testclient.NewSimpleFake(
+				&imageapi.ImageStreamTag{
+					ObjectMeta: kapi.ObjectMeta{Name: "test:other", Namespace: "default"},
+					Image:      *image1,
+				},
+			),
+			attrs: admission.NewAttributesRecord(
+				&buildapi.Build{
+					Spec: buildapi.BuildSpec{
+						CommonSpec: buildapi.CommonSpec{
+							Strategy: buildapi.BuildStrategy{
+								CustomStrategy: &buildapi.CustomBuildStrategy{
+									From: kapi.ObjectReference{Kind: "ImageStreamTag", Name: "test:other"},
+								},
+							},
+						},
+					},
+				}, nil, unversioned.GroupVersionKind{Group: "build.openshift.io", Version: "v1", Kind: "Build"},
+				"default", "build1", unversioned.GroupVersionResource{Group: "build.openshift.io", Version: "v1", Resource: "builds"},
+				"", admission.Create, nil,
+			),
+			admit: true,
+			expect: &buildapi.Build{
+				Spec: buildapi.BuildSpec{
+					CommonSpec: buildapi.CommonSpec{
+						Strategy: buildapi.BuildStrategy{
+							CustomStrategy: &buildapi.CustomBuildStrategy{
+								From: kapi.ObjectReference{Kind: "DockerImage", Name: "integrated.registry/image1/image1@sha256:0000000000000000000000000000000000000000000000000000000000000001"},
+							},
+						},
+					},
+				},
+			},
+		},
 		// resolves builds with image stream images
 		{
 			client: testclient.NewSimpleFake(