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

Fix go modules import issues #524

Closed
wants to merge 1 commit into from
Closed

Conversation

AdamSLevy
Copy link

@AdamSLevy AdamSLevy commented May 29, 2020

  • Remove all nested go.mod and go.sum files.
  • Add single top level go.mod and go.sum files with module import path
    "github.com/Azure/go-autorest/v15"
  • Replace all internal import "github.com/Azure/go-autorest/..." paths
    with "github.com/Azure/go-autorest/v15/..."
  • Run go test && go mod tidy

Thank you for your contribution to Go-AutoRest! We will triage and review it as soon as we can.

Fix #481

As part of submitting, please make sure you can make the following assertions:

  • [ x ] I've tested my changes, adding unit tests if applicable.
  • [ x ] I've added Apache 2.0 Headers to the top of any new source files.
  • [ x ] I'm submitting this PR to the dev branch, except in the case of urgent bug fixes warranting their own release.
  • If I'm targeting master, I've updated CHANGELOG.md to address the changes I'm making.

- Remove all nested go.mod and go.sum files.
- Add single top level go.mod and go.sum files with module import path
  "github.com/Azure/go-autorest/v15"
- Replace all internal import "github.com/Azure/go-autorest/..." paths
  with "github.com/Azure/go-autorest/v15..."
- Run `go test && go mod tidy`
@msftclas
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ AdamSLevy sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

@jhendrixMSFT
Copy link
Member

Having separate modules it to allow user-code (e.g. adal package) to version independently from code used purely by the code generator (e.g. validation package). I would prefer to fix what we have if possible, and if we conclude it cannot be fixed then we can turn this into one big module.

@AdamSLevy
Copy link
Author

AdamSLevy commented May 29, 2020

I highly encourage you to read up on Golang modules then. From everything I read in the issue I strongly feel that you are trying to fit a square peg in a round hole on this effort. This is further evidenced by the numerous "go_mod_tidy_hack.go" files. Go modules works very well when you work with it, but you are going against the grain. go modules has no way to actually know what to do with these mixed go.mod files that have not been properly kept up to date.

Code that is developed together, is interdependent, and is effectively distributed and released together, needs to be versioned together. A module is intended to version a set of interdependent and co-developed packages together.

But regardless of your opinion on where the API boundaries should lay, the current approach of putting a go.mod file in each sub-package is completely untenable. It is causing rather large headaches downstream, and the intention is not clear to outside developers.

Legitimately asking: Why would I ever want to use two different, possibly incompatible (how would I even know?), versions of the adal code and the validation code? Moreover, why does a user need to import the validation code at all? If they don't, then just move that package to an internal folder, and then make whatever changes you need to it without worrying about changing its API for consumers.

If you aren't ready to have a stable API, that's ok too, but you should re-release this repo under a new import path and stick with a major version of v0, which does not promise any API stability to users.

Additionally version tags allow you to freely develop with breaking API changes on master. go modules will only pull the latest tagged version.

One important rule that you are breaking, is that major version changes require a different import path. This is solved by adding /vXX/ right after the module import path and is based on the repo version tag. But with each package its own module, each not declaring their major version in their own import path in the go.mod file, go mod has no way to know what the version of each sub package should be, or which combinations of interdependent versions are acceptable.

I hope you reconsider changing this repo to be inline with the rules of go modules. At the moment this package is very difficult to use confidently.

@jhendrixMSFT
Copy link
Member

Code that is developed together, is interdependent, and is effectively distributed and released together, needs to be versioned together. A module is intended to version a set of interdependent and co-developed packages together.

I agree with this. What I was stating earlier is that not all of the packages in this repo are interdependent and should be allowed to version at their own cadence.

But regardless of your opinion on where the API boundaries should lay, the current approach of putting a go.mod file in each sub-package is completely untenable. It is causing rather large headaches downstream, and the intention is not clear to outside developers.

This is a good point. If we continue to keep multiple modules I believe this can be solved with documentation and examples.

Legitimately asking: Why would I ever want to use two different, possibly incompatible (how would I even know?), versions of the adal code and the validation code? Moreover, why does a user need to import the validation code at all? If they don't, then just move that package to an internal folder, and then make whatever changes you need to it without worrying about changing its API for consumers.

