Cool! On Fri, Apr 3, 2020 at 12:44 PM Kamil Breguła <kamil.breg...@polidea.com> wrote:
> The vote was open for more than 72 hours, so I hope we have a common > consensus and we don't need to vote again. > > This vote has passed with 4 +1 votes and no -1 votes. > > +1 votes: > * Kamil Breguła > * Tomek Urbaszek > * Jarek Potiuk > * Ash Berlin-Taylor > > -1 votes: > N/A > > The adventure will start soon. 😺 > > Kamil Breguła > > On Fri, Apr 3, 2020 at 12:11 PM Jarek Potiuk <jarek.pot...@polidea.com> > wrote: > > > > I think in the future we can treat them as code change I think. I cannot > > imagine us voting on PRs to merge :). That would be quite a burden. > > > > What I really like about the voting process that there is also -0.9, > -0.5, > > -0. +0 .. I feel we should start using those more often. I'd say the -1 > > from Dan was really -0.9 :). > > > > J. > > > > > > On Fri, Apr 3, 2020 at 12:06 PM Ash Berlin-Taylor <a...@apache.org> > wrote: > > > > > It's not clear to me if votes on AIPs count as a code change or not -- > > > for example we passed the Dag serialzation AIP with a -1 from Dan. > > > > > > Anyway, with the change to the proposed permissions model I convert my > > > vote to a +1 anyway :) > > > > > > (Thanks for doing making that change Kamil. We can always revisit the > > > permissions modelling later if it turns out to be much more > > > complex/difficult/bad. Nothing is for ever) > > > > > > -ash > > > > > > On Apr 3 2020, at 10:07 am, Jarek Potiuk <jarek.pot...@polidea.com> > wrote: > > > > > > > I believe -1 from Ash suspended the vote (actually the wors used in > the > > > > Apache Voting rules is "kill the proposal") and he must withdraw it > in > > > > order to continue. See > > > > > https://www.apache.org/foundation/voting.html#votes-on-code-modification > > > > > > > > J. > > > > > > > > On Fri, Apr 3, 2020 at 10:59 AM Kamil Breguła < > kamil.breg...@polidea.com > > > > > > > > wrote: > > > > > > > >> I don't think it will ever be suspended. We have a lower limit of 72 > > > >> hours. This allows anyone to speak regardless of their time zone and > > > >> prevents changes that are not widely recognized. However, if we want > > > >> to talk longer, I do not think that it will be necessary to suspend > > > >> voting. > > > >> > > > >> On Fri, Apr 3, 2020 at 10:28 AM Jarek Potiuk < > jarek.pot...@polidea.com> > > > >> wrote: > > > >> > > > > >> > Looks good to me. > > > >> > > > > >> > +1. Binding > > > >> > > > > >> > Can we resume the vote? > > > >> > > > > >> > J. > > > >> > > > > >> > On Fri, Apr 3, 2020 at 9:17 AM Kamil Breguła < > > > kamil.breg...@polidea.com> > > > >> > wrote: > > > >> > > > > >> > > Hello, > > > >> > > > > > >> > > That's right The authentication will be based on Connexion. > However, > > > >> > > this will not be necessary. Users will be able to add a new > > > >> > > authentication method if necessary. For example, you can easily > > > >> > > integrate Airflow with your own identity proxy, which will > provide > > > >> > > permissions using the JWT token in HTTP headers. > > > >> > > > > > >> > > The author of the authentication method will have to set the > user > > > >> > > attribute in Flask context. FAB and flask_login work in the same > > > way. > > > >> > > A simple code example that authenticates a user with an HTTP > header > > > >> > > looks like this. > > > >> > > > > > >> > > from flask import request, g > > > >> > > > > > >> > > REMOTE_USER_HEADER = 'REMOTE_USER' > > > >> > > > > > >> > > username = request.headers.get(REMOTE_USER_HEADER) > > > >> > > if not username: > > > >> > > raise AuthenticationProblem( > > > >> > > 403, "Forbidden", f"Header {REMOTE_USER_HEADER} is > > > >> missing in > > > >> > > the request" > > > >> > > ) > > > >> > > > > > >> > > if not request.authorization: > > > >> > > user = current_app.appbuilder.sm > > > .auth_user_remote_user(username) > > > >> > > if user is None: > > > >> > > raise AuthenticationProblem( > > > >> > > 403, "Forbidden", f"Not authorized" > > > >> > > ) > > > >> > > log.info("User authorized: %s", user) > > > >> > > g.user = user > > > >> > > > > > >> > > It will be easy to add more methods, but I would not like this > > > AIP-32 > > > >> > > to deal with specific authentication methods. Authentication > methods > > > >> > > depend on the organization and it is not possible to create a > > > >> > > universal mechanism. > > > >> > > > > > >> > > The authorization will be based on FAB. The user will be able to > > > >> > > customize the mechanism through SECURITY_MANAGER_CLASS option in > > > >> > > [AIRFLOW_HOME]/webserver_config.py file. Identical to the > webserver. > > > >> > > We will use the same code. > > > >> > > > > > >> > > Best regards, > > > >> > > Kamil > > > >> > > > > > >> > > > > > >> > > On Sun, Mar 22, 2020 at 9:59 AM Jarek Potiuk < > > > jarek.pot...@polidea.com > > > >> > > > > >> > > wrote: > > > >> > > > > > > >> > > > I think the main idea here was to delegate the authentication > > > >> to what > > > >> > > > connexion provides (it has various authentication plugins). > And I > > > >> agree > > > >> > > > authorization should be addressed in the design as it cannot > be > > > >> solved by > > > >> > > > connexion "standard" plugins nor Open API definition - this is > > > more > > > >> of > > > >> > > > application choice. > > > >> > > > > > > >> > > > I think we should get som simple, yet configurable mechanism > for > > > >> > > > authorization - which should be similar to what we have in > FAB now > > > >> but we > > > >> > > > should learn from its problems. I think we should first agree > > > >> on the > > > >> > > > principles and features we want to achieve and then decide > how to > > > >> > > proceed. > > > >> > > > > > > >> > > > What I think this authorization system for the API > > > >> should/should not > > > >> do > > > >> > > > (this is my personal view - others might have different > > > >> opinion, so > > > >> feel > > > >> > > > free to express yours): > > > >> > > > > > > >> > > > * it should not be view-centered but API endpoint-centered. > > > >> > > > * It should allow assigning the users to different Roles - > example > > > >> roles > > > >> > > - > > > >> > > > "Admin", "Editor", .... > > > >> > > > * It should allow to bundle several API calls together and > assign > > > >> rights > > > >> > > to > > > >> > > > the API "bundles" to the user roles. Example "connections", > > > "dags", > > > >> > > "pools" > > > >> > > > .... . Again - those should be per API not per View. > > > >> > > > * the Bundles should have "Read"/"Write" access types > > > >> > > > * the Roles do not have to have UI to manage it - it could be > done > > > >> by a > > > >> > > > configuration file > > > >> > > > * it should be discoverable - the UI code should be able to > > > discover > > > >> > > which > > > >> > > > API Bundles it has access to (this will allow implementing > dynamic > > > >> views > > > >> > > > that will adapt to the different Roles). > > > >> > > > * optionally the users should have the capability of > constraining > > > >> rights > > > >> > > to > > > >> > > > certain resources per "resource" (so for example access to > some > > > dags > > > >> > > only). > > > >> > > > We could implement a very simple 1-1 mapping of the current > > > "owner" > > > >> > > > approach, and in the future, we could implement "User Groups" > and > > > >> have > > > >> > > > "Resource-per-Group" authorization. I don't think it is for > > > Airflow > > > >> 2.0 > > > >> > > and > > > >> > > > we should add it later (this is application level rather than > API > > > >> level > > > >> > > > feature). > > > >> > > > > > > >> > > > J. > > > >> > > > > > > >> > > > On Thu, Mar 19, 2020 at 1:31 PM Ash Berlin-Taylor < > a...@apache.org > > > > > > > >> > > wrote: > > > >> > > > > > > >> > > > > -1 (binding) for the moment, sorry. This is mostly because > of > > > the > > > >> > > proposed > > > >> > > > > permissions solution. > > > >> > > > > > > > >> > > > > I am happy with the spec-first approach, and feel we can get > > > there > > > >> on > > > >> > > > > the exact API methods, what IDs we expose or don't etc, but > this > > > >> > > > > permissions is a deal breaker for me as it stands. > > > >> > > > > > > > >> > > > > From your last response I am still not sure 100% what you > are > > > >> > > proposing, > > > >> > > > > and it feels like we are fighting against FAB rather than > > > working > > > >> with > > > >> > > > > it. For example you say: > > > >> > > > > > > > >> > > > > > all we have to do is replace steps 4 and retrieve > information > > > >> from > > > >> > > the > > > >> > > > > > code, not the database. > > > >> > > > > > > > >> > > > > Do the permissions management screens in FAB still work -- > > > >> i.e. can > > > >> > > > > someone choose to give a user/role access to only a single > API > > > >> > > endpoint? > > > >> > > > > If so how do we achieve that without having to re-write the > > > >> Security > > > >> > > > > screens from FAB. > > > >> > > > > > > > >> > > > > What is wrong with the FAB database approach that means we > > > >> have to > > > >> > > > > re-write or > > > >> > > > > customize it's behavoiur? > > > >> > > > > > > > >> > > > > Yes our current permissions approach isn't great, but that > is > > > just > > > >> how > > > >> > > > > we've "chosen" to do it in Airflow, it's not a problem with > the > > > >> > > > > underlying permissions model. For example a different way of > > > doing > > > >> it: > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > >> > > > > https://github.com/apache/airflow/pull/7251/files#diff-948e87b4f8f644b3ad8c7950958df033R2719-R2722 > > > >> > > > > > > > >> > > > > This added a (AJAX-style) method to DagModelView, and > reuses the > > > >> > > > > "can_list" permission to achive it - because the > > > >> "action"/verb is > > > >> about > > > >> > > > > listing, it doesn't ever make sense to deny autocompletion > if > > > you > > > >> can > > > >> > > > > list. > > > >> > > > > > > > >> > > > > If I understand your proposal correctly, you are suggesting > > > >> something > > > >> > > > > like "can_edit_variable on APIView"? Why not "can_edit on > > > >> > > > > Variable" (or VariableView), and then that one permission > could > > > >> apply > > > >> > > to > > > >> > > > > web UI and API. (Without the "on X" part I think a lot fo > FAB > > > won't > > > >> > > work > > > >> > > > > right.) > > > >> > > > > > > > >> > > > > So specific things I want to see addressed before I will +1 > > > this. > > > >> > > > > > > > >> > > > > - How do we manage API permissions for custom roles? (i.e. > do > > > the > > > >> > > > > existing screens work, how usable are they?) > > > >> > > > > - What "ViewMenu" is the permission tied to? > > > >> > > > > - What do these look like the Security screens? > > > >> > > > > - How do we manage these API permissions on a per-DAG level? > > > >> > > > > - Are we unifying the API permissions and the front > > > >> end-permissions? > > > >> > > > > > > > >> > > > > I feel we are close on this AIP! > > > >> > > > > > > > >> > > > > Ash > > > >> > > > > > > > >> > > > > On Thu Mar 19, 2020 at 10:55 AM, Jarek Potiuk wrote: > > > >> > > > > > +1 binding > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > On Thu, Mar 19, 2020 at 10:32 AM Tomasz Urbaszek < > > > >> > > > > > tomasz.urbas...@polidea.com> wrote: > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > +1 binding > > > >> > > > > > > > > > >> > > > > > > On Thu, Mar 19, 2020 at 10:29 AM Kamil Bregu=C5=82a > > > >> > > > > <kamil.bregula@polide= > > > >> > > > > > a.com> > > > >> > > > > > > wrote: > > > >> > > > > > > > > > >> > > > > > > > Hello all, > > > >> > > > > > > > > > > >> > > > > > > > This email calls for a vote on the design proposed in > > > AIP-32, > > > >> > > found > > > >> > > > > her= > > > >> > > > > > e > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > >> > > > > > >> > > > > https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-32%3A+Airflow+RES= > > > >> > > > > > T+API > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > >> > > > > > >> > > > > https://lists.apache.org/thread.html/r2a0d0fb3d4610432fa52148d7d9e59c7632= > > > >> > > > > > dd8f2fa61a580430b814c%40%3Cdev.airflow.apache.org%3E > > > >> > > > > > > > > > > >> > > > > > > > A few notes: > > > >> > > > > > > > > > > >> > > > > > > > - The proposed in AIP is to use an the "Specs first" > > > >> approach. > > > >> > > > > > > > First, we make the change in the openapi.yaml file, > and > > > >> then > > > >> > > > > > > > we change the code. > > > >> > > > > > > > - This AIP allows for high granulation. Many people > can > > > work > > > >> on > > > >> > > > > > > > smaller independent tasks. Already the application > from > > > >> > > Outreachy > > > >> > > > > > > > internships asked me how they can work on this AIP. > > > >> > > > > > > > - Details of the API structure may still change until > the > > > >> first > > > >> > > > > version > > > >> > > > > > > > is released. > > > >> > > > > > > > > > > >> > > > > > > > This vote will last for 72 hours until > > > >> 2020-03-22T10:30Z, and > > > >> > > until > > > >> > > > > at > > > >> > > > > > > > least 3 votes have been cast. > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > >> > > > > > >> > > > > https://www.timeanddate.com/worldclock/fixedtime.html?iso=3D20200322T1030= > > > >> > > > > > &p1=3D262 > > > >> > > > > > > > > > > >> > > > > > > > This is my +1 vote. > > > >> > > > > > > > > > > >> > > > > > > > Thanks, > > > >> > > > > > > > Kamil > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > -- > > > >> > > > > > > > > > >> > > > > > > Tomasz Urbaszek > > > >> > > > > > > Polidea <https://www.polidea.com/> | Software Engineer > > > >> > > > > > > > > > >> > > > > > > M: +48 505 628 493 <+48505628493> > > > >> > > > > > > E: tomasz.urbas...@polidea.com < > > > tomasz.urbasz...@polidea.com> > > > >> > > > > > > > > > >> > > > > > > Unique Tech > > > >> > > > > > > Check out our projects! < > https://www.polidea.com/our-work> > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > --=20 > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > Jarek Potiuk > > > >> > > > > > Polidea <https://www.polidea.com/> | Principal Software > > > Engineer > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > M: +48 660 796 129 <+48660796129> > > > >> > > > > > [image: Polidea] <https://www.polidea.com/> > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > >> > > > -- > > > >> > > > > > > >> > > > Jarek Potiuk > > > >> > > > Polidea <https://www.polidea.com/> | Principal Software > Engineer > > > >> > > > > > > >> > > > M: +48 660 796 129 <+48660796129> > > > >> > > > [image: Polidea] <https://www.polidea.com/> > > > >> > > > > > >> > > > > >> > > > > >> > -- > > > >> > > > > >> > Jarek Potiuk > > > >> > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > >> > > > > >> > M: +48 660 796 129 <+48660796129> > > > >> > [image: Polidea] <https://www.polidea.com/> > > > >> > > > > > > > > > > > > -- > > > > > > > > Jarek Potiuk > > > > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > > > > > > M: +48 660 796 129 <+48660796129> > > > > [image: Polidea] <https://www.polidea.com/> > > > > > > > > > > > > > -- > > > > Jarek Potiuk > > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > > M: +48 660 796 129 <+48660796129> > > [image: Polidea] <https://www.polidea.com/> > -- Jarek Potiuk Polidea <https://www.polidea.com/> | Principal Software Engineer M: +48 660 796 129 <+48660796129> [image: Polidea] <https://www.polidea.com/>
