Skip to content

Default namespace to openshift-adp instead of velero#182

Merged
Joeavaikath merged 1 commit intomigtools:oadp-devfrom
Joeavaikath:default-namespace-fix
Apr 29, 2026
Merged

Default namespace to openshift-adp instead of velero#182
Joeavaikath merged 1 commit intomigtools:oadp-devfrom
Joeavaikath:default-namespace-fix

Conversation

@Joeavaikath
Copy link
Copy Markdown
Contributor

@Joeavaikath Joeavaikath commented Apr 29, 2026

Why the changes were made

Fixes OADP-6555 — the CLI defaults to the velero namespace when no VELERO_NAMESPACE env var or ~/.config/velero/config.json exists. This is because the upstream Velero library hardcodes "velero" as DefaultNamespace. For OADP, the correct default is openshift-adp.

The fix sets openshift-adp as the namespace in the root command config when no namespace is otherwise configured. Users who have run oc oadp setup or set VELERO_NAMESPACE are unaffected.

Also removes dead code: CreateVeleroFactory and CreateNonAdminFactory in factories.go were never called.

How to test the changes made

  • Build the CLI and run oc oadp --help without a config file or VELERO_NAMESPACE env var
  • Verify the namespace flag shows (default "openshift-adp") instead of (default "velero")
  • Run the new test: go test ./cmd/ -run TestDefaultNamespaceIsOpenShiftADP -v

Summary by CodeRabbit

  • Bug Fixes

    • The CLI now defaults the namespace to openshift-adp when no namespace is configured, ensuring consistent behavior across modes.
  • Tests

    • Added an integration-style test to verify the new default namespace appears in help output when no config or environment override is present.
  • Refactor

    • Removed obsolete factory constructors to simplify internal client setup.

@openshift-ci openshift-ci Bot requested review from kaovilai and weshayutin April 29, 2026 12:47
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

Warning

Rate limit exceeded

@Joeavaikath has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 30 minutes and 58 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2e1cfb58-df59-4d44-afcb-c8ec044bae63

📥 Commits

Reviewing files that changed from the base of the PR and between 36fa18b and 416d616.

📒 Files selected for processing (3)
  • cmd/root.go
  • cmd/root_test.go
  • cmd/shared/factories.go
📝 Walkthrough

Walkthrough

Sets the default admin namespace to openshift-adp when nonadmin mode is disabled and no namespace is configured, removes two exported factory constructors from shared factories, and adds an integration-style test verifying the CLI help shows the new default namespace.

Changes

Cohort / File(s) Summary
Root command and test
cmd/root.go, cmd/root_test.go
Ensure clientcmd.ConfigKeyNamespace = "openshift-adp" is written when nonadmin mode is disabled and no namespace exists; add integration-style test that verifies --help shows openshift-adp as the default namespace.
Factory constructor removal
cmd/shared/factories.go
Removed exported constructors CreateVeleroFactory and CreateNonAdminFactory, eliminating dependence on the Velero client package; retained config read/write helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • weshayutin
  • kaovilai

Poem

🐰 I hopped through code at break of dawn,
Set namespaces so defaults spawn,
Factories tucked in a cozy heap,
Tests now watch while changes sleep,
Cheers — the CLI bounds on! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: defaulting the namespace to 'openshift-adp' instead of 'velero', which is the primary focus of the changeset.
Description check ✅ Passed The description includes both required sections with substantive content: it explains why the changes are needed (fixing OADP-6555), how they work, mentions additional cleanup, and provides clear testing instructions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 30 minutes and 58 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@Joeavaikath
Copy link
Copy Markdown
Contributor Author

/cherry-pick oadp-1.6

@openshift-cherrypick-robot
Copy link
Copy Markdown

@Joeavaikath: once the present PR merges, I will cherry-pick it on top of oadp-1.6 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick oadp-1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@PrasadJoshi12
Copy link
Copy Markdown

/lgtm

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/root.go`:
- Around line 391-392: The code assumes the VeleroConfig map returned by
clientcmd.LoadConfig() is non-nil and directly assigns
config[clientcmd.ConfigKeyNamespace] = "openshift-adp", which can panic if
LoadConfig() returned nil; update the branch that currently references config
(after calling clientcmd.LoadConfig()) to guard against a nil config by either
initializing config = make(clientcmd.VeleroConfig) before setting
config[clientcmd.ConfigKeyNamespace] or returning an error, and keep the check
around config.Namespace() and the else-if block consistent so you never index
into config when it may be nil (references: clientcmd.LoadConfig, VeleroConfig,
and the assignment to config[clientcmd.ConfigKeyNamespace]).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e794dda1-00dc-4956-846f-a717624106d7

📥 Commits

Reviewing files that changed from the base of the PR and between 98ddb0f and da204da.

📒 Files selected for processing (3)
  • cmd/root.go
  • cmd/root_test.go
  • cmd/shared/factories.go
💤 Files with no reviewable changes (1)
  • cmd/shared/factories.go

Comment thread cmd/root.go
@openshift-ci openshift-ci Bot removed the lgtm label Apr 29, 2026
When no VELERO_NAMESPACE env var or config file is present, the Velero
library falls back to "velero" as the default namespace. Set it to
"openshift-adp" in the root command when no namespace is configured.

Guard against nil config map from LoadConfig errors.

Remove unused CreateVeleroFactory and CreateNonAdminFactory functions
from factories.go.

Signed-off-by: Joseph <jvaikath@redhat.com>
@Joeavaikath Joeavaikath force-pushed the default-namespace-fix branch from 36fa18b to 416d616 Compare April 29, 2026 14:42
Copy link
Copy Markdown
Member

@kaovilai kaovilai left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Apr 29, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Joeavaikath, kaovilai, weshayutin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [Joeavaikath,kaovilai,weshayutin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Joeavaikath Joeavaikath merged commit c7db498 into migtools:oadp-dev Apr 29, 2026
13 checks passed
@openshift-cherrypick-robot
Copy link
Copy Markdown

@Joeavaikath: new pull request created: #183

Details

In response to this:

/cherry-pick oadp-1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants