Skip to content

Revert "fix about wrong strategy ref type" - #1903

Merged
Andrei Kvapil (kvaps) merged 1 commit into
mainfrom
fix/backups
Jan 22, 2026
Merged

Revert "fix about wrong strategy ref type"#1903
Andrei Kvapil (kvaps) merged 1 commit into
mainfrom
fix/backups

Conversation

@androndo

@androndo Andrey Kolkov (androndo) commented Jan 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

This reverts commit 73d6e30.

What this PR does

Revert wrong type change according review request

Release note

[fix strategyRef type in backups]

Summary by CodeRabbit

Release Notes

  • Breaking Changes
    • Backup strategy references have been updated from cluster-scoped to namespace-scoped references. This change enables better namespace isolation and control. Update your backup configurations to reference strategies within the appropriate namespace.

✏️ Tip: You can customize this high-level summary in your review settings.

@dosubot dosubot Bot added size/M This PR changes 30-99 lines, ignoring generated files kind/bug Categorizes issue or PR as related to a bug labels Jan 22, 2026
@gemini-code-assist

gemini-code-assist Bot commented Jan 22, 2026

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello Andrey Kolkov (@androndo), I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request is a direct revert of a previous commit that introduced an incorrect type for the StrategyRef field within the backup API definitions. The change restores the StrategyRef to its intended corev1.TypedLocalObjectReference type, which is a standard Kubernetes API type for referencing objects. This ensures the API's correctness and removes a custom, now-obsolete type definition, simplifying the codebase and preventing potential issues with object referencing.

Highlights

  • Type Reversion: The StrategyRef field in both BackupSpec and BackupClassStrategy has been reverted from a custom TypedClusterObjectReference type back to the standard corev1.TypedLocalObjectReference.
  • Code Cleanup: The custom TypedClusterObjectReference struct definition, along with its DeepCopyInto and DeepCopy methods, has been removed as it is no longer used.
  • Dependency Update: An import for corev1 has been added to api/backups/v1alpha1/backupclass_types.go to support the use of corev1.TypedLocalObjectReference.
  • Internal Consistency: Internal controller logic and test cases have been updated to align with the reverted StrategyRef type, ensuring all components correctly reference backup strategies.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment Gemini (@gemini-code-assist) Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@coderabbitai

coderabbitai Bot commented Jan 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The changes replace a custom cluster-scoped TypedClusterObjectReference type with Kubernetes' standard corev1.TypedLocalObjectReference for backup strategy references. The custom type is removed from the API, and all references throughout the codebase are updated to use the standard namespace-aware reference type.

Changes

Cohort / File(s) Summary
API Type Definitions
api/backups/v1alpha1/backup_types.go, api/backups/v1alpha1/backupclass_types.go		 Changed StrategyRef field type from custom TypedClusterObjectReference to corev1.TypedLocalObjectReference; removed unused custom type definition and its DeepCopy helper methods
Internal Implementation
internal/backupcontroller/backupclass_resolver.go		 Updated ResolvedBackupConfig.StrategyRef type to corev1.TypedLocalObjectReference
Tests
internal/backupcontroller/backupclass_resolver_test.go, internal/backupcontroller/velerostrategy_controller_test.go		 Updated test fixtures and struct literals to use corev1.TypedLocalObjectReference instead of backupsv1alpha1.TypedClusterObjectReference

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A cluster-scoped type hops away,
Leaving room for local refs to stay—
Standard Kubernetes embraced with cheer,
Namespaced references, crystal clear! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: reverting a previous commit about strategy reference type. It is concise and clearly indicates the primary action being taken.

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

✨ Finishing touches
  • 📝 Generate docstrings

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

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

@androndo
Revert "fix about wrong strategy ref type"
d1d0627 
This reverts commit 73d6e30.

Signed-off-by: Andrey Kolkov <[email protected]>
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Jan 22, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request reverts commit 73d6e3013ef6f0a5484061328585260976bee134, which had introduced an incorrect type for StrategyRef. The changes correctly revert StrategyRef from a custom TypedClusterObjectReference to the standard corev1.TypedLocalObjectReference in API definitions (BackupSpec and BackupClassStrategy), internal types, and tests. This change also removes the redundant custom type definition and its associated deepcopy functions, which is a good cleanup. The changes are consistent across the codebase and appear correct. This revert resolves the issue from the original commit and improves code clarity by using standard Kubernetes types.

@kvaps
Andrei Kvapil (kvaps) merged commit cde5873 into main Jan 22, 2026
6 checks passed
@kvaps
Andrei Kvapil (kvaps) deleted the fix/backups branch January 22, 2026 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kvaps Andrei Kvapil (kvaps) Awaiting requested review from kvaps kvaps is a code owner
@lllamnyp Timofei Larkin (lllamnyp) Awaiting requested review from lllamnyp lllamnyp is a code owner
@nbykov0 Nikita (nbykov0) Awaiting requested review from nbykov0
1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

kind/bug Categorizes issue or PR as related to a bug size/M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@androndo @kvaps