> On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > RELEASE-NOTES.md, line 14 > > <https://reviews.apache.org/r/45042/diff/7/?file=1318568#file1318568line14> > > > > s/Support/Added support/ > > s/ZK/ZooKeeper/
Done > On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > docs/operations/security.md, line 26 > > <https://reviews.apache.org/r/45042/diff/7/?file=1318569#file1318569line26> > > > > s/Zookeeper/ZooKeeper/ (capital K) Done > On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > docs/operations/security.md, line 34 > > <https://reviews.apache.org/r/45042/diff/7/?file=1318569#file1318569line34> > > > > "To enable authentication for the announcer, see..." Done > On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > docs/operations/security.md, line 292 > > <https://reviews.apache.org/r/45042/diff/7/?file=1318569#file1318569line292> > > > > "The Thermos executor can be configured to authenticate with ZooKeeper > > and include an [ACL] on the nodes it creates, which will specify..." Done > On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > docs/operations/security.md, line 299 > > <https://reviews.apache.org/r/45042/diff/7/?file=1318569#file1318569line299> > > > > s/ACL/an ACL/ Done > On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > examples/vagrant/announcer-auth.json, line 1 > > <https://reviews.apache.org/r/45042/diff/7/?file=1318570#file1318570line1> > > > > Whoops, in the last round i intended for you to only _add_ world read > > access. We should not remove the auth and restricted write/create access - > > these are valuable to exercise in the vagrant environment. Done. I have a added read and delete permissions for world:anyone as validate_serverset.py also needs delete perms. > On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py, line > > 201 > > <https://reviews.apache.org/r/45042/diff/7/?file=1318572#file1318572line201> > > > > feel free to inline this below to avoid the extra var declaration Done > On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > src/main/python/apache/aurora/executor/common/announcer.py, lines 154-164 > > <https://reviews.apache.org/r/45042/diff/7/?file=1318573#file1318573line154> > > > > Use a list comprehension instead: > > > > ``` > > def to_acl(access): > > return make_acl(...) > > > > default_acl = [to_acl(a) for a in self._zk_auth.acl()] > > ``` Done > On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > src/main/python/apache/aurora/executor/common/announcer.py, lines 168-169 > > <https://reviews.apache.org/r/45042/diff/7/?file=1318573#file1318573line168> > > > > I assume you do the `[]` -> `None` conversion because the behavior is > > different for these args? > > > > If so, you can make the code more concise and avoid these variable > > reassignments: > > ``` > > auth_data = auth_data if auth_data else None > > ... > > default_acl = default_acl if default_acl else None > > ``` > > > > By instead doing this: > > ```python > > return KazooClient(self._ensemble, > > connection_retry=self.DEFAULT_RETRY_POLICY, > > default_acl=default_acl or None, > > auth_data=auth_data or None) > > ``` Done > On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > src/test/sh/org/apache/aurora/e2e/validate_serverset.py, line 50 > > <https://reviews.apache.org/r/45042/diff/7/?file=1318577#file1318577line50> > > > > Is this needed? If so, please include a comment explaining why the > > timeout needs to be at 30. This was an artifact of my separate e2e test. I have reverted the timeout to 10. - Kunal ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review126007 ----------------------------------------------------------- On March 30, 2016, 12:17 a.m., Kunal Thakar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45042/ > ----------------------------------------------------------- > > (Updated March 30, 2016, 12:17 a.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Repository: aurora > > > Description > ------- > > Add ACL support for announcer > https://issues.apache.org/jira/browse/AURORA-1643 > > Adding support for service discovery ZK authentication. ZK authentication > secrets should be stored in a file as json (as follows): > (Updated JSON format for config file) > ```json > { > "auth": [ > { > "scheme": "<scheme>", > "credential": "<plain_credential>" > } > ], > "acls": [ > { > "scheme": "<scheme>", > "credential": "<encrypted_credential>", > "permissions": { > "read": <bool>, > "write": <bool>, > "create": <bool>, > "delete": <bool>, > "admin": <bool>, > "all": <bool> > } > } > ] > } > ``` > > > Diffs > ----- > > RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0 > docs/operations/security.md 1a3d9b7e7ba4ec1952dc886d5fbeb6b85d994fb9 > examples/vagrant/announcer-auth.json PRE-CREATION > examples/vagrant/upstart/aurora-scheduler.conf > 120b89a1dc10a259940cb9527eb2517f19d04471 > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py > 6634506108c346f8c23b2da7cc8d20d09d07d590 > src/main/python/apache/aurora/executor/common/announcer.py > 79a9cfb6ac3a8444f09fb3658e6e859e06941ba4 > src/main/python/apache/aurora/executor/common/announcer_zkauth_schema.py > PRE-CREATION > > src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py > e9f7851292aef3a36da5da9b0fc333a7e7750cf3 > src/test/python/apache/aurora/executor/common/test_announcer.py > 142b58d5e577c9f4b8e2ae8473cffdea94eba21f > src/test/sh/org/apache/aurora/e2e/validate_serverset.py > fca1137bd2e7b1306a03dc2a54d2ef15b59af6a8 > > Diff: https://reviews.apache.org/r/45042/diff/ > > > Testing > ------- > > /vagrant/src/test/sh/org/apache/aurora/e2e/test_announcer_auth_end_to_end.sh > /vagrant/src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > > Thanks, > > Kunal Thakar > >
