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

Azure: Dynamically get subscription ID(s) #1366

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

orestisfl
Copy link
Contributor

@orestisfl orestisfl commented Sep 26, 2023

Summary of your changes

Azure: Dynamically get subscription ID(s)

I've confirmed that with the ARM deployment method, this only retrieves one ID instead of the two I am able to see with my account. This is true both when the role assignment is in the resource group level and the subscription level.

Related Issues

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary README/documentation (if appropriate)

@orestisfl orestisfl self-assigned this Sep 26, 2023
@github-actions
Copy link

github-actions bot commented Sep 26, 2023

📊 Allure Report - 💚 No failures were reported.

Result Count
🟥 Failed 0
🟩 Passed 39
⬜ Skipped 1

I've confirmed that with the ARM deployment method, this only retrieves
one ID instead of the two I am able to see with my account. This is true
both when the role assignment is in the resource group level and the
subscription level.
@orestisfl orestisfl force-pushed the azure-subscription-ids branch from d3f8e3c to 2cd0780 Compare September 26, 2023 09:53
@orestisfl orestisfl marked this pull request as ready for review September 26, 2023 09:54
@orestisfl orestisfl requested a review from a team as a code owner September 26, 2023 09:54
}, nil
}

func (p *ProviderInitializer) getSubscriptionIds(ctx context.Context, azureConfig auth.AzureFactoryConfig) ([]*string, error) {
// TODO: mockable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Init() is not tested so I consider it out of scope to test this function for now, we should handle this technical debt after the feature freeze though

@orestisfl orestisfl enabled auto-merge (squash) September 28, 2023 11:25
@orestisfl orestisfl merged commit df48a53 into elastic:main Sep 28, 2023
@orestisfl orestisfl deleted the azure-subscription-ids branch November 14, 2023 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants