[DISCUSS] Improving security by decoupling the CLI from the database

[Bolke de Bruin <bd...@gmail.com>]

Hi All,

I’ve noticed that we are still implementing new features or are doing
refactoring of CLI commands that directly interface with the database
instead of using the abstractions that should be made available from the
API specification. Why is this an issue? The CLI is used by arbitrary user
to interface with Airflow operations. Airflow relies on the database to be
its single source of truth. A user that is able to read the configuration
of Airflow is currently able to manipulate the database. The CLI requires
database access hence the information to deal with the database is in the
configuration file. To improve security the CLI should use a rest API which
allows for proper authn/authz and segregation of duties.

In the past I have introduced the experimental API with a “local_client”
and “json_client” implementation. The local_client still allows for direct
database access and its only function is to be there during the transition
period to have the full rest api available. After that it should be
deprecated and removed.

My suggestion is to disallow any new functionality in the CLI that directly
relies on “airflow.utils.session” and only allow new functionality to go
through the API client. For now that would mean 2 implementations: local
and json. Of course refactoring the current state should be on the list in
order to remove the “local_client”.

The API client should be available to other packages as well. So maybe we
should package the cli and client api implementations into “airflow-client”.

What are your thoughts?

Thanks
Bolke

Re: [DISCUSS] Improving security by decoupling the CLI from the database

[Ash Berlin-Taylor <as...@apache.org>]

One comment on all of this:

I'm not a fan of a _requiring_ separate api server component to be run - for smaller deployments (of which there are a lot!) this increases the memory requirements by a noticeable fraction.

-ash

On 19 January 2020 19:58:34 GMT+03:00, Jarek Potiuk <Ja...@polidea.com> wrote:
>Some comments from my side:
>
>Bolke:
>
>Direct database access for users is a no go. Direct queries that in
>anyway
>> manipulate the database or any state in Airfl ow (trigger dags,
>connections
>> etc) from the CLI should be prevented.
>
>
>Absolutely agree in principle. But we have to discuss and accept
>profound
>consequences
>of such change. For me - this also means that those commands cannot be
>run
>on the machine where airflow is installed and CLI users should have no
>access to those
>machines - otherwise they can run arbitrary python method/class script
>using airflow and
>access the DB directly (it also means that there direct DB connection
>should not be allowed)
>This pretty much calls for either using scheduler as API server or
>introduce a new
>"API-server" entity.
>
>This is quite a departure from the current Airflow deployment model and
>I
>think it should be
>"very breaking change" (architecturally/deployment wise). It could
>still
>look like a local client for
>the user - so it's not a breaking change from user point of view.
>
>This also means that we do not need maybe 95% of airflow code and that
>we
>should either
>not use "airflow" package (airflow-cli maybe?), or do something with
>the
>"__init__.py" of
>the 'airflow' package. Currently we have a very bad import model if we
>want
>to use "airflow" package
>for everything. By doing 'import airflow.<whatever>" we are pulling in
>almost all airflow code
>because it is either directly or indirectly imported in the
>airflow/__init__.py. Including DB
>models. If we stick to having this __init__.py it means that we will
>continue to have many
>seconds of delays for starting up the CLI. I think it is very bad to
>have
>seconds delays.
>
>For me it really calls even for another package ("airflow-cli") that
>should
>be installable
>separately, Surely for local installation/development etc.  both could
>be
>installed alongside and look
>and behave as the current airflow does - and use localhost for
>communication (and even we
>should have the same backwards-compatible "airflow" binary). We could
>gradually migrate all
>the current CLI methods to switch to the new APIs.
>
>I fundamentally disagree with your position that remote working with
>> Airflow should be done through the Web UI.
>
>
>I agree. As I see it - all the modern tools for cloud world have all
>three
>modes for management -
>Web UI (mostly used for reading and discovery), CLI - mostly used for
>management and sometimes
>for querying, and REST-ish APIS (for whatever custom integration is
>needed).  Look at docker, gcloud
>and many others. From our survey - it's clear that this is also similar
>in
>our case. UI is used for monitoring
>accessing logs etc more frequently than for management, and CLI - while
>used less frequently in general
>has a strong usage as well.
>
>On your points:
>>
>> 1-3: Solved by using FAB
>>
>
>Indeed those three points might be solved by using FAB REST API. I am
>not
>100% sure if FAB is the best
>choice (although I think it is a clear contender). What I really like
>about
>it that we seem to have a way to
>use the same mechanism for authorisation/authentication as in UI. Using
>a
>standard approach here seems
>like a good idea, especially that out-of-the-box we can use all the
>"standard" authentication
>mechanisms:
>https://flask-appbuilder.readthedocs.io/en/latest/security.html# .
>(DB, OpenAI, LDAP, Oauth, custom) -
>they are all available out-of-the-box. What I also like is that FAB
>REST
>API has everything you expect from
>a modern API (
>https://flask-appbuilder.readthedocs.io/en/latest/rest_api.html#openapi-spec
>)
>It implements "open-api" standard, has built-in swagger integration and
>the
>capability of not only generating the
>documentation, having a swagger test server for API testing but also
>capability of automatically generating
>clients in different languages (I'd love however to see POC showing all
>that capabilities).
>IMHO any proposal for API implementation will have to have all those
>properties and
>FAB is hard to beat on that especially that we already use it..
>
>
>> 4. You main points are solved by using FAB. On B) I don’t think it is
>a
>> good idea to have an extra layer between json_client and
>local_client. Both
>> are pretty thin wrappers around `airflow.api.common.*`.
>>
>
>I think we should get rid of the local client asap and introduce only
>REST
>API and API server. It's inevitable
>IMHO and the sooner we introduce it - the better. This way we could
>immediately compress the layers there
>to 1 layer.
>
>
>> 5. Non existent ;-)
>>
>Off-by-one indeed :)
>
>
>> 6. I do agree that there is some code duplication going on, but its
>> intention was there in order to have it in place until `local_client`
>> wasn’t required anymore and the Rest API is mature. So mature the
>API, get
>> rid of local_client.
>>
>
>I think we should do it ASAP and get rid of the local client (but
>provide
>equivalent binary CLI to do
>the same that current CLI does). Once we remove the local
>implementation we
>should switch all the
>CLI methods to use the API and eventually decouple it from main airflow
>code once it is done.
>
>7. Having the CLI manipulate `airflow.cfg` is fine as long as it checks
>if
>> it has the right permissions to do so, the OS will enforce that
>anyway.
>>
>Agree.
>
>
>> 8. They are not in the same namespace. `airflow.api.client.*` vs
>> `airflow.www.api`. If you mean by ‘package’ ‘Airflow’, then I suggest
>maybe
>> packaging the API/Client separately of break up the Airflow package
>in
>> “airflow-common”, “airflow-client”, “airflow-server”.
>
>Same thought exactly ^^^
>
>
>> 9. That’s not unexpected: the interface is documented whilst not
>perfectly.
>> The CLI *should* use the public API. As in eat you own dog food. The
>(json)
>> client api resides in its own namespace which does *not* import
>Airflow at
>> all. So interfacing clients should not use the local_client.
>>
>
>Unfortunately this is not true - due to the way our __init_.py in
>airflow
>package is defined,
>pretty much everything in airflow has an implicit dependency on pretty
>much
>all the code. See above.
>Even if there are no "explicit" imports, just startup time is something
>that we cannot do anything about.
>
>In my opinion you are mixing several issues. The CLI has its issues,
>the
>> API has its issues and maybe our packaging has its issues. We should
>> address them separately.
>>
>
>I think Kamil is right here in mixing the issues. They are already
>mixed
>unfortunately. We need to decide now
>what is the target we want to achieve (separate api server? Separate
>client? Separate binaries? Separate python packages?
>Separate pypi packages?). Even if we do not implement it all and go
>through
>a transition period, we need to know where we
>are heading. Unfortunately we have this nasty airflow.__init__.py
>problem
>that I think severely hampers any decoupling effort
>we want to do - client/server and we need to decide first what we want
>to
>do and how to decouple (in the future) and define
>our roadmap how to get there and where to break backwards
>compatibility.
>
>
>
>Kamil:
>
>
>> I don't like the current remote mode in CLI for several reasons:
>> 1) The default API security configuration makes all API data public
>> 2) We do not provide sufficient security mechanisms to secure the
>API.
>> 3) The current API does not allow you to limit permissions so that
>the
>> user can only use the selected feature. If the user has access to the
>> API, he can do anything.
>>
>
>I think we should not be put-off by current API implementation, It was
>experimental after all. I think we have three options
>- implement completely new architecture/deployment approach
>- promote the current API to be "official" by adding the above
>- implement some kind of mixture with using the same technology -
>FAB but more of the features (open-api) and changing architecture to be
>more of a client-server separation (my preference).
>
>
>> 4) An abstraction leak between json_client and local_client.
>> a) The API Clients have no defined exceptions.
>> b) The API Clients have no defined return types
>>
>
>I think this can all be handled by adding it. And FAB's OpenApi +
>swagger
>specs is one of the more common/standard way of defining those, where
>we
>have documentation, generating clients in multiple languages etc.
>
>
>> 6) This forces duplicate code. This is dangerous because one method
>> may not be sufficiently tested and thus behave unexpectedly.
>>
>
>I agree we should not have duplicated code. We should remove local
>client.
>
>7) Not everything should be possible via the API. e.g. Tomek Urbaszek
>> is now working to add new commands that allow editing of the
>> airflow.cfg file.
>> PR: https://github.com/apache/airflow/pull/7130
>
>
>I think this depends on the general architecture we come up with. If we
>decide
>to come up with clear client/server split, the API for editing
>configuration
>will be the ONLY way to change it and it will be only available
>remotely.
>I think - similarly to Bolke - that in the "production" deployments,
>even the admins should not have access to the DB of Airflow, nor to
>configuration
>of Airflow. It should all be protected via API access mechanisms.
>
>
>> 8) We do not have sufficient separation of the API client and API
>> server. The API client and API server are in the same python package.
>> This seriously threatens the interoperability of this API. It's now
>> very easy to build an API that will return data in an unexpected
>> format.
>>
>
>Agree - what we have now is bad and can be easily mixed and abused.
>I think we should agree on the level of separation (python package/
>pypi package/ client-server deployment) we want to have and base all
>our
>further decisions on that.
>
>9) This means that people who want to use the API will only use the
>> specified client API. The same API client that is used by the CLI,
>> which is very unexpected because it has severe limitations.
>>
>
>Indeed. But I think we should remove those imitations.
>
>
>> I suggest that we do not develop remote mode in current CLI, and even
>> delete it in fear future.
>
>
>I think quite the opposite - we should turn all our interactions into
>remote ones.
>It might be that you want to achieve the same eventually. Maybe we talk
>about the same
>but look at it from a different perspective and different sequence of
>reaching the target.
>We need to talk a bit more about it to really understand whether we are
>not
>talking about
>the same long term goal - and then agree on what will be the technology
>to
>get us there
>and agree the way how to get there.
>
>
>> a) Build an API that meets the basic requirements:
>
>
>Yes. But I think it can be achieved by improving and extending features
>of
>current API. I think
>it is a matter of technology choice and what steps we are going to
>take.
>
>
>> b) Build an API client. It is best if the client is generated
>> automatically.
>>
>Yep. That's what FAB + OpenAPI + Swagger is very likely to deliver. We
>do
>not need to do it
>ourselves - this is a mostly solved problem.
>
>
>> c) We should build a CLI that provides access to the API in a secure
>> manner.
>>
>Agree. And again - this is what FAB + Open API seems to deliver pretty
>much
>out-of-the box.
>
>
>> Of course, we can try to build temporary solutions if the users need
>> it, but this should be very limited and should have a warning about
>> the level of stability and security. So if the user needs it can add
>> commands that support remote mode, but the user should be aware that
>> this is not a stable and secure solution.
>>
>
>I think it's already there -> "experimental" is a clear indication that
>this will likely
>change in the future.
>
>
>> I have plans for Q1 to build a stable API that does not have the
>above
>> issues.
>
>
>I think we need that fairly fast - for Airflow 2.0. And I am pretty
>convinced that if we use some existing
>framework we can implement it rather fast - providing that we will
>"stand
>on the shoulders of giants"
>and use what's already there. APIs in python programs are already
>solved
>problem. We do not need
>to build our own solution for that - just decide on architecture/split,
>plugin a framework and
>implement endpoints.
>
>
>J.
>
>-- 
>
>Jarek Potiuk
>Polidea <https://www.polidea.com/> | Principal Software Engineer
>
>M: +48 660 796 129 <+48660796129>
>[image: Polidea] <https://www.polidea.com/>

Re: [DISCUSS] Improving security by decoupling the CLI from the database

[Jarek Potiuk <Ja...@polidea.com>]

Some comments from my side:

Bolke:

Direct database access for users is a no go. Direct queries that in anyway
> manipulate the database or any state in Airfl ow (trigger dags, connections
> etc) from the CLI should be prevented.


Absolutely agree in principle. But we have to discuss and accept profound
consequences
of such change. For me - this also means that those commands cannot be run
on the machine where airflow is installed and CLI users should have no
access to those
machines - otherwise they can run arbitrary python method/class script
using airflow and
access the DB directly (it also means that there direct DB connection
should not be allowed)
This pretty much calls for either using scheduler as API server or
introduce a new
"API-server" entity.

This is quite a departure from the current Airflow deployment model and I
think it should be
"very breaking change" (architecturally/deployment wise). It could still
look like a local client for
the user - so it's not a breaking change from user point of view.

This also means that we do not need maybe 95% of airflow code and that we
should either
not use "airflow" package (airflow-cli maybe?), or do something with the
"__init__.py" of
the 'airflow' package. Currently we have a very bad import model if we want
to use "airflow" package
for everything. By doing 'import airflow.<whatever>" we are pulling in
almost all airflow code
because it is either directly or indirectly imported in the
airflow/__init__.py. Including DB
models. If we stick to having this __init__.py it means that we will
continue to have many
seconds of delays for starting up the CLI. I think it is very bad to have
seconds delays.

For me it really calls even for another package ("airflow-cli") that should
be installable
separately, Surely for local installation/development etc.  both could be
installed alongside and look
and behave as the current airflow does - and use localhost for
communication (and even we
should have the same backwards-compatible "airflow" binary). We could
gradually migrate all
the current CLI methods to switch to the new APIs.

I fundamentally disagree with your position that remote working with
> Airflow should be done through the Web UI.


I agree. As I see it - all the modern tools for cloud world have all three
modes for management -
Web UI (mostly used for reading and discovery), CLI - mostly used for
management and sometimes
for querying, and REST-ish APIS (for whatever custom integration is
needed).  Look at docker, gcloud
and many others. From our survey - it's clear that this is also similar in
our case. UI is used for monitoring
accessing logs etc more frequently than for management, and CLI - while
used less frequently in general
has a strong usage as well.

On your points:
>
> 1-3: Solved by using FAB
>

Indeed those three points might be solved by using FAB REST API. I am not
100% sure if FAB is the best
choice (although I think it is a clear contender). What I really like about
it that we seem to have a way to
use the same mechanism for authorisation/authentication as in UI. Using a
standard approach here seems
like a good idea, especially that out-of-the-box we can use all the
"standard" authentication
mechanisms: https://flask-appbuilder.readthedocs.io/en/latest/security.html# .
(DB, OpenAI, LDAP, Oauth, custom) -
they are all available out-of-the-box. What I also like is that FAB REST
API has everything you expect from
a modern API (
https://flask-appbuilder.readthedocs.io/en/latest/rest_api.html#openapi-spec
)
It implements "open-api" standard, has built-in swagger integration and the
capability of not only generating the
documentation, having a swagger test server for API testing but also
capability of automatically generating
clients in different languages (I'd love however to see POC showing all
that capabilities).
IMHO any proposal for API implementation will have to have all those
properties and
FAB is hard to beat on that especially that we already use it..


> 4. You main points are solved by using FAB. On B) I don’t think it is a
> good idea to have an extra layer between json_client and local_client. Both
> are pretty thin wrappers around `airflow.api.common.*`.
>

I think we should get rid of the local client asap and introduce only REST
API and API server. It's inevitable
IMHO and the sooner we introduce it - the better. This way we could
immediately compress the layers there
to 1 layer.


> 5. Non existent ;-)
>
Off-by-one indeed :)


> 6. I do agree that there is some code duplication going on, but its
> intention was there in order to have it in place until `local_client`
> wasn’t required anymore and the Rest API is mature. So mature the API, get
> rid of local_client.
>

I think we should do it ASAP and get rid of the local client (but provide
equivalent binary CLI to do
the same that current CLI does). Once we remove the local implementation we
should switch all the
CLI methods to use the API and eventually decouple it from main airflow
code once it is done.

7. Having the CLI manipulate `airflow.cfg` is fine as long as it checks if
> it has the right permissions to do so, the OS will enforce that anyway.
>
Agree.


> 8. They are not in the same namespace. `airflow.api.client.*` vs
> `airflow.www.api`. If you mean by ‘package’ ‘Airflow’, then I suggest maybe
> packaging the API/Client separately of break up the Airflow package in
> “airflow-common”, “airflow-client”, “airflow-server”.

Same thought exactly ^^^


> 9. That’s not unexpected: the interface is documented whilst not perfectly.
> The CLI *should* use the public API. As in eat you own dog food. The (json)
> client api resides in its own namespace which does *not* import Airflow at
> all. So interfacing clients should not use the local_client.
>

Unfortunately this is not true - due to the way our __init_.py in airflow
package is defined,
pretty much everything in airflow has an implicit dependency on pretty much
all the code. See above.
Even if there are no "explicit" imports, just startup time is something
that we cannot do anything about.

In my opinion you are mixing several issues. The CLI has its issues, the
> API has its issues and maybe our packaging has its issues. We should
> address them separately.
>

I think Kamil is right here in mixing the issues. They are already mixed
unfortunately. We need to decide now
what is the target we want to achieve (separate api server? Separate
client? Separate binaries? Separate python packages?
Separate pypi packages?). Even if we do not implement it all and go through
a transition period, we need to know where we
are heading. Unfortunately we have this nasty airflow.__init__.py problem
that I think severely hampers any decoupling effort
we want to do - client/server and we need to decide first what we want to
do and how to decouple (in the future) and define
our roadmap how to get there and where to break backwards compatibility.



Kamil:


> I don't like the current remote mode in CLI for several reasons:
> 1) The default API security configuration makes all API data public
> 2) We do not provide sufficient security mechanisms to secure the API.
> 3) The current API does not allow you to limit permissions so that the
> user can only use the selected feature. If the user has access to the
> API, he can do anything.
>

