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 2020/08/27 11:33:33 UTC
[GitHub] [airflow] gardnerdev opened a new pull request #10600: updating redis to bitnami distribution due to security limitation
gardnerdev opened a new pull request #10600:
URL: https://github.com/apache/airflow/pull/10600
Allows to run as non root
----------------------------------------------------------------
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
[GitHub] [airflow] schnie commented on pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
schnie commented on pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#issuecomment-693412668
This looks fine to me
----------------------------------------------------------------
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
[GitHub] [airflow] XD-DENG commented on a change in pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#discussion_r479107417
##########
File path: chart/values.yaml
##########
@@ -79,8 +79,8 @@ images:
tag: 0.11.0
pullPolicy: IfNotPresent
redis:
- repository: redis
- tag: 6-buster
+ repository: bitnami/redis
Review comment:
Personally I still do not think it's a good idea to deviate away from the official Docker image.
Regarding the necessarily to run containers as non-root in K8S, I think something like K8S [Security Context](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/) already suffices.
----------------------------------------------------------------
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
[GitHub] [airflow] XD-DENG commented on a change in pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#discussion_r478699640
##########
File path: chart/requirements.yaml
##########
@@ -14,9 +14,8 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-
dependencies:
- name: postgresql
- version: 6.3.12
- repository: "@stable"
+ version: 9.1.2
+ repository: "https://charts.bitnami.com/bitnami"
Review comment:
You made the same changes in two PRs (another is https://github.com/apache/airflow/pull/10598).
As shared, more clarifications may be needed to justify the changes ( at least to me). Meanwhile, please clean-up the two PRs.
----------------------------------------------------------------
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
[GitHub] [airflow] XD-DENG commented on a change in pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#discussion_r478696241
##########
File path: chart/values.yaml
##########
@@ -79,8 +79,8 @@ images:
tag: 0.11.0
pullPolicy: IfNotPresent
redis:
- repository: redis
- tag: 6-buster
+ repository: bitnami/redis
Review comment:
@gardnerdev may you provided more detailed clarification for this change? Otherwise it doesn't seem a good idea to deviate away from the default "official" image.
----------------------------------------------------------------
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
[GitHub] [airflow] gardnerdev commented on pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
gardnerdev commented on pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#issuecomment-683978963
@potiuk thank you! "create_user_job" is here by mistake. I made pull request here: #10291 . I'll try to do separate PR. Thank you
----------------------------------------------------------------
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
[GitHub] [airflow] gardnerdev commented on a change in pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
gardnerdev commented on a change in pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#discussion_r478987787
##########
File path: chart/values.yaml
##########
@@ -79,8 +79,8 @@ images:
tag: 0.11.0
pullPolicy: IfNotPresent
redis:
- repository: redis
- tag: 6-buster
+ repository: bitnami/redis
Review comment:
Sure, bitnami/redis is working as non-root process which is following best practices. When you are sharing cluster (namespace e.g) you are not allowed to run root-processes. Please, refer to https://docs.bitnami.com/tutorials/work-with-non-root-containers/
----------------------------------------------------------------
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
[GitHub] [airflow] ryw commented on pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
ryw commented on pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#issuecomment-683795786
@schnie curious to get your take on this one?
----------------------------------------------------------------
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
[GitHub] [airflow] XD-DENG commented on a change in pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#discussion_r479107417
##########
File path: chart/values.yaml
##########
@@ -79,8 +79,8 @@ images:
tag: 0.11.0
pullPolicy: IfNotPresent
redis:
- repository: redis
- tag: 6-buster
+ repository: bitnami/redis
Review comment:
Personally I still don't think it's a good idea to deviate away from the official Docker image.
Regarding the necessarily to run containers as non-root in K8S, I think something like K8S [Security Context](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/) already suffices.
----------------------------------------------------------------
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
[GitHub] [airflow] XD-DENG commented on a change in pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#discussion_r478696241
##########
File path: chart/values.yaml
##########
@@ -79,8 +79,8 @@ images:
tag: 0.11.0
pullPolicy: IfNotPresent
redis:
- repository: redis
- tag: 6-buster
+ repository: bitnami/redis
Review comment:
@gardnerdev may you provide more detailed clarification for this change? Otherwise it doesn't seem a good idea to deviate away from the default "official" image.
----------------------------------------------------------------
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
[GitHub] [airflow] gardnerdev commented on a change in pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
gardnerdev commented on a change in pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#discussion_r479109706
##########
File path: chart/values.yaml
##########
@@ -79,8 +79,8 @@ images:
tag: 0.11.0
pullPolicy: IfNotPresent
redis:
- repository: redis
- tag: 6-buster
+ repository: bitnami/redis
Review comment:
Not really, security context will not work, because of lack of specific users. Bitnami images are widely used and batter than "stable" imo
----------------------------------------------------------------
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
[GitHub] [airflow] potiuk commented on a change in pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#discussion_r480323012
##########
File path: chart/values.yaml
##########
@@ -79,8 +79,8 @@ images:
tag: 0.11.0
pullPolicy: IfNotPresent
redis:
- repository: redis
- tag: 6-buster
+ repository: bitnami/redis
Review comment:
The official docker image already runs as non-root user by default (50000 - Airflow) - precisely as explained by @gardnerdev.
It also has another feature - the image is OpenShift compatible: https://github.com/apache/airflow/issues/9248 which means that it can essentially run as "random" user but the user has to belong to "root" group (this is how OpenShift runs the images).
I think if the bitnami image is also OpenShift compatible - I'd definitely welcome that!
----------------------------------------------------------------
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
[GitHub] [airflow] XD-DENG commented on a change in pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
XD-DENG commented on a change in pull request #10600:
URL: https://github.com/apache/airflow/pull/10600#discussion_r479113205
##########
File path: chart/values.yaml
##########
@@ -79,8 +79,8 @@ images:
tag: 0.11.0
pullPolicy: IfNotPresent
redis:
- repository: redis
- tag: 6-buster
+ repository: bitnami/redis
Review comment:
I would leave it for comments from other committers as well first. But in my view I would give a -1 for now.
----------------------------------------------------------------
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
[GitHub] [airflow] stale[bot] closed pull request #10600: updating redis to bitnami distribution due to security limitation
Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #10600:
URL: https://github.com/apache/airflow/pull/10600
----------------------------------------------------------------
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