Add additive extraArgs support for Velero server and node-agent

[Shreyashxredhat]

Why the changes were made

Closes #2209

Currently, configuring additional Velero server or node-agent arguments requires using full argument overrides, which forces users to replicate the operator-generated defaults. This approach is difficult to maintain and can lead to configuration drift when defaults change in future releases.

This change introduces a new extraArgs field for both VeleroConfig and NodeAgentConfig. The field allows users to add or override specific command-line arguments while preserving the operator-managed defaults.

Key behaviors:

• extraArgs are merged with the operator-generated arguments.
• Existing arguments can be overridden by specifying the same flag in extraArgs.
• Supports both --flag=value and --flag value argument formats.
• Duplicate flags are deduplicated by flag name.
• Unsupported-args annotations continue to have the highest precedence.

Precedence order:

Operator defaults / individual DPA fields / Args override
→ extraArgs
→ unsupported-args annotation ConfigMap

How to test the changes made

1. Deploy the operator with this change.

2. Create a DPA with extraArgs configured:

spec:
  configuration:
    velero:
      extraArgs:
        resource-timeout: "20m"
        custom-flag: "value"
    nodeAgent:
      uploaderType: kopia
      extraArgs:
        data-mover-prepare-timeout: "45m"

3. Verify the Velero deployment contains the additional arguments:

oc get deployment velero -n openshift-adp -o json | jq '.spec.template.spec.containers[0].args'

4. Verify the node-agent daemonset contains the additional arguments:

oc get daemonset node-agent -n openshift-adp -o json | jq '.spec.template.spec.containers[0].args'

5. Verify that specifying an existing flag in extraArgs overrides the operator-generated value.

6. Run the test suite:

make test

Expected results:

• Velero and node-agent include the configured extraArgs.
• Existing flags are overridden when specified in extraArgs.
• Operator-managed defaults remain intact.
• All unit tests pass successfully.

Summary by CodeRabbit

• New Features
  • Added new configuration maps to specify additional Velero and Node-Agent server CLI arguments. These extraArgs are merged additively on top of operator defaults and existing args, with unsupported-args taking highest precedence.
• Testing
  • Added unit and reconciliation tests covering extraArgs composition, overrides, and precedence behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 865de5c8-cbc6-49dd-8d2e-3ea557ea1bed

📥 Commits

Reviewing files that changed from the base of the PR and between ee1c692 and edc8434.

📒 Files selected for processing (6)

• api/v1alpha1/dataprotectionapplication_types.go
• bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml
• config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml
• internal/controller/velero.go
• pkg/common/common.go
• pkg/common/common_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• pkg/common/common_test.go
• bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml
• config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml

---

Walkthrough

This PR adds extraArgs support to both Velero and Node-Agent configurations within the OADP operator. Users can now append additional CLI flags additively instead of overriding all defaults, addressing the need to add single flags without reproducing entire default argument lists.

Changes

ExtraArgs feature for Velero and Node-Agent

Layer / File(s)	Summary
API schema and CRD definitions api/v1alpha1/dataprotectionapplication_types.go, bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml, config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml	VeleroConfig and NodeAgentConfig types now include optional ExtraArgs map[string]string fields. Both bundle and base CRD YAML schemas are updated to expose these as string-to-string maps for users to set custom flag arguments.
MergeExtraArgs utility and unit tests pkg/common/common.go, pkg/common/common_test.go	New MergeExtraArgs helper deterministically merges extra CLI flags into an existing args slice by replacing conflicting flags in-place, deduplicating, and appending new flags in sorted order. Comprehensive unit tests validate nil/empty inputs, both --key=value and --key value flag formats, mixed replacement-and-append scenarios, and deduplication behavior.
NodeAgent extra args integration internal/controller/nodeagent.go, internal/controller/nodeagent_test.go	customizeNodeAgentDaemonset calls MergeExtraArgs to append NodeAgentConfig.ExtraArgs to the NodeAgent container arguments after --log-format logic. Three test cases verify appending behavior, field-value override capability, and precedence when unsupported server-args annotation is present.
Velero extra args integration internal/controller/velero.go, internal/controller/velero_test.go	buildVeleroDeployment conditionally merges VeleroConfig.ExtraArgs into Velero container arguments. Four test cases validate appending to operator defaults, override of conflicting flags, precedence with unsupported server args annotation, and composition with VeleroConfig.Args.

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 14 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (14 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding extraArgs support for Velero server and node-agent configurations.
Description check	✅ Passed	The description includes both required sections: 'Why the changes were made' (with detailed explanation and linked issue) and 'How to test the changes made' (with deployment steps and verification commands).
Linked Issues check	✅ Passed	The PR fully implements all coding requirements from issue #2209: API fields added to VeleroConfig and NodeAgentConfig, MergeExtraArgs helper implemented with deduplication, integration into velero.go and nodeagent.go, and comprehensive unit tests covering merge logic, override behavior, and annotation precedence.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to issue #2209: API definitions, CRD schemas, merge logic implementation, controller integration, and test coverage. One minor change (debug logging removal in velero.go) is a justified refinement for the same feature.
Stable And Deterministic Test Names	✅ Passed	All test names are static and deterministic with no dynamic values, timestamps, UUIDs, or generated identifiers. Table-driven tests use static 'name' fields.
Test Structure And Quality	✅ Passed	Tests use table-driven patterns with descriptive names, meaningful assertions, and proper validation. Not Ginkgo-based, so BeforeEach/AfterEach patterns don't apply.
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests added. PR only adds unit tests using fake Kubernetes clients, which cannot have MicroShift compatibility issues.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	This PR does not add any new Ginkgo e2e tests. Changes are limited to API types, CRD manifests, controller logic, and unit tests; no e2e test files are modified.
Topology-Aware Scheduling Compatibility	✅ Passed	PR introduces only CLI argument merging functionality without any scheduling constraints, affinity rules, node selectors, topology spread constraints, or PodDisruptionBudgets.
Ote Binary Stdout Contract	✅ Passed	PR adds extraArgs support without stdout violations: no fmt.Print in new code, no unredirected klog, no new init(), all logging uses logr interface.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	This PR adds only standard Go unit tests (func TestXXX), not Ginkgo e2e tests. No It(), Context(), When() patterns found. Check is not applicable.
No-Weak-Crypto	✅ Passed	No weak cryptography, custom crypto implementations, or non-constant-time secret comparisons detected. PR adds argument merging functionality for CLI flags without any cryptographic operations.
Container-Privileges	✅ Passed	PR adds ExtraArgs field for CLI argument merging only; no privileged containers, hostPID/Network/IPC, SYS_ADMIN capabilities, or allowPrivilegeEscalation settings are introduced.
No-Sensitive-Data-In-Logs	✅ Passed	ExtraArgs are never logged. MergeExtraArgs() contains no logging. Only safe operation-type logging added. Commit explicitly removed debug logging that could expose sensitive data.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[openshift-ci]

Hi @Shreyashxredhat. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Shreyashxredhat
Once this PR has been reviewed and has the lgtm label, please assign dymurray for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@api/v1alpha1/dataprotectionapplication_types.go`:
- Around line 373-377: Update the comment for the ExtraArgs field(s) to
explicitly document the full precedence order including the unsupported-args
annotation as the final override; specifically, in
dataprotectionapplication_types.go update the comment above the ExtraArgs
map[string]string `json:"extraArgs,omitempty"` (and the similar comment at the
other occurrence) to state that ExtraArgs are applied additively on top of
operator defaults and Args, and then any unsupported-args annotation on the
resource is applied last and overrides conflicting keys.

In `@config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml`:
- Around line 297-304: The extraArgs map currently allows arbitrary keys; add a
propertyNames schema to constrain map keys to valid flag names (no leading "--",
no whitespace or empty strings). Update the extraArgs schema (identifier:
extraArgs in the YAML) to include a propertyNames block with a regex pattern
(e.g. ^[A-Za-z0-9][A-Za-z0-9_.-]*$ or other agreed flag-name pattern) and
optionally minLength: 1 so invalid keys like "--log-level", empty strings or
strings with spaces are rejected at the CRD boundary.

🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 891c2791-4260-4690-bb85-edc1fc97c193

📥 Commits

Reviewing files that changed from the base of the PR and between 627d54e and ee1c692.

⛔ Files ignored due to path filters (1)

• api/v1alpha1/zz_generated.deepcopy.go is excluded by !**/zz_generated*

📒 Files selected for processing (9)

• api/v1alpha1/dataprotectionapplication_types.go
• bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml
• config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml
• internal/controller/nodeagent.go
• internal/controller/nodeagent_test.go
• internal/controller/velero.go
• internal/controller/velero_test.go
• pkg/common/common.go
• pkg/common/common_test.go