You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by ma...@apache.org on 2017/02/15 13:18:34 UTC

ambari git commit: AMBARI-19984. ambari-server upgrade is not idempotent (Attila Magyar via magyari_sandor)

Repository: ambari
Updated Branches:
  refs/heads/branch-2.5 537c95471 -> 29343d775


AMBARI-19984. ambari-server upgrade is not idempotent (Attila Magyar via magyari_sandor)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/29343d77
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/29343d77
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/29343d77

Branch: refs/heads/branch-2.5
Commit: 29343d775b7ef983b67c3c0516190ae33cb02669
Parents: 537c954
Author: Attila Magyar <am...@hortonworks.com>
Authored: Wed Feb 15 14:13:40 2017 +0100
Committer: Sandor Magyari <sm...@hortonworks.com>
Committed: Wed Feb 15 14:17:34 2017 +0100

----------------------------------------------------------------------
 .../server/orm/entities/PermissionEntity.java   | 32 ++++++--
 .../internal/InternalAuthenticationToken.java   | 24 +-----
 .../server/upgrade/AbstractUpgradeCatalog.java  |  2 +-
 .../security/TestAuthenticationFactory.java     | 44 +++-------
 .../authorization/AuthorizationHelperTest.java  | 24 +++---
 .../server/upgrade/UpgradeCatalog242Test.java   | 27 +++----
 .../server/upgrade/UpgradeCatalog250Test.java   | 85 +++++++++++++-------
 7 files changed, 115 insertions(+), 123 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/29343d77/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java
index b6f1557..892bb13 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/orm/entities/PermissionEntity.java
@@ -18,6 +18,9 @@
 
 package org.apache.ambari.server.orm.entities;
 
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.Set;
 
 import javax.persistence.Column;
 import javax.persistence.Entity;
@@ -34,7 +37,8 @@ import javax.persistence.NamedQuery;
 import javax.persistence.OneToOne;
 import javax.persistence.Table;
 import javax.persistence.TableGenerator;
-import java.util.Collection;
+
+import org.apache.ambari.server.security.authorization.RoleAuthorization;
 
 /**
  * Represents an admin permission.
@@ -119,7 +123,7 @@ public class PermissionEntity {
       joinColumns = {@JoinColumn(name = "permission_id")},
       inverseJoinColumns = {@JoinColumn(name = "authorization_id")}
   )
-  private Collection<RoleAuthorizationEntity> authorizations;
+  private Set<RoleAuthorizationEntity> authorizations = new LinkedHashSet<>();
 
   /**
    * The permission's explicit sort order
@@ -229,12 +233,26 @@ public class PermissionEntity {
   }
 
   /**
-   * Sets the collection of granular authorizations for this PermissionEntity
-   *
-   * @param authorizations a collection of granular authorizations
+   * Add roleAuthorization if it's not already added
+   */
+  public void addAuthorization(RoleAuthorizationEntity roleAuthorization) {
+    authorizations.add(roleAuthorization);
+  }
+
+  /**
+   * Add multiple role authorizations
    */
-  public void setAuthorizations(Collection<RoleAuthorizationEntity> authorizations) {
-    this.authorizations = authorizations;
+  public void addAuthorizations(Collection<RoleAuthorization> roleAuthorizations) {
+    for (RoleAuthorization roleAuthorization : roleAuthorizations) {
+      addAuthorization(createRoleAuthorizationEntity(roleAuthorization));
+    }
+  }
+
+  private static RoleAuthorizationEntity createRoleAuthorizationEntity(RoleAuthorization authorization) {
+    RoleAuthorizationEntity roleAuthorizationEntity = new RoleAuthorizationEntity();
+    roleAuthorizationEntity.setAuthorizationId(authorization.getId());
+    roleAuthorizationEntity.setAuthorizationName(authorization.name());
+    return roleAuthorizationEntity;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/ambari/blob/29343d77/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/InternalAuthenticationToken.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/InternalAuthenticationToken.java b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/InternalAuthenticationToken.java
index c83a132..81233e3 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/InternalAuthenticationToken.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/internal/InternalAuthenticationToken.java
@@ -18,16 +18,14 @@
 
 package org.apache.ambari.server.security.authorization.internal;
 
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.List;
+import java.util.EnumSet;
 
 import org.apache.ambari.server.orm.entities.PermissionEntity;
 import org.apache.ambari.server.orm.entities.PrivilegeEntity;
 import org.apache.ambari.server.orm.entities.ResourceEntity;
 import org.apache.ambari.server.orm.entities.ResourceTypeEntity;
-import org.apache.ambari.server.orm.entities.RoleAuthorizationEntity;
 import org.apache.ambari.server.security.authorization.ResourceType;
 import org.apache.ambari.server.security.authorization.RoleAuthorization;
 import org.apache.ambari.server.security.authorization.AmbariGrantedAuthority;
@@ -60,7 +58,7 @@ public class InternalAuthenticationToken implements Authentication {
     PermissionEntity pe = new PermissionEntity();
     pe.setId(PermissionEntity.AMBARI_ADMINISTRATOR_PERMISSION);
     pe.setPermissionName(PermissionEntity.AMBARI_ADMINISTRATOR_PERMISSION_NAME);
-    pe.setAuthorizations(createAdminAuthorizations());
+    pe.addAuthorizations(EnumSet.allOf(RoleAuthorization.class));
     entity.setPermission(pe);
     
     ResourceEntity resource = new ResourceEntity();
@@ -73,24 +71,6 @@ public class InternalAuthenticationToken implements Authentication {
     entity.setResource(resource);
   }
 
-  /**
-   * Creates the collection of RoleAuthorizationEntity objects that an administrative user would have.
-   *
-   * @return a collection of RoleAuthorizationEntity objects
-   */
-  private static Collection<RoleAuthorizationEntity> createAdminAuthorizations() {
-    List<RoleAuthorizationEntity> authorizations = new ArrayList<RoleAuthorizationEntity>();
-
-    for (RoleAuthorization roleAuthorization : RoleAuthorization.values()) {
-      RoleAuthorizationEntity re = new RoleAuthorizationEntity();
-      re.setAuthorizationId(roleAuthorization.getId());
-      re.setAuthorizationName(roleAuthorization.name());
-      authorizations.add(re);
-    }
-
-    return authorizations;
-  }
-
   public InternalAuthenticationToken(String tokenString) {
     this.token = tokenString;
   }

http://git-wip-us.apache.org/repos/asf/ambari/blob/29343d77/ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java
index fe75e5d..9be541f 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/upgrade/AbstractUpgradeCatalog.java
@@ -883,7 +883,7 @@ public abstract class AbstractUpgradeCatalog implements UpgradeCatalog {
 
       PermissionEntity role = permissionDAO.findPermissionByNameAndType(roleName, resourceTypeDAO.findByName(resourceType));
       if (role != null) {
-        role.getAuthorizations().add(roleAuthorization);
+        role.addAuthorization(roleAuthorization);
         permissionDAO.merge(role);
       }
     }

http://git-wip-us.apache.org/repos/asf/ambari/blob/29343d77/ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java b/ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java
index 3038e7a..0ee7106 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java
@@ -22,18 +22,15 @@ import org.apache.ambari.server.orm.entities.PermissionEntity;
 import org.apache.ambari.server.orm.entities.PrivilegeEntity;
 import org.apache.ambari.server.orm.entities.ResourceEntity;
 import org.apache.ambari.server.orm.entities.ResourceTypeEntity;
-import org.apache.ambari.server.orm.entities.RoleAuthorizationEntity;
 import org.apache.ambari.server.security.authorization.AmbariGrantedAuthority;
 import org.apache.ambari.server.security.authorization.ResourceType;
 import org.apache.ambari.server.security.authorization.RoleAuthorization;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
 
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
-import java.util.Set;
 
 public class TestAuthenticationFactory {
   public static Authentication createAdministrator() {
@@ -173,7 +170,7 @@ public class TestAuthenticationFactory {
     PermissionEntity permissionEntity = new PermissionEntity();
     permissionEntity.setId(PermissionEntity.AMBARI_ADMINISTRATOR_PERMISSION);
     permissionEntity.setResourceType(createResourceTypeEntity(ResourceType.AMBARI));
-    permissionEntity.setAuthorizations(createAuthorizations(EnumSet.allOf(RoleAuthorization.class)));
+    permissionEntity.addAuthorizations(EnumSet.allOf(RoleAuthorization.class));
     return permissionEntity;
   }
 
@@ -181,7 +178,7 @@ public class TestAuthenticationFactory {
     PermissionEntity permissionEntity = new PermissionEntity();
     permissionEntity.setId(PermissionEntity.CLUSTER_ADMINISTRATOR_PERMISSION);
     permissionEntity.setResourceType(createResourceTypeEntity(ResourceType.CLUSTER));
-    permissionEntity.setAuthorizations(createAuthorizations(EnumSet.of(
+    permissionEntity.addAuthorizations(EnumSet.of(
         RoleAuthorization.CLUSTER_MANAGE_CREDENTIALS,
         RoleAuthorization.CLUSTER_MODIFY_CONFIGS,
         RoleAuthorization.CLUSTER_MANAGE_CONFIG_GROUPS,
@@ -220,7 +217,7 @@ public class TestAuthenticationFactory {
         RoleAuthorization.CLUSTER_RUN_CUSTOM_COMMAND,
         RoleAuthorization.SERVICE_MANAGE_AUTO_START,
         RoleAuthorization.CLUSTER_MANAGE_AUTO_START,
-        RoleAuthorization.CLUSTER_MANAGE_USER_PERSISTED_DATA)));
+        RoleAuthorization.CLUSTER_MANAGE_USER_PERSISTED_DATA));
     return permissionEntity;
   }
 
@@ -228,7 +225,7 @@ public class TestAuthenticationFactory {
     PermissionEntity permissionEntity = new PermissionEntity();
     permissionEntity.setId(5);
     permissionEntity.setResourceType(createResourceTypeEntity(ResourceType.CLUSTER));
-    permissionEntity.setAuthorizations(createAuthorizations(EnumSet.of(
+    permissionEntity.addAuthorizations(EnumSet.of(
         RoleAuthorization.HOST_VIEW_CONFIGS,
         RoleAuthorization.HOST_ADD_DELETE_COMPONENTS,
         RoleAuthorization.HOST_VIEW_METRICS,
@@ -261,7 +258,7 @@ public class TestAuthenticationFactory {
         RoleAuthorization.SERVICE_VIEW_OPERATIONAL_LOGS,
         RoleAuthorization.SERVICE_MANAGE_AUTO_START,
         RoleAuthorization.CLUSTER_MANAGE_AUTO_START,
-        RoleAuthorization.CLUSTER_MANAGE_CREDENTIALS)));
+        RoleAuthorization.CLUSTER_MANAGE_CREDENTIALS));
     return permissionEntity;
   }
 
@@ -269,7 +266,7 @@ public class TestAuthenticationFactory {
     PermissionEntity permissionEntity = new PermissionEntity();
     permissionEntity.setId(5);
     permissionEntity.setResourceType(createResourceTypeEntity(ResourceType.CLUSTER));
-    permissionEntity.setAuthorizations(createAuthorizations(EnumSet.of(
+    permissionEntity.addAuthorizations(EnumSet.of(
         RoleAuthorization.CLUSTER_VIEW_ALERTS,
         RoleAuthorization.CLUSTER_VIEW_CONFIGS,
         RoleAuthorization.CLUSTER_VIEW_METRICS,
@@ -296,7 +293,7 @@ public class TestAuthenticationFactory {
         RoleAuthorization.SERVICE_VIEW_STATUS_INFO,
         RoleAuthorization.SERVICE_VIEW_OPERATIONAL_LOGS,
         RoleAuthorization.SERVICE_MANAGE_AUTO_START,
-        RoleAuthorization.CLUSTER_MANAGE_USER_PERSISTED_DATA)));
+        RoleAuthorization.CLUSTER_MANAGE_USER_PERSISTED_DATA));
     return permissionEntity;
   }
 
@@ -304,7 +301,7 @@ public class TestAuthenticationFactory {
     PermissionEntity permissionEntity = new PermissionEntity();
     permissionEntity.setId(6);
     permissionEntity.setResourceType(createResourceTypeEntity(ResourceType.CLUSTER));
-    permissionEntity.setAuthorizations(createAuthorizations(EnumSet.of(
+    permissionEntity.addAuthorizations(EnumSet.of(
         RoleAuthorization.SERVICE_VIEW_CONFIGS,
         RoleAuthorization.SERVICE_VIEW_METRICS,
         RoleAuthorization.SERVICE_VIEW_STATUS_INFO,
@@ -322,7 +319,7 @@ public class TestAuthenticationFactory {
         RoleAuthorization.CLUSTER_VIEW_STACK_DETAILS,
         RoleAuthorization.CLUSTER_VIEW_STATUS_INFO,
         RoleAuthorization.CLUSTER_MANAGE_USER_PERSISTED_DATA
-    )));
+    ));
     return permissionEntity;
   }
 
@@ -330,7 +327,7 @@ public class TestAuthenticationFactory {
     PermissionEntity permissionEntity = new PermissionEntity();
     permissionEntity.setId(PermissionEntity.CLUSTER_USER_PERMISSION);
     permissionEntity.setResourceType(createResourceTypeEntity(ResourceType.CLUSTER));
-    permissionEntity.setAuthorizations(createAuthorizations(EnumSet.of(
+    permissionEntity.addAuthorizations(EnumSet.of(
         RoleAuthorization.SERVICE_VIEW_CONFIGS,
         RoleAuthorization.SERVICE_VIEW_METRICS,
         RoleAuthorization.SERVICE_VIEW_STATUS_INFO,
@@ -344,7 +341,7 @@ public class TestAuthenticationFactory {
         RoleAuthorization.CLUSTER_VIEW_STACK_DETAILS,
         RoleAuthorization.CLUSTER_VIEW_STATUS_INFO,
         RoleAuthorization.CLUSTER_MANAGE_USER_PERSISTED_DATA
-    )));
+    ));
     return permissionEntity;
   }
 
@@ -352,9 +349,7 @@ public class TestAuthenticationFactory {
     PermissionEntity permissionEntity = new PermissionEntity();
     permissionEntity.setId(PermissionEntity.VIEW_USER_PERMISSION);
     permissionEntity.setResourceType(createResourceTypeEntity(ResourceType.CLUSTER));
-    permissionEntity.setAuthorizations(createAuthorizations(EnumSet.of(
-      RoleAuthorization.VIEW_USE
-    )));
+    permissionEntity.addAuthorizations(EnumSet.of(RoleAuthorization.VIEW_USE));
     return permissionEntity;
   }
 
@@ -396,21 +391,6 @@ public class TestAuthenticationFactory {
     return resourceTypeEntity;
   }
 
-  private static RoleAuthorizationEntity createRoleAuthorizationEntity(RoleAuthorization authorization) {
-    RoleAuthorizationEntity roleAuthorizationEntity = new RoleAuthorizationEntity();
-    roleAuthorizationEntity.setAuthorizationId(authorization.getId());
-    roleAuthorizationEntity.setAuthorizationName(authorization.name());
-    return roleAuthorizationEntity;
-  }
-
-  private static Collection<RoleAuthorizationEntity> createAuthorizations(Set<RoleAuthorization> roleAuthorizations) {
-    Collection<RoleAuthorizationEntity> roleAuthorizationEntities = new ArrayList<RoleAuthorizationEntity>();
-    for (RoleAuthorization roleAuthorization : roleAuthorizations) {
-      roleAuthorizationEntities.add(createRoleAuthorizationEntity(roleAuthorization));
-    }
-    return roleAuthorizationEntities;
-  }
-
   private static class TestAuthorization implements Authentication {
     private final String name;
     private final Collection<? extends GrantedAuthority> authorities;

http://git-wip-us.apache.org/repos/asf/ambari/blob/29343d77/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AuthorizationHelperTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AuthorizationHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AuthorizationHelperTest.java
index d376d4b..a2a4a83 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AuthorizationHelperTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AuthorizationHelperTest.java
@@ -262,16 +262,16 @@ public class AuthorizationHelperTest  extends EasyMockSupport {
     cluster2ResourceEntity.setId(2L);
 
     PermissionEntity readOnlyPermissionEntity = new PermissionEntity();
-    readOnlyPermissionEntity.setAuthorizations(Collections.singleton(readOnlyRoleAuthorizationEntity));
+    readOnlyPermissionEntity.addAuthorization(readOnlyRoleAuthorizationEntity);
 
     PermissionEntity privilegedPermissionEntity = new PermissionEntity();
-    privilegedPermissionEntity.setAuthorizations(Arrays.asList(readOnlyRoleAuthorizationEntity,
-        privilegedRoleAuthorizationEntity));
+    privilegedPermissionEntity.addAuthorization(readOnlyRoleAuthorizationEntity);
+    privilegedPermissionEntity.addAuthorization(privilegedRoleAuthorizationEntity);
 
     PermissionEntity administratorPermissionEntity = new PermissionEntity();
-    administratorPermissionEntity.setAuthorizations(Arrays.asList(readOnlyRoleAuthorizationEntity,
-        privilegedRoleAuthorizationEntity,
-        administratorRoleAuthorizationEntity));
+    administratorPermissionEntity.addAuthorization(readOnlyRoleAuthorizationEntity);
+    administratorPermissionEntity.addAuthorization(privilegedRoleAuthorizationEntity);
+    administratorPermissionEntity.addAuthorization(administratorRoleAuthorizationEntity);
 
     PrivilegeEntity readOnlyPrivilegeEntity = new PrivilegeEntity();
     readOnlyPrivilegeEntity.setPermission(readOnlyPermissionEntity);
@@ -397,16 +397,16 @@ public class AuthorizationHelperTest  extends EasyMockSupport {
     viewResourceEntity.setId(53L);
 
     PermissionEntity readOnlyPermissionEntity = new PermissionEntity();
-    readOnlyPermissionEntity.setAuthorizations(Collections.singleton(readOnlyRoleAuthorizationEntity));
+    readOnlyPermissionEntity.addAuthorization(readOnlyRoleAuthorizationEntity);
 
     PermissionEntity viewUsePermissionEntity = new PermissionEntity();
-    viewUsePermissionEntity.setAuthorizations(Arrays.asList(readOnlyRoleAuthorizationEntity,
-        viewUseRoleAuthorizationEntity));
+    viewUsePermissionEntity.addAuthorization(readOnlyRoleAuthorizationEntity);
+    viewUsePermissionEntity.addAuthorization(viewUseRoleAuthorizationEntity);
 
     PermissionEntity administratorPermissionEntity = new PermissionEntity();
-    administratorPermissionEntity.setAuthorizations(Arrays.asList(readOnlyRoleAuthorizationEntity,
-        viewUseRoleAuthorizationEntity,
-        administratorRoleAuthorizationEntity));
+    administratorPermissionEntity.addAuthorization(readOnlyRoleAuthorizationEntity);
+    administratorPermissionEntity.addAuthorization(viewUseRoleAuthorizationEntity);
+    administratorPermissionEntity.addAuthorization(administratorRoleAuthorizationEntity);
 
     PrivilegeEntity readOnlyPrivilegeEntity = new PrivilegeEntity();
     readOnlyPrivilegeEntity.setPermission(readOnlyPermissionEntity);

http://git-wip-us.apache.org/repos/asf/ambari/blob/29343d77/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java
index 823455e..e0b24bd 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog242Test.java
@@ -37,7 +37,6 @@ import static org.easymock.EasyMock.verify;
 import java.lang.reflect.Method;
 import java.sql.SQLException;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -386,18 +385,10 @@ public class UpgradeCatalog242Test {
 
     ResourceTypeEntity clusterResourceTypeEntity = easyMockSupport.createMock(ResourceTypeEntity.class);
 
-    Collection<RoleAuthorizationEntity> ambariAdministratorAuthorizations = new ArrayList<RoleAuthorizationEntity>();
-    Collection<RoleAuthorizationEntity> clusterAdministratorAuthorizations = new ArrayList<RoleAuthorizationEntity>();
-
-    PermissionEntity clusterAdministratorPermissionEntity = easyMockSupport.createMock(PermissionEntity.class);
-    expect(clusterAdministratorPermissionEntity.getAuthorizations())
-        .andReturn(clusterAdministratorAuthorizations)
-        .times(1);
-
-    PermissionEntity ambariAdministratorPermissionEntity = easyMockSupport.createMock(PermissionEntity.class);
-    expect(ambariAdministratorPermissionEntity.getAuthorizations())
-        .andReturn(ambariAdministratorAuthorizations)
-        .times(2);
+    PermissionEntity clusterAdministratorPermissionEntity = new PermissionEntity();
+    clusterAdministratorPermissionEntity.setId(1);
+    PermissionEntity ambariAdministratorPermissionEntity = new PermissionEntity();
+    ambariAdministratorPermissionEntity.setId(2);
 
     PermissionDAO permissionDAO = easyMockSupport.createMock(PermissionDAO.class);
     expect(permissionDAO.findPermissionByNameAndType("AMBARI.ADMINISTRATOR", ambariResourceTypeEntity))
@@ -447,11 +438,11 @@ public class UpgradeCatalog242Test {
     Assert.assertEquals("CLUSTER.RUN_CUSTOM_COMMAND", clusterRunCustomCommandEntity.getAuthorizationId());
     Assert.assertEquals("Perform custom cluster-level actions", clusterRunCustomCommandEntity.getAuthorizationName());
 
-    Assert.assertEquals(2, ambariAdministratorAuthorizations.size());
-    Assert.assertTrue(ambariAdministratorAuthorizations.contains(clusterRunCustomCommandEntity));
-    Assert.assertTrue(ambariAdministratorAuthorizations.contains(ambariRunCustomCommandEntity));
+    Assert.assertEquals(2, ambariAdministratorPermissionEntity.getAuthorizations().size());
+    Assert.assertTrue(ambariAdministratorPermissionEntity.getAuthorizations().contains(clusterRunCustomCommandEntity));
+    Assert.assertTrue(ambariAdministratorPermissionEntity.getAuthorizations().contains(ambariRunCustomCommandEntity));
 
-    Assert.assertEquals(1, clusterAdministratorAuthorizations.size());
-    Assert.assertTrue(clusterAdministratorAuthorizations.contains(clusterRunCustomCommandEntity));
+    Assert.assertEquals(1, clusterAdministratorPermissionEntity.getAuthorizations().size());
+    Assert.assertTrue(clusterAdministratorPermissionEntity.getAuthorizations().contains(clusterRunCustomCommandEntity));
   }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/29343d77/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java
index 4ac6fa6..112f212 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog250Test.java
@@ -68,8 +68,6 @@ import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
-import java.util.ArrayList;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -1579,40 +1577,28 @@ public class UpgradeCatalog250Test {
 
     ResourceTypeEntity clusterResourceTypeEntity = easyMockSupport.createMock(ResourceTypeEntity.class);
 
-    Collection<RoleAuthorizationEntity> ambariAdministratorAuthorizations = new ArrayList<RoleAuthorizationEntity>();
-    Collection<RoleAuthorizationEntity> clusterAdministratorAuthorizations = new ArrayList<RoleAuthorizationEntity>();
-
-    PermissionEntity clusterAdministratorPermissionEntity = easyMockSupport.createMock(PermissionEntity.class);
-    expect(clusterAdministratorPermissionEntity.getAuthorizations())
-        .andReturn(clusterAdministratorAuthorizations)
-        .anyTimes();
-
-    PermissionEntity ambariAdministratorPermissionEntity = easyMockSupport.createMock(PermissionEntity.class);
-    expect(ambariAdministratorPermissionEntity.getAuthorizations())
-        .andReturn(ambariAdministratorAuthorizations)
-        .anyTimes();
+    PermissionEntity clusterAdministratorPermissionEntity = new PermissionEntity();
+    clusterAdministratorPermissionEntity.setId(1);
+    PermissionEntity ambariAdministratorPermissionEntity = new PermissionEntity();
+    ambariAdministratorPermissionEntity.setId(2);
 
     PermissionDAO permissionDAO = easyMockSupport.createMock(PermissionDAO.class);
     expect(permissionDAO.findPermissionByNameAndType("AMBARI.ADMINISTRATOR", ambariResourceTypeEntity))
-        .andReturn(ambariAdministratorPermissionEntity)
-        .anyTimes();
+      .andReturn(ambariAdministratorPermissionEntity).atLeastOnce();
     expect(permissionDAO.findPermissionByNameAndType("CLUSTER.ADMINISTRATOR", clusterResourceTypeEntity))
-        .andReturn(clusterAdministratorPermissionEntity)
-        .anyTimes();
+      .andReturn(clusterAdministratorPermissionEntity).atLeastOnce();
     expect(permissionDAO.merge(ambariAdministratorPermissionEntity))
-        .andReturn(ambariAdministratorPermissionEntity)
-        .anyTimes();
+      .andReturn(ambariAdministratorPermissionEntity).atLeastOnce();
     expect(permissionDAO.merge(clusterAdministratorPermissionEntity))
-        .andReturn(clusterAdministratorPermissionEntity)
-        .anyTimes();
+      .andReturn(clusterAdministratorPermissionEntity).atLeastOnce();
 
     ResourceTypeDAO resourceTypeDAO = easyMockSupport.createMock(ResourceTypeDAO.class);
-    expect(resourceTypeDAO.findByName("AMBARI")).andReturn(ambariResourceTypeEntity).anyTimes();
-    expect(resourceTypeDAO.findByName("CLUSTER")).andReturn(clusterResourceTypeEntity).anyTimes();
+    expect(resourceTypeDAO.findByName("AMBARI")).andReturn(ambariResourceTypeEntity).atLeastOnce();
+    expect(resourceTypeDAO.findByName("CLUSTER")).andReturn(clusterResourceTypeEntity).atLeastOnce();
 
     RoleAuthorizationDAO roleAuthorizationDAO = easyMockSupport.createMock(RoleAuthorizationDAO.class);
-    expect(roleAuthorizationDAO.findById("CLUSTER.RUN_CUSTOM_COMMAND")).andReturn(null).anyTimes();
-    expect(roleAuthorizationDAO.findById("AMBARI.RUN_CUSTOM_COMMAND")).andReturn(null).anyTimes();
+    expect(roleAuthorizationDAO.findById("CLUSTER.RUN_CUSTOM_COMMAND")).andReturn(null).atLeastOnce();
+    expect(roleAuthorizationDAO.findById("AMBARI.RUN_CUSTOM_COMMAND")).andReturn(null).atLeastOnce();
 
     Capture<RoleAuthorizationEntity> captureClusterRunCustomCommandEntity = newCapture();
     roleAuthorizationDAO.create(capture(captureClusterRunCustomCommandEntity));
@@ -1640,12 +1626,49 @@ public class UpgradeCatalog250Test {
     Assert.assertEquals("CLUSTER.RUN_CUSTOM_COMMAND", clusterRunCustomCommandEntity.getAuthorizationId());
     Assert.assertEquals("Perform custom cluster-level actions", clusterRunCustomCommandEntity.getAuthorizationName());
 
-    Assert.assertEquals(2, ambariAdministratorAuthorizations.size());
-    Assert.assertTrue(ambariAdministratorAuthorizations.contains(clusterRunCustomCommandEntity));
-    Assert.assertTrue(ambariAdministratorAuthorizations.contains(ambariRunCustomCommandEntity));
+    Assert.assertEquals(2, ambariAdministratorPermissionEntity.getAuthorizations().size());
+    Assert.assertTrue(ambariAdministratorPermissionEntity.getAuthorizations().contains(clusterRunCustomCommandEntity));
+    Assert.assertTrue(ambariAdministratorPermissionEntity.getAuthorizations().contains(ambariRunCustomCommandEntity));
+
+    Assert.assertEquals(1, clusterAdministratorPermissionEntity.getAuthorizations().size());
+    Assert.assertTrue(clusterAdministratorPermissionEntity.getAuthorizations().contains(clusterRunCustomCommandEntity));
+  }
+
+
+  @Test
+  public void testAddingRoleAuthorizationIsIdempotent() throws Exception {
+    EasyMockSupport easyMockSupport = new EasyMockSupport();
+    ResourceTypeEntity ambariResourceTypeEntity = new ResourceTypeEntity();
+    PermissionEntity ambariAdministratorPermissionEntity = new PermissionEntity();
+
+    final PermissionDAO permissionDAO = easyMockSupport.createNiceMock(PermissionDAO.class);
+    expect(permissionDAO.findPermissionByNameAndType("AMBARI.ADMINISTRATOR", ambariResourceTypeEntity))
+      .andReturn(ambariAdministratorPermissionEntity)
+      .anyTimes();
+
+    final ResourceTypeDAO resourceTypeDAO = easyMockSupport.createNiceMock(ResourceTypeDAO.class);
+    expect(resourceTypeDAO.findByName("AMBARI")).andReturn(ambariResourceTypeEntity).anyTimes();
+
+    final RoleAuthorizationDAO roleAuthorizationDAO = easyMockSupport.createNiceMock(RoleAuthorizationDAO.class);
+    expect(roleAuthorizationDAO.findById("CLUSTER.RUN_CUSTOM_COMMAND")).andReturn(null).anyTimes();
+
+    Capture<RoleAuthorizationEntity> captureAmbariRunCustomCommandEntity = newCapture();
+    roleAuthorizationDAO.create(capture(captureAmbariRunCustomCommandEntity));
+    expectLastCall().times(2);
+
+    Injector injector = easyMockSupport.createNiceMock(Injector.class);
+    expect(injector.getInstance(RoleAuthorizationDAO.class)).andReturn(roleAuthorizationDAO).atLeastOnce();
+    expect(injector.getInstance(PermissionDAO.class)).andReturn(permissionDAO).atLeastOnce();
+    expect(injector.getInstance(ResourceTypeDAO.class)).andReturn(resourceTypeDAO).atLeastOnce();
+
+    easyMockSupport.replayAll();
+
+    new UpgradeCatalog242(injector).createRoleAuthorizations();
+    new UpgradeCatalog242(injector).createRoleAuthorizations();
+    easyMockSupport.verifyAll();
+
+    Assert.assertEquals(2, ambariAdministratorPermissionEntity.getAuthorizations().size());
 
-    Assert.assertEquals(1, clusterAdministratorAuthorizations.size());
-    Assert.assertTrue(clusterAdministratorAuthorizations.contains(clusterRunCustomCommandEntity));
   }
 
   @Test