docs: add ADR for catalog chaining#569
Draft
yocaba wants to merge 1 commit into
Draft
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds ADR 013, documenting the Solar Catalog Chaining architecture decision. The document evaluates patterns for transporting OCM packages across security boundaries via ARC and proposes Option A-1 (pull packages via ARC custom workflow) paired with Option B (discover and create Components/ComponentVersions locally) as the recommended approach. ChangesSolar Catalog Chaining ADR
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/developer-guide/adrs/013-catalog-chaining.md (2)
145-150:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDocument lifecycle/conflict/security handling for the selected recommendation.
The proposed decision on Line 145 does not yet describe how the chosen approach handles updates, deletions, version conflicts, and attestation validation. Those are explicit acceptance criteria for this ADR and should be captured in the decision itself (or in
Decision Outcome).Suggested ADR patch
## Proposed Solution Option A-1 for "Pulling OCM packages" and Option B for "Creating Components and ComponentVersions", for separation of concerns and a single source of truth. + +Operational policy for this choice: +- **Updates:** reconcile Orders from source catalog scan; destination discovery upserts ComponentVersions. +- **Deletions:** remove obsolete Orders and define destination catalog tombstone/retention behavior. +- **Version conflicts:** destination is authoritative for conflict resolution (idempotent upsert + deterministic naming/tag policy). +- **Attestation validation:** verify signatures/attestations before promotion to catalog-visible state; define fail-closed behavior and alerting.🤖 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 `@docs/developer-guide/adrs/013-catalog-chaining.md` around lines 145 - 150, Update the "Decision Outcome" section of the ADR to explicitly document lifecycle, conflict and security handling for the selected recommendation (Option A-1 and Option B): describe how updates and deletions are applied (e.g., immutability or upsert semantics), how version conflicts are detected and resolved (e.g., semantic versioning, last-write-wins, or merge/CR conflict workflow) and how attestations/validation are performed and stored (e.g., signature checks, replay protection, provenance metadata and revocation handling); reference the terms "Option A-1", "Option B" and the "Decision Outcome" heading so readers can find the new text and include concrete acceptance criteria for each item (updates, deletions, conflicts, attestation validation).
149-150:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComplete the empty
Decision Outcomesection.
## Decision Outcomeis present but has no content, so rationale, consequences, and follow-up actions are not recorded.Suggested ADR patch
## Decision Outcome + +- **Chosen:** Option A-1 (package pull) + Option B (component creation). +- **Why:** preserves source-catalog authority while keeping import logic in Solar Discovery. +- **Consequences:** requires Solar API read access from destination ARC workflow; transfer success and catalog availability are decoupled and need monitoring. +- **Follow-ups:** define reconciliation intervals, deletion policy, conflict policy, and attestation gates.🤖 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 `@docs/developer-guide/adrs/013-catalog-chaining.md` around lines 149 - 150, The `## Decision Outcome` heading is empty; fill it with a concise summary of the chosen decision for this ADR (catalog chaining), explicitly state the final decision outcome, list key consequences and trade-offs (e.g., performance, complexity, compatibility), and add concrete follow-up actions and owners (e.g., migration tasks, monitoring, future reviews) plus a target review date; update the section under the existing "## Decision Outcome" header in 013-catalog-chaining.md so it contains the decision summary, consequences, and next steps with responsible parties.
🤖 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/developer-guide/adrs/013-catalog-chaining.md`:
- Around line 139-142: Expand the OCI URL Re-Mapping section into explicit
per-layer contracts: enumerate ownership and remapping rules for
Component.Spec.Registry, ComponentVersion.Spec.Resources[*].Repository,
descriptor repositoryContext/resource access, and Helm values.yaml registry
references; for each surface specify whether remapping is hostname-only vs
full-path, how aliases and ARC-derived mappings apply, what failure modes are
allowed (e.g., path changes vs hostname replacement), and which component (OCM,
ARC, ocm-kit, Solar Discovery) is authoritative for transformation and
validation; add examples and a short decision table describing input shape,
remap rule, and fallback behavior so the ADR meets the “comprehensive remapping
strategy” acceptance target.
---
Outside diff comments:
In `@docs/developer-guide/adrs/013-catalog-chaining.md`:
- Around line 145-150: Update the "Decision Outcome" section of the ADR to
explicitly document lifecycle, conflict and security handling for the selected
recommendation (Option A-1 and Option B): describe how updates and deletions are
applied (e.g., immutability or upsert semantics), how version conflicts are
detected and resolved (e.g., semantic versioning, last-write-wins, or merge/CR
conflict workflow) and how attestations/validation are performed and stored
(e.g., signature checks, replay protection, provenance metadata and revocation
handling); reference the terms "Option A-1", "Option B" and the "Decision
Outcome" heading so readers can find the new text and include concrete
acceptance criteria for each item (updates, deletions, conflicts, attestation
validation).
- Around line 149-150: The `## Decision Outcome` heading is empty; fill it with
a concise summary of the chosen decision for this ADR (catalog chaining),
explicitly state the final decision outcome, list key consequences and
trade-offs (e.g., performance, complexity, compatibility), and add concrete
follow-up actions and owners (e.g., migration tasks, monitoring, future reviews)
plus a target review date; update the section under the existing "## Decision
Outcome" header in 013-catalog-chaining.md so it contains the decision summary,
consequences, and next steps with responsible parties.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c2f17b3-3d8a-47a0-a559-d8364ccc9714
⛔ Files ignored due to path filters (2)
docs/developer-guide/adrs/img/013-catalog-chaining.svgis excluded by!**/*.svgdocs/developer-guide/adrs/img/013-proposed-solution.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
docs/developer-guide/adrs/013-catalog-chaining.md
Comment on lines
+139
to
+142
| ### OCI URL Re-Mapping | ||
|
|
||
| When OCM packages are transferred across security boundaries via ARC, OCI URLs change. URLs within the OCM component descriptor are handled by OCM itself, as it is designed as a transfer format; configuring OCM to use relative paths is an alternative worth considering. Because the Component and ComponentVersion are created on the destination side, no re-mapping is needed for them. Registry references in Helm charts are covered by the Helm value templating support implemented in Solar Discovery and rendered by leveraging ocm-kit. No additional features are needed. | ||
|
|
Contributor
There was a problem hiding this comment.
Expand OCI URL remapping into explicit per-layer contracts.
Line 141 currently compresses remapping into a single paragraph and skips explicit ownership/rules for the required surfaces (e.g., Component.Spec.Registry, ComponentVersion.Spec.Resources[*].Repository, descriptor repositoryContext/resource access, Helm values.yaml references), plus edge/failure cases (path changes vs hostname-only replacement, alias interaction, static vs ARC-derived mapping).
This leaves the ADR short of the “comprehensive remapping strategy” acceptance target.
Suggested ADR patch
### OCI URL Re-Mapping
-When OCM packages are transferred across security boundaries via ARC, OCI URLs change. URLs within the OCM component descriptor are handled by OCM itself, as it is designed as a transfer format; configuring OCM to use relative paths is an alternative worth considering. Because the Component and ComponentVersion are created on the destination side, no re-mapping is needed for them. Registry references in Helm charts are covered by the Helm value templating support implemented in Solar Discovery and rendered by leveraging ocm-kit. No additional features are needed.
+When OCM packages are transferred across security boundaries via ARC, OCI URLs change. Remapping responsibilities are:
+
+- **OCM descriptor (`repositoryContext`, resource access):** rewritten by OCM transfer behavior; validate whether rewrite is host-only or host+path for each registry topology.
+- **Solar API objects (`Component.Spec.Registry`, `ComponentVersion.Spec.Resources[*].Repository`):** derived from destination-side discovery output; no source URL should be persisted in destination resources.
+- **Helm chart image references (`values.yaml`):** handled via Helm values templating in Solar Discovery (rendered with ocm-kit input).
+- **Registry aliases (`Registry.Spec.Aliases`):** define precedence and conflict behavior when alias mapping differs from ARC route mapping.
+
+Failure modes and handling:
+- non-rewritable hardcoded image refs in charts,
+- descriptor entries that cannot be rewritten cleanly across path-changing registries,
+- alias/mapping conflicts,
+- partial transfer states where package moved but discovery/import is delayed.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### OCI URL Re-Mapping | |
| When OCM packages are transferred across security boundaries via ARC, OCI URLs change. URLs within the OCM component descriptor are handled by OCM itself, as it is designed as a transfer format; configuring OCM to use relative paths is an alternative worth considering. Because the Component and ComponentVersion are created on the destination side, no re-mapping is needed for them. Registry references in Helm charts are covered by the Helm value templating support implemented in Solar Discovery and rendered by leveraging ocm-kit. No additional features are needed. | |
| ### OCI URL Re-Mapping | |
| When OCM packages are transferred across security boundaries via ARC, OCI URLs change. Remapping responsibilities are: | |
| - **OCM descriptor (`repositoryContext`, resource access):** rewritten by OCM transfer behavior; validate whether rewrite is host-only or host+path for each registry topology. | |
| - **Solar API objects (`Component.Spec.Registry`, `ComponentVersion.Spec.Resources[*].Repository`):** derived from destination-side discovery output; no source URL should be persisted in destination resources. | |
| - **Helm chart image references (`values.yaml`):** handled via Helm values templating in Solar Discovery (rendered with ocm-kit input). | |
| - **Registry aliases (`Registry.Spec.Aliases`):** define precedence and conflict behavior when alias mapping differs from ARC route mapping. | |
| Failure modes and handling: | |
| - non-rewritable hardcoded image refs in charts, | |
| - descriptor entries that cannot be rewritten cleanly across path-changing registries, | |
| - alias/mapping conflicts, | |
| - partial transfer states where package moved but discovery/import is delayed. |
🤖 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 `@docs/developer-guide/adrs/013-catalog-chaining.md` around lines 139 - 142,
Expand the OCI URL Re-Mapping section into explicit per-layer contracts:
enumerate ownership and remapping rules for Component.Spec.Registry,
ComponentVersion.Spec.Resources[*].Repository, descriptor
repositoryContext/resource access, and Helm values.yaml registry references; for
each surface specify whether remapping is hostname-only vs full-path, how
aliases and ARC-derived mappings apply, what failure modes are allowed (e.g.,
path changes vs hostname replacement), and which component (OCM, ARC, ocm-kit,
Solar Discovery) is authoritative for transformation and validation; add
examples and a short decision table describing input shape, remap rule, and
fallback behavior so the ADR meets the “comprehensive remapping strategy”
acceptance target.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes #418
Checklist
Tests added/updatedn/aSummary by CodeRabbit
Documentation