Skip to content

Document that connection secrets without conn_type remain supported#69780

Merged
henry3260 merged 3 commits into
apache:mainfrom
henry3260:fix-connection-secret-validation
Jul 23, 2026
Merged

Document that connection secrets without conn_type remain supported#69780
henry3260 merged 3 commits into
apache:mainfrom
henry3260:fix-connection-secret-validation

Conversation

@henry3260

Copy link
Copy Markdown
Contributor

Why

Connection secrets in JSON format could omit conn_type. Although the secrets backend deserialized those values, the Execution API requires a connection type and the task could ultimately receive a misleading “connection not found” error.

This addresses the conn_type portion of related: #57864.

What

  • Reject JSON connection secrets whose conn_type is missing, null, empty, or whitespace-only.
  • Raise a clear ValueError before constructing an incomplete Connection.
  • Add regression coverage for each invalid conn_type value.

Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@henry3260
henry3260 force-pushed the fix-connection-secret-validation branch 3 times, most recently from 2b0332f to 01a1b77 Compare July 13, 2026 12:21
@Vamsi-klu

Copy link
Copy Markdown
Contributor

Went through _deserialize_connection_value and the new guard reads correct to me. Good call accepting uri as an alternative rather than requiring conn_type outright, since from_json passes a bare uri to the constructor which routes through from_uri and parses the scheme into the conn_type, so those blobs still build a usable connection and the {"conn_type": null, "uri": ...} test covers exactly that. Raising a ValueError up front is a clear improvement over the misleading connection not found error the PR describes. Only nit, you json.loads(value) in the guard and then from_json parses the same string again, negligible for a small secret blob so I wouldn't bother.

@henry3260
henry3260 requested a review from Lee-W July 22, 2026 06:49
Lee-W
Lee-W previously approved these changes Jul 22, 2026
Comment thread shared/secrets_backend/src/airflow_shared/secrets_backend/base.py Outdated
@henry3260
henry3260 force-pushed the fix-connection-secret-validation branch from 01a1b77 to 8326268 Compare July 22, 2026 09:36
@henry3260 henry3260 changed the title Reject connection secrets without a connection type Document that connection secrets without conn_type remain supported Jul 22, 2026
@henry3260
henry3260 requested review from Lee-W and amoghrajesh July 22, 2026 10:07
Comment thread shared/secrets_backend/src/airflow_shared/secrets_backend/base.py Outdated
Comment thread shared/secrets_backend/src/airflow_shared/secrets_backend/base.py Outdated
Comment thread shared/secrets_backend/tests/secrets_backend/test_base.py Outdated
Comment thread shared/secrets_backend/src/airflow_shared/secrets_backend/base.py Outdated
@henry3260
henry3260 force-pushed the fix-connection-secret-validation branch from 59749be to db28d49 Compare July 23, 2026 07:46
@henry3260
henry3260 requested a review from amoghrajesh July 23, 2026 07:50
henry3260 and others added 3 commits July 23, 2026 17:08
Prevent malformed JSON secrets from being reported as missing connections at runtime.
Review on the PR pointed out that rejecting JSON connection secrets
without a conn_type would regress the Airflow 2 -> 3 migration
compatibility deliberately established in apache#61728: secrets stored by
Airflow 2-era backends (e.g. AWS Secrets Manager) commonly omit both
conn_type and uri, and the worker-local secrets backend path must keep
resolving them. Replace the rejection with a comment at the
deserialization site and a regression test, so the compatibility
guarantee is visible to future readers and enforced by CI.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Apply review suggestions: tighten the migration-compatibility comment and
test docstring, and mark the compatibility path for removal once the
minimum supported Airflow version in providers is 3.0.
@henry3260
henry3260 force-pushed the fix-connection-secret-validation branch from db28d49 to 5455062 Compare July 23, 2026 09:08
@henry3260
henry3260 merged commit 0014422 into apache:main Jul 23, 2026
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants