Bundle namespace-credential-provisioner into management setup phase#74
Merged
HiranAdikari merged 1 commit intomainfrom Apr 28, 2026
Merged
Conversation
📝 WalkthroughThis pull request integrates the namespace-credential-provisioner into the management-phase setup sequence and marks modules for migration/usage guidance. Changes Made
Impact
Closes issue WalkthroughThe PR adds a management-phase module, namespace-credential-provisioner, documented as a long‑running reconciler deployed to the Harvester cluster. It backfills and continuously provisions per-tenant namespace authentication artifacts: a scoped ServiceAccount, namespace-scoped RoleBindings, and a Sequence Diagram(s)sequenceDiagram
autonumber
participant Operator
participant Provisioner as Namespace-Credential-Provisioner
participant HarvesterAPI as Harvester Kubernetes API
participant Namespace as Tenant Namespace
participant Provider as Harvester Terraform Provider
Operator->>Provisioner: Deploy provisioner (after harvester-integration)
Provisioner->>HarvesterAPI: List existing namespaces (startup backfill)
HarvesterAPI-->>Provisioner: Namespace list
loop For each namespace missing kubeconfig
Provisioner->>HarvesterAPI: Create ServiceAccount
HarvesterAPI-->>Provisioner: SA created
Provisioner->>HarvesterAPI: Create namespace-scoped RoleBindings
HarvesterAPI-->>Provisioner: RoleBindings created
Provisioner->>HarvesterAPI: Create `harvester-vm-kubeconfig` Secret
HarvesterAPI-->>Provisioner: Secret created
end
Note over Provider,Namespace: Consumers use namespace-scoped kubeconfig to provision VMs
Provider->>HarvesterAPI: Authenticate using namespace kubeconfig
HarvesterAPI-->>Provider: Authorized
sequenceDiagram
autonumber
participant HarvesterAPI as Harvester Kubernetes API
participant Provisioner as Namespace-Credential-Provisioner
participant Namespace as Tenant Namespace
participant Provider as Harvester Terraform Provider
Namespace->>HarvesterAPI: Create namespace (event)
HarvesterAPI->>Provisioner: Notify about namespace creation
Provisioner->>HarvesterAPI: Create SA, RoleBindings, kubeconfig Secret
HarvesterAPI-->>Provisioner: Resources created
Note right of Provisioner: On namespace deletion
Namespace->>HarvesterAPI: Delete namespace (event)
HarvesterAPI->>Provisioner: Notify about namespace deletion
Provisioner->>HarvesterAPI: Remove cross-namespace `harvester-public` RoleBindings
HarvesterAPI-->>Provisioner: RoleBindings removed
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
gnudeep
previously approved these changes
Apr 23, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/architecture.md (1)
191-239:⚠️ Potential issue | 🟠 MajorUpdate the dependency graph to include Phase 2e.
The module dependency graph omits the
namespace-credential-provisioner(Phase 2e) entirely, even though it's documented in the text above (lines 110-122) as a required step. The graph currently showsrbac(Phase 2d) flowing directly toidentity(Phase 3a), skipping the provisioner.Operators following the graph will miss this critical deployment step.
📊 Suggested graph update
Insert the provisioner between rbac and identity:
└────────┘ │ │ projects/namespaces ready ▼ + ┌───────────────────────────┐ + │ namespace-credential- │ + │ provisioner (Phase 2e) │ + └─────────┬─────────────────┘ + │ credentials ready + ▼ ┌───────────────────┐ │ identity │🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/architecture.md` around lines 191 - 239, The ASCII dependency graph omits the namespace-credential-provisioner (Phase 2e) between rbac (Phase 2d) and identity (Phase 3a); update the diagram so the flow goes rbac (Phase 2d) → namespace-credential-provisioner (Phase 2e) → identity (Phase 3a), adding a box labeled "namespace-credential-provisioner (Phase 2e)" and adjusting the connecting arrows so projects/namespaces ready from rbac feed into the new provisioner, which then outputs the OIDC active input into identity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modules/management/namespace-credential-provisioner/README.md`:
- Around line 32-41: The README's deployment sequence mislabels "tenant-space"
as "Phase 5" which doesn't match docs/architecture.md; either remove the phase
number in modules/management/namespace-credential-provisioner/README.md and
refer to it as "the tenant-space module" (or similar), or update
docs/architecture.md to add a corresponding numbered phase for "tenant-space" so
both files match; edit the line containing "Phase 5 tenant-space" (and any
nearby references to "Phase 5") or add a new phase entry in docs/architecture.md
to keep numbering consistent.
In `@modules/workloads/harvester-cloud-credential/README.md`:
- Around line 32-37: The Outputs section in README.md is out of sync with
outputs.tf: remove the non-existent `kubeconfig` entry, add the actual
`service_account_name` output (as defined in outputs.tf), and ensure the
`secret_name` description matches the Secret produced by the module;
specifically update the table to include `service_account_name` with a brief
description (e.g., name of the created ServiceAccount) and delete the
`kubeconfig` row so the README matches outputs.tf.
---
Outside diff comments:
In `@docs/architecture.md`:
- Around line 191-239: The ASCII dependency graph omits the
namespace-credential-provisioner (Phase 2e) between rbac (Phase 2d) and identity
(Phase 3a); update the diagram so the flow goes rbac (Phase 2d) →
namespace-credential-provisioner (Phase 2e) → identity (Phase 3a), adding a box
labeled "namespace-credential-provisioner (Phase 2e)" and adjusting the
connecting arrows so projects/namespaces ready from rbac feed into the new
provisioner, which then outputs the OIDC active input into identity.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c9abad9-65bf-4029-b609-5661938d0146
📒 Files selected for processing (3)
docs/architecture.mdmodules/management/namespace-credential-provisioner/README.mdmodules/workloads/harvester-cloud-credential/README.md
HiranAdikari
force-pushed
the
feature/73-bundle-provisioner-into-management
branch
from
c3f89a6 to
fe4dc86
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/architecture.md`:
- Line 224: Replace the ambiguous label "vm-access kubeconfig" in the dependency
graph with the canonical artifact name "harvester-vm-kubeconfig" so the diagram
matches the rest of the docs and avoids interpretation drift; search for the
literal "vm-access kubeconfig" and update it to "harvester-vm-kubeconfig"
wherever the dependency graph or related labels are defined (ensure any
references, captions, or alt text are updated consistently).
- Line 122: Update the documentation line that currently reads "Provider
dependencies: `hashicorp/kubernetes ~> 2.30`" to match the actual constraint
used by the module `namespace-credential-provisioner` in
`modules/management/namespace-credential-provisioner/versions.tf` by changing it
to `hashicorp/kubernetes >= 2.0`; ensure the text around the "Provider
dependencies" entry reflects this `>= 2.0` constraint for the
`namespace-credential-provisioner` module.
In `@modules/management/namespace-credential-provisioner/README.md`:
- Around line 34-38: The fenced code block in README.md triggers MD040; update
the triple-backtick fence that contains "Phase 2a harvester-integration ..." to
include a language hint (e.g., add ```text) so the block is explicitly marked as
plaintext; modify the block surrounding the lines "Phase 2a
harvester-integration — registers Harvester with Rancher" and "Phase 2e
namespace-credential-provisioner ← deploy here" to use ```text instead of ```
to satisfy the markdown lint rule.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1378f1d9-b20d-48c7-84d8-c48199f7ee1a
📒 Files selected for processing (3)
docs/architecture.mdmodules/management/namespace-credential-provisioner/README.mdmodules/workloads/harvester-cloud-credential/README.md
HiranAdikari
force-pushed
the
feature/73-bundle-provisioner-into-management
branch
from
7ed8947 to
49c94ea
Compare
gnudeep
approved these changes
Apr 28, 2026
Adds README for namespace-credential-provisioner documenting the module's purpose, deployment sequence relative to harvester-integration and tenant-space, and usage example. Updates docs/architecture.md: - Adds namespace-credential-provisioner (Phase 2e) to the dependency graph between rbac (Phase 2d) and identity (Phase 3a) - Fixes provider constraint from ~> 2.30 to >= 2.0 to match versions.tf - Uses canonical secret name harvester-vm-kubeconfig in graph label Fixes harvester-cloud-credential README: replaces non-existent kubeconfig output entry with the actual service_account_name output. Closes #73 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
HiranAdikari
force-pushed
the
feature/73-bundle-provisioner-into-management
branch
from
292b43f to
a9aa1a1
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
modules/workloads/harvester-cloud-credential/README.md (1)
21-22: Clarify deployment phase as “Phase 2e” to avoid sequencing ambiguity.Line 22 currently says “Phase 2,” but this PR introduces and depends on the explicit Phase 2e step. Please change this to “Phase 2e” so operators don’t mis-sequence deployment.
As per coding guidelines, “Provide concise, actionable feedback focused on correctness and best practices.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/workloads/harvester-cloud-credential/README.md` around lines 21 - 22, Update the README text so it explicitly references Phase 2e instead of Phase 2 to avoid sequencing ambiguity: change the line that says "deploy `management/namespace-credential-provisioner` as part of Phase 2" to "deploy `management/namespace-credential-provisioner` as part of Phase 2e" (keep the surrounding sentence and backticks for `management/namespace-credential-provisioner` intact).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@modules/workloads/harvester-cloud-credential/README.md`:
- Around line 21-22: Update the README text so it explicitly references Phase 2e
instead of Phase 2 to avoid sequencing ambiguity: change the line that says
"deploy `management/namespace-credential-provisioner` as part of Phase 2" to
"deploy `management/namespace-credential-provisioner` as part of Phase 2e" (keep
the surrounding sentence and backticks for
`management/namespace-credential-provisioner` intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 670d77d2-8d30-4844-be65-4439367a481f
📒 Files selected for processing (3)
docs/architecture.mdmodules/management/namespace-credential-provisioner/README.mdmodules/workloads/harvester-cloud-credential/README.md
✅ Files skipped from review due to trivial changes (1)
- modules/management/namespace-credential-provisioner/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/architecture.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
README.mdfornamespace-credential-provisionerdocumenting it as a required Phase 2e step, its deployment sequence, inputs/outputs, security model, and upgrade pathREADME.mdforharvester-cloud-credentialmarking it deprecated — retained only for brownfield clusters; all new deployments should use the provisionerdocs/architecture.mdto include Phase 2e with a clear note that it must run beforetenant-spaceWithout this, operators following OCD docs have no guidance that the provisioner exists or that it's required before consumer teams can use the
workloads/vmmodule. They would either hand out admin kubeconfigs or build their own credential distribution mechanism.Test plan
harvester-cloud-credentialdeprecation notice is visible without breaking any existing TF configs (README only, no code changes)Closes #73
🤖 Generated with Claude Code