Skip to content

fix(openbao): improve install command#430

Open
Jcing95 wants to merge 5 commits into
mainfrom
openbao/improve-idempotency
Open

fix(openbao): improve install command#430
Jcing95 wants to merge 5 commits into
mainfrom
openbao/improve-idempotency

Conversation

@Jcing95
Copy link
Copy Markdown
Contributor

@Jcing95 Jcing95 commented May 19, 2026

Summary

  • Add --namespace / -n flag to oms install openbao to allow deploying OpenBao to a custom Kubernetes namespace (defaults to vault for backward compatibility)
  • Fix multi-namespace support: the Bank-Vaults Operator is cluster-scoped, so skip re-deployment if it already exists in another namespace (prevents Helm ownership conflicts on ClusterRole resources)
  • Ensure the target namespace is created before applying the Vault CR, since Helm no longer implicitly creates it when the operator lives elsewhere
  • Fix unchecked os.Setenv return value flagged by the linter

@Jcing95 Jcing95 self-assigned this May 19, 2026
@Jcing95 Jcing95 changed the title fix(openbao): improve backup strategy fix(openbao): improve install command May 19, 2026
Jcing95 and others added 3 commits May 19, 2026 16:22
Signed-off-by: Jcing95 <23337729+Jcing95@users.noreply.github.com>
Signed-off-by: Jcing95 <23337729+Jcing95@users.noreply.github.com>
@Jcing95 Jcing95 requested a review from Copilot May 21, 2026 09:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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/-y flags 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: true in 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

  • hasExistingDeployment lists 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 {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants