Merge pull request #66391 from jennybuckley/dry-run-admission

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Support dry run in admission plugins

**What this PR does / why we need it**:
Adds support for dry run to admission controllers as outlined by kubernetes/community#2387

- [x] add IsDryRun() to admission.Attributes interface
- [x] add dry run support to NamespaceAutoProvision
- [x] add dry run support to ResourceQuota
- [x] add dry run support to EventRateLimit

The following is being done in a follow up PR:
- [x] add DryRun to ```admission.k8s.io/v1beta1.AdmissionReview```
- [x] add DryRunnable to ```admissionregistration.k8s.io/v1beta1.(Valid|Mut)atingWebhookConfiguration```
- [x] add dry run support to (Valid|Mut)atingAdmissionWebhook

/sig api-machinery

**Release note**:
```release-note
In clusters where the DryRun feature is enabled, dry-run requests will go through the normal admission chain. Because of this, ImagePolicyWebhook authors should especially make sure that their webhooks do not rely on side effects.
```

Here is a list of the admission controllers that were considered when making this PR:
- AlwaysAdmit: No side effects
- AlwaysPullImages: No side effects
- LimitPodHardAntiAffinityTopology: No side effects
- DefaultTolerationSeconds: No side effects
- AlwaysDeny: No side effects
- EventRateLimit: Has side possible effect of affecting the rate, skipping this entire plugin in dry-run case since it won't correspond to an actual write to etcd anyway
- DenyEscalatingExec: No side effects
- DenyExecOnPrivileged: Deprecated, and has no side effects
- ExtendedResourceToleration: No side effects
- OwnerReferencesPermissionEnforcement: No side effects
- ImagePolicyWebhook: No side effects* (*this uses a webhook but it is very specialized. It only sees pod container images, for the purpose of accepting or rejecting certain image sources, so it is very unlikely that it would rely on side effects.)
- LimitRanger: No side effects
- NamespaceAutoProvision: Has possible side effect of creating a namespace, skipping the create in the dry-run case
- NamespaceExists: No side effects
- NodeRestriction: No side effects
- PodNodeSelector: No side effects
- PodPreset: No side effects
- PodTolerationRestriction: No side effects
- Priority: No side effects
- ResourceQuota: Has side possible effect of taking up quota, will only check quota but skip changing quota in the dry-run case
- PodSecurityPolicy: No side effects
- SecurityContextDeny: No side effects
- ServiceAccount: No side effects
- PersistentVolumeLabel: No side effects
- PersistentVolumeClaimResize: No side effects
- DefaultStorageClass: No side effects
- StorageObjectInUseProtection: No side effects
- Initializers: No side effects
- NamespaceLifecycle: No side effects
- MutatingAdmissionWebhook: Same as below
- ValidatingAdmissionWebhook: Has possible side effects depending on if webhook authors depend on side effects and a reconciliation mechanism. To fix this we will expose whether or not a request is dry-run to webhooks through AdmissionReview, and require that all called webhooks understand the field by checking if DryRunnable true is specified in the webhook config. This will be done in a separate PR because it requires an api-change

plugin/pkg/admission/admit/admission_test.go

@@ -25,7 +25,7 @@ import (
 
 func TestAdmissionNonNilAttribute(t *testing.T) {
 	handler := NewAlwaysAdmit()
-	err := handler.(*alwaysAdmit).Admit(admission.NewAttributesRecord(nil, nil, api.Kind("kind").WithVersion("version"), "namespace", "name", api.Resource("resource").WithVersion("version"), "subresource", admission.Create, nil))
+	err := handler.(*alwaysAdmit).Admit(admission.NewAttributesRecord(nil, nil, api.Kind("kind").WithVersion("version"), "namespace", "name", api.Resource("resource").WithVersion("version"), "subresource", admission.Create, false, nil))
 	if err != nil {
 		t.Errorf("Unexpected error returned from admission handler")
 	}

plugin/pkg/admission/alwayspullimages/admission_test.go

@@ -47,7 +47,7 @@ func TestAdmission(t *testing.T) {
 			},
 		},
 	}
-	err := handler.Admit(admission.NewAttributesRecord(&pod, nil, api.Kind("Pod").WithVersion("version"), pod.Namespace, pod.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, nil))
+	err := handler.Admit(admission.NewAttributesRecord(&pod, nil, api.Kind("Pod").WithVersion("version"), pod.Namespace, pod.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil))
 	if err != nil {
 		t.Errorf("Unexpected error returned from admission handler")
 	}
@@ -84,7 +84,7 @@ func TestValidate(t *testing.T) {
 		},
 	}
 	expectedError := `pods "123" is forbidden: spec.initContainers[0].imagePullPolicy: Unsupported value: "": supported values: "Always"`
-	err := handler.Validate(admission.NewAttributesRecord(&pod, nil, api.Kind("Pod").WithVersion("version"), pod.Namespace, pod.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, nil))
+	err := handler.Validate(admission.NewAttributesRecord(&pod, nil, api.Kind("Pod").WithVersion("version"), pod.Namespace, pod.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, false, nil))
 	if err == nil {
 		t.Fatal("missing expected error")
 	}
@@ -139,7 +139,7 @@ func TestOtherResources(t *testing.T) {
 	for _, tc := range tests {
 		handler := &AlwaysPullImages{}
 
-		err := handler.Admit(admission.NewAttributesRecord(tc.object, nil, api.Kind(tc.kind).WithVersion("version"), namespace, name, api.Resource(tc.resource).WithVersion("version"), tc.subresource, admission.Create, nil))
+		err := handler.Admit(admission.NewAttributesRecord(tc.object, nil, api.Kind(tc.kind).WithVersion("version"), namespace, name, api.Resource(tc.resource).WithVersion("version"), tc.subresource, admission.Create, false, nil))
 
 		if tc.expectError {
 			if err == nil {

plugin/pkg/admission/antiaffinity/admission_test.go

@@ -199,7 +199,7 @@ func TestInterPodAffinityAdmission(t *testing.T) {
 	}
 	for _, test := range tests {
 		pod.Spec.Affinity = test.affinity
-		err := handler.Validate(admission.NewAttributesRecord(&pod, nil, api.Kind("Pod").WithVersion("version"), "foo", "name", api.Resource("pods").WithVersion("version"), "", "ignored", nil))
+		err := handler.Validate(admission.NewAttributesRecord(&pod, nil, api.Kind("Pod").WithVersion("version"), "foo", "name", api.Resource("pods").WithVersion("version"), "", "ignored", false, nil))
 
 		if test.errorExpected && err == nil {
 			t.Errorf("Expected error for Anti Affinity %+v but did not get an error", test.affinity)
@@ -267,7 +267,7 @@ func TestOtherResources(t *testing.T) {
 	for _, tc := range tests {
 		handler := &Plugin{}
 
-		err := handler.Validate(admission.NewAttributesRecord(tc.object, nil, api.Kind(tc.kind).WithVersion("version"), namespace, name, api.Resource(tc.resource).WithVersion("version"), tc.subresource, admission.Create, nil))
+		err := handler.Validate(admission.NewAttributesRecord(tc.object, nil, api.Kind(tc.kind).WithVersion("version"), namespace, name, api.Resource(tc.resource).WithVersion("version"), tc.subresource, admission.Create, false, nil))
 
 		if tc.expectError {
 			if err == nil {

plugin/pkg/admission/defaulttolerationseconds/admission_test.go

@@ -395,7 +395,7 @@ func TestForgivenessAdmission(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		err := handler.Admit(admission.NewAttributesRecord(&test.requestedPod, nil, api.Kind("Pod").WithVersion("version"), "foo", "name", api.Resource("pods").WithVersion("version"), "", "ignored", nil))
+		err := handler.Admit(admission.NewAttributesRecord(&test.requestedPod, nil, api.Kind("Pod").WithVersion("version"), "foo", "name", api.Resource("pods").WithVersion("version"), "", "ignored", false, nil))
 		if err != nil {
 			t.Errorf("[%s]: unexpected error %v for pod %+v", test.description, err, test.requestedPod)
 		}

plugin/pkg/admission/deny/admission_test.go

@@ -25,7 +25,7 @@ import (
 
 func TestAdmission(t *testing.T) {
 	handler := NewAlwaysDeny()
-	err := handler.(*alwaysDeny).Admit(admission.NewAttributesRecord(nil, nil, api.Kind("kind").WithVersion("version"), "namespace", "name", api.Resource("resource").WithVersion("version"), "subresource", admission.Create, nil))
+	err := handler.(*alwaysDeny).Admit(admission.NewAttributesRecord(nil, nil, api.Kind("kind").WithVersion("version"), "namespace", "name", api.Resource("resource").WithVersion("version"), "subresource", admission.Create, false, nil))
 	if err == nil {
 		t.Error("Expected error returned from admission handler")
 	}

plugin/pkg/admission/eventratelimit/admission.go

@@ -87,6 +87,13 @@ func (a *Plugin) Validate(attr admission.Attributes) (err error) {
 		return nil
 	}
 
+	// ignore all requests that specify dry-run
+	// because they don't correspond to any calls to etcd,
+	// they should not be affected by the ratelimit
+	if attr.IsDryRun() {
+		return nil
+	}
+
 	var errors []error
 	// give each limit enforcer a chance to reject the event
 	for _, enforcer := range a.limitEnforcers {

plugin/pkg/admission/eventratelimit/admission_test.go

@@ -46,6 +46,7 @@ func attributesForRequest(rq request) admission.Attributes {
 		api.Resource("resource").WithVersion("version"),
 		"",
 		admission.Create,
+		rq.dryRun,
 		&user.DefaultInfo{Name: rq.username})
 }
 
@@ -56,6 +57,7 @@ type request struct {
 	event     *api.Event
 	delay     time.Duration
 	accepted  bool
+	dryRun    bool
 }
 
 func newRequest(kind string) request {
@@ -91,6 +93,11 @@ func (r request) withEventComponent(component string) request {
 	})
 }
 
+func (r request) withDryRun(dryRun bool) request {
+	r.dryRun = dryRun
+	return r
+}
+
 func (r request) withUser(name string) request {
 	r.username = name
 	return r
@@ -153,6 +160,21 @@ func TestEventRateLimiting(t *testing.T) {
 				newEventRequest().blocked(),
 			},
 		},
+		{
+			name:        "event not blocked by dry-run requests",
+			serverBurst: 3,
+			requests: []request{
+				newEventRequest(),
+				newEventRequest(),
+				newEventRequest().withDryRun(true),
+				newEventRequest().withDryRun(true),
+				newEventRequest().withDryRun(true),
+				newEventRequest().withDryRun(true),
+				newEventRequest(),
+				newEventRequest().blocked(),
+				newEventRequest().withDryRun(true),
+			},
+		},
 		{
 			name:        "non-event not blocked after tokens exhausted",
 			serverBurst: 3,

plugin/pkg/admission/exec/admission_test.go

@@ -123,7 +123,7 @@ func testAdmission(t *testing.T, pod *api.Pod, handler *DenyExec, shouldAccept b
 	// pods/exec
 	{
 		req := &rest.ConnectRequest{Name: pod.Name, ResourcePath: "pods/exec"}
-		err := handler.Validate(admission.NewAttributesRecord(req, nil, api.Kind("Pod").WithVersion("version"), "test", "name", api.Resource("pods").WithVersion("version"), "exec", admission.Connect, nil))
+		err := handler.Validate(admission.NewAttributesRecord(req, nil, api.Kind("Pod").WithVersion("version"), "test", "name", api.Resource("pods").WithVersion("version"), "exec", admission.Connect, false, nil))
 		if shouldAccept && err != nil {
 			t.Errorf("Unexpected error returned from admission handler: %v", err)
 		}
@@ -135,7 +135,7 @@ func testAdmission(t *testing.T, pod *api.Pod, handler *DenyExec, shouldAccept b
 	// pods/attach
 	{
 		req := &rest.ConnectRequest{Name: pod.Name, ResourcePath: "pods/attach"}
-		err := handler.Validate(admission.NewAttributesRecord(req, nil, api.Kind("Pod").WithVersion("version"), "test", "name", api.Resource("pods").WithVersion("version"), "attach", admission.Connect, nil))
+		err := handler.Validate(admission.NewAttributesRecord(req, nil, api.Kind("Pod").WithVersion("version"), "test", "name", api.Resource("pods").WithVersion("version"), "attach", admission.Connect, false, nil))
 		if shouldAccept && err != nil {
 			t.Errorf("Unexpected error returned from admission handler: %v", err)
 		}

plugin/pkg/admission/extendedresourcetoleration/admission_test.go

@@ -354,7 +354,7 @@ func TestAdmit(t *testing.T) {
 		},
 	}
 	for i, test := range tests {
-		err := plugin.Admit(admission.NewAttributesRecord(&test.requestedPod, nil, core.Kind("Pod").WithVersion("version"), "foo", "name", core.Resource("pods").WithVersion("version"), "", "ignored", nil))
+		err := plugin.Admit(admission.NewAttributesRecord(&test.requestedPod, nil, core.Kind("Pod").WithVersion("version"), "foo", "name", core.Resource("pods").WithVersion("version"), "", "ignored", false, nil))
 		if err != nil {
 			t.Errorf("[%d: %s] unexpected error %v for pod %+v", i, test.description, err, test.requestedPod)
 		}

plugin/pkg/admission/gc/gc_admission_test.go

@@ -270,7 +270,7 @@ func TestGCAdmission(t *testing.T) {
 				operation = admission.Update
 			}
 			user := &user.DefaultInfo{Name: tc.username}
-			attributes := admission.NewAttributesRecord(tc.newObj, tc.oldObj, schema.GroupVersionKind{}, metav1.NamespaceDefault, "foo", tc.resource, tc.subresource, operation, user)
+			attributes := admission.NewAttributesRecord(tc.newObj, tc.oldObj, schema.GroupVersionKind{}, metav1.NamespaceDefault, "foo", tc.resource, tc.subresource, operation, false, user)
 
 			err = gcAdmit.Validate(attributes)
 			if !tc.checkError(err) {
@@ -518,7 +518,7 @@ func TestBlockOwnerDeletionAdmission(t *testing.T) {
 			operation = admission.Update
 		}
 		user := &user.DefaultInfo{Name: tc.username}
-		attributes := admission.NewAttributesRecord(tc.newObj, tc.oldObj, schema.GroupVersionKind{}, metav1.NamespaceDefault, "foo", tc.resource, tc.subresource, operation, user)
+		attributes := admission.NewAttributesRecord(tc.newObj, tc.oldObj, schema.GroupVersionKind{}, metav1.NamespaceDefault, "foo", tc.resource, tc.subresource, operation, false, user)
 
 		err := gcAdmit.Validate(attributes)
 		if !tc.checkError(err) {

plugin/pkg/admission/imagepolicy/admission_test.go

@@ -472,7 +472,7 @@ func TestTLSConfig(t *testing.T) {
 				return
 			}
 			pod := goodPod(strconv.Itoa(rand.Intn(1000)))
-			attr := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, &user.DefaultInfo{})
+			attr := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, false, &user.DefaultInfo{})
 
 			// Allow all and see if we get an error.
 			service.Allow()
@@ -561,7 +561,7 @@ func TestWebhookCache(t *testing.T) {
 		{statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCached: true},
 	}
 
-	attr := admission.NewAttributesRecord(goodPod("test"), nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, &user.DefaultInfo{})
+	attr := admission.NewAttributesRecord(goodPod("test"), nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, false, &user.DefaultInfo{})
 
 	serv.allow = true
 
@@ -573,7 +573,7 @@ func TestWebhookCache(t *testing.T) {
 		{statusCode: 200, expectedErr: false, expectedAuthorized: true, expectedCached: false},
 		{statusCode: 500, expectedErr: false, expectedAuthorized: true, expectedCached: true},
 	}
-	attr = admission.NewAttributesRecord(goodPod("test2"), nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, &user.DefaultInfo{})
+	attr = admission.NewAttributesRecord(goodPod("test2"), nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, false, &user.DefaultInfo{})
 
 	testWebhookCacheCases(t, serv, wh, attr, tests)
 }
@@ -747,7 +747,7 @@ func TestContainerCombinations(t *testing.T) {
 				return
 			}
 
-			attr := admission.NewAttributesRecord(tt.pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, &user.DefaultInfo{})
+			attr := admission.NewAttributesRecord(tt.pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, false, &user.DefaultInfo{})
 
 			err = wh.Validate(attr)
 			if tt.wantAllowed {
@@ -825,7 +825,7 @@ func TestDefaultAllow(t *testing.T) {
 				return
 			}
 
-			attr := admission.NewAttributesRecord(tt.pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, &user.DefaultInfo{})
+			attr := admission.NewAttributesRecord(tt.pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, false, &user.DefaultInfo{})
 
 			err = wh.Validate(attr)
 			if tt.wantAllowed {
@@ -917,7 +917,7 @@ func TestAnnotationFiltering(t *testing.T) {
 			pod := goodPod("test")
 			pod.Annotations = tt.annotations
 
-			attr := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, &user.DefaultInfo{})
+			attr := admission.NewAttributesRecord(pod, nil, api.Kind("Pod").WithVersion("version"), "namespace", "", api.Resource("pods").WithVersion("version"), "", admission.Create, false, &user.DefaultInfo{})
 
 			err = wh.Validate(attr)
 			if err != nil {

plugin/pkg/admission/limitranger/admission_test.go

@@ -694,16 +694,16 @@ func TestLimitRangerIgnoresSubresource(t *testing.T) {
 	informerFactory.Start(wait.NeverStop)
 
 	testPod := validPod("testPod", 1, api.ResourceRequirements{})
-	err = handler.Admit(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, nil))
+	err = handler.Admit(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil))
 	if err != nil {
 		t.Fatal(err)
 	}
-	err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, nil))
+	err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil))
 	if err == nil {
 		t.Errorf("Expected an error since the pod did not specify resource limits in its update call")
 	}
 
-	err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "status", admission.Update, nil))
+	err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "status", admission.Update, false, nil))
 	if err != nil {
 		t.Errorf("Should have ignored calls to any subresource of pod %v", err)
 	}
@@ -720,16 +720,16 @@ func TestLimitRangerAdmitPod(t *testing.T) {
 	informerFactory.Start(wait.NeverStop)
 
 	testPod := validPod("testPod", 1, api.ResourceRequirements{})
-	err = handler.Admit(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, nil))
+	err = handler.Admit(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil))
 	if err != nil {
 		t.Fatal(err)
 	}
-	err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, nil))
+	err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil))
 	if err == nil {
 		t.Errorf("Expected an error since the pod did not specify resource limits in its update call")
 	}
 
-	err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "status", admission.Update, nil))
+	err = handler.Validate(admission.NewAttributesRecord(&testPod, nil, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "testPod", api.Resource("pods").WithVersion("version"), "status", admission.Update, false, nil))
 	if err != nil {
 		t.Errorf("Should have ignored calls to any subresource of pod %v", err)
 	}
@@ -738,7 +738,7 @@ func TestLimitRangerAdmitPod(t *testing.T) {
 	terminatingPod := validPod("terminatingPod", 1, api.ResourceRequirements{})
 	now := metav1.Now()
 	terminatingPod.DeletionTimestamp = &now
-	err = handler.Validate(admission.NewAttributesRecord(&terminatingPod, &terminatingPod, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "terminatingPod", api.Resource("pods").WithVersion("version"), "", admission.Update, nil))
+	err = handler.Validate(admission.NewAttributesRecord(&terminatingPod, &terminatingPod, api.Kind("Pod").WithVersion("version"), limitRange.Namespace, "terminatingPod", api.Resource("pods").WithVersion("version"), "", admission.Update, false, nil))
 	if err != nil {
 		t.Errorf("LimitRange should ignore a pod marked for termination")
 	}

plugin/pkg/admission/namespace/autoprovision/admission.go

@@ -55,6 +55,11 @@ var _ = kubeapiserveradmission.WantsInternalKubeClientSet(&Provision{})
 
 // Admit makes an admission decision based on the request attributes
 func (p *Provision) Admit(a admission.Attributes) error {
+	// Don't create a namespace if the request is for a dry-run.
+	if a.IsDryRun() {
+		return nil
+	}
+
 	// if we're here, then we've already passed authentication, so we're allowed to do what we're trying to do
 	// if we're here, then the API server has found a route, which means that if we have a non-empty namespace
 	// its a namespaced resource.