-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Enable Server Side Authentication when using the Kubernetes Proxy #16649
Enable Server Side Authentication when using the Kubernetes Proxy #16649
Conversation
Changed Packages
|
Uffizzi Ephemeral Environment Deploying☁️ https://app.uffizzi.com/github.com/backstage/backstage/pull/16649 ⚙️ Updating now by workflow run 4558169106. What is Uffizzi? Learn more! |
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.
A neat improvement here that might be nice to declare in a changeset is that KubernetesBuilder
now has a method (currently named setAuthTranslatorMap
) which allows integrators to bring their own KubernetesAuthTranslator
s without forking this plugin.
Furthermore, can we maybe rename KubernetesAuthTranslatorGenerator
? Since it's no longer a factory function, it's not really "generating" anything -- rather it's dispatching calls to its decorateClusterDetailsWithAuth
method to particular translators based on their name. Maybe DispatchingKubernetesAuthTranslator
, DelegatingKubernetesAuthTranslator
, or maybe even just the plural KubernetesAuthTranslators
in keeping with the plural KubernetesAuthProviders
in the frontend?
.../kubernetes-backend/src/kubernetes-auth-translator/KubernetesAuthTranslatorGenerator.test.ts
Outdated
Show resolved
Hide resolved
.../kubernetes-backend/src/kubernetes-auth-translator/KubernetesAuthTranslatorGenerator.test.ts
Outdated
Show resolved
Hide resolved
.../kubernetes-backend/src/kubernetes-auth-translator/KubernetesAuthTranslatorGenerator.test.ts
Outdated
Show resolved
Hide resolved
.../kubernetes-backend/src/kubernetes-auth-translator/KubernetesAuthTranslatorGenerator.test.ts
Outdated
Show resolved
Hide resolved
.../kubernetes-backend/src/kubernetes-auth-translator/KubernetesAuthTranslatorGenerator.test.ts
Outdated
Show resolved
Hide resolved
.../kubernetes-backend/src/kubernetes-auth-translator/KubernetesAuthTranslatorGenerator.test.ts
Outdated
Show resolved
Hide resolved
f5330a5
to
cfe2fe1
Compare
plugins/kubernetes-backend/src/service/KubernetesProxy.test.ts
Dismissed
Show dismissed
Hide dismissed
cfe2fe1
to
5ec2d87
Compare
@mclarke47 do you wanna take a look at this? 🙏 |
This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If the PR was closed and you want it re-opened, let us know and we'll re-open the PR so that you can continue the contribution! |
@benjdlambert @mclarke47 removing stale label? |
5ec2d87
to
d7ef541
Compare
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.
Looking pretty good to me, just one thing to fix.
I wonder if any of these docs need updating https://github.com/backstage/backstage/blob/36e81329174addc261646bae19eeead8d5a0fd93/docs/features/kubernetes/proxy.md also?
While we're here, it does seem like a good idea to remove this disclaimer in the docs: backstage/docs/features/kubernetes/proxy.md Lines 65 to 71 in aefc5a2
And maybe add some discussion of backstage/docs/features/kubernetes/proxy.md Lines 49 to 61 in aefc5a2
|
d7ef541
to
400f54f
Compare
I added a small section under 'How it works' to explain that each request is now decorated with auth by default in this current state. Also removed disclaimer under the 'Authentication section'. |
7798c1c
to
859d545
Compare
plugins/kubernetes-backend/src/service/KubernetesProxy.test.ts
Dismissed
Show dismissed
Hide dismissed
plugins/kubernetes-backend/src/service/KubernetesProxy.test.ts
Dismissed
Show dismissed
Hide dismissed
plugins/kubernetes-backend/src/service/KubernetesProxy.test.ts
Dismissed
Show dismissed
Hide dismissed
...ubernetes-backend/src/kubernetes-auth-translator/DispatchingKubernetesAuthTranslator.test.ts
Outdated
Show resolved
Hide resolved
859d545
to
f5bff2a
Compare
…e a translator map as a parameter to use when decorating cluster details Signed-off-by: Ruben Vallejo <rvallejo@vmware.com>
…r provided by the KubernetesFanOutHandler constructor Signed-off-by: Ruben Vallejo <rvallejo@vmware.com>
Signed-off-by: Ruben Vallejo <rvallejo@vmware.com>
f5bff2a
to
a3439dd
Compare
@benjdlambert just wanted to check in to see if there were any other changes you wanted me to address? |
Thanks! Seems like @backstage/kubernetes-maintainers are happy so gonna merge this and get it in the |
Thank you for contributing to Backstage! The changes in this pull request will be part of the |
Hey, I just made a Pull Request!
Current State:
The Kubernetes Proxy requires authentication headers as part of its request in order to display k8s resource information.
The
KubernetesAuthTranslatorGenerator
class uses a static class method to instantiate a variety of different auth translators depending on the authProvider that is provided.Suggested Change:
Move the dependency of
kubernetesAuthTranslator
creation to the plugin that requires it and provide it via atranslatorMap
parameter to theKubernetesAuthTranslatorGenerator
Provide Backstages Integrator access to k8s operations by default without the need to negotiate k8s tokens client-side. This will allow users to consume plugins with complex k8s logic without having kubectl access themselves.
With these suggested changes a user can create a kind cluster to test. Provision a high powered service account token.
and use:
to see all the namespaces on that kind cluster without the need of supplying authentication headers.
✔️ Checklist
Signed-off-by
line in the message. (more info)