Address CodeRabbit review comments on PR #25

bootstrap/main.tf:
- Add check block: create_ssh_key must be true when create_cloudinit_secret
  is true (cloud-init template embeds the generated SSH public key)
- Add check block: existing_cloudinit_secret_name must be non-empty when
  create_cloudinit_secret = false

bootstrap/examples/basic/main.tf:
- Replace removed rancher_admin_password with bootstrap_password

management/cluster-roles/main.tf:
- Split harvesterhci.io rule: virtualmachineimages stays read-only;
  keypairs gets create+delete so vm-manager can inject/remove SSH keys
  via workloads/vm without RBAC denials

management/cluster-roles/README.md:
- Add network-manager to roles table and network_manager_role_id to
  outputs table to document the cluster-scoped role

management/harvester-integration/examples/basic/main.tf:
- Remove kubernetes provider (no longer required by module)
- Replace removed rancher_hostname/rancher_lb_ip with cloud_credential_name
  and cluster_labels; update module source to wso2 org

management/networking/examples/basic/main.tf:
management/storage/examples/basic/main.tf:
- Bump harvester provider from ~> 0.6.0 to ~> 1.7 to match all other
  modules and prevent init failures from conflicting constraints

workloads/vm/main.tf:
- Replace sensitive-derived count with non-sensitive create_ssh_key bool
- Make wait_for_lease configurable via variable (default true)

workloads/vm/variables.tf:
- Add create_ssh_key and wait_for_lease variables

workloads/vm/README.md:
- Fix cloud-init resource description (no standalone secret is created)
- Use file(pathexpand("~/.ssh/...")) in examples (file() does not expand ~)
- Add create_ssh_key and wait_for_lease to inputs table

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: HiranAdikari

modules/bootstrap/examples/basic/main.tf

