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 2020/03/24 00:51:19 UTC

[GitHub] [airflow] kaxil opened a new pull request #7846: Standardize SecretBackend class names

kaxil opened a new pull request #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846
 
 
   - AwsSsmSecretsBackend -> AwsSsmBackend
   - CloudSecretsManagerSecretsBackend -> CloudSecretsManagerBackend
   - VaultSecrets -> VaultBackend
   - EnvironmentVariablesSecretsBackend -> EnvironmentVariablesBackend
   - MetastoreSecretsBackend -> MetastoreBackend
   
   ---
   Issue link: WILL BE INSERTED BY [boring-cyborg](https://github.com/kaxil/boring-cyborg)
   
   Make sure to mark the boxes below before creating PR: [x]
   
   - [x] Description above provides context of the change
   - [x] Unit tests coverage for changes (not needed for documentation changes)
   - [x] Commits follow "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)"
   - [x] Relevant documentation is updated including usage instructions.
   - [x] I will engage committers as explained in [Contribution Workflow Example](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#contribution-workflow-example).
   
   ---
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   Read the [Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines) for more information.
   
   cc @xinbinhuang 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603547473
 
 
   `SystemManagerParametersBackend` stripping AWS as we don't use GCP or Hashicorp in their respective backends too.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603545387
 
 
   > I think SecretsManager may be something else. Go AWS!
   
   🤦‍♂ I am confused. WTH!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] xinbinhuang commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603096290
 
 
   Nice! Like the name standardization. I am also thinking if we should standardize the `connection_prefix` (used in AWS SSM & GCP Secrets Manager) and `connection_path` ( in Hashicorp Vault)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603545674
 
 
   `AwsSystemsManagerParameterStoreBackend` wow that name would be huge
   
   `AwsSMParameterStoreBackend` ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603436714
 
 
   > Nice! Like the name standardization. I am also thinking if we should standardize the `connection_prefix` (used in AWS SSM & GCP Secrets Manager) and `connection_path` ( in Hashicorp Vault)
   
   What name would you suggest ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603557518
 
 
   > For some context, this is a snapshot of the `parameter store` service. So I feel like better to still have the `ParameterStore` part
   > 
   > ![image](https://user-images.githubusercontent.com/27927454/77485455-6e635580-6dea-11ea-8ce8-a58920c62264.png)
   
   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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603823189
 
 
   > > Options:
   > > 
   > > 1. `connections_path_prefix`
   > > 2. `connections_prefix`
   > > 3. `connections_path`
   > > 
   > > What do others think @potiuk @ashb
   > 
   > Add a comment here to keep a record on what we discussed on Slack:
   > 
   > * @kaxil and I agree on using `connections_prefix` because it is more generic than `path` and may apply to different systems better (GCP Secrets Manager does not support `/`, so `airflow/connections` is invalid.)
   
   Hey, Let's keep the args as it is. After reading some more docs on **Hashicorp Vault** and talking to Ash, I am going to not change the args for now.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil merged pull request #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] ashb commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603545231
 
 
   I think SecretsManager may be something else. Go AWS!

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil edited a comment on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603545674
 
 
   `AwsSystemsManagerParameterStoreBackend` wow that name would be huge
   
   `SystemsManagerParameterStoreBackend` ? We can remove `aws` as it is in module_path and `providers` path too.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] ashb commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603546886
 
 
   I think the "Store" part is not needed, so AwsSMParametersBackend, AwsSystemManagerParametersBackend or soemthing like that, or perhaps AwsSSMParametersBackend.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] ashb commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603546227
 
 
   https://docs.aws.amazon.com/systems-manager/latest/userguide/what-is-systems-manager.html
   
   > *Systems Manager Service Name History*
   >
   >  AWS Systems Manager (Systems Manager) was formerly known as " Amazon Simple Systems Manager (SSM) " and " Amazon EC2 Systems Manager (SSM) ". The original abbreviated name of the service, " SSM ", is still reflected in various AWS resources, including a few other service consoles. Some examples:
   >
   >        Systems Manager Agent: SSM Agent
   >
   >        Systems Manager parameters: SSM parameters
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603543023
 
 
   Options:
   
   1. `connections_path_prefix`
   2. `connections_prefix`
   3. `connections_path`
   
   What do others think @potiuk @ashb 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil edited a comment on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603547473
 
 
   `SystemsManagerParametersBackend` stripping AWS as we don't use GCP or Hashicorp in their respective backends too.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603544562
 
 
   > Aws SSM
   
   How about `AwdSecretsManager`? (Based on https://docs.aws.amazon.com/secretsmanager/latest/userguide/intro.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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] xinbinhuang edited a comment on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
xinbinhuang edited a comment on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603593164
 
 
   > Options:
   > 
   > 1. `connections_path_prefix`
   > 2. `connections_prefix`
   > 3. `connections_path`
   > 
   > What do others think @potiuk @ashb
   
   Add a comment here to keep a record on what we discussed on Slack:
   
   - @kaxil and I agree on using `connections_prefix` because it is more generic than `path` and may apply to different systems better (GCP Secrets Manager does not support `/`, so `airflow/connections` is invalid.)  

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] xinbinhuang commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603553216
 
 
   For some context, this is a snapshot of the `parameter store` service. So I feel like better to still have the `ParameterStore` part
   
   ![image](https://user-images.githubusercontent.com/27927454/77485455-6e635580-6dea-11ea-8ce8-a58920c62264.png)
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] xinbinhuang commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603593164
 
 
   > Options:
   > 
   > 1. `connections_path_prefix`
   > 2. `connections_prefix`
   > 3. `connections_path`
   > 
   > What do others think @potiuk @ashb
   
   Add a comment here to keep a record on what we discussed on Slack:
   
   - we agree on using `connections_prefix` because it is more generic than `path` and may apply to different systems better (GCP Secrets Manager does not support `/`, so `airflow/connections` is invalid.)  

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] ashb commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603544999
 
 
   In fact it seems that "SSM" is the "Systems Manager Agent" https://docs.aws.amazon.com/systems-manager/latest/userguide/ssm-agent.html.
   
   The full product name of the feature we are using is the AWS Systems Manager Parameter Store.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] kaxil edited a comment on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
kaxil edited a comment on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603544562
 
 
   > Aws SSM
   
   How about `AwsSecretsManager`? (Based on https://docs.aws.amazon.com/secretsmanager/latest/userguide/intro.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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [airflow] ashb commented on issue #7846: Standardize SecretBackend class names

Posted by GitBox <gi...@apache.org>.
ashb commented on issue #7846: Standardize SecretBackend class names
URL: https://github.com/apache/airflow/pull/7846#issuecomment-603543950
 
 
   Aws SSM is a suite of tools/products so calling it AwsSSMBackend is too generic. AwsSSMParameterStoreBackend (though it's too long a name)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services