Versioning of Go-AutoRest #481
Comments
|
The versioning of some Azure SDKs for Go seems to be really broken. Since e.g. code that imports package main
import (
"context"
"fmt"
"github.com/Azure/azure-event-hubs-go/v2"
"github.com/Azure/azure-sdk-for-go/services/keyvault/mgmt/2016-10-01/keyvault"
)
func main() {
vaultsClient := keyvault.New("SubscriptionID")
fmt.Printf(vaultsClient.BaseURI)
//...
connStr := "Endpoint=sb://namespace.servicebus.windows.net/;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=superSecret1234=;EntityPath=hubName"
hub, _ := eventhub.NewHubFromConnectionString(connStr)
hub.Close(context.Background())
}I really don't know how this version conflict could be solved as long as all major versions use the same import path. |
|
I want to apologize upfront for the headaches this has caused. If I had to do it over again I wouldn't have rolled out modules in this repo so soon (the SDK was supposed to get modules shortly after). I agree that the repo's inter-dependencies have made the situation complex, I underestimated this. My thinking was to separate the packages used by the code generator (e.g. Before we go the route of moving the repo and all the fall-out that entails, I think we can remove the majority of extra modules so the problem goes away. See here for details on the approach. I'm not sure what modules we'd end up with, but off-the-cuff I think the plan would be to include all package hierarchies that have inter-repo dependencies in the same module (i.e. most everything under ./autorest would become one big module). My fear is that this would somehow make matters worse which is definitely something I don't want to do, so it's gonna take some planning and testing. Thoughts on this? |
|
I understand that this problem has grown historically, and only came to light with the introduction of Go Modules. Anyways, Go Modules is very opinionated about the way import versioning should work. They suggest that every major version with breaking changes must include the version at the end of the import path in the |
|
After reading through some of the linked issues I'm no longer convinced that reducing the number of modules (and eliminating the inter-dependencies) will solve the underlying issue at present (although it will help reduce the needless complexity). |
|
Agree, the problem here is not about the complexity. It's the dependencies and incompatibility between different versions of Go modules. Here is a diagram to illustrate the problem even better: I guess normally you could use |
|
Sorry for the delay on this. Regarding the SDK, our current POR is to release each service/API version tuple as its own module (compute/2015-06-15/compute, compute/2016-03-30/compute, etc). While I realize this seems like a lot of modules it provides the best insulation against breaking changes having inadvertent impact across API versions. In addition, I've released a v3 of event hubs (for a different reason) that will address the above build issue. |
teddyking
added a commit
to vmware-tanzu/projects-operator
that referenced
this issue
Jan 3, 2020
... when attempting to pull projects-operator in as a dependency to developer-console, we ran into a whole universe of pain regarding Azure/go-autorest ... bumping our dependencies here seems to have solved the issue. For refernce, here is a go-autorest issue detailing the problem: Azure/go-autorest#481 [#170129412] Co-authored-by: Glen Rodgers <grodgers@pivotal.io>
Is there a workaround for this error?
EDIT 2: Nope, the aforementioned solution doesn't build: EDIT 3: The advice on gophers slack appears to be to carefully upgrade dependencies until versions match: https://gophers.slack.com/archives/C9BMAAFFB/p1575914322115600. EDIT 4: Seems to have worked this time, I had to manually update both $ go get github.com/Azure/go-autorest@latest
$ go get github.com/Azure/go-autorest/autorest/adal@latest |
That's broken again. and the second |
|
@tv42 what module(s) is pulling in the dependency on |
|
|
Solving this issue can be dramatically simplified by simply removing the current go.mod files and replacing them with a single go.mod file at the root. Finally bump a major version, add a tag, and append /vXX to the end of the module import path within the go.mod. Going forward simply version the entire set of packages together. Push anything you want to master, but when you tag a new release make it a major release if it has any exported API breaking changes. If you want to shield certain packages from this versioning requirement, put the packages in an Anything short of the above is only going to cause issues down the road for consumers of this package. Consumers of this package need only update all import paths to |
|
I am having the exact same issue as @tv42, thought I fixed it but on running |
|
@dewey thanks for the info. I feel like this is the problematic line. As there is no repo-level module for go-autorest, this picks up What content are you using from |
|
I'm just using it through the following dependencies. So these are the ones importing it. Edit: Related issue google/go-cloud#2722 |
|
Thanks I can repro this issue. I believe this is due to the following issue. Azure/azure-storage-blob-go#173 I will work with the storage team to get this resolved. |
4 tasks
jhendrixMSFT
added a commit
that referenced
this issue
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 issue
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>
|
This should now be resolved. Big kudos to @miquella for digging in to understand the issue and putting together the PRs to fix it. If anybody continues to experience version problems please let me know. |
|
@jhendrixMSFT It's working for me now! Thanks for following up and thanks for putting in the work @miquella! |
|
@tombuildsstuff would love to get your ack that ambiguous import errors have gone away when you have a chance to try it. |
|
I am facing similar issue with keyvault sdk for go modules. Building the module throwing the following error. |
|
@sonujose your version of the autorest module is too old. Can you please update it? |
|
Thanks @jhendrixMSFT Updating autorest version worked. Thanks |
|
@jhendrixMSFT we're still using a fork of go-autorest (since we're blocked on a couple of PR's) - so that's not so easy to test right now, unfortunately |
jhendrixMSFT
added a commit
that referenced
this issue
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>
|
@tombuildsstuff sorry for the delay on the PRs. I've cleaned up and merged the fixes for shared key and SAS tokens (tagged as autorest/v0.11.3). There is one remaining issue about a panic however we need more info. Also, bumping go-autorest modules to their latest versions I was able to build giovanni without the need for replace directives in go.mod. I submitted a PR here. This includes the above fixes. |
jhendrixMSFT
added a commit
that referenced
this issue
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>
|
@jhendrixMSFT thanks for that - unfortunately it appears that v45 of the Azure SDK has multiple issues which prevent us from upgrading, so whilst this appears ok (and thanks for the PR!) I can't confirm for sure until there's a version of the Azure SDK that works for us |
|
@tombuildsstuff glad at least part of this has been fixed. Can you elaborate on the problems with the SDK? |
|
@jhendrixMSFT sure, from our side there's breaking changes to the Networking package (PR to fix) and an issue with a discriminator which are preventing us from upgrading |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
👋🏻
As a consumer of this repository (via both the Azure SDK for Go and Giovanni) whilst I can understand the desire to version each package in this repository independently I think it's causing more complexity than it's solving since there's dependencies between packages.
From our side we've lost a substantial amount of time (days) over the past few months trying to diagnose versioning issues with go-autorest and the Azure SDK for Go. Ultimately we've had to roll back to using an older version of the Azure SDK for Go / go-autorest, since switching to the versioning per package breaks vendoring Terraform Core into downstream codebases, unless the downstream codebases also include a
replacestatement in theirgo.modfiles - which unfortunately isn't a viable solution for us.My concern is that these issues will be exacerbated when the Azure SDK for Go moves to a Go Module per Service Package (e.g.
./services/foobar/mgmt/2019-01-01); where different service packages/modules depend on different versions of this package.Given these issues are caused by there being two different tagging approaches in this repository (which can't be removed at this point since they'll be cached via the Go Module Cache) - I'm wondering if it'd be worth taking a different approach instead.
Since this library depends on itself, I believe that versioning per package may be causing more issues than it's solving. In particular whilst I understand grouping everything into a single module means the SemVer for this library may need to be bumped (which @jhendrixMSFT previously outlined here) I'm wondering if bumping the major version is a concern for this library in practice?
Given the problems above, I'm wondering if it'd be worth essentially duplicating this repository (preferably with, but it might be less hassle without the history) but in either case without the existing tags. This'd allow this repository to be versioned starting from (say) 1.0 but critically as single unit (in the root).
There's two affects on this on downstream consumers; downstream SDK's, such as:
which (technically) would require a major version bump.
Software proudcts on the other hand would only require a patch release, including:
To ease the transition for slower moving dependencies it could be beneficial to use Type Aliases in this repository to the types in the new repository (in the same way Profiles do in the Azure SDK for Go), but I don't think that's a blocker (and if it is it's easy enough to retroactively add it).
Whilst I appreciate this is a big ask/change - since the versioning issues has affected multiple downstream projects (including Terraform and Kubernetes which in turn has affected both CoreDNS and Rancher (and I believe Prometheus too, albeit I can't find the link now?)) - I'm wondering if splitting this out into it's own repository provides the simplest path forward.
Thoughts?
The text was updated successfully, but these errors were encountered: