-
Notifications
You must be signed in to change notification settings - Fork 463
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
[1.17] Warn on missing TLS secret #9938
Merged
soloio-bulldozer
merged 7 commits into
v1.17.x
from
jbohanon/backports/v1.17.x/missing-tls-secret
Aug 26, 2024
Merged
[1.17] Warn on missing TLS secret #9938
soloio-bulldozer
merged 7 commits into
v1.17.x
from
jbohanon/backports/v1.17.x/missing-tls-secret
Aug 26, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* update api and code * codegen * kubernetes e2e * fix tests * add changelog * fixes add warnings to proxy report so it appears in the warnings after translation only return a warning if the error produced by ResolveCommonSslConfig is SslSecretNotFoundError * Adding changelog file to new location * Deleting changelog file from old location * fix listener_subsystem_test * tee gha output and grep for success/fail * fix kubernetes e2e test * kube2e * update action * unset ns env var when test installation run finishes * fix helm test import * revert gha * Adding changelog file to new location * Deleting changelog file from old location * PR feedback * update comment * add settings API for warning * settings option for warning instead of error * update changelog * helm and tests * fix build issues and codegen * missing curlies >_> * helm values fixes * missed one * wrong value in test setup * helm tests are actually passing now... excellent... * fix translation tests * put breaking change verbage in changelog * add setting to preserve missing secret error to test manifest * revert allow_warnings test * add icky sleep * expand admin server assertions, move server tls test * remove extra skeleton * rename bool and fix logic * update setting in always accept test --------- Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com> Co-authored-by: changelog-bot <changelog-bot>
github-actions
bot
added
the
keep pr updated
signals bulldozer to keep pr up to date with base branch
label
Aug 23, 2024
Issues linked to changelog: |
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.
LGTM!
bewebi
reviewed
Aug 26, 2024
sam-heilbron
approved these changes
Aug 26, 2024
bewebi
approved these changes
Aug 26, 2024
soloio-bulldozer
bot
deleted the
jbohanon/backports/v1.17.x/missing-tls-secret
branch
August 26, 2024 17:04
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport #9875
Description
Updates the condition of a VirtualService referencing a TLS secret that does not exist from an error state to a warning state. This is to allow for eventual consistency with VS creation and TLS secret creation.
Fill out any of the following sections that are relevant and remove the others
API changes
Code changes
Docs changes
TODO
Context
Users ran into this eventual consistency issue when applying a cert-manager
Certificate
resource at the same time as aVirtualService
resource. Because theCertificate
does not synchronously create the TLS secret, theVirtualService
is rejected by validation.Interesting decisions
--connect-to
flag. this is to be less intrusive than refactoring the curl tool to support both--connect-to
AND--resolve
.Testing steps
# if you don't have a cluster, create one kind create cluster
# curl to validate that we're getting traffic curl -k --connect-to vs-1:8443:127.0.0.1 https://vs-1:8443
# curl to show we are still receiving traffic curl -k --connect-to vs-1:8443:127.0.0.1 https://vs-1:8443
# restart gloo deployment to roll the pod k rollout restart deploy/gloo -n gloo-system k rollout status deploy/gloo -n gloo-system
# curl to show that we are NO LONGER receiving traffic, even on the good VS curl -k --connect-to vs-1:8443:127.0.0.1 https://vs-1:8443
# restart gloo deployment to roll the pod k rollout restart deploy/gloo -n gloo-system k rollout status deploy/gloo -n gloo-system
# curl to show that we are receiving traffic on the good VS, but not on the invalid VS curl -k --connect-to vs-1:8443:127.0.0.1 https://vs-1:8443 curl -k --connect-to vs-2:8443:127.0.0.1 https://vs-2:8443
# curl to show that we are receiving traffic on both, now valid VS curl -k --connect-to vs-1:8443:127.0.0.1 https://vs-1:8443 curl -k --connect-to vs-2:8443:127.0.0.1 https://vs-2:8443
Checklist: