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.

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

[Docker Images] Default Pulsar docker images to run as a non-root user #11269

Closed
michaeljmarshall opened this issue Jul 8, 2021 · 14 comments
Closed
Assignees
Labels
type/feature The PR added a new feature or issue requested a new feature
Milestone

Comments

@michaeljmarshall
Copy link
Member

michaeljmarshall commented Jul 8, 2021

Is your feature request related to a problem? Please describe.
Update the Pulsar docker images to run as a non root user by default.

Describe the solution you'd like
The right solution will meet the following requirements:

  • Pulsar docker images run as a non root user by default.
  • Pulsar docker images are able to run on OpenShift (a platform with stricter requirements than basic kubernetes)
    • Mainly, we'll need to make sure that the root group has sufficient permissions to write to all necessary directories/files.
    • Additionally, someone mentioned to me that they had trouble writing to a persistent file after restarting a Pulsar docker image on OpenShift. We should make sure the solution includes the ability to restart pulsar components successfully.
  • The Pulsar helm chart includes an easy way for end users to upgrade without any breaking changes.
    • We will likely be able to make use of the kubernetes feature that will chown persistent volumes to the configured fsGroup. However, I'm uncertain how this works on OpenShift, so I will need to research this a bit more.
  • The non root user and the root group only receive write permissions where necessary for each pulsar component to run.

Additionally, I think we should produce images that are minimal. Making minimal containers means that we won't include debug tools in them. This will make it harder to debug, but more importantly, it increases the security of the container by removing possible attack vectors. Note that it is trivial for developers to extend our docker images to add any debug tool they would like to use. This prevents us from having to curate and maintain a list of extra tools in the docker image.