The validation package is used by methods created by our code generator, see here for an example; there's nothing in that package that's useful for SDK consumers. Mixing it (let's call this "non-user code") in with user-consumable code (e.g. auth and to packages are the most common) means that user-code has to pay the tax of a major version bump when a breaking change is introduced in non-user code. It would be ideal to allow user and non-user code to independently version. (Really, the non-user code should have been internal packages in the SDK repo which likely would have avoided this).

One important rule that you are breaking, is that major version changes require a different import path. This is solved by adding /vXX/ right after the module import path and is based on the repo version tag. But with each package its own module, each not declaring their major version in their own import path in the go.mod file, go mod has no way to know what the version of each sub package should be, or which combinations of interdependent versions are acceptable.

All of the modules in this repo are v0 which precludes the need for a major version suffix. This was an intentional decision to test out the current strategy (without introducing any breaking changes due to semantic import versioning), providing a bit of latitude if we need to refactor the module boundaries.

Multi-module repositories are supported, and there is real value in having user and non-user code independently version. So before we just abandon this split I think it's worth due diligence to ensure if it will/won't reliably work.

jhendrixMSFT added a commit that referenced this pull request Jun 23, 2020
* Fix Go module ambiguous import errors

This is an extension of the mitigations introduced in #455.
Unfortunately, the original mitigations didn't address the primary cause
of ambiguous import errors: the github.com/Azure/go-autorest module.

The issue stems from the fact that old versions of the root module
(github.com/Azure/go-autorest) provide the same packages as the newer
submodules.

To correct this situation, the _root module_ needs to be upgraded to a
version that no longer provides those packages (a version where the
submodules are present). Fortunately, the submodules can be leveraged to
provide the necessary version bump.

See: #414 (comment)

----

Caveat: in order for this to work, an importable version of the root
package needs to be referenceable.

PR #527 makes the root package importable.

The go.mod files assume that this importable version will be
referenceable as v14.2.0. If the version where the importable package is
available ends up being different, these files will need to be updated.

See also: #395, #413, #414, #455, #481, #524

* Update go.sum files

Co-authored-by: Joel Hendrix <jhendrix@microsoft.com>
jhendrixMSFT added a commit that referenced this pull request Jun 23, 2020
* Deserialize additionalInfo in ARM error

* Allow a new authorizer to be created from a configuration file by specifying a resource instead of a base url.

This enables resource like KeyVault and Container Registry to use an authorizer configured from a configuration file.

* [WIP] Using the Context from the timeout if provided (#315)

* Using the timeout from the context if available

- Makes PollingDuration optional

* Renaming the registration start time

* Making PollingDuration not a pointer

* fixing a broken reference

* Add NewAuthorizerFromCli method which uses Azure CLI to obtain a token for the currently logged in user, for  local development scenarios. (#316)

* Adding User assigned identity support for the MSIConfig authorizor (#332)

* Adding ByteSlicePtr (#399)

* Adding a new `WithXML` method (#402)

* Add HTTP status code response helpers (#403)

Added IsHTTPStatus() and HasHTTPStatus() methods to autorest.Response

* adding a new preparer for `MERGE` used in the Storage API's (#406)

* New Preparer/Responder for `Unmarshalling Bytes` (#407)

* New Preparer: WithBytes

* New Responder: `ByUnmarshallingBytes`

* Reusing the bytes, rather than copying them

* Fixing the broken test / switching to read the bytes directly

* Support HTTP-Date in Retry-After header (#410)

RFC specifies Retry-After header can be integer value expressing seconds
or an HTTP-Date indicating when to try again.
Removed superfluous check for HTTP status code.

* Add support for multi-tenant authentication (#412)

* Add support for multi-tenant authentication

Support for multi-tenant via x-ms-authorization-auxiliary header has
been added for client credentials with secret scenario; this basically
bundles multiple OAuthConfig and ServicePrincipalToken types into
corresponding MultiTenant* types along with a new authorizer that adds
the primary and auxiliary token headers to the reqest.
The authenticaion helpers have been updated to support this scenario; if
environment var AZURE_AUXILIARY_TENANT_IDS is set with a semicolon
delimited list of tenants the multi-tenant codepath will kick in to
create the appropriate authorizer.

* feedback

* rename Options to OAuthOptions (#415)

* Support custom SendDecorator chains via context (#417)

* Support custom SendDecorator chains via context

Added `autorest.WithSendDecorators` and `autorest.GetSendDecorators` for
adding and retrieving a custom chain of SendDecorators to the provided
context.
Added `autorest.DoRetryForStatusCodesWithCap` and
`autorest.DelayForBackoffWithCap` to enforce an upper bound on the
duration between retries.
Fixed up some code comments.

* small refactor based on PR feedback

* remove some changes for dev branch

* merge master into dev (#427)

* v12.3.0 (#418)

* Deserialize additionalInfo in ARM error

* Allow a new authorizer to be created from a configuration file by specifying a resource instead of a base url.

This enables resource like KeyVault and Container Registry to use an authorizer configured from a configuration file.

* [WIP] Using the Context from the timeout if provided (#315)

* Using the timeout from the context if available

- Makes PollingDuration optional

* Renaming the registration start time

* Making PollingDuration not a pointer

* fixing a broken reference

* Add NewAuthorizerFromCli method which uses Azure CLI to obtain a token for the currently logged in user, for  local development scenarios. (#316)

* Adding User assigned identity support for the MSIConfig authorizor (#332)

* Adding ByteSlicePtr (#399)

* Adding a new `WithXML` method (#402)

* Add HTTP status code response helpers (#403)

Added IsHTTPStatus() and HasHTTPStatus() methods to autorest.Response

* adding a new preparer for `MERGE` used in the Storage API's (#406)

* New Preparer/Responder for `Unmarshalling Bytes` (#407)

* New Preparer: WithBytes

* New Responder: `ByUnmarshallingBytes`

* Reusing the bytes, rather than copying them

* Fixing the broken test / switching to read the bytes directly

* Support HTTP-Date in Retry-After header (#410)

RFC specifies Retry-After header can be integer value expressing seconds
or an HTTP-Date indicating when to try again.
Removed superfluous check for HTTP status code.

* Add support for multi-tenant authentication (#412)

* Add support for multi-tenant authentication

Support for multi-tenant via x-ms-authorization-auxiliary header has
been added for client credentials with secret scenario; this basically
bundles multiple OAuthConfig and ServicePrincipalToken types into
corresponding MultiTenant* types along with a new authorizer that adds
the primary and auxiliary token headers to the reqest.
The authenticaion helpers have been updated to support this scenario; if
environment var AZURE_AUXILIARY_TENANT_IDS is set with a semicolon
delimited list of tenants the multi-tenant codepath will kick in to
create the appropriate authorizer.

* feedback

* rename Options to OAuthOptions (#415)

* Support custom SendDecorator chains via context (#417)

* Support custom SendDecorator chains via context

Added `autorest.WithSendDecorators` and `autorest.GetSendDecorators` for
adding and retrieving a custom chain of SendDecorators to the provided
context.
Added `autorest.DoRetryForStatusCodesWithCap` and
`autorest.DelayForBackoffWithCap` to enforce an upper bound on the
duration between retries.
Fixed up some code comments.

* small refactor based on PR feedback

* remove some changes for dev branch

* v12.3.0

* add yaml file for azure devops CI (#419)

* add status badge for azure devops CI (#420)

* enable build and test on linux (#421)

* enable build and test on linux

* fail on first error and use portable std*

* update test to run on devops

* Refactor azure devops pipeline (#422)

Break monolithic script into separate scripts with useful names.
Moved formatting checks to the end with succeededOrFailed conditions.

* remove travis artifacts (#423)

* remove unnecessary trigger section from devops (#424)

* Use accessTokens.json from AZURE_CONFIG_DIR if AZURE_ACCESS_TOKEN_FILE is not set before falling back on ~/.azure/ (#471)

* support for parsing error messages from xml responses (#465)

* support for parsing error messages from xml responses

* fixing the linting

* removed some duplicate code

* fix bug introduced in refactoring

* added XML test and fixed bug it uncovered

* fix godoc comment for methods that are safe for concurrent use (#475)

* New Authorizers for Azure Storage (#416)

* Authorizers for Blob, File, Queue and Table Storage

* Adding a SharedKey authorizer

* refactor based on existing storage implementation

* add missing storage emulator account name

* replace hard-coded strings with constants

* changed to by-ref

* Adding a new Authorizer for SAS Token Authentication (#478)

* Adding a new Authorizer for SAS Token Authentication

This commit introduces a new Authorizer for authenticating with
Blob Storage using a SAS Token

```
$ go test -v ./autorest/ -run="TestSas"
=== RUN   TestSasNewSasAuthorizerEmptyToken
--- PASS: TestSasNewSasAuthorizerEmptyToken (0.00s)
=== RUN   TestSasNewSasAuthorizerEmptyTokenWithWhitespace
--- PASS: TestSasNewSasAuthorizerEmptyTokenWithWhitespace (0.00s)
=== RUN   TestSasNewSasAuthorizerValidToken
--- PASS: TestSasNewSasAuthorizerValidToken (0.00s)
=== RUN   TestSasAuthorizerRequest
--- PASS: TestSasAuthorizerRequest (0.00s)
    authorization_sas_test.go:76: [DEBUG] Testing Case "empty querystring without a prefix"..
    authorization_sas_test.go:76: [DEBUG] Testing Case "empty querystring with a prefix"..
    authorization_sas_test.go:76: [DEBUG] Testing Case "existing querystring without a prefix"..
    authorization_sas_test.go:76: [DEBUG] Testing Case "existing querystring with a prefix"..
PASS
ok  	github.com/Azure/go-autorest/autorest	0.011s
```

* minor clean-up

* token: support for a custom refresh func (#476)

* token: support for a custom refresh func

* pass closures by value

* minor clean-up

* Fix Dropped Errors (#480)

* autorest: fix dropped errror

* autorest/adal: fix dropped test error

* Duration order consistency when multiplying number by time unit (#499)

* Drain response bodies (#432)

The retry helpers and a few other methods weren't reading and closing
response bodies leading to connection leaks.

* Enable exponential back-off when retrying on 429 (#503)

* Enable exponential back-off when retrying on 429

* enforce a 2-minute cap on delays if there isn't one

* updated comment

* fix type-o

* Expose OAuth token provider for use outside autorest (#520)

* feat: extract token creation to public method for MSI auth

* Add getter for token provider on BearerAuthorizer

* Fix Go module ambiguous import errors (#528)

* Fix Go module ambiguous import errors

This is an extension of the mitigations introduced in #455.
Unfortunately, the original mitigations didn't address the primary cause
of ambiguous import errors: the github.com/Azure/go-autorest module.

The issue stems from the fact that old versions of the root module
(github.com/Azure/go-autorest) provide the same packages as the newer
submodules.

To correct this situation, the _root module_ needs to be upgraded to a
version that no longer provides those packages (a version where the
submodules are present). Fortunately, the submodules can be leveraged to
provide the necessary version bump.

See: #414 (comment)

----

Caveat: in order for this to work, an importable version of the root
package needs to be referenceable.

PR #527 makes the root package importable.

The go.mod files assume that this importable version will be
referenceable as v14.2.0. If the version where the importable package is
available ends up being different, these files will need to be updated.

See also: #395, #413, #414, #455, #481, #524

* Update go.sum files

Co-authored-by: Joel Hendrix <jhendrix@microsoft.com>

* Update resourceManagerVMDNSSuffix for AzureUSGovernmentCloud (#531)

* This endpoint changed in AzureChinaCloud (#530)

See from Azurre China portal - this is now cloudapp.chinacloudapi.cn

Co-authored-by: Jin Soon Lim <jilim@microsoft.com>
Co-authored-by: Nick Muller <muller_nicky@hotmail.com>
Co-authored-by: Tom Harvey <tombuildsstuff@users.noreply.github.com>
Co-authored-by: Sam Kreter <samkreter@gmail.com>
Co-authored-by: Delyan Raychev <49918230+draychev@users.noreply.github.com>
Co-authored-by: Patrick Decat <pdecat@gmail.com>
Co-authored-by: Tony Abboud <tdabboud@hotmail.com>
Co-authored-by: Lars Lehtonen <lars.lehtonen@gmail.com>
Co-authored-by: Maxim Fominykh <vominyh@yandex.ru>
Co-authored-by: alespour <42931850+alespour@users.noreply.github.com>
Co-authored-by: Mark Severson <miquella@gmail.com>
Co-authored-by: Panic Stevenson <panic.stevenson@gmail.com>
Co-authored-by: Mauro Giusti <MaurGi@users.noreply.github.com>
@jhendrixMSFT
Copy link
Member

This has been fixed via #528 which is now in master.

jhendrixMSFT added a commit that referenced this pull request Aug 5, 2020
* Deserialize additionalInfo in ARM error

* Allow a new authorizer to be created from a configuration file by specifying a resource instead of a base url.

This enables resource like KeyVault and Container Registry to use an authorizer configured from a configuration file.

* [WIP] Using the Context from the timeout if provided (#315)

* Using the timeout from the context if available

- Makes PollingDuration optional

* Renaming the registration start time

* Making PollingDuration not a pointer

* fixing a broken reference

* Add NewAuthorizerFromCli method which uses Azure CLI to obtain a token for the currently logged in user, for  local development scenarios. (#316)

* Adding User assigned identity support for the MSIConfig authorizor (#332)

* Adding ByteSlicePtr (#399)

* Adding a new `WithXML` method (#402)

* Add HTTP status code response helpers (#403)

Added IsHTTPStatus() and HasHTTPStatus() methods to autorest.Response

* adding a new preparer for `MERGE` used in the Storage API's (#406)

* New Preparer/Responder for `Unmarshalling Bytes` (#407)

* New Preparer: WithBytes

* New Responder: `ByUnmarshallingBytes`

* Reusing the bytes, rather than copying them

* Fixing the broken test / switching to read the bytes directly

* Support HTTP-Date in Retry-After header (#410)

RFC specifies Retry-After header can be integer value expressing seconds
or an HTTP-Date indicating when to try again.
Removed superfluous check for HTTP status code.

* Add support for multi-tenant authentication (#412)

* Add support for multi-tenant authentication

Support for multi-tenant via x-ms-authorization-auxiliary header has
been added for client credentials with secret scenario; this basically
bundles multiple OAuthConfig and ServicePrincipalToken types into
corresponding MultiTenant* types along with a new authorizer that adds
the primary and auxiliary token headers to the reqest.
The authenticaion helpers have been updated to support this scenario; if
environment var AZURE_AUXILIARY_TENANT_IDS is set with a semicolon
delimited list of tenants the multi-tenant codepath will kick in to
create the appropriate authorizer.

* feedback

* rename Options to OAuthOptions (#415)

* Support custom SendDecorator chains via context (#417)

* Support custom SendDecorator chains via context

Added `autorest.WithSendDecorators` and `autorest.GetSendDecorators` for
adding and retrieving a custom chain of SendDecorators to the provided
context.
Added `autorest.DoRetryForStatusCodesWithCap` and
`autorest.DelayForBackoffWithCap` to enforce an upper bound on the
duration between retries.
Fixed up some code comments.

* small refactor based on PR feedback

* remove some changes for dev branch

* merge master into dev (#427)

* v12.3.0 (#418)

* Deserialize additionalInfo in ARM error

* Allow a new authorizer to be created from a configuration file by specifying a resource instead of a base url.

This enables resource like KeyVault and Container Registry to use an authorizer configured from a configuration file.

* [WIP] Using the Context from the timeout if provided (#315)

* Using the timeout from the context if available

- Makes PollingDuration optional

* Renaming the registration start time

* Making PollingDuration not a pointer

* fixing a broken reference

* Add NewAuthorizerFromCli method which uses Azure CLI to obtain a token for the currently logged in user, for  local development scenarios. (#316)

* Adding User assigned identity support for the MSIConfig authorizor (#332)

* Adding ByteSlicePtr (#399)

* Adding a new `WithXML` method (#402)

* Add HTTP status code response helpers (#403)

Added IsHTTPStatus() and HasHTTPStatus() methods to autorest.Response

* adding a new preparer for `MERGE` used in the Storage API's (#406)

* New Preparer/Responder for `Unmarshalling Bytes` (#407)

* New Preparer: WithBytes

* New Responder: `ByUnmarshallingBytes`

* Reusing the bytes, rather than copying them

* Fixing the broken test / switching to read the bytes directly

* Support HTTP-Date in Retry-After header (#410)

RFC specifies Retry-After header can be integer value expressing seconds
or an HTTP-Date indicating when to try again.
Removed superfluous check for HTTP status code.

* Add support for multi-tenant authentication (#412)

* Add support for multi-tenant authentication

Support for multi-tenant via x-ms-authorization-auxiliary header has
been added for client credentials with secret scenario; this basically
bundles multiple OAuthConfig and ServicePrincipalToken types into
corresponding MultiTenant* types along with a new authorizer that adds
the primary and auxiliary token headers to the reqest.
The authenticaion helpers have been updated to support this scenario; if
environment var AZURE_AUXILIARY_TENANT_IDS is set with a semicolon
delimited list of tenants the multi-tenant codepath will kick in to
create the appropriate authorizer.

* feedback

* rename Options to OAuthOptions (#415)

* Support custom SendDecorator chains via context (#417)

* Support custom SendDecorator chains via context

Added `autorest.WithSendDecorators` and `autorest.GetSendDecorators` for
adding and retrieving a custom chain of SendDecorators to the provided
context.
Added `autorest.DoRetryForStatusCodesWithCap` and
`autorest.DelayForBackoffWithCap` to enforce an upper bound on the
duration between retries.
Fixed up some code comments.

* small refactor based on PR feedback

* remove some changes for dev branch

* v12.3.0

* add yaml file for azure devops CI (#419)

* add status badge for azure devops CI (#420)

* enable build and test on linux (#421)

* enable build and test on linux

* fail on first error and use portable std*

* update test to run on devops

* Refactor azure devops pipeline (#422)

Break monolithic script into separate scripts with useful names.
Moved formatting checks to the end with succeededOrFailed conditions.

* remove travis artifacts (#423)

* remove unnecessary trigger section from devops (#424)

* Use accessTokens.json from AZURE_CONFIG_DIR if AZURE_ACCESS_TOKEN_FILE is not set before falling back on ~/.azure/ (#471)

* support for parsing error messages from xml responses (#465)

* support for parsing error messages from xml responses

* fixing the linting

* removed some duplicate code

* fix bug introduced in refactoring

* added XML test and fixed bug it uncovered

* fix godoc comment for methods that are safe for concurrent use (#475)

* New Authorizers for Azure Storage (#416)

* Authorizers for Blob, File, Queue and Table Storage

* Adding a SharedKey authorizer

* refactor based on existing storage implementation

* add missing storage emulator account name

* replace hard-coded strings with constants

* changed to by-ref

* Adding a new Authorizer for SAS Token Authentication (#478)

* Adding a new Authorizer for SAS Token Authentication

This commit introduces a new Authorizer for authenticating with
Blob Storage using a SAS Token

```
$ go test -v ./autorest/ -run="TestSas"
=== RUN   TestSasNewSasAuthorizerEmptyToken
--- PASS: TestSasNewSasAuthorizerEmptyToken (0.00s)
=== RUN   TestSasNewSasAuthorizerEmptyTokenWithWhitespace
--- PASS: TestSasNewSasAuthorizerEmptyTokenWithWhitespace (0.00s)
=== RUN   TestSasNewSasAuthorizerValidToken
--- PASS: TestSasNewSasAuthorizerValidToken (0.00s)
=== RUN   TestSasAuthorizerRequest
--- PASS: TestSasAuthorizerRequest (0.00s)
    authorization_sas_test.go:76: [DEBUG] Testing Case "empty querystring without a prefix"..
    authorization_sas_test.go:76: [DEBUG] Testing Case "empty querystring with a prefix"..
    authorization_sas_test.go:76: [DEBUG] Testing Case "existing querystring without a prefix"..
    authorization_sas_test.go:76: [DEBUG] Testing Case "existing querystring with a prefix"..
PASS
ok  	github.com/Azure/go-autorest/autorest	0.011s
```

* minor clean-up

* token: support for a custom refresh func (#476)

* token: support for a custom refresh func

* pass closures by value

* minor clean-up

* Fix Dropped Errors (#480)

* autorest: fix dropped errror

* autorest/adal: fix dropped test error

* Duration order consistency when multiplying number by time unit (#499)

* Drain response bodies (#432)

The retry helpers and a few other methods weren't reading and closing
response bodies leading to connection leaks.

* Enable exponential back-off when retrying on 429 (#503)

* Enable exponential back-off when retrying on 429

* enforce a 2-minute cap on delays if there isn't one

* updated comment

* fix type-o

* autorest: remove testing.T.Fatal() from test goroutine

* Expose OAuth token provider for use outside autorest (#520)

* feat: extract token creation to public method for MSI auth

* Add getter for token provider on BearerAuthorizer

* Fix Go module ambiguous import errors (#528)

* Fix Go module ambiguous import errors

This is an extension of the mitigations introduced in #455.
Unfortunately, the original mitigations didn't address the primary cause
of ambiguous import errors: the github.com/Azure/go-autorest module.

The issue stems from the fact that old versions of the root module
(github.com/Azure/go-autorest) provide the same packages as the newer
submodules.

To correct this situation, the _root module_ needs to be upgraded to a
version that no longer provides those packages (a version where the
submodules are present). Fortunately, the submodules can be leveraged to
provide the necessary version bump.

See: #414 (comment)

----

Caveat: in order for this to work, an importable version of the root
package needs to be referenceable.

PR #527 makes the root package importable.

The go.mod files assume that this importable version will be
referenceable as v14.2.0. If the version where the importable package is
available ends up being different, these files will need to be updated.

See also: #395, #413, #414, #455, #481, #524

* Update go.sum files

Co-authored-by: Joel Hendrix <jhendrix@microsoft.com>

* Update resourceManagerVMDNSSuffix for AzureUSGovernmentCloud (#531)

* This endpoint changed in AzureChinaCloud (#530)

See from Azurre China portal - this is now cloudapp.chinacloudapi.cn

Co-authored-by: Jin Soon Lim <jilim@microsoft.com>
Co-authored-by: Joel Hendrix <jhendrix@microsoft.com>
Co-authored-by: Nick Muller <muller_nicky@hotmail.com>
Co-authored-by: Tom Harvey <tombuildsstuff@users.noreply.github.com>
Co-authored-by: Sam Kreter <samkreter@gmail.com>
Co-authored-by: Delyan Raychev <49918230+draychev@users.noreply.github.com>
Co-authored-by: Patrick Decat <pdecat@gmail.com>
Co-authored-by: Tony Abboud <tdabboud@hotmail.com>
Co-authored-by: Maxim Fominykh <vominyh@yandex.ru>
Co-authored-by: alespour <42931850+alespour@users.noreply.github.com>
Co-authored-by: Mark Severson <miquella@gmail.com>
Co-authored-by: Panic Stevenson <panic.stevenson@gmail.com>
Co-authored-by: Mauro Giusti <MaurGi@users.noreply.github.com>
jhendrixMSFT added a commit that referenced this pull request Aug 7, 2020
* Deserialize additionalInfo in ARM error

* Allow a new authorizer to be created from a configuration file by specifying a resource instead of a base url.

This enables resource like KeyVault and Container Registry to use an authorizer configured from a configuration file.

* [WIP] Using the Context from the timeout if provided (#315)

* Using the timeout from the context if available

- Makes PollingDuration optional

* Renaming the registration start time

* Making PollingDuration not a pointer

* fixing a broken reference

* Add NewAuthorizerFromCli method which uses Azure CLI to obtain a token for the currently logged in user, for  local development scenarios. (#316)

* Adding User assigned identity support for the MSIConfig authorizor (#332)

* Adding ByteSlicePtr (#399)

* Adding a new `WithXML` method (#402)

* Add HTTP status code response helpers (#403)

Added IsHTTPStatus() and HasHTTPStatus() methods to autorest.Response

* adding a new preparer for `MERGE` used in the Storage API's (#406)

* New Preparer/Responder for `Unmarshalling Bytes` (#407)

* New Preparer: WithBytes

* New Responder: `ByUnmarshallingBytes`

* Reusing the bytes, rather than copying them

* Fixing the broken test / switching to read the bytes directly

* Support HTTP-Date in Retry-After header (#410)

RFC specifies Retry-After header can be integer value expressing seconds
or an HTTP-Date indicating when to try again.
Removed superfluous check for HTTP status code.

* Add support for multi-tenant authentication (#412)

* Add support for multi-tenant authentication

Support for multi-tenant via x-ms-authorization-auxiliary header has
been added for client credentials with secret scenario; this basically
bundles multiple OAuthConfig and ServicePrincipalToken types into
corresponding MultiTenant* types along with a new authorizer that adds
the primary and auxiliary token headers to the reqest.
The authenticaion helpers have been updated to support this scenario; if
environment var AZURE_AUXILIARY_TENANT_IDS is set with a semicolon
delimited list of tenants the multi-tenant codepath will kick in to
create the appropriate authorizer.

* feedback

* rename Options to OAuthOptions (#415)

* Support custom SendDecorator chains via context (#417)

* Support custom SendDecorator chains via context

Added `autorest.WithSendDecorators` and `autorest.GetSendDecorators` for
adding and retrieving a custom chain of SendDecorators to the provided
context.
Added `autorest.DoRetryForStatusCodesWithCap` and
`autorest.DelayForBackoffWithCap` to enforce an upper bound on the
duration between retries.
Fixed up some code comments.

* small refactor based on PR feedback

* remove some changes for dev branch

* merge master into dev (#427)

* v12.3.0 (#418)

* Deserialize additionalInfo in ARM error

* Allow a new authorizer to be created from a configuration file by specifying a resource instead of a base url.

This enables resource like KeyVault and Container Registry to use an authorizer configured from a configuration file.

* [WIP] Using the Context from the timeout if provided (#315)

* Using the timeout from the context if available

- Makes PollingDuration optional

* Renaming the registration start time

* Making PollingDuration not a pointer

* fixing a broken reference

* Add NewAuthorizerFromCli method which uses Azure CLI to obtain a token for the currently logged in user, for  local development scenarios. (#316)

* Adding User assigned identity support for the MSIConfig authorizor (#332)

* Adding ByteSlicePtr (#399)

* Adding a new `WithXML` method (#402)

* Add HTTP status code response helpers (#403)

Added IsHTTPStatus() and HasHTTPStatus() methods to autorest.Response

* adding a new preparer for `MERGE` used in the Storage API's (#406)

* New Preparer/Responder for `Unmarshalling Bytes` (#407)

* New Preparer: WithBytes

* New Responder: `ByUnmarshallingBytes`

* Reusing the bytes, rather than copying them

* Fixing the broken test / switching to read the bytes directly

* Support HTTP-Date in Retry-After header (#410)

RFC specifies Retry-After header can be integer value expressing seconds
or an HTTP-Date indicating when to try again.
Removed superfluous check for HTTP status code.

* Add support for multi-tenant authentication (#412)

* Add support for multi-tenant authentication

Support for multi-tenant via x-ms-authorization-auxiliary header has
been added for client credentials with secret scenario; this basically
bundles multiple OAuthConfig and ServicePrincipalToken types into
corresponding MultiTenant* types along with a new authorizer that adds
the primary and auxiliary token headers to the reqest.
The authenticaion helpers have been updated to support this scenario; if
environment var AZURE_AUXILIARY_TENANT_IDS is set with a semicolon
delimited list of tenants the multi-tenant codepath will kick in to
create the appropriate authorizer.

* feedback

* rename Options to OAuthOptions (#415)

* Support custom SendDecorator chains via context (#417)

* Support custom SendDecorator chains via context

Added `autorest.WithSendDecorators` and `autorest.GetSendDecorators` for
adding and retrieving a custom chain of SendDecorators to the provided
context.
Added `autorest.DoRetryForStatusCodesWithCap` and
`autorest.DelayForBackoffWithCap` to enforce an upper bound on the
duration between retries.
Fixed up some code comments.

* small refactor based on PR feedback

* remove some changes for dev branch

* v12.3.0

* add yaml file for azure devops CI (#419)

* add status badge for azure devops CI (#420)

* enable build and test on linux (#421)

* enable build and test on linux

* fail on first error and use portable std*

* update test to run on devops

* Refactor azure devops pipeline (#422)

Break monolithic script into separate scripts with useful names.
Moved formatting checks to the end with succeededOrFailed conditions.

* remove travis artifacts (#423)

* remove unnecessary trigger section from devops (#424)

* Use accessTokens.json from AZURE_CONFIG_DIR if AZURE_ACCESS_TOKEN_FILE is not set before falling back on ~/.azure/ (#471)

* support for parsing error messages from xml responses (#465)

* support for parsing error messages from xml responses

* fixing the linting

* removed some duplicate code

* fix bug introduced in refactoring

* added XML test and fixed bug it uncovered

* fix godoc comment for methods that are safe for concurrent use (#475)

* New Authorizers for Azure Storage (#416)

* Authorizers for Blob, File, Queue and Table Storage

* Adding a SharedKey authorizer

* refactor based on existing storage implementation

* add missing storage emulator account name

* replace hard-coded strings with constants

* changed to by-ref

* Adding a new Authorizer for SAS Token Authentication (#478)

* Adding a new Authorizer for SAS Token Authentication

This commit introduces a new Authorizer for authenticating with
Blob Storage using a SAS Token

```
$ go test -v ./autorest/ -run="TestSas"
=== RUN   TestSasNewSasAuthorizerEmptyToken
--- PASS: TestSasNewSasAuthorizerEmptyToken (0.00s)
=== RUN   TestSasNewSasAuthorizerEmptyTokenWithWhitespace
--- PASS: TestSasNewSasAuthorizerEmptyTokenWithWhitespace (0.00s)
=== RUN   TestSasNewSasAuthorizerValidToken
--- PASS: TestSasNewSasAuthorizerValidToken (0.00s)
=== RUN   TestSasAuthorizerRequest
--- PASS: TestSasAuthorizerRequest (0.00s)
    authorization_sas_test.go:76: [DEBUG] Testing Case "empty querystring without a prefix"..
    authorization_sas_test.go:76: [DEBUG] Testing Case "empty querystring with a prefix"..
    authorization_sas_test.go:76: [DEBUG] Testing Case "existing querystring without a prefix"..
    authorization_sas_test.go:76: [DEBUG] Testing Case "existing querystring with a prefix"..
PASS
ok  	github.com/Azure/go-autorest/autorest	0.011s
```

* minor clean-up

* token: support for a custom refresh func (#476)

* token: support for a custom refresh func

* pass closures by value

* minor clean-up

* Fix Dropped Errors (#480)

* autorest: fix dropped errror

* autorest/adal: fix dropped test error

* Duration order consistency when multiplying number by time unit (#499)

* Drain response bodies (#432)

The retry helpers and a few other methods weren't reading and closing
response bodies leading to connection leaks.

* Enable exponential back-off when retrying on 429 (#503)

* Enable exponential back-off when retrying on 429

* enforce a 2-minute cap on delays if there isn't one

* updated comment

* fix type-o

* Expose OAuth token provider for use outside autorest (#520)

* feat: extract token creation to public method for MSI auth

* Add getter for token provider on BearerAuthorizer

* Fix Go module ambiguous import errors (#528)

* Fix Go module ambiguous import errors

This is an extension of the mitigations introduced in #455.
Unfortunately, the original mitigations didn't address the primary cause
of ambiguous import errors: the github.com/Azure/go-autorest module.

The issue stems from the fact that old versions of the root module
(github.com/Azure/go-autorest) provide the same packages as the newer
submodules.

To correct this situation, the _root module_ needs to be upgraded to a
version that no longer provides those packages (a version where the
submodules are present). Fortunately, the submodules can be leveraged to
provide the necessary version bump.

See: #414 (comment)

----

Caveat: in order for this to work, an importable version of the root
package needs to be referenceable.

PR #527 makes the root package importable.

The go.mod files assume that this importable version will be
referenceable as v14.2.0. If the version where the importable package is
available ends up being different, these files will need to be updated.

See also: #395, #413, #414, #455, #481, #524

* Update go.sum files

Co-authored-by: Joel Hendrix <jhendrix@microsoft.com>

* Update resourceManagerVMDNSSuffix for AzureUSGovernmentCloud (#531)

* This endpoint changed in AzureChinaCloud (#530)

See from Azurre China portal - this is now cloudapp.chinacloudapi.cn

* allow MSI login with "mi_res_id" (#544)

* allow login with resourceID

* test

* tweaks

* fix

* tested with cmd

* fix unittest

* add new test, remove debug trace

* fix unittest

* fix with url encode

Co-authored-by: Jin Soon Lim <jilim@microsoft.com>
Co-authored-by: Nick Muller <muller_nicky@hotmail.com>
Co-authored-by: Tom Harvey <tombuildsstuff@users.noreply.github.com>
Co-authored-by: Sam Kreter <samkreter@gmail.com>
Co-authored-by: Delyan Raychev <49918230+draychev@users.noreply.github.com>
Co-authored-by: Patrick Decat <pdecat@gmail.com>
Co-authored-by: Tony Abboud <tdabboud@hotmail.com>
Co-authored-by: Lars Lehtonen <lars.lehtonen@gmail.com>
Co-authored-by: Maxim Fominykh <vominyh@yandex.ru>
Co-authored-by: alespour <42931850+alespour@users.noreply.github.com>
Co-authored-by: Mark Severson <miquella@gmail.com>
Co-authored-by: Panic Stevenson <panic.stevenson@gmail.com>
Co-authored-by: Mauro Giusti <MaurGi@users.noreply.github.com>
Co-authored-by: Haitao Chen <haitch@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants