You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2018/10/16 19:44:21 UTC
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11696
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
This patch adds a test coverage for catalogd restart when authorization
is enabled to ensure all privileges in the impalad's catalogs get reset
after the catalogd restart to avoid stale privileges in the impalad's
catalogs, which can pose a security issue.
Testing:
- Ran all E2E authorization tests
- Added a new test
Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
---
M tests/authorization/test_authorization.py
1 file changed, 58 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/11696/2
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1061/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 20:18:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3322/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 01:36:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 2:
(2 comments)
Thanks for adding the test!
http://gerrit.cloudera.org:8080/#/c/11696/2/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11696/2/tests/authorization/test_authorization.py@397
PS2, Line 397: role_name_prefix = "cr_role"
We use "unique database" in some places. Roles are global, so we should maybe use unique role_name_prefix here to make sure this is clean in that sense.
http://gerrit.cloudera.org:8080/#/c/11696/2/tests/authorization/test_authorization.py@413
PS2, Line 413: result = self.client.execute("show grant role %s_foo" % role_name_prefix)
: assert len(result.data) == 1
: TestAuthorization._check_privileges(result.data[0].split("\t"),
: ["database", "functional", "", "", "",
: "all", "false"])
:
: result = self.client.execute("show grant role %s_bar" % role_name_prefix)
: assert len(result.data) == 1
: TestAuthorization._check_privileges(result.data[0].split("\t"),
: ["database", "functional_kudu", "", "", "",
: "all", "false"])
:
: result = self.client.execute("show grant role %s_baz" % role_name_prefix)
: assert len(result.data) == 1
: TestAuthorization._check_privileges(result.data[0].split("\t"),
: ["database", "functional_avro", "", "", "",
: "all", "false"])
Can you run this both before and after the restart?
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 20:03:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1071/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 02:08:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 5: Code-Review+2
(2 comments)
Carry Phil's +2.
http://gerrit.cloudera.org:8080/#/c/11696/4/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11696/4/tests/authorization/test_authorization.py@399
PS4, Line 399: TestAuthorization._check_privileges(result, [["database", "functional",
: "", "", "", "all", "false"]])
:
: result = self.client.execute("show grant role %s_bar"
> If you really wanted to, you could make 399 part of _check_privileges(). An
I like the conciseness. Done.
http://gerrit.cloudera.org:8080/#/c/11696/4/tests/authorization/test_authorization.py@443
PS4, Line 443:
> Where is create_time here? I don't quite see what this is skipping.
I forgot to remove this. It's no longer needed. Done.
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 01:34:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 4: Code-Review+2
(2 comments)
I think this is fine. You could make the asserts tighter, but I don't care too much: they're clear enough.
http://gerrit.cloudera.org:8080/#/c/11696/4/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11696/4/tests/authorization/test_authorization.py@399
PS4, Line 399: assert len(result.data) == 1
: TestAuthorization._check_privileges(result.data[0].split("\t"),
: ["database", "functional", "", "", "",
: "all", "false"])
If you really wanted to, you could make 399 part of _check_privileges(). And you could probably make this entirely:
assert_equals([[...]], map(lambda x: x.split(..), result.data))
or something
http://gerrit.cloudera.org:8080/#/c/11696/4/tests/authorization/test_authorization.py@443
PS4, Line 443: # We ignore the create_time since this can be NULL depending on the Sentry refresh.
Where is create_time here? I don't quite see what this is skipping.
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 20:48:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11696/2/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11696/2/tests/authorization/test_authorization.py@397
PS2, Line 397: self.role_cleanup(unique_role)
> We use "unique database" in some places. Roles are global, so we should may
Using unique_database will create a database which we don't need. Furthermore, it also requires a special CREATE privilege which needs to be set up, but is unnecessary for this particular test. I decided to create a new fixture called unique_role instead, which returns a unique role name.
http://gerrit.cloudera.org:8080/#/c/11696/2/tests/authorization/test_authorization.py@413
PS2, Line 413: "all", "false"])
:
: result = self.client.execute("show grant role %s_bar" % unique_role)
: assert len(result.data) == 1
: TestAuthorization._check_privileges(result.data[0].split("\t"),
: ["database", "functional_kudu", "", "", "",
: "all", "false"])
:
: result = self.client.execute("show grant role %s_baz" % unique_role)
: assert len(result.data) == 1
: TestAuthorization._check_privileges(result.data[0].split("\t"),
: ["database", "functional_avro", "", "", "",
: "all", "false"])
:
: self._start_impala_cluster(["--catalogd_args=--sentry_config=%s" %
: SENTRY_CONFIG_FILE, "--restart_catalogd_only"])
:
> Can you run this both before and after the restart?
Done
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 20:25:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
This patch adds a test coverage for catalogd restart when authorization
is enabled to ensure all privileges in the impalad's catalogs get reset
after the catalogd restart to avoid stale privileges in the impalad's
catalogs, which can pose a security issue.
Testing:
- Ran all E2E authorization tests
- Added a new test
Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
---
M tests/authorization/test_authorization.py
M tests/conftest.py
2 files changed, 87 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/11696/3
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
This patch adds a test coverage for catalogd restart when authorization
is enabled to ensure all privileges in the impalad's catalogs get reset
after the catalogd restart to avoid stale privileges in the impalad's
catalogs, which can pose a security issue.
Testing:
- Ran all E2E authorization tests
- Added a new test
Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
---
M tests/authorization/test_authorization.py
M tests/conftest.py
2 files changed, 72 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/11696/4
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
This patch adds a test coverage for catalogd restart when authorization
is enabled to ensure all privileges in the impalad's catalogs get reset
after the catalogd restart to avoid stale privileges in the impalad's
catalogs, which can pose a security issue.
Testing:
- Ran all E2E authorization tests
- Added a new test
Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Reviewed-on: http://gerrit.cloudera.org:8080/11696
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/authorization/test_authorization.py
M tests/conftest.py
2 files changed, 67 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11696/3/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11696/3/tests/authorization/test_authorization.py@409
PS3, Line 409: result = self.client.execute("show grant role %s_foo" % unique_role)
: assert len(result.data) == 1
: TestAuthorization._check_privileges(result.data[0].split("\t"),
: ["database", "functional", "", "", "",
: "all", "false"])
:
: result = self.client.execute("show grant role %s_bar" % unique_role)
: assert len(result.data) == 1
: TestAuthorization._check_privileges(result.data[0].split("\t"),
: ["database", "functional_kudu", "", "", "",
: "all", "false"])
:
: result = self.client.execute("show grant role %s_baz" % unique_role)
: assert len(result.data) == 1
: TestAuthorization._check_privileges(result.data[0].split("\t"),
: ["database", "functional_avro", "", "", "",
: "all", "false"])
Can you wrap this in a function (can be local) and then run it twice?
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 20:37:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1063/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 20:59:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 05:33:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1064/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 21:12:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 01:36:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
This patch adds a test coverage for catalogd restart when authorization
is enabled to ensure all privileges in the impalad's catalogs get reset
after the catalogd restart to avoid stale privileges in the impalad's
catalogs, which can pose a security issue.
Testing:
- Ran all E2E authorization tests
- Added a new test
Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
---
M tests/authorization/test_authorization.py
M tests/conftest.py
2 files changed, 67 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/96/11696/5
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11696 )
Change subject: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11696/3/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11696/3/tests/authorization/test_authorization.py@409
PS3, Line 409:
: result = self.client.execute("show grant role %s_baz" % unique_role)
: assert len(result.data) == 1
: TestAuthorization._check_privileges(result.data[0].split("\t"),
: ["database", "functional_avro", "", "", "",
: "all", "false"])
:
: self.role_cleanup(unique_role)
: try:
: self.client.execute("create role %s_foo" % unique_role)
: self.client.execute("create role %s_bar" % unique_role)
: self.client.execute("create role %s_baz" % unique_role)
: self.client.execute("grant all on database functional to role %s_foo" %
: unique_role)
: self.client.execute("grant all on database functional_kudu to role %s_bar" %
: unique_role)
: self.client.execute("grant all on database functional
> Can you wrap this in a function (can be local) and then run it twice?
Done
--
To view, visit http://gerrit.cloudera.org:8080/11696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Gerrit-Change-Number: 11696
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 20:39:57 +0000
Gerrit-HasComments: Yes