-
Notifications
You must be signed in to change notification settings - Fork 19
Preserve ssh keys and resolv.conf #917
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnhance the test setup by preserving SSH keys and DNS resolver configuration through the build context and into the container image. Flow diagram for copying SSH keys and resolv.conf during container buildflowchart TD
A["Start Container Build"] --> B["COPY .ssh to /var/roothome/.ssh"]
A --> C["COPY resolv.conf to /etc/resolv.conf"]
B --> D["Container has SSH keys"]
C --> E["Container has DNS config"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `setup/bootc_configure_kernel_ima_module/Containerfile:8-10` </location>
<code_context>
COPY 10-ima_kargs.toml /usr/lib/bootc/kargs.d/10-ima_kargs.toml
COPY ima-policy /etc/ima/ima-policy
COPY yum.repos.d/* /etc/yum.repos.d/
+COPY .ssh /var/roothome/.ssh
+COPY resolv.conf /etc/resolv.conf
ARG KEYLIME_BOOTC_INSTALL_PACKAGES="rsync beakerlib selinux-policy-devel swtpm swtpm-tools nmap keylime expect"
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider file permissions and ownership for copied SSH directory.
Ensure that after copying the .ssh directory, you set appropriate permissions and ownership to protect private keys and restrict access to the intended user. Consider adding a RUN command for this purpose.
```suggestion
COPY .ssh /var/roothome/.ssh
RUN chown -R root:root /var/roothome/.ssh && \
chmod 700 /var/roothome/.ssh && \
find /var/roothome/.ssh -type f -name "id_*" -exec chmod 600 {} \;
COPY resolv.conf /etc/resolv.conf
ARG KEYLIME_BOOTC_INSTALL_PACKAGES="rsync beakerlib selinux-policy-devel swtpm swtpm-tools nmap keylime expect"
```
</issue_to_address>
### Comment 2
<location> `setup/bootc_configure_kernel_ima_module/Containerfile:9` </location>
<code_context>
COPY ima-policy /etc/ima/ima-policy
COPY yum.repos.d/* /etc/yum.repos.d/
+COPY .ssh /var/roothome/.ssh
+COPY resolv.conf /etc/resolv.conf
ARG KEYLIME_BOOTC_INSTALL_PACKAGES="rsync beakerlib selinux-policy-devel swtpm swtpm-tools nmap keylime expect"
RUN dnf -y install ${KEYLIME_BOOTC_INSTALL_PACKAGES}
</code_context>
<issue_to_address>
**question (bug_risk):** Overwriting /etc/resolv.conf may interfere with container DNS resolution.
Copying resolv.conf may disrupt container DNS settings. Please confirm this is required, and if so, document the reason and provide a way to restore or update DNS configuration as needed.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
COPY .ssh /var/roothome/.ssh | ||
COPY resolv.conf /etc/resolv.conf | ||
ARG KEYLIME_BOOTC_INSTALL_PACKAGES="rsync beakerlib selinux-policy-devel swtpm swtpm-tools nmap keylime expect" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 suggestion (security): Consider file permissions and ownership for copied SSH directory.
Ensure that after copying the .ssh directory, you set appropriate permissions and ownership to protect private keys and restrict access to the intended user. Consider adding a RUN command for this purpose.
COPY .ssh /var/roothome/.ssh | |
COPY resolv.conf /etc/resolv.conf | |
ARG KEYLIME_BOOTC_INSTALL_PACKAGES="rsync beakerlib selinux-policy-devel swtpm swtpm-tools nmap keylime expect" | |
COPY .ssh /var/roothome/.ssh | |
RUN chown -R root:root /var/roothome/.ssh && \ | |
chmod 700 /var/roothome/.ssh && \ | |
find /var/roothome/.ssh -type f -name "id_*" -exec chmod 600 {} \; | |
COPY resolv.conf /etc/resolv.conf | |
ARG KEYLIME_BOOTC_INSTALL_PACKAGES="rsync beakerlib selinux-policy-devel swtpm swtpm-tools nmap keylime expect" |
COPY ima-policy /etc/ima/ima-policy | ||
COPY yum.repos.d/* /etc/yum.repos.d/ | ||
COPY .ssh /var/roothome/.ssh | ||
COPY resolv.conf /etc/resolv.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (bug_risk): Overwriting /etc/resolv.conf may interfere with container DNS resolution.
Copying resolv.conf may disrupt container DNS settings. Please confirm this is required, and if so, document the reason and provide a way to restore or update DNS configuration as needed.
Summary by Sourcery
Include host SSH keys and resolv.conf in the bootc kernel IMA module test image build to maintain SSH access and DNS resolution
Enhancements: