Get Airflow configs with sensitive data from Secret Backends #9645

kaxil · 2020-07-03T15:41:37Z

We should be able to get the followings config containing passwords and other sensitive data from Secret Backends:

        ('core', 'sql_alchemy_conn'),
        ('core', 'fernet_key'),
        ('celery', 'broker_url'),
        ('celery', 'flower_basic_auth'),
        ('celery', 'result_backend'),
        ('atlas', 'password'),
        ('smtp', 'smtp_password'),
        ('ldap', 'bind_password'),
        ('kubernetes', 'git_password'),

Make sure to mark the boxes below before creating PR: [x]

Description above provides context of the change
Unit tests coverage for changes (not needed for documentation changes)
Target Github ISSUE in description if exists
Commits follow "How to write a good git commit message"
Relevant documentation is updated including usage instructions.
I will engage committers as explained in Contribution Workflow Example.

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

airflow/configuration.py

ashb · 2020-07-03T15:57:04Z

airflow/configuration.py

Hmmmm, not a huge fan of the duplication here and in airflow/secrets/__init__.py.

Since this would only be called after the class is constructed would we instead do

Suggested change

@cached_property

def _secrets_backend_client(self):

if super().has_option('secrets', 'backend') is False:

return None

secrets_backend_cls = super().get('secrets', 'backend')

if not secrets_backend_cls:

return None

print("secrets_backend_cls", secrets_backend_cls)

secrets_backend = import_string(secrets_backend_cls)

if secrets_backend:

try:

alternative_secrets_config_dict = json.loads(

super().get('secrets', 'backend_kwargs', fallback='{}')

)

except json.JSONDecodeError:

alternative_secrets_config_dict = {}

return secrets_backend(**alternative_secrets_config_dict)

@cached_property

def _secrets_backend_client(self):

if super().has_option('secrets', 'backend') is False:

return None

from airflow.secrets import secrets_backend_list

return secrets_backend_list

(give or take).

By delaying the import to inside the function no import loop is created.

This probably also means adding a get_config(section, key) to airflow.secrets, and possibly changing the default implemenation from NotImplementedError to return None.

Note: we should pass they key and section in directly, and let the secret backends convert that to a single parameter if needed (for instance we could have /config/core/sql_alchemy_conn, rather than config/core__sql_alchemy_conn)

The main reason there is some code duplication here is we can't use airflow.secrets as otherwise we would have infinite loop (even when the import is inside the loop) since the DEFAULT_SECRET_BACKEND ( Environment Variable and Connection ) needs sql_achemy_conn value.

I can separate the code to a different file and import that file at both places if the concern is just duplication

sql_alchemy_conn_secret = conn/sql_alchemy_conn

should already allow that.

Whatever they pass to sql_alchemy_conn_secret will be directly used to get secret

Updated with TYPE_CHECKING in ed8cc1a

ashb · 2020-07-03T16:04:33Z

airflow/providers/amazon/aws/secrets/secrets_manager.py

Suggested change

And if configurations_prefix is ``airflow/configurations/sql_alchemy_conn``, this would be accessible

And if configurations_prefix is ``airflow/config/core/sql_alchemy_conn``, this would be accessible

ashb · 2020-07-03T16:04:39Z

airflow/providers/amazon/aws/secrets/secrets_manager.py

Suggested change

if you provide ``{"configurations_prefix": "airflow/configurations"}`` and request variable

if you provide ``{"configurations_prefix": "airflow/config"}`` and request variable

ashb · 2020-07-03T16:04:49Z

airflow/providers/amazon/aws/secrets/secrets_manager.py

Suggested change

configurations_prefix: str = 'airflow/configuration',

config_prefix: str = 'airflow/config',

