feat(rover): implement PatchAuthentication to map clientAuthMethod values

[stefan-ctrl]

feat(rover-ctl): implement PatchAuthentication to map clientAuthMethod values

Add only to rover-ctl to check against expected values from internal document, but do not store in controlplane.

Since is done for downwards comability, but the values is never used in identity domain. In the future, checks and casting to rover-server might be added, if this feature is wished by stakeholders.*

Branch as WIP that nearly implemented it: https://github.com/telekom/controlplane/tree/client-authentication-method

---

End-to-end mapping

Layer	Field	basic (default)	body	Tested in
rover-ctl YAML (legacy input)	spec.authentication.m2m.clientAuthMethod	basic	body	—
rover-ctl → rover-server API	authentication.clientAuthMethod	BASIC	POST	rover_test.go
rover-server inbound → CRD	spec.authentication.m2m.tokenRequest	header	body	rover_test.go
CRD → rover-server outbound	authentication.clientAuthMethod	BASIC	POST	rover_test.go
rover CRD (stored in K8s)	spec.authentication.m2m.tokenRequest	header (default)	body	rover_types.go enum: body;header
ExternalIDP CRD (aligned)	externalIDP.tokenRequest	header	body	security_types.go enum: body;header

Semantic alignment

Legacy meaning	Legacy value	API value	CRD value	Matches ExternalIDP
Credentials in HTTP Authorization header	basic	BASIC	header	header
Credentials in request body	body	POST	body	body

[Copilot]

Pull request overview

This PR adds a rover-ctl-side request patch to translate legacy YAML spec.authentication.m2m.clientAuthMethod values into the rover-server API’s spec.authentication.clientAuthMethod enum values, with accompanying handler tests.

Changes:

• Add PatchAuthentication to PatchRoverRequest to map basic/body to BASIC/POST.
• Introduce a mapping table for client auth method translation.
• Add Ginkgo tests for the authentication patch behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
rover-ctl/pkg/handlers/v0/rover.go	Adds PatchAuthentication hook and mapping logic for clientAuthMethod.
rover-ctl/pkg/handlers/v0/rover_test.go	Adds tests validating mapping and current drop-on-invalid behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[BjoernKarma]

PR Review: feat(rover): implement PatchAuthentication to map clientAuthMethod values

Summary

This PR adds a clientAuthMethod mapping pipeline across three layers: rover-ctl (YAML basic/body → API BASIC/POST), rover-server inbound mapper (API BASIC/POST → CRD header/body), and rover-server outbound mapper (CRD → API). A new Authentication field is added to the Rover CRD. The feature is documented as not yet enforced by the identity domain. The implementation is reasonable and well-tested, but there's a critical CI failure from a stale generated CRD, and the CRD contains enum values that don't match the Go types.

Technical

Check	Status	Notes
T1 Conventional commits	🟡	Most commits follow convention, but several merge commits and one truncated headline (feat(rover-ctl): implement PatchAuthentication to map clientAuthMetho…) reduce clarity. Not blocking.
T2 REUSE / SPDX compliance	✅	All modified source files already have SPDX headers. No new files created without headers.
T3 Build	✅	CI build passes for rover, rover-ctl, and rover-server.
T4 Lint	✅	Rover-CTL and rover-server static checks pass.
T5 Tests exist	✅	Comprehensive tests added: rover_test.go in rover-ctl, rover-server inbound/outbound mapper tests, and controller integration tests.
T6 Test coverage	✅	All rover-related test suites pass in CI.
T7 go mod tidy	✅	No go.mod/go.sum changes in the diff.
T8 Generated code	🔴	CI fails. The committed CRD YAML (rover.cp.ei.telekom.de_rovers.yaml) does not match what controller-gen produces from the Go types. The Go type defines TokenRequest with enum body;header, but the committed CRD has clientAuthMethod with enum NONE;POST;BASIC and default BASIC. Running make manifests generate in the rover module would produce different output.
T9 Error handling	✅	Functions silently return on invalid/missing data, which is appropriate for this optional mapping — no errors to propagate for absent fields.
T10 Security	✅	No secrets, no injection vectors. Mapping is data transformation only.
T11 API compatibility	🟡	New CRD field is optional with a pointer type — additive and backwards compatible. However the CRD as committed has wrong field names/enums (see T8).
T12 Dependencies	✅	No new dependencies added.

Business

Check	Status	Notes
B1 Acceptance criteria	✅	PR description defines a mapping table; the code implements it correctly at each layer.
B2 Requirements coverage	✅	Scoped to rover-ctl mapping only, as stated. No scope creep.
B3 Business logic correctness	✅	The mapping chain is correct: basic→BASIC→header, body→POST→body. This aligns with the existing ExternalIDP.TokenRequest values (body/header) used elsewhere in the codebase (gateway, api modules).
B4 Edge cases	✅	Three edge cases tested: (1) missing authentication field — no-op ✅; (2) invalid clientAuthMethod value — authentication left untouched ✅; (3) already in rover-server format (no m2m key) — left untouched ✅. One additional edge case to note: PatchAuthentication replaces the entire authentication map (dropping any other fields that might exist alongside m2m), which is acceptable since m2m.clientAuthMethod is the only field expected at this layer.
B5 Naming and domain language	✅	TokenRequest aligns with existing ExternalIDP.TokenRequest in gateway and api modules. RoverAuthentication/RoverM2MAuthentication follows the pattern of other rover types.
B6 Observability	🟡	No logging added for the mapping. Since this is a silent data transformation in a pre-request hook, logging isn't strictly necessary, but a V(1) debug log when a mapping occurs could help troubleshoot issues. Minor.

Findings

1. 🔴 CRD YAML out of sync with Go types — rover/config/crd/bases/rover.cp.ei.telekom.de_rovers.yaml
   The committed CRD defines the field as clientAuthMethod with enum values NONE, POST, BASIC and default BASIC. However, the Go type RoverM2MAuthentication defines the field as TokenRequest with enum values body, header and default header. CI confirms this: Rover / Static Checks fails because make manifests generate produces a diff. The CRD file needs to be regenerated by running make manifests generate in the rover/ directory.

2. 🟡 Snapshot timestamp changes appear unrelated — rover-server/internal/controller/__snapshots__/suite_controller_test.snap and rover-server/internal/mapper/status/__snapshots__/status_test.snap
   The snapshot changes alter timestamps from UTC (2025-09-18T08:39:44Z) to local timezone format (2025-09-18T10:39:44+02:00). These appear to be timezone-dependent snapshot updates that happened to be captured alongside this PR. They could make tests fragile across different CI environments.

3. 🟡 PatchAuthentication destructively replaces the authentication map — rover-ctl/pkg/handlers/v0/rover.go:115-117
   When the mapping succeeds, the function replaces spec["authentication"] entirely with map[string]any{"clientAuthMethod": apiValue}, discarding any other keys that might have been in the original authentication map (e.g., other future fields). Currently fine since only m2m.clientAuthMethod exists, but worth noting.

4. 🟡 Api Tests CI failure is unrelated — The Api / Tests & Coverage failure is a timeout waiting for kube-apiserver to stop in AfterSuite, not a test failure (94/94 tests passed). This is a flaky envtest teardown issue, not caused by this PR.

Verdict

REQUEST CHANGES

The CRD YAML is out of sync with the Go types (finding #1), which causes the Rover Static Checks CI to fail. Run make manifests generate in the rover/ directory and commit the regenerated CRD to fix this.

---

🤖 Generated with Claude Code

[ron96g]

Easy enough. Just a few questions