Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix bug where KeyVault with ARMID owner can't be recovered #4127

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

matthchr
Copy link
Member

Fixes #4117.

Also fix an issue where we gave a confusing and non-fatal error when attempting to recover a KeyVault in a different ResourceGroup than it was originally in. Changed the error to be clearer and also immediately fatal so that no retries happen.

If applicable:

  • this PR contains documentation
  • this PR contains tests
  • this PR contains YAML Samples

@matthchr
Copy link
Member Author

/ok-to-test sha=8e92842

@@ -19,12 +21,13 @@ import (

keyvault "github.com/Azure/azure-service-operator/v2/api/keyvault/v1api20230701/storage"
resources "github.com/Azure/azure-service-operator/v2/api/resources/v1api20200601/storage"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I like separating named imports from regular imports.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I brought these back but AFAIK that's not a common pattern so tools (gofump/goimports/goland import ordering) don't have an option to do this. They preserve them if there but AFAIK the standard ordering is either stdlib / everything else (2 groups) or stdlib / external / internal (3 groups) and this results in 4.

Fixes Azure#4117.

Also fix an issue where we gave a confusing and non-fatal error when
attempting to recover a KeyVault in a different ResourceGroup than it
was originally in. Changed the error to be clearer and also immediately
fatal so that no retries happen.
@matthchr matthchr force-pushed the feature/keyvault-owner-fix branch from 8e92842 to 27f11b4 Compare June 25, 2024 19:11
@matthchr
Copy link
Member Author

/ok-to-test sha=27f11b4

@matthchr matthchr added this to the v2.9.0 milestone Jun 25, 2024
@matthchr matthchr added this pull request to the merge queue Jun 25, 2024
Merged via the queue into Azure:main with commit 589a90c Jun 25, 2024
7 checks passed
@matthchr matthchr deleted the feature/keyvault-owner-fix branch June 25, 2024 21:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Bug: KeyVault createMode and armID owner doesn't work
2 participants