Add Kubernetes Secrets Backend to cncf.kubernetes provider#61527
Conversation
Add a new secrets backend that reads Airflow connections, variables, and configurations from Kubernetes Secrets. This enables integration with External Secrets Operator (ESO) or any tool that creates Kubernetes secrets with a predictable naming scheme. Key design decisions: - Uses kubernetes.config.load_incluster_config() directly instead of KubernetesHook to avoid circular dependencies (the secrets backend cannot depend on Airflow connections since it IS the mechanism for resolving them). - Auto-detects namespace from pod service account metadata with fallback to 'default'. - Sanitizes secret names for Kubernetes DNS compatibility by converting underscores to hyphens and lowercasing. - Supports configurable prefixes and data keys for connections, variables, and configurations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7c013fa to
4ddac0b
Compare
|
Thanks for adding. That makes sense. Not some static check fail because secret backends need to be registered into provider.yaml that described providers capabilties. |
There was a problem hiding this comment.
Looks good overall. This is a solid addition that is well-implemented.
It appears that this backend is explicitly scoped to in-cluster usage and bypasses KubernetesHook, which is reasonable here, as that could introduce circular dependencies.
There are 2 non-blocking suggestions I have left if you want to improve it further. And I would resolve the CI failures. One looks like a missing key from get_provider_info (@jscheffl has pointed this out). The other seems unrelated to your change but I would check anyway.
Nataneljpwd
left a comment
There was a problem hiding this comment.
Looks good overall, I have left a few comments, I would appreciate it if you could answer them.
|
thanks for the review and comments, however before I will address them, I would like to also get your opinion on the secrets discovery method I chose - the secret name mapping... which after more thinking is not a good ide. What do you think about the options (I would go for option 2) ---
# labels (option 1)
# backend_kwargs = {"connections_lable_type_name": "secret-type", "connections_lable_name_name": "secret-name", ...}
apiVersion: v1
kind: Secret
metadata:
name: anything
labels:
airflow.apache.org/secret-type: connection
airflow.apache.org/secret-name: <conn_name>
data:
value: <conn_value>
---
# labels (options 2)
# backend_kwargs = {"connections_secret_lable_name": "connection-name", ... }
apiVersion: v1
kind: Secret
metadata:
name: anything
labels:
airflow.apache.org/connection-name: <conn_name>
data:
value: <conn_value>
---
# single secret to manage all connections (option 3)
# backend_kwargs = {"connections_secret_name": "airflow-connections", ... }
apiVersion: v1
kind: Secret
metadata:
name: airflow-connections
data:
<conn_name1>: <conn_value>
<conn_name2>: <conn_value>
labels search could be equipped with same cacheing feature, if a secrets has been discovered on previous runs, it does not perform search (i knows that the secret/connection exists) |
Option 1 seems reasonable, it allows to distinguish between connections and variables easily, while option 2 does not, so here I would prefer option 1, yet if you decide to make it with labels, you need to make sure that there are no duplicated labels before the name was the unique constraint enforced by k8s, now you will need to enforce uniqueness by yourself, in case you create, update or delete a secret. About the caching, we can just do a k8s query with a resource version of 0 (which is very quick) yet we will be updated on when the connection or variable is deleted, and so caching on airflow side is probably not worth implementing, as we can rely on k8s api server cache. |
|
in opotion 2, i did not add it, but it would go like
as per maintainig uniquness on secrets/labels, this is not a backend role to maintain proper secrets deployment, secrets backend reads connections, if the connections are wrongly deployed to a namespace this is a totally different problem (this is how I see it ) |
So it would have a hardcoded label prefix plus a name for the connection/variable/config, alright, seems reasonable and probably a good Idea, yet I still don't understand what is a 'config'? is it an airflow configuration? as I do not know about any airflow object of type config other than the core airflow configuration, which from what I remember, cannot be pulled at runtime. Option 1 still seems like the better option, as in my opinion it will look cleaner in code rather than using formatted strings in a bunch of places, but it is not a must, nor is it a blocker from my side, just personal preference. Any of the options, 1 or 2 will suffice |
I would go with option 2. If there are duplicates for the same field, there’s less ambiguity about them being genuine duplicates vs them being 2 separate entities that share the same field as in option 1. Please avoid option 3 as that expands the blast radius for secrets misconfiguration, subverts RBAC and makes secrets rotation harder. I can’t see any genuine argument in favour of it. |
I agree with you. I think it should be the user’s responsibility to enforce secrets/labels uniqueness. But is it possible for your new secrets backend to detect duplicate secret configurations (by name) and log a warning? |
secrets backend and config , what you could do: apiVersion: v1
kind: Secret
metadata:
name: secretname
labels:
airflow.apache.org/config-name: sql_alchemy_conn_value
data:
value: postgresql://secretuser:secretpassword@postgres:5432)the metadata database connection string with secrets (password and user) is pulled from secrets backend in runtime |
Address PR review feedback by removing duplicate tests: - Remove TestKubernetesSecretsBackendTeamName (team_name is ignored, already covered by existing connection/variable tests) - Remove TestKubernetesSecretsBackendResourceVersion (resource_version="0" is already verified in 4+ other tests via assert_called_once_with) Also document that multi-team isolation is not currently supported in get_conn_value and get_variable docstrings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…61527) The `if label is None: return None` check was repeated in get_conn_value, get_variable, and get_config. Since _get_secret already receives the label as a parameter, it is the natural place for this guard. This simplifies the public methods to single-line delegations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Nataneljpwd
left a comment
There was a problem hiding this comment.
Looks great, one last minor change (and a test for the change) and I think that the pr is ready
Nataneljpwd
left a comment
There was a problem hiding this comment.
Looks great!
Thank you for the addition of the secrets backend!
I think it is a great QOL improvement for cluster administrators!
jscheffl
left a comment
There was a problem hiding this comment.
Nit in CI for spellcheck, if this is fixed then LGTM!
Ah and small nit in static checks, provider_info.py needs to be re-generated. Best would be to install |
…s/secrets/kubernetes_secrets_backend.py Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com>
Nataneljpwd
left a comment
There was a problem hiding this comment.
Looks amazing, can't wait to try this!
Fun fact, from k8s 1.31 reads from cache can be consistent, I think this can be leveraged in a lot of places when airflow drops support for 1.30
jscheffl
left a comment
There was a problem hiding this comment.
Hope CI is getting green now, then LGTM!
|
@piotrlinski Fantastic work! super useful, been waiting for this! |
) * Add Kubernetes Secrets Backend to cncf.kubernetes provider Add a new secrets backend that reads Airflow connections, variables, and configurations from Kubernetes Secrets. This enables integration with External Secrets Operator (ESO) or any tool that creates Kubernetes secrets with a predictable naming scheme. Key design decisions: - Uses kubernetes.config.load_incluster_config() directly instead of KubernetesHook to avoid circular dependencies (the secrets backend cannot depend on Airflow connections since it IS the mechanism for resolving them). - Auto-detects namespace from pod service account metadata with fallback to 'default'. - Sanitizes secret names for Kubernetes DNS compatibility by converting underscores to hyphens and lowercasing. - Supports configurable prefixes and data keys for connections, variables, and configurations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * label based approch * Update docs to reflect configurable namespace parameter - Fix docstring to reference automountServiceAccountToken instead of "not running inside a Kubernetes pod" (matching error message) - Update RST prerequisites to mention "target namespace" instead of assuming same namespace as Airflow pod - Add namespace as first parameter in backend_kwargs documentation - Rewrite authentication section to explain namespace override option Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Elevate log level from debug to warning for missing secrets When a secret or data key is not found during label-based lookup, a debug message is easy to miss. Upgrading to warning ensures operators are promptly notified of misconfigured or missing secrets. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Extract label defaults and namespace path to class-level constants Move hard-coded label keys and service account namespace path to class constants (DEFAULT_CONNECTIONS_LABEL, DEFAULT_VARIABLES_LABEL, DEFAULT_CONFIG_LABEL, SERVICE_ACCOUNT_NAMESPACE_PATH) for better discoverability and a single source of truth. Rename _get_secret_by_label to _get_secret, fix label values to use standard Airflow conventions (connection-id, variable-key, config-key), and fix formatting issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove redundant tests from KubernetesSecretsBackend (apache#61527) Address PR review feedback by removing duplicate tests: - Remove TestKubernetesSecretsBackendTeamName (team_name is ignored, already covered by existing connection/variable tests) - Remove TestKubernetesSecretsBackendResourceVersion (resource_version="0" is already verified in 4+ other tests via assert_called_once_with) Also document that multi-team isolation is not currently supported in get_conn_value and get_variable docstrings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Move None label guard into _get_secret to reduce duplication (apache#61527) The `if label is None: return None` check was repeated in get_conn_value, get_variable, and get_config. Since _get_secret already receives the label as a parameter, it is the natural place for this guard. This simplifies the public methods to single-line delegations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/secrets/kubernetes_secrets_backend.py Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com> * Fix static checks via prek rnu -a update-providers-build-files --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com> Co-authored-by: Jens Scheffler <jscheffl@apache.org>
) * Add Kubernetes Secrets Backend to cncf.kubernetes provider Add a new secrets backend that reads Airflow connections, variables, and configurations from Kubernetes Secrets. This enables integration with External Secrets Operator (ESO) or any tool that creates Kubernetes secrets with a predictable naming scheme. Key design decisions: - Uses kubernetes.config.load_incluster_config() directly instead of KubernetesHook to avoid circular dependencies (the secrets backend cannot depend on Airflow connections since it IS the mechanism for resolving them). - Auto-detects namespace from pod service account metadata with fallback to 'default'. - Sanitizes secret names for Kubernetes DNS compatibility by converting underscores to hyphens and lowercasing. - Supports configurable prefixes and data keys for connections, variables, and configurations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * label based approch * Update docs to reflect configurable namespace parameter - Fix docstring to reference automountServiceAccountToken instead of "not running inside a Kubernetes pod" (matching error message) - Update RST prerequisites to mention "target namespace" instead of assuming same namespace as Airflow pod - Add namespace as first parameter in backend_kwargs documentation - Rewrite authentication section to explain namespace override option Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Elevate log level from debug to warning for missing secrets When a secret or data key is not found during label-based lookup, a debug message is easy to miss. Upgrading to warning ensures operators are promptly notified of misconfigured or missing secrets. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Extract label defaults and namespace path to class-level constants Move hard-coded label keys and service account namespace path to class constants (DEFAULT_CONNECTIONS_LABEL, DEFAULT_VARIABLES_LABEL, DEFAULT_CONFIG_LABEL, SERVICE_ACCOUNT_NAMESPACE_PATH) for better discoverability and a single source of truth. Rename _get_secret_by_label to _get_secret, fix label values to use standard Airflow conventions (connection-id, variable-key, config-key), and fix formatting issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove redundant tests from KubernetesSecretsBackend (apache#61527) Address PR review feedback by removing duplicate tests: - Remove TestKubernetesSecretsBackendTeamName (team_name is ignored, already covered by existing connection/variable tests) - Remove TestKubernetesSecretsBackendResourceVersion (resource_version="0" is already verified in 4+ other tests via assert_called_once_with) Also document that multi-team isolation is not currently supported in get_conn_value and get_variable docstrings. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Move None label guard into _get_secret to reduce duplication (apache#61527) The `if label is None: return None` check was repeated in get_conn_value, get_variable, and get_config. Since _get_secret already receives the label as a parameter, it is the natural place for this guard. This simplifies the public methods to single-line delegations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/secrets/kubernetes_secrets_backend.py Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com> * Fix static checks via prek rnu -a update-providers-build-files --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Jens Scheffler <95105677+jscheffl@users.noreply.github.com> Co-authored-by: Jens Scheffler <jscheffl@apache.org>
Add a new secrets backend that reads Airflow connections, variables, and configuration from Kubernetes Secrets via the Kubernetes API.
This enables a native integration for Airflow deployments running on Kubernetes, allowing secrets to be managed through Kubernetes-native tooling or synced from external stores using External Secrets Operator (ESO).
Key design decisions:
Was generative AI tooling used to co-author this PR?
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.