🐛 upsert fcc-agent namespace independently of join

[arturshadnik]

Summary by CodeRabbit

• New Features

  • Validation preventing simultaneous use of in-cluster and secret-based kubeconfigs for hub, spoke, and managed clusters.
  • Safe defaults: context set to empty string and inCluster defaulted to false in kubeconfig blocks.

• Bug Fixes

  • More reliable agent-namespace creation during spoke join; logging consistently references the spoke.

• Chores

  • Helm values cleaned: removed secretReference.namespace fields.

• Documentation

  • Chart README updated to reflect removed secretReference.namespace fields.

[coderabbitai]

Walkthrough

Adds Helm template mutual-exclusion checks and safe defaults for kubeconfig blocks, removes several secretReference.namespace fields from chart values/README, and refactors the Spoke reconciler to centralize agent-namespace creation by changing createAgentNamespace to accept a *Spoke, updating call sites and logging.

Changes

Cohort / File(s)	Summary
Helm templates: exclusivity checks & defaults fleetconfig-controller/charts/fleetconfig-controller/templates/fleetconfig.yaml	Adds fail checks disallowing simultaneous inCluster and secretReference for hub.kubeconfig, spoke.kubeconfig, and klusterlet.managedClusterKubeconfig; sets default context: "" and inCluster: false where unspecified.
Helm values & docs: removed secretReference.namespace fleetconfig-controller/charts/fleetconfig-controller/values.yaml, fleetconfig-controller/charts/fleetconfig-controller/README.md	Removes secretReference.namespace fields for hub, spoke, and klusterlet kubeconfig secret references; formatting/spacing adjustments only for existing inCluster values; README updated to reflect removed namespace fields.
Controller refactor: agent namespace creation fleetconfig-controller/internal/controller/v1beta1/spoke_handler.go	Changes createAgentNamespace signature to accept (*v1beta1.Spoke), updates callers to pass the Spoke object, adds early-exit checks (Unified, hub-as-spoke, PivotComplete), obtains spoke kubeconfig from the Spoke spec to build a rest.Config and create the agent namespace via a Kubernetes client, and updates logging to use spoke.Name.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• refactor: fleetconfig-controller as an addon #69 — touches the same createAgentNamespace refactor and Spoke handler call-site changes.
• ✨implement separate hub and spoke resources #59 — overlaps on Helm kubeconfig validation/defaults and mutual-exclusivity checks in the charts.

Suggested labels

lgtm

Suggested reviewers

• ahmad-ibra
• TylerGillson

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title concisely captures the central code change of decoupling fcc-agent namespace creation from the join process by upserting it independently and directly reflects the updates to createAgentNamespace in the spoke_handler logic; it is focused and clear in conveying the developer’s primary intent. However, the leading emoji is stylistic noise and not necessary for understanding.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3f5c2d2 and 8bc79ec.

📒 Files selected for processing (1)

• fleetconfig-controller/charts/fleetconfig-controller/README.md (0 hunks)

💤 Files with no reviewable changes (1)

• fleetconfig-controller/charts/fleetconfig-controller/README.md

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

• GitHub Check: e2e (fleetconfig-controller) / e2e
• GitHub Check: e2e (fleetconfig-controller) / e2e
• GitHub Check: test (fleetconfig-controller) / Ensure PR is reviewable & run tests
• GitHub Check: test (fleetconfig-controller) / Run Helm Chart Tests
• GitHub Check: test (fleetconfig-controller) / Ensure PR is reviewable & run 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.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab0bba5 and 22c7db2.

📒 Files selected for processing (3)

