You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gastón Kleiman <ga...@mesosphere.io> on 2017/03/07 17:14:32 UTC
Review Request 57386: Introduced changes to the auth photos needed for
DeleteNestedContainer.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/
-----------------------------------------------------------
Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
Repository: mesos
Description
-------
Introduced changes to the auth photos needed for DeleteNestedContainer.
Diffs
-----
include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
Diff: https://reviews.apache.org/r/57386/diff/1/
Testing
-------
Thanks,
Gast�n Kleiman
Re: Review Request 57386: Introduced changes to the auth protos
needed for DeleteNestedContainer.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
> On March 8, 2017, 1:06 a.m., Kevin Klues wrote:
> > I do not know enough about the auth code to do a proper review of this.
> >
> > Can you fix the summary though -- s/photos/protos/
Good catch! macOS' autocorrect isn't geeky enough, so I disabled it =).
- Gast�n
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/#review168227
-----------------------------------------------------------
On March 8, 2017, 11:08 a.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57386/
> -----------------------------------------------------------
>
> (Updated March 8, 2017, 11:08 a.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
>
>
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Introduced changes to the auth protos needed for DeleteNestedContainer.
>
>
> Diffs
> -----
>
> include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
> include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
>
>
> Diff: https://reviews.apache.org/r/57386/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 57386: Introduced changes to the auth photos
needed for DeleteNestedContainer.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/#review168227
-----------------------------------------------------------
I do not know enough about the auth code to do a proper review of this.
Can you fix the summary though -- s/photos/protos/
- Kevin Klues
On March 7, 2017, 5:14 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57386/
> -----------------------------------------------------------
>
> (Updated March 7, 2017, 5:14 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
>
>
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Introduced changes to the auth photos needed for DeleteNestedContainer.
>
>
> Diffs
> -----
>
> include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
> include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
>
>
> Diff: https://reviews.apache.org/r/57386/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 57386: Introduced changes to the authz protos
needed for RemoveNestedContainer.
Posted by Alexander Rojas <al...@mesosphere.io>.
> On March 9, 2017, 2:09 a.m., Vinod Kone wrote:
> > include/mesos/authorizer/acls.proto
> > Lines 448 (patched)
> > <https://reviews.apache.org/r/57386/diff/2/?file=1659012#file1659012line448>
> >
> > move this to #440.
Sorry to have seen this too late, but in the authorization protobufs we try to keep the acl's in order, given that there are too many entries and was getting hard to handle the introduction of new acls and the ones we deleted already.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/#review168389
-----------------------------------------------------------
On March 10, 2017, 7:02 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57386/
> -----------------------------------------------------------
>
> (Updated March 10, 2017, 7:02 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
>
>
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Introduced changes to the authz protos needed for RemoveNestedContainer.
>
>
> Diffs
> -----
>
> include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
> include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
>
>
> Diff: https://reviews.apache.org/r/57386/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 57386: Introduced changes to the auth protos
needed for DeleteNestedContainer.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/#review168389
-----------------------------------------------------------
Fix it, then Ship it!
LGTM modulo picking the right name.
include/mesos/authorizer/acls.proto
Lines 448 (patched)
<https://reviews.apache.org/r/57386/#comment240626>
move this to #440.
include/mesos/authorizer/authorizer.proto
Lines 191 (patched)
<https://reviews.apache.org/r/57386/#comment240627>
move this to #173
- Vinod Kone
On March 8, 2017, 11:08 a.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57386/
> -----------------------------------------------------------
>
> (Updated March 8, 2017, 11:08 a.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
>
>
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Introduced changes to the auth protos needed for DeleteNestedContainer.
>
>
> Diffs
> -----
>
> include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
> include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
>
>
> Diff: https://reviews.apache.org/r/57386/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 57386: Introduced changes to the auth protos
needed for DeleteNestedContainer.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/#review168447
-----------------------------------------------------------
In summary and description, change /auth/authz/. Using just auth doesn't clarify if you mean authorization or authentication (the best is to use the whole word).
Also, I don't think it is a good practice to use the same text in the summary as in the description, there was even a public discussion discouraging us about it.
- Alexander Rojas
On March 9, 2017, 2:54 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57386/
> -----------------------------------------------------------
>
> (Updated March 9, 2017, 2:54 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
>
>
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Introduced changes to the auth protos needed for DeleteNestedContainer.
>
>
> Diffs
> -----
>
> include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
> include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
>
>
> Diff: https://reviews.apache.org/r/57386/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 57386: Introduced changes to the authz protos
needed for RemoveNestedContainer.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
> On March 13, 2017, 9:15 p.m., Jiang Yan Xu wrote:
> > include/mesos/authorizer/authorizer.proto
> > Lines 174 (patched)
> > <https://reviews.apache.org/r/57386/diff/4/?file=1661470#file1661470line174>
> >
> > The comment above says
> >
> > ```
> > // NOTE: Values in this enum should be kept in
> > // numerical order to prevent accidental aliasing.
> > ```
> >
> > ?
Good catch! Fixed this in https://reviews.apache.org/r/57593/.
- Gast�n
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/#review168834
-----------------------------------------------------------
On March 10, 2017, 6:02 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57386/
> -----------------------------------------------------------
>
> (Updated March 10, 2017, 6:02 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
>
>
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Introduced changes to the authz protos needed for RemoveNestedContainer.
>
>
> Diffs
> -----
>
> include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
> include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
>
>
> Diff: https://reviews.apache.org/r/57386/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 57386: Introduced changes to the authz protos
needed for RemoveNestedContainer.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/#review168834
-----------------------------------------------------------
include/mesos/authorizer/authorizer.proto
Lines 174 (patched)
<https://reviews.apache.org/r/57386/#comment241079>
The comment above says
```
// NOTE: Values in this enum should be kept in
// numerical order to prevent accidental aliasing.
```
?
- Jiang Yan Xu
On March 10, 2017, 10:02 a.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57386/
> -----------------------------------------------------------
>
> (Updated March 10, 2017, 10:02 a.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
>
>
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Introduced changes to the authz protos needed for RemoveNestedContainer.
>
>
> Diffs
> -----
>
> include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
> include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
>
>
> Diff: https://reviews.apache.org/r/57386/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 57386: Introduced changes to the authz protos
needed for RemoveNestedContainer.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/#review168800
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rukletsov
On March 10, 2017, 6:02 p.m., Gast�n Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57386/
> -----------------------------------------------------------
>
> (Updated March 10, 2017, 6:02 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
>
>
> Bugs: MESOS-7120
> https://issues.apache.org/jira/browse/MESOS-7120
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Introduced changes to the authz protos needed for RemoveNestedContainer.
>
>
> Diffs
> -----
>
> include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
> include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
>
>
> Diff: https://reviews.apache.org/r/57386/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Gast�n Kleiman
>
>
Re: Review Request 57386: Introduced changes to the authz protos
needed for RemoveNestedContainer.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/
-----------------------------------------------------------
(Updated March 10, 2017, 6:02 p.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
Changes
-------
Renamed the protos.
Summary (updated)
-----------------
Introduced changes to the authz protos needed for RemoveNestedContainer.
Bugs: MESOS-7120
https://issues.apache.org/jira/browse/MESOS-7120
Repository: mesos
Description (updated)
-------
Introduced changes to the authz protos needed for RemoveNestedContainer.
Diffs (updated)
-----
include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
Diff: https://reviews.apache.org/r/57386/diff/4/
Changes: https://reviews.apache.org/r/57386/diff/3-4/
Testing
-------
Thanks,
Gast�n Kleiman
Re: Review Request 57386: Introduced changes to the authz protos
needed for DeleteNestedContainer.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/
-----------------------------------------------------------
(Updated March 9, 2017, 2:33 p.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
Changes
-------
Updated the summary/description.
Summary (updated)
-----------------
Introduced changes to the authz protos needed for DeleteNestedContainer.
Bugs: MESOS-7120
https://issues.apache.org/jira/browse/MESOS-7120
Repository: mesos
Description (updated)
-------
See the summary.
Diffs
-----
include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
Diff: https://reviews.apache.org/r/57386/diff/3/
Testing
-------
Thanks,
Gast�n Kleiman
Re: Review Request 57386: Introduced changes to the auth protos
needed for DeleteNestedContainer.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/
-----------------------------------------------------------
(Updated March 9, 2017, 1:54 p.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
Changes
-------
Addressed Vinod's feedback.
Bugs: MESOS-7120
https://issues.apache.org/jira/browse/MESOS-7120
Repository: mesos
Description
-------
Introduced changes to the auth protos needed for DeleteNestedContainer.
Diffs (updated)
-----
include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
Diff: https://reviews.apache.org/r/57386/diff/3/
Changes: https://reviews.apache.org/r/57386/diff/2-3/
Testing
-------
Thanks,
Gast�n Kleiman
Re: Review Request 57386: Introduced changes to the auth protos
needed for DeleteNestedContainer.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/
-----------------------------------------------------------
(Updated March 8, 2017, 11:08 a.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
Changes
-------
Fixed typo in summary.
Summary (updated)
-----------------
Introduced changes to the auth protos needed for DeleteNestedContainer.
Bugs: MESOS-7120
https://issues.apache.org/jira/browse/MESOS-7120
Repository: mesos
Description (updated)
-------
Introduced changes to the auth protos needed for DeleteNestedContainer.
Diffs (updated)
-----
include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
Diff: https://reviews.apache.org/r/57386/diff/2/
Changes: https://reviews.apache.org/r/57386/diff/1-2/
Testing
-------
Thanks,
Gast�n Kleiman
Re: Review Request 57386: Introduced changes to the auth photos
needed for DeleteNestedContainer.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57386/
-----------------------------------------------------------
(Updated March 7, 2017, 5:14 p.m.)
Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, Jie Yu, Kevin Klues, and Vinod Kone.
Bugs: MESOS-7120
https://issues.apache.org/jira/browse/MESOS-7120
Repository: mesos
Description
-------
Introduced changes to the auth photos needed for DeleteNestedContainer.
Diffs
-----
include/mesos/authorizer/acls.proto 8389917d12f9a9a3c9b4281f48e23ade14c20528
include/mesos/authorizer/authorizer.proto fdc4817ce74c45d792fc47f064f7909a83b1657d
Diff: https://reviews.apache.org/r/57386/diff/1/
Testing
-------
Thanks,
Gast�n Kleiman