You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "damondouglas (via GitHub)" <gi...@apache.org> on 2023/03/03 19:37:30 UTC
[GitHub] [beam] damondouglas commented on a diff in pull request #25508: [Playground] Playground network policy+MiTM proxy
damondouglas commented on code in PR #25508:
URL: https://github.com/apache/beam/pull/25508#discussion_r1124912067
##########
playground/backend/containers/go/Dockerfile:
##########
@@ -58,17 +58,6 @@ ENV PROPERTY_PATH=/opt/playground/backend/properties.yaml
## Copy build result
COPY src/configs /opt/playground/backend/configs/
Review Comment:
Why are "configs" embeded in a Dockerfile? Given the kubernetes context? Why not rely on a ConfigMap instead of coupling the binary with configuration?
##########
playground/backend/containers/go/Dockerfile:
##########
@@ -58,17 +58,6 @@ ENV PROPERTY_PATH=/opt/playground/backend/properties.yaml
## Copy build result
COPY src/configs /opt/playground/backend/configs/
-# Install mitmpoxy
-RUN mkdir /opt/mitmproxy &&\
- cd /opt/mitmproxy &&\
- wget -q https://snapshots.mitmproxy.org/7.0.4/mitmproxy-7.0.4-linux.tar.gz &&\
- tar -zxvf mitmproxy-7.0.4-linux.tar.gz &&\
- mkdir /usr/local/share/ca-certificates/extra
-COPY allow_list_proxy.py /opt/mitmproxy/
-COPY allow_list.py /opt/mitmproxy/
-ENV HTTP_PROXY="http://127.0.0.1:8081"
-ENV HTTPS_PROXY="http://127.0.0.1:8081"
-
COPY src/properties.yaml /opt/playground/backend/properties.yaml
Review Comment:
Again, why not rely on a ConfigMap and mount that to the container, decoupling configuration from binary? If we are still stuck with appengine, terraform has a way to provide environment variables via the app_engine_flexible_app_version block https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/app_engine_flexible_app_version#env_variables
##########
playground/backend/containers/java/build.gradle:
##########
@@ -43,14 +43,6 @@ task copyDockerfileDependencies(type: Copy) {
from 'entrypoint.sh'
into 'build/'
}
- copy {
- from '../../../infrastructure/proxy/allow_list.py'
- into 'build/'
- }
- copy {
- from '../../../infrastructure/proxy/allow_list_proxy.py'
- into 'build/'
- }
copy {
Review Comment:
Why can't we use https://docs.docker.com/engine/reference/builder/#copy in the Dockerfile? Why does this need to happen in gradle? Isn't gradle designed for building code and automating test execution? Why are we coupling gradle with deployment operations when we have these kinds of tasks as part of the docker ecosystem?
##########
playground/backend/containers/go/entrypoint.sh:
##########
@@ -14,13 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-nohup /opt/mitmproxy/mitmdump -s /opt/mitmproxy/allow_list_proxy.py -p 8081 &
-while [ ! -f /home/appuser/.mitmproxy/mitmproxy-ca.pem ] ;
-do
- sleep 2
-done
-openssl x509 -in /home/appuser/.mitmproxy/mitmproxy-ca.pem -inform PEM -out /home/appuser/.mitmproxy/mitmproxy-ca.crt
-cp /home/appuser/.mitmproxy/mitmproxy-ca.crt /usr/local/share/ca-certificates/extra/
-update-ca-certificates
-
+if [[ -n "$PLAYGROUND_MITM_SERVICE_HOST" ]] && [[ -n "$PLAYGROUND_MITM_SERVICE_PORT" ]]
+then
+ export http_proxy=http://$PLAYGROUND_MITM_SERVICE_HOST:$PLAYGROUND_MITM_SERVICE_PORT
Review Comment:
What does this proxy achieve?
##########
playground/backend/containers/java/entrypoint.sh:
##########
@@ -14,13 +14,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-nohup /opt/mitmproxy/mitmdump -s /opt/mitmproxy/allow_list_proxy.py -p 8081 &
-while [ ! -f /home/appuser/.mitmproxy/mitmproxy-ca.pem ] ;
-do
- sleep 2
-done
-openssl x509 -in /home/appuser/.mitmproxy/mitmproxy-ca.pem -inform PEM -out /home/appuser/.mitmproxy/mitmproxy-ca.crt
-cp /home/appuser/.mitmproxy/mitmproxy-ca.crt /usr/local/share/ca-certificates/extra/
-update-ca-certificates
-
+if [[ -n "$PLAYGROUND_MITM_SERVICE_HOST" ]] && [[ -n "$PLAYGROUND_MITM_SERVICE_PORT" ]]
+then
+ export http_proxy=http://$PLAYGROUND_MITM_SERVICE_HOST:$PLAYGROUND_MITM_SERVICE_PORT
+ export https_proxy=http://$PLAYGROUND_MITM_SERVICE_HOST:$PLAYGROUND_MITM_SERVICE_PORT
Review Comment:
Again why do we need this?
##########
playground/backend/containers/java/entrypoint.sh:
##########
@@ -14,13 +14,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-nohup /opt/mitmproxy/mitmdump -s /opt/mitmproxy/allow_list_proxy.py -p 8081 &
-while [ ! -f /home/appuser/.mitmproxy/mitmproxy-ca.pem ] ;
-do
- sleep 2
-done
-openssl x509 -in /home/appuser/.mitmproxy/mitmproxy-ca.pem -inform PEM -out /home/appuser/.mitmproxy/mitmproxy-ca.crt
-cp /home/appuser/.mitmproxy/mitmproxy-ca.crt /usr/local/share/ca-certificates/extra/
-update-ca-certificates
-
+if [[ -n "$PLAYGROUND_MITM_SERVICE_HOST" ]] && [[ -n "$PLAYGROUND_MITM_SERVICE_PORT" ]]
+then
+ export http_proxy=http://$PLAYGROUND_MITM_SERVICE_HOST:$PLAYGROUND_MITM_SERVICE_PORT
+ export https_proxy=http://$PLAYGROUND_MITM_SERVICE_HOST:$PLAYGROUND_MITM_SERVICE_PORT
+ export JAVA_TOOL_OPTIONS="$JAVA_TOOL_OPTIONS -Dhttp.proxyHost=$PLAYGROUND_MITM_SERVICE_HOST -Dhttp.proxyPort=$PLAYGROUND_MITM_SERVICE_PORT -Dhttps.proxyHost=$PLAYGROUND_MITM_SERVICE_HOST -Dhttps.proxyPort=$PLAYGROUND_MITM_SERVICE_PORT"
Review Comment:
Again, why are we setting an environment variable in a script embedded in the Dockerfile? Why can't we for example use a ConfigMap and derive environment variables from this? If we are still stuck with appengine, terraform has a way to provide environment variables via the app_engine_flexible_app_version block https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/app_engine_flexible_app_version#env_variables
##########
playground/backend/containers/mitmproxy/Dockerfile:
##########
@@ -0,0 +1,27 @@
+###############################################################################
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+###############################################################################
+FROM mitmproxy/mitmproxy:9.0.1
Review Comment:
Document the point of mitmproxy
##########
playground/backend/containers/mitmproxy/build.gradle:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * License); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+apply plugin: 'org.apache.beam.module'
+apply plugin: 'base'
+applyDockerNature()
+
+def playgroundJobServerProject = "${project.path.replace('-container', '')}"
+
+description = project(playgroundJobServerProject).description + " :: Container"
+
+configurations {
+ dockerDependency
+}
+
+dependencies {
+ dockerDependency project(path: playgroundJobServerProject, configuration: "shadow")
+}
+
+task copyDockerfileDependencies(type: Copy) {
+ copy {
+ from 'entrypoint.sh'
+ into 'build/'
+ }
+ copy {
+ from 'config.yaml'
+ into 'build/'
+ }
+ copy {
+ from '../../../infrastructure/proxy/allow_list.py'
+ into 'build/'
+ }
+ copy {
+ from '../../../infrastructure/proxy/allow_list_proxy.py'
+ into 'build/'
+ }
Review Comment:
Again, why can't we use https://docs.docker.com/engine/reference/builder/#copy in the Dockerfile? Why does this need to happen in gradle? Isn't gradle designed for building code and automating test execution? Why are we coupling gradle with deployment operations when we have these kinds of tasks as part of the docker ecosystem?
##########
playground/infrastructure/helm-playground/templates/deployment-mitmproxy.yaml:
##########
@@ -0,0 +1,39 @@
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+ name: playground-mitm
+ labels:
+ app: backend-mitm
+ type: backend
+spec:
+ template:
+ metadata:
+ name: playground
+ labels:
+ app: backend-mitm
+ type: backend
+ spec:
+ containers:
+ - name: playground-backend-mitm
+ image: "{{ .Values.registry}}/{{ .Values.image.mitm_image }}:{{ .Values.tag }}"
+ imagePullPolicy: {{ .Values.image.pullPolicy }}
+ replicas: {{ .Values.replicaCount }}
+ selector:
+ matchLabels:
+ app: backend-mitm
Review Comment:
Ahain, what does this achieve?
##########
playground/docker-compose.local.yaml:
##########
@@ -87,10 +101,13 @@ services:
CACHE_TYPE: remote
CACHE_ADDRESS: redis:6379
SERVER_PORT: 8090
+ PLAYGROUND_MITM_SERVICE_HOST: mitmproxy
+ PLAYGROUND_MITM_SERVICE_PORT: 8080
ports:
- "8090:8090"
depends_on:
- redis
+ - mitmproxy
frontend:
image: apache/beam_playground-frontend
Review Comment:
Why do we have to maintain both docker compose and kubernetes manifests? What if there is drift from these two artifacts that would lead to errors in production? Why can't we have a single source of truth and someone just setup minikube on their local workstation and deploy the kubernetes manifests instead of docker compose?
##########
playground/infrastructure/helm-playground/templates/network-policy.yaml:
##########
@@ -0,0 +1,66 @@
+#!/bin/bash
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+---
+apiVersion: networking.k8s.io/v1
+kind: NetworkPolicy
+metadata:
+ name: egress-deny
+spec:
+ podSelector:
+ matchExpressions:
+ - key: app
+ operator: In
+ values:
+ - backend-go
+ - backend-java
+ - backend-python
+ - backend-scio
+ policyTypes:
+ - Egress
+ egress:
+ - to:
+ - namespaceSelector:
+ matchLabels:
+ kubernetes.io/metadata.name: kube-system
+ podSelector:
+ matchLabels:
+ k8s-app: kube-dns
+ ports:
+ - port: 53
+ protocol: UDP
+ - port: 53
+ protocol: TCP
+ - to:
+ - ipBlock:
+ cidr: 10.0.0.0/8
+ ports:
+ - protocol: TCP
+ port: 6379
+ - protocol: TCP
+ port: 8080
+ - to:
+ - ipBlock:
+ cidr: 169.254.169.252/32
+ ports:
+ - protocol: TCP
+ port: 988
+ - to:
+ - ipBlock:
+ cidr: 169.254.169.254/32
+ ports:
+ - protocol: TCP
+ port: 80
Review Comment:
Why is this needed?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org