You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by ka...@apache.org on 2021/05/23 17:07:39 UTC

[airflow] branch master updated: Handle special characters in password sfor Helm Chart (#16004)

This is an automated email from the ASF dual-hosted git repository.

kamilbregula pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/master by this push:
     new ce358b2  Handle special characters in password sfor Helm Chart (#16004)
ce358b2 is described below

commit ce358b21533eeb7a237e6b0833872bf2daab7e30
Author: Kamil BreguĊ‚a <mi...@users.noreply.github.com>
AuthorDate: Sun May 23 19:07:19 2021 +0200

    Handle special characters in password sfor Helm Chart (#16004)
---
 chart/templates/secrets/elasticsearch-secret.yaml  |  4 +-
 .../secrets/metadata-connection-secret.yaml        |  6 ++-
 .../templates/secrets/pgbouncer-stats-secret.yaml  |  2 +-
 chart/templates/secrets/redis-secrets.yaml         |  2 +-
 .../secrets/result-backend-connection-secret.yaml  |  4 +-
 chart/tests/test_elasticsearch_secret.py           | 51 ++++++++++++++++++++++
 chart/tests/test_metadata_connection_secret.py     | 18 ++++++++
 chart/tests/test_redis.py                          |  8 ++--
 .../tests/test_result_backend_connection_secret.py | 17 ++++++++
 9 files changed, 102 insertions(+), 10 deletions(-)

diff --git a/chart/templates/secrets/elasticsearch-secret.yaml b/chart/templates/secrets/elasticsearch-secret.yaml
index cae8158..b220ab5 100644
--- a/chart/templates/secrets/elasticsearch-secret.yaml
+++ b/chart/templates/secrets/elasticsearch-secret.yaml
@@ -32,5 +32,7 @@ metadata:
 {{- end }}
 type: Opaque
 data:
-  connection: {{ (printf "http://%s:%s@%s:%s" .Values.elasticsearch.connection.user .Values.elasticsearch.connection.pass .Values.elasticsearch.connection.host (.Values.elasticsearch.connection.port | toString)) | b64enc | quote }}
+  {{- with .Values.elasticsearch.connection }}
+  connection: {{ urlJoin (dict "scheme" "http" "userinfo" (printf "%s:%s" (.user | urlquery) (.pass | urlquery)) "host" (printf "%s:%s" .host ((default .port 80) | toString) ) ) | b64enc | quote }}
+  {{- end }}
 {{- end }}
diff --git a/chart/templates/secrets/metadata-connection-secret.yaml b/chart/templates/secrets/metadata-connection-secret.yaml
index 67a206c..a4441c4 100644
--- a/chart/templates/secrets/metadata-connection-secret.yaml
+++ b/chart/templates/secrets/metadata-connection-secret.yaml
@@ -24,7 +24,7 @@
 {{- $host := ternary $pgbouncerHost $metadataHost .Values.pgbouncer.enabled }}
 {{- $port := ((ternary .Values.ports.pgbouncer .Values.data.metadataConnection.port .Values.pgbouncer.enabled) | toString) }}
 {{- $database := (ternary (printf "%s-%s" .Release.Name "metadata") .Values.data.metadataConnection.db .Values.pgbouncer.enabled) }}
-{{- $extras := ternary (printf "?sslmode=%s" .Values.data.metadataConnection.sslmode) "" (eq .Values.data.metadataConnection.protocol "postgresql") }}
+{{- $query := ternary (printf "sslmode=%s" .Values.data.metadataConnection.sslmode) "" (eq .Values.data.metadataConnection.protocol "postgresql") }}
 
 kind: Secret
 apiVersion: v1
@@ -40,5 +40,7 @@ metadata:
 {{- end }}
 type: Opaque
 data:
-  connection: {{ (printf "%s://%s:%s@%s:%s/%s%s" .Values.data.metadataConnection.protocol .Values.data.metadataConnection.user .Values.data.metadataConnection.pass $host $port $database $extras) | b64enc | quote }}
+  {{- with .Values.data.metadataConnection }}
+  connection: {{ urlJoin (dict "scheme" .protocol "userinfo" (printf "%s:%s" (.user | urlquery) (.pass | urlquery) ) "host" (printf "%s:%s" $host $port) "path" (printf "/%s" $database) "query" $query) | b64enc | quote }}
+  {{- end }}
 {{- end }}
diff --git a/chart/templates/secrets/pgbouncer-stats-secret.yaml b/chart/templates/secrets/pgbouncer-stats-secret.yaml
index 92ad97b..b8669b3 100644
--- a/chart/templates/secrets/pgbouncer-stats-secret.yaml
+++ b/chart/templates/secrets/pgbouncer-stats-secret.yaml
@@ -34,5 +34,5 @@ metadata:
 {{- end }}
 type: Opaque
 data:
-  connection: {{ (printf "postgresql://%s:%s@127.0.0.1:%s/pgbouncer?sslmode=disable" .Values.data.metadataConnection.user .Values.data.metadataConnection.pass (.Values.ports.pgbouncer | toString)) | b64enc | quote }}
+  connection: {{ urlJoin (dict "scheme" "postgresql" "userinfo" (printf "%s:%s" (.Values.data.metadataConnection.user | urlquery) (.Values.data.metadataConnection.pass | urlquery) ) "host" (printf "127.0.0.1::%s" (.Values.ports.pgbouncer | toString)) "path" "/pgbouncer" "query" "sslmode=disable") | b64enc | quote }}
 {{- end }}
diff --git a/chart/templates/secrets/redis-secrets.yaml b/chart/templates/secrets/redis-secrets.yaml
index df26ad6..f36fcbe 100644
--- a/chart/templates/secrets/redis-secrets.yaml
+++ b/chart/templates/secrets/redis-secrets.yaml
@@ -68,7 +68,7 @@ metadata:
 type: Opaque
 data:
 {{- if .Values.redis.enabled }}
-  connection: {{ (printf "redis://:%s@%s-redis:6379/0" (default $random_redis_password .Values.redis.password) .Release.Name) | b64enc | quote }}
+  connection: {{ urlJoin (dict "scheme" "redis" "userinfo" (printf ":%s" ((default $random_redis_password .Values.redis.password) | urlquery)) "host" (printf "%s-redis:6379" .Release.Name ) "path" "/0") | b64enc | quote }}
 {{- else }}
   connection: {{ (printf "%s" .Values.data.brokerUrl) | b64enc | quote }}
 {{- end }}
diff --git a/chart/templates/secrets/result-backend-connection-secret.yaml b/chart/templates/secrets/result-backend-connection-secret.yaml
index c773972..46fd603 100644
--- a/chart/templates/secrets/result-backend-connection-secret.yaml
+++ b/chart/templates/secrets/result-backend-connection-secret.yaml
@@ -27,7 +27,7 @@
 {{- $host := ternary $pgbouncerHost $resultBackendHost .Values.pgbouncer.enabled }}
 {{- $port := (ternary .Values.ports.pgbouncer $connection.port .Values.pgbouncer.enabled) | toString }}
 {{- $database := ternary (printf "%s-%s" .Release.Name "result-backend") $connection.db .Values.pgbouncer.enabled }}
-{{- $extras := ternary (printf "?sslmode=%s" $connection.sslmode) "" (eq $connection.protocol "postgresql") }}
+{{- $query := ternary (printf "sslmode=%s" $connection.sslmode) "" (eq $connection.protocol "postgresql") }}
 kind: Secret
 apiVersion: v1
 metadata:
@@ -42,6 +42,6 @@ metadata:
 {{- end }}
 type: Opaque
 data:
-  connection: {{ (printf "db+%s://%s:%s@%s:%s/%s%s" $connection.protocol $connection.user $connection.pass $host $port $database $extras) | b64enc | quote }}
+  connection: {{ urlJoin (dict "scheme" (printf "db+%s" $connection.protocol) "userinfo" (printf "%s:%s" ($connection.user|urlquery) ($connection.pass | urlquery)) "host" (printf "%s:%s" $host $port) "path" (printf "/%s" $database) "query" $query) | b64enc | quote }}
 {{- end }}
 {{- end }}
diff --git a/chart/tests/test_elasticsearch_secret.py b/chart/tests/test_elasticsearch_secret.py
new file mode 100644
index 0000000..9d2c2b9
--- /dev/null
+++ b/chart/tests/test_elasticsearch_secret.py
@@ -0,0 +1,51 @@
+# 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.
+
+import base64
+import unittest
+
+import jmespath
+
+from tests.helm_template_generator import render_chart
+
+
+class ElasticsearchSecretTest(unittest.TestCase):
+    def _get_connection(self, values: dict) -> str:
+        docs = render_chart(
+            values=values,
+            show_only=["templates/secrets/elasticsearch-secret.yaml"],
+        )
+        encoded_connection = jmespath.search("data.connection", docs[0])
+        return base64.b64decode(encoded_connection).decode()
+
+    def test_should_correctly_handle_password_with_special_characters(self):
+        connection = self._get_connection(
+            {
+                "elasticsearch": {
+                    "connection": {
+                        "user": "username!@#$%%^&*()",
+                        "pass": "password!@#$%%^&*()",
+                        "host": "elastichostname",
+                    }
+                }
+            }
+        )
+
+        assert (
+            "http://username%21%40%23$%25%25%5E&%2A%28%29:password%21%40%23$%25%25%5E&%2A%28%29@"
+            "elastichostname:80" == connection
+        )
diff --git a/chart/tests/test_metadata_connection_secret.py b/chart/tests/test_metadata_connection_secret.py
index 0df0190..34be3e8 100644
--- a/chart/tests/test_metadata_connection_secret.py
+++ b/chart/tests/test_metadata_connection_secret.py
@@ -106,3 +106,21 @@ class MetadataConnectionSecretTest(unittest.TestCase):
 
         # sslmode is only added for postgresql
         assert "mysql://someuser:somepass@somehost:7777/somedb" == connection
+
+    def test_should_correctly_handle_password_with_special_characters(self):
+        values = {
+            "data": {
+                "metadataConnection": {
+                    **self.non_chart_database_values,
+                    "user": "username@123123",
+                    "pass": "password@!@#$^&*()",
+                }
+            }
+        }
+        connection = self._get_connection(values)
+
+        # sslmode is only added for postgresql
+        assert (
+            "postgresql://username%40123123:password%40%21%40%23$%5E&%2A%28%29@somehost:7777/"
+            "somedb?sslmode=disable" == connection
+        )
diff --git a/chart/tests/test_redis.py b/chart/tests/test_redis.py
index 65f5a53..067f285 100644
--- a/chart/tests/test_redis.py
+++ b/chart/tests/test_redis.py
@@ -111,7 +111,7 @@ class RedisTest(unittest.TestCase):
         self.assert_password_and_broker_url_secrets(
             k8s_obj_by_key,
             expected_password_match=r"\w+",
-            expected_broker_url_match=fr"redis://:\w+@{RELEASE_NAME_REDIS}-redis:6379/0",
+            expected_broker_url_match=fr"redis://:.+@{RELEASE_NAME_REDIS}-redis:6379/0",
         )
 
         self.assert_broker_url_env(k8s_obj_by_key)
@@ -123,7 +123,7 @@ class RedisTest(unittest.TestCase):
             {
                 "executor": executor,
                 "networkPolicies": {"enabled": True},
-                "redis": {"enabled": True, "password": "test-redis-password"},
+                "redis": {"enabled": True, "password": "test-redis-password!@#$%^&*()_+"},
             },
         )
         k8s_obj_by_key = prepare_k8s_lookup_dict(k8s_objects)
@@ -134,7 +134,9 @@ class RedisTest(unittest.TestCase):
         self.assert_password_and_broker_url_secrets(
             k8s_obj_by_key,
             expected_password_match="test-redis-password",
-            expected_broker_url_match=f"redis://:test-redis-password@{RELEASE_NAME_REDIS}-redis:6379/0",
+            expected_broker_url_match=re.escape(
+                "redis://:test-redis-password%21%40%23$%25%5E&%2A%28%29_+@TEST-REDIS-redis:6379/0"
+            ),
         )
 
         self.assert_broker_url_env(k8s_obj_by_key)
diff --git a/chart/tests/test_result_backend_connection_secret.py b/chart/tests/test_result_backend_connection_secret.py
index 2c1c684..4e40aad 100644
--- a/chart/tests/test_result_backend_connection_secret.py
+++ b/chart/tests/test_result_backend_connection_secret.py
@@ -153,3 +153,20 @@ class ResultBackendConnectionSecretTest(unittest.TestCase):
         connection = self._get_connection(values)
 
         assert "db+postgresql://anotheruser:anotherpass@somehost:7777/somedb?sslmode=allow" == connection
+
+    def test_should_correctly_handle_password_with_special_characters(self):
+        values = {
+            "data": {
+                "resultBackendConnection": {
+                    **self.non_chart_database_values,
+                    "user": "username@123123",
+                    "pass": "password@!@#$^&*()",
+                },
+            }
+        }
+        connection = self._get_connection(values)
+
+        assert (
+            "db+postgresql://username%40123123:password%40%21%40%23$%5E&%2A%28%29@somehost:7777/"
+            "somedb?sslmode=allow" == connection
+        )