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
Adding an e2e test for admission webhook #54165
Adding an e2e test for admission webhook #54165
Conversation
f79e105
to
627a60d
Compare
4c17329
to
88a8121
Compare
fdeb1ed
to
db41f93
Compare
cluster/gce/config-default.sh
Outdated
@@ -175,7 +175,7 @@ if [[ ${KUBE_ENABLE_INSECURE_REGISTRY:-false} == "true" ]]; then | |||
fi | |||
|
|||
# Optional: customize runtime config | |||
RUNTIME_CONFIG="${KUBE_RUNTIME_CONFIG:-}" | |||
RUNTIME_CONFIG="${KUBE_RUNTIME_CONFIG:-admissionregistration.k8s.io/v1alpha1}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to be useful, e2e test says
I1018 22:29:18.099] [It] Should be able to deny pod creation
I1018 22:29:18.099] test/e2e/apimachinery/webhook.go:56
I1018 22:29:18.100] Oct 18 22:28:53.647: INFO: dynamic configuration of initializers requires the alpha admissionregistration.k8s.io group to be enabled
And apiserver log says:
I1018 22:17:17.431016 5 master.go:407] Skipping disabled API group "admissionregistration.k8s.io".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change it to RUNTIME_CONFIG=admissionregistration.k8s.io/v1alpha1
, or else it will still read in KUBE_RUNTIME_CONFIG
cluster/gce/config-default.sh
Outdated
@@ -175,7 +175,8 @@ if [[ ${KUBE_ENABLE_INSECURE_REGISTRY:-false} == "true" ]]; then | |||
fi | |||
|
|||
# Optional: customize runtime config | |||
RUNTIME_CONFIG="${KUBE_RUNTIME_CONFIG:-}" | |||
#RUNTIME_CONFIG="${KUBE_RUNTIME_CONFIG:-admissionregistration.k8s.io/v1alpha1}" | |||
RUNTIME_CONFIG="admissionregistration.k8s.io/v1alpha1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got the same result. In apiserver.log:
I1018 23:08:04.489359 5 master.go:407] Skipping disabled API group "admissionregistration.k8s.io".
and
I1018 23:08:03.520458 5 flags.go:52] FLAG: --runtime-config="batch/v2alpha1=true,extensions/v1beta1="
@krzyzacy help?
/retest |
d311f58
to
c129ae4
Compare
Webhook is broken at HEAD... |
/shrug |
c129ae4
to
6491f64
Compare
/assign @cheftako |
/release-note-none |
8c2c5df
to
f0e404e
Compare
I removed the mutual tls requirement from the webhook image. The test will still verify other parts of the webhook feature. I'll open an issue reminding ourselves to add the authn back after we reached agreement on what CA to use (#53828) and figure out how to plumb the CA to the |
framework.ExpectNoError(err, "registering webhook config %s with namespace %s", webhookConfigName, namespace) | ||
|
||
// The webhook configuration is honored in 1s. | ||
time.Sleep(2 * time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like this highlights the problem that we have no API to tell us which webhooks are enabled and functioning. I believe we are generally going to want that functionality and once we have it we can fix this sleep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tracked in #54712
5344f56
to
4aef071
Compare
Tracking the client authentication in #54709. |
4aef071
to
052be04
Compare
/lgtm |
/approve |
052be04
to
0f54fdb
Compare
Rebased. |
/approve no-issue |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caesarxuchao, cheftako, lavalamp No associated issue. Update pull-request body to add a reference to an issue, or get approval with The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
Automatic merge from submit-queue (batch tested with PRs 54165, 53909). If you want to cherry-pick this change to another branch, please follow the instructions here. |
Part of kubernetes/enhancements#492
The purpose of this test is making sure the webhooks get called, and the apiserver can communicate with the webhook.
We will expand the test cover more webhook features in followups.
The webhook used in the test rejects pods with container names "webhook-disallow". Will upload the source code of the example in a follow up PR.