HYPERFLEET-1178 - test: add negative conformance tests for v1 adapter contract

[pnguyen44]

Summary

HYPERFLEET-1178

Adds negative conformance tests that verify the v1.0.0 API correctly rejects pre-migration
adapter behavior. These tests ensure partner teams get clear, early failures when their
adapters still use the old v0.2.0 contract instead of silent, surprising breakage. This
covers the "conformance (negative)" acceptance criterion from the ticket.

Changes

• Added three Tier0 negative conformance tests: POST to statuses returns 405, PUT with
  missing mandatory conditions returns 400, and Ready condition is absent on reconciled
  clusters
• Added PostClusterStatuses client method to send raw POST requests for negative
  status-code assertions
• Added test case design document covering all three negative conformance scenarios

Test plan

• make check passes cleanly
• Unit tests added/updated
• make test-all passes
• make lint passes
• Helm chart changes validated with make test-helm (if applicable)
• Deployed to a development cluster and verified (if Helm/config changes)
• E2E tests passed (if cross-component or major changes)

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign aredenba-rh for approval. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 719bd910-7a10-4c7e-8586-a386a3016671

📥 Commits

Reviewing files that changed from the base of the PR and between 4717af7 and 06b88c3.

📒 Files selected for processing (1)

• e2e/adapter/conformance_negative.go

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• openshift-hyperfleet/architecture (manual)
• openshift-hyperfleet/hyperfleet-api (manual)
• openshift-hyperfleet/hyperfleet-sentinel (manual)
• openshift-hyperfleet/hyperfleet-adapter (manual)
• openshift-hyperfleet/hyperfleet-broker (manual)

💤 Files with no reviewable changes (1)

• e2e/adapter/conformance_negative.go

---

📝 Walkthrough

Summary by CodeRabbit

• Tests

  • Added negative conformance coverage for adapter behavior, including invalid status requests and missing required conditions.
  • Verified that reconciled clusters do not expose a resource-level Ready condition, while Reconciled and LastKnownReconciled remain present.

• Documentation

  • Added test case specifications for the Adapter Contract v1.0.0 negative scenarios.

Walkthrough

Adds two new files to the repository. test-design/testcases/adapter-conformance-negative.md specifies three negative conformance tests for the Adapter Contract v1.0.0: POST to /clusters/{id}/statuses must return HTTP 405, PUT with only a Ready condition must return HTTP 400, and a reconciled cluster must not expose a resource-level Ready condition. e2e/adapter/conformance_negative.go implements the third case as a Ginkgo Tier0 E2E test using polling, deferred cleanup, and Gomega assertions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

---

Direct findings — no praise, no filler.

CWE-754 (Improper Check for Unusual or Exceptional Conditions): The deferred cleanup only logs a failure (GinkgoLogr.Error) but does not mark the test as failed. A leaked cluster in a shared environment is a real state pollution risk, not just a logging concern. Consider Expect(err).NotTo(HaveOccurred()) inside the cleanup closure or at minimum Fail(...) on error.

Spec/implementation gap (HTTP 405 and HTTP 400 cases): adapter-conformance-negative.md specifies two additional negative cases (POST → 405, PUT missing mandatory conditions → 400) with no corresponding E2E implementation. The markdown is a dead spec until those tests are written. Shipping an incomplete test matrix under a Tier0 label creates a false signal about coverage.

Hardcoded string constant conditionTypeReady = "Ready": If the condition type name is already declared as a typed constant elsewhere in the Adapter or API package, this is a duplicate that can drift silently. Verify against the canonical type definitions in the platform packages and import the constant rather than re-declaring it as a raw string.

No assertion on cluster creation success before polling: CreateCluster result is not checked for error before entering the poll loop. A failed creation will cause the poll to time out rather than fail fast, obscuring the root cause. Check the error immediately (CWE-391: Unchecked Error Condition).

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: negative conformance tests for the v1 adapter contract.
Description check	✅ Passed	The description is directly related to the changeset and matches the added negative conformance tests and test design.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Sec-02: Secrets In Log Output	✅ Passed	No log statements contain tokens, passwords, credentials, or secrets. Single Printf logs only clusterID and error value—neither is sensitive.
No Hardcoded Secrets	✅ Passed	No hardcoded secrets found in the added files; only non-sensitive test strings and docs. CWE-798 not triggered.
No Weak Cryptography	✅ Passed	Touched files add tests/docs and a model-field tweak; no crypto primitives, ECB, custom crypto, or secret comparisons found. No CWE-327/328 issue.
No Injection Vectors	✅ Passed	No CWE-89/78/79/502 vectors in the new e2e test; inputs are hardcoded/trusted, and the markdown testcase file is non-executable.
No Privileged Containers	✅ Passed	No privileged Kubernetes/OpenShift manifests or Helm templates were added. The only root USERs are in CI/test Dockerfiles, which are exempt from this check.
No Pii Or Sensitive Data In Logs	✅ Passed	No CWE-532 issue: added logs only warn on cleanup failure and print cluster ID/error; no PII, bodies, tokens, or credentialed hostnames found.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

✨ Simplify code

• Create PR with simplified code

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@e2e/adapter/conformance_negative.go`:
- Around line 39-41: The error returned by the CleanupTestCluster method is
being discarded in the DeferCleanup callbacks instead of being checked and
handled. Remove the underscore assignment that discards the error and instead
check the error result from h.CleanupTestCluster(ctx, clusterID). If an error
occurs during cleanup, handle it appropriately by logging it or failing the test
to prevent resource leaks and ensure cleanup failures are visible. This same
issue appears in three locations throughout the file where DeferCleanup is used
with CleanupTestCluster calls.
- Around line 17-20: The constant conformanceNegativeAdapter is hardcoded with
the literal value "conformance-test", which violates the coding guideline that
adapter names must come from runtime configuration. Remove the hardcoded
constant definition and replace all references to conformanceNegativeAdapter (at
lines 45-46 and 82-83 as well) with the actual runtime adapter names obtained
from h.Cfg.Adapters.Cluster or h.Cfg.Adapters.NodePool depending on the context
of use. This ensures the conformance tests validate against the correct adapter
names from the runtime configuration rather than a hardcoded literal.

🪄 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: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: a500fd06-a944-466f-ba7e-7c416869309c

📥 Commits

Reviewing files that changed from the base of the PR and between dcc6da0 and 7777381.

📒 Files selected for processing (3)

• e2e/adapter/conformance_negative.go
• pkg/client/cluster.go
• test-design/testcases/adapter-conformance-negative.md

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• openshift-hyperfleet/architecture (manual)
• openshift-hyperfleet/hyperfleet-api (manual)
• openshift-hyperfleet/hyperfleet-sentinel (manual)
• openshift-hyperfleet/hyperfleet-adapter (manual)
• openshift-hyperfleet/hyperfleet-broker (manual)

[pnguyen44]

/retest