Repository navigation
test: establish OSAC SP contract coverage - #65
Conversation
Signed-off-by: Thomas Stetson <tstetson@redhat.com>
…E tests - Add `scripts/deploy-osac-backend.sh` and Makefile targets to deploy/teardown the backend on OCP, plus macOS `port-forward-osac` helpers. - Integrate `--deploy-osac-backend` into `deploy-dcm.sh`: auto-detects backend credentials, injects TLS and Darwin port-forward compose overlays, tears down with `--tear-down`, and patches profile-gated `depends_on` for podman-compose compatibility. - Activate the `environment-agent` profile so the merged agent service starts under podman-compose, and fix the OSAC SP registration URL path. - Harden OSAC SP E2E tests: exact provider registration assertions, AEP-132 `results` wrapper, stricter CloudEvent envelope checks, 502 skip handling, RFC 9457 "Bad Request" title, and idempotent create returns original resource. - Document setup in CLAUDE.md and Cursor prompts; ignore `deploy/` artifacts. Signed-off-by: Thomas Stetson <tstetson@redhat.com>
…t skipped coverage - Update the CLI provider test suite to target `/agents`, rename provider references to agent, and document the CLI + assertion blockers (FLPATH-4895) keeping the suite pending (`PDescribe`). - Implement the previously pending RFC 7807 contract test for 422/406 provider-error responses by stopping all providers. - Implement sequential failover orphan cleanup and deferred-deletion queue checks, tracking superseded resource IDs across rehydrations. - Update `restartSPRM()` to restart the merged `environment-agent` container. - Annotate the pending OSAC `max_page_size > 100` test with the known backend gap and Jira references (FLPATH-4459/4463). Signed-off-by: Thomas Stetson <tstetson@redhat.com>
…y for Phase 2 - Strengthen cluster/VM API assertions: RFC 9457 problem details for missing and invalid fields, explicit non-null `results` array checks, and table-driven boundary tests for storage and query parameters. - Align health/status tests with observed SP behavior: detail required when degraded, accept "PROGRESSING" cluster status, and expect CloudEvent source prefix `dcm/providers/osac-sp`. - Capture `spec` in cluster/VM structs to enable persisted-field round-trip checks. - Extend `scripts/deploy-osac-backend.sh` with optional Phase 2 components (osac-operator, BMFO, aap-mock, fixtures), upstream manifest pinning, `--skip-phase2`, and safer teardown of cluster-scoped/APIService resources. - Document test-credential provenance and gRPC TLS CA-cert troubleshooting in CLAUDE.md and Cursor prompts. Signed-off-by: Thomas Stetson <tstetson@redhat.com>
…p-contract # Conflicts: # tests/run-e2e.sh
PR Summary by QodoEstablish OSAC service-provider contract and backend E2E coverage
AI Description
Diagram
High-Level Assessment
Files changed (38)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| log "Enabling HTTP/2 on the OCP ingress controller" | ||
| oc get ingresses.config.openshift.io cluster \ | ||
| -o jsonpath='{.metadata.annotations.ingress\.operator\.openshift\.io/default-enable-http2}' \ | ||
| 2>/dev/null > "${DEPLOY_DIR}/osac-previous-http2" |
There was a problem hiding this comment.
[P1] Create the output directory before this redirected write. The deploy/ directory is ignored by git and is not created before the shell opens this redirect, so on a fresh checkout the command fails before oc runs and set -e exits before deployment. Add mkdir -p before the first write.
| "name": "OSAC administrator", | ||
| "enabled": true, | ||
| "clientAuthenticatorType": "client-secret", | ||
| "secret": "tierb-osac-admin-secret", |
There was a problem hiding this comment.
[P1] This reuses the static osac-admin secret that the peer repo's Tier B README scopes to throwaway Kind CI and says not to reuse for real deployments: https://github.com/dcm-project/osac-service-provider/blob/main/test/e2e/tierb-config/README.md#L11-L14. This PR provisions that secret in its OCP path and creates a Keycloak Route. If the target OCP is persistent or reachable beyond an isolated test boundary, the committed credential is reusable against the routed service. Please enforce the disposable/isolated boundary or generate per-deployment credentials before exposing the route.
| | oc apply -f - | ||
| oc create clusterrolebinding osac-fulfillment-hub-reader --clusterrole=osac-fulfillment-hub-reader \ | ||
| --serviceaccount=default:osac-fulfillment-hub --dry-run=client -o yaml | oc apply -f - | ||
| oc label clusterrole osac-fulfillment-hub-reader "${PHASE2_PART_OF_KEY}=${PHASE2_PART_OF_LABEL}" --overwrite |
There was a problem hiding this comment.
[P2] Check ownership before applying and relabeling these fixed-name cluster RBAC resources. On a same-name unowned collision, this can replace existing ClusterRole rules or binding subjects and then mark the objects as owned; teardown may delete the preexisting resources. Refuse unowned collisions or use deployment-specific names.
| oc label clusterrole osac-fulfillment-hub-reader "${PHASE2_PART_OF_KEY}=${PHASE2_PART_OF_LABEL}" --overwrite | ||
| oc label clusterrolebinding osac-fulfillment-hub-reader "${PHASE2_PART_OF_KEY}=${PHASE2_PART_OF_LABEL}" --overwrite | ||
| local hub_token hub_ca | ||
| hub_token="$(oc create token osac-fulfillment-hub -n default --duration=1h)" |
There was a problem hiding this comment.
[P2] This requests a 1h service-account token and persists it in the Hub kubeconfig. The AlreadyExists branch below treats the existing Hub as success without refreshing its spec, so a later setup can retain an expired token. The requested duration is server-controlled; update the existing Hub with a fresh credential or add a renewal path.
| // KNOWN BACKEND GAP: fulfillment-service does not enforce the AEP-132 constraint | ||
| // that max_page_size > 100 must be rejected with 400. It returns 200 instead. | ||
| // Tracked under FLPATH-4459 (epic) / FLPATH-4463 (story). | ||
| PIt("rejects max_page_size > 100 with 400 (AEP-132)", func() { |
There was a problem hiding this comment.
[P2] PIt marks this case pending, so Ginkgo does not execute the assertion. The PR description says the max_page_size > 100 assertion is enabled and reproduces the issue, but this test currently provides no CI evidence for that claim. Please align the description with the pending status or add executable coverage that reflects the tracked backend gap.
|
|
||
| var listResp osacClusterListResponse | ||
| decodeJSON(resp, &listResp) | ||
| Expect(listResp.Results).To(ContainElement(HaveField("ID", clusterID)), |
There was a problem hiding this comment.
[P2] This assertion checks only the first Results page; the post-delete absence check does the same. The peer implementation supports page_token/next_page_token pagination (https://github.com/dcm-project/osac-service-provider/blob/main/internal/cluster/list.go#L25-L82), so a created cluster can be on a later page and a lingering deleted cluster can be missed. Follow next_page_token until exhausted or constrain the fixture to one page.
|
@gabriel-farache I think I covered everything that I can, plus some missing Qodo stuff. Keep me honest when you have time. |
|
Worth noting: The rehydration-specific pagination and provider-stop findings remain intentionally out of scope for this PR stack. There's a broader follow-up to build out a production-like environment and expand testing across all SPs and whatever other integrations. |
| // drains messages until a matching VM ID is found. | ||
| // ------------------------------------------------------------------ # | ||
|
|
||
| Context("VM status events", Label("cluster"), func() { |
There was a problem hiding this comment.
This Context is Label("cluster") but it covers VM NATS events. Running with --label-filter=cluster will execute these VM specs, and --label-filter=vm will miss them. Change to Label("vm").
| }) | ||
|
|
||
| It("deleted VM no longer appears in the list", func() { | ||
| resp, err := doOsacVMRequest(http.MethodGet, "/vms", "") |
There was a problem hiding this comment.
The cluster suite uses paginated listAllOsacClusters() but this VM check reads one page. osacVMListResponse also lacks NextPageToken. A listAllOsacVMs() helper would keep the two suites consistent.
| // Use a UUID-format ID so OSAC does not reject it as malformed. | ||
| resp, err := doOsacClusterRequest(http.MethodDelete, | ||
| "/clusters/00000000-e2e0-4000-8000-delete0cl0000", "") | ||
| Expect(err).NotTo(HaveOccurred()) |
There was a problem hiding this comment.
delete0cl0000 has non-hex chars (l, t), same for delete0vm0000 (v, m). These duplicate the malformed-ID tests instead of exercising well-formed nonexistent delete. Use real hex, e.g. 00000000-e2e0-4000-8000-de1e7ec10000.
| # provisioning OpenShift clusters (serviceType: cluster) and VMs (serviceType: vm) | ||
| # via OSAC's gRPC fulfillment API. | ||
| # | ||
| # Requires: OSAC dev/MOC credentials set in the shell or deploy/.env: |
There was a problem hiding this comment.
This says credentials can be in "the shell or deploy/.env" but validate_osac_provider only sources deploy/osac-backend.env. Remove the deploy/.env mention or note it only applies at compose runtime.
| }) | ||
|
|
||
| It("coerces max_page_size > 100 to the contract maximum", func() { | ||
| resp, err := doOsacVMRequest(http.MethodGet, "/vms?max_page_size=101", "") |
There was a problem hiding this comment.
The cluster suite has a PIt for max_page_size > 100 but the VM suite has no matching pending case. Adding one keeps both suites symmetric.
Summary
Establishes the OSAC Service Provider contract-test foundation and real fulfillment-service E2E path.
Scope
Shared Harness Impact
This branch includes shared environment-agent/SPRM migration compatibility and rehydration contract/failover test updates required by the current E2E architecture. These tests are not OSAC-specific, but are included because the OSAC SP integration depends on the same provider-registration, restart, and E2E helper paths.
Validation
max_page_size > 100assertion. The SP currently returns HTTP 200 instead of the required HTTP 400.Review Boundary
This PR validates the OSAC SP contract and its real fulfillment-service integration. It does not claim Agent registration, BMFO/BMI/BMH allocation, Ironic/BMC provisioning, AAP playbook correctness, HostedCluster/NodePool readiness, VM networking, or physical infrastructure lifecycle.
Stack
Both dependent PRs remain draft.