Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promote admission webhook API to v1 #79549

Merged
merged 11 commits into from
Jul 12, 2019

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Jun 29, 2019

What type of PR is this?
/kind api-change
/kind feature

What this PR does / why we need it:

  • Promotes the {Validating,Mutating}WebhookConfiguration types to v1
  • failurePolicy default changed from Ignore to Fail for v1
  • matchPolicy default changed from Exact to Equivalent for v1
  • timeout default changed from 30s to 10s for v1
  • sideEffects default value is removed and the field made required for v1
  • admissionReviewVersions default value is removed and the field made required for v1
  • webhook names required to be unique when creating v1 configuration objects (Admission Webhook Names are not guaranteed to be unique within their configuration #78510)
  • Adds unit tests for new defaults
  • Adds unit tests for new validation

Fixes #78510

Special notes for your reviewer:

  • The first commit just copies the existing types, changes package names, and finds/replaces v1beta1 with v1
  • The subsequent commits change or remove a specific default in the v1 API, or tighten validation in the v1 API
  • The last commit is generated files

Does this PR introduce a user-facing change?:

The `MutatingWebhookConfiguration` and `ValidatingWebhookConfiguration` APIs have been promoted to `admissionregistration.k8s.io/v1`:
* `failurePolicy` default changed from `Ignore` to `Fail` for v1
* `matchPolicy` default changed from `Exact` to `Equivalent` for v1
* `timeout` default changed from `30s` to `10s` for v1
* `sideEffects` default value is removed, and the field made required, and only `None` and `NoneOnDryRun` are permitted for v1
* `admissionReviewVersions` default value is removed and the field made required for v1 (supported versions for AdmissionReview are `v1` and `v1beta1`)
* The `name` field for specified webhooks must be unique for `MutatingWebhookConfiguration` and `ValidatingWebhookConfiguration` objects created via `admissionregistration.k8s.io/v1`
The `admissionregistration.k8s.io/v1beta1` versions of `MutatingWebhookConfiguration` and `ValidatingWebhookConfiguration` are deprecated and will no longer be served in v1.19.

/sig api-machinery
/cc @sttts @jpbetz @roycaihw

@k8s-ci-robot k8s-ci-robot requested a review from jpbetz June 29, 2019 00:04
@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jun 29, 2019
@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 29, 2019
@liggitt liggitt added this to the v1.16 milestone Jun 29, 2019
@k8s-ci-robot k8s-ci-robot added area/kubectl area/test sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/testing Categorizes an issue or PR as relevant to SIG Testing. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 29, 2019
@liggitt liggitt force-pushed the admission-webhooks-v1 branch 3 times, most recently from 38632e9 to 875eac0 Compare June 29, 2019 00:54
@liggitt
Copy link
Member Author

liggitt commented Jun 29, 2019

/retest

1 similar comment
@liggitt
Copy link
Member Author

liggitt commented Jun 29, 2019

/retest

@k8s-ci-robot k8s-ci-robot added the sig/apps Categorizes an issue or PR as relevant to SIG Apps. label Jun 29, 2019
@liggitt
Copy link
Member Author

liggitt commented Jul 2, 2019

/priority important-soon

@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jul 2, 2019
@liggitt
Copy link
Member Author

liggitt commented Jul 11, 2019

/retest

@sttts
Copy link
Contributor

sttts commented Jul 11, 2019

lgtm
/approve

/assign @smarterclayton

for final lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, sttts

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@smarterclayton
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 11, 2019
@k8s-ci-robot k8s-ci-robot merged commit 3dd8add into kubernetes:master Jul 12, 2019
@liggitt liggitt deleted the admission-webhooks-v1 branch July 25, 2019 14:46
@roycaihw
Copy link
Member

/area admission-control

Ark-kun added a commit to Ark-kun/pipelines that referenced this pull request Oct 17, 2020
The sideEffects field field became required in v1 version of the resource kubernetes/kubernetes#79549

Also adding failurePolicy: Ignore, because the default value has changed to Fail in v1.16.

These changes are not needed for v1beta1, but I still add them for those cases as well for consistency.
k8s-ci-robot pushed a commit to kubeflow/pipelines that referenced this pull request Oct 20, 2020
…ixes #4627 (#4632)

* Backend - Caching - Fixed deployer failure on Kubernetes v1.16+

The sideEffects field field became required in v1 version of the resource kubernetes/kubernetes#79549

Also adding failurePolicy: Ignore, because the default value has changed to Fail in v1.16.

These changes are not needed for v1beta1, but I still add them for those cases as well for consistency.

* The admissionReviewVersions field became required in the v1 API in v1.16

See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#request
Bobgy pushed a commit to Bobgy/pipelines that referenced this pull request Oct 22, 2020
…ixes kubeflow#4627 (kubeflow#4632)

* Backend - Caching - Fixed deployer failure on Kubernetes v1.16+

The sideEffects field field became required in v1 version of the resource kubernetes/kubernetes#79549

Also adding failurePolicy: Ignore, because the default value has changed to Fail in v1.16.

These changes are not needed for v1beta1, but I still add them for those cases as well for consistency.

* The admissionReviewVersions field became required in the v1 API in v1.16

See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#request
Bobgy pushed a commit to Bobgy/pipelines that referenced this pull request Oct 22, 2020
…ixes kubeflow#4627 (kubeflow#4632)

* Backend - Caching - Fixed deployer failure on Kubernetes v1.16+

The sideEffects field field became required in v1 version of the resource kubernetes/kubernetes#79549

Also adding failurePolicy: Ignore, because the default value has changed to Fail in v1.16.

These changes are not needed for v1beta1, but I still add them for those cases as well for consistency.

* The admissionReviewVersions field became required in the v1 API in v1.16

See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#request
Bobgy pushed a commit to kubeflow/pipelines that referenced this pull request Oct 22, 2020
…ixes #4627 (#4632)

* Backend - Caching - Fixed deployer failure on Kubernetes v1.16+

The sideEffects field field became required in v1 version of the resource kubernetes/kubernetes#79549

Also adding failurePolicy: Ignore, because the default value has changed to Fail in v1.16.

These changes are not needed for v1beta1, but I still add them for those cases as well for consistency.

* The admissionReviewVersions field became required in the v1 API in v1.16

See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#request
Jeffwan pushed a commit to Jeffwan/pipelines that referenced this pull request Dec 9, 2020
…ixes kubeflow#4627 (kubeflow#4632)

* Backend - Caching - Fixed deployer failure on Kubernetes v1.16+

The sideEffects field field became required in v1 version of the resource kubernetes/kubernetes#79549

Also adding failurePolicy: Ignore, because the default value has changed to Fail in v1.16.

These changes are not needed for v1beta1, but I still add them for those cases as well for consistency.

* The admissionReviewVersions field became required in the v1 API in v1.16

See https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/admission-control area/dependency Issues or PRs related to dependency changes area/kubectl area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
Status: API review completed, 1.16
Development

Successfully merging this pull request may close these issues.

Admission Webhook Names are not guaranteed to be unique within their configuration
6 participants