ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Hurley" <jhur...@hortonworks.com>
Subject Re: Review Request 41358: Enforce granular role-based access control for alert functions
Date Tue, 15 Dec 2015 21:48:52 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41358/#review110565
-----------------------------------------------------------



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java
(line 174)
<https://reviews.apache.org/r/41358/#comment170512>

    Consider that you can have 100,000 historical alerts in the system. Do you really want
to iterate over all of them every time? That's a lot of entity checking. Meanwhile, alert
history is read-only, so you should only have to check this once outside of the loop, no?



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java
(line 164)
<https://reviews.apache.org/r/41358/#comment170511>

    Consider that you can have 10,000 current alerts in the system. Do you really want to
iterate over all of them every time? That's a lot of entity checking. Meanwhile, alert notices
are read-only, so you should only have to check this once outside of the loop, no?



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java
(lines 131 - 134)
<https://reviews.apache.org/r/41358/#comment170509>

    Alert targets are actually not bound to a cluster; they are defined on the api/v1/alert_target
endpoint. As a result, should this be a different permission?



ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java
(line 75)
<https://reviews.apache.org/r/41358/#comment170510>

    Alert targets are their own endpoint, not scoped under a cluster.


- Jonathan Hurley


On Dec. 14, 2015, 1:51 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41358/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2015, 1:51 p.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sid
Wagle.
> 
> 
> Bugs: AMBARI-14141
>     https://issues.apache.org/jira/browse/AMBARI-14141
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for alert functions:
> 
>                                 | Cluster User | Service Operator | Service Administrator
| Cluster Operator | Cluster Administrator | Administrator 
> --------------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
                               
> View alerts (service)           | (+)          | (+)              | (+)             
     | (+)              | (+)                   | (+)
> Enable/disable alerts (service) |              |                  | (+)             
     | (+)              | (+)                   | (+)
> View alerts (cluster)           | (+)          | (+)              | (+)             
     | (+)              | (+)                   | (+)
> Enable/disable alerts (cluster) |              |                  |                 
     |                  | (+)                   | (+)
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java
d47d8d3 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java
bc5f956 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java
215bc8e 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java
89ee69a 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java
8f0e526 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java
4dc4dcf 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProviderUtils.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java
a310259 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java
d817ad7 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/RoleAuthorization.java
02eb5b4 
>   ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog230.java
ee2b9b1 
>   ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 788c2a7 
>   ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql ae560d9 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 155a6a7 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql 4c20767 
>   ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql dc08960 
>   ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 10b1ac6 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProviderTest.java
e589537 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProviderTest.java
a41eecf 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProviderTest.java
99aca45 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProviderTest.java
3322da6 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertResourceProviderTest.java
4f0263b 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java
6cde0c2 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java
1c440eb 
> 
> Diff: https://reviews.apache.org/r/41358/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results: 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 1:00:59.413s
> [INFO] Finished at: Mon Dec 14 13:37:40 EST 2015
> [INFO] Final Memory: 70M/1085M
> [INFO] ------------------------------------------------------------------------
> 
> # Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message