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.

Sign up for GitHub

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

Jump to bottom

KeyVaultChallengePolicy violates guarantees because it is not goroutine-safe #24031

Open
strager opened this issue Jan 31, 2025 · 7 comments · May be fixed by #24032
Open

KeyVaultChallengePolicy violates guarantees because it is not goroutine-safe #24031

strager opened this issue Jan 31, 2025 · 7 comments · May be fixed by #24032
Assignees
@gracewilcox
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault
Milestone
2025-03

Comments

@strager
Copy link

strager commented Jan 31, 2025

Bug Report

  • import path: github.com/Azure/azure-sdk-for-go/sdk/keyvault/azsecrets

  • SDK version: unsure. Here is what go.mod says:

	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.11.1
	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0
	github.com/Azure/azure-sdk-for-go/sdk/keyvault/azsecrets v0.12.0

	github.com/Azure/azure-sdk-for-go/sdk/internal v1.8.0 // indirect
	github.com/Azure/azure-sdk-for-go/sdk/keyvault/internal v0.7.1 // indirect

  • output of go version: go version go1.23.2 darwin/arm64

  • What happened?

When I use an azsecret.Client from multiple goroutines, Go's race detector complains. For example:

WARNING: DATA RACE
Write at 0x00c000306b90 by goroutine 31:
  github.com/Azure/azure-sdk-for-go/sdk/keyvault/internal.(*KeyVaultChallengePolicy).findScopeAndTenant()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/keyvault/[email protected]/challenge_policy.go:187 +0x330
  github.com/Azure/azure-sdk-for-go/sdk/keyvault/internal.(*KeyVaultChallengePolicy).Do()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/keyvault/[email protected]/challenge_policy.go:81 +0x180
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:107 +0x1e8
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*retryPolicy).Do()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_retry.go:121 +0x550
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:107 +0x1e8
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.telemetryPolicy.Do()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_telemetry.go:70 +0x20c
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*telemetryPolicy).Do()
      <autogenerated>:1 +0x4c
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:107 +0x1e8
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.includeResponsePolicy()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_include_response.go:19 +0x28
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.PolicyFunc.Do()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:107 +0x1e8
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.Pipeline.Do()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/pipeline.go:76 +0x88
  github.com/Azure/azure-sdk-for-go/sdk/keyvault/azsecrets.(*Client).GetSecret()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/keyvault/[email protected]/client.go:192 +0xc4
  github.com/traduality/Traduality/beedo.(*AppConfigLoader).GetSecretLocked()
      /Users/traduality/Projects/Traduality/beedo/config.go:72 +0x194
  github.com/traduality/Traduality/beedo.(*ConfigSecret).UnmarshalTOML.(*AppConfigLoader).GetSecretAsyncLocked.func1()
      /Users/traduality/Projects/Traduality/beedo/config.go:109 +0x68
  github.com/hashicorp/go-multierror.(*Group).Go.func1()
      /Us
Previous write at 0x00c000306b90 by goroutine 25:
  github.com/Azure/azure-sdk-for-go/sdk/keyvault/internal.(*KeyVaultChallengePolicy).findScopeAndTenant()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/keyvault/[email protected]/challenge_policy.go:187 +0x330
  github.com/Azure/azure-sdk-for-go/sdk/keyvault/internal.(*KeyVaultChallengePolicy).Do()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/keyvault/[email protected]/challenge_policy.go:81 +0x180
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:107 +0x1e8
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*retryPolicy).Do()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_retry.go:121 +0x550
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()
      /Users/traduality/  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.telemetryPolicy.Do()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_telemetry.go:70 +0x20c
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.(*telemetryPolicy).Do()
      <autogenerated>:1 +0x4c
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Next()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:107 +0x1e8
  github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime.includeResponsePolicy()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/runtime/policy_include_response.go:19 +0x28
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.PolicyFunc.Do()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/request.go:177 +0x34
  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.(*Request).Nex  github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported.Pipeline.Do()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/[email protected]/internal/exported/pipeline.go:76 +0x88
  github.com/Azure/azure-sdk-for-go/sdk/keyvault/azsecrets.(*Client).GetSecret()
      /Users/traduality/go/pkg/mod/github.com/!azure/azure-sdk-for-go/sdk/keyvault/[email protected]/client.go:192 +0xc4
  github.com/traduality/Traduality/beedo.(*AppConfigLoader).GetSecretLocked()
      /Users/traduality/Projects/Traduality/beedo/config.go:72 +0x194
  github.com/traduality/Traduality/beedo.(*ConfigSecret).UnmarshalTOML.(*AppConfigLoader).GetSecretAsyncLocked.func1()
      /Users/traduality/Projects/Traduality/beedo/config.go:109 +0x68
  github.com/hashicorp/go-multierror.(*Group).Go.func1()
      /Users/traduality/go/pkg/mod/github.com/hashicorp/[email protected]/group.go:23 +0x88

