Add a skip parameter to Variable to skip the secret backend

[raphaelauv]

close #19251

the new parameter will skip all external the secret backend for variables

so it will not skip the env variables set by AIRFLOW_VAR_FOO

and the variables inside the airflow db

and (if setup) a LocalFilesystemBackend

[potiuk]

Just one NIT: with the name

[potiuk]

should we rename it to "skip_secret_backend"? I think this would be more consistent with the currently used naming

https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/index.html

For example we can have local FilesystemBackend (which is not external). Also it would indicate better the intentions.

What we want to do by adding the flag we want to explicitly read values that we know are not "secret" and we do not want to look for them in secret backend.

[potiuk]

Also plural suggests that we can have more than one "external/secret" backend configured - which we don't have (and will not likely have), so I suggest to use singular form.

[raphaelauv]

the "problem" is that the LocalFilesystemBackend need to be setup as a secret_backend https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/secrets-backend/local-filesystem-secrets-backend.html

but we don't want skip it cause it's not an "external" backend

so skip_secret_backend will not be very clear for people with a LocalFilesystemBackend setup

( that's why originally I come up with the name skip_external_backends )

what should we do ?

the documentation of the arg should precise the 3 not skipped backends ?

[ashb]

Not needed, possible via secrets backend configuration

airflow/airflow/providers/google/cloud/secrets/secret_manager.py

Lines 60 to 62 in ae04488

	:param variables_prefix: Specifies the prefix of the secret to read to get Variables.
	If set to None (null), requests for variables will not be sent to GCP Secrets Manager
	:type variables_prefix: str

[potiuk]

Not really. This one is for selective disabling of secret backends only for some cases which user chooses consciously to skip secrets querying (knowing that the data is not secret). This is an optimisation of cost mostly (many secret backends are paid by access)

[ashb]

@potiuk That strikes me as a very-hard to reason about feature -- depending on how the DAG accesses the variable means that you get different behaviour. If I do it one way the it will load the secret from the secrets store, but if I do it another way it will use the value in the env/DB.

This makes the runtime behaviour hard to reason about and makes the system as a whole hard to reason about.

This is a -1/veto from me, sorry.

If you still want this sort of feature @raphaelauv then I would suggest you write a custom secrets backend (subclassing the existing GCS backend) and make a decision based on the name/prefix of the variable. That way the source of a variable doesn't depend on how it is accessed in different DAGS.

[raphaelauv]

The option is false by default. That mean only when user is using Variable.get() himself he can custom the behavior. So I don't see any hidden behavior.

Yes extending the custom_backend is an option , I still think that for the majority of the calls to the variables are for the airflow DB and not the custom secret backend.

[ashb]

My problem with this solution is making the decision something that each dag author has to do on a case-by-base basis, rather than a "global" setting that applies to a given variable (name) always.

[raphaelauv]

Yes it's perflectly true that it's going to be boring to setup almost all the variables to

Variable.get("toto",skip_secret_backend=True)

a better option would be a prefix-pattern-rule for variables to lookup

so a new common option to backend_kwargs

"only_look_up_prefix_variables" : "secret_"

for Variable.get("secret_toto") the secret_backend would be call

and Variable.get("toto") the secret_backend would directly return None and the other backend would then be check

This would be great and explicit

[potiuk]

Proposal: Let's continue the discussion in #19251

[nxhuy-github]

Hi @potiuk @ashb and @raphaelauv , do we have any news on this feature please? I feel like this PR was never merged yet.

[raphaelauv]

this PR is close and will never be merge

[nxhuy-github]

Ah, so we need to custom your secret backend the same way you did, if not, we need to accept the fact that these errors will always be there no matter what when we use Secret Manager, am I right, please? @raphaelauv

[raphaelauv]

It depend the implementation of the secret_backend , the secret_backend part of the airflow providers log errors , you just have to code something better/different