-
Notifications
You must be signed in to change notification settings - Fork 71
Update Helm chart to include pod mutating webhook for readiness gates #612
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
Changes from all commits
7437063
5b45e1d
b601b06
f5c4145
532bc8f
1e77fd5
404eabd
0102af8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,16 +101,8 @@ docker-push: ## Push docker image with the manager. | |
| # also generates a placeholder cert for the webhook - this cert is not intended to be valid | ||
| .PHONY: build-deploy | ||
| build-deploy: ## Create a deployment file that can be applied with `kubectl apply -f deploy.yaml` | ||
| $(eval TEMP_KEY := $(shell mktemp)) | ||
| $(eval TEMP_CERT := $(shell mktemp)) | ||
| cd config/manager && kustomize edit set image controller=${ECRIMAGES} | ||
| kustomize build config/default > deploy.yaml | ||
| openssl req -x509 -nodes -days 1 -newkey rsa:2048 -keyout $(TEMP_KEY) -out $(TEMP_CERT) -subj "/CN=not-a-real-cn/O=not-a-real-o" > /dev/null 2>&1 | ||
| export KEY_B64=`cat $(TEMP_KEY) | base64` && \ | ||
| export CERT_B64=`cat $(TEMP_CERT) | base64` && \ | ||
| yq -i e '(.[] as $$item | select(.metadata.name == "webhook-cert" and .kind == "Secret") | .data."tls.crt") = env(CERT_B64)' deploy.yaml && \ | ||
| yq -i e '(.[] as $$item | select(.metadata.name == "webhook-cert" and .kind == "Secret") | .data."tls.key") = env(KEY_B64)' deploy.yaml 2>&1 | ||
| rm $(TEMP_KEY) $(TEMP_CERT) | ||
|
|
||
| .PHONY: manifest | ||
| manifest: ## Generate CRD manifest | ||
|
|
@@ -155,11 +147,8 @@ docs: | |
| mkdocs build | ||
|
|
||
| # NB webhook tests can only run if the controller is deployed to the cluster | ||
| webhook-e2e-test-namespace := "webhook-e2e-test" | ||
|
|
||
|
Comment on lines
-158
to
-159
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The webhook tests don't actually use this namespace. Instead they create new ones from within the tests. |
||
| .PHONY: webhook-e2e-test | ||
| webhook-e2e-test: | ||
| @kubectl create namespace $(webhook-e2e-test-namespace) > /dev/null 2>&1 || true # ignore already exists error | ||
| LOG_LEVEL=debug | ||
| cd test && go test \ | ||
| -p 1 \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,3 +30,21 @@ If release name contains chart name it will be used as a full name. | |
| {{- define "service-account.name" -}} | ||
| {{ default "default" .Values.serviceAccount.name }} | ||
| {{- end -}} | ||
|
|
||
| {{/* Import or generate certificates for webhook */}} | ||
| {{- define "aws-gateway-controller.webhookTLS" -}} | ||
| {{- if (and .Values.webhookTLS.caCert .Values.webhookTLS.cert .Values.webhookTLS.key) -}} | ||
| caCert: {{ .Values.webhookTLS.caCert }} | ||
| cert: {{ .Values.webhookTLS.cert }} | ||
| key: {{ .Values.webhookTLS.key }} | ||
| {{- else -}} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's something we can consider adding? For now if folks use the auto-installed cert the new one will be just as good. If they are using their own cert the same values for ca/cert/key should work across installs. |
||
| {{- $ca := genCA "aws-gateway-controller-ca" 36500 -}} | ||
| {{- $serviceDefaultName:= printf "webhook-service.%s.svc" .Release.Namespace -}} | ||
| {{- $secretName := "webhook-cert" -}} | ||
| {{- $altNames := list ($serviceDefaultName) (printf "%s.cluster.local" $serviceDefaultName) -}} | ||
| {{- $cert := genSignedCert $serviceDefaultName nil $altNames 36500 $ca -}} | ||
| caCert: {{ $ca.Cert | b64enc }} | ||
| cert: {{ $cert.Cert | b64enc }} | ||
| key: {{ $cert.Key | b64enc }} | ||
| {{- end -}} | ||
| {{- end -}} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| {{ $tls := fromYaml ( include "aws-gateway-controller.webhookTLS" . ) }} | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I learned through this process that the |
||
| --- | ||
| apiVersion: admissionregistration.k8s.io/v1 | ||
| kind: MutatingWebhookConfiguration | ||
| metadata: | ||
| name: aws-appnet-gwc-mutating-webhook | ||
| webhooks: | ||
| - admissionReviewVersions: | ||
| - v1 | ||
| clientConfig: | ||
| caBundle: {{ $tls.caCert }} | ||
| service: | ||
| name: webhook-service | ||
| namespace: {{ .Release.Namespace }} | ||
| path: /mutate-pod | ||
| failurePolicy: Fail | ||
| name: mpod.gwc.k8s.aws | ||
| rules: | ||
| - apiGroups: | ||
| - "" | ||
| apiVersions: | ||
| - v1 | ||
| operations: | ||
| - CREATE | ||
| resources: | ||
| - pods | ||
| sideEffects: None | ||
| namespaceSelector: | ||
| matchExpressions: | ||
| - key: application-networking.k8s.aws/pod-readiness-gate-inject | ||
| operator: In | ||
| values: | ||
| - enabled | ||
| objectSelector: | ||
| matchExpressions: | ||
| - key: app.kubernetes.io/name | ||
| operator: NotIn | ||
| values: | ||
| - gateway-api-controller | ||
| --- | ||
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: webhook-service | ||
| namespace: {{ .Release.Namespace }} | ||
| spec: | ||
| ports: | ||
| - port: 443 | ||
| targetPort: webhook-server | ||
| selector: | ||
| control-plane: gateway-api-controller | ||
| --- | ||
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: webhook-cert | ||
| namespace: {{ .Release.Namespace }} | ||
| type: kubernetes.io/tls | ||
| data: | ||
| ca.crt: {{ $tls.caCert }} | ||
| tls.crt: {{ $tls.cert }} | ||
| tls.key: {{ $tls.key }} | ||
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.
Did you test github worker node can run the
make webhook-e2e-testsuccess? or hard to test it until this code merge to mainline?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.
I didn't. I think it does a Helm install before this so it should "just work".