• fleetconfig-controller/charts/fleetconfig-controller/templates/fleetconfig.yaml (3 hunks)
• fleetconfig-controller/charts/fleetconfig-controller/values.yaml (1 hunks)
• fleetconfig-controller/internal/controller/v1beta1/spoke_handler.go (3 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-09-22T18:42:03.404Z

Learnt from: arturshadnik
PR: open-cluster-management-io/lab#59
File: fleetconfig-controller/PROJECT:28-31
Timestamp: 2025-09-22T18:42:03.404Z
Learning: In the open-cluster-management-io/lab repository's fleetconfig-controller, the PROJECT file defines multiple API resources with different webhook configurations: FleetConfig v1alpha1 has defaulting: true (requiring MutatingWebhookConfiguration), while Hub and Spoke v1beta1 resources have defaulting: false. MutatingWebhookConfiguration resources in the manifests serve the v1alpha1 FleetConfig, not the v1beta1 Hub/Spoke resources.

Applied to files:

• fleetconfig-controller/charts/fleetconfig-controller/templates/fleetconfig.yaml

🧬 Code graph analysis (1)

fleetconfig-controller/internal/controller/v1beta1/spoke_handler.go (3)

fleetconfig-controller/internal/controller/v1beta1/spoke_controller.go (1)

• SpokeReconciler (52-58)

fleetconfig-controller/api/v1beta1/constants.go (2)

• InstanceTypeUnified (89-89)
• PivotComplete (31-31)

fleetconfig-controller/internal/kube/kube.go (1)

• KubeconfigFromSecretOrCluster (113-119)

🪛 GitHub Check: test (fleetconfig-controller) / Run Helm Chart Tests

fleetconfig-controller/charts/fleetconfig-controller/values.yaml

[failure] 222-222:
222:22 [trailing-spaces] trailing spaces

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: e2e (fleetconfig-controller) / e2e
• GitHub Check: test (fleetconfig-controller) / Ensure PR is reviewable & run tests
• GitHub Check: e2e (fleetconfig-controller) / e2e
• GitHub Check: test (fleetconfig-controller) / Ensure PR is reviewable & run tests

🔇 Additional comments (7)

fleetconfig-controller/charts/fleetconfig-controller/templates/fleetconfig.yaml (3)

43-52: LGTM! Mutual exclusivity check correctly prevents configuration errors.

The template correctly validates that inCluster and secretReference are mutually exclusive for hub kubeconfig, and applies safe defaults (empty string for context, false for inCluster). This prevents runtime configuration conflicts.

Based on learnings: The v1beta1 Hub/Spoke resources don't have defaulting webhooks, so these template-level checks and defaults are the correct approach.

---

87-97: LGTM! Spoke kubeconfig validation mirrors hub implementation.

The mutual exclusivity check and defaults for spoke kubeconfig follow the same pattern as the hub kubeconfig, ensuring consistent behavior across both resource types.

---

116-127: LGTM! ManagedClusterKubeconfig validation follows established pattern.

The mutual exclusivity validation for managedClusterKubeconfig is consistent with the hub and spoke kubeconfig implementations, providing uniform validation across all kubeconfig sources in the template.

fleetconfig-controller/internal/controller/v1beta1/spoke_handler.go (3)

279-311: Refactor improves encapsulation and consistency.

The refactored createAgentNamespace method now:

• Accepts the entire Spoke object instead of individual parameters
• Self-fetches the kubeconfig from the Spoke's spec using the Spoke's namespace
• Contains clear early-exit logic for cases where namespace creation is not needed
• Uses consistent logging with spoke.Name

This improves maintainability and aligns with the pattern used elsewhere in the codebase (e.g., KubeconfigFromSecretOrCluster at line 286).

---

282-284: Early-exit conditions look correct.

The early exits for Unified, hub-as-spoke, and PivotComplete instances are appropriate:

• Unified: The agent runs locally, no remote namespace needed
• Hub-as-spoke: Special case where hub acts as its own spoke
• PivotComplete: Agent has already taken over, namespace already exists

These align with the instance type checks in the related code snippets (spoke_controller.go).

---

173-181: Agent namespace creation now upserts on every reconcile
Moved createAgentNamespace outside the initial join block to align with “upsert fcc-agent namespace independently of join.” It’s safe and idempotent thanks to the IsAlreadyExists check.

fleetconfig-controller/charts/fleetconfig-controller/values.yaml (1)

225-225: Replace placeholder secret name
Use an empty default ("") instead of "foog" in fleetconfig-controller/charts/fleetconfig-controller/values.yaml.

-        name: "foog"
+        name: ""

If this is for testing, add a comment or note in the PR explaining its use.

[openshift-ci]

@karl-cardenas-coding: changing LGTM is restricted to collaborators

In response to this:

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.

[karl-cardenas-coding]

@karl-cardenas-coding: changing LGTM is restricted to collaborators

I don't need your sassy attitude robot 🤖

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: arturshadnik, karl-cardenas-coding, TylerGillson

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

The pull request process is described here

Needs approval from an approver in each of these files:

• fleetconfig-controller/OWNERS [TylerGillson,arturshadnik]

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