update image policy API for first class resolution

pkg/image/admission/imagepolicy/accept.go

@@ -12,6 +12,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/validation/field"
 
 	"github.com/openshift/origin/pkg/api/meta"
+	imagepolicyapi "github.com/openshift/origin/pkg/image/admission/imagepolicy/api"
 	"github.com/openshift/origin/pkg/image/admission/imagepolicy/rules"
 	imageapi "github.com/openshift/origin/pkg/image/api"
 )
@@ -26,40 +27,57 @@ type policyDecision struct {
 	err    error
 }
 
-func accept(accepter rules.Accepter, resolver imageResolver, m meta.ImageReferenceMutator, attr admission.Attributes, excludedRules sets.String) error {
+func accept(accepter rules.Accepter, imageResolutionType imagepolicyapi.ImageResolutionType, resolver imageResolver, m meta.ImageReferenceMutator, attr admission.Attributes, excludedRules sets.String) error {
 	var decisions policyDecisions
 
 	gr := attr.GetResource().GroupResource()
-	requiresImage := accepter.RequiresImage(gr)
-	resolvesImage := accepter.ResolvesImage(gr)
 
 	errs := m.Mutate(func(ref *kapi.ObjectReference) error {
 		// create the attribute set for this particular reference, if we have never seen the reference
 		// before
 		decision, ok := decisions[*ref]
 		if !ok {
-			var attrs *rules.ImagePolicyAttributes
-			var err error
-			if requiresImage || resolvesImage {
-				// convert the incoming reference into attributes to pass to the accepter
-				attrs, err = resolver.ResolveObjectReference(ref, attr.GetNamespace())
+			if imagepolicyapi.RequestsResolution(imageResolutionType) {
+				resolvedAttrs, err := resolver.ResolveObjectReference(ref, attr.GetNamespace())
+
+				switch {
+				case err != nil && imagepolicyapi.FailOnResolutionFailure(imageResolutionType):
+					// if we had a resolution error and we're supposed to fail, fail
+					decision.err = err
+					decision.tested = true
+					decisions[*ref] = decision
+					return err
+
+				case err != nil:
+					// if we had an error, but aren't supposed to fail, just don't do anything else and keep track of
+					// the resolution failure
+					decision.err = err
+
+				case err == nil:
+					// if we resolved properly, assign the attributes and rewrite the pull spec if we need to
+					decision.attrs = resolvedAttrs
+
+					if imagepolicyapi.RewriteImagePullSpec(imageResolutionType) {
+						ref.Namespace = ""
+						ref.Name = decision.attrs.Name.Exact()
+						ref.Kind = "DockerImage"
+					}
+				}
 			}
-			// if the incoming reference is of a Kind that needed a lookup, but that lookup failed,
-			// use the most generic policy rule here because we don't even know the image name
-			if attrs == nil {
-				attrs = &rules.ImagePolicyAttributes{}
+
+			// if we don't have any image policy attributes, attempt a best effort parse for the remaining tests
+			if decision.attrs == nil {
+				decision.attrs = &rules.ImagePolicyAttributes{}
 
 				// an objectref that is DockerImage ref will have a name that corresponds to its pull spec.  We can parse that
 				// to a docker image ref
 				if ref != nil && ref.Kind == "DockerImage" {
-					attrs.Name, _ = imageapi.ParseDockerImageReference(ref.Name)
+					decision.attrs.Name, _ = imageapi.ParseDockerImageReference(ref.Name)
 				}
 			}
-			attrs.Resource = gr
-			attrs.ExcludedRules = excludedRules
 
-			decision.attrs = attrs
-			decision.err = err
+			decision.attrs.Resource = gr
+			decision.attrs.ExcludedRules = excludedRules
 		}
 
 		// we only need to test a given input once for acceptance
@@ -83,18 +101,6 @@ func accept(accepter rules.Accepter, resolver imageResolver, m meta.ImageReferen
 			}
 		}
 
-		// if resolution was requested, and no error was present, transform the
-		// reference back into a string to a DockerImage
-		if resolvesImage && decision.err == nil {
-			ref.Namespace = ""
-			ref.Name = decision.attrs.Name.Exact()
-			ref.Kind = "DockerImage"
-		}
-
-		if decision.err != nil {
-			glog.V(5).Infof("Ignored resolution error for %v: %v", ref, decision.err)
-		}
-
 		return nil
 	})
 

pkg/image/admission/imagepolicy/api/helpers.go

@@ -0,0 +1,28 @@
+package api
+
+// RequestsResolution returns true if you should attempt to resolve image pull specs
+func RequestsResolution(imageResolutionType ImageResolutionType) bool {
+	switch imageResolutionType {
+	case RequiredRewrite, Required, AttemptRewrite, Attempt:
+		return true
+	}
+	return false
+}
+
+// FailOnResolutionFailure returns true if you should fail when resolution fails
+func FailOnResolutionFailure(imageResolutionType ImageResolutionType) bool {
+	switch imageResolutionType {
+	case RequiredRewrite, Required:
+		return true
+	}
+	return false
+}
+
+// RewriteImagePullSpec returns true if you should rewrite image pull specs when resolution succeeds
+func RewriteImagePullSpec(imageResolutionType ImageResolutionType) bool {
+	switch imageResolutionType {
+	case RequiredRewrite, AttemptRewrite:
+		return true
+	}
+	return false
+}

pkg/image/admission/imagepolicy/api/types.go

@@ -13,18 +13,35 @@ const IgnorePolicyRulesAnnotation = "alpha.image.policy.openshift.io/ignore-rule
 type ImagePolicyConfig struct {
 	unversioned.TypeMeta
 
+	// ResolveImages indicates what kind of image resolution should be done.  If a rewriting policy is chosen,
+	// then the image pull specs will be updated.
+	ResolveImages ImageResolutionType
+
 	// ExecutionRules determine whether the use of an image is allowed in an object with a pod spec.
 	// By default, these rules only apply to pods, but may be extended to other resource types.
 	ExecutionRules []ImageExecutionPolicyRule
 }
 
+// ImageResolutionType is an enumerated string that indicates how image pull spec resolution should be handled
+type ImageResolutionType string
+
+var (
+	// require resolution to succeed and rewrite the resource to use it
+	RequiredRewrite ImageResolutionType = "RequiredRewrite"
+	// require resolution to succeed, but don't rewrite the image pull spec
+	Required ImageResolutionType = "Required"
+	// attempt resolution, rewrite if successful
+	AttemptRewrite ImageResolutionType = "AttemptRewrite"
+	// attempt resolution, don't rewrite
+	Attempt ImageResolutionType = "Attempt"
+	// don't attempt resolution
+	DoNotAttempt ImageResolutionType = "DoNotAttempt"
+)
+
 // ImageExecutionPolicyRule determines whether a provided image may be used on the platform.
 type ImageExecutionPolicyRule struct {
 	ImageCondition
 
-	// Resolve indicates that images referenced by this resource must be resolved
-	Resolve bool
-
 	// Reject means this rule, if it matches the condition, will cause an immediate failure. No
 	// other rules will be considered.
 	Reject bool
@@ -53,9 +70,9 @@ type ImageCondition struct {
 	// registries match, this condition is satisfied.
 	MatchRegistries []string
 
-	// AllowResolutionFailure allows the subsequent conditions to be bypassed if the integrated registry does
+	// SkipOnResolutionFailure allows the subsequent conditions to be bypassed if the integrated registry does
 	// not have access to image metadata (no image exists matching the image digest).
-	AllowResolutionFailure bool
+	SkipOnResolutionFailure bool
 
 	// MatchDockerImageLabels checks against the resolved image for the presence of a Docker label. All conditions
 	// must match.

pkg/image/admission/imagepolicy/api/v1/default-policy.yaml

@@ -1,5 +1,10 @@
 kind: ImagePolicyConfig
 apiVersion: v1
+# To require that all images running on the platform be imported first, you may uncomment the
+# following rule. Any image that refers to a registry outside of OpenShift will be rejected unless it
+# unless it points directly to an image digest (myregistry.com/myrepo/image@sha256:ea83bcf...) and that
+# digest has been imported via the import-image flow.
+#resolveImages: Required
 executionRules:
 - name: execution-denied
   # Reject all images that have the annotation images.openshift.io/deny-execution set to true.
@@ -11,10 +16,5 @@ executionRules:
   matchImageAnnotations:
   - key: images.openshift.io/deny-execution
     value: "true"
-  allowResolutionFailure: true
-# To require that all images running on the platform be imported first, you may uncomment the
-# following rule. Any image that refers to a registry outside of OpenShift will be rejected unless it
-# unless it points directly to an image digest (myregistry.com/myrepo/image@sha256:ea83bcf...) and that
-# digest has been imported via the import-image flow.
-#- name: require-imported-images
-#  allowResolutionFailure: false
+  skipOnResolutionFailure: true
+

pkg/image/admission/imagepolicy/api/v1/defaults.go

@@ -0,0 +1,32 @@
+package v1
+
+import (
+	kapi "k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/runtime"
+)
+
+func SetDefaults_ImagePolicyConfig(obj *ImagePolicyConfig) {
+	if obj == nil {
+		return
+	}
+
+	if len(obj.ResolveImages) == 0 {
+		obj.ResolveImages = Attempt
+	}
+
+	for i := range obj.ExecutionRules {
+		if len(obj.ExecutionRules[i].OnResources) == 0 {
+			obj.ExecutionRules[i].OnResources = []GroupResource{{Resource: "pods", Group: kapi.GroupName}}
+		}
+	}
+
+}
+
+func addDefaultingFuncs(scheme *runtime.Scheme) {
+	err := scheme.AddDefaultingFuncs(
+		SetDefaults_ImagePolicyConfig,
+	)
+	if err != nil {
+		panic(err)
+	}
+}

pkg/image/admission/imagepolicy/api/v1/register.go

@@ -1,7 +1,6 @@
 package v1
 
 import (
-	kapi "k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	"k8s.io/kubernetes/pkg/conversion"
 	"k8s.io/kubernetes/pkg/runtime"
@@ -17,15 +16,7 @@ func AddToScheme(scheme *runtime.Scheme) {
 	scheme.AddKnownTypes(SchemeGroupVersion,
 		&ImagePolicyConfig{},
 	)
-	scheme.AddDefaultingFuncs(
-		func(c *ImagePolicyConfig) {
-			for i := range c.ExecutionRules {
-				if len(c.ExecutionRules[i].OnResources) == 0 {
-					c.ExecutionRules[i].OnResources = []GroupResource{{Resource: "pods", Group: kapi.GroupName}}
-				}
-			}
-		},
-	)
+	addDefaultingFuncs(scheme)
 	scheme.AddConversionFuncs(
 		// TODO: remove when MatchSignatures is implemented
 		func(in *ImageCondition, out *api.ImageCondition, s conversion.Scope) error {

pkg/image/admission/imagepolicy/api/v1/swagger_doc.go

@@ -23,7 +23,7 @@ var map_ImageCondition = map[string]string{
 	"invertMatch":             "InvertMatch means the value of the condition is logically inverted (true -> false, false -> true).",
 	"matchIntegratedRegistry": "MatchIntegratedRegistry will only match image sources that originate from the configured integrated registry.",
 	"matchRegistries":         "MatchRegistries will match image references that point to the provided registries. The image registry must match at least one of these strings.",
-	"allowResolutionFailure":  "AllowResolutionFailure allows the subsequent conditions to be bypassed if the integrated registry does not have access to image metadata (no image exists matching the image digest).",
+	"skipOnResolutionFailure": "SkipOnResolutionFailure allows the subsequent conditions to be bypassed if the integrated registry does not have access to image metadata (no image exists matching the image digest).",
 	"matchDockerImageLabels":  "MatchDockerImageLabels checks against the resolved image for the presence of a Docker label. All conditions must match.",
 	"matchImageLabels":        "MatchImageLabels checks against the resolved image for a label. All conditions must match.",
 	"matchImageAnnotations":   "MatchImageAnnotations checks against the resolved image for an annotation. All conditions must match.",
@@ -34,9 +34,8 @@ func (ImageCondition) SwaggerDoc() map[string]string {
 }
 
 var map_ImageExecutionPolicyRule = map[string]string{
-	"":        "ImageExecutionPolicyRule determines whether a provided image may be used on the platform.",
-	"resolve": "Resolve indicates that images referenced by this resource must be resolved",
-	"reject":  "Reject means this rule, if it matches the condition, will cause an immediate failure. No other rules will be considered.",
+	"":       "ImageExecutionPolicyRule determines whether a provided image may be used on the platform.",
+	"reject": "Reject means this rule, if it matches the condition, will cause an immediate failure. No other rules will be considered.",
 }
 
 func (ImageExecutionPolicyRule) SwaggerDoc() map[string]string {
@@ -45,6 +44,7 @@ func (ImageExecutionPolicyRule) SwaggerDoc() map[string]string {
 
 var map_ImagePolicyConfig = map[string]string{
 	"":               "ImagePolicyConfig is the configuration for control of images running on the platform.",
+	"resolveImages":  "ResolveImages indicates what kind of image resolution should be done.  If a rewriting policy is chosen, then the image pull specs will be updated.",
 	"executionRules": "ExecutionRules determine whether the use of an image is allowed in an object with a pod spec. By default, these rules only apply to pods, but may be extended to other resource types. If all execution rules are negations, the default behavior is allow all. If any execution rule is an allow, the default behavior is to reject all.",
 }
 

pkg/image/admission/imagepolicy/api/v1/types.go

@@ -8,20 +8,37 @@ import (
 type ImagePolicyConfig struct {
 	unversioned.TypeMeta `json:",inline"`
 
+	// ResolveImages indicates what kind of image resolution should be done.  If a rewriting policy is chosen,
+	// then the image pull specs will be updated.
+	ResolveImages ImageResolutionType `json:"resolveImages"`
+
 	// ExecutionRules determine whether the use of an image is allowed in an object with a pod spec.
 	// By default, these rules only apply to pods, but may be extended to other resource types.
 	// If all execution rules are negations, the default behavior is allow all. If any execution rule
 	// is an allow, the default behavior is to reject all.
 	ExecutionRules []ImageExecutionPolicyRule `json:"executionRules"`
 }
 
+// ImageResolutionType is an enumerated string that indicates how image pull spec resolution should be handled
+type ImageResolutionType string
+
+var (
+	// require resolution to succeed and rewrite the resource to use it
+	RequiredRewrite ImageResolutionType = "RequiredRewrite"
+	// require resolution to succeed, but don't rewrite the image pull spec
+	Required ImageResolutionType = "Required"
+	// attempt resolution, rewrite if successful
+	AttemptRewrite ImageResolutionType = "AttemptRewrite"
+	// attempt resolution, don't rewrite
+	Attempt ImageResolutionType = "Attempt"
+	// don't attempt resolution
+	DoNotAttempt ImageResolutionType = "DoNotAttempt"
+)
+
 // ImageExecutionPolicyRule determines whether a provided image may be used on the platform.
 type ImageExecutionPolicyRule struct {
 	ImageCondition `json:",inline"`
 
-	// Resolve indicates that images referenced by this resource must be resolved
-	Resolve bool `json:"resolve"`
-
 	// Reject means this rule, if it matches the condition, will cause an immediate failure. No
 	// other rules will be considered.
 	Reject bool `json:"reject"`
@@ -58,9 +75,9 @@ type ImageCondition struct {
 	// must match at least one of these strings.
 	MatchRegistries []string `json:"matchRegistries"`
 
-	// AllowResolutionFailure allows the subsequent conditions to be bypassed if the integrated registry does
+	// SkipOnResolutionFailure allows the subsequent conditions to be bypassed if the integrated registry does
 	// not have access to image metadata (no image exists matching the image digest).
-	AllowResolutionFailure bool `json:"allowResolutionFailure"`
+	SkipOnResolutionFailure bool `json:"skipOnResolutionFailure"`
 
 	// MatchDockerImageLabels checks against the resolved image for the presence of a Docker label. All
 	// conditions must match.

pkg/image/admission/imagepolicy/api/validation/validation.go

@@ -26,5 +26,21 @@ func Validate(config *api.ImagePolicyConfig) field.ErrorList {
 			}
 		}
 	}
+
+	// if you don't attempt resolution, you'll never be able to pass any rule that logically requires it
+	if config.ResolveImages == api.DoNotAttempt {
+		for i, rule := range config.ExecutionRules {
+			if len(rule.MatchDockerImageLabels) > 0 {
+				allErrs = append(allErrs, field.Invalid(field.NewPath(api.PluginName, "executionRules").Index(i).Child("matchDockerImageLabels"), rule.MatchDockerImageLabels, "images are not being resolved, this condition will always fail"))
+			}
+			if len(rule.MatchImageLabels) > 0 {
+				allErrs = append(allErrs, field.Invalid(field.NewPath(api.PluginName, "executionRules").Index(i).Child("matchImageLabels"), rule.MatchImageLabels, "images are not being resolved, this condition will always fail"))
+			}
+			if len(rule.MatchImageAnnotations) > 0 {
+				allErrs = append(allErrs, field.Invalid(field.NewPath(api.PluginName, "executionRules").Index(i).Child("matchImageAnnotations"), rule.MatchImageAnnotations, "images are not being resolved, this condition will always fail"))
+			}
+		}
+	}
+
 	return allErrs
 }