I think we should not be put-off by current API implementation, It was
experimental after all. I think we have three options
- implement completely new architecture/deployment approach
- promote the current API to be "official" by adding the above
- implement some kind of mixture with using the same technology -
FAB but more of the features (open-api) and changing architecture to be
more of a client-server separation (my preference).


> 4) An abstraction leak between json_client and local_client.
> a) The API Clients have no defined exceptions.
> b) The API Clients have no defined return types
>

I think this can all be handled by adding it. And FAB's OpenApi + swagger
specs is one of the more common/standard way of defining those, where we
have documentation, generating clients in multiple languages etc.


> 6) This forces duplicate code. This is dangerous because one method
> may not be sufficiently tested and thus behave unexpectedly.
>

I agree we should not have duplicated code. We should remove local client.

7) Not everything should be possible via the API. e.g. Tomek Urbaszek
> is now working to add new commands that allow editing of the
> airflow.cfg file.
> PR: https://github.com/apache/airflow/pull/7130


I think this depends on the general architecture we come up with. If we
decide
to come up with clear client/server split, the API for editing configuration
will be the ONLY way to change it and it will be only available remotely.
I think - similarly to Bolke - that in the "production" deployments,
even the admins should not have access to the DB of Airflow, nor to
configuration
of Airflow. It should all be protected via API access mechanisms.


> 8) We do not have sufficient separation of the API client and API
> server. The API client and API server are in the same python package.
> This seriously threatens the interoperability of this API. It's now
> very easy to build an API that will return data in an unexpected
> format.
>

Agree - what we have now is bad and can be easily mixed and abused.
I think we should agree on the level of separation (python package/
pypi package/ client-server deployment) we want to have and base all our
further decisions on that.

9) This means that people who want to use the API will only use the
> specified client API. The same API client that is used by the CLI,
> which is very unexpected because it has severe limitations.
>

Indeed. But I think we should remove those imitations.


> I suggest that we do not develop remote mode in current CLI, and even
> delete it in fear future.


I think quite the opposite - we should turn all our interactions into
remote ones.
It might be that you want to achieve the same eventually. Maybe we talk
about the same
but look at it from a different perspective and different sequence of
reaching the target.
We need to talk a bit more about it to really understand whether we are not
talking about
the same long term goal - and then agree on what will be the technology to
get us there
and agree the way how to get there.


> a) Build an API that meets the basic requirements:


Yes. But I think it can be achieved by improving and extending features of
current API. I think
it is a matter of technology choice and what steps we are going to take.


> b) Build an API client. It is best if the client is generated
> automatically.
>
Yep. That's what FAB + OpenAPI + Swagger is very likely to deliver. We do
not need to do it
ourselves - this is a mostly solved problem.


> c) We should build a CLI that provides access to the API in a secure
> manner.
>
Agree. And again - this is what FAB + Open API seems to deliver pretty much
out-of-the box.


> Of course, we can try to build temporary solutions if the users need
> it, but this should be very limited and should have a warning about
> the level of stability and security. So if the user needs it can add
> commands that support remote mode, but the user should be aware that
> this is not a stable and secure solution.
>

I think it's already there -> "experimental" is a clear indication that
this will likely
change in the future.


> I have plans for Q1 to build a stable API that does not have the above
> issues.


I think we need that fairly fast - for Airflow 2.0. And I am pretty
convinced that if we use some existing
framework we can implement it rather fast - providing that we will "stand
on the shoulders of giants"
and use what's already there. APIs in python programs are already solved
problem. We do not need
to build our own solution for that - just decide on architecture/split,
plugin a framework and
implement endpoints.


J.

-- 

Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer

M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>

Re: [DISCUSS] Improving security by decoupling the CLI from the database

[Bolke de Bruin <bd...@gmail.com>]

Hi Kamil,

Direct database access for users is a no go. Direct queries that in anyway
manipulate the database or any state in Airfl ow (trigger dags, connections
etc) from the CLI should be prevented. Not even root should be able to do
this as root does not have any meaning over the wire to Airflow. We cannot
trust the user. Period. I agree with you that the current state of the API
is abysmal. Hence I mentioned using Flask Application Builder framework for
this which does do fine-grained permissions and simplifies what we try to
accomplish.

I fundamentally disagree with your position that remote working with
Airflow should be done through the Web UI. Unix first and foremost
interface is the cli. I can use pipes, automation with other tools etc. I
use git from the command line. I use curl from the command line. If
triggering a dag becomes impossible from the command line we loose one of
our main selling points.

On your points:

1-3: Solved by using FAB
4. You main points are solved by using FAB. On B) I don’t think it is a
good idea to have an extra layer between json_client and local_client. Both
are pretty thin wrappers around `airflow.api.common.*`.
5. Non existent ;-)
6. I do agree that there is some code duplication going on, but its
intention was there in order to have it in place until `local_client`
wasn’t required anymore and the Rest API is mature. So mature the API, get
rid of local_client.
7. Having the CLI manipulate `airflow.cfg` is fine as long as it checks if
it has the right permissions to do so, the OS will enforce that anyway.
8. They are not in the same namespace. `airflow.api.client.*` vs
`airflow.www.api`. If you mean by ‘package’ ‘Airflow’, then I suggest maybe
packaging the API/Client separately of break up the Airflow package in
“airflow-common”, “airflow-client”, “airflow-server”.
9. That’s not unexpected: the interface is documented whilst not perfectly.
The CLI *should* use the public API. As in eat you own dog food. The (json)
client api resides in its own namespace which does *not* import Airflow at
all. So interfacing clients should not use the local_client.

In my opinion you are mixing several issues. The CLI has its issues, the
API has its issues and maybe our packaging has its issues. We should
address them separately.

Cheers
Bolke


On 18 January 2020 at 16:41:21, Kamil Breguła (kamil.bregula@polidea.com)
wrote:

In my opinion, we should go completely in the opposite direction. We
should limit remote mode from the current CLI and add support only
commands that use direct queries to the database as long term
solution. If users need remote mode, we can add these options, but
this should be marked with stability and safe warnings.

I don't like the current remote mode in CLI for several reasons:
1) The default API security configuration makes all API data public
2) We do not provide sufficient security mechanisms to secure the API.
Supported mechanisms are:
a) deny_all
b) accept_all
b) kerberos
This prevents many users from using it in production systems if
Kerberos is not used. If someone even unknowingly tries to use it will
be unsafe in too large case.
3) The current API does not allow you to limit permissions so that the
user can only use the selected feature. If the user has access to the
API, he can do anything.
4) An abstraction leak between json_client and local_client.
a) The API Clients have no defined exceptions.
This seriously hinders the development of CLI. In the past I wanted to
fix CLI so that unhandled exceptions would not appear, but too
much-complicated
b) The API Clients have no defined return types
This should be resolved by introducing an additional layer between
json_client and local_client. It should serialize and deserialize
response to common type. It should not be possible to return a
partially correct value e.g. without the key.
6) This forces duplicate code. This is dangerous because one method
may not be sufficiently tested and thus behave unexpectedly.
7) Not everything should be possible via the API. e.g. Tomek Urbaszek
is now working to add new commands that allow editing of the
airflow.cfg file.
PR: https://github.com/apache/airflow/pull/7130
8) We do not have sufficient separation of the API client and API
server. The API client and API server are in the same python package.
This seriously threatens the interoperability of this API. It's now
very easy to build an API that will return data in an unexpected
format.
9) This means that people who want to use the API will only use the
specified client API. The same API client that is used by the CLI,
which is very unexpected because it has severe limitations. The
biggest problem is that it's in the airflow package. Importing this
package has a lot of side effects. I don't know about everything that
happens, but I have two main issues:
a) a lot of new modules are being loaded into memory.
>>> len(sys.modules)
85
>>> import airflow
[2020-01-18 14:22:07,030] {settings.py:179} INFO -
settings.configure_orm(): Using pool settings. pool_size=5,
max_overflow=10, pool_recycle=1800, pid=136
>>> len(sys.modules)
1635
This problem has been reduced, but completely resolved by this PR.
https://github.com/apache/airflow/pull/6594

b) Changes logger configurations:
>>> import logging
>>> logging._handlers.data
{}
>>> import airflow
[2020-01-18 14:29:36,809] {settings.py:179} INFO -
settings.configure_orm(): Using pool settings. pool_size=5,
max_overflow=10, pool_recycle=1800, pid=150
>>> logging._handlers.data
{'console': <weakref at 0x7fd26e20aa40; to 'RedirectStdHandler' at
0x7fd26e1f1b38>, 'processor': <weakref at 0x7fd26e20ac50; to
'FileProcessorHandler' at 0x7fd26e435cf8>, 'task': <weakref at
0x7fd26e20ad00; to 'FileTaskHandler' at 0x7fd26e435f60>}
Changing the configuration of the logger in an unexpected way may
cause that some event will not be logged, which is harmful to the
audibility of system. This API Client cannot be used on financial
systems.

We can solve these problems at the code level, but I think we should
solve them at the design level. This is an open source project and we
cannot require users to be aware of how their change fully affects
applications. Now if we added the code in one place it affects the
client, server and CLI. The design should prevent such a situation.
This is possible through code isolation and appropriate integration
tests.

In my opinion, using this CLI in remote mode is too dangerous to be
used by end-users. The only secure way to remotely manage Airflow is
to use Web UI. Investing time in a solution whose design is so
dangerous can bring too many unexpected problems.

I suggest that we do not develop remote mode in current CLI, and even
delete it in fear future. Then we began to build the necessary
components step by step.
a) Build an API that meets the basic requirements:
- Secure: It should provide security mechanisms such as permission
levels, secure authentication.
- Interoperable: Using the API should be possible for any client. The
API should not be technology-dependent.
- Stable: The API should not undergo a big change between versions.
b) Build an API client. It is best if the client is generated
automatically.
Example: https://github.com/kubernetes-client/gen
c) We should build a CLI that provides access to the API in a secure
manner.

Of course, we can try to build temporary solutions if the users need
it, but this should be very limited and should have a warning about
the level of stability and security. So if the user needs it can add
commands that support remote mode, but the user should be aware that
this is not a stable and secure solution.

I have plans for Q1 to build a stable API that does not have the above
issues. If God allows me, I will start working on the client and CLI
in Q2, but this is not confirmed. Only work on a stable API is planned
for now. I will provide more information (including design docs/AIP)
when I finish working on AIP-21. AIP-21 is very much expected by my
clients and I want to focus on it first.

Thank you Bolke very much for raising the discussion about it. It is
definitely a component that requires a lot of attention and I hope
that we will be able to work out a solution that will satisfy
everyone. Thank you also for adding this mode to the project, because
I know that it solves the problem of many users, but unfortunately my
clients cannot use it and want to develop a solution that can be used
by current and future users.


On Sat, Jan 18, 2020 at 1:46 PM Bolke de Bruin <bd...@gmail.com> wrote:
>
> My suggestion is to have new funtionality (or even updates maybe) to
require implementing an API in local and json. That's an intermediate step
to stop allowing direct DB access.
>
> Basically I'm saying no new and updated CLI functionality is allowed to
use "airflow.utils.session" directly and needs to use "airflow.api.common"
(or alike).
>
> This can be done now.
>
> Recent FAB has some nice stuff for Rest APIs including authentication.
This integrates well with the rest of the wee stuff we have. Although i
would love to see a grpc implementation.
>
> B.
>
> Sent from my iPhone
>
> > On 18 Jan 2020, at 13:19, Jarek Potiuk <Ja...@polidea.com>
wrote:
> >
> > Absolutely yes. I think this is really what is in the current
plans/roadmap
> > :).
> >
> > It's just a matter of when and how to enforce it. The current
experimental
> > API is well .... still experimental.
> >
> > What we really need to do is implement much more complete API
> > support/approach - which is on the Airllow 2.0
> > roadmap. AFAIK - Kamil is going to start discussions and make some
> > proposals for that this week.
> > And decoupling CLI from DB is rather high on the list for the API I
believe.
> >
> > I think it's really important to solve it "well" - i.e. introduce
flexible
> > authorisation/authentication mechanism to the api,
> > have a way to decouple both client and web server from database
operations
> > and eventually decouple
> > workers from DB access. This is our ultimate goal and I think we should
> > define a broader picture/target now
> > (i.e. how to design the API so that it serves all that options in the
> > future) and have a plan on how to gradually
> > introduce it so that several contributors/commiters can take part in
this
> > process. One of the sequences
> > of introducing the API might be for example:
> >
> > 1) either graduate existing experimental API to be "official" or
introduce
> > a new "official"
> > API solution if we find it better
> > 2) reimplement all CLI commands to use the new API
> > 3) Reimplement web server to use the new API.
> > 4) (very long term) decouple workers from the database.
> >
> > I think forbidding CLI to access database should happen between 1) and
2) -
> > when we have an "official" API solution
> > in place (and it should be automatically verified - we can easily add
> > pylint plugin that can check CLI package for
> > db usage). In my opinion we cannot expect people to use API until it
goes
> > out of experimental/ we have a viable
> > stable long term alternative agreed.
> >
> > Once this is in place - no new CLI command should be allowed with the
> > direct DB access.
> >
> > J.
> >
> >
> >> On Sat, Jan 18, 2020 at 12:59 PM Bolke de Bruin <bd...@gmail.com>
wrote:
> >>
> >> Hi All,
> >>
> >> I’ve noticed that we are still implementing new features or are doing
> >> refactoring of CLI commands that directly interface with the database
> >> instead of using the abstractions that should be made available from
the
> >> API specification. Why is this an issue? The CLI is used by arbitrary
user
> >> to interface with Airflow operations. Airflow relies on the database
to be
> >> its single source of truth. A user that is able to read the
configuration
> >> of Airflow is currently able to manipulate the database. The CLI
requires
> >> database access hence the information to deal with the database is in
the
> >> configuration file. To improve security the CLI should use a rest API
which
> >> allows for proper authn/authz and segregation of duties.
> >>
> >> In the past I have introduced the experimental API with a
“local_client”
> >> and “json_client” implementation. The local_client still allows for
direct
> >> database access and its only function is to be there during the
transition
> >> period to have the full rest api available. After that it should be
> >> deprecated and removed.
> >>
> >> My suggestion is to disallow any new functionality in the CLI that
directly
> >> relies on “airflow.utils.session” and only allow new functionality to
go
> >> through the API client. For now that would mean 2 implementations:
local
> >> and json. Of course refactoring the current state should be on the
list in
> >> order to remove the “local_client”.
> >>
> >> The API client should be available to other packages as well. So maybe
we
> >> should package the cli and client api implementations into
> >> “airflow-client”.
> >>
> >> What are your thoughts?
> >>
> >> Thanks
> >> Bolke
> >>
> >
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>

Re: [DISCUSS] Improving security by decoupling the CLI from the database

[Kamil Breguła <ka...@polidea.com>]

In my opinion, we should go completely in the opposite direction. We
should limit remote mode from the current CLI and add support only
commands that use direct queries to the database as long term
solution. If users need remote mode, we can add these options, but
this should be marked with stability and safe warnings.

I don't like the current remote mode in CLI for several reasons:
1) The default API security configuration makes all API data public
2) We do not provide sufficient security mechanisms to secure the API.
Supported mechanisms are:
a) deny_all
b) accept_all
b) kerberos
This prevents many users from using it in production systems if
Kerberos is not used. If someone even unknowingly tries to use it will
be unsafe in too large case.
3) The current API does not allow you to limit permissions so that the
user can only use the selected feature. If the user has access to the
API, he can do anything.
4) An abstraction leak between json_client and local_client.
a) The API Clients have no defined exceptions.
This seriously hinders the development of CLI. In the past I wanted to
fix CLI so that unhandled exceptions would not appear, but too
much-complicated
b) The API Clients have no defined return types
This should be resolved by introducing an additional layer between
json_client and local_client. It should serialize and deserialize
response to common type. It should not be possible to return a
partially correct value e.g. without the key.
6) This forces duplicate code. This is dangerous because one method
may not be sufficiently tested and thus behave unexpectedly.
7) Not everything should be possible via the API. e.g. Tomek Urbaszek
is now working to add new commands that allow editing of the
airflow.cfg file.
PR:  https://github.com/apache/airflow/pull/7130
8) We do not have sufficient separation of the API client and API
server. The API client and API server are in the same python package.
This seriously threatens the interoperability of this API. It's now
very easy to build an API that will return data in an unexpected
format.
9) This means that people who want to use the API will only use the
specified client API. The same API client that is used by the CLI,
which is very unexpected because it has severe limitations. The
biggest problem is that it's in the airflow package. Importing this
package has a lot of side effects. I don't know about everything that
happens, but I have two main issues:
a) a lot of new modules are being loaded into memory.
>>> len(sys.modules)
85
>>> import airflow
[2020-01-18 14:22:07,030] {settings.py:179} INFO -
settings.configure_orm(): Using pool settings. pool_size=5,
max_overflow=10, pool_recycle=1800, pid=136
>>> len(sys.modules)
1635
This problem has been reduced, but completely resolved by this PR.
https://github.com/apache/airflow/pull/6594

b) Changes logger configurations:
>>> import logging
>>> logging._handlers.data
{}
>>> import airflow
[2020-01-18 14:29:36,809] {settings.py:179} INFO -
settings.configure_orm(): Using pool settings. pool_size=5,
max_overflow=10, pool_recycle=1800, pid=150
>>> logging._handlers.data
{'console': <weakref at 0x7fd26e20aa40; to 'RedirectStdHandler' at
0x7fd26e1f1b38>, 'processor': <weakref at 0x7fd26e20ac50; to
'FileProcessorHandler' at 0x7fd26e435cf8>, 'task': <weakref at
0x7fd26e20ad00; to 'FileTaskHandler' at 0x7fd26e435f60>}
Changing the configuration of the logger in an unexpected way may
cause that some event will not be logged, which is harmful to the
audibility of system. This API Client cannot be used on financial
systems.

We can solve these problems at the code level, but I think we should
solve them at the design level.  This is an open source project and we
cannot require users to be aware of how their change fully affects
applications. Now if we added the code in one place it affects the
client, server and CLI. The design should prevent such a situation.
This is possible through code isolation and appropriate integration
tests.

In my opinion, using this CLI in remote mode is too dangerous to be
used by end-users. The only secure way to remotely manage Airflow is
to use Web UI.  Investing time in a solution whose design is so
dangerous can bring too many unexpected problems.

I suggest that we do not develop remote mode in current CLI, and even
delete it in fear future. Then we began to build the necessary
components step by step.
a) Build an API that meets the basic requirements:
- Secure: It should provide security mechanisms such as permission
levels, secure authentication.
- Interoperable: Using the API should be possible for any client. The
API should not be technology-dependent.
- Stable: The API should not undergo a big change between versions.
b) Build an API client. It is best if the client is generated automatically.
Example: https://github.com/kubernetes-client/gen
c) We should build a CLI that provides access to the API in a secure manner.

Of course, we can try to build temporary solutions if the users need
it, but this should be very limited and should have a warning about
the level of stability and security. So if the user needs it can add
commands that support remote mode, but the user should be aware that
this is not a stable and secure solution.

I have plans for Q1 to build a stable API that does not have the above
issues. If God allows me, I will start working on the client and CLI
in Q2, but this is not confirmed. Only work on a stable API is planned
for now. I will provide more information (including design docs/AIP)
when I finish working on AIP-21. AIP-21 is very much expected by my
clients and I want to focus on it first.

Thank you Bolke very much for raising the discussion about it. It is
definitely a component that requires a lot of attention and I hope
that we will be able to work out a solution that will satisfy
everyone.  Thank you also for adding this mode to the project, because
I know that it solves the problem of many users, but unfortunately my
clients cannot use it and want to develop a solution that can be used
by current and future users.


On Sat, Jan 18, 2020 at 1:46 PM Bolke de Bruin <bd...@gmail.com> wrote:
>
> My suggestion is to have new funtionality (or even updates maybe) to require implementing an API in local and json. That's an intermediate step to stop allowing direct DB access.
>
> Basically I'm saying no new and updated CLI functionality is allowed to use "airflow.utils.session" directly and needs to use "airflow.api.common" (or alike).
>
> This can be done now.
>
> Recent FAB has some nice stuff for Rest APIs including authentication. This integrates well with the rest of the wee stuff we have. Although i would love to see a grpc implementation.
>
> B.
>
> Sent from my iPhone
>
> > On 18 Jan 2020, at 13:19, Jarek Potiuk <Ja...@polidea.com> wrote:
> >
> > Absolutely yes. I think this is really what is in the current plans/roadmap
> > :).
> >
> > It's just a matter of when and how to enforce it. The current experimental
> > API is well .... still experimental.
> >
> > What we really need to do is implement much more complete API
> > support/approach - which is on the Airllow 2.0
> > roadmap. AFAIK - Kamil is going to start discussions and make some
> > proposals for that this week.
> > And decoupling CLI from DB is rather high on the list for the API I believe.
> >
> > I think it's really important to solve it "well" - i.e. introduce flexible
> > authorisation/authentication mechanism to the api,
> > have a way to decouple both client and web server from database operations
> > and eventually decouple
> > workers from DB access. This is our ultimate goal and I think we should
> > define a broader picture/target now
> > (i.e. how to design the API so that it serves all that options in the
> > future) and have a plan on how to gradually
> > introduce it so that several contributors/commiters can take part in this
> > process. One of the sequences
> > of introducing the API might be for example:
> >
> > 1) either graduate existing experimental API to be "official" or introduce
> > a new "official"
> >    API solution if we find it better
> > 2) reimplement all CLI commands to use the new API
> > 3) Reimplement web server to use the new API.
> > 4) (very long term) decouple workers from the database.
> >
> > I think forbidding CLI to access database should happen between 1) and 2) -
> > when we have an "official" API solution
> > in place (and it should be automatically verified - we can easily add
> > pylint plugin that can check CLI package for
> > db usage). In my opinion we cannot expect people to use API until it goes
> > out of experimental/ we have a viable
> > stable long term alternative agreed.
> >
> > Once this is in place - no new CLI command should be allowed with the
> > direct DB access.
> >
> > J.
> >
> >
> >> On Sat, Jan 18, 2020 at 12:59 PM Bolke de Bruin <bd...@gmail.com> wrote:
> >>
> >> Hi All,
> >>
> >> I’ve noticed that we are still implementing new features or are doing
> >> refactoring of CLI commands that directly interface with the database
> >> instead of using the abstractions that should be made available from the
> >> API specification. Why is this an issue? The CLI is used by arbitrary user
> >> to interface with Airflow operations. Airflow relies on the database to be
> >> its single source of truth. A user that is able to read the configuration
> >> of Airflow is currently able to manipulate the database. The CLI requires
> >> database access hence the information to deal with the database is in the
> >> configuration file. To improve security the CLI should use a rest API which
> >> allows for proper authn/authz and segregation of duties.
> >>
> >> In the past I have introduced the experimental API with a “local_client”
> >> and “json_client” implementation. The local_client still allows for direct
> >> database access and its only function is to be there during the transition
> >> period to have the full rest api available. After that it should be
> >> deprecated and removed.
> >>
> >> My suggestion is to disallow any new functionality in the CLI that directly
> >> relies on “airflow.utils.session” and only allow new functionality to go
> >> through the API client. For now that would mean 2 implementations: local
> >> and json. Of course refactoring the current state should be on the list in
> >> order to remove the “local_client”.
> >>
> >> The API client should be available to other packages as well. So maybe we
> >> should package the cli and client api implementations into
> >> “airflow-client”.
> >>
> >> What are your thoughts?
> >>
> >> Thanks
> >> Bolke
> >>
> >
> >
> > --
> >
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>

Re: [DISCUSS] Improving security by decoupling the CLI from the database

[Bolke de Bruin <bd...@gmail.com>]

My suggestion is to have new funtionality (or even updates maybe) to require implementing an API in local and json. That's an intermediate step to stop allowing direct DB access. 

Basically I'm saying no new and updated CLI functionality is allowed to use "airflow.utils.session" directly and needs to use "airflow.api.common" (or alike). 

This can be done now. 

Recent FAB has some nice stuff for Rest APIs including authentication. This integrates well with the rest of the wee stuff we have. Although i would love to see a grpc implementation.

B.

Sent from my iPhone

