> On Oct. 20, 2016, 12:44 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PrivilegeResourceProvider.java, > > line 337 > > <https://reviews.apache.org/r/53060/diff/1/?file=1542253#file1542253line337> > > > > containsKey() could have a null bound to it, right? Should this just > > try to retrieve the entity and null check it, or are you confident that the > > key won't be bound to null.
This seems to be the pattern for similar tasks in the method... but I think I like your accessment of this and protect against `null`. Thanks. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53060/#review153412 ----------------------------------------------------------- On Oct. 20, 2016, 10:20 a.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53060/ > ----------------------------------------------------------- > > (Updated Oct. 20, 2016, 10:20 a.m.) > > > Review request for Ambari, Aleksandr Kovalenko, DIPAYAN BHOWMICK, Jonathan > Hurley, Nate Cole, and Sebastian Toader. > > > Bugs: AMBARI-18635 > https://issues.apache.org/jira/browse/AMBARI-18635 > > > Repository: ambari > > > Description > ------- > > Authorizations given to roles, should use generic role-based principals > rather than hard-coded resource types. > > Access to views can be assigned to all users with a given role. The > implementation for this lead to the creation of hard-coded principals that > represent the current set of roles. This is not dynamic enough for possibly > future enhancements where new roles may be created by administrators. > > This needs to be changed such that rather that using the hard-coded > pseudo-role-principals, the dynamically generated role-principals are to be > used. > > The hard-coded pseudo-role-principals have the following `adminprincipaltype` > values as opposed to "ROLE": > > * ALL.CLUSTER.ADMINISTRATOR > * ALL.CLUSTER.OPERATOR > * ALL.SERVICE.ADMINISTRATOR > * ALL.SERVICE.OPERATOR > * ALL.CLUSTER.USER > > These should be removed along with the associated `adminprincipal` records. > > Also, the FE should be updated to set permissions using the dynamic > role-principals. > > Finally, code should be cleaned up to remove unneeded code in > > - > org.apache.ambari.server.security.authorization.ClusterInheritedPermissionHelper > - > org.apache.ambari.server.controller.internal.GroupPrivilegeResourceProvider#getResources > - > org.apache.ambari.server.controller.internal.PrivilegeResourceProvider#toEntity > - > org.apache.ambari.server.controller.internal.UserPrivilegeResourceProvider#getResources > - > org.apache.ambari.server.security.authorization.AuthorizationHelper#isAuthorized > - org.apache.ambari.server.view.ViewRegistry#addClusterInheritedPermissions > - ... > > > Diffs > ----- > > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/ambariViews/ViewsEditCtrl.js > bd74b16 > ambari-admin/src/main/resources/ui/admin-web/app/scripts/i18n.config.js > af22d7f > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/PermissionLoader.js > 988986b > > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/PermissionsSaver.js > c7b9295 > ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/View.js > 5bc0509 > > ambari-admin/src/main/resources/ui/admin-web/app/views/ambariViews/edit.html > 69eb1c1 > > ambari-admin/src/main/resources/ui/admin-web/test/unit/services/PermissionSaver_test.js > fa36d98 > > ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/ClusterPrivilegeChangeRequestAuditEvent.java > b28bb2a > > ambari-server/src/main/java/org/apache/ambari/server/audit/event/request/ViewPrivilegeChangeRequestAuditEvent.java > 11c558c > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/PrivilegeEventCreator.java > 5c476c6 > > ambari-server/src/main/java/org/apache/ambari/server/audit/request/eventcreator/ViewPrivilegeEventCreator.java > 56d35c0 > > ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java > 56e2398 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProvider.java > e5c95cb > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProvider.java > 8f37764 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/GroupPrivilegeResourceProvider.java > 94d1cad > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/PrivilegeResourceProvider.java > 34111df > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserPrivilegeResourceProvider.java > bdd73a6 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProvider.java > e5bd224 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PermissionDAO.java > 88d9775 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PrincipalDAO.java > efbdfab > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/PrincipalTypeDAO.java > 7823d56 > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java > f091bab > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PrincipalTypeEntity.java > 716d4f7 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AuthorizationHelper.java > 8639a2f > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/ClusterInheritedPermissionHelper.java > 9922bb2 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java > a4f0031 > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog242.java > a5276c2 > ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java > 156fd5b > > ambari-server/src/main/java/org/apache/ambari/server/view/configuration/AutoInstanceConfig.java > 11efc76 > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql 4b64955 > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 15a84cd > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 6c3c036 > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 570b684 > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql 170e430 > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 1501143 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AbstractPrivilegeResourceProviderTest.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AmbariPrivilegeResourceProviderTest.java > 99962ee > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterPrivilegeResourceProviderTest.java > f00a21a > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/GroupPrivilegeResourceProviderTest.java > c3510a8 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UserPrivilegeResourceProviderTest.java > 1f3cb52 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ViewPrivilegeResourceProviderTest.java > d85b37b > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AuthorizationHelperTest.java > 47211ef > > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java > 4457858 > > ambari-server/src/test/java/org/apache/ambari/server/view/configuration/AutoInstanceConfigTest.java > 3c4a440 > > Diff: https://reviews.apache.org/r/53060/diff/ > > > Testing > ------- > > Manually tested new cluster and upgraded cluster. > > # Local test results: > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 32:38.187s > [INFO] Finished at: Thu Oct 20 09:42:17 EDT 2016 > [INFO] Final Memory: 67M/993M > [INFO] > ------------------------------------------------------------------------ > > # Jenkins test results: PENDING > > > Thanks, > > Robert Levas > >
