Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add info about JSON Connection format for AWS SSM Parameter Store Secrets Backend #27134

Merged
merged 3 commits into from
Nov 10, 2022

Conversation

Taragolis
Copy link
Contributor

@Taragolis Taragolis commented Oct 19, 2022

Since Airflow 2.3 Connection could be represented as JSON object which perfectly work without any changes in SystemsManagerParameterStoreBackend.get_conn_value

  • Add appropriate info in AWS SSM Parameter Store Secrets Backend doc
  • Add tests that it actually works with SSM Parameter Store SB (also helps if something changed in the future)
  • Add changes in deprecated method SystemsManagerParameterStoreBackend.get_conn_uri so it return always URI even if it stored as JSON objects. I do not know but might be better just remove this method since it should not use internally in Airflow since 2.3 however it might use in some user code.


In some cases, URI's you will need stored in Secrets Manager may not be intuitive, for example when using HTTP / HTTPS or SPARK, you may need URI's that will look like this:
In some cases, URI's you will need stored in AWS SSM Parameter Store may not be intuitive,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it me or the sentence In some cases, URI's you will need stored in AWS SSM Parameter Store may not be intuitive does not make a lot of sense ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point to change it. If it confuse you than it might confuse others.
From my side I've just change "Secrets Manager" to "AWS SSM Parameter Store" and split long line.

Copy link
Contributor

@vincbeck vincbeck Oct 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I would make it even simpler.

In some cases, URIs stored in AWS SSM Parameter Store may not be intuitive,

Copy link
Contributor Author

@Taragolis Taragolis Oct 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that initial author tried to highlight that Airflow connection URI not a specific connection URI.

E.g. user expected that HTTP https://example.org/path would work however in Airflow we should use http://https%3A%2F%2Fexample.org%2Fpath

There is also a link to example how to generate URI exists in current doc: https://airflow.apache.org/docs/apache-airflow/stable/howto/connection.html#generating-connection-uri

@Taragolis Taragolis force-pushed the ssm-secret-backend-allow-json branch from a9ac812 to 096a8ea Compare October 24, 2022 16:05
@Taragolis Taragolis force-pushed the ssm-secret-backend-allow-json branch from 096a8ea to 52b20f9 Compare October 26, 2022 19:07
@potiuk
Copy link
Member

potiuk commented Oct 31, 2022

Any more comments here :) ?

@Taragolis Taragolis force-pushed the ssm-secret-backend-allow-json branch from 52b20f9 to 7e2cd9d Compare October 31, 2022 11:17
@Taragolis
Copy link
Contributor Author

Any more comments here :) ?

I've just add reuse same pytest parameters in different tests cases

@Taragolis Taragolis force-pushed the ssm-secret-backend-allow-json branch 2 times, most recently from 82da4bb to d3d4c49 Compare November 3, 2022 09:55
@Taragolis Taragolis force-pushed the ssm-secret-backend-allow-json branch from d3d4c49 to d9379e8 Compare November 4, 2022 06:28
@potiuk potiuk merged commit a5c0aeb into apache:main Nov 10, 2022
ashb added a commit to astronomer/airflow that referenced this pull request Nov 10, 2022
Adityamalik123 pushed a commit to Adityamalik123/airflow that referenced this pull request Nov 12, 2022
@Taragolis Taragolis deleted the ssm-secret-backend-allow-json branch January 14, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants