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) update certificates when they change #2258

Merged
merged 9 commits into from
Feb 23, 2022

Conversation

rainest
Copy link
Contributor

@rainest rainest commented Feb 11, 2022

What this PR does / why we need it:
The admission controller doesn't update certificates when the file updates. This makes the controller update that certificate when the file updates).

Which issue this PR fixes:

fixes #986 (only for files though, we can't reload/change the environment after start)

Fixes a CI issue where targeted runs weren't actually using their custom image.

Special notes for your reviewer:
https://github.com/Kong/kubernetes-ingress-controller/actions/runs/1866651736 is an example run with the change.

This is not yet supported in the chart: Kong/charts#543

This additionally reorganizes the E2E tests, which had almost everything in all_in_one_test.go.

Minor bugfixes and changes to the E2E port forward helper, because I originally tried to use that with weird results (forwards would inexplicably and randomly die after coming up), so I exposed the service instead.

This adds a new E2E helper to retrieve logs from the test cluster. This isn't actually used because I didn't have a good way to have it only log when it's of interest (when deployKong() fails), and it's spammy otherwise. It's still useful for debugging CI-specific issues, but you need to add it to a debug branch temporarily for now. We may be able to merge in use by refactoring around https://pkg.go.dev/github.com/stretchr/testify/suite?utm_source=godoc#AfterTest, unsure. For reference, actually using it looks like log.diff.txt

PR Readiness Checklist:

Complete these before marking the PR as ready to review:

  • the CHANGELOG.md release notes have been updated to reflect any significant (and particularly user-facing) changes introduced by this PR

@rainest rainest requested a review from a team as a code owner February 11, 2022 23:16
@rainest rainest temporarily deployed to Configure ci February 11, 2022 23:16 Inactive
@rainest rainest temporarily deployed to Configure ci February 11, 2022 23:18 Inactive
@rainest rainest temporarily deployed to Configure ci February 11, 2022 23:23 Inactive
@rainest rainest temporarily deployed to Configure ci February 11, 2022 23:34 Inactive
@rainest rainest temporarily deployed to Configure ci February 11, 2022 23:49 Inactive
@rainest rainest marked this pull request as draft February 12, 2022 00:13
@rainest
Copy link
Contributor Author

rainest commented Feb 12, 2022

E2E in CI seems to not flip after 4m, but does locally. Not sure if there's any explanation beyond CI being slower--even locally it does take quite a while after pushing the update for it to be visible. Will poke around.

@rainest rainest temporarily deployed to Configure ci February 14, 2022 19:29 Inactive
@rainest rainest temporarily deployed to Configure ci February 14, 2022 19:44 Inactive
@rainest rainest temporarily deployed to Configure ci February 15, 2022 21:27 Inactive
@rainest rainest temporarily deployed to Configure ci February 15, 2022 21:36 Inactive
@rainest rainest temporarily deployed to Configure ci February 15, 2022 21:40 Inactive
@rainest rainest temporarily deployed to Configure ci February 15, 2022 21:40 Inactive
@rainest rainest temporarily deployed to Configure ci February 15, 2022 21:55 Inactive
@rainest rainest temporarily deployed to Configure ci February 15, 2022 22:17 Inactive
@rainest rainest temporarily deployed to Configure ci February 15, 2022 22:19 Inactive
@rainest rainest temporarily deployed to Configure ci February 15, 2022 22:35 Inactive
@rainest rainest temporarily deployed to Configure ci February 18, 2022 21:20 Inactive
@rainest rainest temporarily deployed to Configure ci February 18, 2022 21:29 Inactive
@rainest rainest temporarily deployed to Configure ci February 18, 2022 21:29 Inactive
@rainest rainest temporarily deployed to Configure ci February 18, 2022 21:30 Inactive
@rainest rainest temporarily deployed to Configure ci February 18, 2022 21:47 Inactive
@rainest rainest marked this pull request as ready for review February 18, 2022 22:03
internal/admission/server.go Show resolved Hide resolved
internal/admission/server.go Show resolved Hide resolved
internal/admission/server.go Outdated Show resolved Hide resolved
internal/admission/server.go Outdated Show resolved Hide resolved
test/e2e/features_test.go Show resolved Hide resolved
test/e2e/features_test.go Outdated Show resolved Hide resolved
test/e2e/features_test.go Outdated Show resolved Hide resolved
test/e2e/features_test.go Show resolved Hide resolved
test/e2e/utils_test.go Outdated Show resolved Hide resolved
test/e2e/utils_test.go Show resolved Hide resolved
Co-authored-by: Shane Utt <shaneutt@linux.com>
@rainest rainest temporarily deployed to Configure ci February 22, 2022 22:32 Inactive
@rainest rainest temporarily deployed to Configure ci February 22, 2022 22:32 Inactive
@rainest rainest temporarily deployed to Configure ci February 22, 2022 22:47 Inactive
@rainest rainest temporarily deployed to Configure ci February 22, 2022 23:30 Inactive
@rainest rainest temporarily deployed to Configure ci February 22, 2022 23:31 Inactive
@rainest rainest temporarily deployed to Configure ci February 22, 2022 23:34 Inactive
@rainest rainest temporarily deployed to Configure ci February 22, 2022 23:34 Inactive
@rainest rainest temporarily deployed to Configure ci February 22, 2022 23:38 Inactive
@rainest rainest temporarily deployed to Configure ci February 22, 2022 23:38 Inactive
@rainest rainest enabled auto-merge (squash) February 22, 2022 23:38
@rainest rainest temporarily deployed to Configure ci February 22, 2022 23:53 Inactive
@shaneutt shaneutt self-requested a review February 23, 2022 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically handle updated certificates
3 participants