Skip to content

Fix CLI connection display reporting wrong is_extra_encrypted flag - #67561

Merged
potiuk merged 2 commits into
apache:mainfrom
jonathanleek:fix-is-extra-encrypted-cli-mapping
Jun 24, 2026
Merged

potiuk merged 2 commits into
apache:mainfrom
jonathanleek:fix-is-extra-encrypted-cli-mapping

Conversation

@jonathanleek

Copy link
Copy Markdown
Contributor

airflow connections get showed the password's encryption status under both is_encrypted and is_extra_encrypted keys, because the display mapper sourced both from Connection.is_encrypted. These flags are independent — the extra field (which usually holds API keys, certs, etc.) can be encrypted while the password is not, or vice versa. Operators checking the readout could be misled about whether extra is encrypted at rest.

The fix changes is_extra_encrypted to source from Connection.is_extra_encrypted in both ConnectionDisplayMapper.full_details and ConnectionDisplayMapper.masked_sensitive. A parametrized regression test exercises both methods across the two divergent flag combinations to prevent re-introducing the bug.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

@boring-cyborg

boring-cyborg Bot commented May 26, 2026

Copy link
Copy Markdown

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example Dag that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@jonathanleek
jonathanleek force-pushed the fix-is-extra-encrypted-cli-mapping branch from 71c4113 to 040fe0a Compare May 26, 2026 15:08
`airflow connections get` showed the password's encryption status under
both `is_encrypted` and `is_extra_encrypted` keys, because the display
mapper sourced both from `Connection.is_encrypted`. These flags are
independent — the extra field (which usually holds API keys, certs)
can be encrypted while the password is not, or vice versa. Operators
checking the readout could be misled about whether `extra` is encrypted
at rest.

Adds a regression test that exercises both `ConnectionDisplayMapper`
methods (full_details, masked_sensitive) across the divergent input
combinations.
@jonathanleek
jonathanleek force-pushed the fix-is-extra-encrypted-cli-mapping branch from 040fe0a to ffdc3a5 Compare May 27, 2026 13:58

@Pedrinhonitz Pedrinhonitz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the adjustment, it looks good to me.

@potiuk potiuk added the ready for maintainer review Set by triage when all criteria pass, or by the fork CI reconciler when fork CI is green. label May 28, 2026
@jroachgolf84

Copy link
Copy Markdown
Collaborator

@eladkal - can you take a peek at this one?

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM — clear bug: is_extra_encrypted was sourced from conn.is_encrypted, so the two independent flags always reported the same value. Fix is correct and the parametrized test covers both mappers × both flag combinations. (Minor: since this is a user-visible CLI output fix, a bugfix newsfragment would've been fine to keep — but not worth blocking.) Thanks!


Drafted-by: Claude Code (Opus 4.8); reviewed by @potiuk before posting

@potiuk potiuk removed the ready for maintainer review Set by triage when all criteria pass, or by the fork CI reconciler when fork CI is green. label Jun 22, 2026
@jroachgolf84

Copy link
Copy Markdown
Collaborator

@potiuk - why was ready for maintainer review being removed from PR's?

@potiuk

potiuk commented Jun 24, 2026

Copy link
Copy Markdown
Member

@potiuk - why was ready for maintainer review being removed from PR's?

My mistake - I thought I already merged that one.

@potiuk potiuk added the ready for maintainer review Set by triage when all criteria pass, or by the fork CI reconciler when fork CI is green. label Jun 24, 2026
@potiuk
potiuk merged commit b9148fa into apache:main Jun 24, 2026
72 checks passed
@boring-cyborg

boring-cyborg Bot commented Jun 24, 2026

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

karenbraganz pushed a commit to karenbraganz/airflow that referenced this pull request Jun 30, 2026
…pache#67561)

`airflow connections get` showed the password's encryption status under
both `is_encrypted` and `is_extra_encrypted` keys, because the display
mapper sourced both from `Connection.is_encrypted`. These flags are
independent — the extra field (which usually holds API keys, certs)
can be encrypted while the password is not, or vice versa. Operators
checking the readout could be misled about whether `extra` is encrypted
at rest.

Adds a regression test that exercises both `ConnectionDisplayMapper`
methods (full_details, masked_sensitive) across the divergent input
combinations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:CLI ready for maintainer review Set by triage when all criteria pass, or by the fork CI reconciler when fork CI is green.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants