Merge branch 'platform9:main' into forkmain

Author: indradhanush

CLAUDE.md

@@ -6,18 +6,22 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 
 Cluster API Provider BYOH (BringYourOwnHost) is a Kubernetes infrastructure provider that lets operators declare, provision, and manage Kubernetes clusters on already-provisioned Linux hosts. It decouples node provisioning from host provisioning by running an agent daemon on each BYO host that registers with a management cluster.
 
+This repo (`platform9/cluster-api-provider-bringyourownhost`) is Platform9's fork of the upstream `vmware-tanzu/cluster-api-provider-bringyourownhost`. The root Go module keeps the upstream import path (`github.com/vmware-tanzu/cluster-api-provider-bringyourownhost`) for compatibility with existing imports; new Platform9-authored code (`cmd/byohctl`) uses its own module under the `github.com/platform9/...` path instead. Keep this distinction in mind when adding imports — don't assume the whole tree shares one module or one import prefix.
+
 ## Build & Development Commands
 
 ```bash
 # Build
 make build                    # Build manager binary to bin/manager
 make host-agent-binaries      # Build host agent binaries
+cd cmd/byohctl && make build  # Build byohctl CLI (separate Go module, see Architecture)
 
 # Run tests
-make test                     # All unit tests with coverage
+make test                     # All unit tests with coverage (includes cmd-test)
 make controller-test          # Controller tests only
 make agent-test               # Agent tests only
 make webhook-test             # Webhook tests only
+make cmd-test                 # byohctl tests (cd cmd && go test ./...)
 make test-e2e                 # End-to-end tests (requires a cluster)
 
 # Code quality
@@ -44,11 +48,12 @@ ginkgo -v -focus "description of test" ./controllers/infrastructure/
 
 ## Architecture
 
-### Two-Binary Design
+### Three-Binary Design
 
-The project produces two binaries:
+The project produces three binaries, two of which share the root Go module:
 - **Manager** (`main.go`) — runs in the management cluster; reconciles `ByoCluster`, `ByoMachine`, `ByoHost`, and related CRs.
 - **Host Agent** (`agent/main.go`) — runs as a daemon on each BYO host; registers the host with the management cluster and drives Kubernetes installation.
+- **byohctl** (`cmd/byohctl/`) — operator-facing CLI for onboarding, deauthorizing, and decommissioning a host (`cmd/byohctl/cmd/{onboard,deauthorise,decommission}.go`). Lives in its own Go module (`cmd/go.mod`) with its own `cmd/byohctl/Makefile`; built and tested independently of the root module — see `make cmd-test`.
 
 ### Custom Resources (`apis/infrastructure/v1beta1/`)
 
@@ -103,4 +108,23 @@ Markers in type files (e.g. `// +kubebuilder:object:root=true`) drive controller
 
 ## Linting
 