@@ -47,7 +47,7 @@ module "bootstrap" {
 
   # Credentials – supply via tfvars or environment variables; never hardcode
   vm_password            = var.vm_password
-  rancher_admin_password = var.rancher_admin_password
+  bootstrap_password = var.bootstrap_password
 
   rancher_hostname = "rancher.example.internal"
 
@@ -63,7 +63,7 @@ variable "vm_password" {
   sensitive = true
 }
 
-variable "rancher_admin_password" {
+variable "bootstrap_password" {
   type      = string
   sensitive = true
 }

modules/bootstrap/main.tf

@@ -53,6 +53,25 @@ locals {
   cloudinit_secret_name = var.create_cloudinit_secret ? harvester_cloudinit_secret.cloudinit[0].name : var.existing_cloudinit_secret_name
 }
 
+# ── Input validation ──────────────────────────────────────────────────────────
+
+# The cloud-init template embeds the generated SSH public key. If create_ssh_key
+# is false the tls_private_key resource is empty, causing an invalid-index error.
+check "ssh_key_required_for_cloudinit" {
+  assert {
+    condition     = !var.create_cloudinit_secret || var.create_ssh_key
+    error_message = "create_ssh_key must be true when create_cloudinit_secret is true (the cloud-init template embeds the generated SSH public key)."
+  }
+}
+
+# When reusing an existing cloud-init secret, the name must be provided.
+check "existing_cloudinit_secret_name_required" {
+  assert {
+    condition     = var.create_cloudinit_secret || var.existing_cloudinit_secret_name != ""
+    error_message = "existing_cloudinit_secret_name is required when create_cloudinit_secret = false."
+  }
+}
+
 # ── Rancher server VM ─────────────────────────────────────────────────────────
 resource "harvester_virtualmachine" "rancher_server" {
   count                = var.node_count

modules/management/cluster-roles/README.md

@@ -17,6 +17,7 @@ module is not required.
 | Role Name | Context | Purpose |
 |-----------|---------|---------|
 | `vm-manager` | project | Full lifecycle management of VMs: create, configure, start/stop/restart, console, and delete. |
+| `network-manager` | cluster | Manage Harvester VLAN infrastructure and NetworkAttachmentDefinitions. Bind only via `rancher2_cluster_role_template_binding`. |
 | `vm-metrics-observer` | project | Read-only access to VM status and metrics. No mutating verbs. |
 
 ### `vm-manager` Permissions
@@ -27,7 +28,8 @@ module is not required.
 | `subresources.kubevirt.io` | `virtualmachines/start`, `virtualmachines/stop`, `virtualmachines/restart`, `virtualmachines/migrate`, `virtualmachineinstances/vnc`, `virtualmachineinstances/console`, `virtualmachineinstances/portforward`, `virtualmachineinstances/pause`, `virtualmachineinstances/unpause` | `get`, `update` |
 | `subresources.kubevirt.io` | `virtualmachineinstances/metrics` | `get` |
 | `cdi.kubevirt.io` | `datavolumes` | `get`, `list`, `watch`, `create`, `update`, `patch`, `delete` |
-| `harvesterhci.io` | `virtualmachineimages`, `keypairs` | `get`, `list`, `watch` |
+| `harvesterhci.io` | `virtualmachineimages` | `get`, `list`, `watch` |
+| `harvesterhci.io` | `keypairs` | `get`, `list`, `watch`, `create`, `delete` |
 | `""` (core) | `secrets`, `configmaps` | `get`, `list`, `watch`, `create`, `update`, `patch`, `delete` |
 | `""` (core) | `services/proxy` | `get` |
 
@@ -94,6 +96,7 @@ module "tenant_space_iam_observer" {
 | Name | Description |
 |------|-------------|
 | `vm_manager_role_id` | Role template ID for `vm-manager`. Pass to `tenant-space` `group_role_bindings`. |
+| `network_manager_role_id` | Role template ID for `network-manager` (cluster-scoped). Pass to `rancher2_cluster_role_template_binding`. |
 | `vm_metrics_observer_role_id` | Role template ID for `vm-metrics-observer`. Pass to `tenant-space` `group_role_bindings`. |
 
 ## Notes

modules/management/cluster-roles/main.tf

@@ -38,13 +38,20 @@ resource "rancher2_role_template" "vm_manager" {
     verbs      = ["get", "list", "watch", "create", "update", "patch", "delete"]
   }
 
-  # Read access to VM images and SSH keypairs available in the namespace
+  # Read access to VM images available in the namespace
   rules {
     api_groups = ["harvesterhci.io"]
-    resources  = ["virtualmachineimages", "keypairs"]
+    resources  = ["virtualmachineimages"]
     verbs      = ["get", "list", "watch"]
   }
 
+  # SSH keypairs — full CRUD so tenants can inject and remove keys via workloads/vm
+  rules {
+    api_groups = ["harvesterhci.io"]
+    resources  = ["keypairs"]
+    verbs      = ["get", "list", "watch", "create", "delete"]
+  }
+
   # Cloud-init secrets and SSH key secrets
   rules {
     api_groups = [""]

modules/management/harvester-integration/examples/basic/main.tf

@@ -2,16 +2,14 @@
 #
 # This example integrates a Harvester cluster into an existing Rancher server
 # by enabling the Harvester feature flag, installing the UI extension, creating
-# a cloud credential, patching CoreDNS so Harvester can resolve the internal
-# Rancher FQDN, and applying the registration manifest.
+# a cloud credential, and applying the registration manifest.
 #
 # Prerequisites:
 #   - Rancher deployed and accessible (e.g. via the bootstrap module)
 #   - The Harvester kubeconfig available locally
 #   - kubectl available in PATH (used by local-exec provisioners)
 #   - The rancher2 provider configured with your Rancher URL and access key
 #   - The harvester provider configured with your Harvester kubeconfig
-#   - The kubernetes provider configured against the Harvester cluster
 
 terraform {
   required_version = ">= 1.3"
@@ -25,10 +23,6 @@ terraform {
       source  = "harvester/harvester"
       version = "~> 1.7"
     }
-    kubernetes = {
-      source  = "hashicorp/kubernetes"
-      version = "~> 3.0"
-    }
   }
 }
 
@@ -39,18 +33,14 @@ provider "rancher2" {
 }
 
 provider "harvester" {
-  kubeconfig = file("~/.kube/harvester-config")
-}
-
-provider "kubernetes" {
-  config_path = "~/.kube/harvester-config"
+  kubeconfig = file(pathexpand("~/.kube/harvester-config"))
 }
 
 module "harvester_integration" {
-  source = "github.com/wso2-enterprise/open-cloud-datacenter//modules/management/harvester-integration?ref=v0.1.0"
+  source = "github.com/wso2/open-cloud-datacenter//modules/management/harvester-integration?ref=v0.2.0"
 
-  harvester_kubeconfig   = file("~/.kube/harvester-config")
+  harvester_kubeconfig   = file(pathexpand("~/.kube/harvester-config"))
   harvester_cluster_name = "harvester-hci"
-  rancher_hostname       = "rancher.example.internal"
-  rancher_lb_ip          = "192.168.10.10"
+  cloud_credential_name  = "harvester-local-creds"
+  cluster_labels         = {}
 }

modules/management/networking/examples/basic/main.tf

@@ -13,7 +13,7 @@ terraform {
   required_providers {
     harvester = {
       source  = "harvester/harvester"
-      version = "~> 0.6.0"
+      version = "~> 1.7"
     }
   }
 }

modules/management/storage/examples/basic/main.tf

@@ -15,7 +15,7 @@ terraform {
   required_providers {
     harvester = {
       source  = "harvester/harvester"
-      version = "~> 0.6.0"
+      version = "~> 1.7"
     }
   }
 }

modules/workloads/vm/README.md

@@ -6,7 +6,7 @@ by product teams who have been granted a `tenant-space` with the `vm-manager` ro
 This module creates:
 - A `harvester_virtualmachine` with configurable CPU, memory, disk, and networking
 - Optionally, a `harvester_ssh_key` (when `ssh_public_key` is set)
-- Optionally, a `harvester_cloudinit_secret` (when `user_data` is set)
+- Optionally, cloud-init user-data/network-data attached through the VM resource (when `user_data` is set)
 
 ## When to Use
 
@@ -57,7 +57,7 @@ module "app_vm" {
   disk_size      = "80Gi"
   image_name     = data.terraform_remote_state.management.outputs.image_ids["ubuntu-22-04"]
   network_name   = "iam-team-vlan"
-  ssh_public_key = file("~/.ssh/id_rsa.pub")
+  ssh_public_key = file(pathexpand("~/.ssh/id_rsa.pub"))
 }
 ```
 
@@ -71,7 +71,7 @@ module "app_vm" {
   namespace      = "iam-team-ns"
   image_name     = data.terraform_remote_state.management.outputs.image_ids["ubuntu-22-04"]
   network_name   = "iam-team-vlan"
-  ssh_public_key = file("~/.ssh/id_rsa.pub")
+  ssh_public_key = file(pathexpand("~/.ssh/id_rsa.pub"))
 
   user_data = <<-EOT
     #cloud-config
@@ -112,7 +112,9 @@ locals {
 | `image_name` | Harvester image in `namespace/name` format | `string` | — | yes |
 | `network_name` | Harvester network attachment name | `string` | — | yes |
 | `run_strategy` | `RerunOnFailure`, `Always`, `Halted`, or `Manual` | `string` | `"RerunOnFailure"` | no |
-| `ssh_public_key` | SSH public key content. Creates a `harvester_ssh_key` when set. | `string` | `null` | no |
+| `ssh_public_key` | SSH public key content. Used when `create_ssh_key = true`. | `string` | `null` | no |
+| `create_ssh_key` | When true, create a `harvester_ssh_key` from `ssh_public_key`. | `bool` | `false` | no |
+| `wait_for_lease` | Wait for IP lease on primary NIC. Set false for static cloud-init IPs. | `bool` | `true` | no |
 | `user_data` | Cloud-init user-data YAML. Creates a cloud-init secret when set. | `string` | `null` | no |
 | `network_data` | Cloud-init network-data config (requires `user_data` to be set). | `string` | `""` | no |
 
@@ -132,7 +134,9 @@ locals {
 - `network_name` must match a network attachment definition in the same Harvester cluster
   (created by `management/networking`).
 - The VM's IP address is available in `network_interfaces[0].ip_address` after the
-  lease is obtained (Terraform will wait due to `wait_for_lease = true`).
+  lease is obtained (requires `wait_for_lease = true`, which is the default).
+  Set `wait_for_lease = false` when using static IPs via cloud-init `network_data`
+  without qemu-guest-agent.
 - The `vm-manager` custom role from `management/cluster-roles` must be bound to the
   team's group in their `tenant-space` before they can create VMs in the namespace.
 - Removing this module or running `terraform destroy` **deletes the VM and its disk**.

modules/workloads/vm/main.tf

@@ -1,6 +1,6 @@
 # Optional SSH key — created only when ssh_public_key is provided.
 resource "harvester_ssh_key" "this" {
-  count = var.ssh_public_key != null ? 1 : 0
+  count = var.create_ssh_key ? 1 : 0
 
   name       = "${var.name}-key"
   namespace  = var.namespace
@@ -18,11 +18,11 @@ resource "harvester_virtualmachine" "this" {
   run_strategy = var.run_strategy
   machine_type = "q35"
 
-  ssh_keys = var.ssh_public_key != null ? [harvester_ssh_key.this[0].id] : []
+  ssh_keys = var.create_ssh_key ? [harvester_ssh_key.this[0].id] : []
 
   network_interface {
     name           = "nic-1"
-    wait_for_lease = true
+    wait_for_lease = var.wait_for_lease
     network_name   = var.network_name
   }
 

modules/workloads/vm/variables.tf

@@ -64,3 +64,15 @@ variable "network_data" {
   description = "Cloud-init network-data config. Ignored if user_data is null."
   default     = ""
 }
+
+variable "create_ssh_key" {
+  type        = bool
+  description = "When true, create a harvester_ssh_key from ssh_public_key and attach it to the VM. Must be true for ssh_public_key to have any effect."
+  default     = false
+}
+
+variable "wait_for_lease" {
+  type        = bool
+  description = "Whether Terraform should wait for an IP lease on the primary NIC. Set to false when using static IPs via cloud-init network_data without qemu-guest-agent."
+  default     = true
+}