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

fix admission plugins orders #6900

Merged
merged 1 commit into from
Jan 12, 2018
Merged

Conversation

hzxuzhonghu
Copy link
Member

@hzxuzhonghu hzxuzhonghu commented Jan 9, 2018

fix admission plugins orders, MutatingAdmissionWebhook should be ahead of ValidatingAdmissionWebhook.
see https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/server/options/admission.go#L69

NOTE: After opening the PR, please un-check and re-check the "Allow edits from maintainers" box so that maintainers can work on your patch and speed up the review process. This is a temporary workaround to address a known issue with GitHub.>

Please delete this note before submitting the pull request.

Allow edits from maintainers checkbox


This change is Reviewable

fix admission plugins orders, MutatingAdmissionWebhook should be ahead of ValidatingAdmissionWebhook.
see https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/server/options/admission.go#L69
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 9, 2018
@hzxuzhonghu
Copy link
Member Author

/assign @tengqm

@k8sio-netlify-preview-bot
Copy link
Collaborator

Deploy preview ready!

Built with commit 1ad787a

https://deploy-preview-6900--kubernetes-io-master-staging.netlify.com

Copy link
Contributor

@tengqm tengqm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the source code, there are comments like this:

		// This list is mix of mutating admission plugins and validating
		// admission plugins. The apiserver always runs the validating ones
		// after all the mutating ones, so their relative order in this list
		// doesn't matter.

So, if the code works as designed, the order should not matter. However, moving mutating ahead of validating sounds a better recommendation.

@hzxuzhonghu
Copy link
Member Author

@tengqm That's what i mean to, we should not mislead users.

@tengqm
Copy link
Contributor

tengqm commented Jan 12, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 12, 2018
@zacharysarah zacharysarah merged commit 7768029 into kubernetes:master Jan 12, 2018
chenopis added a commit that referenced this pull request Jan 12, 2018
…henopis-user-journeys

* 'master' of https://github.com/kubernetes/website:
  fix admission plugins orders (#6900)
  link to other terms (#6625)
  Fix See Also links. (#6948)
  --experimental-nvidia-gpus error (#6527)
  Add HA guide for kubeadm (#6458)
  remove duplicated and conflicted TOC instrument
  fix deadlink
  Clarify the token webhook posts the TokenReview object
  Update federation.md
  Update create-cluster-kubeadm.md
  Add note about viewing changes locally. (#6936)
  Create page that lists supported doc versions. (#6882)
  Document how to generate reference pages. (#6366)
  Update networking.md to list ACI CNI plugin (#6367)
  Update kubeadm-init.md (#6932)
  fix typo in doc kubeadm-alpha
  dulplicated sentences
  Update certificates.md
  Replace all kube-ui with kubernetes dashboard
  Add openstack internal load balancer option
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants