Skip to content

Conversation

anisurrahman75
Copy link

@anisurrahman75 anisurrahman75 commented Aug 6, 2025

Explaining the PR

When creating a NooBaa instance that references an external PostgreSQL secret, the secret may not exist at the time of creation. This change ensures that once the external secret is created, the NooBaa system receives a reconciliation request to process it accordingly.

Summary by CodeRabbit

  • New Features
    • The system now automatically reacts to changes in Kubernetes Secrets referenced by NooBaa external Postgres settings, triggering reconciliation so secret updates are applied.
    • RPC messages now prompt a rate-limited reconcile for the NooBaa system, ensuring external events can force timely refreshes and state convergence.

Copy link

coderabbitai bot commented Aug 6, 2025

Walkthrough

Added a Secret watch to the NooBaa controller to map Secret events to NooBaa reconciliations, a utility to obtain a NooBaa external Postgres Secret reference, a mapping function to find NooBaa resources referencing a Secret, and updated the global RPC handler to enqueue a reconcile for the NooBaa system on each RPC message.

Changes

Cohort / File(s) Change Summary
Controller: Secret watch & RPC enqueue
pkg/controller/noobaa/noobaa_controller.go
Added a watch for Secret resources that maps Secret events to NooBaa reconcile requests via system.MapSecretToNooBaa, using logEventsPredicate and returning errors on setup failure. Updated the global RPC handler to push a rate-limited reconcile.Request (NooBaa system NamespacedName) into notificationSource on each RPC message.
System: Secret-to-NooBaa Mapping
pkg/system/system.go
Added exported MapSecretToNooBaa(types.NamespacedName) []reconcile.Request which lists NooBaa resources in the secret's namespace and returns reconcile requests for those whose external Postgres secret matches the provided Secret.
Util: External Secret Reference Helper
pkg/util/util.go
Added exported GetNooBaaExternalPgSecret(*nbv1.NooBaa) *corev1.SecretReference to construct/normalize the external Postgres SecretReference, defaulting namespace to the NooBaa resource's namespace when unset.

Sequence Diagram(s)

sequenceDiagram
    participant K8sAPI as K8s API Server
    participant SecretWatch as Secret Watch
    participant Controller as NooBaa Controller
    participant System as system.MapSecretToNooBaa
    participant NooBaa as NooBaa CRs
    participant RPC as Global RPC handler
    participant Queue as notificationSource (rate-limited)

    K8sAPI->>SecretWatch: Secret event (create/update/delete)
    SecretWatch->>Controller: Enqueue mapping request (NamespacedName)
    Controller->>System: MapSecretToNooBaa(secret)
    System->>NooBaa: List NooBaa resources in namespace
    System->>Controller: Return matching reconcile requests
    Controller->>NooBaa: Enqueue reconciliation for affected NooBaa resources

    Note right of RPC: Separate flow for incoming RPC messages
    RPC->>Queue: Push reconcile.Request(NooBaa system NamespacedName)
    Queue->>Controller: Trigger reconciliation (rate-limited)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@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

🧹 Nitpick comments (2)
pkg/system/system.go (2)

1297-1298: Fix inconsistent comment.

The comment mentions "backingstores" but the function is clearly designed to map secrets to NooBaa resources, not backing stores.

-// MapSecretToBackingStores returns a list of backingstores that uses the secret in their secretReference
-// used by backingstore_controller to watch secrets changes
+// MapSecretToNooBaa returns a list of NooBaa resources that reference the secret as their external Postgres secret
+// used by noobaa_controller to watch secret changes

1306-1306: Fix grammatical error in log message.

-		log.Infof("Could not found NooBaa in namespace %q, while trying to find NooBaa that uses %s secret", secret.Namespace, secret.Name)
+		log.Infof("Could not find NooBaa resources in namespace %q, while trying to find NooBaa that uses %s secret", secret.Namespace, secret.Name)
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e52237f and c642a9c.

📒 Files selected for processing (3)
  • pkg/controller/noobaa/noobaa_controller.go (1 hunks)
  • pkg/system/system.go (1 hunks)
  • pkg/util/util.go (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: Neon-White
PR: noobaa/noobaa-operator#1587
File: pkg/system/reconciler.go:287-291
Timestamp: 2025-07-14T10:59:21.959Z
Learning: NooBaa operator design constraint: Multiple NooBaa instances are not supported in the same namespace, only across different namespaces. This means hard-coded resource names like ConfigMaps are acceptable within a namespace scope.
🔇 Additional comments (2)
pkg/controller/noobaa/noobaa_controller.go (1)

138-147: LGTM! Well-integrated Secret watch implementation.

The Secret watch implementation follows the established pattern used by other resource watches in this controller:

  • Uses EnqueueRequestsFromMapFunc handler pattern consistent with the StorageClass watch
  • Properly extracts the secret's namespaced name and delegates to system.MapSecretToNooBaa
  • Applies logging predicate consistently with other watches
  • Includes proper error handling

This enables the NooBaa controller to react to changes in external PostgreSQL secrets, supporting the PR objective of reconciliation logic for external secret creation.

pkg/system/system.go (1)

1312-1324: LGTM! Core logic is well implemented.

The function correctly:

  • Iterates through NooBaa resources in the namespace
  • Uses the utility function to get external Postgres secret references
  • Handles nil checks appropriately
  • Matches secret names accurately
  • Builds proper reconcile requests

This will enable the controller to properly trigger reconciliation when external Postgres secrets are created or updated.

@nimrod-becker
Copy link
Contributor

@dannyzaken @jackyalbo

Copy link

@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

🧹 Nitpick comments (1)
pkg/system/system.go (1)

1297-1299: Fix misleading doc comment (mentions BackingStores and wrong controller).

This exported function maps Secrets to NooBaa reconciles, not BackingStores. Update the comment so godoc and readers don’t get misled.

-// MapSecretToBackingStores returns a list of backingstores that uses the secret in their secretReference
-// used by backingstore_controller to watch secrets changes
-//
+// MapSecretToNooBaa maps a Secret event to NooBaa reconciliations.
+// It returns requests for NooBaa CRs whose Spec.ExternalPgSecret matches the Secret.
+// Used by the NooBaa controller's Secret watch.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c642a9c and 2a09666.

📒 Files selected for processing (3)
  • pkg/controller/noobaa/noobaa_controller.go (1 hunks)
  • pkg/system/system.go (1 hunks)
  • pkg/util/util.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • pkg/util/util.go
  • pkg/controller/noobaa/noobaa_controller.go
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/system/system.go (2)
pkg/util/util.go (3)
  • Logger (902-904)
  • KubeList (645-666)
  • GetNooBaaExternalPgSecret (1866-1875)
pkg/apis/noobaa/v1alpha1/noobaa_types.go (1)
  • NooBaaList (59-70)

@anisurrahman75
Copy link
Author

@dannyzaken, Brother, can you review this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants