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

Update go-autorest to v9.10.0 #18877

Merged
merged 1 commit into from
Oct 15, 2018
Merged

Conversation

gsacavdm
Copy link

Update go-autorest to v9.10.0 to fix #18875 since go-autorest v9.7.0 has the required fix necessary to address the Azure AD auth endpoint update for Azure US Government. See announcement. While we could limit the update to v.9.7.0, might as well go to the highest non-major update for that version.

As indicated in the issue, even though this upgrades from 8.3 to 9.10, this isn't really a major update.

As per which references the go-autorest's changelog notes on v9.0.0:

IMPORTANT: This release was initially labeled incorrectly as v8.4.0. From the time it was released, it should have been marked v9.0.0 because it contains breaking changes to the MSI packages. We apologize for any inconvenience this causes.

Since upgrading from v8.3.0 to v.8.4.0 isn't a major update and based on the note above, upgrading from v8.4.0/v9.0.0 to v9.7.0 isn't upgrading across major releases either, this should be a low risk upgrade. Also, no breaking changes are indicated in the release that occurred between these versions.

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.

hey @gsacavdm

Thanks for this PR - taking a look through could we update the vendored code to use the tag (v9.10.0) rather than just pinning to a given commit - this'd ensure the same version of AutoRest is used across the vendored code.

Thanks!

vendor/vendor.json Show resolved Hide resolved
vendor/github.com/Azure/go-autorest/version/version.go Outdated Show resolved Hide resolved
@gsacavdm
Copy link
Author

Fixed @tombuildsstuff :)

@gsacavdm
Copy link
Author

Ping @tombuildsstuff

@tombuildsstuff
Copy link
Contributor

tombuildsstuff commented Sep 25, 2018

@gsacavdm I've pushed a commit to update the vendoring to use v9.10.0 (rather than v11.x) - hope you don't mind. Going to ask someone else internally to review this since I've pushed those changes.

@tombuildsstuff tombuildsstuff added the dependencies Auto-pinning label Sep 25, 2018
@gsacavdm
Copy link
Author

Do you mean 9.10 or 9.1? I'm assuming 9.10 which is ok given that the required change is on v9.7, so anything greater than that is ok. Do you need anything from me to make this happen?

@gsacavdm
Copy link
Author

gsacavdm commented Oct 8, 2018

Hey @tombuildsstuff , any updates on getting this merged?

@paultyng
Copy link
Contributor

paultyng commented Oct 9, 2018

We are planning to get this in for the next TF patch release, just need to finish up some testing I believe.

@tombuildsstuff
Copy link
Contributor

@gsacavdm as @paultyng has mentioned this'll go out as a part of the next patch release - we just need to do some edge-case testing to confirm there's nothing else broken here (which is on my list for this week) - sorry for the delay here!

@paultyng paultyng merged commit f74774f into hashicorp:master Oct 15, 2018
@gsacavdm gsacavdm deleted the update-goautorest branch October 15, 2018 16:56
@gsacavdm
Copy link
Author

Hurray! Thanks guys!

@ghost
Copy link

ghost commented Apr 1, 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 Apr 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dependencies Auto-pinning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Authentication for azurerm module with environment usgovernment uses incorrect AAD endpoint
3 participants