Replace init-certs webhook initContainer with Helm template #5558
Conversation
ddl-ebrown
force-pushed
the
replace-webhook-cert-with-Helm-functions
branch
from
ba968ac
to
0ca2557
Compare
ddl-ebrown
changed the title
ddl-ebrown
force-pushed
the
replace-webhook-cert-with-Helm-functions
branch
from
0ca2557
to
3fd4ed9
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5558 +/- ##
=======================================
Coverage 60.98% 60.98%
=======================================
Files 796 796
Lines 51647 51647
=======================================
Hits 31498 31498
Misses 17249 17249
Partials 2900 2900
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ddl-ebrown
force-pushed
the
replace-webhook-cert-with-Helm-functions
branch
from
3fd4ed9
to
74a5b67
Compare
| @@ -56,7 +56,8 @@ ${GOPATH:-~/go}/bin/helm-docs -c ${DIR}/../charts/ | |||
|
|
|||
| # This section is used by GitHub workflow to ensure that the generation step was run | |||
| if [ -n "$DELTA_CHECK" ]; then | |||
| DIRTY=$(git status --porcelain) | |||
| # find only deleted or removed lines, not replaced values | |||
| DIRTY=$(git diff --word-diff | grep "^[{\[]") | |||
There was a problem hiding this comment.
The CI check needs to be updated to understand that some values may change when regenerating Helm charts
An alternative approach would be to add values.yaml entries and plumb them through -- but that feels a bit overkill
There was a problem hiding this comment.
It looks like a more fine-grained comparison. What do you think @eapolinario?
- Replicates the functionality from the webhook init-certs cli command from Flyte: https://github.com/flyteorg/flyte/blob/master/flytepropeller/pkg/webhook/init_cert.go This produces a ca.crt, tls.crt and tls.key value needed for the webhook, rather than needing to create a container that needs to have network and Kubernetes access. - Uses the Helm lookup helper to prevent regenerating on upgrades - Update CI check to only fail when lines are deleted or removed from the generated Helm output, not when values are modified Signed-off-by: ddl-ebrown <[email protected]>
ddl-ebrown
force-pushed
the
replace-webhook-cert-with-Helm-functions
branch
from
74a5b67
to
7070739
Compare
Great - thanks for double checking things on your end! We've been using this patch in our automated / CI deployments since I put up this PR and things are also working great on our side as well. |
Tracking issue
Why are the changes needed?
Removes unnecessary webhook initContainer
What changes were proposed in this pull request?
Replicates the functionality from the webhook init-certs cli command from Flyte:
https://github.com/flyteorg/flyte/blob/master/flytepropeller/pkg/webhook/init_cert.go
This produces a ca.crt, tls.crt and tls.key value needed for the webhook, rather than needing to create a container that needs to have network and Kubernetes access.
Uses the Helm lookup helper to prevent regenerating on upgrades
How was this patch tested?
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link