[testetson22]

Adds the OCP-backed OSAC Tier B++ dispatch boundary and opt-in real-AAP integration on top of the merged OSAC SP contract foundation.

Scope

• Adds the tier-b-dispatch suite with exact SP-to-ClusterOrder identity correlation.
• Validates translated template identity, ClusterOrder conditions, provisioning job history, SP status, and NATS correlation.
• Adds mock versus real AAP deployment modes.
• Adds disposable AAP Controller/Gateway setup and subscription-manifest activation.
• Adds OSAC backend Phase 2 operator/BMFO/AAP integration and cleanup prerequisites.
• Fixes fulfillment Route/JWKS/TLS handling.
• Adds macOS Keycloak/gRPC port-forward automation.
• Adds ownership-safe namespace, CRD, RBAC, APIService, ClusterIssuer, and teardown behavior.
• Uses a least-privilege Hub ServiceAccount credential instead of persisting the caller’s kubeconfig.
• Requires explicit --allow-tierb-credentials acknowledgement for committed test-only credentials.
• Adds a focused control-plane → environment-agent → external OSAC provider smoke-test target.
• Keeps Tier B++ coverage separate from Tier C infrastructure provisioning.

Test Targets

make test-osac-dispatch
make test-osac-external

Remaining Stack

PR #67: deterministic OSAC SP simulator and state/error coverage.

[qodo-code-review]

PR Summary by Qodo

Add OSAC Tier B++ dispatch tests and optional real-AAP backend

🧪 Tests ✨ Enhancement 🐞 Bug fix 📝 Documentation 🕐 40+ Minutes

AI Description

• Add OCP-backed tests correlating OSAC SP requests with ClusterOrders, conditions, and NATS events.
• Add optional disposable AAP deployment and an external-provider smoke target, separate from Tier C
 provisioning.
• Harden backend TLS, identity setup, macOS forwarding, and cleanup prerequisites for repeatable
 runs.

Diagram

sequenceDiagram
    participant Test as E2E tests
    participant Agent as Environment agent
    participant SP as OSAC SP
    participant Backend as Fulfillment service
    participant Order as ClusterOrder
    participant Operator as OSAC operator
    participant AAP as AAP backend
    participant NATS as NATS
    Test->>Agent: Optional external smoke create
    Agent->>SP: Forward provider request
    Test->>SP: Tier B++ cluster create
    SP->>Backend: Submit cluster request
    Backend->>Order: Create linked order
    Order-->>Operator: Reconcile order
    Operator->>AAP: Optional real-mode no-op job
    SP-->>NATS: Publish cluster event
    NATS-->>Test: Correlate cluster ID
    Test->>Order: Inspect label and conditions

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use a pre-provisioned external AAP

• ➕ Avoids installing and maintaining a disposable AAP stack on the OCP test cluster.
• ➕ Allows testing against an independently managed AAP deployment.
• ➖ Makes the real-AAP path less reproducible and dependent on external credentials and availability.
• ➖ Does not validate this PR's in-cluster Gateway setup.

Recommendation: Keep the default mock mode and the opt-in disposable real-AAP mode: they serve different test costs and preserve an isolated dispatch boundary. A pre-provisioned AAP is useful where cluster capacity is constrained, but should complement rather than replace the self-contained path. Treat successful no-op dispatch as Tier B++ evidence, not Tier C provisioning.

Files changed (18) +1728 / -64

Enhancement (4) +702 / -30

deploy-dcm.shWire OSAC AAP modes, CA checks, and macOS forwarding +90/-9

Wire OSAC AAP modes, CA checks, and macOS forwarding

• Passes the selected AAP mode to backend deployment and ensures macOS port-forwards before provider validation. Validates and normalizes the OSAC CA path before adding the Compose TLS overlay.

scripts/deploy-dcm.sh

deploy-osac-aap.shDeploy disposable Gateway-backed AAP integration +322/-0

Deploy disposable Gateway-backed AAP integration

• Installs AAP operator resources, Controller and Gateway, activates a subscription manifest, and prepares no-op create/delete job templates. Configures the OSAC operator to use the Gateway-backed API and provides teardown for the AAP namespace and token secret.

scripts/deploy-osac-aap.sh

deploy-osac-backend.shPrepare the Phase 2 backend for correlated dispatch +249/-20

Prepare the Phase 2 backend for correlated dispatch

• Adds real-AAP mode integration, Keycloak initialization, external-IP CRDs, and Agent read permissions. Routes external API traffic through fulfillment-api, normalizes token issuers, validates extracted CA material, and refreshes the registered Hub credential.

scripts/deploy-osac-backend.sh

osac-port-forward.shEnsure required macOS backend port-forwards +41/-1

Ensure required macOS backend port-forwards

• Adds an idempotent ensure mode that reuses listening forwards and waits for both required ports after launchd registration.

scripts/osac-port-forward.sh

Bug fix (2) +90 / -0

normalize-fulfillment-token-issuer.shAlign fulfillment token issuers with the HTTPS Route +54/-0

Align fulfillment token issuers with the HTTPS Route

• Rewrites and verifies token-issuer arguments for the gRPC server and console proxy in rendered manifests, replacing the chart's internal-port URL.

scripts/normalize-fulfillment-token-issuer.sh

osac-ca.shShare strict OSAC CA certificate validation +36/-0

Share strict OSAC CA certificate validation

• Rejects missing, unreadable, empty, malformed, or non-CA certificate files before they are used by deployment or Compose.

scripts/osac-ca.sh

Documentation (3) +235 / -29

deploy-osac-backend.mdDocument turnkey backend and real-AAP workflows +35/-14

Document turnkey backend and real-AAP workflows

• Adds the combined deployment command, macOS forwarding sequence, AAP mode behavior, and CA validation requirements. Clarifies that Phase 2 dispatch does not provide Tier C provisioning.

.cursor/prompts/deploy-osac-backend.md

CLAUDE.mdExplain Tier B++ scope and deployment prerequisites +28/-7

Explain Tier B++ scope and deployment prerequisites

• Documents the combined macOS workflow, optional real AAP, CA checks, and the observed boundary before Agent-backed provisioning.

CLAUDE.md

OSAC-tier-c-test-plan.mdSeparate observed Tier B++ dispatch from Tier C goals +172/-8

Separate observed Tier B++ dispatch from Tier C goals

• Records dispatch checkpoints, observed Phase 2 results, topology considerations, and gates for Agent allocation and real provisioning. Refines later Tier C assertions and product-blocker notes.

test-plans/OSAC-tier-c-test-plan.md

Other (9) +701 / -5

validate-scripts.yamlRun OSAC CA and deployment-wiring checks in CI +27/-0

Run OSAC CA and deployment-wiring checks in CI

• Adds path triggers and two validation jobs for certificate checks and turnkey deployment wiring.

.github/workflows/validate-scripts.yaml

MakefileAdd focused OSAC dispatch and external smoke targets +7/-1

Add focused OSAC dispatch and external smoke targets

• Adds Ginkgo label-filtered targets for Tier B++ dispatch and the control-plane-to-external-provider smoke path.

Makefile

compose-osac-sp-tls.yamlRequire an explicit OSAC CA bind mount +2/-3

Require an explicit OSAC CA bind mount

• Removes the fallback CA path so Compose fails when OSAC_CA_CERT_FILE is unset.

tests/compose-osac-sp-tls.yaml

sp_osac_dispatch_test.goAssert exact Tier B++ dispatch identity and progress +384/-0

Assert exact Tier B++ dispatch identity and progress

• Adds ordered tests for SP create/GET/list and NATS identity, a newly linked ClusterOrder and its template ID, and operator conditions. In real-AAP mode, also waits for successful provisioning-job history; cleanup deletes the created SP cluster.

tests/e2e/sp_osac_dispatch_test.go

sp_osac_external_path_test.goSmoke-test external OSAC provider forwarding +79/-0

Smoke-test external OSAC provider forwarding

• Creates a pre-seeded OSAC catalog item instance through the control plane, checks its resource ID and exposed status, and deletes the instance afterward.

tests/e2e/sp_osac_external_path_test.go

keycloak.yamlUse a Keycloak version with organization support +2/-1

Use a Keycloak version with organization support

• Moves the test backend to Keycloak 26.6.0 for organization-group behavior required by fulfillment-service.

tests/osac-backend/keycloak.yaml

realm.jsonSeed the tenant identity-provider manager role +9/-0

Seed the tenant identity-provider manager role

• Adds the realm role needed by the fulfillment-service tenant identity workflow.

tests/osac-backend/realm.json

test-osac-ca-validation.shTest acceptance and rejection of OSAC CA files +57/-0

Test acceptance and rejection of OSAC CA files

• Generates CA and leaf certificates and checks validation of valid, absent, empty, malformed, directory, and non-CA inputs.

tests/test-osac-ca-validation.sh

test-osac-deploy-wiring.shCheck issuer, port-forward, and AAP deploy wiring +134/-0

Check issuer, port-forward, and AAP deploy wiring

• Checks issuer rewriting and preservation of unrelated deployments, port-forward reuse, and deployment-stage ordering. Also verifies the Route target, mode toggle, and cleanup prerequisites are wired into the scripts.

tests/test-osac-deploy-wiring.sh

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (8) 📘 Rule violations (2) 📜 Skill insights (0)

1. Real-AAP platform ignores its configuration ✓ Resolved

Description

In deploy-osac-aap.sh, the AnsibleAutomationPlatform manifest puts admin_password_secret,
ingress_type, route_tls_termination_mechanism, controller, api, metrics, and database
under metadata: and has no spec:. In real mode, the resource may be rejected or lose that
configuration, preventing the Gateway wait from completing; if it reconciles with defaults, it does
not link the osac-aap controller or use the generated admin password, so the later Gateway token
request with admin:${admin_password} fails.

Code

scripts/deploy-osac-aap.sh[R190-196]

+metadata:
+  name: osac-aap-platform
+  namespace: ${AAP_NAMESPACE}
+  admin_password_secret: aap-controller-admin-password
+  ingress_type: Route
+  route_tls_termination_mechanism: Edge
+  controller: {name: osac-aap, disabled: false}

Evidence

Lines 190–201 place every platform setting under metadata: without a spec: key. The script then
waits for the Gateway deployment and, at lines 225–229, posts a token request using the controller
admin password, which the platform only uses when spec.admin_password_secret is set.

scripts/deploy-osac-aap.sh[187-229]
scripts/deploy-osac-aap.sh[187-205]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The AnsibleAutomationPlatform manifest places its configuration under `metadata` instead of `spec`, leaving the resource without the intended platform settings.

## Fix Focus Areas
- scripts/deploy-osac-aap.sh[187-202]

## Recommended Fix
Keep only `name` and `namespace` under `metadata:`. Add a top-level `spec:` after them and move `admin_password_secret`, `ingress_type`, `route_tls_termination_mechanism`, `controller`, `api`, `metrics`, and `database` beneath it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Teardown can delete another AAP namespace 🐞 Bug ☼ Reliability

Description

deploy-osac-aap.sh --tear-down deletes AAP_NAMESPACE without checking whether this deployment
created or owns it. If that namespace already existed when real mode was deployed, teardown removes
its existing resources too.

Code

scripts/deploy-osac-aap.sh[R45-48]

+if [[ "${TEAR_DOWN}" == true ]]; then
+    log "Removing disposable AAP namespace ${AAP_NAMESPACE}"
+    oc delete namespace "${AAP_NAMESPACE}" --ignore-not-found --wait=true
+    oc delete secret osac-aap-gateway-token -n "${OSAC_NAMESPACE}" --ignore-not-found

Evidence

Deployment accepts an existing namespace without recording ownership, whereas teardown
unconditionally deletes it.

scripts/deploy-osac-aap.sh[45-56]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Real-AAP teardown deletes a namespace that may predate this deployment.
## Fix Focus Areas
- scripts/deploy-osac-aap.sh[45-56]
## Recommended Fix
Mark namespaces created by this script with an ownership label and delete a namespace only after verifying that ownership; otherwise remove only individually owned resources.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

