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

Fix empty paths in Vault secrets backend #29908

Merged
merged 2 commits into from Mar 4, 2023

Conversation

hussein-awala
Copy link
Member

@hussein-awala hussein-awala commented Mar 3, 2023

related: #29666


Currently when we provide an empty string for connections_path, variables_path and config_path, Vault secrets backend tries to read the key /secret_key instead of secret_key. This PR handle the case when those paths are empty string.

Why this is necessary?

#29734 added support for multiple mount_point, and these mount points can have different paths for variables, config and secrets, so instead of supporting multiple paths, providing the full secret path with empty string for connections_path, variables_path and config_path can solve the problem.

Comment on lines 176 to +179
if self.connections_path is None or conn_key is None:
return None

secret_path = self.build_path(self.connections_path, conn_key)
if self.connections_path == "":
secret_path = conn_key
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we consider None and "" to be dealt differently?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we want to use Vault secrets backend only for connections, we can provide None for variables_path and config_path to return a quick None, then Airflow will check in the other backend secrets (Metadata then environment variables).

So I kept None here to deactivate some functionalities in the secrets backend.

Copy link
Member

Choose a reason for hiding this comment

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

I believe this is because None can really only happens if there is no "/" in path and mount_point is not defined. Which is a basic requirement for vault - any time you expect something from vault you need to specify the mount_point and "/" is a way how it can be specified directly in the path.

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.

None yet

3 participants