-
Notifications
You must be signed in to change notification settings - Fork 485
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
Fix Dockerfile to have the proper permissions in directories #4967
Conversation
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
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've seen that several test suites currently create workload entries using the selector "unix:uid:0"
:
- ghostunnel-federation
- join-token
- envoy-sds-v3-spiffe-auth
Should we also update these to use a "unix:uid:1000"
?
Thank you for the observation, @maxlambrecht. Yes, I've already added a commit to have that fixed. |
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
…on test Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Dockerfile
Outdated
CMD [] | ||
COPY --link --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ | ||
COPY --link --from=builder --chown=${spireuid}:${spiregid} --chmod=777 /empty-dir /opt/spire |
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.
Could we use separate stages for creating the directories and then apply the permissions when they are copied to the final images? Something like this:
# Setting up SPIRE Server directories
FROM alpine as prep-server
RUN mkdir -p /opt/spire/bin \
/etc/spire/server \
/run/spire/server/private \
/tmp/spire-server/private \
/var/lib/spire/server
# Setting up SPIRE Agent directories
FROM alpine as prep-agent
RUN mkdir -p /opt/spire/bin \
/etc/spire/agent \
/run/spire/agent/public \
/tmp/spire-agent/public \
/var/lib/spire/agent
# SPIRE Server
FROM spire-base AS spire-server
ARG spireuid=1000
ARG spiregid=1000
USER ${spireuid}:${spiregid}
COPY --link --from=prep-server --chown=${spireuid}:${spiregid} --chmod=755 / /
COPY --link --from=builder --chown=${spireuid}:${spiregid} --chmod=755 /spire/bin/static/spire-server /opt/spire/bin/
ENTRYPOINT ["/opt/spire/bin/spire-server", "run"]
# Similar configuration for SPIRE Agent
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.
Also spire-base
could be just:
FROM --platform=${BUILDPLATFORM} scratch AS spire-base
COPY --link --from=builder --chown=root:root --chmod=755 /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/
WORKDIR /opt/spire
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.
Thank you @maxlambrecht for the suggestions, I think that they make sense.
I've updated the Dockerfile accordingly.
…ration test to be able to access the Docker daemon socket Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Dockerfile
Outdated
FROM alpine as prep-server | ||
RUN mkdir -p /opt/spire/bin \ | ||
/etc/spire/server \ | ||
/run/spire/server/private \ | ||
/tmp/spire-server/private \ | ||
/var/lib/spire/server |
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 noticed that my earlier suggestion would include the entire alpine
filesystem in our final image, which isn't ideal. Here's an optimized approach:
FROM alpine as prep-server
RUN mkdir -p /spire/opt/spire/bin \
/spire/etc/spire/server \
/spire/run/spire/server/private \
/spire/tmp/spire-server/private \
/spire/var/lib/spire/server
We should only copy the /spire
directory:
COPY --link --from=prep-server --chown=${spireuid}:${spiregid} --chmod=755 /spire /
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.
Oh right. Yeah, I thought about that at some point and then didn't check it properly.
Thanks again for the suggestions!
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
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.
LGTM!
test/integration/suites/oidc-discovery-provider/docker-compose.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
@@ -111,6 +111,10 @@ spec: | |||
hostNetwork: true | |||
dnsPolicy: ClusterFirstWithHostNet | |||
serviceAccountName: spire-agent | |||
# Make sure that we can create the directory for the socket in the host |
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.
Why does only this one test need to run as root to create the socket directory? Could this be avoided with a config change in the test?
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.
This is needed because we use a hostPath volume to share the socket for the Workload API. I've moved the securityContext
field to affect the spire-agent container only.
Closing this for now. We can re-open and merge after 1.9.3 ships. |
Signed-off-by: Agustín Martínez Fayó <amartinezfayo@gmail.com>
In the Dockerfile, proper permissions in directories were intended to be set in the
install
commands that are run when setting up the directories. But those directories are then copied using theCOPY
instruction.With the
COPY
instruction, all files and directories copied from the build context are created with a UID and GID of 0 unless the--chown
flag specifies a given username, groupname, or UID/GID combination to request specific ownership of the copied content.This PR fixes the Dockerfile to use
--chown
and--chmod
flags in theCOPY
instructions to set the permissions.Fixes #4903.
Supersedes #4871.