3. The external smoke test stops at create 🐞 Bug ≡ Correctness

Description

sp_osac_external_path_test.go requires resource_ids directly in the catalog-instance create
response, although the current response uses run_id and requires service-type-instance discovery.
Against that response, the assertion fails before status verification and before the cleanup hook is
registered.

Code

tests/e2e/sp_osac_external_path_test.go[R41-47]

+		var instance struct {
+			UID         string   `json:"uid"`
+			ResourceIDs []string `json:"resource_ids"`
+		}
+		Expect(json.Unmarshal(body, &instance)).To(Succeed())
+		Expect(instance.UID).NotTo(BeEmpty())
+		Expect(instance.ResourceIDs).NotTo(BeEmpty(), "external placement must return a resource ID")

Evidence

The existing resolver documents the removed response field and handles run_id with STI discovery;
existing create tests use it rather than a top-level resource_ids field.

tests/e2e/sp_osac_external_path_test.go[35-54]
tests/e2e/api_helpers_test.go[183-224]
tests/e2e/api_instances_test.go[91-112]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The external smoke test expects a resource_ids field removed from the current create response.
## Fix Focus Areas
- tests/e2e/sp_osac_external_path_test.go[35-56]
- tests/e2e/api_helpers_test.go[183-224]
## Recommended Fix
Snapshot service-type-instance IDs before POST, register cleanup as soon as the instance UID is known, and use `resolveResourceIDAfterCreate` to obtain the placement ID.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

View high (5)

4. External-provider cleanup can leak its response body 📘 Rule violation ☼ Reliability

Description

The external-provider test calls doRequest for the create operation and passes the response
directly to readBody without first deferring resp.Body.Close() after the error assertion. If the
assertion or body processing exits before readBody closes the body, the response remains open and
repeated polling or parallel runs can exhaust HTTP connections.

Code

tests/e2e/sp_osac_external_path_test.go[R35-37]

+		resp, err := doRequest(http.MethodPost, "/catalog-item-instances", string(payload))
+		Expect(err).NotTo(HaveOccurred())
+		body := readBody(resp)

Evidence

The changed test performs doRequest, checks the error, and immediately calls readBody(resp)
without the required adjacent response-body close.

Rule 3176575: Always close HTTP response bodies after doRequest in E2E tests
tests/e2e/sp_osac_external_path_test.go[35-37]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new external-provider create request does not close its HTTP response body immediately after asserting that the request succeeded.

## Fix Focus Areas
- tests/e2e/sp_osac_external_path_test.go[35-37]

## Recommended Fix
Add `defer resp.Body.Close()` immediately after `Expect(err).NotTo(HaveOccurred())`, before reading or asserting the response body.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

5. The certificate helper lacks strict failure handling 📘 Rule violation ☼ Reliability

Description

osac-ca.sh defines validate_osac_ca_cert without enabling set -euo pipefail near the top of
the sourced shell file. Any future command added to this helper can continue after an unset
variable, pipeline failure, or intermediate command error when it is executed outside the
already-strict callers.

Code

scripts/osac-ca.sh[R1-3]

