Skip to content

Commit

Permalink
Merge pull request #66936 from jennybuckley/dry-run-webhooks
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 67576, 66936). 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 webhooks

**What this PR does / why we need it**:
Follow up to #66391
- [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

Includes all the api-changes outlined by kubernetes/community#2387

/sig api-machinery

**Release note**:
```release-note
To address the possibility dry-run requests overwhelming admission webhooks that rely on side effects and a reconciliation mechanism, a new field is being added to admissionregistration.k8s.io/v1beta1.ValidatingWebhookConfiguration and admissionregistration.k8s.io/v1beta1.MutatingWebhookConfiguration so that webhooks can explicitly register as having dry-run support. If a dry-run request is made on a resource that triggers a non dry-run supporting webhook, the request will be completely rejected, with "400: Bad Request". Additionally, a new field is being added to the admission.k8s.io/v1beta1.AdmissionReview API object, exposing to webhooks whether or not the request being reviewed is a dry-run.
```
  • Loading branch information
Kubernetes Submit Queue committed Aug 23, 2018
2 parents 687553a + c61eac7 commit 5a16163
Show file tree
Hide file tree
Showing 27 changed files with 407 additions and 117 deletions.
4 changes: 4 additions & 0 deletions api/openapi-spec/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions api/swagger-spec/admissionregistration.k8s.io_v1beta1.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/apis/admission/types.go
Expand Up @@ -73,6 +73,11 @@ type AdmissionRequest struct {
// OldObject is the existing object. Only populated for UPDATE requests.
// +optional
OldObject runtime.Object
// DryRun indicates that modifications will definitely not be persisted for this request.
// Calls to webhooks must have no side effects if DryRun is true.
// Defaults to false.
// +optional
DryRun *bool
}

// AdmissionResponse describes an admission response.
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/admission/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/apis/admission/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/apis/admissionregistration/fuzzer/fuzzer.go
Expand Up @@ -30,6 +30,8 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
c.FuzzNoCustom(obj) // fuzz self without calling this function again
p := admissionregistration.FailurePolicyType("Fail")
obj.FailurePolicy = &p
s := admissionregistration.SideEffectClassUnknown
obj.SideEffects = &s
},
}
}
25 changes: 25 additions & 0 deletions pkg/apis/admissionregistration/types.go
Expand Up @@ -112,6 +112,22 @@ const (
Fail FailurePolicyType = "Fail"
)

type SideEffectClass string

const (
// SideEffectClassUnknown means that no information is known about the side effects of calling the webhook.
// If a request with the dry-run attribute would trigger a call to this webhook, the request will instead fail.
SideEffectClassUnknown SideEffectClass = "Unknown"
// SideEffectClassNone means that calling the webhook will have no side effects.
SideEffectClassNone SideEffectClass = "None"
// SideEffectClassSome means that calling the webhook will possibly have side effects.
// If a request with the dry-run attribute would trigger a call to this webhook, the request will instead fail.
SideEffectClassSome SideEffectClass = "Some"
// SideEffectClassNoneOnDryRun means that calling the webhook will possibly have side effects, but if the
// request being reviewed has the dry-run attribute, the side effects will be suppressed.
SideEffectClassNoneOnDryRun SideEffectClass = "NoneOnDryRun"
)

// +genclient
// +genclient:nonNamespaced
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down Expand Up @@ -235,6 +251,15 @@ type Webhook struct {
// Default to the empty LabelSelector, which matches everything.
// +optional
NamespaceSelector *metav1.LabelSelector

// SideEffects states whether this webhookk has side effects.
// Acceptable values are: Unknown, None, Some, NoneOnDryRun
// Webhooks with side effects MUST implement a reconciliation system, since a request may be
// rejected by a future step in the admission change and the side effects therefore need to be undone.
// Requests with the dryRun attribute will be auto-rejected if they match a webhook with
// sideEffects == Unknown or Some. Defaults to Unknown.
// +optional
SideEffects *SideEffectClass
}

// RuleWithOperations is a tuple of Operations and Resources. It is recommended to make
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/admissionregistration/v1beta1/defaults.go
Expand Up @@ -35,4 +35,9 @@ func SetDefaults_Webhook(obj *admissionregistrationv1beta1.Webhook) {
selector := metav1.LabelSelector{}
obj.NamespaceSelector = &selector
}
if obj.SideEffects == nil {
// TODO: revisit/remove this default and possibly make the field required when promoting to v1
unknown := admissionregistrationv1beta1.SideEffectClassUnknown
obj.SideEffects = &unknown
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions pkg/apis/admissionregistration/validation/validation.go
Expand Up @@ -192,6 +192,9 @@ func validateWebhook(hook *admissionregistration.Webhook, fldPath *field.Path) f
if hook.FailurePolicy != nil && !supportedFailurePolicies.Has(string(*hook.FailurePolicy)) {
allErrors = append(allErrors, field.NotSupported(fldPath.Child("failurePolicy"), *hook.FailurePolicy, supportedFailurePolicies.List()))
}
if hook.SideEffects != nil && !supportedSideEffectClasses.Has(string(*hook.SideEffects)) {
allErrors = append(allErrors, field.NotSupported(fldPath.Child("sideEffects"), *hook.SideEffects, supportedSideEffectClasses.List()))
}

if hook.NamespaceSelector != nil {
allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.NamespaceSelector, fldPath.Child("namespaceSelector"))...)
Expand Down Expand Up @@ -291,6 +294,13 @@ var supportedFailurePolicies = sets.NewString(
string(admissionregistration.Fail),
)

var supportedSideEffectClasses = sets.NewString(
string(admissionregistration.SideEffectClassUnknown),
string(admissionregistration.SideEffectClassNone),
string(admissionregistration.SideEffectClassSome),
string(admissionregistration.SideEffectClassNoneOnDryRun),
)

var supportedOperations = sets.NewString(
string(admissionregistration.OperationAll),
string(admissionregistration.Create),
Expand Down
15 changes: 15 additions & 0 deletions pkg/apis/admissionregistration/validation/validation_test.go
Expand Up @@ -499,6 +499,21 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
}),
expectedError: `webhooks[0].failurePolicy: Unsupported value: "other": supported values: "Fail", "Ignore"`,
},
{
name: "SideEffects can only be \"Unknown\", \"None\", \"Some\", or \"NoneOnDryRun\"",
config: newValidatingWebhookConfiguration(
[]admissionregistration.Webhook{
{
Name: "webhook.k8s.io",
ClientConfig: validClientConfig,
SideEffects: func() *admissionregistration.SideEffectClass {
r := admissionregistration.SideEffectClass("other")
return &r
}(),
},
}),
expectedError: `webhooks[0].sideEffects: Unsupported value: "other": supported values: "None", "NoneOnDryRun", "Some", "Unknown"`,
},
{
name: "both service and URL missing",
config: newValidatingWebhookConfiguration(
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/admissionregistration/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 5a16163

Please sign in to comment.