You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/11/03 11:49:10 UTC

[GitHub] [airflow] raphaelauv opened a new issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

raphaelauv opened a new issue #19251:
URL: https://github.com/apache/airflow/issues/19251


   ### Description
   
   I use the gcp [secret_manager](https://airflow.apache.org/docs/apache-airflow-providers-google/stable/secrets-backends/google-cloud-secret-manager-backend.html) as a secret_backend 
   
   2 problems : 
   
   - the implementation always first look for the secret_backend before trying the airflow variables , no way to skip the check to the secret_backend
   
   something like 
   ```python
   Variable.get("totot",skip_secret_backend=True)
   ```
   
   so change variable.py
   
   ```python
       @classmethod
       def get(
           cls,
           key: str,
           default_var: Any = __NO_DEFAULT_SENTINEL,
           deserialize_json: bool = False,
           skip_secret_backend: bool = False,
       ) -> Any:
   ```
   
   and also change the macro
   ```
   {{ var.value.get('my.var', 'fallback') }}
   ```
   
   - every variable that is not in the secret_backend but in the airflow variable will produce an ERROR log line , for some dag is really confusing to see at every run : 
   
   ```
   [2021-10-27 10:16:19,103] {secret_manager_client.py:93} ERROR - Google Cloud API Call Error (PermissionDenied): No access for Secret ID airflow-prod-variable-XXXXXX-XXXXX.
                   Did you add 'secretmanager.versions.access' permission?
   ```
   
   Replace the log level ERROR to WARNING would be better since we don't know if the secret do not exist or if it's really a problem of access permission.
   
   ### Use case/motivation
   
   Every Variable.get make a call to the secret_backend , would be great to make it configurable ( to first control the cost and the load on the secret_backend )
   
   ### Related issues
   
   _No response_
   
   ### Are you willing to submit a PR?
   
   - [X] Yes I am willing to submit a PR!
   
   ### Code of Conduct
   
   - [X] I agree to follow this project's [Code of Conduct](https://github.com/apache/airflow/blob/main/CODE_OF_CONDUCT.md)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958956103






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb closed issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb closed issue #19251:
URL: https://github.com/apache/airflow/issues/19251


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958961239






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958961239


   I closed the PR as the implementation is not close to something I am happy with, so any new solution will have share none of the current changes. But we can edit this issue yes, so sorry for closing the issue as well - that was hasty of me. Sorry.
   
   If a variable is a secret/sensitive, why not store it in a connection? We could add a new "generic" type of connection and then you can access it as `{{ conn.some_name.pass }}` etc. Using that approach then it would be a) clear if something is sensitive or not (Variable: not sensitive, Connection: sensitive) and then it's easy for an install to be configured to not pull variables from the secrets store.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958718560






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958828680






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958992113


   my quick and dirty solution -> 
   
   ```yaml
   AIRFLOW__SECRETS__BACKEND=toto.CustomCloudSecretManagerBackend
   #AIRFLOW__SECRETS__BACKEND_KWARGS={"connections_prefix":"airflow-XXX-connection","variables_prefix":"airflow-XXX-variable","sep":"-","secret_lookup_prefix":"secret_"}
   ```
   
   ```python
   from typing import Optional
   
   from airflow.providers.google.cloud.secrets.secret_manager import CloudSecretManagerBackend
   
   
   class CustomCloudSecretManagerBackend(CloudSecretManagerBackend):
        def __init__(
               self,
               secret_lookup_prefix: Optional[str] = None,
               **kwargs,
       ):
           super().__init__(**kwargs)
           self.secret_lookup_prefix = secret_lookup_prefix
   
       def get_variable(self, key: str) -> Optional[str]:
           if self.variables_prefix is None:
               return None
   
           if self.secret_lookup_prefix is not None:
               if not key.startswith(self.secret_lookup_prefix):
                   return None
   
           return self._get_secret(self.variables_prefix, key)
   
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958986014


   > Another possible solution I can think of is to build another Secret Backend that actually allows "skipping" touching the "external" secret backend entirely for some conditions ("prefix", "patterns"). So users don't need to update all their DAGs when they need to change something and will be a single source of truth of where the values for this variable/connection comes from.
   
   Yeah - I thought about it too, but it would have to be "mix-in" type - we should be able to take any of the backends out there and "mix-in" the selection. That was my initial idea as well :). But I think having a callable is more Pythonic way - doing pretty much the same without the overhead of creating a "backend class" that would take "other backend" as the "real backend to use".
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958840599


   no you didn't understood @ashb 
   
   I need to keep the secret_backend but I don't want airflow to try to read ALL the variables from it all the times
   
   I want an option to skip the secret_backend lookup for some variables ( the vast majority like said @potiuk )


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958770196


   ok , current implementation is only with the `skip_***` arg
   
   (but never skipping  EnvironmentVariablesBackend,  LocalFilesystemBackend and MetastoreBackend )


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958956103

I would not close @ashb, but rather discuss how we can implement, I think there are varying opinions here. At the very least, if others also have such strong opinion as you - we should document how to handle this - very common it seems - use case.

I think we still can discuss how to do it - and maybe we should do it differently - but I think the use/case and rationale is very-sound and - more than that - pretty common. It has been raised quite a few times by different people.

Since we have connections where most people keep their secret passwords, I think the ratio of secret variables vs. the non-secret ones is small and this use case is very common. I'd say more often than not people will have connections kept in secret, but they will have no secret variables. We are basically telling people to pay more for their secret backends and we do not even tell them how to avoid this. And I think making people write their onwn secret backend to handle such case is kinda over-the-top - especially that it seems common pattern for all the different secret backends. And our users do not often realise, that they could do their own implementation very easily.

I actually see the point of the "different access" pattern that @ashb and I agree it is not perfect. I think what is even bigger problem is that it's the "DAG" writers to decide how each variable should be retrieved, which - I quite agree - is very prone to different kinds of problems

But maybe we can find a a good solution that serves the use case but does not introduce the "different access" pattern. I

Maybe a good solution will be to give them ready-to use simple implementation of such custom backend that you could "MixIn" with any other secret backend where for example chosing whether to use secret or not would be done based on Regexp match of the variable name?

Maybe simply have a possibility to define an optional callable configurable in `secrets` group, that will get the "name" and "type" (variable/connection/config) of the retrieved object and return true/false if it should use secret ? That could also give an opportunity to handle a different case we have currently problem with. Currentlly when someone tries to set a variable whcih is already available as "secret". this variable is set in the Metadatada DB (and not accessible), you do not get any warning or error when you do so, which might lead to really strange problems.

If the installation has this "callable" defined however, that could be much better, because we could check whether the variable should be read from secret and if so - we would just fail such an attempt.

I think that would be a really nice and consistent approach.

WDYT everyone?

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958968508

> If a variable is a secret/sensitive, why not store it in a connection? We could add a new "generic" type of connection and then you can access it as `{{ conn.some_name.pass }}` etc. Using that approach then it would be a) clear if something is sensitive or not (Variable: not sensitive, Connection: sensitive) and then it's easy for an install to be configured to not pull variables from the secrets store.

Just a comment from my side (as I was involved with a discussion including our users - very much related). One problem with that is that some users do not wan't (or can't - because their ) store their secrets in the "connection URL form", and that would force them to make airflow-specific format for secrets where they are using the same secret accross different services not only airflow.

We have a very good example here recently (this comes from big, enterprise user) https://github.com/apache/airflow/pull/19164 where corporate user already has their secret service account encrypted in their secret backend and rotated frequently automatically (and used by other services). This is perfect case for "secret variables" but would not work if we use connections.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958986014


   > Another possible solution I can think of is to build another Secret Backend that actually allows "skipping" touching the "external" secret backend entirely for some conditions ("prefix", "patterns"). So users don't need to update all their DAGs when they need to change something and will be a single source of truth of where the values for this variable/connection comes from.
   
   Yeah - I thought about it too, but it would have to be "mix-in" type - we should be able to take any of the backends out there and "mix-in" the selection logic. That was my initial idea as well :). But I think having a callable is more Pythonic way - doing pretty much the same without the overhead of creating a "backend class" that would take "other backend" as the "real backend to use".
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-953090374


   Yep. I also thought this might be a good idea. I often found that things would be simpler this way, and there is a huge potential of optimizing the traffic to secret backends. I tihnk the ratio of "secret" variables vs. "non-secret ones"  are like 1-100 and seems that we have to pay the price of roundtrip to secret backends every time we access it. Of course, we then have to go to DB instead, but if the secret backend does not contain the variable we do it anyway, so we can save a lot by adding a context "this variable is not supposed to be looked up in context" 
   
   @kaxil  - WDYT ? I think you were the master-mind behind the secrets implementation, do you see any obvious problems with this approach ? I cannot see any "bad scenario" here.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] kaxil commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

kaxil commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958981398


   Another possible solution I can think of is to build another Secret Backend that actually allows "skipping" touching the "external" secret backend entirely for some conditions ("prefix", "patterns"). So users don't need to update all their DAGs when they need to change something and will be a single source of truth of where the values for this variable/connection comes from.
   
   For observability, we can probably create a page under "Admin" of where this secret comes from (similar to our airflow configuration page that has **Running Configurations**).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958831862


   This is already possible at the secrets backend configuration https://github.com/apache/airflow/blob/ae044884d1dacce8dbf47c618f543b58f9ff101f/airflow/providers/google/cloud/secrets/secret_manager.py#L60-L62


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] kaxil commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

kaxil commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958981398


   Another possible solution I can think of is to build another Secret Backend that actually allows "skipping" touching the "external" secret backend entirely for some conditions ("prefix", "patterns"). So users don't need to update all their DAGs when they need to change something and will be a single source of truth of where the values for this variable/connection comes from.
   
   For observability, we can probably create a page under "Admin" of where this secret comes from (similar to our airflow configuration page that has **Running Configurations**).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958840599






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958770196






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958968508

We have a very good example here recently (this comes from big, enterprise user) https://github.com/apache/airflow/pull/19164 where corporate user already have their secret service accounts encrypted in their secret backend and rotated frequently automatically (and used by other services). This is perfect case for "secret variables" but would not work if we use connections.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958956103






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958956103

But maybe we can find a a good solution that serves the use case but does not introduce the "different access" pattern. I

Maybe simply have a possibility to define an optional callable configurable in `secrets` group, that will get the "name" and "type" (variable/connection/config) of the retrieved object and return true/false if it should use secret ? That could also give an opportunity to handle a different case we have currently problem with. Currentlly when someone tries to set a variable whcih is already available as "secret". this variable is set in the Metadatada DB (and not accessible). You do not get any warning or error when you do so, which might lead to really strange problems.

I think that would be a really nice and consistent approach.

WDYT everyone?

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a way to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-959037158


   > my quick and dirty solution ->
   
   Yep. Very much this, it is indeed EASY to write your own secret backend like that - but it's not easy to discover by the users that they can do it. Some way of either documenting this pattern, or (IMHO a bit better) supporting the "mix-in" should solve the problem nicely.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958770196






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-953090374


   Yep. I also thought this might be a good idea. I often found that things would be simpler this way, and there is a huge potential of optimizing the traffic to secret backends. I tihnk the ratio of "secret" variables vs. "non-secret ones"  are like 1-100 and seems that we have to pay the price of roundtrip to secret backends every time we access it. Of course, we then have to go to DB instead, but if the secret backend does not contain the variable we do it anyway, so we can save a lot by adding a context "this variable is not supposed to be looked up in context" 
   
   And yeah - cost for accessing secrets also matters.
   
   @kaxil  - WDYT ? I think you were the master-mind behind the secrets implementation, do you see any obvious problems with this approach ? I cannot see any "bad scenario" here.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958718560


   I personally think `skip` is a bit more "straightforward" and simple. Using more than one secret backend is not supported currently and we would have to implement the capability of definining multiple backands (with multiple configs) for the "look_in" type of setting to make sense.
   
   I think the most "common" use cases are where secrets are kept in "secret" backend and all other variables are kept in metastore, so having just "skip_external_backend" makes sense (however I'd rename the parameter to 'skip_secret_backend` to be more precise.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958770196






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958968508

Just a comment from my side (as I was involved with a discussion including our users - very much related). One problem with that is that some users do not wan't (or can't - because their policies/tools limitaiton/shared secret approach) store their secrets in the "connection URL form", and that would force them to make airflow-specific format for secrets where they are using the same secret accross different services not only airflow.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958968508

If a variable is a secret/sensitive, why not store it in a connection? We could add a new "generic" type of connection and then you can access it as `{{ conn.some_name.pass }}` etc. Using that approach then it would be a) clear if something is sensitive or not (Variable: not sensitive, Connection: sensitive) and then it's easy for an install to be configured to not pull variables from the secrets store.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958828680






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958828680


   Do you want to skip the secrets backend for _all_ variables, or just a specific one?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958961239


   I closed the PR as the implementation is not close to something I am happy with, so any new solution will have share none of the current changes. But we can edit this issue yes, so sorry for closing the issue and the PR - that was hasty of me. Sorry.
   
   If a variable is a secret/sensitive, why not store it in a connection? We could add a new "generic" type of connection and then you can access it as `{{ conn.some_name.pass }}` etc. Using that approach then it would be a) clear if something is sensitive or not (Variable: not sensitive, Connection: sensitive) and then it's easy for an install to be configured to not pull variables from the secrets store.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958986014


   > Another possible solution I can think of is to build another Secret Backend that actually allows "skipping" touching the "external" secret backend entirely for some conditions ("prefix", "patterns"). So users don't need to update all their DAGs when they need to change something and will be a single source of truth of where the values for this variable/connection comes from.
   
   Yeah - I thought about it too, but it would have to be "mix-in" type - we should be able to take any of the backends out there and "mix-in" the selection logic. That was my initial idea as well :). But I think having a callable is more Pythonic way - doing pretty much the same without the overhead of creating a "backend class" that would take "other backend class" as the "real backend to use".
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958988390


   A possible third option (because what we need is _more_ options): Create a new `Secret` class that parallels Variables, and somehow move towards Variables to being _not_ secret at all, and then Secret is basically a single value.
   
   (Conceptually it's a simple idea, but the path forward from where Airflow is right now to that isn't so clear)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958961239


   I closed the PR as the implementation is not close to something I am happy with, so any new solution will have share none of the current changes. But we can edit this issue yes, so sorry for closing the issue as well - that was hasty of me. Sorry.
   
   If a variable is a secret/sensitive, why not store it in a connection? We could add a new "generic" type of connection and then you can access it as `{{ conn.some_name.pass }}` etc. Using that approach then it would be a) clear if something is sensitive or not (Variable: not sensitive, Connection: sensitive) and then it's easy for an install to be configured to not pull variables from the secrets store.
   
   What do you think of this idea @raphaelauv ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] kaxil commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

