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

Add info back into configuration #1466

Merged
merged 4 commits into from
Jun 9, 2022

Conversation

leuyentran
Copy link
Contributor

@leuyentran leuyentran commented Jun 8, 2022

Change Overview

Add non-mandatory Azure information back into config structure. The purpose of such change, is to bring back information regarding Azure config such as AZURE_RESOURCE_MANAGER_ENDPOINT , AZURE_AD_ENDPOINT, AZURE_AD_RESOURCE, and AZURE_CLOUD_ENV_ID to callers and eventually end-users.

The end goal is to have all Azure config information will be available to end-user.

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Test Plan

  • 💪 Manual
    Manual test was ran downstream to ensure informations are being passed back
  • ⚡ Unit test
    Was run as a sanity check. No unit test added nor changed since there was no functional change
  • 💚 E2E

@github-actions
Copy link

github-actions bot commented Jun 8, 2022

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

@infraq infraq added this to In Progress in Kanister Jun 8, 2022
@leuyentran leuyentran requested a review from bathina2 June 8, 2022 21:02
@pavannd1 pavannd1 requested a review from ihcsim June 8, 2022 22:44
@pavannd1
Copy link
Contributor

pavannd1 commented Jun 8, 2022

Are the downstream callers being changed to use the config values from the map?

@leuyentran
Copy link
Contributor Author

Are the downstream callers being changed to use the config values from the map?

@pavannd1 Downstream callers will store these information alongside with clientID, tenantID, clientSecret and other config information. At the moment, it it intended for information surfacing, i.e. showing end-user the complete suite of config being used for Azure Client. No data management functional changes.

Kanister automation moved this from In Progress to Reviewer approved Jun 9, 2022
Copy link
Contributor

@bathina2 bathina2 left a comment

Choose a reason for hiding this comment

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

minor nit, will approve

pkg/blockstorage/azure/client.go Outdated Show resolved Hide resolved
leuyentran and others added 2 commits June 9, 2022 09:54
Co-authored-by: Sirish Bathina <sirish@kasten.io>
Copy link
Contributor

@ihcsim ihcsim left a comment

Choose a reason for hiding this comment

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

LGTM.

@mergify mergify bot merged commit 4678669 into master Jun 9, 2022
Kanister automation moved this from Reviewer approved to Done Jun 9, 2022
@mergify mergify bot deleted the Add-Azure-info-to-config-post-validation branch June 9, 2022 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants