fix(openbao): improve install command#430
Open
Jcing95 wants to merge 5 commits into
Open
Conversation
Signed-off-by: Jcing95 <23337729+Jcing95@users.noreply.github.com>
Signed-off-by: Jcing95 <23337729+Jcing95@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the oms install openbao workflow to support deploying OpenBao into a configurable Kubernetes namespace while avoiding Helm ownership conflicts when the (cluster-scoped) Bank-Vaults Operator is already installed elsewhere. It also adjusts the install flow to create the target namespace explicitly, improves DR-restore handling of unseal keys, and introduces an interactive confirmation for destructive fresh initialization.
Changes:
- Add
--namespace/-n,--age-key-file/-k, and--yes/-yflags to the CLI and document them. - Update the OpenBao installer to be namespace-aware, skip operator redeploy when already present cluster-wide, ensure namespace creation before applying the Vault CR, and refine initialization/cleanup logic.
- Enable
preFlightChecks: truein the Vault CR manifest and update installer tests accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
NOTICE |
Fix jsonpatch/v2 license URL. |
internal/tmpl/NOTICE |
Keep NOTICE template in sync with corrected license URL. |
internal/installer/openbao.go |
Namespace-aware install flow, operator redeploy avoidance, DR restore secret handling, new pod readiness wait, expanded cleanup. |
internal/installer/openbao_test.go |
Update/add tests for new operator deployment behavior and manifest expectations. |
internal/installer/manifests/openbao/vault-cr.yaml |
Enable bank-vaults unseal preFlightChecks. |
docs/oms_install_openbao.md |
Document new CLI flags (--namespace, --age-key-file, --yes). |
cli/cmd/install_openbao.go |
Wire new flags, set SOPS key file env var, add destructive-operation confirmation prompt. |
Comments suppressed due to low confidence (1)
internal/installer/openbao.go:618
hasExistingDeploymentlists PVCs in the target namespace. If the namespace doesn't exist yet (common for a new--namespace), the List call can return a NotFound error and currently bubbles up, aborting the install during the confirmation check. It should treat a NotFound namespace as “no existing deployment” and return (false, nil).
// Vault CR gone but PVCs may linger (e.g. CR was manually deleted).
pvcList, err := o.Clientset.CoreV1().PersistentVolumeClaims(o.Config.Namespace).List(
o.ctx, metav1.ListOptions{LabelSelector: "vault_cr=openbao"},
)
if err != nil {
return false, fmt.Errorf("listing PVCs: %w", err)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| o.ctx, metav1.ListOptions{LabelSelector: "vault_cr=openbao"}, | ||
| ) | ||
| if err != nil { | ||
| return fmt.Errorf("listing stale PVCs: %w", err) |
Comment on lines
+463
to
+469
| readyCount := 0 | ||
| for i := range list.Items { | ||
| if isPodReady(&list.Items[i]) { | ||
| readyCount++ | ||
| } | ||
| } | ||
| return readyCount >= expected, nil |
Comment on lines
+236
to
+239
| // Store backup unseal keys for later use in WaitForInitialization. | ||
| // We do NOT write them to Kubernetes yet — the operator may delete or | ||
| // recreate the secret during Vault CR reconciliation, so we defer | ||
| // secret creation to the initialization wait loop where we can retry. |
Comment on lines
+52
to
+56
| // If --age-key-file is provided, set SOPS_AGE_KEY_FILE so ResolveAgeKey | ||
| // picks it up. Otherwise, fall back to the normal auto-discovery chain. | ||
| if c.Opts.AgeKeyFile != "" { | ||
| if err := os.Setenv("SOPS_AGE_KEY_FILE", c.Opts.AgeKeyFile); err != nil { | ||
| return fmt.Errorf("setting SOPS_AGE_KEY_FILE: %w", err) |
| pvcList, err := o.Clientset.CoreV1().PersistentVolumeClaims(o.Config.Namespace).List( | ||
| o.ctx, metav1.ListOptions{LabelSelector: "vault_cr=openbao"}, | ||
| ) | ||
| if err != nil { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--namespace/-nflag tooms install openbaoto allow deploying OpenBao to a custom Kubernetes namespace (defaults tovaultfor backward compatibility)os.Setenvreturn value flagged by the linter