[GitHub] [pulsar] michaeljmarshall commented on a change in pull request #8796: [Issue 8751] Update Dockerfile for Pulsar and Dashboard to Create and Use pulsar User (nonroot user)

[GitBox <gi...@apache.org>]

michaeljmarshall commented on a change in pull request #8796:
URL: https://github.com/apache/pulsar/pull/8796#discussion_r539069260



##########
File path: docker/pulsar/Dockerfile
##########
@@ -53,21 +55,25 @@ RUN python3.7 get-pip.py
 
 RUN update-alternatives --install /usr/bin/python python /usr/bin/python3 10
 
-ADD target/python-client/ /pulsar/pulsar-client
-ADD target/cpp-client/ /pulsar/cpp-client
+ADD --chown=pulsar:pulsar target/python-client/ /pulsar/pulsar-client
+ADD --chown=pulsar:pulsar target/cpp-client/ /pulsar/cpp-client
 RUN echo networkaddress.cache.ttl=1 >> $JAVA_HOME/jre/lib/security/java.security
 RUN apt-get update \
      && apt install -y /pulsar/cpp-client/*.deb \
      && apt-get clean \
+     && chown -R pulsar:pulsar /pulsar/cpp-client/
      && rm -rf /var/lib/apt/lists/*
 
+# Start using the pulsar user to ensure container defaults to run as non root user
+USER pulsar
+
+# Directories will have correct permission because we switched to the pulsar user
+RUN mkdir /pulsar/conf /pulsar/data

Review comment:
       Kubernetes will not change the file system permissions by default. If the volume is owned by `root`, the container will also view the mounted volume as owned by `root`, which would be a problem for the container produced from this docker image.
   
   In Linux, the mounted volume will need to be writable by user 1000. I'm not certain how Docker users map to host users in other operating systems.
   
   [Here](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#configure-volume-permission-and-ownership-change-policy-for-pods) is the Kubernetes documentation on permissions and pods. It looks like there has been active development in the mounted volume permission domain.
   
   I'm not sure how the pulsar project handles this type of change. Given that administrators of pulsar clusters should have access to the nodes running pulsar, it seems reasonable to me that those administrators could change the ownership of the relevant directories before upgrading to the next version of pulsar. The current containers would have root permissions, and would be able to write to directories/files owned by user 1000, so a rolling upgrade could still be achieved.
   
   Is there any precedent for this type of change? I would really like to get this into the 2.8.0 release, so that we can increase the security of pulsar containers and I can stop maintaining my own fork of the project. However, I also recognize the importance of carefully releasing major changes to a project.
   
   As I mentioned in my initial PR description, it might be relevant to provide migration documentation, but I'm not sure where to put it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org