Skip to content

Feature/warn cli UI secrets backend - #27034

Closed
RNHTTR wants to merge 11 commits into
apache:mainfrom
RNHTTR:feature/warn-cli-ui-secrets-backend
Closed

RNHTTR wants to merge 11 commits into
apache:mainfrom
RNHTTR:feature/warn-cli-ui-secrets-backend

Conversation

@RNHTTR

@RNHTTR RNHTTR commented Oct 13, 2022 •

Copy link
Copy Markdown
Contributor

I accidentally closed #24679 when I moved the code to a new branch in my fork, and I can't seem to be able to point the existing PR to the new branch & re-open the PR. So, I just opened this PR instead.


The Airflow CLI and UI don't show Variables or Connections that are stored in a secrets backend. This can be really confusing if you don't already know that. This PR adds a warning to CLI commands and a warning banner to the list connections and list variables views in the UI when a user has enabled a secrets backend. The warning just explains that connections and variables stored in secrets backends won't be displayed using the CLI or UI.

Closes: #10867

@boring-cyborg boring-cyborg Bot added area:CLI area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Oct 13, 2022
@o-nikolas
o-nikolas requested a review from uranusjr October 13, 2022 20:33
@o-nikolas

Copy link
Copy Markdown
Contributor

CC: @blag @Taragolis

@potiuk

potiuk commented Oct 23, 2022

Copy link
Copy Markdown
Member

It's cool and it will remove a source of confusion for our users.

First of all - you will need to rebase @RNHTTR - there are some conflicts.

One general comment I have - the warning message mentions EnvironmentVariablesBackend but this one is not really part of custom secret backends so the use of it is not really detected.

I thought we should really treat Environment variables a little differently than other custom secrets. As opposed to all other secrets we can actually check if any custom connection/variable is defined in Environment and print the warning when it is. We just have to check if there are anny AIRFLOW__CONNECTION__ env variables or AIRFLOW__VARIABLES__.*

If you do it this way (additionally to checking if there is a custom backend defined), the warning will also be printed when there are any custom variables defined via env variables (and you can slightly modify the warning message to explain it).

I think that would be a more comple solution to the problem.

@uranusjr uranusjr 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.

Also need to update against the newest changes in main.

from airflow.utils.session import create_session
from airflow.utils.warnings import warn_list_secrets_alternative_backend

CUSTOM_BACKEND: bool = True if get_custom_secret_backend() else False

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.

This shouldn’t need to be repeated. Should we instead extract this to somewhere shared between both modules?

@potiuk

potiuk commented Nov 17, 2022

Copy link
Copy Markdown
Member

Static checks ?

@potiuk

potiuk commented Dec 3, 2022

Copy link
Copy Markdown
Member

Still working on it @RNHTTR ? needs rebase and fixes :(

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions Bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Jan 18, 2023
@github-actions github-actions Bot closed this Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:CLI area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues stale Stale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Warning about invisible connections/variable in the Web UI

4 participants