HYPERFLEET-1195 - docs: add common when patterns and combination guide#238
Conversation
📝 WalkthroughWalkthroughAdds a “Common Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Adapter
participant Payload
participant ClusterAPI
Adapter->>Payload: Evaluate adapter.resourcesSkipped
Payload->>ClusterAPI: PUT cluster status
Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/adapter-authoring-guide.md`:
- Line 1234: The “Skip when no work done” pattern uses the inverted condition
`!adapter.resourcesSkipped`, which runs when work was done. Rename it to “Run
when work was done,” or change the example condition to
`adapter.resourcesSkipped` so the label and behavior match.
🪄 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: b3a3b097-7e3c-403a-b51a-eaab83d640bc
📒 Files selected for processing (1)
docs/adapter-authoring-guide.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)
Risk Score: 0 —
|
| Signal | Detail | Points |
|---|---|---|
| PR size | 77 lines | +0 |
| Sensitive paths | none | +0 |
Computed by hyperfleet-risk-scorer
c716072 to
a46c325
Compare
Add skipped-path example (adapter.resourcesSkipped) to the kubernetes example adapter, complementing the existing normal-path when gate. Demonstrates conditional payloads and post-actions for adapter authors. Enhance authoring guide with common when patterns table, guidance on combining when on payloads and post-actions, and cross-reference to the kubernetes example.
a46c325 to
e865be9
Compare
There was a problem hiding this comment.
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 `@charts/examples/kubernetes/adapter-task-config.yaml`:
- Around line 171-191: Update the resourcesSkipped handling in the adapter build
configuration so this payload is selected only for the successful
PreconditionNotMet outcome, rather than every skipped-resource case. Add a
separate failure-reporting path for PreconditionFailed and other skip causes,
preserving accurate outcome-specific condition type, status, reason, and message
metadata instead of hardcoding Health=True, NoErrors, and PreconditionsNotMet.
- Around line 168-172: The Kubernetes example defines skippedStatusPayload
without any precondition or skip-triggering flow. Update the task configuration
around checkClusterState and skippedStatusPayload to add a concrete precondition
with explicit match and no-match handling that sets adapter.resourcesSkipped
when resources are skipped, or remove the skipped status payload branch if this
example should not support skipping.
🪄 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: 8b3b8ec0-552e-4cb8-83e4-4a56f7c29912
📒 Files selected for processing (2)
charts/examples/kubernetes/adapter-task-config.yamldocs/adapter-authoring-guide.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)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/adapter-authoring-guide.md
| # Skipped path: preconditions not met — report simplified status without | ||
| # evaluating resources.* (which may not exist in this context) | ||
| - name: "skippedStatusPayload" | ||
| when: | ||
| expression: "adapter.resourcesSkipped" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 \
'^[[:space:]]*preconditions:|ResourcesSkipped|SetSkipped|PreconditionFailed|ResourceNotFound|skippedStatusPayload|reportClusterStatusSkipped' \
internal/executor charts/examples/kubernetes/adapter-task-config.yamlRepository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 38076
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' charts/examples/kubernetes/adapter-task-config.yamlRepository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 8256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('charts/examples/kubernetes/adapter-task-config.yaml')
for start, end in [(1, 260)]:
lines = p.read_text().splitlines()
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 9582
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f -a . charts/examples | sed -n '1,120p'Repository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 1769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'preconditions:|resourcesSkipped|skippedStatusPayload|reportClusterStatusSkipped' charts/examplesRepository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 11862
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' charts/examples/kubernetes/README.mdRepository: openshift-hyperfleet/hyperfleet-adapter
Length of output: 3968
Add a real skip path to the Kubernetes example (CWE-561). charts/examples/kubernetes/adapter-task-config.yaml documents skippedStatusPayload, but this example never defines any preconditions: or other skip-triggering branch, so adapter.resourcesSkipped cannot drive that payload/post-action. Wire checkClusterState into a concrete precondition with both match and no-match cases, or drop the skipped branch from the example.
🤖 Prompt for 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.
In `@charts/examples/kubernetes/adapter-task-config.yaml` around lines 168 - 172,
The Kubernetes example defines skippedStatusPayload without any precondition or
skip-triggering flow. Update the task configuration around checkClusterState and
skippedStatusPayload to add a concrete precondition with explicit match and
no-match handling that sets adapter.resourcesSkipped when resources are skipped,
or remove the skipped status payload branch if this example should not support
skipping.
Source: Path instructions
| when: | ||
| expression: "adapter.resourcesSkipped" | ||
| build: | ||
| adapter: "{{ .adapter.name }}" | ||
| conditions: | ||
| - type: "Applied" | ||
| status: "False" | ||
| reason: "PreconditionsNotMet" | ||
| message: | ||
| expression: | | ||
| "Resources skipped: " + adapter.?skipReason.orValue("preconditions not met") | ||
| - type: "Available" | ||
| status: "False" | ||
| reason: "PreconditionsNotMet" | ||
| message: | ||
| expression: | | ||
| "Resources not available: " + adapter.?skipReason.orValue("preconditions not met") | ||
| - type: "Health" | ||
| status: "True" | ||
| reason: "NoErrors" | ||
| message: "Adapter is healthy, no work performed due to preconditions" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not classify every resourcesSkipped outcome as a healthy precondition skip (CWE-754).
This payload hardcodes Health=True, NoErrors, and PreconditionsNotMet, but resourcesSkipped also represents failed precondition evaluation and resource-not-found handling. A failed precondition can therefore be persisted through the new PUT action as a healthy status, while a not-found case is mislabeled as a precondition skip. (raw.githubusercontent.com)
Use distinct outcome metadata: select this payload only for a successful PreconditionNotMet business outcome, and add a separate failure-reporting path for PreconditionFailed and other skip causes.
As per path instructions: “Prioritize Critical and Major severity issues” and validate cross-layer contracts.
🤖 Prompt for 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.
In `@charts/examples/kubernetes/adapter-task-config.yaml` around lines 171 - 191,
Update the resourcesSkipped handling in the adapter build configuration so this
payload is selected only for the successful PreconditionNotMet outcome, rather
than every skipped-resource case. Add a separate failure-reporting path for
PreconditionFailed and other skip causes, preserving accurate outcome-specific
condition type, status, reason, and message metadata instead of hardcoding
Health=True, NoErrors, and PreconditionsNotMet.
Source: Path instructions
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rh-amarin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test presubmits-integration |
be45443
into
openshift-hyperfleet:main
3 participants
Summary
adapter.resourcesSkipped) to thekubernetes/example adapter, complementing the existing normal-pathwhengate — demonstrates both conditional payloads and conditional post-actionswhenpatterns" quick-reference table with 5 common expressionswhenon payloads and post-actions" subsection explaining the dual-gate pattern vs. auto-skip behaviorContext
HYPERFLEET-1195
Test plan
whenexpressions use standard CEL patterns