Test criteria
There are several important test cases to cover.

  1. Make sure we're able to upgrade and downgrade pulsar components (mainly all of the ones utilizing persistent storage). Do this using the official Apache Pulsar helm chart.
  2. Test the image on OpenShift. I plan to use a local OpenShift cluster on my Mac, but perhaps someone would be able to validate our docker images on a real cluster.
  3. Be sure to include test cases for function workers. (They were one of the missed cases before that led [Issue 8751] Update Dockerfile for Pulsar and Dashboard to Create and Use pulsar User (nonroot user) #8796 to get reverted in Revert "[Issue 8751] Update Dockerfile for Pulsar and Dashboard to Cr… #10861).
  4. Test the chown feature for volumes in kubernetes. Make sure that managed kubernetes offerings from the major cloud providers actually support this chown feature. (I mention it because I'm not sure if this is an add-on or a native feature within the kubelet.)

Please let me know if you have any additional test cases you'd like to see covered.

Communication
Communicate this change on the mailing list to ensure that the community has time to test the new docker images before we begin the release process.

Describe alternatives you've considered
The main alternative here is whether or not the docker image should contain debugging tools. Otherwise, I think the community has generally accepted this feature, as is.

Additional context
I plan to contribute this fix later this month.

Relevant PRs and Issues: #10861, #8751, #8242,
#10815

@hpvd
Copy link

hpvd commented Jul 9, 2021

Perfect. Non root and making minimal images available will be a huge step in security.

The one thing I would like to think about is the future availability of full images in the actual manner including debugtools.

Imho one often start with pulsar or a new release version in a dev environment.

Having the possibility directly using ready to use dev images in addition to in sync production ones, would be at least neat and comfortable (and lower any hurdl)

@eolivelli
Copy link
Contributor

@hpvd
during the first iteration of the rootless docker image I sent this PR
#10815

that is in the direction of your comment

@hpvd
Copy link

hpvd commented Jul 9, 2021

@eolivelli
many thanks for pointing to this discussion!

In an ideal world we would have both:

  • one pure image without any tools
  • and one ready to start to use and dive in

Having these both offers, everyone can choose which image best meets their needs.
The big question: is there an efficient way to do so?

edit: in the end we can track download numbers of both ;-)

@eolivelli
Copy link
Contributor

having more images to maintain is very costly because ideally you would have to run tests and security auditing against all the released images.

I suggest to just add the basic tools to the standard images, and if you want to add more than you can create a custom image locally and create an issue/send a PR to add the missing tool to the next version of the standard public image

@hpvd
Copy link

hpvd commented Aug 5, 2021

since it's also a non-root (security) topic and is somehow related to this:
latest Kubernetes 1.22 support running all Kubernetes node components (including the kubelet, kube-proxy, and container runtime) as a non-root user.
https://kubernetes.io/blog/2021/08/04/kubernetes-1-22-release-announcement/

Should we open an issue to address this on places like setup-guides, testing environment etc.?

@michaeljmarshall
Copy link
Member Author

having more images to maintain is very costly because ideally you would have to run tests and security auditing against all the released images.

I don't know if this would be as costly as you're suggesting. If we build the base pulsar image and then extend that base image to build the dev image, the dev image will share the same initial layers. I am not familiar with docker image security scanning tools, but I would think that they should be optimized to not re-scan the same layer within some period of time. This deserves more research, though.

Should we open an issue to address this on places like setup-guides, testing environment etc.?

@hpvd - can you clarify what setup-guides and testing environments need updated documentation?

@michaeljmarshall
Copy link
Member Author

Note that using a distroless base image (https://github.com/GoogleContainerTools/distroless/tree/main/java) is not a viable option for pulsar 2.x because we use shell scripts to configure each component before executing the java command and changing that configuration paradigm would be a breaking change.

@lhotari
Copy link
Member

lhotari commented Feb 2, 2022

I created a separate enhancement request to provide a slim docker image: #14092

codelipenghui pushed a commit that referenced this issue Feb 17, 2022
)

Master Issue: #11269

### Motivation

In order to increase the overall security of our Pulsar docker images, they should default to run as the non-root user. While updating these permissions, I make sure to comply with the OpenShift spec so the docker image can run on that platform out of the box.

Once we finalize these changes, we will need to update the Apache Pulsar Helm chart to make sure that deployments take advantage of this feature. We'll use the `fsGroup` to make sure that k8s sets the appropriate file system permissions for the zookeeper, bookkeeper, and function pods.

### Modifications

* Default to run as UID 10000. As noted in the `Dockerfile`, this UID is arbitrary. No logic should rely on this id.
* Update filesystem permissions so that the group user has sufficient write permission. The group user is 0 (root).
* Remove unnecessary write access.
    * The `/pulsar/{conf,data,logs}` directories and their members must be writable by the root group. I don't know of any other directories that need to be written to. Note that the `bin/pulsar-admin` too creates a log file in the `/pulsar/logs` directory. Please let me know if there are any additional
    * Note also that the executable file permissions are already set in our git repo. Those permissions are inherited by the docker image when we run the `COPY` directive in the `Dockerfile`.
* There are no changes to the function worker in the k8s runtime. We do not need them because we already merged 04b5da0.
* Add note to `conf/bkenv.sh`, as it is a `.sh` script that is not executable (and doesn't need to be).
* Update test docker image and `supervisord` configuration.

Note: it's unclear to me how the OpenShift spec handles restarts. I know that the UID is arbitrary. It's possible that the umask needs to be switched from `022` to `002`. Setting the umask in the docker image does not persist for consumers of the image, so this would need to be set in a helm chart.

### Verifying this change

You can access a test image built with these changes here: `michaelmarshall/pulsar:2.10.0-SNAPSHOT`. I have already run some manual tests like `bin/pulsar standalone` in the container. I still need to deploy an actual cluster to verify that all of the unique components work correctly. Because we already merged 04b5da0, the upgrade scenarios are already simplified. If this change is in 2.10.0, that means 2.8 and 2.9 will be compatible for certain function worker upgrade scenarios.

I wrote test criteria in #11269. I'll need to follow up on that criteria using my newly build image. I should be able to look closer at this tomorrow.

We'll also need tests to pass, as I modified some tests with this PR.

### References

The following links were useful in understanding how to make these changes:

* https://engineering.bitnami.com/articles/running-non-root-containers-on-openshift.html
* https://cloud.redhat.com/blog/a-guide-to-openshift-and-uids

### Does this pull request potentially affect one of the following parts:

This PR updates our Docker images in a breaking way. It could result in bookkeepers, zookeepers, or functions with insufficient permissions. We will mitigate these permissions by updating the helm chart. These changes are easily overridden by extending the docker image. In k8s, you can use the pod's `securityContext` to override the user or group.
codelipenghui pushed a commit that referenced this issue Feb 17, 2022
)

Master Issue: #11269

### Motivation

In order to increase the overall security of our Pulsar docker images, they should default to run as the non-root user. While updating these permissions, I make sure to comply with the OpenShift spec so the docker image can run on that platform out of the box.

Once we finalize these changes, we will need to update the Apache Pulsar Helm chart to make sure that deployments take advantage of this feature. We'll use the `fsGroup` to make sure that k8s sets the appropriate file system permissions for the zookeeper, bookkeeper, and function pods.

### Modifications

* Default to run as UID 10000. As noted in the `Dockerfile`, this UID is arbitrary. No logic should rely on this id.
* Update filesystem permissions so that the group user has sufficient write permission. The group user is 0 (root).
* Remove unnecessary write access.
    * The `/pulsar/{conf,data,logs}` directories and their members must be writable by the root group. I don't know of any other directories that need to be written to. Note that the `bin/pulsar-admin` too creates a log file in the `/pulsar/logs` directory. Please let me know if there are any additional
    * Note also that the executable file permissions are already set in our git repo. Those permissions are inherited by the docker image when we run the `COPY` directive in the `Dockerfile`.
* There are no changes to the function worker in the k8s runtime. We do not need them because we already merged 04b5da0.
* Add note to `conf/bkenv.sh`, as it is a `.sh` script that is not executable (and doesn't need to be).
* Update test docker image and `supervisord` configuration.

Note: it's unclear to me how the OpenShift spec handles restarts. I know that the UID is arbitrary. It's possible that the umask needs to be switched from `022` to `002`. Setting the umask in the docker image does not persist for consumers of the image, so this would need to be set in a helm chart.

### Verifying this change

You can access a test image built with these changes here: `michaelmarshall/pulsar:2.10.0-SNAPSHOT`. I have already run some manual tests like `bin/pulsar standalone` in the container. I still need to deploy an actual cluster to verify that all of the unique components work correctly. Because we already merged 04b5da0, the upgrade scenarios are already simplified. If this change is in 2.10.0, that means 2.8 and 2.9 will be compatible for certain function worker upgrade scenarios.

I wrote test criteria in #11269. I'll need to follow up on that criteria using my newly build image. I should be able to look closer at this tomorrow.

We'll also need tests to pass, as I modified some tests with this PR.

### References

The following links were useful in understanding how to make these changes:

* https://engineering.bitnami.com/articles/running-non-root-containers-on-openshift.html
* https://cloud.redhat.com/blog/a-guide-to-openshift-and-uids

### Does this pull request potentially affect one of the following parts:

This PR updates our Docker images in a breaking way. It could result in bookkeepers, zookeepers, or functions with insufficient permissions. We will mitigate these permissions by updating the helm chart. These changes are easily overridden by extending the docker image. In k8s, you can use the pod's `securityContext` to override the user or group.

(cherry picked from commit f7f8619)
codelipenghui pushed a commit to codelipenghui/incubator-pulsar that referenced this issue Feb 17, 2022
…che#13376)

Master Issue: apache#11269

### Motivation

In order to increase the overall security of our Pulsar docker images, they should default to run as the non-root user. While updating these permissions, I make sure to comply with the OpenShift spec so the docker image can run on that platform out of the box.

Once we finalize these changes, we will need to update the Apache Pulsar Helm chart to make sure that deployments take advantage of this feature. We'll use the `fsGroup` to make sure that k8s sets the appropriate file system permissions for the zookeeper, bookkeeper, and function pods.

### Modifications

* Default to run as UID 10000. As noted in the `Dockerfile`, this UID is arbitrary. No logic should rely on this id.
* Update filesystem permissions so that the group user has sufficient write permission. The group user is 0 (root).
* Remove unnecessary write access.
    * The `/pulsar/{conf,data,logs}` directories and their members must be writable by the root group. I don't know of any other directories that need to be written to. Note that the `bin/pulsar-admin` too creates a log file in the `/pulsar/logs` directory. Please let me know if there are any additional
    * Note also that the executable file permissions are already set in our git repo. Those permissions are inherited by the docker image when we run the `COPY` directive in the `Dockerfile`.
* There are no changes to the function worker in the k8s runtime. We do not need them because we already merged apache@04b5da0.
* Add note to `conf/bkenv.sh`, as it is a `.sh` script that is not executable (and doesn't need to be).
* Update test docker image and `supervisord` configuration.

Note: it's unclear to me how the OpenShift spec handles restarts. I know that the UID is arbitrary. It's possible that the umask needs to be switched from `022` to `002`. Setting the umask in the docker image does not persist for consumers of the image, so this would need to be set in a helm chart.

### Verifying this change

You can access a test image built with these changes here: `michaelmarshall/pulsar:2.10.0-SNAPSHOT`. I have already run some manual tests like `bin/pulsar standalone` in the container. I still need to deploy an actual cluster to verify that all of the unique components work correctly. Because we already merged apache@04b5da0, the upgrade scenarios are already simplified. If this change is in 2.10.0, that means 2.8 and 2.9 will be compatible for certain function worker upgrade scenarios.

I wrote test criteria in apache#11269. I'll need to follow up on that criteria using my newly build image. I should be able to look closer at this tomorrow.

We'll also need tests to pass, as I modified some tests with this PR.

### References

The following links were useful in understanding how to make these changes:

* https://engineering.bitnami.com/articles/running-non-root-containers-on-openshift.html
* https://cloud.redhat.com/blog/a-guide-to-openshift-and-uids

### Does this pull request potentially affect one of the following parts:

This PR updates our Docker images in a breaking way. It could result in bookkeepers, zookeepers, or functions with insufficient permissions. We will mitigate these permissions by updating the helm chart. These changes are easily overridden by extending the docker image. In k8s, you can use the pod's `securityContext` to override the user or group.

(cherry picked from commit f7f8619)
@codelipenghui
Copy link
Contributor

The issue had no activity for 30 days, mark with Stale label.

@frankjkelly
Copy link
Contributor

bump - not urgent for us but without "Run as non-root" we are forced to maintain our own mirror of Pulsar and re-apply the changes to the Dockerfile to force it to run as non-root user so we can pass our docker image scans. (NOTE: we don't use functions).

@michaeljmarshall
Copy link
Member Author

@frankjkelly - the 2.10.0 release will be with a non-root docker image: #13376. I forgot to close this issue when we merged that PR--I'll close it now.

Note also that you can build any published/official version of Pulsar with this modified Dockerfile: https://gist.github.com/michaeljmarshall/d3b9aea4180c519836fd3cb7627bc137. You'll still need a fork of the project, but you won't rely on any locally built Pulsar components in the image. Note that functions won't work correctly before 2.8.

@frankjkelly
Copy link
Contributor

Awesome that's great news @michaeljmarshall yeah we have our own Mirror where we've done manual changes to the Dockerfile but are excited to have a version coming where we won't have to do that anymore. Thanks for your leadership on this!

Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this issue Apr 20, 2022
…che#13376)

Master Issue: apache#11269

### Motivation

In order to increase the overall security of our Pulsar docker images, they should default to run as the non-root user. While updating these permissions, I make sure to comply with the OpenShift spec so the docker image can run on that platform out of the box.

Once we finalize these changes, we will need to update the Apache Pulsar Helm chart to make sure that deployments take advantage of this feature. We'll use the `fsGroup` to make sure that k8s sets the appropriate file system permissions for the zookeeper, bookkeeper, and function pods.

### Modifications

* Default to run as UID 10000. As noted in the `Dockerfile`, this UID is arbitrary. No logic should rely on this id.
* Update filesystem permissions so that the group user has sufficient write permission. The group user is 0 (root).
* Remove unnecessary write access.
    * The `/pulsar/{conf,data,logs}` directories and their members must be writable by the root group. I don't know of any other directories that need to be written to. Note that the `bin/pulsar-admin` too creates a log file in the `/pulsar/logs` directory. Please let me know if there are any additional
    * Note also that the executable file permissions are already set in our git repo. Those permissions are inherited by the docker image when we run the `COPY` directive in the `Dockerfile`.
* There are no changes to the function worker in the k8s runtime. We do not need them because we already merged apache@04b5da0.
* Add note to `conf/bkenv.sh`, as it is a `.sh` script that is not executable (and doesn't need to be).
* Update test docker image and `supervisord` configuration.

Note: it's unclear to me how the OpenShift spec handles restarts. I know that the UID is arbitrary. It's possible that the umask needs to be switched from `022` to `002`. Setting the umask in the docker image does not persist for consumers of the image, so this would need to be set in a helm chart.

### Verifying this change

You can access a test image built with these changes here: `michaelmarshall/pulsar:2.10.0-SNAPSHOT`. I have already run some manual tests like `bin/pulsar standalone` in the container. I still need to deploy an actual cluster to verify that all of the unique components work correctly. Because we already merged apache@04b5da0, the upgrade scenarios are already simplified. If this change is in 2.10.0, that means 2.8 and 2.9 will be compatible for certain function worker upgrade scenarios.

I wrote test criteria in apache#11269. I'll need to follow up on that criteria using my newly build image. I should be able to look closer at this tomorrow.

We'll also need tests to pass, as I modified some tests with this PR.

### References

The following links were useful in understanding how to make these changes:

* https://engineering.bitnami.com/articles/running-non-root-containers-on-openshift.html
* https://cloud.redhat.com/blog/a-guide-to-openshift-and-uids

### Does this pull request potentially affect one of the following parts:

This PR updates our Docker images in a breaking way. It could result in bookkeepers, zookeepers, or functions with insufficient permissions. We will mitigate these permissions by updating the helm chart. These changes are easily overridden by extending the docker image. In k8s, you can use the pod's `securityContext` to override the user or group.
@michaeljmarshall
Copy link
Member Author

The last requirement for completing non-root feature support is updating the helm chart. Here is the PR to make the non-root docker image fully supported in the Pulsar ecosystem: apache/pulsar-helm-chart#266.

michaeljmarshall added a commit to apache/pulsar-helm-chart that referenced this issue Jun 14, 2022
#266)

Master Issue: apache/pulsar#11269

### Motivation

Apache Pulsar's docker images for 2.10.0 and above are non-root by default. In order to ensure there is a safe upgrade path, we need to expose the `securityContext` for the Bookkeeper and Zookeeper StatefulSets. Here is the relevant k8s documentation on this k8s feature: https://kubernetes.io/docs/tasks/configure-pod-container/security-context.

Once released, all deployments using the default `values.yaml` configuration for the `securityContext` will pay a one time penalty on upgrade where the kubelet will recursively chown files to be root group writable. It's possible to temporarily avoid this penalty by setting `securityContext: {}`.

### Modifications

* Add config blocks for the `bookkeeper.securityContext` and `zookeeper.securityContext`.
* Default to `fsGroup: 0`. This is already the default group id in the docker image, and the docker image assumes the user has root group permission.
* Default to `fsGroupChangePolicy: "OnRootMismatch"`. This configuration will work for all deployments where the user id is stable. If the user id switches between restarts, like it does in OpenShift, please set to `Always`.
* Remove gc configuration writing to directory that the user lacks permission. (Perhaps we want to write to `/pulsar/log/bookie-gc.log`?) 
* Add documentation to the README.

### Verifying this change

I first attempted verification of this change with minikube. It did not work because minikube uses hostPath volumes by default. I then tested on EKS v1.21.9-eks-0d102a7. I tested by deploying the current, latest version of the helm chart (2.9.3) and then upgrading to this PR's version of the helm chart along with using the 2.10.0 docker image. I also tested upgrading from a default version 

Test 1 is a plain upgrade using the default 2.9.3 version of the chart, then upgrading to this PR's version of the chart with the modification to use the 2.10.0 docker images. It worked as expected.

```bash
$ helm install test apache/pulsar
$ # Wait for chart to deploy, then run the following, which uses Pulsar version 2.10.0:
$  helm upgrade test -f charts/pulsar/values.yaml charts/pulsar/
```

Test 2 is a plain upgrade using the default 2.9.3 version of the chart, then an upgrade to this PR's version of the chart, then an upgrade to this PR's version of the chart using 2.10.0 docker images. There is a minor error described in the `README.md`. The solution is to chown the bookie's data directory.

```bash
$ helm install test apache/pulsar
$ # Wait for chart to deploy, then run the following, which uses Pulsar version 2.9.2:
$  helm upgrade test -f charts/pulsar/values.yaml charts/pulsar/
$ # Upgrade using Pulsar version 2.10.0
$  helm upgrade test -f charts/pulsar/values.yaml charts/pulsar/
```

### GC Logging

In my testing, I ran into the following errors when using `-Xlog:gc:/var/log/bookie-gc.log`:

```
pulsar-bookkeeper-verify-clusterid [0.008s] Error opening log file '/var/log/bookie-gc.log': Permission denied
pulsar-bookkeeper-verify-clusterid [0.008s] Initialization of output 'file=/var/log/bookie-gc.log' using options '(null)' failed.
pulsar-bookkeeper-verify-clusterid [0.005s] Error opening log file '/var/log/bookie-gc.log': Permission denied
pulsar-bookkeeper-verify-clusterid [0.006s] Initialization of output 'file=/var/log/bookie-gc.log' using options '(null)' failed.
pulsar-bookkeeper-verify-clusterid Invalid -Xlog option '-Xlog:gc:/var/log/bookie-gc.log', see error log for details.
pulsar-bookkeeper-verify-clusterid Error: Could not create the Java Virtual Machine.
pulsar-bookkeeper-verify-clusterid Error: A fatal exception has occurred. Program will exit.
pulsar-bookkeeper-verify-clusterid Invalid -Xlog option '-Xlog:gc:/var/log/bookie-gc.log', see error log for details.
pulsar-bookkeeper-verify-clusterid Error: Could not create the Java Virtual Machine.
pulsar-bookkeeper-verify-clusterid Error: A fatal exception has occurred. Program will exit.
```

I resolved the error by removing the setting.

### OpenShift Observations

I wanted to seamlessly support OpenShift, so I investigated using configuring the bookkeeper and zookeeper process with `umask 002` so that they would create files and directories that are group writable (OpenShift has a stable group id, but gives the process a random user id). That worked for most tools when switching the user id, but not for RocksDB, which creates a lock file at `/pulsar/data/bookkeeper/ledgers/current/ledgers/LOCK` with the permission `0644` ignoring the umask. Here is the relevant error:

```
2022-05-14T03:45:06,903+0000  ERROR org.apache.bookkeeper.server.Main - Failed to build bookie server
java.io.IOException: Error open RocksDB database
    at org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageRocksDB.<init>(KeyValueStorageRocksDB.java:199) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageRocksDB.<init>(KeyValueStorageRocksDB.java:88) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageRocksDB.lambda$static$0(KeyValueStorageRocksDB.java:62) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.bookie.storage.ldb.LedgerMetadataIndex.<init>(LedgerMetadataIndex.java:68) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.bookie.storage.ldb.SingleDirectoryDbLedgerStorage.<init>(SingleDirectoryDbLedgerStorage.java:169) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.bookie.storage.ldb.DbLedgerStorage.newSingleDirectoryDbLedgerStorage(DbLedgerStorage.java:150) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.bookie.storage.ldb.DbLedgerStorage.initialize(DbLedgerStorage.java:129) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.bookie.Bookie.<init>(Bookie.java:818) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.proto.BookieServer.newBookie(BookieServer.java:152) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.proto.BookieServer.<init>(BookieServer.java:120) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.server.service.BookieService.<init>(BookieService.java:52) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.server.Main.buildBookieServer(Main.java:304) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.server.Main.doMain(Main.java:226) [org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    at org.apache.bookkeeper.server.Main.main(Main.java:208) [org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
Caused by: org.rocksdb.RocksDBException: while open a file for lock: /pulsar/data/bookkeeper/ledgers/current/ledgers/LOCK: Permission denied
    at org.rocksdb.RocksDB.open(Native Method) ~[org.rocksdb-rocksdbjni-6.10.2.jar:?]
    at org.rocksdb.RocksDB.open(RocksDB.java:239) ~[org.rocksdb-rocksdbjni-6.10.2.jar:?]
    at org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageRocksDB.<init>(KeyValueStorageRocksDB.java:196) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
    ... 13 more
```

As such, in order to support OpenShift, I exposed the `fsGroupChangePolicy`, which allows for OpenShift support, but not necessarily _seamless_ support.
@hpvd
Copy link

hpvd commented May 8, 2023

opened an idea in pulsar discussion for the above mentioned distroless topic:
plz comment/vote
#20253

lhotari pushed a commit to apache/pulsar-sql that referenced this issue Oct 18, 2024
…376)

Master Issue: apache/pulsar#11269

### Motivation

In order to increase the overall security of our Pulsar docker images, they should default to run as the non-root user. While updating these permissions, I make sure to comply with the OpenShift spec so the docker image can run on that platform out of the box.

Once we finalize these changes, we will need to update the Apache Pulsar Helm chart to make sure that deployments take advantage of this feature. We'll use the `fsGroup` to make sure that k8s sets the appropriate file system permissions for the zookeeper, bookkeeper, and function pods.

### Modifications

* Default to run as UID 10000. As noted in the `Dockerfile`, this UID is arbitrary. No logic should rely on this id.
* Update filesystem permissions so that the group user has sufficient write permission. The group user is 0 (root).
* Remove unnecessary write access.
    * The `/pulsar/{conf,data,logs}` directories and their members must be writable by the root group. I don't know of any other directories that need to be written to. Note that the `bin/pulsar-admin` too creates a log file in the `/pulsar/logs` directory. Please let me know if there are any additional
    * Note also that the executable file permissions are already set in our git repo. Those permissions are inherited by the docker image when we run the `COPY` directive in the `Dockerfile`.
* There are no changes to the function worker in the k8s runtime. We do not need them because we already merged apache/pulsar@04b5da0.
* Add note to `conf/bkenv.sh`, as it is a `.sh` script that is not executable (and doesn't need to be).
* Update test docker image and `supervisord` configuration.

Note: it's unclear to me how the OpenShift spec handles restarts. I know that the UID is arbitrary. It's possible that the umask needs to be switched from `022` to `002`. Setting the umask in the docker image does not persist for consumers of the image, so this would need to be set in a helm chart.

### Verifying this change

You can access a test image built with these changes here: `michaelmarshall/pulsar:2.10.0-SNAPSHOT`. I have already run some manual tests like `bin/pulsar standalone` in the container. I still need to deploy an actual cluster to verify that all of the unique components work correctly. Because we already merged apache/pulsar@04b5da0, the upgrade scenarios are already simplified. If this change is in 2.10.0, that means 2.8 and 2.9 will be compatible for certain function worker upgrade scenarios.

I wrote test criteria in apache/pulsar#11269. I'll need to follow up on that criteria using my newly build image. I should be able to look closer at this tomorrow.

We'll also need tests to pass, as I modified some tests with this PR.

### References

The following links were useful in understanding how to make these changes:

* https://engineering.bitnami.com/articles/running-non-root-containers-on-openshift.html
* https://cloud.redhat.com/blog/a-guide-to-openshift-and-uids

### Does this pull request potentially affect one of the following parts:

This PR updates our Docker images in a breaking way. It could result in bookkeepers, zookeepers, or functions with insufficient permissions. We will mitigate these permissions by updating the helm chart. These changes are easily overridden by extending the docker image. In k8s, you can use the pod's `securityContext` to override the user or group.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

No branches or pull requests

6 participants