Skip to content

Dockerfile fixed to include .crt files as well#144

Merged
oto-macenauer-absa merged 5 commits intomasterfrom
143-dockerfile-fix-to-include-crt-certificate-extension
Apr 27, 2026
Merged

Dockerfile fixed to include .crt files as well#144
oto-macenauer-absa merged 5 commits intomasterfrom
143-dockerfile-fix-to-include-crt-certificate-extension

Conversation

@muharemd
Copy link
Copy Markdown
Contributor

@muharemd muharemd commented Apr 27, 2026

Dockerfile fix to include .crt certificate extension.

Release Notes

  • Currently the docker image build fails with certificate issue as the for loop on the line 32 only imports the .pem files but we need to catch .crt newly added files as well.
  • fix: for FILE in ls /opt/certs/.pem /opt/certs/.crf;

Related

Closes #143

Summary by CodeRabbit

  • Chores
    • Enhanced container certificate import to accept additional certificate formats.
    • Ensured external package downloads during build explicitly use the imported CA bundle for trusted TLS verification.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 74b28527-5e46-4101-a9f2-f664f4482af8

📥 Commits

Reviewing files that changed from the base of the PR and between 2d952e9 and 1eb4c58.

📒 Files selected for processing (1)
  • Dockerfile
🚧 Files skipped from review as they are similar to previous changes (1)
  • Dockerfile

Walkthrough

The Dockerfile's certificate import step now processes both .pem and .crt files and the external downloads (librdkafka and confluent-kafka-python) instruct wget to use the populated CA bundle at /etc/pki/tls/certs/ca-bundle.crt via --ca-certificate, instead of relying on default system CA resolution.

Changes

Cohort / File(s) Summary
Dockerfile Certificate Handling
Dockerfile
Expanded the trusted-certificate import glob to include *.crt in addition to *.pem. Updated wget invocations for external downloads to pass --ca-certificate=/etc/pki/tls/certs/ca-bundle.crt so downloads use the bundled certs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through the Dockerfile night,
Found .crt hiding just out of sight.
One tiny glob and a wget flag true,
Now certs are bundled — the build says woo! 🥕✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately describes the main change: expanding Dockerfile certificate import to include .crt files alongside existing .pem files.
Description check ✅ Passed Description includes all required template sections (Overview, Release Notes, Related) with concrete details about the certificate import issue and fix, though release notes could be more polished.
Linked Issues check ✅ Passed Changes directly address issue #143 by modifying the Dockerfile to include .crt files in certificate import, matching the stated problem and required outcome.
Out of Scope Changes check ✅ Passed All changes are scoped to the Dockerfile certificate handling requirements; modifications to wget CA bundle parameters are directly related to supporting the new .crt files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 143-dockerfile-fix-to-include-crt-certificate-extension

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Dockerfile`:
- Around line 32-33: Fix the typo and make the loop robust: replace the
backticked ls line "for FILE in `ls /opt/certs/*.pem /opt/certs/*.crf`;" with a
glob-based loop over .pem and .crt files (e.g. "for FILE in /opt/certs/*.pem
/opt/certs/*.crt; do [ -f \"$FILE\" ] && cat \"$FILE\" >>
/etc/pki/tls/certs/ca-bundle.crt; done && \") so you correct .crf → .crt and
avoid failing the Docker build when no matches exist by checking [ -f "$FILE" ]
before concatenating.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4845bef3-a7e1-4ef6-87fb-735991867e60

📥 Commits

Reviewing files that changed from the base of the PR and between 04f327e and 2d952e9.

📒 Files selected for processing (1)
  • Dockerfile

Comment thread Dockerfile Outdated
Copy link
Copy Markdown
Collaborator

@oto-macenauer-absa oto-macenauer-absa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix the typo in the file extension also wget needs to be provided with the cert bundle otherwise it will fail:
e.g.

wget --ca-certificate=/etc/pki/tls/certs/ca-bundle.crt https://github.com/edenhill/librdkafka/archive/v2.4.0.tar.gz && \

Comment thread Dockerfile Outdated
@oto-macenauer-absa oto-macenauer-absa merged commit 11b787d into master Apr 27, 2026
11 checks passed
@oto-macenauer-absa oto-macenauer-absa deleted the 143-dockerfile-fix-to-include-crt-certificate-extension branch April 27, 2026 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dockerfile fix to include .crt certificate extension.

2 participants