-Config is in `.golangci.yml` (timeout 10 min). Key enabled linters include `gosec`, `staticcheck`, `errcheck`, `gocyclo`, and `depguard`. Run `make lint` before submitting; CI enforces this.
+Config is in `.golangci.yml` (v2 schema, timeout 10 min). Key enabled linters include `gosec`, `staticcheck`, `errcheck`, `gocyclo`, and `depguard`. Run `make lint` before submitting; CI enforces this via `golangci-lint-action@v9` pinned to v2.12.2 (`.github/workflows/lint.yml`).
+
+Gotcha: the `golangci-lint` target in the Makefile only installs the binary if `bin/golangci-lint` doesn't already exist, and pins an older v1.64.8 install script — if you have a stale v1 binary in `bin/`, `make lint` will run against the v2-schema config and fail or disagree with CI. Delete `bin/golangci-lint` and re-run `make lint` if results look wrong.
+
+## Licensing
+
+Most existing files carry a VMware copyright header, e.g.:
+```go
+// Copyright 2021 VMware, Inc. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+```
+
+- Never remove or replace an existing copyright header, including VMware's.
+- When editing a file going forward, add a Platform9 copyright line above the `SPDX-License-Identifier` line (don't replace the existing one) using the current year:
+```go
+// Copyright 2021 VMware, Inc. All Rights Reserved.
+// Copyright 2026 Platform9, Inc. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+```
+- New files that have no prior header get a Platform9-only header.

agent/reconciler/host_reconciler.go

@@ -376,7 +376,11 @@ func (r *HostReconciler) removeAnnotations(ctx context.Context, byoHost *infrast
 	byoHost.Spec.BootstrapSecret = nil
 
 	// Remove UninstallationSecret reference (secret deletion is handled manager-side)
-	byoHost.Spec.UninstallationSecret = nil
+	//
+	// FIXME: Currently we cleanup uninstallation secret from the management plane controller.
+	// This means we have split-ownership of cleanup between the agent and the management plane controller.
+	// We should ensure agent's boundary for cleanup remains within the host itself and it should not be modifying the ByoHost CR at all. The management of the ByoHost CR is the management plane's responsibility, until the host decides to "deboard". And even then it may only ask the management plane to initiate the cleanup of the CR, but not do so itself.
+	// byoHost.Spec.UninstallationSecret = nil
 
 	// Remove cluster-name label
 	delete(byoHost.Labels, clusterv1.ClusterNameLabel)

agent/reconciler/reconciler_test.go

@@ -590,7 +590,7 @@ runCmd:
 				Expect(updatedByoHost.Annotations).NotTo(HaveKey(infrastructurev1beta1.EndPointIPAnnotation))
 				Expect(updatedByoHost.Annotations).NotTo(HaveKey(infrastructurev1beta1.K8sVersionAnnotation))
 				Expect(updatedByoHost.Annotations).NotTo(HaveKey(infrastructurev1beta1.BundleLookupBaseRegistryAnnotation))
-				Expect(updatedByoHost.Spec.UninstallationSecret).To(BeNil(),
+				Expect(updatedByoHost.Spec.UninstallationSecret).ToNot(BeNil(),
 					"UninstallationSecret reference should be cleared after successful cleanup")
 
 				k8sNodeBootstrapSucceeded := conditions.Get(updatedByoHost, infrastructurev1beta1.K8sNodeBootstrapSucceeded)
@@ -786,7 +786,7 @@ runCmd:
 				updatedByoHost := &infrastructurev1beta1.ByoHost{}
 				err := k8sClient.Get(ctx, byoHostLookupKey, updatedByoHost)
 				Expect(err).ToNot(HaveOccurred())
-				Expect(updatedByoHost.Spec.UninstallationSecret).To(BeNil())
+				Expect(updatedByoHost.Spec.UninstallationSecret).ToNot(BeNil())
 			})
 
 			It("should skip uninstallation if skip-installation flag is set", func() {

apis/infrastructure/v1beta1/byohost_webhook.go

@@ -83,9 +83,13 @@ func (v *ByoHostValidator) handleCreateUpdate(req *admission.Request) admission.
 	// An agent's username encodes the host it owns as the third colon-separated segment
 	// (format: byoh:host:<hostname>). Reject requests where the encoded host does not
 	// match the target ByoHost — an agent must not create or update another agent's host.
-	if len(substrs) >= 3 && !strings.Contains(byoHost.Name, substrs[2]) {
-		return admission.Denied(fmt.Sprintf("%s cannot create/update resource %s", userName, byoHost.Name))
-	}
+
+	// FIXME: We only support token based kubeconfig for now. cert based flow needs a redesign. Disable it for now to allow host onboarding for the time being.
+	// NOTE: When you're fixing this, see the tests that were skipped as part of the commit that disabled this check.
+	//
+	// if len(substrs) >= 3 && !strings.Contains(byoHost.Name, substrs[2]) {
+	// 	return admission.Denied(fmt.Sprintf("%s cannot create/update resource %s", userName, byoHost.Name))
+	// }
 
 	return admission.Allowed("")
 }

apis/infrastructure/v1beta1/byohost_webhook_internal_test.go

@@ -66,6 +66,7 @@ var _ = Describe("ByohostWebhook/Unit", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 		})
 		It("Should reject create request from invalid user", func() {
+			Skip("feature not implemented yet")
 			admissionRequest := admissionv1.AdmissionRequest{
 				Operation: admissionv1.Create,
 				UserInfo:  v1.UserInfo{Username: unauthorizedUser},
@@ -79,6 +80,7 @@ var _ = Describe("ByohostWebhook/Unit", func() {
 			Expect(string(resp.AdmissionResponse.Result.Reason)).To(Equal(fmt.Sprintf("%s is not a valid agent username", unauthorizedUser)))
 		})
 		It("Should reject request from another agent user in the group", func() {
+			Skip("feature not implemented yet")
 			admissionRequest := admissionv1.AdmissionRequest{
 				Operation: admissionv1.Create,
 				UserInfo:  v1.UserInfo{Username: byohHostTwoUser},
@@ -179,6 +181,7 @@ var _ = Describe("ByohostWebhook/Unit", func() {
 		})
 
 		It("Should reject request from another agent user in the group", func() {
+			Skip("feature not implemented yet")
 			admissionRequest := admissionv1.AdmissionRequest{
 				Operation: admissionv1.Update,
 				UserInfo:  v1.UserInfo{Username: byohHostTwoUser},
@@ -307,7 +310,7 @@ func TestByoHostValidator_handleCreateUpdate(t *testing.T) {
 		{
 			name:      "agent encoding a different host is denied",
 			userName:  byohHostTwoUser,
-			wantAllow: false,
+			wantAllow: true, // FIXME: This test should fail when we fix the check.
 			wantMsg:   "byoh:host:host2 cannot create/update resource host1",
 		},
 		{

cmd/byohctl/service/constants.go

@@ -13,7 +13,7 @@ const (
 	DefaultFilePerms = 0644
 
 	// ByohAgentDebPackageURL is the URL to download the agent package
-	ByohAgentDebPackageURL = "quay.io/platform9/byoh-agent-deb:0.1.366"
+	ByohAgentDebPackageURL = "quay.io/platform9/byoh-agent-deb:0.1.423"
 	// ByohAgentDebPackageFilename is the filename of the agent package
 	ByohAgentDebPackageFilename = "pf9-byohost-agent.deb"
 	// ByohAgentServiceName is the name of the agent service