Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Bump mark to 9.11.1 #16

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Bump mark to 9.11.1 #16

Changes from 13 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9eacde1
Update mark to 9.11.1
ramirezalfredo Jan 15, 2024
6f20bdb
Update Dockerfile
ramirezalfredo Jan 16, 2024
7932d8b
Update user in Dockerfile
ramirezalfredo Jan 16, 2024
b1ea90c
Add user to Dockerfile
ramirezalfredo Jan 16, 2024
8af74ad
Test no user
ramirezalfredo Jan 16, 2024
717bb61
Downgrade mark version
ramirezalfredo Jan 16, 2024
2e3d22b
Update running container with headless-shell
ramirezalfredo Jan 17, 2024
9db971a
Update Dockerfile
ramirezalfredo Jan 17, 2024
ef5427c
Update Dockerfile add dumb-init
ramirezalfredo Jan 17, 2024
e07e7ee
Clean-up
ramirezalfredo Jan 17, 2024
174bb41
Update Dockerfile to remove USER
ramirezalfredo Jan 17, 2024
71f4b44
Bump mark version to 9.11.1
ramirezalfredo Jan 18, 2024
c9180ed
Merge branch 'main' into bump-mark-version
ramirezalfredo Jan 24, 2024
6f6e5a2
Set headless-shell version/tag
ramirezalfredo Jan 24, 2024
c0215a5
Update Python tag
ramirezalfredo Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,24 @@
FROM python:3-slim AS builder
ENV MARK="9.9.0"

FROM python:3.11-slim-bookworm AS builder
ENV MARK="9.11.1"
ramirezalfredo marked this conversation as resolved.
Show resolved Hide resolved
ADD . /app
WORKDIR /app

RUN pip install --target=/app -r requirements.txt && \
apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install --no-install-recommends -y tar curl gnupg2 && \
rm -rf /var/lib/apt/lists/* && \
curl -LO https://github.com/kovetskiy/mark/releases/download/${MARK}/mark_Linux_x86_64.tar.gz && \
tar -xvzf mark_Linux_x86_64.tar.gz && chmod +x mark && mv mark /usr/local/bin/mark \
&& curl -L https://dl-ssl.google.com/linux/linux_signing_key.pub |apt-key add - \
&& echo "deb http://dl.google.com/linux/chrome/deb/ stable main" >> /etc/apt/sources.list.d/google.list \
&& apt update && apt-get install -y google-chrome-stable
tar -xvzf mark_Linux_x86_64.tar.gz && chmod +x mark && mv mark /usr/local/bin/mark && rm mark_Linux_x86_64.tar.gz \
&& apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*

FROM python:3-slim
FROM chromedp/headless-shell:latest
RUN apt-get update \
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about this change instead of copy from builder...you are moving from 156MB of base image to more 285MB this will have a sensible impact on our build times
image

Copy link

Choose a reason for hiding this comment

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

Looks like this comment got marked as outdated. I agree with mbovo on this. No need to use a different/larger container.

&& apt-get install --no-install-recommends -qq ca-certificates bash sed git dumb-init python3 \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*
COPY --from=builder /app /app
COPY --from=builder /usr/local/bin/mark /usr/bin/mark
COPY --from=builder /usr/bin/google-chrome /usr/bin/google-chrome
WORKDIR /app
ENV PYTHONPATH /app
ENV DOC_PREFIX /github/workspace/
ENV LOGURU_FORMAT "<lvl>{level:7} {message}</lvl>"
USER 1001:1001
ENTRYPOINT [ "python" ]
Copy link
Member

Choose a reason for hiding this comment

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

This is a quite big security concern, it will require an explicit exception to run it on our clusters. (Kyverno will prevent it to run at all, afaik)

Copy link

Choose a reason for hiding this comment

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

Agreed. How come we have to perform this change now? Did it work before with the unprivileged user?
The doc seems to go in the direction of that change though: https://docs.github.com/en/actions/creating-actions/dockerfile-support-for-github-actions#user

Copy link

Choose a reason for hiding this comment

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

no not really. I had to add this to my action to get it to work which is awful

   - name: chown files to UID 1001
     shell: bash
     run: sudo chown -R 1001:1001 *

CMD ["/app/mark2confluence/main.py"]
ENTRYPOINT ["/usr/bin/dumb-init", "--"]
CMD ["python3", "/app/mark2confluence/main.py"]
Loading