> On 18 Jan 2020, at 13:19, Jarek Potiuk <Ja...@polidea.com> wrote:
> 
> Absolutely yes. I think this is really what is in the current plans/roadmap
> :).
> 
> It's just a matter of when and how to enforce it. The current experimental
> API is well .... still experimental.
> 
> What we really need to do is implement much more complete API
> support/approach - which is on the Airllow 2.0
> roadmap. AFAIK - Kamil is going to start discussions and make some
> proposals for that this week.
> And decoupling CLI from DB is rather high on the list for the API I believe.
> 
> I think it's really important to solve it "well" - i.e. introduce flexible
> authorisation/authentication mechanism to the api,
> have a way to decouple both client and web server from database operations
> and eventually decouple
> workers from DB access. This is our ultimate goal and I think we should
> define a broader picture/target now
> (i.e. how to design the API so that it serves all that options in the
> future) and have a plan on how to gradually
> introduce it so that several contributors/commiters can take part in this
> process. One of the sequences
> of introducing the API might be for example:
> 
> 1) either graduate existing experimental API to be "official" or introduce
> a new "official"
>    API solution if we find it better
> 2) reimplement all CLI commands to use the new API
> 3) Reimplement web server to use the new API.
> 4) (very long term) decouple workers from the database.
> 
> I think forbidding CLI to access database should happen between 1) and 2) -
> when we have an "official" API solution
> in place (and it should be automatically verified - we can easily add
> pylint plugin that can check CLI package for
> db usage). In my opinion we cannot expect people to use API until it goes
> out of experimental/ we have a viable
> stable long term alternative agreed.
> 
> Once this is in place - no new CLI command should be allowed with the
> direct DB access.
> 
> J.
> 
> 
>> On Sat, Jan 18, 2020 at 12:59 PM Bolke de Bruin <bd...@gmail.com> wrote:
>> 
>> Hi All,
>> 
>> I’ve noticed that we are still implementing new features or are doing
>> refactoring of CLI commands that directly interface with the database
>> instead of using the abstractions that should be made available from the
>> API specification. Why is this an issue? The CLI is used by arbitrary user
>> to interface with Airflow operations. Airflow relies on the database to be
>> its single source of truth. A user that is able to read the configuration
>> of Airflow is currently able to manipulate the database. The CLI requires
>> database access hence the information to deal with the database is in the
>> configuration file. To improve security the CLI should use a rest API which
>> allows for proper authn/authz and segregation of duties.
>> 
>> In the past I have introduced the experimental API with a “local_client”
>> and “json_client” implementation. The local_client still allows for direct
>> database access and its only function is to be there during the transition
>> period to have the full rest api available. After that it should be
>> deprecated and removed.
>> 
>> My suggestion is to disallow any new functionality in the CLI that directly
>> relies on “airflow.utils.session” and only allow new functionality to go
>> through the API client. For now that would mean 2 implementations: local
>> and json. Of course refactoring the current state should be on the list in
>> order to remove the “local_client”.
>> 
>> The API client should be available to other packages as well. So maybe we
>> should package the cli and client api implementations into
>> “airflow-client”.
>> 
>> What are your thoughts?
>> 
>> Thanks
>> Bolke
>> 
> 
> 
> -- 
> 
> Jarek Potiuk
> Polidea <https://www.polidea.com/> | Principal Software Engineer
> 
> M: +48 660 796 129 <+48660796129>
> [image: Polidea] <https://www.polidea.com/>

Re: [DISCUSS] Improving security by decoupling the CLI from the database

[Jarek Potiuk <Ja...@polidea.com>]

Absolutely yes. I think this is really what is in the current plans/roadmap
:).

It's just a matter of when and how to enforce it. The current experimental
API is well .... still experimental.

What we really need to do is implement much more complete API
support/approach - which is on the Airllow 2.0
roadmap. AFAIK - Kamil is going to start discussions and make some
proposals for that this week.
And decoupling CLI from DB is rather high on the list for the API I believe.

I think it's really important to solve it "well" - i.e. introduce flexible
authorisation/authentication mechanism to the api,
have a way to decouple both client and web server from database operations
and eventually decouple
workers from DB access. This is our ultimate goal and I think we should
define a broader picture/target now
(i.e. how to design the API so that it serves all that options in the
future) and have a plan on how to gradually
introduce it so that several contributors/commiters can take part in this
process. One of the sequences
of introducing the API might be for example:

1) either graduate existing experimental API to be "official" or introduce
a new "official"
    API solution if we find it better
2) reimplement all CLI commands to use the new API
3) Reimplement web server to use the new API.
4) (very long term) decouple workers from the database.

I think forbidding CLI to access database should happen between 1) and 2) -
when we have an "official" API solution
in place (and it should be automatically verified - we can easily add
pylint plugin that can check CLI package for
db usage). In my opinion we cannot expect people to use API until it goes
out of experimental/ we have a viable
stable long term alternative agreed.

Once this is in place - no new CLI command should be allowed with the
direct DB access.

J.


On Sat, Jan 18, 2020 at 12:59 PM Bolke de Bruin <bd...@gmail.com> wrote:

> Hi All,
>
> I’ve noticed that we are still implementing new features or are doing
> refactoring of CLI commands that directly interface with the database
> instead of using the abstractions that should be made available from the
> API specification. Why is this an issue? The CLI is used by arbitrary user
> to interface with Airflow operations. Airflow relies on the database to be
> its single source of truth. A user that is able to read the configuration
> of Airflow is currently able to manipulate the database. The CLI requires
> database access hence the information to deal with the database is in the
> configuration file. To improve security the CLI should use a rest API which
> allows for proper authn/authz and segregation of duties.
>
> In the past I have introduced the experimental API with a “local_client”
> and “json_client” implementation. The local_client still allows for direct
> database access and its only function is to be there during the transition
> period to have the full rest api available. After that it should be
> deprecated and removed.
>
> My suggestion is to disallow any new functionality in the CLI that directly
> relies on “airflow.utils.session” and only allow new functionality to go
> through the API client. For now that would mean 2 implementations: local
> and json. Of course refactoring the current state should be on the list in
> order to remove the “local_client”.
>
> The API client should be available to other packages as well. So maybe we
> should package the cli and client api implementations into
> “airflow-client”.
>
> What are your thoughts?
>
> Thanks
> Bolke
>


-- 

Jarek Potiuk
Polidea <https://www.polidea.com/> | Principal Software Engineer

M: +48 660 796 129 <+48660796129>
[image: Polidea] <https://www.polidea.com/>