You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/10/29 17:09:10 UTC
[GitHub] [airflow] pgvishnuram opened a new pull request #19315: pgbouncer exporter version update
pgvishnuram opened a new pull request #19315:
URL: https://github.com/apache/airflow/pull/19315
Problem:
* pgbouncer_exporter run as non root
* alpine version 3.13 vulnerabilities
*
How does this PR fix the problem above:
* this commit adds a non root user and allows the pgbouncer exporter image to as non root user
* migrating alpine image to latest version
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#discussion_r739628702
##########
File path: chart/dockerfiles/pgbouncer-exporter/build_and_push.sh
##########
@@ -21,13 +21,13 @@ readonly DOCKERHUB_USER
DOCKERHUB_REPO=${DOCKERHUB_REPO:="airflow"}
readonly DOCKERHUB_REPO
-PGBOUNCER_EXPORTER_VERSION="0.5.0"
+PGBOUNCER_EXPORTER_VERSION="0.13.0"
readonly PGBOUNCER_EXPORTER_VERSION
-AIRFLOW_PGBOUNCER_EXPORTER_VERSION="2021.04.28"
+AIRFLOW_PGBOUNCER_EXPORTER_VERSION="2021.10.29"
readonly AIRFLOW_PGBOUNCER_EXPORTER_VERSION
-EXPECTED_GO_VERSION="1.15.1"
+EXPECTED_GO_VERSION="1.17"
readonly EXPECTED_GO_VERSION
COMMIT_SHA=$(git rev-parse HEAD)
Review comment:
I created a PR: https://github.com/apache/airflow/pull/19327
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#discussion_r739676311
##########
File path: chart/dockerfiles/pgbouncer-exporter/Dockerfile
##########
@@ -28,13 +28,19 @@ LABEL org.apache.airflow.airflow-pgbouncer-exporter.version="${AIRFLOW_PGBOUNCER
LABEL org.apache.airflow.commit-sha="${COMMIT_SHA}"
LABEL maintainer="Apache Airflow Community <de...@airflow.apache.org>"
+# Add a non root user
+RUN addgroup -S pgbouncer_exporter \
Review comment:
If we use nobody, it makes a bit simpler.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#discussion_r739676132
##########
File path: chart/dockerfiles/pgbouncer-exporter/Dockerfile
##########
@@ -28,13 +28,19 @@ LABEL org.apache.airflow.airflow-pgbouncer-exporter.version="${AIRFLOW_PGBOUNCER
LABEL org.apache.airflow.commit-sha="${COMMIT_SHA}"
LABEL maintainer="Apache Airflow Community <de...@airflow.apache.org>"
+# Add a non root user
+RUN addgroup -S pgbouncer_exporter \
Review comment:
How about using user "nobody" (Like in the pgbouncer itself). I think this case is not the same as airflow - in airflow often people want to use different user or use the "arbitrary user id" to avoid some potential "user' clashes between containers running on the same machine and they evn might want to choose a specific ID because they can have volumes mounted with specific users etc. So it makes sense to support the "arbitrary user id". We also created a home directory there because airflow workers might spawn pretty much arbitrary operators (thus any external libraries/binaries) and many of those extarnal binaries/librraries might rely on the ${HOME} directory set and present (and writeable) - for example when they do any caching, they often do that in the ${HOME} dir.
But `pgbouncer_exporter` is really simple, "standalone" binary tool that is rather simple - it connects to pgbouncer, queries it for stats and exposes to prometheus via non-priviledged port (9127 which is > 1024) - so there is no need for any special priviledges, also it is unlikely it needs HOME directory (we can check that likely).
This is nobody user:
```
nobody:x:65534:65534:nobody:/nonexistent:/usr/sbin/nologin
```
WDYT?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] closed pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #19315:
URL: https://github.com/apache/airflow/pull/19315
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#discussion_r739585387
##########
File path: chart/dockerfiles/pgbouncer-exporter/build_and_push.sh
##########
@@ -21,13 +21,13 @@ readonly DOCKERHUB_USER
DOCKERHUB_REPO=${DOCKERHUB_REPO:="airflow"}
readonly DOCKERHUB_REPO
-PGBOUNCER_EXPORTER_VERSION="0.5.0"
+PGBOUNCER_EXPORTER_VERSION="0.13.0"
readonly PGBOUNCER_EXPORTER_VERSION
-AIRFLOW_PGBOUNCER_EXPORTER_VERSION="2021.04.28"
+AIRFLOW_PGBOUNCER_EXPORTER_VERSION="2021.10.29"
readonly AIRFLOW_PGBOUNCER_EXPORTER_VERSION
-EXPECTED_GO_VERSION="1.15.1"
+EXPECTED_GO_VERSION="1.17"
readonly EXPECTED_GO_VERSION
COMMIT_SHA=$(git rev-parse HEAD)
Review comment:
Not PR related to that, but why not use [a multi-stage build](https://docs.docker.com/develop/develop-images/multistage-build/) if we need two images to build? This will simplify the build as the entire build process will be in one Dockerfile and the bash scripts will be much simpler.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#discussion_r739585387
##########
File path: chart/dockerfiles/pgbouncer-exporter/build_and_push.sh
##########
@@ -21,13 +21,13 @@ readonly DOCKERHUB_USER
DOCKERHUB_REPO=${DOCKERHUB_REPO:="airflow"}
readonly DOCKERHUB_REPO
-PGBOUNCER_EXPORTER_VERSION="0.5.0"
+PGBOUNCER_EXPORTER_VERSION="0.13.0"
readonly PGBOUNCER_EXPORTER_VERSION
-AIRFLOW_PGBOUNCER_EXPORTER_VERSION="2021.04.28"
+AIRFLOW_PGBOUNCER_EXPORTER_VERSION="2021.10.29"
readonly AIRFLOW_PGBOUNCER_EXPORTER_VERSION
-EXPECTED_GO_VERSION="1.15.1"
+EXPECTED_GO_VERSION="1.17"
readonly EXPECTED_GO_VERSION
COMMIT_SHA=$(git rev-parse HEAD)
Review comment:
Not PR related to that, but why not use [a multi-stage build](https://docs.docker.com/develop/develop-images/multistage-build/) if we need two images to build?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#issuecomment-997303081
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pgvishnuram commented on a change in pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
pgvishnuram commented on a change in pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#discussion_r739682056
##########
File path: chart/dockerfiles/pgbouncer-exporter/Dockerfile
##########
@@ -28,13 +28,19 @@ LABEL org.apache.airflow.airflow-pgbouncer-exporter.version="${AIRFLOW_PGBOUNCER
LABEL org.apache.airflow.commit-sha="${COMMIT_SHA}"
LABEL maintainer="Apache Airflow Community <de...@airflow.apache.org>"
+# Add a non root user
+RUN addgroup -S pgbouncer_exporter \
Review comment:
yeah both approaches are right and USER nobody option is simpler this service just exposes metrics on http endpoint. Thanks @potiuk
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on a change in pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#discussion_r740229505
##########
File path: chart/dockerfiles/pgbouncer-exporter/Dockerfile
##########
@@ -28,13 +28,19 @@ LABEL org.apache.airflow.airflow-pgbouncer-exporter.version="${AIRFLOW_PGBOUNCER
LABEL org.apache.airflow.commit-sha="${COMMIT_SHA}"
LABEL maintainer="Apache Airflow Community <de...@airflow.apache.org>"
+# Add a non root user
+RUN addgroup -S pgbouncer_exporter \
Review comment:
Can you make that change @pgvishnuram so we can merge this PR
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#issuecomment-958597467
ping @pgvishnuram Can you rebase the PR on the latest main and address the comment please
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pgvishnuram commented on a change in pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
pgvishnuram commented on a change in pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#discussion_r739653994
##########
File path: chart/dockerfiles/pgbouncer-exporter/Dockerfile
##########
@@ -28,13 +28,19 @@ LABEL org.apache.airflow.airflow-pgbouncer-exporter.version="${AIRFLOW_PGBOUNCER
LABEL org.apache.airflow.commit-sha="${COMMIT_SHA}"
LABEL maintainer="Apache Airflow Community <de...@airflow.apache.org>"
+# Add a non root user
+RUN addgroup -S pgbouncer_exporter \
Review comment:
we are defining userid's in securityContext: , if the securityContext is left black or id's are in range of openshift we wont have any issues.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#discussion_r739633659
##########
File path: chart/dockerfiles/pgbouncer-exporter/Dockerfile
##########
@@ -28,13 +28,19 @@ LABEL org.apache.airflow.airflow-pgbouncer-exporter.version="${AIRFLOW_PGBOUNCER
LABEL org.apache.airflow.commit-sha="${COMMIT_SHA}"
LABEL maintainer="Apache Airflow Community <de...@airflow.apache.org>"
+# Add a non root user
+RUN addgroup -S pgbouncer_exporter \
Review comment:
What do you think about support arbitrary user ids? https://docs.openshift.com/container-platform/4.9/openshift_images/create-images.html#images-create-guide-openshift_create-images
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#issuecomment-958597467
ping @pgvishnuram Can you rebase the PR on the latest main and address the comment please
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #19315: pgbouncer exporter version update
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #19315:
URL: https://github.com/apache/airflow/pull/19315#issuecomment-958597467
ping @pgvishnuram Can you rebase the PR on the latest main and address the comment please
--
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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org