kaxil commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958178683


   Yeah I was thinking of this sort of feature, I think I described in one of the issues too.
   
   Instead of `skip`, should we instead include what backend it should look at? No flaw for either of the features though. And for sure they would be useful.
   
   cc @fhoda 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958840599






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958718560






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] kaxil commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

kaxil commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958981398






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958828680






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958855234


   You want to use variables from the secret backend for some variables but not others?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958956103

I would not close @ashb, but rather discuss how we can implement, I think there are varying opinions here. At thev very least) if others also have such strong opinion as you - we shoudl document how to handle this - very common it seems use case.

I think we still can discuss how to do it - and maybe we should do it differently - but I think the use/case and rationale is very-sound and - more than that - pretty common.

I think this issue has been raised quite a few times by different people.

Since we have connections where, I think the ratio of secret variables vs. the non-secret ones is small and this use case is very common. I'd say more often than not people will have connections kept in secret, but they will have no secret variables. We are basically telling people to pay more for their secret backends and we do not even tell them how to avoid this. And I think making people write their onwn secret backend to handle such case is kinda over-the-top - especially that it seems common pattern for all the different secrets. And our users do not often realise, that they could do their own implementation very easily.

But maybe we can find a a good solution that serves the use case but does not introduce the "different access" pattern. I

