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

KeyVault refactor #320

Merged
merged 6 commits into from
Jul 20, 2022
Merged

KeyVault refactor #320

merged 6 commits into from
Jul 20, 2022

Conversation

Gordonby
Copy link
Collaborator

@Gordonby Gordonby commented Jul 13, 2022 •
edited
Loading

PR Summary

  • Refactoring KeyVault to modules (1 for resource, 1 for rbac),
  • Realign parameter names used by Keyvault for consistency. breaking change
  • Addressing some of the new bicep linter warnings

References #313

Parameter changes

Several parameters have been renamed.

azureKeyvaultSecretsProvider -> keyVaultAksCSI
kvPollInterval -> keyVaultAksCSIPollInterval
createKV -> keyVaultCreate
KeyVaultSoftDelete - > keyVaultSoftDelete
KeyVaultPurgeProtection -> keyVaultPurgeProtection
kvIPAllowlist -> keyVaultIPAllowlist
kvOfficerRolePrincipalId -> keyVaultOfficerRolePrincipalId

UI Screengrab

image

PR Checklist

  • PR has a meaningful title
  • Summarized changes
  • This PR is ready to merge and is not Work in Progress
  • Link to a filed issue
  • Screenshot of UI changes (if PR includes UI changes)

@Gordonby Gordonby temporarily deployed to azurecirgs July 13, 2022 11:14 Inactive
@Gordonby Gordonby added needs-release-before-merge a preview release is required before merge to main test-deploy-privateconfig A PR flag to make sure that the Private deployment is run before merge test-deploy-wizard Used as part of release. Instructs PR to include a full release test. needed-for-v1 Needed for v1.0 labels Jul 13, 2022
@Gordonby Gordonby requested a review from khowling July 13, 2022 11:20
@Gordonby Gordonby temporarily deployed to azurecirgs July 13, 2022 11:22 Inactive
@Gordonby Gordonby temporarily deployed to azurecirgs July 13, 2022 11:22 Inactive
@Gordonby Gordonby self-assigned this Jul 13, 2022
@Gordonby Gordonby temporarily deployed to azurecirgs July 18, 2022 13:36 Inactive
@Gordonby Gordonby temporarily deployed to azurecirgs July 18, 2022 13:36 Inactive
@Gordonby Gordonby temporarily deployed to azurecirgs July 19, 2022 11:49 Inactive
@Gordonby Gordonby temporarily deployed to azurecirgs July 19, 2022 11:49 Inactive
@Gordonby Gordonby added the test-deploy-byoconfig A PR flag to make sure that the BYO-vnet deployment is run before merge label Jul 19, 2022
@Gordonby Gordonby marked this pull request as ready for review July 19, 2022 14:42
@Gordonby Gordonby temporarily deployed to azurecirgs July 19, 2022 14:42 Inactive
@Gordonby Gordonby temporarily deployed to azurecirgs July 19, 2022 14:42 Inactive
@Gordonby
Copy link
Collaborator Author

Marking this as Ready to Review after our conversation @khowling

Add any review questions to this PR.

@Gordonby Gordonby temporarily deployed to azurecirgs July 19, 2022 14:45 Inactive
cspell typos
7fdf0b8 
Signed-off-by: Gordonby <[email protected]>
@Gordonby Gordonby temporarily deployed to azurecirgs July 19, 2022 15:19 Inactive
@Gordonby Gordonby temporarily deployed to azurecirgs July 19, 2022 15:21 Inactive
@khowling khowling merged commit 247ba23 into main Jul 20, 2022
@khowling khowling deleted the gb-kvrefactor branch July 20, 2022 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khowling khowling khowling approved these changes

Assignees

@Gordonby Gordonby

Labels
helper-ui needed-for-v1 Needed for v1.0 needs-release-before-merge a preview release is required before merge to main test-deploy-byoconfig A PR flag to make sure that the BYO-vnet deployment is run before merge test-deploy-privateconfig A PR flag to make sure that the Private deployment is run before merge test-deploy-wizard Used as part of release. Instructs PR to include a full release test.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Gordonby @khowling