Merge pull request #15118 from smarterclayton/dont_resolve_jobs

Merged by openshift-bot

Author: OpenShift Bot

pkg/image/admission/imagepolicy/accept.go

@@ -67,7 +67,7 @@ func accept(accepter rules.Accepter, policy imageResolutionPolicy, resolver imag
 					// if we resolved properly, assign the attributes and rewrite the pull spec if we need to
 					decision.attrs = resolvedAttrs
 
-					if policy.RewriteImagePullSpec(resolvedAttrs, gr) {
+					if policy.RewriteImagePullSpec(resolvedAttrs, attr.GetOperation() == admission.Update, gr) {
 						ref.Namespace = ""
 						ref.Name = decision.attrs.Name.Exact()
 						ref.Kind = "DockerImage"

pkg/image/admission/imagepolicy/imagepolicy.go

@@ -83,7 +83,7 @@ type imageResolutionPolicy interface {
 	// FailOnResolutionFailure returns true if you should fail when resolution fails
 	FailOnResolutionFailure(schema.GroupResource) bool
 	// RewriteImagePullSpec returns true if you should rewrite image pull specs when resolution succeeds
-	RewriteImagePullSpec(*rules.ImagePolicyAttributes, schema.GroupResource) bool
+	RewriteImagePullSpec(attr *rules.ImagePolicyAttributes, isUpdate bool, gr schema.GroupResource) bool
 }
 
 // imagePolicyPlugin returns an admission controller for pods that controls what images are allowed to run on the
@@ -439,8 +439,24 @@ func (config resolutionConfig) FailOnResolutionFailure(gr schema.GroupResource)
 	return api.FailOnResolutionFailure(config.config.ResolveImages)
 }
 
+var skipImageRewriteOnUpdate = map[schema.GroupResource]struct{}{
+	// Job template specs are immutable, they cannot be updated.
+	{Group: "extensions", Resource: "jobs"}: {},
+	{Group: "batch", Resource: "jobs"}:      {},
+	// Build specs are immutable, they cannot be updated.
+	{Group: "", Resource: "builds"}:                   {},
+	{Group: "build.openshift.io", Resource: "builds"}: {},
+	// TODO: remove when statefulsets allow spec.template updates in 3.7
+	{Group: "apps", Resource: "statefulsets"}: {},
+}
+
 // RewriteImagePullSpec applies to implicit rewrite attributes and local resources as well as if the policy requires it.
-func (config resolutionConfig) RewriteImagePullSpec(attr *rules.ImagePolicyAttributes, gr schema.GroupResource) bool {
+func (config resolutionConfig) RewriteImagePullSpec(attr *rules.ImagePolicyAttributes, isUpdate bool, gr schema.GroupResource) bool {
+	if isUpdate {
+		if _, ok := skipImageRewriteOnUpdate[gr]; ok {
+			return false
+		}
+	}
 	if api.RequestsResolution(config.config.ResolveImages) {
 		return true
 	}

pkg/image/admission/imagepolicy/imagepolicy_test.go

@@ -970,6 +970,7 @@ func TestResolutionConfig(t *testing.T) {
 		config   *api.ImagePolicyConfig
 		resource schema.GroupResource
 		attrs    rules.ImagePolicyAttributes
+		update   bool
 
 		resolve bool
 		fail    bool
@@ -1042,6 +1043,63 @@ func TestResolutionConfig(t *testing.T) {
 			resolve:  true,
 			rewrite:  true,
 		},
+		// resource match skips on job update
+		{
+			attrs: rules.ImagePolicyAttributes{LocalRewrite: true},
+			config: &api.ImagePolicyConfig{
+				ResolveImages: api.DoNotAttempt,
+				ResolutionRules: []api.ImageResolutionPolicyRule{
+					{LocalNames: true, TargetResource: metav1.GroupResource{Group: "extensions", Resource: "jobs"}},
+				},
+			},
+			resource: schema.GroupResource{Group: "extensions", Resource: "jobs"},
+			update:   true,
+			resolve:  true,
+			rewrite:  false,
+		},
+		// resource match succeeds on job create
+		{
+			attrs: rules.ImagePolicyAttributes{LocalRewrite: true},
+			config: &api.ImagePolicyConfig{
+				ResolveImages: api.DoNotAttempt,
+				ResolutionRules: []api.ImageResolutionPolicyRule{
+					{LocalNames: true, TargetResource: metav1.GroupResource{Group: "extensions", Resource: "jobs"}},
+				},
+			},
+			resource: schema.GroupResource{Group: "extensions", Resource: "jobs"},
+			update:   false,
+			resolve:  true,
+			rewrite:  true,
+		},
+		// resource match skips on build update
+		{
+			attrs: rules.ImagePolicyAttributes{LocalRewrite: true},
+			config: &api.ImagePolicyConfig{
+				ResolveImages: api.DoNotAttempt,
+				ResolutionRules: []api.ImageResolutionPolicyRule{
+					{LocalNames: true, TargetResource: metav1.GroupResource{Group: "build.openshift.io", Resource: "builds"}},
+				},
+			},
+			resource: schema.GroupResource{Group: "build.openshift.io", Resource: "builds"},
+			update:   true,
+			resolve:  true,
+			rewrite:  false,
+		},
+		// resource match skips on statefulset update
+		// TODO: remove in 3.7
+		{
+			attrs: rules.ImagePolicyAttributes{LocalRewrite: true},
+			config: &api.ImagePolicyConfig{
+				ResolveImages: api.DoNotAttempt,
+				ResolutionRules: []api.ImageResolutionPolicyRule{
+					{LocalNames: true, TargetResource: metav1.GroupResource{Group: "apps", Resource: "statefulsets"}},
+				},
+			},
+			resource: schema.GroupResource{Group: "apps", Resource: "statefulsets"},
+			update:   true,
+			resolve:  true,
+			rewrite:  false,
+		},
 	}
 
 	for i, test := range testCases {
@@ -1052,7 +1110,7 @@ func TestResolutionConfig(t *testing.T) {
 		if c.FailOnResolutionFailure(test.resource) != test.fail {
 			t.Errorf("%d: resolution failure != %t", i, test.fail)
 		}
-		if c.RewriteImagePullSpec(&test.attrs, test.resource) != test.rewrite {
+		if c.RewriteImagePullSpec(&test.attrs, test.update, test.resource) != test.rewrite {
 			t.Errorf("%d: rewrite != %t", i, test.rewrite)
 		}
 	}