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
Use GetCloud from grafana-azure-sdk-go to get resource graph scope and fallback to using clusterUrl for ADX scope for unknown clouds #868
Conversation
return scopes, nil | ||
scopeTmpl, ok := "", false | ||
if scopeTmpl, ok = adxScopes[azureCloud]; !ok { | ||
scopeTmpl = "{clusterUrl}/.default" |
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.
I'd add a clarifying comment here that this is applicable for all other clouds
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, just one nit in the comments to address 😊 there's also a merge conflict that needs resolving
…audienceToScopes pattern found elsewhere in Grafana
3458a5c
to
c95b293
Compare
@aangelisc thanks for the review. I have fixed the merge conflict and added the comment - please take another look. |
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.
Thanks @JonCole!
Once #875 is merged then we can merge this PR 😊 |
Using GetCloud from the azure sdk package gives us a consolidated location to get information about a specific cloud, allowing easier expansion of supported clouds in the future. Also, even though Azure Public and Azure China use support special scopes, using the cluster url in the scope is the recommended practice for other clouds.
Fixes #877