Conversation
Add config validation webhook to Pilot e2e tests. Now that kubernetes/kubernetes#49987 is fixed validation webhooks can be enable on GKE >= 1.8. The following changes are made as part of this PR: * Rename external-admission-webhook-gke-1-7-x-workaround.sh to generate-webhook-cert.sh and refactor use cfssl and k8s API to generate and sign webhook certificate with k8s CA. * Refactored pilot and e2e test defaults to use `istio-pilot` instead of `istio-pilot-external` service name. The k8s api-server will invoke the istio-pilot's webhook service directly to validate config. * Added test code to generate self-signed CA and signed server cert/key. This enables validation webhook in automated e2e testing.
|
cc @wattli for cert management as defined by https://docs.google.com/document/d/1f61uHoPvozoVhlut6TG66L2XQ_DvFQWUUeFjHJeypHQ/edit#bookmark=id.74mjrvtdgjl4 cc @jmuk for equivalent mixer webhook validation integration |
|
@ayj: Adding do-not-merge/release-note-label-needed because the release note process has not been followed. DetailsOne of the following labels is required "release-note", "release-note-action-required", or "release-note-none". Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
So do we want to remove 1.7 workaround? I think the new script is a little different about the load balancer hack, which doesn't work with 1.7.x. |
|
Anyone using the workaround on GKE 1.7 is already on an alpha cluster. They would be likely to upgrade to 1.8.x anyway since alpha clusters expire after 30 days. The updated script should work for non-GKE clusters >= 1.7 and GKE >= 1.8. |
|
ah, fair enough, thanks. |
cmluciano
left a comment
There was a problem hiding this comment.
Cannot get this to work locally yet
| @@ -0,0 +1,128 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
Can you make this file executable?
There was a problem hiding this comment.
Thanks, fixed. I also changed to #!/bin/bash.
|
|
||
| set -e | ||
|
|
||
| function usage() { |
There was a problem hiding this comment.
This function specification does not work. You can remove the function keyword and just have usage.
| exit 1 | ||
| } | ||
|
|
||
| while [[ $# -gt 0 ]]; do |
There was a problem hiding this comment.
I'm unable to get this to work with the following example
./generate-webhook-cert.sh --service test --namespace default --secret webhook --secret-namespace default
There was a problem hiding this comment.
I'm unable to see or reproduce the error. Any additional information on what's not working here?
There was a problem hiding this comment.
ok I think I must have had a typo. It works with the latest patchset now.
* s/sh/bash * remove `function` keyword
Codecov Report
@@ Coverage Diff @@
## master #1987 +/- ##
==========================================
+ Coverage 81.81% 82.15% +0.33%
==========================================
Files 190 200 +10
Lines 15675 16273 +598
==========================================
+ Hits 12825 13369 +544
- Misses 2414 2466 +52
- Partials 436 438 +2
Continue to review full report at Codecov.
|
…hook-validation-by-default
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: cmluciano Assign the PR to them by writing The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these OWNERS Files:You can indicate your approval by writing |
Automatic merge from submit-queue. Revert servicecontrol adapter BUILD change from PR 1987 **What this PR does / why we need it**: Test code shouldn't be compiled into prod code. This reverts change from #1987. It appeared it was an accident. **Release note**: ```release-note NONE ```
Automatic merge from submit-queue. Revert servicecontrol adapter BUILD change from PR 1987 **What this PR does / why we need it**: Test code shouldn't be compiled into prod code. This reverts change from istio/istio#1987. It appeared it was an accident. **Release note**: ```release-note NONE ```
Add config validation webhook to Pilot e2e tests. Now that
kubernetes/kubernetes#49987 is fixed
validation webhooks can be enable on GKE >= 1.8. The following changes
are made as part of this PR:
Rename external-admission-webhook-gke-1-7-x-workaround.sh to
generate-webhook-cert.sh and refactor use cfssl and k8s API to
generate and sign webhook certificate with k8s CA.
Refactored pilot and e2e test defaults to use
istio-pilotinsteadof
istio-pilot-externalservice name. The k8s api-server willinvoke the istio-pilot's webhook service directly to validate
config.
Added test code to generate self-signed CA and signed server
cert/key. This enables validation webhook in automated e2e testing.
ref: #1478