OSAC-2077: CaaS template writes VIPs to ClusterOrder status

[ori-amizur]

The external_access service role now writes the discovered API and ingress VIPs as annotations on the ClusterOrder CR after the network-class-specific provisioning completes.

The ClusterOrder controller copies these annotations into the status fields (apiEndpoint, ingressEndpoint) during reconciliation. The feedback controller then syncs them to the fulfillment-service Cluster object, enabling the ExternalIPAttachment flow.

Uses the established annotation-to-status pattern since the kubernetes.core Ansible modules don't support the status subresource.

Assisted-by: Claude Code noreply@anthropic.com

[openshift-ci-robot]

@ori-amizur: This pull request references OSAC-2077 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

The external_access service role now writes the discovered API and ingress VIPs as annotations on the ClusterOrder CR after the network-class-specific provisioning completes.

The ClusterOrder controller copies these annotations into the status fields (apiEndpoint, ingressEndpoint) during reconciliation. The feedback controller then syncs them to the fulfillment-service Cluster object, enabling the ExternalIPAttachment flow.

Uses the established annotation-to-status pattern since the kubernetes.core Ansible modules don't support the status subresource.

Assisted-by: Claude Code noreply@anthropic.com

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ori-amizur

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• osac-aap/OWNERS [ori-amizur]
• osac-operator/OWNERS [ori-amizur]

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

[coderabbitai]

Warning

Review limit reached

@ori-amizur, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 76 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d4c1e30d-2b4d-429f-b0eb-cd7a1101c8e4

📥 Commits

Reviewing files that changed from the base of the PR and between eee3ec2 and e674c41.

📒 Files selected for processing (4)

• osac-aap/collections/ansible_collections/osac/service/roles/external_access/tasks/create.yaml
• osac-operator/internal/controller/clusterorder_controller.go
• osac-operator/internal/controller/clusterorder_controller_test.go
• osac-operator/internal/controller/clusterorder_names.go

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:56 PM UTC · Completed 4:14 PM UTC

Commit: e674c41 · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [missing consumer / incomplete data path] osac-operator/internal/controller/feedback_controller.go:115 — The PR description states "The feedback controller then syncs them to the fulfillment-service Cluster object," but newClusterOrderSyncUpdate does not sync ApiEndpoint or IngressEndpoint to the remote Cluster proto object. The proto API has SetApiEndpoint/SetIngressEndpoint methods available, and the fulfillment-service rendering tables reference status.api_endpoint and status.ingress_endpoint, but no code calls these setters. The VIP endpoints will populate ClusterOrder.Status correctly (the ExternalIPAttachment controller can consume them), but they will not appear in the fulfillment-service API or CLI output. This may be intentionally deferred to a follow-up PR.
  Remediation: Add calls to remote.GetStatus().SetApiEndpoint(...) and remote.GetStatus().SetIngressEndpoint(...) in newClusterOrderSyncUpdate, or update the PR description to clarify the sync is deferred.

Low

• [test coverage gap] osac-operator/internal/controller/clusterorder_controller_test.go:864 — Tests cover the happy path (both annotations present) and nil-annotations case but not partial annotations (one set, one absent). A partial-annotation test would guard against future refactoring that couples the independent checks.
• [input validation] osac-operator/internal/controller/clusterorder_controller.go:391 — reconcileVIPEndpoints copies annotation values to status without format validation. Values flow through to the ExternalIPAttachment controller's resolveClusterEndpoint. Risk is mitigated by ClusterOrders living in the operator-managed osac-orders namespace, but a net.ParseIP check would provide defense-in-depth.
• [missing-documentation] osac-aap/docs/netris-integration.md:97 — The External Access create flow description does not mention the shared role's post-step VIP annotation write to ClusterOrder.
• [missing-documentation] osac-aap/docs/nico-integration.md:154 — The NICo external_access flow description does not mention the shared role's post-step VIP annotation write to ClusterOrder.

---

Labels: PR adds new VIP endpoint reconciliation feature spanning osac-aap and osac-operator components

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

[low] test coverage gap

Tests cover both-annotations-present and nil-annotations cases but not partial annotations (one annotation set, the other absent). The implementation handles each independently and is correct, but a partial-annotation test would guard against future refactoring that accidentally couples the checks.

[fullsend-ai-review]

[low] input validation

reconcileVIPEndpoints copies annotation values directly to status fields without IP format validation. Values flow to the ExternalIPAttachment controller resolveClusterEndpoint. Risk is mitigated by ClusterOrders living in the operator-managed osac-orders namespace, but net.ParseIP validation would provide defense-in-depth against non-IP strings propagating through the ExternalIPAttachment flow.

Suggested fix: Add net.ParseIP validation before assigning annotation values to status fields; log a warning and skip when the value is not a valid IP.