etc (haven't made this change everywhere)

:D I went back and forth with this

ashb · 2020-07-03T16:05:27Z

airflow/secrets/base_secrets.py

Suggested change

def get_configuration(self, key: str) -> Optional[str]:

def get_configuration(self, section: str, key: str) -> Optional[str]:

…kends

…ret Backends

…rom Secret Backends

… data from Secret Backends

aneesh-joseph · 2020-07-07T08:02:36Z

thank you for picking this one and sorting it quickly @kaxil - this is super useful, hoping to see this in the next 1.10.* release

aneesh-joseph · 2020-07-07T08:12:00Z

airflow/configuration.py

+        env_var_cmd = env_var + '_SECRET'
+        if env_var_cmd in os.environ:
+            # if this is a valid command key...
+            if (section, key) in self.sensitive_config_values:
+                return _get_config_value_from_secret_backend(os.environ[env_var_cmd])


Suggested change

env_var_cmd = env_var + '_SECRET'

if env_var_cmd in os.environ:

# if this is a valid command key...

if (section, key) in self.sensitive_config_values:

return _get_config_value_from_secret_backend(os.environ[env_var_cmd])

env_var_secret_path = env_var + '_SECRET'

if env_var_secret_path in os.environ:

# if this is a valid secret key...

if (section, key) in self.sensitive_config_values:

return _get_config_value_from_secret_backend(os.environ[env_var_secret_path])

Fixed in 032d33f

ashb · 2020-07-07T09:01:54Z

airflow/secrets/base_secrets.py

        """
        raise NotImplementedError()
+
+    def get_config(self, key: str) -> Optional[str]:    # pylint: disable=unused-argument


I think this should be

Suggested change

def get_config(self, key: str) -> Optional[str]: # pylint: disable=unused-argument

def get_config(self, section: str, key: str) -> Optional[str]: # pylint: disable=unused-argument

Oh, except it comes from a single value in the config file. Hmmmm.

Perhaps update the example to show include a / in it?

[core] sql_alchemy_conn_secret = core/sql_alchemy_conn

Dunno.

added docs in 032d33f

ashb · 2020-07-07T09:03:41Z

docs/howto/set-config.rst

+.. code-block:: ini
+
+    [core]
+    sql_alchemy_conn_secret = sql_alchemy_conn


Hmmmmm, I wonder if this will be confusing for people running in Kube, who might expect this to come from a Kube secret.

(I don't have a suggestion on how to deal with this.)

for Kubernetes, the section would be kubernetes_secrets:

ashb · 2020-07-07T09:06:58Z

docs/howto/set-config.rst

 The universal order of precedence for all configuration options is as follows:

 #. set as an environment variable
 #. set as a command environment variable


Suggested change

#. set as a command environment variable

#. set as a command environment variable

#. set as a secret environment variable

Though the wording on this isn't that clear :(

fixed in 032d33f

…nsitive data from Secret Backends

ashb · 2020-07-08T11:10:09Z

airflow/configuration.py

    return output


+def _get_config_value_from_secret_backend(config_key):


Any reason why this is a function in the module, and not a method on the AirflowConfiguration class?

Just to keep it similar to def run_command(command):, no other reason

airflow/configuration.py

ashb · 2020-07-08T11:14:58Z

airflow/providers/amazon/aws/secrets/secrets_manager.py

+    If variables prefix is ``airflow/variables/hello``, this would be accessible
    if you provide ``{"variables_prefix": "airflow/variables"}`` and request variable key ``hello``.
+    And if configs_prefix is ``airflow/configs/sql_alchemy_conn``, this would be accessible
+    if you provide ``{"configs_prefix": "airflow/configs"}`` and request variable


Suggested change

if you provide ``{"configs_prefix": "airflow/configs"}`` and request variable

if you provide ``{"configs_prefix": "airflow/configs"}`` and request config

Updated in d8d5b8f

ashb · 2020-07-08T11:16:50Z

airflow/providers/amazon/aws/secrets/secrets_manager.py

        self,
        connections_prefix: str = 'airflow/connections',
        variables_prefix: str = 'airflow/variables',
+        configs_prefix: str = 'airflow/configs',


This should probably just be config, not configs -- as config is both singular and plural, at least in British English. (Configurations/configs) sounds odd to my ears.

fixed in 35d75e5

…with sensitive data from Secret Backends

…onfigs with sensitive data from Secret Backends

Adds support to AWS SSM for feature added in apache#9645

Adds support to Google Cloud CloudSecretManagerBackend for feature added in apache#9645

) Adds support to AWS SSM for feature added in #9645

…9645) (cherry picked from commit 2f31b30)

…023) Adds support to AWS SSM for feature added in apache/airflow#9645 GitOrigin-RevId: 2410f592a4ab160b377f1a9e5de3b7262b9851cc

kaxil requested a review from ashb July 3, 2020 15:41

boring-cyborg bot added area:secrets provider:amazon AWS/Amazon - related issues labels Jul 3, 2020

kaxil marked this pull request as draft July 3, 2020 15:41

ashb reviewed Jul 3, 2020

View reviewed changes

airflow/configuration.py Outdated Show resolved Hide resolved

ashb reviewed Jul 3, 2020

View reviewed changes

kaxil added 6 commits July 4, 2020 02:39

Get Airflow configs with sensitive data from Secret Backends

46010c5

fixup! Get Airflow configs with sensitive data from Secret Backends

6b1b4c3

fixup! fixup! Get Airflow configs with sensitive data from Secret Bac…

e0f1b25

…kends

fixup! fixup! fixup! Get Airflow configs with sensitive data from Sec…

67bc20d

…ret Backends

fixup! fixup! fixup! fixup! Get Airflow configs with sensitive data f…

c071725

…rom Secret Backends

fixup! fixup! fixup! fixup! fixup! Get Airflow configs with sensitive…

3b38109

… data from Secret Backends

kaxil force-pushed the get-secret-config-from-backend branch from ada4ac6 to 3b38109 Compare July 4, 2020 01:39

kaxil marked this pull request as ready for review July 4, 2020 01:40

aneesh-joseph reviewed Jul 7, 2020

View reviewed changes

ashb reviewed Jul 7, 2020

View reviewed changes

fixup! fixup! fixup! fixup! fixup! fixup! Get Airflow configs with se…

032d33f

…nsitive data from Secret Backends

ashb reviewed Jul 8, 2020

View reviewed changes

airflow/configuration.py Show resolved Hide resolved

ashb reviewed Jul 8, 2020

View reviewed changes

kaxil added 2 commits July 8, 2020 12:26

fixup! fixup! fixup! fixup! fixup! fixup! fixup! Get Airflow configs …

35d75e5

…with sensitive data from Secret Backends

fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup! Get Airflow c…

d8d5b8f

…onfigs with sensitive data from Secret Backends

ashb approved these changes Jul 8, 2020

View reviewed changes

kaxil merged commit 2f31b30 into apache:master Jul 8, 2020

kaxil deleted the get-secret-config-from-backend branch July 8, 2020 12:29

kaxil added this to the Airflow 1.10.12 milestone Jul 8, 2020

kaxil added a commit to astronomer/airflow that referenced this pull request Sep 19, 2020

Get Airflow configs with sensitive data from AWS Systems Manager

0a26d64

Adds support to AWS SSM for feature added in apache#9645

kaxil mentioned this pull request Sep 19, 2020

Get Airflow configs with sensitive data from AWS Systems Manager #11023

Merged

kaxil added a commit to astronomer/airflow that referenced this pull request Sep 19, 2020

Get Airflow configs with sensitive data from CloudSecretManagerBackend

fc4e423

Adds support to Google Cloud CloudSecretManagerBackend for feature added in apache#9645

kaxil mentioned this pull request Sep 19, 2020

Get Airflow configs with sensitive data from CloudSecretManagerBackend #11024

Merged

kaxil added a commit that referenced this pull request Sep 19, 2020

Get Airflow configs with sensitive data from AWS Systems Manager (#11023

2410f59

) Adds support to AWS SSM for feature added in #9645

cfei18 pushed a commit to cfei18/incubator-airflow that referenced this pull request Mar 5, 2021

Get Airflow configs with sensitive data from Secret Backends (apache#…

d98baf6

…9645) (cherry picked from commit 2f31b30)

-    @cached_property
-    def _secrets_backend_client(self):
-        if super().has_option('secrets', 'backend') is False:
-            return None
-        secrets_backend_cls = super().get('secrets', 'backend')
-        if not secrets_backend_cls:
-            return None
-        print("secrets_backend_cls", secrets_backend_cls)
-        secrets_backend = import_string(secrets_backend_cls)
-        if secrets_backend:
-            try:
-                alternative_secrets_config_dict = json.loads(
-                    super().get('secrets', 'backend_kwargs', fallback='{}')
-                )
-            except json.JSONDecodeError:
-                alternative_secrets_config_dict = {}
-            return secrets_backend(**alternative_secrets_config_dict)
+    @cached_property
+    def _secrets_backend_client(self):
+        if super().has_option('secrets', 'backend') is False:
+            return None
+        from airflow.secrets import secrets_backend_list
+        return secrets_backend_list

	And if configurations_prefix is ``airflow/configurations/sql_alchemy_conn``, this would be accessible
	And if configurations_prefix is ``airflow/config/core/sql_alchemy_conn``, this would be accessible

	if you provide ``{"configurations_prefix": "airflow/configurations"}`` and request variable
	if you provide ``{"configurations_prefix": "airflow/config"}`` and request variable

	configurations_prefix: str = 'airflow/configuration',
	config_prefix: str = 'airflow/config',

	def get_configuration(self, key: str) -> Optional[str]:
	def get_configuration(self, section: str, key: str) -> Optional[str]:

	def get_config(self, key: str) -> Optional[str]: # pylint: disable=unused-argument
	def get_config(self, section: str, key: str) -> Optional[str]: # pylint: disable=unused-argument

	#. set as a command environment variable
	#. set as a command environment variable
	#. set as a secret environment variable

		return output


		def _get_config_value_from_secret_backend(config_key):

	if you provide ``{"configs_prefix": "airflow/configs"}`` and request variable
	if you provide ``{"configs_prefix": "airflow/configs"}`` and request config

Get Airflow configs with sensitive data from Secret Backends #9645

Get Airflow configs with sensitive data from Secret Backends #9645

Uh oh!

Conversation

kaxil commented Jul 3, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaxil Jul 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaxil Jul 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aneesh-joseph commented Jul 7, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaxil Jul 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

kaxil Jul 3, 2020 •

edited

Loading

kaxil Jul 4, 2020 •

edited

Loading

kaxil Jul 7, 2020 •

edited

Loading