-
Notifications
You must be signed in to change notification settings - Fork 4
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
Update dockerfile with pa11y-ci #7
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,22 +11,14 @@ LABEL maintainer="Phase2 <outrigger@phase2technology.com>" \ | |
org.label-schema.docker.debug="docker exec -it $CONTAINER bash" \ | ||
org.label-schema.schema-version="1.0" | ||
|
||
# It's a good idea to use dumb-init to help prevent zombie chrome processes. | ||
ADD https://github.com/Yelp/dumb-init/releases/download/v1.2.0/dumb-init_1.2.0_amd64 /usr/local/bin/dumb-init | ||
RUN chmod +x /usr/local/bin/dumb-init | ||
USER root | ||
|
||
# Let's get pa11y v5 in here. | ||
RUN yarn global add pa11y@beta | ||
|
||
# Add user so we don't need --no-sandbox. | ||
RUN groupadd -r pptruser && useradd -r -g pptruser -G audio,video pptruser \ | ||
&& mkdir -p /home/pptruser/Downloads \ | ||
&& chown -R pptruser:pptruser /home/pptruser \ | ||
&& chown -R pptruser:pptruser /screenshots \ | ||
&& chown -R pptruser:pptruser /usr/local/share/.config/yarn/global/node_modules | ||
RUN yarn global add pa11y@5 | ||
RUN yarn global add pa11y-ci@2 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we have one of the following to minimize the number of layers?
or
Also, is it the case we need to explicitly add pa11y? We can't invoke it if it's a dependency of pa11y-ci? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have confirmed that pa11y needs to be explicitly added. If pa11y-ci gets added globally, that doesn't give access to pa11y globally |
||
|
||
USER pptruser | ||
|
||
ENTRYPOINT ["dumb-init", "--", "pa11y"] | ||
ENTRYPOINT ["dumb-init", "--"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the flexibility here, but if we remove pa11y as part of the entrypoint, the -h flag in command will be an error. I think we'll need to remove the CMD statement entirely. This approach leaves us with:
or
a bit redundant, but we can work with that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From sidebar chat, maybe CMD should be "bash -c"? |
||
|
||
CMD ["-h"] |
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.
By dropping these permissions, can the pptuser still take screenshots?
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.
I believe so, but I have not tested the screenshot capability myself. These permissions are now included in the starting dockerfile here: https://hub.docker.com/r/alekzonder/puppeteer/~/dockerfile/