Goroutine 31 (running) created at:
  github.com/hashicorp/go-multierror.(*Group).Go()
      /Users/traduality/go/pkg/mod/gith  github.com/traduality/Traduality/beedo.(*AppConfigLoader).GetSecretAsyncLocked()
      /Users/traduality/Projects/Traduality/beedo/config.go:107 +0x2d8
  github.com/traduality/Traduality/beedo.(*ConfigSecret).UnmarshalTOML()
      /Users/traduality/Projects/Traduality/beedo/config.go:140 +0x198
  github.com/BurntSushi/toml.(*MetaData).unify()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:221 +0x214
  github.com/BurntSushi/toml.(*MetaData).unifyStruct()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:300 +0x56c
  github.com/BurntSushi/toml.(*MetaData).unify()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:246 +0x520
  github.com/BurntSushi/toml.(*MetaData).unifyStruct()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:300 +0x56c
  github.com/BurntSushi/toml.(  github.com/BurntSushi/toml.(*Decoder).Decode()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:180 +0x654
  github.com/BurntSushi/toml.Unmarshal()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:28 +0xd8
  github.com/traduality/Traduality/beedo.LoadAppConfigFiles()
      /Users/traduality/Projects/Traduality/beedo/config.go:273 +0x264
  github.com/traduality/Traduality/beedo.ServerMain()
      /Users/traduality/Projects/Traduality/beedo/server.go:101 +0x21c
  main.main()
      /Users/traduality/Projects/Traduality/beedo/cmd/server/server.go:10 +0x20

Goroutine 25 (running) created at:
  github.com/hashicorp/go-multierror.(*Group).Go()
      /Users/traduality/go/pkg/mod/github.com/hashicorp/[email protected]/group.go:20 +0xd8
  github.com/traduality/Traduality/beedo.(*AppConfigLoader).GetSecretAsyncLocked()
      /Users/traduality/Projects/Traduality/beedo/config.go:107 +0x2d8
  gi  github.com/BurntSushi/toml.(*MetaData).unify()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:221 +0x214
  github.com/BurntSushi/toml.(*MetaData).unifyStruct()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:300 +0x56c
  github.com/BurntSushi/toml.(*MetaData).unify()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:246 +0x520
  github.com/BurntSushi/toml.(*MetaData).unifyStruct()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:300 +0x56c
  github.com/BurntSushi/toml.(*MetaData).unify()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:246 +0x520
  github.com/BurntSushi/toml.(*MetaData).unifyStruct()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/d  github.com/BurntSushi/toml.(*MetaData).unify()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:246 +0x520
  github.com/BurntSushi/toml.(*Decoder).Decode()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:180 +0x654
  github.com/BurntSushi/toml.Unmarshal()
      /Users/traduality/go/pkg/mod/github.com/!burnt!sushi/[email protected]/decode.go:28 +0xd8
  github.com/traduality/Traduality/beedo.LoadAppConfigFiles()
      /Users/traduality/Projects/Traduality/beedo/config.go:273 +0x264
  github.com/traduality/Traduality/beedo.ServerMain()
      /Users/traduality/Projects/Traduality/beedo/server.go:101 +0x21c
  main.main()
      /Users/traduality/Projects/Traduality/beedo/cmd/server/server.go:10 +0x20
==================
  • What did you expect or want to happen?

I expect to be able to use a Client from multiple goroutines. This is my understanding of the documentation here: https://learn.microsoft.com/en-us/azure/developer/go/azure-sdk-core-concepts Specifically: "Goroutine safety allows multiple goroutines to access a single client object concurrently."

  • How can we reproduce it?

I can work on a reduce repro if you need it.

  • Anything we should know about your environment.
@github-actions github-actions bot added customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Jan 31, 2025
@strager
Copy link
Author

strager commented Jan 31, 2025

I updated the SDK to the latest version and the issue persists. go.mod snippet:

	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.17.0
	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.8.1
	github.com/Azure/azure-sdk-for-go/sdk/keyvault/azsecrets v0.12.0

	github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 // indirect
	github.com/Azure/azure-sdk-for-go/sdk/keyvault/internal v0.7.1 // indirect

@strager
Copy link
Author

strager commented Jan 31, 2025

Also happens on SDK Git commit 90c29ccd6952363d37f7f2c2aa2e8d329ee59015 (which is main at the time of writing).

@strager strager linked a pull request Jan 31, 2025 that will close this issue
Open
5 tasks
@gracewilcox gracewilcox added KeyVault feature-request This issue requires a new behavior in the product in order be resolved. and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Jan 31, 2025
@github-actions github-actions bot added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jan 31, 2025
@gracewilcox gracewilcox self-assigned this Jan 31, 2025
@gracewilcox gracewilcox removed the question The issue doesn't require a change to the product in order to be resolved. Most issues start as that label Jan 31, 2025
@gracewilcox
Copy link
Member

Hi @strager! Thank you for your detailed report and PR! We'll look into this issue and decide if it makes sense to add to the SDK.

@gracewilcox gracewilcox added this to the 2025-03 milestone Jan 31, 2025
@gracewilcox gracewilcox added bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. and removed needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team feature-request This issue requires a new behavior in the product in order be resolved. labels Jan 31, 2025
@jhendrixMSFT
Copy link
Member

The versions of azsecrets and keyvault/internal predate GA releases. Can you confirm if this issue persists with the latest versions (v1.3.0 and v1.1.1 respectively)?

@jhendrixMSFT jhendrixMSFT added the needs-author-feedback Workflow: More information is needed from author to address the issue. label Feb 3, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 3, 2025

Hi @strager. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

@strager
Copy link
Author

strager commented Feb 3, 2025

@jhendrixMSFT I just tested with github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/[email protected] and github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/[email protected]. The issue still occurred.

I don't see a version v1.1.1 for github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal:

$ go get github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/[email protected]
go: github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/[email protected]: invalid version: unknown revision sdk/security/keyvault/internal/v1.1.1

@github-actions github-actions bot added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-author-feedback Workflow: More information is needed from author to address the issue. labels Feb 3, 2025
@jhendrixMSFT
Copy link
Member

Ooops my fault, v1.1.0 is correct. Thanks for confirming this is still an issue.

@gracewilcox gracewilcox removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Feb 6, 2025
@gracewilcox gracewilcox moved this from Untriaged to In Progress in Azure SDK for Key Vault Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gracewilcox gracewilcox

Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. KeyVault
Projects
Azure SDK for Key Vault
Status: In Progress
Milestone
2025-03
Development

Successfully merging a pull request may close this issue.

[keyvault/azsecrets] make azsecrets.Client thread-safe traduality/azure-sdk-for-go
3 participants
@strager @jhendrixMSFT @gracewilcox