From ec1f69b1a27683cefb4e3f1a83a2de1c8f963e50 Mon Sep 17 00:00:00 2001 From: Topher Anderson <48180628+topherinternational@users.noreply.github.com> Date: Fri, 3 Jan 2025 14:32:52 +0100 Subject: [PATCH] Chart: add OpenSearch remote logging options (#45082) * add OpenSearch remote logging options --- chart/templates/_helpers.yaml | 13 + chart/templates/check-values.yaml | 15 + .../scheduler/scheduler-deployment.yaml | 8 +- .../templates/secrets/opensearch-secret.yaml | 44 +++ chart/values.schema.json | 68 ++++ chart/values.yaml | 21 +- helm_tests/airflow_aux/test_airflow_common.py | 3 + helm_tests/airflow_aux/test_remote_logging.py | 374 ++++++++++++++++++ .../security/test_elasticsearch_secret.py | 45 --- helm_tests/security/test_opensearch_secret.py | 119 ++++++ 10 files changed, 659 insertions(+), 51 deletions(-) create mode 100644 chart/templates/secrets/opensearch-secret.yaml create mode 100644 helm_tests/airflow_aux/test_remote_logging.py create mode 100644 helm_tests/security/test_opensearch_secret.py diff --git a/chart/templates/_helpers.yaml b/chart/templates/_helpers.yaml index 887e9bd33cbb1..c76cefa843e34 100644 --- a/chart/templates/_helpers.yaml +++ b/chart/templates/_helpers.yaml @@ -142,6 +142,15 @@ If release name contains chart name it will be used as a full name. key: connection {{- end }} {{- end }} + {{- if .Values.opensearch.enabled }} + {{- if .Values.enableBuiltInSecretEnvVars.AIRFLOW__OPENSEARCH__HOST }} + - name: AIRFLOW__OPENSEARCH__HOST + valueFrom: + secretKeyRef: + name: {{ template "opensearch_secret" . }} + key: connection + {{- end }} + {{- end }} {{- end }} {{/* User defined Airflow environment variables */}} @@ -427,6 +436,10 @@ If release name contains chart name it will be used as a full name. {{- default (printf "%s-elasticsearch" (include "airflow.fullname" .)) .Values.elasticsearch.secretName }} {{- end }} +{{- define "opensearch_secret" -}} + {{- default (printf "%s-opensearch" (include "airflow.fullname" .)) .Values.opensearch.secretName }} +{{- end }} + {{- define "flower_secret" -}} {{- default (printf "%s-flower" (include "airflow.fullname" .)) .Values.flower.secretName }} {{- end }} diff --git a/chart/templates/check-values.yaml b/chart/templates/check-values.yaml index 6dfbbd9554108..a4ede57f3bc9c 100644 --- a/chart/templates/check-values.yaml +++ b/chart/templates/check-values.yaml @@ -62,6 +62,10 @@ The sole purpose of this yaml file is it to check the values file is consistent {{- end }} + {{- if and .Values.elasticsearch.enabled .Values.opensearch.enabled }} + {{ required "You must not set both values elasticsearch.enabled and opensearch.enabled" nil }} + {{- end }} + {{- if .Values.elasticsearch.enabled }} {{- if and .Values.elasticsearch.secretName .Values.elasticsearch.connection }} {{ required "You must not set both values elasticsearch.secretName and elasticsearch.connection" nil }} @@ -72,3 +76,14 @@ The sole purpose of this yaml file is it to check the values file is consistent {{- end }} {{- end }} + + {{- if .Values.opensearch.enabled }} + {{- if and .Values.opensearch.secretName .Values.opensearch.connection }} + {{ required "You must not set both values opensearch.secretName and opensearch.connection" nil }} + {{- end }} + + {{- if not (or .Values.opensearch.secretName .Values.opensearch.connection) }} + {{ required "You must set one of the values opensearch.secretName or opensearch.connection when using OpenSearch" nil }} + {{- end }} + + {{- end }} diff --git a/chart/templates/scheduler/scheduler-deployment.yaml b/chart/templates/scheduler/scheduler-deployment.yaml index 551ab94c8c48d..ed63f1ef46903 100644 --- a/chart/templates/scheduler/scheduler-deployment.yaml +++ b/chart/templates/scheduler/scheduler-deployment.yaml @@ -30,8 +30,8 @@ {{- $stateful := and $local $persistence }} # We can skip DAGs mounts on scheduler if dagProcessor is enabled, except with $local mode {{- $localOrDagProcessorDisabled := or (not .Values.dagProcessor.enabled) $local }} -# If we're using elasticsearch logging -{{- $elasticsearch := .Values.elasticsearch.enabled }} +# If we're using elasticsearch or opensearch logging +{{- $remoteLogging := or .Values.elasticsearch.enabled .Values.opensearch.enabled }} {{- $nodeSelector := or .Values.scheduler.nodeSelector .Values.nodeSelector }} {{- $affinity := or .Values.scheduler.affinity .Values.affinity }} {{- $tolerations := or .Values.scheduler.tolerations .Values.tolerations }} @@ -217,8 +217,8 @@ spec: {{- else }} {{- include "scheduler_startup_check_command" . | indent 14 }} {{- end }} - {{- if and $local (not $elasticsearch) }} - # Serve logs if we're in local mode and we don't have elasticsearch enabled. + {{- if and $local (not $remoteLogging) }} + # Serve logs if we're in local mode and we have neither elasticsearch nor opensearch enabled. ports: - name: worker-logs containerPort: {{ .Values.ports.workerLogs }} diff --git a/chart/templates/secrets/opensearch-secret.yaml b/chart/templates/secrets/opensearch-secret.yaml new file mode 100644 index 0000000000000..e0e281d6f4113 --- /dev/null +++ b/chart/templates/secrets/opensearch-secret.yaml @@ -0,0 +1,44 @@ +{{/* + 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. +*/}} + +################################ +## OpenSearch Secret +################################# +{{- if (and .Values.opensearch.enabled (not .Values.opensearch.secretName)) }} +apiVersion: v1 +kind: Secret +metadata: + name: {{ include "airflow.fullname" . }}-opensearch + labels: + release: {{ .Release.Name }} + chart: {{ .Chart.Name }} + heritage: {{ .Release.Service }} + {{- with .Values.labels }} + {{- toYaml . | nindent 4 }} + {{- end }} +type: Opaque +data: + {{- with .Values.opensearch.connection }} + {{- if and .user .pass }} + connection: {{ urlJoin (dict "scheme" (default "http" .scheme) "userinfo" (printf "%s:%s" (.user | urlquery) (.pass | urlquery)) "host" (printf "%s:%s" .host ((default 9200 .port) | toString) ) ) | b64enc | quote }} + {{- else }} + connection: {{ urlJoin (dict "scheme" (default "http" .scheme) "host" (printf "%s:%s" .host ((default 9200 .port) | toString))) | b64enc | quote }} + {{- end }} + {{- end }} +{{- end }} diff --git a/chart/values.schema.json b/chart/values.schema.json index ce8d3e8b8944d..8443b1a8cf655 100644 --- a/chart/values.schema.json +++ b/chart/values.schema.json @@ -1093,6 +1093,11 @@ "description": "Enable ``AIRFLOW__ELASTICSEARCH__ELASTICSEARCH_HOST`` variable to be read from the Elasticsearch Host Secret - Airflow <1.10.4 variant", "type": "boolean", "default": true + }, + "AIRFLOW__OPENSEARCH__HOST": { + "description": "Enable ``AIRFLOW__OPENSEARCH__HOST`` variable to be read from the OpenSearch Host Secret", + "type": "boolean", + "default": true } } }, @@ -8044,6 +8049,69 @@ } } }, + "opensearch": { + "description": "OpenSearch logging configuration.", + "type": "object", + "x-docsSection": "Airflow", + "additionalProperties": false, + "properties": { + "enabled": { + "description": "Enable OpenSearch task logging.", + "type": "boolean", + "default": false + }, + "secretName": { + "description": "A secret containing the connection string.", + "type": [ + "string", + "null" + ], + "default": null + }, + "connection": { + "description": "OpenSearch connection configuration.", + "type": "object", + "default": {}, + "additionalProperties": false, + "properties": { + "scheme": { + "description": "Scheme", + "type": "string", + "default": "http" + }, + "user": { + "description": "Username", + "type": "string", + "default": "" + }, + "pass": { + "description": "Password", + "type": "string", + "default": "" + }, + "host": { + "description": "Host", + "type": "string", + "default": "" + }, + "port": { + "description": "Port", + "type": "number", + "default": 80 + } + }, + "examples": [ + { + "scheme": "https", + "user": "...", + "pass": "...", + "host": "...", + "port": "..." + } + ] + } + } + }, "ports": { "description": "All ports used by chart.", "type": "object", diff --git a/chart/values.yaml b/chart/values.yaml index adf68c3a194d3..2e27b7a4686ee 100644 --- a/chart/values.yaml +++ b/chart/values.yaml @@ -366,6 +366,7 @@ enableBuiltInSecretEnvVars: AIRFLOW__CELERY__BROKER_URL: true AIRFLOW__ELASTICSEARCH__HOST: true AIRFLOW__ELASTICSEARCH__ELASTICSEARCH_HOST: true + AIRFLOW__OPENSEARCH__HOST: true # Priority Classes that will be installed by charts. # Ideally, there should be an entry for dagProcessor, flower, @@ -2473,6 +2474,22 @@ elasticsearch: # port: ~ connection: {} +# OpenSearch logging configuration +opensearch: + # Enable opensearch task logging + enabled: false + # A secret containing the connection + secretName: ~ + # Or an object representing the connection + # Example: + # connection: + # scheme: ~ + # user: ~ + # pass: ~ + # host: ~ + # port: ~ + connection: {} + # All ports used by chart ports: flowerUI: 5555 @@ -2591,9 +2608,9 @@ config: executor: '{{ .Values.executor }}' # For Airflow 1.10, backward compatibility; moved to [logging] in 2.0 colored_console_log: 'False' - remote_logging: '{{- ternary "True" "False" .Values.elasticsearch.enabled }}' + remote_logging: '{{- ternary "True" "False" (or .Values.elasticsearch.enabled .Values.opensearch.enabled) }}' logging: - remote_logging: '{{- ternary "True" "False" .Values.elasticsearch.enabled }}' + remote_logging: '{{- ternary "True" "False" (or .Values.elasticsearch.enabled .Values.opensearch.enabled) }}' colored_console_log: 'False' metrics: statsd_on: '{{ ternary "True" "False" .Values.statsd.enabled }}' diff --git a/helm_tests/airflow_aux/test_airflow_common.py b/helm_tests/airflow_aux/test_airflow_common.py index d56d648f19af4..0cd4218f7627b 100644 --- a/helm_tests/airflow_aux/test_airflow_common.py +++ b/helm_tests/airflow_aux/test_airflow_common.py @@ -321,7 +321,10 @@ def test_should_disable_some_variables(self): "AIRFLOW__CORE__SQL_ALCHEMY_CONN": False, "AIRFLOW__DATABASE__SQL_ALCHEMY_CONN": False, "AIRFLOW__WEBSERVER__SECRET_KEY": False, + # the following vars only appear if remote logging is set, so disabling them in this test is kind of a no-op "AIRFLOW__ELASTICSEARCH__HOST": False, + "AIRFLOW__ELASTICSEARCH__ELASTICSEARCH_HOST": False, + "AIRFLOW__OPENSEARCH__HOST": False, }, }, show_only=[ diff --git a/helm_tests/airflow_aux/test_remote_logging.py b/helm_tests/airflow_aux/test_remote_logging.py new file mode 100644 index 0000000000000..b6de5ef6288db --- /dev/null +++ b/helm_tests/airflow_aux/test_remote_logging.py @@ -0,0 +1,374 @@ +# 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 __future__ import annotations + +import re +from subprocess import CalledProcessError + +import jmespath +import pytest + +from tests.charts.helm_template_generator import render_chart + +ES_SECRET_TEMPLATE = "templates/secrets/elasticsearch-secret.yaml" +OS_SECRET_TEMPLATE = "templates/secrets/opensearch-secret.yaml" +SCHEDULER_DEPLOYMENT_TEMPLATE = "templates/scheduler/scheduler-deployment.yaml" +CONFIGMAP_TEMPLATE = "templates/configmaps/configmap.yaml" + +CORE_CFG_REGEX = re.compile(r"\[core]\n.*?\n\n", flags=re.RegexFlag.DOTALL) +LOGGING_CFG_REGEX = re.compile(r"\[logging]\n.*?\n\n", flags=re.RegexFlag.DOTALL) + + +class TestElasticsearchConfig: + """Tests elasticsearch configuration behaviors.""" + + def test_should_not_generate_secret_document_if_elasticsearch_disabled(self): + docs = render_chart( + values={"elasticsearch": {"enabled": False}}, + show_only=[ES_SECRET_TEMPLATE], + ) + + assert len(docs) == 0 + + def test_should_raise_error_when_connection_not_provided(self): + with pytest.raises(CalledProcessError) as ex_ctx: + render_chart( + values={ + "elasticsearch": { + "enabled": True, + } + }, + show_only=[ES_SECRET_TEMPLATE], + ) + assert ( + "You must set one of the values elasticsearch.secretName or elasticsearch.connection " + "when using a Elasticsearch" in ex_ctx.value.stderr.decode() + ) + + def test_should_raise_error_when_conflicting_options(self): + with pytest.raises(CalledProcessError) as ex_ctx: + render_chart( + values={ + "elasticsearch": { + "enabled": True, + "secretName": "my-test", + "connection": { + "user": "username!@#$%%^&*()", + "pass": "password!@#$%%^&*()", + "host": "elastichostname", + }, + }, + }, + show_only=[ES_SECRET_TEMPLATE], + ) + assert ( + "You must not set both values elasticsearch.secretName and elasticsearch.connection" + in ex_ctx.value.stderr.decode() + ) + + def test_scheduler_should_add_log_port_when_local_executor_and_elasticsearch_disabled(self): + docs = render_chart( + values={"executor": "LocalExecutor"}, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + + assert jmespath.search("spec.template.spec.containers[0].ports", docs[0]) == [ + {"name": "worker-logs", "containerPort": 8793} + ] + + def test_scheduler_should_omit_log_port_when_elasticsearch_enabled(self): + docs = render_chart( + values={ + "executor": "LocalExecutor", + "elasticsearch": { + "enabled": True, + "secretName": "test-elastic-secret", + }, + }, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + + assert "ports" not in jmespath.search("spec.template.spec.containers[0]", docs[0]) + + def test_env_should_omit_elasticsearch_host_var_if_es_disabled(self): + docs = render_chart( + values={}, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + + scheduler_env_keys = jmespath.search("spec.template.spec.containers[0].env[*].name", docs[0]) + assert "AIRFLOW__ELASTICSEARCH__HOST" not in scheduler_env_keys + + def test_env_should_add_elasticsearch_host_var_if_es_enabled(self): + docs = render_chart( + values={ + "elasticsearch": { + "enabled": True, + "secretName": "test-elastic-secret", + }, + }, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + + scheduler_env = jmespath.search("spec.template.spec.containers[0].env", docs[0]) + + assert { + "name": "AIRFLOW__ELASTICSEARCH__HOST", + "valueFrom": {"secretKeyRef": {"name": "test-elastic-secret", "key": "connection"}}, + } in scheduler_env + + def test_env_should_omit_elasticsearch_host_var_if_es_disabled_legacy(self): + """AIRFLOW__ELASTICSEARCH__ELASTICSEARCH_HOST was the environment key prior to Airflow 1.10.4 + (see https://github.com/apache/airflow/pull/5048), this test can be removed when the Helm chart + no longer supports Airflow 1.10.3""" + docs = render_chart( + values={}, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + + scheduler_env_keys = jmespath.search("spec.template.spec.containers[0].env[*].name", docs[0]) + assert "AIRFLOW__ELASTICSEARCH__ELASTICSEARCH_HOST" not in scheduler_env_keys + + def test_env_should_add_elasticsearch_host_var_if_es_enabled_legacy(self): + """AIRFLOW__ELASTICSEARCH__ELASTICSEARCH_HOST was the environment key prior to Airflow 1.10.4 + (see https://github.com/apache/airflow/pull/5048), this test can be removed when the Helm chart + no longer supports Airflow 1.10.3""" + docs = render_chart( + values={ + "elasticsearch": { + "enabled": True, + "secretName": "test-elastic-secret", + }, + }, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + + scheduler_env = jmespath.search("spec.template.spec.containers[0].env", docs[0]) + assert { + "name": "AIRFLOW__ELASTICSEARCH__ELASTICSEARCH_HOST", + "valueFrom": {"secretKeyRef": {"name": "test-elastic-secret", "key": "connection"}}, + } in scheduler_env + + def test_airflow_cfg_should_set_remote_logging_false_if_es_disabled(self): + docs = render_chart( + values={}, + show_only=[CONFIGMAP_TEMPLATE], + ) + + airflow_cfg_text = jmespath.search('data."airflow.cfg"', docs[0]) + + logging_lines = LOGGING_CFG_REGEX.findall(airflow_cfg_text)[0].strip().splitlines() + assert "remote_logging = False" in logging_lines + + def test_airflow_cfg_should_set_remote_logging_true_if_es_enabled(self): + docs = render_chart( + values={ + "elasticsearch": { + "enabled": True, + "secretName": "test-elastic-secret", + }, + }, + show_only=[CONFIGMAP_TEMPLATE], + ) + + airflow_cfg_text = jmespath.search('data."airflow.cfg"', docs[0]) + + logging_lines = LOGGING_CFG_REGEX.findall(airflow_cfg_text)[0].strip().splitlines() + assert "remote_logging = True" in logging_lines + + def test_airflow_cfg_should_set_remote_logging_false_if_es_disabled_legacy(self): + """core.remote_logging was the config location prior to Airflow 2.0.0, this test can be removed + when the Helm chart no longer supports Airflow 1.x""" + docs = render_chart( + values={}, + show_only=[CONFIGMAP_TEMPLATE], + ) + + airflow_cfg_text = jmespath.search('data."airflow.cfg"', docs[0]) + + core_lines = CORE_CFG_REGEX.findall(airflow_cfg_text)[0].strip().splitlines() + assert "remote_logging = False" in core_lines + + def test_airflow_cfg_should_set_remote_logging_true_if_es_enabled_legacy(self): + """core.remote_logging was the config location prior to Airflow 2.0.0, this test can be removed + when the Helm chart no longer supports Airflow 1.x""" + docs = render_chart( + values={ + "elasticsearch": { + "enabled": True, + "secretName": "test-elastic-secret", + }, + }, + show_only=[CONFIGMAP_TEMPLATE], + ) + + airflow_cfg_text = jmespath.search('data."airflow.cfg"', docs[0]) + + core_lines = CORE_CFG_REGEX.findall(airflow_cfg_text)[0].strip().splitlines() + assert "remote_logging = True" in core_lines + + +class TestOpenSearchConfig: + """Tests opensearch configuration behaviors.""" + + def test_should_not_generate_secret_document_if_opensearch_disabled(self): + docs = render_chart( + values={"opensearch": {"enabled": False}}, + show_only=[OS_SECRET_TEMPLATE], + ) + + assert len(docs) == 0 + + def test_should_raise_error_when_connection_not_provided(self): + with pytest.raises(CalledProcessError) as ex_ctx: + render_chart( + values={ + "opensearch": { + "enabled": True, + } + }, + show_only=[OS_SECRET_TEMPLATE], + ) + assert ( + "You must set one of the values opensearch.secretName or opensearch.connection " + "when using OpenSearch" in ex_ctx.value.stderr.decode() + ) + + def test_should_raise_error_when_conflicting_options(self): + with pytest.raises(CalledProcessError) as ex_ctx: + render_chart( + values={ + "opensearch": { + "enabled": True, + "secretName": "my-test", + "connection": { + "user": "username!@#$%%^&*()", + "pass": "password!@#$%%^&*()", + "host": "opensearchhostname", + }, + }, + }, + show_only=[OS_SECRET_TEMPLATE], + ) + assert ( + "You must not set both values opensearch.secretName and opensearch.connection" + in ex_ctx.value.stderr.decode() + ) + + def test_scheduler_should_add_log_port_when_local_executor_and_opensearch_disabled(self): + docs = render_chart( + values={"executor": "LocalExecutor"}, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + + assert jmespath.search("spec.template.spec.containers[0].ports", docs[0]) == [ + {"name": "worker-logs", "containerPort": 8793} + ] + + def test_scheduler_should_omit_log_port_when_opensearch_enabled(self): + docs = render_chart( + values={ + "executor": "LocalExecutor", + "opensearch": { + "enabled": True, + "secretName": "test-elastic-secret", + }, + }, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + + assert "ports" not in jmespath.search("spec.template.spec.containers[0]", docs[0]) + + def test_env_should_omit_opensearch_host_var_if_os_disabled(self): + docs = render_chart( + values={}, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + + scheduler_env_keys = jmespath.search("spec.template.spec.containers[0].env[*].name", docs[0]) + assert "AIRFLOW__OPENSEARCH__HOST" not in scheduler_env_keys + + def test_env_should_add_opensearch_host_var_if_os_enabled(self): + docs = render_chart( + values={ + "opensearch": { + "enabled": True, + "secretName": "test-opensearch-secret", + }, + }, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + + scheduler_env = jmespath.search("spec.template.spec.containers[0].env", docs[0]) + assert { + "name": "AIRFLOW__OPENSEARCH__HOST", + "valueFrom": {"secretKeyRef": {"name": "test-opensearch-secret", "key": "connection"}}, + } in scheduler_env + + def test_airflow_cfg_should_set_remote_logging_false_if_os_disabled(self): + docs = render_chart( + values={}, + show_only=[CONFIGMAP_TEMPLATE], + ) + + airflow_cfg_text = jmespath.search('data."airflow.cfg"', docs[0]) + + core_lines = CORE_CFG_REGEX.findall(airflow_cfg_text)[0].strip().splitlines() + assert "remote_logging = False" in core_lines + + logging_lines = LOGGING_CFG_REGEX.findall(airflow_cfg_text)[0].strip().splitlines() + assert "remote_logging = False" in logging_lines + + def test_airflow_cfg_should_set_remote_logging_true_if_os_enabled(self): + docs = render_chart( + values={ + "opensearch": { + "enabled": True, + "secretName": "test-elastic-secret", + }, + }, + show_only=[CONFIGMAP_TEMPLATE], + ) + + airflow_cfg_text = jmespath.search('data."airflow.cfg"', docs[0]) + + core_lines = CORE_CFG_REGEX.findall(airflow_cfg_text)[0].strip().splitlines() + assert "remote_logging = True" in core_lines + + logging_lines = LOGGING_CFG_REGEX.findall(airflow_cfg_text)[0].strip().splitlines() + assert "remote_logging = True" in logging_lines + + +def test_should_raise_error_when_both_elasticsearch_and_opensearch_enabled(): + with pytest.raises(CalledProcessError) as ex_ctx: + render_chart( + values={ + "elasticsearch": { + "enabled": True, + "secretName": "test-elastic-secret", + }, + "opensearch": { + "enabled": True, + "secretName": "test-elastic-secret", + }, + }, + show_only=[SCHEDULER_DEPLOYMENT_TEMPLATE], + ) + assert ( + "You must not set both values elasticsearch.enabled and opensearch.enabled" + in ex_ctx.value.stderr.decode() + ) diff --git a/helm_tests/security/test_elasticsearch_secret.py b/helm_tests/security/test_elasticsearch_secret.py index 57f2f93f0d079..78bc3da9ae99c 100644 --- a/helm_tests/security/test_elasticsearch_secret.py +++ b/helm_tests/security/test_elasticsearch_secret.py @@ -17,7 +17,6 @@ from __future__ import annotations import base64 -from subprocess import CalledProcessError import jmespath import pytest @@ -28,50 +27,6 @@ class TestElasticsearchSecret: """Tests elasticsearch secret.""" - def test_should_not_generate_a_document_if_elasticsearch_disabled(self): - docs = render_chart( - values={"elasticsearch": {"enabled": False}}, - show_only=["templates/secrets/elasticsearch-secret.yaml"], - ) - - assert len(docs) == 0 - - def test_should_raise_error_when_connection_not_provided(self): - with pytest.raises(CalledProcessError) as ex_ctx: - render_chart( - values={ - "elasticsearch": { - "enabled": True, - } - }, - show_only=["templates/secrets/elasticsearch-secret.yaml"], - ) - assert ( - "You must set one of the values elasticsearch.secretName or elasticsearch.connection " - "when using a Elasticsearch" in ex_ctx.value.stderr.decode() - ) - - def test_should_raise_error_when_conflicting_options(self): - with pytest.raises(CalledProcessError) as ex_ctx: - render_chart( - values={ - "elasticsearch": { - "enabled": True, - "secretName": "my-test", - "connection": { - "user": "username!@#$%%^&*()", - "pass": "password!@#$%%^&*()", - "host": "elastichostname", - }, - }, - }, - show_only=["templates/secrets/elasticsearch-secret.yaml"], - ) - assert ( - "You must not set both values elasticsearch.secretName and elasticsearch.connection" - in ex_ctx.value.stderr.decode() - ) - def _get_connection(self, values: dict) -> str: docs = render_chart( values=values, diff --git a/helm_tests/security/test_opensearch_secret.py b/helm_tests/security/test_opensearch_secret.py new file mode 100644 index 0000000000000..fa6803782f6df --- /dev/null +++ b/helm_tests/security/test_opensearch_secret.py @@ -0,0 +1,119 @@ +# 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 __future__ import annotations + +import base64 + +import jmespath +import pytest + +from tests.charts.helm_template_generator import render_chart + + +class TestOpenSearchSecret: + """Tests opensearch secret.""" + + def _get_connection(self, values: dict) -> str: + docs = render_chart( + values=values, + show_only=["templates/secrets/opensearch-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( + { + "opensearch": { + "enabled": True, + "connection": { + "user": "username!@#$%%^&*()", + "pass": "password!@#$%%^&*()", + "host": "opensearchhostname", + }, + } + } + ) + + assert ( + connection + == "http://username%21%40%23$%25%25%5E&%2A%28%29:password%21%40%23$%25%25%5E&%2A%28%29@" + "opensearchhostname:9200" + ) + + def test_should_generate_secret_with_specified_port(self): + connection = self._get_connection( + { + "opensearch": { + "enabled": True, + "connection": { + "user": "username", + "pass": "password", + "host": "opensearchhostname", + "port": 2222, + }, + } + } + ) + + assert connection == "http://username:password@opensearchhostname:2222" + + @pytest.mark.parametrize("scheme", ["http", "https"]) + def test_should_generate_secret_with_specified_schemes(self, scheme): + connection = self._get_connection( + { + "opensearch": { + "enabled": True, + "connection": { + "scheme": scheme, + "user": "username", + "pass": "password", + "host": "opensearchhostname", + }, + } + } + ) + + assert f"{scheme}://username:password@opensearchhostname:9200" == connection + + @pytest.mark.parametrize( + "extra_conn_kwargs, expected_user_info", + [ + # When both user and password are empty. + ({}, ""), + # When password is empty + ({"user": "admin"}, ""), + # When user is empty + ({"pass": "password"}, ""), + # Valid username/password + ({"user": "admin", "pass": "password"}, "admin:password"), + ], + ) + def test_url_generated_when_user_pass_empty_combinations(self, extra_conn_kwargs, expected_user_info): + connection = self._get_connection( + { + "opensearch": { + "enabled": True, + "connection": {"host": "opensearchhostname", "port": 8080, **extra_conn_kwargs}, + } + } + ) + + if not expected_user_info: + assert connection == "http://opensearchhostname:8080" + else: + assert f"http://{expected_user_info}@opensearchhostname:8080" == connection