+#!/usr/bin/env bash
+
+validate_osac_ca_cert() {

Evidence

The new shell helper starts with a Bash shebang and function definition but has no `set -euo
pipefail`, violating the required strict-mode rule.

Rule 3176433: Shell scripts must start with set -euo pipefail
scripts/osac-ca.sh[1-3]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new shell helper does not begin with the required `set -euo pipefail` safeguard.

## Fix Focus Areas
- scripts/osac-ca.sh[1-3]

## Recommended Fix
Add `set -euo pipefail` immediately after the shell declaration or otherwise ensure the sourced helper explicitly enforces strict handling without weakening the calling scripts.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

6. Provider requests carry control-plane credentials ✓ Resolved

Description

The new dispatch suite calls doOsacClusterRequest, which delegates to doOsacRequest and sends
through the shared authenticated httpClient rather than an unauthenticated provider client. Every
direct OSAC service-provider request in the new suite therefore inherits the control-plane
bearer-token transport whenever authentication is enabled.

Code

tests/e2e/sp_osac_dispatch_test.go[R55-56]

+		resp, err := doOsacClusterRequest(http.MethodPost,
+			fmt.Sprintf("/clusters?id=%s", clusterName), osacClusterPayload(clusterName))

Evidence

The changed test invokes the direct OSAC request helper. That helper delegates to httpClient, and
client configuration replaces httpClient with an authTransport when authentication is enabled.

Rule 3639466: Direct service-provider E2E requests must use an unauthenticated client
tests/e2e/sp_osac_dispatch_test.go[55-56]
tests/e2e/sp_osac_helpers_test.go[264-283]
tests/e2e/api_auth_helpers_test.go[104-119]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new direct OSAC provider requests use a helper backed by the shared authenticated control-plane HTTP client.

## Fix Focus Areas
- tests/e2e/sp_osac_dispatch_test.go[55-56]
- tests/e2e/sp_osac_helpers_test.go[264-283]
- tests/e2e/api_auth_helpers_test.go[94-119]

## Recommended Fix
Introduce or use a dedicated unauthenticated client for direct OSAC service-provider calls, while retaining the authenticated client only for control-plane requests.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

7. Podman networking CI job loses its steps ✓ Resolved

Description

The new validate-osac-ca: job header is inserted between timeout-minutes: 5 and steps: of
validate-podman-compose-networking. That job is left with no steps, which makes the workflow
invalid, and the podman install/networking steps now run under the OSAC CA job with a 2-minute
timeout instead of 5.

Code

.github/workflows/validate-scripts.yaml[R169-172]

+  validate-osac-ca:
+    name: OSAC CA validation
+    runs-on: ubuntu-latest
+    timeout-minutes: 2

Evidence

In the file, validate-podman-compose-networking ends at timeout-minutes: 5 and the next key is
validate-osac-ca:. The steps: block that follows (podman install, bridge-network check, then the
CA validation step) belongs to the new job, so the original job has no steps.

.github/workflows/validate-scripts.yaml[165-210]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `validate-osac-ca` job header was inserted inside `validate-podman-compose-networking`, which takes that job's `steps` away from it.

## Fix Focus Areas
- .github/workflows/validate-scripts.yaml[165-210]

## Recommended Fix
Move the `validate-osac-ca:` header (name/runs-on/timeout-minutes) to after the podman networking steps. Give it its own `steps:` containing `actions/checkout@v6` and `bash tests/test-osac-ca-validation.sh`, and leave the podman steps under `validate-podman-compose-networking` with its 5-minute timeout.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. Missing placements can pass the smoke test 🐞 Bug ≡ Correctness

Description

readStatus converts every non-200 service-type-instance response into a non-empty http-NNN
status. The test's non-empty assertion and subsequent check against only FAILED both accept
http-404 or http-500, so a missing or broken placement can satisfy the smoke check.

Code

tests/e2e/sp_osac_external_path_test.go[R62-65]

+			defer statusResp.Body.Close()
+			if statusResp.StatusCode != http.StatusOK {
+				return fmt.Sprintf("http-%d", statusResp.StatusCode)
+			}

Evidence

The polling assertions accept any non-empty string except FAILED, and the reader constructs such
strings for HTTP errors.

tests/e2e/sp_osac_external_path_test.go[57-77]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
HTTP errors are turned into statuses that satisfy the smoke test assertions.
## Fix Focus Areas
- tests/e2e/sp_osac_external_path_test.go[57-77]
## Recommended Fix
Return an error or empty result for non-200 responses, require an HTTP 200 with a recognized placement status, and make the consistency check fail on request or decode errors.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

9. AAP teardown may run against the wrong cluster ✓ Resolved

Description

In the --tear-down path, deploy-dcm.sh calls deploy-osac-aap.sh --tear-down without passing
the resolved DCM_KUBECONFIG, unlike the adjacent backend teardown. When --kubeconfig or
auto-detection selects a cluster different from the ambient oc context, AAP namespace and
gateway-token secret deletion targets the ambient cluster or fails.

Code

scripts/deploy-dcm.sh[R1261-1264]

+        if [[ "${OSAC_AAP_MODE}" == "real" ]]; then
+            OSAC_BACKEND_NAMESPACE="${OSAC_BACKEND_NAMESPACE:-osac-test-backend}" \
+                bash "${REPO_ROOT}/scripts/deploy-osac-aap.sh" --tear-down
+        fi

Evidence

Line 1260 resolves DCM_KUBECONFIG, and line 1265 explicitly passes it to backend teardown, but the
AAP teardown call at lines 1262–1263 does not. The AAP helper runs oc delete namespace using its
inherited kubeconfig, so it does not necessarily operate on the selected cluster.

scripts/deploy-dcm.sh[1258-1267]
scripts/deploy-osac-aap.sh[45-50]
scripts/deploy-dcm.sh[611-622]
scripts/deploy-dcm.sh[1258-1265]
scripts/deploy-osac-aap.sh[45-48]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
AAP teardown does not receive deploy-dcm's resolved kubeconfig and can use the ambient `oc` context instead.

## Fix Focus Areas
- scripts/deploy-dcm.sh[1258-1265]

## Recommended Fix
Prefix the `deploy-osac-aap.sh --tear-down` invocation with `KUBECONFIG="${DCM_KUBECONFIG}"`, matching the adjacent backend teardown.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

10. The regular OSAC target runs opt-in tests 🐞 Bug ≡ Correctness

Description

test-osac-dispatch adds a distinct label filter, but the new dispatch suite still carries the
osac label selected by the existing test-osac-sp target. When a template ID and cluster access
are configured for regular OSAC tests, that target also runs the OCP-backed dispatch assertions
rather than keeping them opt-in.

Code

tests/e2e/sp_osac_dispatch_test.go[22]

+var _ = Describe("OSAC SP — Tier B++ dispatch discovery", Label("sp", "osac", "tier-b-dispatch", "cluster"), Ordered, func() {

Evidence

The regular Makefile target filters on osac, which is also present on the new suite; its setup
proceeds when the template and Kubernetes prerequisites are available.

Makefile[54-61]
tests/e2e/sp_osac_dispatch_test.go[22-42]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ordinary OSAC label filter includes the new opt-in dispatch suite.
## Fix Focus Areas
- Makefile[54-61]
- tests/e2e/sp_osac_dispatch_test.go[22-22]
## Recommended Fix
Change the regular target to exclude `tier-b-dispatch` and `external-provider`, while retaining explicit filters for the dedicated targets.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

11. Custom AAP namespace breaks the operator install ✓ Resolved

Description

The quoted OperatorGroup/Subscription heredoc hardcodes osac-aap-test instead of interpolating
AAP_NAMESPACE, and the CSV wait selector uses the same literal namespace. When
OSAC_AAP_NAMESPACE differs from the default, the script creates and waits in the requested
namespace but targets operator installation elsewhere, so the CSV wait times out while other
resources use ${AAP_NAMESPACE}.

Code

scripts/deploy-osac-aap.sh[R58-66]

+oc apply -f - <<'YAML'
+apiVersion: operators.coreos.com/v1
+kind: OperatorGroup
+metadata:
+  name: osac-aap-test
+  namespace: osac-aap-test
+spec:
+  targetNamespaces:
+    - osac-aap-test

Evidence

Line 8 makes AAP_NAMESPACE configurable, and the usage text documents the override. Although the
script creates the requested namespace, lines 58–79 apply operator resources containing the literal
osac-aap-test, and line 83 uses that literal in the CSV label selector.

scripts/deploy-osac-aap.sh[8-8]
scripts/deploy-osac-aap.sh[58-84]
scripts/deploy-osac-aap.sh[54-84]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The OperatorGroup, Subscription, and CSV label selector hardcode `osac-aap-test` even though `OSAC_AAP_NAMESPACE` can be overridden, causing a custom namespace to be ignored during operator installation and CSV selection.

## Fix Focus Areas
- scripts/deploy-osac-aap.sh[58-84]

## Recommended Fix
Change the quoted heredoc to `<<YAML` and replace the hardcoded namespace in the OperatorGroup, Subscription, and target namespace with `${AAP_NAMESPACE}`. Use `-l operators.coreos.com/ansible-automation-platform-operator.${AAP_NAMESPACE}` for the CSV wait, then test deployment with a non-default namespace.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View medium (4)

12. The sourced helper uses the wrong shell declaration ✓ Resolved

Description

osac-ca.sh starts with #!/usr/bin/env bash even though it is sourced by deploy-dcm.sh and
deploy-osac-backend.sh as a library. Shell-checking and maintainers therefore cannot identify it
as a sourced Bash library using the required # shellcheck shell=bash declaration.

Code

scripts/osac-ca.sh[R1-3]

+#!/usr/bin/env bash
+
+validate_osac_ca_cert() {

Evidence

Both changed deploy scripts source scripts/osac-ca.sh, while the helper itself begins with an
executable shebang rather than the sourced-library declaration required by the checklist.

Rule 3176515: Bash scripts must use #!/usr/bin/env bash shebang
scripts/osac-ca.sh[1-3]
scripts/deploy-dcm.sh[54-57]
scripts/deploy-osac-backend.sh[42-46]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new helper is sourced rather than executed, but it uses an executable Bash shebang instead of the required sourced-library declaration.

## Fix Focus Areas
- scripts/osac-ca.sh[1-3]

## Recommended Fix
Replace the executable shebang with `# shellcheck shell=bash` and keep the helper suitable for sourcing by the strict-mode caller scripts.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

13. Failed deletes leave dispatch test data 🐞 Bug ☼ Reliability

Description

AfterAll calls deleteTestOsacCluster without checking the DELETE result or waiting for the
linked resource to disappear. Because that helper ignores request errors and HTTP status codes, a
passing dispatch run can leave its cluster and ClusterOrder behind when deletion fails.

Code

tests/e2e/sp_osac_dispatch_test.go[R75-77]

+		if created {
+			deleteTestOsacCluster(clusterID)
+		}

Evidence

The teardown discards the helper result, and the helper itself closes a successful response without
inspecting its status.

tests/e2e/sp_osac_dispatch_test.go[68-77]
tests/e2e/sp_osac_helpers_test.go[368-379]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The dispatch suite silently ignores failed deletion of the resource it creates.
## Fix Focus Areas
- tests/e2e/sp_osac_dispatch_test.go[68-77]
- tests/e2e/sp_osac_helpers_test.go[368-379]
## Recommended Fix
Use a cleanup operation that checks DELETE errors and status, then poll for SP 404 and disappearance of the linked ClusterOrder; emit diagnostics if cleanup does not complete.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

14. Operator failures can pass the dispatch gate 🐞 Bug ≡ Correctness

Description

TBP-030 succeeds once the linked ClusterOrder has any non-empty conditions array, then checks only
that its entries have a type and a valid condition-status string. An order with a failure condition
or only an unrelated initialization condition can pass because the test never requires the expected
progression or relates it to the SP state.

Code

tests/e2e/sp_osac_dispatch_test.go[R146-150]

+			conditions, found = nestedSlice(current, "status", "conditions")
+			if !found || len(conditions) == 0 {
+				return fmt.Errorf("ClusterOrder %s has no status.conditions yet; object=%s", name, compactJSON(current))
+			}
+			return nil

Evidence

The conditions poll returns success for any entry, the loop validates only shape, and the SP
assertion accepts any documented status.

tests/e2e/sp_osac_dispatch_test.go[139-164]
tests/e2e/sp_osac_dispatch_test.go[179-188]
test-plans/OSAC-tier-c-test-plan.md[273-280]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The dispatch gate accepts arbitrary ClusterOrder conditions without verifying the intended operator stop point.
## Fix Focus Areas
- tests/e2e/sp_osac_dispatch_test.go[139-188]
## Recommended Fix
Poll for the observed named progression/blocking condition and reason, reject failure conditions, and assert a corresponding SP status instead of only checking status vocabulary.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

15. Real-AAP job check silently skipped in tests 🐞 Bug ≡ Correctness

Description

TBP-030 checks successful AAP provisioning-job history only when its test process receives
OSAC_AAP_MODE=real, but deploy-dcm.sh leaves that value unexported and the generated backend
environment does not include the selected mode. When real mode is selected through `deploy-dcm.sh
--osac-aap-mode real, the later make test-osac-dispatch` run can skip the job assertion unless the
user sets the variable separately.

Code

tests/e2e/sp_osac_dispatch_test.go[R165-177]

+		if os.Getenv("OSAC_AAP_MODE") == "real" {
+			Eventually(func() error {
+				current, err := osacGetClusterOrder(name, namespace)
+				if err != nil {
+					return err
+				}
+				clusterOrder = current
+				if !osacProvisioningJobSucceeded(current) {
+					return fmt.Errorf("ClusterOrder %s has no successful AAP provisioning job yet", name)
+				}
+				return nil
+			}).WithTimeout(3 * time.Minute).WithPolling(5 * time.Second).Should(Succeed())
+		}

Evidence

TBP-030 gates its job-history assertion on os.Getenv("OSAC_AAP_MODE"). deploy-dcm.sh assigns the
mode without exporting it, the generated backend environment provides credentials and a CA path but
not the selected mode, and the Makefile test target does not set it; together, these show why the
documented real-AAP test run does not necessarily enter the assertion.

scripts/deploy-dcm.sh[1039-1039]
Makefile[57-58]
tests/e2e/sp_osac_dispatch_test.go[165-177]
scripts/deploy-osac-backend.sh[1428-1446]
scripts/deploy-dcm.sh[1116-1120]
Makefile[57-61]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
TBP-030 silently skips its real-AAP provisioning-job assertion when the test process does not receive `OSAC_AAP_MODE=real`, even if deployment selected real mode.

## Fix Focus Areas
- tests/e2e/sp_osac_dispatch_test.go[165-177]
- scripts/deploy-osac-backend.sh[1428-1446]

## Recommended Fix
Make real mode available to the test invocation, or detect it from the cluster, for example by checking for the `osac-aap-gateway-token` secret in the backend namespace. If retaining the environment-variable gate, document `OSAC_AAP_MODE=real make test-osac-dispatch` or provide a dedicated real-mode test target; log when the assertion is skipped, and fail explicitly when real-AAP verification was requested but its mode is unavailable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

16. Test plan gives conflicting TBP-030 status 🐞 Bug ⚙ Maintainability

Description

The exit-criteria item and the traceability row say TBP-030 currently fails because no ClusterOrder
status condition is emitted. The status header, the 'Observed Phase 2 discovery result' section and
CLAUDE.md say TBP-030 passes, so readers cannot tell what the current gate status is.

Code

test-plans/OSAC-tier-c-test-plan.md[R298-299]

+   the *reachable* control-plane path, not allocation. Current evidence is TBP-010/020
+   pass and TBP-030 fail due to the missing ClusterOrder status condition.

Evidence

Line 7 and lines 253-256 say TBP-030 passes. Lines 298-299 and line 528 say it fails.

test-plans/OSAC-tier-c-test-plan.md[250-260]
test-plans/OSAC-tier-c-test-plan.md[528-528]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test plan says TBP-030 both passes and fails.

## Fix Focus Areas
- test-plans/OSAC-tier-c-test-plan.md[296-299]
- test-plans/OSAC-tier-c-test-plan.md[528-528]

## Recommended Fix
Rewrite both places to match the current result (TBP-010/020/030 passing).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

---

Context sources

❌ Tickets: failed to load — Ticket collection could not establish complete evidence

✅ Compliance rules (platform): 25 rules

Review mode: Auto: 🧠 Deep: Broad deployment, security, integration, and test logic creates many independent defect opportunities.

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