Get ingress host and CA from space through connect #535
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
avalanche123
approved these changes
May 23, 2024
adamwg
approved these changes
May 23, 2024
RedbackThomson
force-pushed
the
kubeconfig-ca
branch
from
e9f96b6
to
32c9902
Compare
cbuto
reviewed
May 30, 2024
| nsn := types.NamespacedName{Name: space.Name, Namespace: space.Namespace} | ||
|
|
||
| // cache hit | ||
| if i, ok := c.ingresses[nsn]; ok { |
There was a problem hiding this comment.
do we need a mutex here? or use something like https://github.com/hashicorp/golang-lru to handle it for us
There was a problem hiding this comment.
I don't expect concurrent/multi-threaded access to any of these methods, but I am happy to move to using a purpose built cache just in case
cbuto
approved these changes
May 30, 2024
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.
Description of your changes
Closes https://github.com/upbound/team-orchestration/issues/99
Instead of connecting directly to the space using its FQDN and with skipping TLS, query the space through connect to get the ingress host and CA data. This allows for a TLS-secured connection to the space, and unlocks the ability to connect to "connected spaces" (assuming their host is accessible).
With the requirement to pass a context writer and now an ingress reader to a lot of different methods, I decided to wrap these into a new "context" object (
navCtx) to make things cleaner - rather than passing many args into every method.I have:
make reviewableto ensure this PR is ready for review.backport release-x.ylabels to auto-backport this PR, as appropriate.How has this code been tested
Blocked on testing in dev until https://github.com/upbound/spaces/pull/1067 is merged, but tested using stubbed responses.