forked from criticalstack/e2d
-
Notifications
You must be signed in to change notification settings - Fork 2
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 backups #2
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
9c86ab7
Add initial azure backup code
samfreiberg-oh 8f29376
Move container name into config
samfreiberg-oh 377b2e3
Add cli flags for Azure
samfreiberg-oh 8909b05
Add end to end tests.
samfreiberg-oh 9b3fad3
Make concrete implementation of azure snapshotter private
samfreiberg-oh 12186bf
Add comments for public types/funcs
samfreiberg-oh cbb9523
Remove debug statements
samfreiberg-oh 359bf9f
Code cleanup
samfreiberg-oh e9f2689
Refactor code and cleanup dangling io.ReadCloser
samfreiberg-oh 52cc9e0
Make Azure timeout configurable
samfreiberg-oh 6f3e7c5
Add option for max retries in azure
samfreiberg-oh c641a48
Fix comment alignment
samfreiberg-oh f507e3a
Add azure to the list of valid schemes
samfreiberg-oh cc62895
Remove todo
samfreiberg-oh d857480
Fix documentation
samfreiberg-oh 30573f3
Document and cleanup end to end test
samfreiberg-oh 6fe6284
Add Azure Managed Identity
samfreiberg-oh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Azure recommends avoiding using Account Keys since they're not unique to a user, and instead using Managed Service Identities.
Unfortunately, like everything Azure, it's not super straightforward to use a MSI since there's multiple types of MSIs:
It might be good to just add a
--azure-msi-id
flag to use a specific UUID if the system managed identity isn't sufficient.There's no need to remove the account key flag, since some users (not us) may likely be using the legacy azure permission model (or are testing from their laptops!)
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.
Done.