Wizard: add the container installer official image (HMS-11165) - #4738
Wizard: add the container installer official image (HMS-11165)#4738lucasgarfield wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
useImagePullValidation, the payload image existence logic duplicates much of the primary image path; consider extracting a small helper to encapsulate theneedsPayload/ loading / error branches so future changes (e.g., different payload rules) only need to be implemented once. - The installer and payload image references are hard-coded in multiple places (tests and
KNOWN_IMAGES); centralizing these strings (or deriving the test constants fromKNOWN_IMAGES) would reduce the risk of drift if the registry paths change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `useImagePullValidation`, the payload image existence logic duplicates much of the primary image path; consider extracting a small helper to encapsulate the `needsPayload` / loading / error branches so future changes (e.g., different payload rules) only need to be implemented once.
- The installer and payload image references are hard-coded in multiple places (tests and `KNOWN_IMAGES`); centralizing these strings (or deriving the test constants from `KNOWN_IMAGES`) would reduce the risk of drift if the registry paths change.
## Individual Comments
### Comment 1
<location path="src/store/slices/wizard/listeners.ts" line_range="158" />
<code_context>
+
+ // The container installer needs a payload container; default to the
+ // one the selected official image ships with.
+ const payloadRef = selected.iso_payload_references?.[0];
+ if (
+ targetType === 'bootable-container-iso' &&
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against `selected` being undefined when deriving `payloadRef`.
Here `selected` is treated as always defined, but `KNOWN_IMAGES.find(...)` can return `undefined`. Since the optional chaining only applies to `iso_payload_references`, accessing `selected.iso_payload_references` will throw if `selected` is undefined.
Consider updating this to:
```ts
const payloadRef = selected?.iso_payload_references?.[0];
```
to avoid a runtime error when no matching image is found.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| // The container installer needs a payload container; default to the | ||
| // one the selected official image ships with. | ||
| const payloadRef = selected.iso_payload_references?.[0]; |
There was a problem hiding this comment.
issue (bug_risk): Guard against selected being undefined when deriving payloadRef.
Here selected is treated as always defined, but KNOWN_IMAGES.find(...) can return undefined. Since the optional chaining only applies to iso_payload_references, accessing selected.iso_payload_references will throw if selected is undefined.
Consider updating this to:
const payloadRef = selected?.iso_payload_references?.[0];to avoid a runtime error when no matching image is found.
3be3d70 to
0b95eda
Compare
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## lucas/cockpit/4-target-sync #4738 +/- ##
===============================================================
- Coverage 79.22% 78.39% -0.84%
===============================================================
Files 264 264
Lines 7004 7016 +12
Branches 2576 2583 +7
===============================================================
- Hits 5549 5500 -49
- Misses 1358 1437 +79
+ Partials 97 79 -18
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 19 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
0b95eda to
c15dec4
Compare
c15dec4 to
672de56
Compare
672de56 to
3ccac49
Compare
0b95eda to
f83bd2a
Compare
f83bd2a to
39693b2
Compare
The image-builder CLI calls the installer ISO bootc-generic-iso, not bootable-container-iso; passing the frontend alias made the build unit fail before producing any output. Add the pair to the existing frontend/backend type maps.
Add the RHEL 10.3 container installer (bootable-container-iso) to the official on-prem images. Selecting its target environment shows a second read-only container section for the payload - the base image the installer deploys - with its own pull button. The payload reference is stored in the wizard state, defaulted by the resolveOfficialImage listener when the installer type is selected, and flows to the image-builder CLI via the existing bootc argument helper. Building needs both containers in local storage, so the pull validation now requires the payload to be pulled as well as the installer image, with the same logged-out copy pointing at the registry login. Subscriptions are not supported for the installer iso: the existing per-type restrictions hide the registration step and reset the registration type when it is selected. Resolves #4714
39693b2 to
358a62e
Compare
|
Replaced by #4756 |
Adds the RHEL 10.3 container installer ISO as a selectable official image, with a second read-only container section for its payload.
Stack created with GitHub Stacks CLI • Give Feedback 💬