HYPERFLEET-1254 - docs: update adapter frame design doc#186
Conversation
📝 WalkthroughWalkthroughThe adapter framework design document revises component wiring, layered configuration, broker and API contracts, Kubernetes and Maestro transport routing, executor post-action semantics, stage-aware CEL evaluation, startup sequencing, environment-based observability, lifecycle scope, and unit-test coverage. Estimated code review effort: 2 (Simple) | ~15 minutes 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: 3
🤖 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 `@hyperfleet/components/adapter/framework/adapter-frame-design.md`:
- Around line 698-700: Resolve the lifecycle contradiction between the “Status
Reporter” description and the later post-processing flow: choose and document
one consistent condition for executing configured post.post_actions, including
behavior when preconditions fail or no resources are created. Update the
affected design flow and implementation contract so status reporting follows
that same condition everywhere.
- Around line 82-115: Make the configuration contract consistent across the
architecture diagram and deployment example: include the required
adapter-broker-config ConfigMap in the configuration node and deployment
volumes/environment wiring, and standardize the task configuration filename/key
to adapter-task-config.yaml everywhere, including the mount example.
- Around line 279-291: Update ExecutionContext.GetCELVariables() to stop
injecting raw env.* and config.* maps into CEL. Replace them with an explicit
allowlist of non-secret environment and configuration fields, excluding all
credentials, tokens, keys, authentication, and client settings; preserve the
existing adapter, resources, event, and parameter variables.
🪄 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: fe40e7b6-9103-424d-9538-f630bbd09e02
📒 Files selected for processing (1)
hyperfleet/components/adapter/framework/adapter-frame-design.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)
79a317d to
1936642
Compare
| ``` | ||
| 1. **Deployment Config ConfigMap** (`adapter-config`): Infra, clients, logging, broker — maps to `adapter-config.yaml` | ||
| 2. **Task Config ConfigMap** (`adapter-task-config`): Params, preconditions, resources, post-actions — maps to `task-config.yaml` | ||
| 3. **Broker ConfigMap** (`adapter-broker-config`): Message broker settings (env vars for broker library) |
There was a problem hiding this comment.
Warning
Blocking
Category: Inconsistency
The broker config delivery description is inconsistent across three sections in this PR:
- Here (config layers): says "env vars for broker library"
- Startup diagram (~line 953): says "Broker library loads config from environment variables"
- Deployment YAML (~line 234): mounts
adapter-broker-configas a file at/etc/broker/broker.yamlwith noenvFromorenventries
The old deployment used envFrom for broker config — this PR replaced it with file mounts, but the text and diagram still describe the old env var-based delivery. If the broker library now reads from a config file, update the description here and the startup diagram to reflect file-based delivery. If it still reads env vars, the deployment YAML needs envFrom instead of the file mount.
| | `env.*` | map | All OS environment variables accessible to the process | | ||
| | `event.*` | map | CloudEvent data payload fields (id, kind, href, generation, etc.) | | ||
| | `config.*` | map | Full adapter deployment config as nested map | | ||
|
|
There was a problem hiding this comment.
Tip
nit — non-blocking suggestion
Category: Inconsistency
The CEL variables table above lists config.* as a runtime-injected namespace alongside adapter.*, resources.*, env.*, and event.*, but this reserved names list only protects four of the five — config is missing. A parameter named config could shadow the runtime namespace. If that's intentional, maybe worth a note saying so. If not:
| > **Reserved names:** `adapter`, `resources`, `env`, `event`, `config` are overwritten by the runtime regardless of any param with the same name. |
1936642 to
89c3050
Compare
|
This PR updates post-processing to “always runs,” but a few nearby workflow sections still describe the old “only when resources already exist” model (High-Level Processing Steps, Status Reporting Patterns, Key Workflow Points). Aligning those leftovers would keep the lifecycle story consistent with the current executor behavior this PR is documenting. |
89c3050 to
f1085b7
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@hyperfleet/components/adapter/framework/adapter-frame-design.md`:
- Around line 753-758: Update the `adapter config-dump` workflow to redact every
credential-bearing field, including JWT credentials, broker passwords, tokens,
and keys, before output is written. Make redaction mandatory with no option or
mode that permits unredacted configuration, and ensure the same redacted
representation is used for terminal output and captured logs.
- Around line 830-834: Update the high-level and status workflow sections,
including the corresponding content near the post-processing/evaluation
descriptions, to state that post-processing runs for every eligible path after
the resources phase. Remove wording that creation reports exit, post-processing
waits for a later event, or evaluation is limited to existing resources; retain
only parameter-extraction failure and precondition ResourceNotFound as early
exits before post-actions.
- Around line 501-510: Update the subscription example around Subscriber and
Handler to show the broker callback receiving an event.Event pointer and the
topic string through Subscribe(ctx, topic, handler), replacing the outdated
[]byte callback signature while preserving the existing dispatch flow.
🪄 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: 7720af6e-9511-4358-a476-321f3e662243
📒 Files selected for processing (1)
hyperfleet/components/adapter/framework/adapter-frame-design.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)
2150006 to
895d094
Compare
895d094 to
45b377b
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ldornele 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 |
aaf6eae
into
openshift-hyperfleet:main
3 participants
Summary
HYPERFLEET-1254 updates
adapter-frame-design.mdto reflect the current state of the hyperfleet-adapter codebase.Test Plan
./hack/markdownlint.sh- no errors