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

replace global vault handlers with newVaultHandlers() #27515

Merged
merged 5 commits into from
Jun 18, 2024

Conversation

thyton
Copy link
Contributor

@thyton thyton commented Jun 17, 2024

Description

This PR addresses the data race caused by #27394 after merged into the ENT repo

 =============== test-go (4) ===========================
WARNING: DATA RACE
Write at 0x00c002d54180 by goroutine 63655:
  runtime.mapassign_faststr()
      /home/runner/actions-runner/_work/_tool/go/1.22.4/x64/src/runtime/map_faststr.go:203 +0x0
  maps.Copy[go.shape.map[string]github.com/hashicorp/vault/sdk/physical.Factory,go.shape.map[string]github.com/hashicorp/vault/sdk/physical.Factory,go.shape.string,go.shape.func(map[string]string, github.com/hashicorp/go-hclog.Logger) (github.com/hashicorp/vault/sdk/physical.Backend, error)]()
      /home/runner/actions-runner/_work/_tool/go/1.22.4/x64/src/maps/maps.go:55 +0x167
  github.com/hashicorp/vault/command.extendAddonCommands()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/commands_full.go:94 +0x36
  github.com/hashicorp/vault/command.initCommands()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/commands.go:175 +0x56
  github.com/hashicorp/vault/command.RunCustom()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/main.go:225 +0xfee
  github.com/hashicorp/vault/command.execPKIVerifyNonJson()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/pki_verify_sign_test.go:150 +0x2d6
  github.com/hashicorp/vault/command.execPKIVerifyJson()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/pki_verify_sign_test.go:131 +0x92
  github.com/hashicorp/vault/command.runPkiListIntermediateTests()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/pki_list_intermediate_test.go:211 +0x212a
  github.com/hashicorp/vault/command.TestPKIListIntermediate()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/pki_list_intermediate_test.go:36 +0x7a
  testing.tRunner()
      /home/runner/actions-runner/_work/_tool/go/1.22.4/x64/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      /home/runner/actions-runner/_work/_tool/go/1.22.4/x64/src/testing/testing.go:1742 +0x44

Previous write at 0x00c002d54180 by goroutine 63654:
  runtime.mapassign_faststr()
      /home/runner/actions-runner/_work/_tool/go/1.22.4/x64/src/runtime/map_faststr.go:203 +0x0
  maps.Copy[go.shape.map[string]github.com/hashicorp/vault/sdk/physical.Factory,go.shape.map[string]github.com/hashicorp/vault/sdk/physical.Factory,go.shape.string,go.shape.func(map[string]string, github.com/hashicorp/go-hclog.Logger) (github.com/hashicorp/vault/sdk/physical.Backend, error)]()
      /home/runner/actions-runner/_work/_tool/go/1.22.4/x64/src/maps/maps.go:55 +0x167
  github.com/hashicorp/vault/command.extendAddonCommands()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/commands_full.go:94 +0x36
  github.com/hashicorp/vault/command.initCommands()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/commands.go:175 +0x56
  github.com/hashicorp/vault/command.RunCustom()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/main.go:225 +0xfee
  github.com/hashicorp/vault/command.createComplicatedIssuerSetUpWithIssueIntermediate()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/pki_issue_intermediate_test.go:151 +0x1653
  github.com/hashicorp/vault/command.TestPKIIssueIntermediate()
      /home/runner/actions-runner/_work/vault-enterprise/vault-enterprise/command/pki_issue_intermediate_test.go:35 +0x6b
  testing.tRunner()
      /home/runner/actions-runner/_work/_tool/go/1.22.4/x64/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      /home/runner/actions-runner/_work/_tool/go/1.22.4/x64/src/testing/testing.go:1742 +0x44

The PR replaces all global vault handlers in commands.go with newVaultHandlers() that returns *vaultHandlers struct that encapsulates all vault handlers. https://github.com/hashicorp/vault-enterprise/pull/6084's been created to test and address the issue in the ENT repo.

@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jun 17, 2024
@thyton thyton added pr/no-changelog pr/no-milestone and removed hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed labels Jun 17, 2024
Copy link

github-actions bot commented Jun 17, 2024

Build Results:
All builds succeeded! ✅

Copy link

github-actions bot commented Jun 17, 2024

CI Results:
All Go tests succeeded! ✅

@peteski22 peteski22 changed the title replace global vault handlers with newVaultHanlders() replace global vault handlers with newVaultHandlers() Jun 17, 2024
command/commands.go Outdated Show resolved Hide resolved
command/commands.go Show resolved Hide resolved
command/commands.go Show resolved Hide resolved
command/commands.go Show resolved Hide resolved
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Jun 17, 2024
@thyton thyton force-pushed the VAULT-27576-replace-global-vault-handlers branch from c293ac0 to 7308312 Compare June 17, 2024 15:15
@thyton thyton enabled auto-merge (squash) June 18, 2024 15:15
@thyton thyton merged commit 28c2e94 into main Jun 18, 2024
83 checks passed
@thyton thyton deleted the VAULT-27576-replace-global-vault-handlers branch June 18, 2024 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-changelog pr/no-milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants