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

Discover based on subscription tags #557

Merged
merged 5 commits into from
Dec 12, 2022

Conversation

simongottschlag
Copy link
Member

@simongottschlag simongottschlag commented Nov 16, 2022

This PR introduces a way to run discover and menu without having Directory reader role (or permission to read all applications which is required to use them today).

To use this, tags starting with _azad-kube-proxy can be added to the subscriptions that we think the users have access to (having read access to a resource group in the subscription grants you access to read the subscription tags).

Example:

_azad-kube-proxy_dev={"cluster_name":"aks-dev-we","resource":"https://aks-we.dev.foobar.io","proxy_url":"https://aks-we.dev.foobar.io"}
_azad-kube-proxy_qa={"cluster_name":"aks-qa-we","resource":"https://aks-we.qa.foobar.io","proxy_url":"https://aks-we.qa.foobar.io"}
_azad-kube-proxy_prod={"cluster_name":"aks-prod-we","resource":"https://aks-we.prod.foobar.io","proxy_url":"https://aks-we.prod.foobar.io"}

Fixes #155

@simongottschlag simongottschlag added the enhancement New feature or request label Nov 16, 2022
@simongottschlag simongottschlag self-assigned this Nov 16, 2022
@coveralls
Copy link

coveralls commented Nov 16, 2022

Coverage Status

Coverage decreased (-2.8%) to 86.781% when pulling 3fb0f9c on discovery-subscription-tags into c108ae3 on main.

@simongottschlag simongottschlag marked this pull request as ready for review November 16, 2022 21:57
Copy link
Contributor

@NissesSenap NissesSenap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one minor comment which is a personal preference of mine. I understand if you don't agree.
Thanks for quick action :)

cmd/kubectl-azad-proxy/actions/discover.go Outdated Show resolved Hide resolved
Meeting @NissesSenap's review comment.
@NissesSenap
Copy link
Contributor

@phillebaba do you have any thoughts on this PR?

@simongottschlag simongottschlag merged commit 8492531 into main Dec 12, 2022
@simongottschlag simongottschlag deleted the discovery-subscription-tags branch December 12, 2022 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ducument required Azure AD permissions to use discovery
3 participants