I think that would be a really nice and consistent approach.

WDYT everyone?

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958968508

We have a very good example here recently (this comes from big, enterprise user) https://github.com/apache/airflow/pull/19164 where corporate user already has their secret service accounts encrypted in their secret backend and rotated frequently automatically (and used by other services). This is perfect case for "secret variables" but would not work if we use connections.

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk commented on issue #19251: Add a way to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-959037158


   > my quick and dirty solution ->
   
   Yep. Very much this, it is indeed EASY to write your own secret backend like that - but it's not easy to discover by the users that you can do it. Some way of either documenting this pattern, or (IMHO a bit better) supporting the "mix-in" should solve the problem nicely.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958718560






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958182796


   I implemented the skip option in the PR , I could also implement the select option
   
   But what should be the augment string give by the user ?
   
   The name of the class of the secret_back to only look to?
   
   Like 
   
   ```python
   Variable.get("toto",look_in="MetastoreBackend")
   
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958961239






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv commented on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv commented on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958840599


   no you didn't understood @ashb 
   
   I need to keep the secret_backend but I don't want airflow to try to read ALL the variables from it all the times
   
   I want option to skip the secret_backend lookup


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb closed issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb closed issue #19251:
URL: https://github.com/apache/airflow/issues/19251


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958956103

But maybe we can find a a good solution that serves the use case but does not introduce the "different access" pattern. I

I think that would be a really nice and consistent approach.

WDYT everyone?

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] potiuk edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

potiuk edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958956103

Since we have connections where most people keep their secrets, I think the ratio of secret variables vs. the non-secret ones is small and this use case is very common. I'd say more often than not people will have connections kept in secret, but they will have no secret variables. We are basically telling people to pay more for their secret backends and we do not even tell them how to avoid this. And I think making people write their onwn secret backend to handle such case is kinda over-the-top - especially that it seems common pattern for all the different secrets. And our users do not often realise, that they could do their own implementation very easily.

But maybe we can find a a good solution that serves the use case but does not introduce the "different access" pattern. I

I think that would be a really nice and consistent approach.

WDYT everyone?

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] raphaelauv edited a comment on issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

raphaelauv edited a comment on issue #19251:
URL: https://github.com/apache/airflow/issues/19251#issuecomment-958992113


   my quick and dirty solution -> 
   
   ```yaml
   AIRFLOW__SECRETS__BACKEND=toto.CustomCloudSecretManagerBackend
   AIRFLOW__SECRETS__BACKEND_KWARGS={"connections_prefix":"airflow-XXX-connection","variables_prefix":"airflow-XXX-variable","sep":"-","secret_lookup_prefix":"secret_"}
   ```
   
   ```python
   from typing import Optional
   
   from airflow.providers.google.cloud.secrets.secret_manager import CloudSecretManagerBackend
   
   
   class CustomCloudSecretManagerBackend(CloudSecretManagerBackend):
        def __init__(
               self,
               secret_lookup_prefix: Optional[str] = None,
               **kwargs,
       ):
           super().__init__(**kwargs)
           self.secret_lookup_prefix = secret_lookup_prefix
   
       def get_variable(self, key: str) -> Optional[str]:
           if self.variables_prefix is None:
               return None
   
           if self.secret_lookup_prefix is not None:
               if not key.startswith(self.secret_lookup_prefix):
                   return None
   
           return self._get_secret(self.variables_prefix, key)
   
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [airflow] ashb closed issue #19251: Add a skipper arg to variable.get() to skip the secret_backend

Posted by GitBox <gi...@apache.org>.

ashb closed issue #19251:
URL: https://github.com/apache/airflow/issues/19251






-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org