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 2021/08/05 14:45:39 UTC
[GitHub] [airflow] JavierLopezT opened a new pull request #17448: Aws secrets manager backend
JavierLopezT opened a new pull request #17448:
URL: https://github.com/apache/airflow/pull/17448
One of the main advantages of using AWS Secrets Manager is its ability to automatically create secrets of RDS databases and Redshift databases. Those secrets consist of several keys with their values, i.e user, pass, etc. Also, it is normal to store API Keys, sftp, or whatever using different values, as shown in the picture below:
With the current code, all the keys and values obtained from a secret are stored in the schema attribute of the conn object, unless you have just one key with the conn_uri in the value. Thus, the current situation is forcing to use Secrets Manager in a way it is not intended to.
With this proposed modification, you can use AWS Secrets Manager using keys and values and have some kind of freedom to choose different words for each key to make the get_conn work.
Third attempt. Coming from here: https://github.com/apache/airflow/pull/15104
@xinbinhuang I have tried to implement all your suggestions (thanks!) but regarding https://github.com/apache/airflow/pull/15104#discussion_r606090823 I received an error that Connection could not be imported. Maybe it is because I am using the secrets backend replacing the original file with a COPY in the Dockerfile, I dunno
@ dstandish Regarding https://github.com/apache/airflow/pull/15104#issuecomment-812596462, I like your suggestion but I would rather keep the code as it is now. It's been a while since my first pull request for this (more than a year https://github.com/apache/airflow/pull/9008) and I want to have it merge ASAP
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-898961663
> Can you also update the docs?
> http://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/secrets-backends/aws-secrets-manager.html
Done. If you have any suggestion about the docs, please let me know
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r699094690
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
Review comment:
```suggestion
def _format_uri_with_extra(self, secret, conn_string):
try:
extra_dict = secret['extra']
except KeyError:
return conn_string
conn_string = f"{conn_string}?{urlencode(extra_dict)}"
return conn_string
```
Or this should do what the function name says and _just_ get extra, and leave the formatting part to `get_uri_from_secret`:
```suggestion
def _get_encoded_extra(self, secret):
try:
extra_dict = secret['extra']
except KeyError:
return None
return urlencode(extra_dict)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-908980780
Pity it did not make it to yesterday's provider's release .. But it will go to the next one (BTW. I would like to wait a bit with merging the provider changes as I am just about to start a try of a new CHANGELOG preparation mechanism, so I'd love that you participate in it @JavierLopezT (and since the next release will come i ~3-4 weeks it can wait a couple of days with merging?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r702397014
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -55,6 +55,24 @@ class SecretsManagerBackend(BaseSecretsBackend, LoggingMixin):
You can also pass additional keyword arguments like ``aws_secret_access_key``, ``aws_access_key_id``
or ``region_name`` to this class and they would be passed on to Boto3 client.
+ There are two ways of storing secrets in Secret Manager for using them with this operator:
+ storing them as a conn URI in one field, or taking advantage of native approach of Secrets Manager
+ and storing them in multiple fields. There are certain words that will be searched in the name
+ of fileds for trying to retrieve a connection part. Those words are:
+
+ .. code-block:: ini
+
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
Review comment:
I just thought it was the standard way to define a code block in the doc. What should I put there instead?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r699095847
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
+
+ def get_uri_from_secret(self, secret):
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
Review comment:
Should we provide a mechanism for users to specify a key not in this list?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] SohailChamadia-everestek commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
SohailChamadia-everestek commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-950974688
This only works for connection but not for config like sql_alchemy_conn which can certainly benefit from the same
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-908942992
> I believe (please correct me if I am wrong), there is no backwards-compatibiity here? I think it should be possible to keep using the previous "full-url" storage, not only the feild-broken one.
You were right, I just added backwards-compatibility
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT removed a comment on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT removed a comment on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-933282983
Hello, this might sound ridiculous but I am trying to test this MR for https://github.com/apache/airflow/issues/18638#issuecomment-932991258 and I am unable to do it. Could you help me, please?
While I was coding the MR and for several months, I have been using the Secrets Backend extending my docker image as follow:
```
COPY plugins/aws_secrets_manager_backend.py /home/airflow/.local/lib/python3.9/site-packages/airflow/providers/amazon/aws/secrets/secrets_manager.py
COPY aws_config /home/airflow/.aws/config
COPY aws_credentials /home/airflow/.aws/credentials
```
In the code I had the custom defaults I needed, so no need for backend kwargs. Thus, my docker-compose only needed one env variable for having the backend working:
` AIRFLOW__SECRETS__BACKEND: airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend`
Now, I have pinned in my requirements file the amazon library `apache-airflow-providers-amazon==2.3.0rc1`. I have deleted the COPY command of the secrets file from the Dockerfile and added the kwargs as env variable as follow:
` AIRFLOW__SECRETS__BACKEND_KWARGS: '{"connections_prefix": "data", "full_url_mode": False, "sep": "_"}'`
I'm trying to connect to the secret `data_db_airflow_snowflake`. I have tried also with `connections_prefix` as an empty string (and the full name of the connection as an argument).
In any case, I am having an error like the following:
```
File "/opt/airflow/plugins/snowflake_hook.py", line 213, in get_pandas_df
df = super(SnowflakeHook, self).get_pandas_df(sql, parameters, **kwargs)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/hooks/dbapi.py", line 116, in get_pandas_df
with closing(self.get_conn()) as conn:
File "/opt/airflow/plugins/snowflake_hook.py", line 124, in get_conn
conn_config = self._get_conn_params()
File "/opt/airflow/plugins/snowflake_hook.py", line 63, in _get_conn_params
conn = self.get_connection(self.snowflake_conn_id)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/hooks/base.py", line 67, in get_connection
conn = Connection.get_connection_from_secrets(conn_id)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/models/connection.py", line 379, in get_connection_from_secrets
raise AirflowNotFoundException(f"The conn_id `{conn_id}` isn't defined")
airflow.exceptions.AirflowNotFoundException: The conn_id `db_snowflake_***` isn't defined
```
Entering in the container, the env variables, as well as the secrets file, seems to be right.
Any idea? Thank you very much
@mik-laj @potiuk @uranusjr
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r702392881
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -55,6 +55,24 @@ class SecretsManagerBackend(BaseSecretsBackend, LoggingMixin):
You can also pass additional keyword arguments like ``aws_secret_access_key``, ``aws_access_key_id``
or ``region_name`` to this class and they would be passed on to Boto3 client.
+ There are two ways of storing secrets in Secret Manager for using them with this operator:
+ storing them as a conn URI in one field, or taking advantage of native approach of Secrets Manager
+ and storing them in multiple fields. There are certain words that will be searched in the name
+ of fileds for trying to retrieve a connection part. Those words are:
+
+ .. code-block:: ini
+
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
Review comment:
Why is this ini?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r702397014
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -55,6 +55,24 @@ class SecretsManagerBackend(BaseSecretsBackend, LoggingMixin):
You can also pass additional keyword arguments like ``aws_secret_access_key``, ``aws_access_key_id``
or ``region_name`` to this class and they would be passed on to Boto3 client.
+ There are two ways of storing secrets in Secret Manager for using them with this operator:
+ storing them as a conn URI in one field, or taking advantage of native approach of Secrets Manager
+ and storing them in multiple fields. There are certain words that will be searched in the name
+ of fileds for trying to retrieve a connection part. Those words are:
+
+ .. code-block:: ini
+
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
Review comment:
I just thought it was the standard way to define a code block in the doc. What should I put there instead?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r699098468
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
+
+ def get_uri_from_secret(self, secret):
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
+
+ conn_d = {}
+ for conn_field, possible_words in possible_words_for_conn_fields.items():
+ try:
+ conn_d[conn_field] = [v for k, v in secret.items() if k in possible_words][0]
+ except IndexError:
+ conn_d[conn_field] = ''
+
+ conn_type = conn_d['conn_type']
+ user = conn_d['user']
+ password = conn_d['password']
+ host = conn_d['host']
+ port = conn_d['port']
+ schema = conn_d['schema']
+ conn_string = f'{conn_type}://{user}:{password}@{host}:{port}/{schema}'
Review comment:
This is actually one of the situations where `str.format` works better:
```suggestion
conn_string = "{conn_type}://{user}:{password}@{host}:{port}/{schema}".format(**conn_d)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT edited a comment on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT edited a comment on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-933282983
Hello, this might sound ridiculous but I am trying to test this MR for https://github.com/apache/airflow/issues/18638#issuecomment-932991258 and I am unable to do it. Could you help me, please?
While I was coding the MR and for several months, I have been using the Secrets Backend extending my docker image as follow:
```
COPY plugins/aws_secrets_manager_backend.py /home/airflow/.local/lib/python3.9/site-packages/airflow/providers/amazon/aws/secrets/secrets_manager.py
COPY aws_config /home/airflow/.aws/config
COPY aws_credentials /home/airflow/.aws/credentials
```
In the code I had the custom defaults I needed, so no need for backend kwargs. Thus, my docker-compose only needed one env variable for having the backend working:
` AIRFLOW__SECRETS__BACKEND: airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend`
Now, I have pinned in my requirements file the amazon library `apache-airflow-providers-amazon==2.3.0rc1`. I have deleted the COPY command of the secrets file from the Dockerfile and added the kwargs as env variable as follow:
` AIRFLOW__SECRETS__BACKEND_KWARGS: '{"connections_prefix": "data", "full_url_mode": False, "sep": "_"}'`
I'm trying to connect to the secret `data_db_airflow_snowflake`. I have tried also with `connections_prefix` as an empty string (and the full name of the connection as an argument).
In any case, I am having an error like the following:
```
File "/opt/airflow/plugins/snowflake_hook.py", line 213, in get_pandas_df
df = super(SnowflakeHook, self).get_pandas_df(sql, parameters, **kwargs)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/hooks/dbapi.py", line 116, in get_pandas_df
with closing(self.get_conn()) as conn:
File "/opt/airflow/plugins/snowflake_hook.py", line 124, in get_conn
conn_config = self._get_conn_params()
File "/opt/airflow/plugins/snowflake_hook.py", line 63, in _get_conn_params
conn = self.get_connection(self.snowflake_conn_id)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/hooks/base.py", line 67, in get_connection
conn = Connection.get_connection_from_secrets(conn_id)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/models/connection.py", line 379, in get_connection_from_secrets
raise AirflowNotFoundException(f"The conn_id `{conn_id}` isn't defined")
airflow.exceptions.AirflowNotFoundException: The conn_id `db_snowflake_***` isn't defined
```
Entering in the container, the env variables, as well as the secrets file, seems to be right.
Any idea? Thank you very much
@mik-laj @potiuk @uranusjr
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil edited a comment on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-916511819
Argh :( I merged with the same PR title, we should have had a better Commit Message / PR Title
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-908942992
> I believe (please correct me if I am wrong), there is no backwards-compatibiity here? I think it should be possible to keep using the previous "full-url" storage, not only the feild-broken one.
You were right, I just added backwards-compatibility
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r702392945
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -81,6 +105,7 @@ def __init__(
profile_name: Optional[str] = None,
sep: str = "/",
full_url_mode: bool = True,
+ extra_conn_words: Optional[dict] = {},
Review comment:
[Don’t usage mutable values as function argument default](https://docs.python-guide.org/writing/gotchas/)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r702302154
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
+
+ def get_uri_from_secret(self, secret):
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
Review comment:
Done
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-933282983
Hello, this might sound ridiculous but I am trying to test this MR for https://github.com/apache/airflow/issues/18638#issuecomment-932991258 and I am unable to do it. Could you help me, please?
While I was coding the MR and for several months, I have been using the Secrets Backend extending my docker image as follow:
```
COPY plugins/aws_secrets_manager_backend.py /home/airflow/.local/lib/python3.9/site-packages/airflow/providers/amazon/aws/secrets/secrets_manager.py
COPY aws_config /home/airflow/.aws/config
COPY aws_credentials /home/airflow/.aws/credentials
```
In the code I had the custom defaults I needed, so no need for backend kwargs. Thus, my docker-compose only needed one env variable for having the backend working:
AIRFLOW__SECRETS__BACKEND: airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend
Now, I have pinned in my requirements file the amazon library `apache-airflow-providers-amazon==2.3.0rc1`. I have deleted the COPY command of the secrets file from the Dockerfile and added the kwargs as env variable as follow:
AIRFLOW__SECRETS__BACKEND_KWARGS: '{"connections_prefix": "data", "full_url_mode": False, "sep": "_"}'
I'm trying to connect to the secret 'data_db_airflow_snowflake'. I have tried both with connections_prefix as an empty string (and the full name of the connection as argument) and the above example without the 'data_' part.
In both cases, I am having an error like the following:
```
File "/opt/airflow/plugins/snowflake_hook.py", line 213, in get_pandas_df
df = super(SnowflakeHook, self).get_pandas_df(sql, parameters, **kwargs)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/hooks/dbapi.py", line 116, in get_pandas_df
with closing(self.get_conn()) as conn:
File "/opt/airflow/plugins/snowflake_hook.py", line 124, in get_conn
conn_config = self._get_conn_params()
File "/opt/airflow/plugins/snowflake_hook.py", line 63, in _get_conn_params
conn = self.get_connection(self.snowflake_conn_id)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/hooks/base.py", line 67, in get_connection
conn = Connection.get_connection_from_secrets(conn_id)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/models/connection.py", line 379, in get_connection_from_secrets
raise AirflowNotFoundException(f"The conn_id `{conn_id}` isn't defined")
airflow.exceptions.AirflowNotFoundException: The conn_id `db_snowflake_***` isn't defined
```
Entering in the container, the env variables, as well as the secrets file, seems to be right.
Any idea? Thank you very much
@mik-laj @potiuk @uranusjr
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-893875377
Can you also update the docs?
http://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/secrets-backends/aws-secrets-manager.html
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r699098468
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
+
+ def get_uri_from_secret(self, secret):
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
+
+ conn_d = {}
+ for conn_field, possible_words in possible_words_for_conn_fields.items():
+ try:
+ conn_d[conn_field] = [v for k, v in secret.items() if k in possible_words][0]
+ except IndexError:
+ conn_d[conn_field] = ''
+
+ conn_type = conn_d['conn_type']
+ user = conn_d['user']
+ password = conn_d['password']
+ host = conn_d['host']
+ port = conn_d['port']
+ schema = conn_d['schema']
+ conn_string = f'{conn_type}://{user}:{password}@{host}:{port}/{schema}'
Review comment:
This is actually one of the situations where `str.format` works better:
```suggestion
conn_string = f'{conn_type}://{user}:{password}@{host}:{port}/{schema}'.format(**conn_d)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r702396133
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -81,6 +105,7 @@ def __init__(
profile_name: Optional[str] = None,
sep: str = "/",
full_url_mode: bool = True,
+ extra_conn_words: Optional[dict] = {},
Review comment:
Oh. YEAH. That one is so non-obvious, but so dangerous. I think that was about the only really useful Pylint check that we miss now. But I just checked that we could use https://pypi.org/project/flake8-mutable/ extension.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT edited a comment on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT edited a comment on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-933282983
Hello, this might sound ridiculous but I am trying to test this MR for https://github.com/apache/airflow/issues/18638#issuecomment-932991258 and I am unable to do it. Could you help me, please?
While I was coding the MR and for several months, I have been using the Secrets Backend extending my docker image as follow:
```
COPY plugins/aws_secrets_manager_backend.py /home/airflow/.local/lib/python3.9/site-packages/airflow/providers/amazon/aws/secrets/secrets_manager.py
COPY aws_config /home/airflow/.aws/config
COPY aws_credentials /home/airflow/.aws/credentials
```
In the code I had the custom defaults I needed, so no need for backend kwargs. Thus, my docker-compose only needed one env variable for having the backend working:
` AIRFLOW__SECRETS__BACKEND: airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend`
Now, I have pinned in my requirements file the amazon library `apache-airflow-providers-amazon==2.3.0rc1`. I have deleted the COPY command of the secrets file from the Dockerfile and added the kwargs as env variable as follow:
` AIRFLOW__SECRETS__BACKEND_KWARGS: '{"connections_prefix": "data", "full_url_mode": False, "sep": "_"}'`
I'm trying to connect to the secret 'data_db_airflow_snowflake'. I have tried both with connections_prefix as an empty string (and the full name of the connection as argument) and the above example without the 'data_' part.
In both cases, I am having an error like the following:
```
File "/opt/airflow/plugins/snowflake_hook.py", line 213, in get_pandas_df
df = super(SnowflakeHook, self).get_pandas_df(sql, parameters, **kwargs)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/hooks/dbapi.py", line 116, in get_pandas_df
with closing(self.get_conn()) as conn:
File "/opt/airflow/plugins/snowflake_hook.py", line 124, in get_conn
conn_config = self._get_conn_params()
File "/opt/airflow/plugins/snowflake_hook.py", line 63, in _get_conn_params
conn = self.get_connection(self.snowflake_conn_id)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/hooks/base.py", line 67, in get_connection
conn = Connection.get_connection_from_secrets(conn_id)
File "/home/airflow/.local/lib/python3.9/site-packages/airflow/models/connection.py", line 379, in get_connection_from_secrets
raise AirflowNotFoundException(f"The conn_id `{conn_id}` isn't defined")
airflow.exceptions.AirflowNotFoundException: The conn_id `db_snowflake_***` isn't defined
```
Entering in the container, the env variables, as well as the secrets file, seems to be right.
Any idea? Thank you very much
@mik-laj @potiuk @uranusjr
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r692378470
##########
File path: docs/apache-airflow-providers-amazon/secrets-backends/aws-secrets-manager.rst
##########
@@ -71,18 +68,43 @@ Verify that you can get the secret:
"ARN": "arn:aws:secretsmanager:us-east-2:314524341751:secret:airflow/connections/smtp_default-7meuul",
"Name": "airflow/connections/smtp_default",
"VersionId": "34f90eff-ea21-455a-9c8f-5ee74b21be672",
- "SecretString": "smtps://user:host@relay.example.com:465",
+ "SecretString": "{\n \"user\":\"nice_user\",\n \"pass\":\"this_is_the_password\"\n,
+ \n \"host\":\"ec2.8399.com\"\n,\n \"port\":\"999\"\n}\n",
"VersionStages": [
"AWSCURRENT"
],
"CreatedDate": "2020-04-08T02:10:35.132000+01:00"
}
-The value of the secret must be the :ref:`connection URI representation <generating_connection_uri>`
-of the connection object.
Storing and Retrieving Variables
""""""""""""""""""""""""""""""""
If you have set ``variables_prefix`` as ``airflow/variables``, then for an Variable key of ``hello``,
you would want to store your Variable at ``airflow/variables/hello``.
+
+Optional lookup
+"""""""""""""""
+
+Optionally connections, variables, or config may be looked up exclusive of each other or in any combination.
+This will prevent requests being sent to AWS Secrets Manager for the excluded type.
+
+If you want to look up some and not others in AWS Secrets Manager you may do so by setting the relevant ``*_prefix`` parameter of the ones to be excluded as ``null``.
+
+For example, if you want to set parameter ``connections_prefix`` to ``"airflow/connections"`` and not look up variables, your configuration file should look like this:
+
+.. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections", "variables_prefix": null, "profile_name": "default"}
+
+Storing Google Cloud Secrets
Review comment:
I am not sure I understand the question. But if you are asking whether or not this backend works with Google cloud secrets, it does. I have a few Dags using GC operators with a secret in secrets manager as connection
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r683842856
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -96,28 +97,74 @@ def __init__(
@cached_property
def client(self):
- """Create a Secrets Manager client"""
+ """
+ Create a Secrets Manager client
+ """
session = boto3.session.Session(
- profile_name=self.profile_name,
+ profile_name=self.profile_name
)
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ kvs = "&".join([f"{key}={value}" for key, value in extra_dict.items()])
Review comment:
What do you think about [urllib.parse.urlencode(](https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlencode). I'm afraid the current implementation may have problems with some special characters.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r699094690
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
Review comment:
```suggestion
def _get_extra(self, secret, conn_string):
try:
extra_dict = secret['extra']
except KeyError:
return conn_string
conn_string = f"{conn_string}?{urlencode(extra_dict)}"
return conn_string
```
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
+
+ def get_uri_from_secret(self, secret):
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
Review comment:
Should we provide a mechanism for users to specify a key not in this list?
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
Review comment:
```suggestion
def _format_uri_with_extra(self, secret, conn_string):
try:
extra_dict = secret['extra']
except KeyError:
return conn_string
conn_string = f"{conn_string}?{urlencode(extra_dict)}"
return conn_string
```
Or this should do what the function name says and _just_ get extra, and leave the formatting part to `get_uri_from_secret`:
```suggestion
def _get_encoded_extra(self, secret):
try:
extra_dict = secret['extra']
except KeyError:
return None
return urlencode(extra_dict)
```
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
+
+ def get_uri_from_secret(self, secret):
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
+
+ conn_d = {}
+ for conn_field, possible_words in possible_words_for_conn_fields.items():
+ try:
+ conn_d[conn_field] = [v for k, v in secret.items() if k in possible_words][0]
+ except IndexError:
+ conn_d[conn_field] = ''
+
+ conn_type = conn_d['conn_type']
+ user = conn_d['user']
+ password = conn_d['password']
+ host = conn_d['host']
+ port = conn_d['port']
+ schema = conn_d['schema']
+ conn_string = f'{conn_type}://{user}:{password}@{host}:{port}/{schema}'
Review comment:
This is actually one of the situations where `str.format` works better:
```suggestion
conn_string = f'{conn_type}://{user}:{password}@{host}:{port}/{schema}'.format(**conn_d)
```
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
+
+ def get_uri_from_secret(self, secret):
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
+
+ conn_d = {}
+ for conn_field, possible_words in possible_words_for_conn_fields.items():
+ try:
+ conn_d[conn_field] = [v for k, v in secret.items() if k in possible_words][0]
+ except IndexError:
+ conn_d[conn_field] = ''
+
+ conn_type = conn_d['conn_type']
+ user = conn_d['user']
+ password = conn_d['password']
+ host = conn_d['host']
+ port = conn_d['port']
+ schema = conn_d['schema']
+ conn_string = f'{conn_type}://{user}:{password}@{host}:{port}/{schema}'
Review comment:
This is actually one of the situations where `str.format` works better:
```suggestion
conn_string = "{conn_type}://{user}:{password}@{host}:{port}/{schema}".format(**conn_d)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-908979691
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-908980780
Pity it did not make it to yesterday's provider's release .. But it will go to the next one (BTW. I would like to wait a bit with merging the provider changes as I am just about to start a try of a new CHANGELOG preparation mechanism, so I'd love that you participate in it @JavierLopezT (and since the next release will come i ~3-4 weeks it can wait a couple of days with merging?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r699094690
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
Review comment:
```suggestion
def _get_extra(self, secret, conn_string):
try:
extra_dict = secret['extra']
except KeyError:
return conn_string
conn_string = f"{conn_string}?{urlencode(extra_dict)}"
return conn_string
```
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
+
+ def get_uri_from_secret(self, secret):
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
Review comment:
Should we provide a mechanism for users to specify a key not in this list?
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
Review comment:
```suggestion
def _format_uri_with_extra(self, secret, conn_string):
try:
extra_dict = secret['extra']
except KeyError:
return conn_string
conn_string = f"{conn_string}?{urlencode(extra_dict)}"
return conn_string
```
Or this should do what the function name says and _just_ get extra, and leave the formatting part to `get_uri_from_secret`:
```suggestion
def _get_encoded_extra(self, secret):
try:
extra_dict = secret['extra']
except KeyError:
return None
return urlencode(extra_dict)
```
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
+
+ def get_uri_from_secret(self, secret):
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
+
+ conn_d = {}
+ for conn_field, possible_words in possible_words_for_conn_fields.items():
+ try:
+ conn_d[conn_field] = [v for k, v in secret.items() if k in possible_words][0]
+ except IndexError:
+ conn_d[conn_field] = ''
+
+ conn_type = conn_d['conn_type']
+ user = conn_d['user']
+ password = conn_d['password']
+ host = conn_d['host']
+ port = conn_d['port']
+ schema = conn_d['schema']
+ conn_string = f'{conn_type}://{user}:{password}@{host}:{port}/{schema}'
Review comment:
This is actually one of the situations where `str.format` works better:
```suggestion
conn_string = f'{conn_type}://{user}:{password}@{host}:{port}/{schema}'.format(**conn_d)
```
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
+
+ def get_uri_from_secret(self, secret):
+ possible_words_for_conn_fields = {
+ 'user': ['user', 'username', 'login', 'user_name'],
+ 'password': ['password', 'pass', 'key'],
+ 'host': ['host', 'remote_host', 'server'],
+ 'port': ['port'],
+ 'schema': ['database', 'schema'],
+ 'conn_type': ['conn_type', 'conn_id', 'connection_type', 'engine'],
+ }
+
+ conn_d = {}
+ for conn_field, possible_words in possible_words_for_conn_fields.items():
+ try:
+ conn_d[conn_field] = [v for k, v in secret.items() if k in possible_words][0]
+ except IndexError:
+ conn_d[conn_field] = ''
+
+ conn_type = conn_d['conn_type']
+ user = conn_d['user']
+ password = conn_d['password']
+ host = conn_d['host']
+ port = conn_d['port']
+ schema = conn_d['schema']
+ conn_string = f'{conn_type}://{user}:{password}@{host}:{port}/{schema}'
Review comment:
This is actually one of the situations where `str.format` works better:
```suggestion
conn_string = "{conn_type}://{user}:{password}@{host}:{port}/{schema}".format(**conn_d)
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-908979691
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] JavierLopezT commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
JavierLopezT commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r692378470
##########
File path: docs/apache-airflow-providers-amazon/secrets-backends/aws-secrets-manager.rst
##########
@@ -71,18 +68,43 @@ Verify that you can get the secret:
"ARN": "arn:aws:secretsmanager:us-east-2:314524341751:secret:airflow/connections/smtp_default-7meuul",
"Name": "airflow/connections/smtp_default",
"VersionId": "34f90eff-ea21-455a-9c8f-5ee74b21be672",
- "SecretString": "smtps://user:host@relay.example.com:465",
+ "SecretString": "{\n \"user\":\"nice_user\",\n \"pass\":\"this_is_the_password\"\n,
+ \n \"host\":\"ec2.8399.com\"\n,\n \"port\":\"999\"\n}\n",
"VersionStages": [
"AWSCURRENT"
],
"CreatedDate": "2020-04-08T02:10:35.132000+01:00"
}
-The value of the secret must be the :ref:`connection URI representation <generating_connection_uri>`
-of the connection object.
Storing and Retrieving Variables
""""""""""""""""""""""""""""""""
If you have set ``variables_prefix`` as ``airflow/variables``, then for an Variable key of ``hello``,
you would want to store your Variable at ``airflow/variables/hello``.
+
+Optional lookup
+"""""""""""""""
+
+Optionally connections, variables, or config may be looked up exclusive of each other or in any combination.
+This will prevent requests being sent to AWS Secrets Manager for the excluded type.
+
+If you want to look up some and not others in AWS Secrets Manager you may do so by setting the relevant ``*_prefix`` parameter of the ones to be excluded as ``null``.
+
+For example, if you want to set parameter ``connections_prefix`` to ``"airflow/connections"`` and not look up variables, your configuration file should look like this:
+
+.. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections", "variables_prefix": null, "profile_name": "default"}
+
+Storing Google Cloud Secrets
Review comment:
I am not sure I understand the question. But if you are asking whether or not this backend works with Google cloud connections, it does. I have a few Dags using GC operators with a secret in secrets manager as connection
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil merged pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #17448:
URL: https://github.com/apache/airflow/pull/17448
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil commented on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-916511819
Argh :( Merged with the same PR description, we should have had a better Commit Message / PR Title
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r683842856
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -96,28 +97,74 @@ def __init__(
@cached_property
def client(self):
- """Create a Secrets Manager client"""
+ """
+ Create a Secrets Manager client
+ """
session = boto3.session.Session(
- profile_name=self.profile_name,
+ profile_name=self.profile_name
)
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ kvs = "&".join([f"{key}={value}" for key, value in extra_dict.items()])
Review comment:
What do you think about [urllib.parse.urlencode](https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urlencode). I'm afraid the current implementation may have problems with some special characters.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r699094690
##########
File path: airflow/providers/amazon/aws/secrets/secrets_manager.py
##########
@@ -75,49 +80,100 @@ def __init__(
config_prefix: str = 'airflow/config',
profile_name: Optional[str] = None,
sep: str = "/",
+ full_url_mode: bool = True,
**kwargs,
):
super().__init__()
if connections_prefix is not None:
- self.connections_prefix = connections_prefix.rstrip("/")
+ self.connections_prefix = connections_prefix.rstrip(sep)
else:
self.connections_prefix = connections_prefix
if variables_prefix is not None:
- self.variables_prefix = variables_prefix.rstrip('/')
+ self.variables_prefix = variables_prefix.rstrip(sep)
else:
self.variables_prefix = variables_prefix
if config_prefix is not None:
- self.config_prefix = config_prefix.rstrip('/')
+ self.config_prefix = config_prefix.rstrip(sep)
else:
self.config_prefix = config_prefix
self.profile_name = profile_name
self.sep = sep
+ self.full_url_mode = full_url_mode
self.kwargs = kwargs
@cached_property
def client(self):
"""Create a Secrets Manager client"""
- session = boto3.session.Session(
- profile_name=self.profile_name,
- )
+ session = boto3.session.Session(profile_name=self.profile_name)
+
return session.client(service_name="secretsmanager", **self.kwargs)
- def get_conn_uri(self, conn_id: str) -> Optional[str]:
+ def _get_extra(self, secret, conn_string):
+ if 'extra' in secret:
+ extra_dict = secret['extra']
+ conn_string = f"{conn_string}?{urlencode(extra_dict)}"
+ return conn_string
+
+ return conn_string
Review comment:
```suggestion
def _get_extra(self, secret, conn_string):
try:
extra_dict = secret['extra']
except KeyError:
return conn_string
conn_string = f"{conn_string}?{urlencode(extra_dict)}"
return conn_string
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r692371810
##########
File path: docs/apache-airflow-providers-amazon/secrets-backends/aws-secrets-manager.rst
##########
@@ -71,18 +68,43 @@ Verify that you can get the secret:
"ARN": "arn:aws:secretsmanager:us-east-2:314524341751:secret:airflow/connections/smtp_default-7meuul",
"Name": "airflow/connections/smtp_default",
"VersionId": "34f90eff-ea21-455a-9c8f-5ee74b21be672",
- "SecretString": "smtps://user:host@relay.example.com:465",
+ "SecretString": "{\n \"user\":\"nice_user\",\n \"pass\":\"this_is_the_password\"\n,
+ \n \"host\":\"ec2.8399.com\"\n,\n \"port\":\"999\"\n}\n",
"VersionStages": [
"AWSCURRENT"
],
"CreatedDate": "2020-04-08T02:10:35.132000+01:00"
}
-The value of the secret must be the :ref:`connection URI representation <generating_connection_uri>`
-of the connection object.
Storing and Retrieving Variables
""""""""""""""""""""""""""""""""
If you have set ``variables_prefix`` as ``airflow/variables``, then for an Variable key of ``hello``,
you would want to store your Variable at ``airflow/variables/hello``.
+
+Optional lookup
+"""""""""""""""
+
+Optionally connections, variables, or config may be looked up exclusive of each other or in any combination.
+This will prevent requests being sent to AWS Secrets Manager for the excluded type.
+
+If you want to look up some and not others in AWS Secrets Manager you may do so by setting the relevant ``*_prefix`` parameter of the ones to be excluded as ``null``.
+
+For example, if you want to set parameter ``connections_prefix`` to ``"airflow/connections"`` and not look up variables, your configuration file should look like this:
+
+.. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections", "variables_prefix": null, "profile_name": "default"}
+
+Storing Google Cloud Secrets
Review comment:
Just checking - does AWS secret manager allows pass-through to GCP's secrets ?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kaxil edited a comment on pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#issuecomment-916511819
Argh :( I merged with the same PR description, we should have had a better Commit Message / PR Title
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #17448: Aws secrets manager backend
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #17448:
URL: https://github.com/apache/airflow/pull/17448#discussion_r692382093
##########
File path: docs/apache-airflow-providers-amazon/secrets-backends/aws-secrets-manager.rst
##########
@@ -71,18 +68,43 @@ Verify that you can get the secret:
"ARN": "arn:aws:secretsmanager:us-east-2:314524341751:secret:airflow/connections/smtp_default-7meuul",
"Name": "airflow/connections/smtp_default",
"VersionId": "34f90eff-ea21-455a-9c8f-5ee74b21be672",
- "SecretString": "smtps://user:host@relay.example.com:465",
+ "SecretString": "{\n \"user\":\"nice_user\",\n \"pass\":\"this_is_the_password\"\n,
+ \n \"host\":\"ec2.8399.com\"\n,\n \"port\":\"999\"\n}\n",
"VersionStages": [
"AWSCURRENT"
],
"CreatedDate": "2020-04-08T02:10:35.132000+01:00"
}
-The value of the secret must be the :ref:`connection URI representation <generating_connection_uri>`
-of the connection object.
Storing and Retrieving Variables
""""""""""""""""""""""""""""""""
If you have set ``variables_prefix`` as ``airflow/variables``, then for an Variable key of ``hello``,
you would want to store your Variable at ``airflow/variables/hello``.
+
+Optional lookup
+"""""""""""""""
+
+Optionally connections, variables, or config may be looked up exclusive of each other or in any combination.
+This will prevent requests being sent to AWS Secrets Manager for the excluded type.
+
+If you want to look up some and not others in AWS Secrets Manager you may do so by setting the relevant ``*_prefix`` parameter of the ones to be excluded as ``null``.
+
+For example, if you want to set parameter ``connections_prefix`` to ``"airflow/connections"`` and not look up variables, your configuration file should look like this:
+
+.. code-block:: ini
+
+ [secrets]
+ backend = airflow.providers.amazon.aws.secrets.secrets_manager.SecretsManagerBackend
+ backend_kwargs = {"connections_prefix": "airflow/connections", "variables_prefix": null, "profile_name": "default"}
+
+Storing Google Cloud Secrets
Review comment:
Ah. I see now. I misunderstood it. It looks a bit strange to see Google in AWS secret manager. I understand now that you can configure Google service account in AWS. I thought of something else (it looked like you can configure the backend so that it will look up secrets stored in GCP secret manager 😱 .... Maybe you should change header to:
`Example of storing Google Secrets in AWS Secrets Manger`
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org