Skip to content

fix(openbao): improve install command#430

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

fix(openbao): improve install command#430
Jcing95 wants to merge 7 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 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.

Comment thread internal/installer/openbao.go Outdated
Comment thread internal/installer/openbao.go Outdated
Comment thread internal/installer/openbao.go
Comment thread cli/cmd/install_openbao.go
Comment thread internal/installer/openbao.go
@Jcing95 Jcing95 force-pushed the openbao/improve-idempotency branch 2 times, most recently from 3614c52 to 8ac7141 Compare May 27, 2026 14:00
@Jcing95 Jcing95 requested a review from Copilot May 27, 2026 14:01
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

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment thread internal/installer/openbao.go
Comment thread internal/installer/openbao.go
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

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comment thread internal/installer/openbao.go
Comment thread internal/installer/openbao.go Outdated
Comment thread internal/installer/openbao.go
@Jcing95 Jcing95 force-pushed the openbao/improve-idempotency branch from 42eba86 to 6044373 Compare May 28, 2026 15:45
@Jcing95 Jcing95 requested a review from Copilot May 28, 2026 15:46
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

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

}

// NewOpenBaoInstaller constructs an OpenBaoInstaller with real Kubernetes and Helm clients.
func NewOpenBaoInstaller(cfg OpenBaoInstallerConfig) (*OpenBaoInstaller, error) {
Comment on lines +590 to +594
for i := range pvcList.Items {
delErr = o.Clientset.CoreV1().PersistentVolumeClaims(o.Config.Namespace).Delete(
o.ctx, pvcList.Items[i].Name, metav1.DeleteOptions{},
)
if delErr != nil && !k8serrors.IsNotFound(delErr) {
openbao.cmd.Flags().StringVar(&openbao.Opts.StorageSize, "storage-size", "10Gi", "PVC storage size for each OpenBao replica")
openbao.cmd.Flags().DurationVar(&openbao.Opts.Timeout, "timeout", 5*time.Minute, "Timeout for waiting on initialization")
openbao.cmd.Flags().StringVarP(&openbao.Opts.AgeKeyFile, "age-key-file", "k", "", "Path to age private key file for SOPS encryption/decryption (auto-detected if not set)")
openbao.cmd.Flags().BoolVarP(&openbao.Opts.Yes, "yes", "y", false, "Auto-approve fresh initialization when no DR backup is found")
--secrets-engine string Name of the KV-v2 secrets engine to provision (default "cs-secrets-engine")
--storage-size string PVC storage size for each OpenBao replica (default "10Gi")
--timeout duration Timeout for waiting on initialization (default 5m0s)
-y, --yes Auto-approve fresh initialization when no DR backup is found
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