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

vendor: upgrade Azure SDK and Azure/go-autorest #23496

Merged
merged 1 commit into from
Dec 2, 2019

Conversation

apparentlymart
Copy link
Contributor

This involves some minor changes to the "azure" backend code to account for upstream breaking changes. This fixes #23493.

However, I don't have a functioning Azure environment to test against, so I'll need the help of someone on the Azure provider team to take this for a spin and make sure it's otherwise working as expected. There are acceptance tests in this package which should can hopefully give us a good signal to start, but I'm not sure if they have coverage for SAS tokens in particular.

This involves some minor changes to the "azure" backend code to account
for upstream breaking changes.
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@tombuildsstuff
Copy link
Contributor

Backend tests pass:

Screenshot 2019-11-26 at 10 35 14

@phekmat
Copy link
Contributor

phekmat commented Nov 26, 2019

Can we add a warning to the docs around using https,http as a value for spr when using a SAS token? The SDK behavior there of defaulting to HTTP is surprising and dangerous IMO.

@pselle
Copy link
Contributor

pselle commented Dec 2, 2019

@phekmat You make a good point, however that is a separate concern from the PR upgrading the dependencies ... the docs are in this repo if you'd like to make a PR, or open an issue (some direction on wording the warning would be helpful) and we can pick it up.

@apparentlymart apparentlymart merged commit 6db3cf8 into master Dec 2, 2019
@apparentlymart apparentlymart deleted the b-azure-backend-https branch December 2, 2019 18:04
@ghost
Copy link

ghost commented Mar 28, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

State transmitted in cleartext for azurerm backend with SAS token
4 participants