Move to fedora-minimal - #347
Conversation
Change deployment base image from fedora to fedora-minimal Signed-off-by: Yair Podemsky <ypodemsk@redhat.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideSwitch all runtime stages in the Containerfile from the standard Fedora 43 base image to the lighter fedora-minimal 43 image, keeping build outputs and entrypoints unchanged. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="Containerfile" line_range="75" />
<code_context>
# Distribution stages
-FROM quay.io/fedora/fedora:43 AS operator
+FROM quay.io/fedora/fedora-minimal:43 AS operator
COPY --from=builder /output/operator /usr/bin
ENTRYPOINT ["/usr/bin/operator"]
</code_context>
<issue_to_address>
**issue (broader_impact):** The runtime images do not install `ca-certificates`, so the Rust Kubernetes and HTTPS clients cannot verify the cluster API server or other TLS endpoints and fail with certificate-verification errors. The previous Fedora image supplied the system CA bundle that these dynamically linked OpenSSL/native-TLS clients rely on.
**Triggers:** When the Kubernetes API or configured Trustee/registration endpoints use TLS, which is the normal deployment configuration.
**Suggested fix:** Install `ca-certificates` in each minimal runtime stage, or retain a runtime base image that provides the CA bundle.
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: Containerfile:75
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
I think it 's better to do it only for 'release' not for 'debug', however doing that makes the Containerfile more complex. In sourcery-ai review, missing 'ca-certificates' package is mentioned. |
Both images have the same 'ca-certificates' package. it should be a complete drop-in replacement. |
We could have it change depending by making the image a variable that gets different values depending on the value of build_type, but as you say, it's a more complex Containerfile. |
Jakob-Naucke
left a comment
There was a problem hiding this comment.
I think it 's better to do it only for 'release' not for 'debug', however doing that makes the Containerfile more complex.
@uril when would that matter? even for debug builds, we don't include enough packages for e.g. a debugger (then you'd want libstdc++-debuginfo and others)
@Jakob-Naucke Yes, we can add those to debug builds (although we can do that with fedora-minimal too). Let's resolve this comment. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Jakob-Naucke, yairpod The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Change deployment base image from fedora to fedora-minimal
Fedora minimal reduces the base image size from uncompressed ~261MB to ~191MB.
This change requires no other changes from us.
Summary by Sourcery
Enhancements: