AMBARI-10900. kerberos-env/kdc_type must not be mandatory if kerberos-env/manage_identities is false (rlevas)

ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java

@@ -1216,14 +1216,22 @@ public class KerberosHelper {
       throw new AmbariException(message);
     }
 
-    KDCType kdcType;
+    kerberosDetails.setSecurityType(cluster.getSecurityType());
+    kerberosDetails.setDefaultRealm(kerberosEnvProperties.get("realm"));
+
+    kerberosDetails.setKerberosEnvProperties(kerberosEnvProperties);
+
+    // If set, override the manage identities behavior
+    kerberosDetails.setManageIdentities(manageIdentities);
+
     String kdcTypeProperty = kerberosEnvProperties.get("kdc_type");
-    if (kdcTypeProperty == null) {
+    if ((kdcTypeProperty == null) && kerberosDetails.manageIdentities()) {
       String message = "The 'kerberos-env/kdc_type' value must be set to a valid KDC type";
       LOG.error(message);
       throw new KerberosInvalidConfigurationException(message);
     }
 
+    KDCType kdcType;
     try {
       kdcType = KDCType.translate(kdcTypeProperty);
     } catch (IllegalArgumentException e) {
@@ -1232,17 +1240,9 @@ public class KerberosHelper {
       throw new AmbariException(message);
     }
 
-    kerberosDetails.setSecurityType(cluster.getSecurityType());
-    kerberosDetails.setDefaultRealm(kerberosEnvProperties.get("realm"));
-
     // Set the KDCType to the the MIT_KDC as a fallback.
     kerberosDetails.setKdcType((kdcType == null) ? KDCType.MIT_KDC : kdcType);
 
-    kerberosDetails.setKerberosEnvProperties(kerberosEnvProperties);
-
-    // If set, override the manage identities behavior
-    kerberosDetails.setManageIdentities(manageIdentities);
-
     return kerberosDetails;
   }
 
@@ -2577,10 +2577,11 @@ public class KerberosHelper {
       commandParameters.put(KerberosServerAction.AUTHENTICATED_USER_NAME, ambariManagementController.getAuthName());
       commandParameters.put(KerberosServerAction.DATA_DIRECTORY, dataDirectory.getAbsolutePath());
       commandParameters.put(KerberosServerAction.DEFAULT_REALM, kerberosDetails.getDefaultRealm());
-      commandParameters.put(KerberosServerAction.KDC_TYPE, kerberosDetails.getKdcType().name());
-      commandParameters.put(KerberosServerAction.ADMINISTRATOR_CREDENTIAL, getEncryptedAdministratorCredentials(cluster));
 
       if (kerberosDetails.manageIdentities()) {
+        commandParameters.put(KerberosServerAction.KDC_TYPE, kerberosDetails.getKdcType().name());
+        commandParameters.put(KerberosServerAction.ADMINISTRATOR_CREDENTIAL, getEncryptedAdministratorCredentials(cluster));
+
         // *****************************************************************
         // Create stage to create principals
         addCreatePrincipalsStage(cluster, clusterHostInfoJson, hostParamsJson, event, commandParameters,
@@ -2669,8 +2670,6 @@ public class KerberosHelper {
       commandParameters.put(KerberosServerAction.AUTHENTICATED_USER_NAME, ambariManagementController.getAuthName());
       commandParameters.put(KerberosServerAction.DATA_DIRECTORY, dataDirectory.getAbsolutePath());
       commandParameters.put(KerberosServerAction.DEFAULT_REALM, kerberosDetails.getDefaultRealm());
-      commandParameters.put(KerberosServerAction.KDC_TYPE, kerberosDetails.getKdcType().name());
-      commandParameters.put(KerberosServerAction.ADMINISTRATOR_CREDENTIAL, getEncryptedAdministratorCredentials(cluster));
 
       // If there are configurations to set, create a (temporary) data file to store the configuration
       // updates and fill it will the relevant configurations.
@@ -2787,6 +2786,9 @@ public class KerberosHelper {
           roleCommandOrder, requestStageContainer);
 
       if(kerberosDetails.manageIdentities()) {
+        commandParameters.put(KerberosServerAction.KDC_TYPE, kerberosDetails.getKdcType().name());
+        commandParameters.put(KerberosServerAction.ADMINISTRATOR_CREDENTIAL, getEncryptedAdministratorCredentials(cluster));
+
         // *****************************************************************
         // Create stage to remove principals
         addDestroyPrincipalsStage(cluster, clusterHostInfoJson, hostParamsJson, event, commandParameters,
@@ -2883,23 +2885,26 @@ public class KerberosHelper {
         commandParameters.put(KerberosServerAction.AUTHENTICATED_USER_NAME, ambariManagementController.getAuthName());
         commandParameters.put(KerberosServerAction.DATA_DIRECTORY, dataDirectory.getAbsolutePath());
         commandParameters.put(KerberosServerAction.DEFAULT_REALM, kerberosDetails.getDefaultRealm());
-        commandParameters.put(KerberosServerAction.KDC_TYPE, kerberosDetails.getKdcType().name());
-        commandParameters.put(KerberosServerAction.ADMINISTRATOR_CREDENTIAL, getEncryptedAdministratorCredentials(cluster));
-        commandParameters.put(KerberosServerAction.REGENERATE_ALL, (regenerateAllKeytabs) ? "true" : "false");
 
-        // *****************************************************************
-        // Create stage to create principals
-        addCreatePrincipalsStage(cluster, clusterHostInfoJson, hostParamsJson, event,
-            commandParameters, roleCommandOrder, requestStageContainer);
+        if (kerberosDetails.manageIdentities()) {
+          commandParameters.put(KerberosServerAction.KDC_TYPE, kerberosDetails.getKdcType().name());
+          commandParameters.put(KerberosServerAction.ADMINISTRATOR_CREDENTIAL, getEncryptedAdministratorCredentials(cluster));
+          commandParameters.put(KerberosServerAction.REGENERATE_ALL, (regenerateAllKeytabs) ? "true" : "false");
 
-        // *****************************************************************
-        // Create stage to generate keytabs
-        addCreateKeytabFilesStage(cluster, clusterHostInfoJson, hostParamsJson, event,
-            commandParameters, roleCommandOrder, requestStageContainer);
+          // *****************************************************************
+          // Create stage to create principals
+          addCreatePrincipalsStage(cluster, clusterHostInfoJson, hostParamsJson, event,
+              commandParameters, roleCommandOrder, requestStageContainer);
 
-        // Create stage to distribute keytabs
-        addDistributeKeytabFilesStage(cluster, serviceComponentHosts, clusterHostInfoJson,
-            hostParamsJson, commandParameters, roleCommandOrder, requestStageContainer, hostsWithValidKerberosClient);
+          // *****************************************************************
+          // Create stage to generate keytabs
+          addCreateKeytabFilesStage(cluster, clusterHostInfoJson, hostParamsJson, event,
+              commandParameters, roleCommandOrder, requestStageContainer);
+
+          // Create stage to distribute keytabs
+          addDistributeKeytabFilesStage(cluster, serviceComponentHosts, clusterHostInfoJson,
+              hostParamsJson, commandParameters, roleCommandOrder, requestStageContainer, hostsWithValidKerberosClient);
+        }
       }
 
       return requestStageContainer.getLastStageId();
@@ -2967,18 +2972,21 @@ public class KerberosHelper {
         commandParameters.put(KerberosServerAction.AUTHENTICATED_USER_NAME, ambariManagementController.getAuthName());
         commandParameters.put(KerberosServerAction.DATA_DIRECTORY, dataDirectory.getAbsolutePath());
         commandParameters.put(KerberosServerAction.DEFAULT_REALM, kerberosDetails.getDefaultRealm());
-        commandParameters.put(KerberosServerAction.KDC_TYPE, kerberosDetails.getKdcType().name());
-        commandParameters.put(KerberosServerAction.ADMINISTRATOR_CREDENTIAL, getEncryptedAdministratorCredentials(cluster));
 
-        // *****************************************************************
-        // Create stage to delete principals
-        addDestroyPrincipalsStage(cluster, clusterHostInfoJson, hostParamsJson, event,
-            commandParameters, roleCommandOrder, requestStageContainer);
+        if (kerberosDetails.manageIdentities()) {
+          commandParameters.put(KerberosServerAction.KDC_TYPE, kerberosDetails.getKdcType().name());
+          commandParameters.put(KerberosServerAction.ADMINISTRATOR_CREDENTIAL, getEncryptedAdministratorCredentials(cluster));
 
-        // *****************************************************************
-        // Create stage to delete keytabs
-        addDeleteKeytabFilesStage(cluster, serviceComponentHosts, clusterHostInfoJson,
-            hostParamsJson, commandParameters, roleCommandOrder, requestStageContainer, hostsWithValidKerberosClient);
+          // *****************************************************************
+          // Create stage to delete principals
+          addDestroyPrincipalsStage(cluster, clusterHostInfoJson, hostParamsJson, event,
+              commandParameters, roleCommandOrder, requestStageContainer);
+
+          // *****************************************************************
+          // Create stage to delete keytabs
+          addDeleteKeytabFilesStage(cluster, serviceComponentHosts, clusterHostInfoJson,
+              hostParamsJson, commandParameters, roleCommandOrder, requestStageContainer, hostsWithValidKerberosClient);
+        }
       }
 
       return requestStageContainer.getLastStageId();

ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KDCType.java

@@ -24,6 +24,11 @@ import com.sun.istack.NotNull;
  * Enumerates the supported KDC types
  */
 public enum KDCType {
+  /**
+   * Indicates not KDC type is relevant. This is expected when Ambari is not managing Kerberos identities.
+   */
+  NONE,
+
   /**
    * Indicates an MIT KDC (or similar)
    */
@@ -44,9 +49,12 @@ public enum KDCType {
    * @param value a String value to convert to a KDCType
    * @return A KDCType
    * @throws java.lang.IllegalArgumentException if this enum type has no constant with the specified name
-   * @throws java.lang.NullPointerException if the value to translate is null
    */
-  public static KDCType translate(@NotNull String value) {
-    return KDCType.valueOf(value.replace("-", "_").toUpperCase());
+  public static KDCType translate(String value) {
+    if((value == null) || value.isEmpty()) {
+      return NONE;
+    }else {
+      return KDCType.valueOf(value.replace("-", "_").toUpperCase());
+    }
   }
 }

ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java

@@ -60,6 +60,7 @@ import org.apache.ambari.server.serveraction.kerberos.KDCType;
 import org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileWriter;
 import org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileWriterFactory;
 import org.apache.ambari.server.serveraction.kerberos.KerberosCredential;
+import org.apache.ambari.server.serveraction.kerberos.KerberosInvalidConfigurationException;
 import org.apache.ambari.server.serveraction.kerberos.KerberosMissingAdminCredentialsException;
 import org.apache.ambari.server.serveraction.kerberos.KerberosOperationException;
 import org.apache.ambari.server.serveraction.kerberos.KerberosOperationHandler;
@@ -253,13 +254,28 @@ public class KerberosHelperTest extends EasyMockSupport {
 
   @Test
   public void testEnableKerberos() throws Exception {
-    testEnableKerberos(new KerberosCredential("principal", "password", "keytab"), true, false);
+    testEnableKerberos(new KerberosCredential("principal", "password", "keytab"), "mit-kdc", "true", true, false);
+  }
+
+  @Test
+  public void testEnableKerberos_ManageIdentitiesFalseKdcNone() throws Exception {
+    testEnableKerberos(new KerberosCredential("principal", "password", "keytab"), "none", "false", true, false);
+  }
+
+  @Test(expected = AmbariException.class)
+  public void testEnableKerberos_ManageIdentitiesTrueKdcNone() throws Exception {
+    testEnableKerberos(new KerberosCredential("principal", "password", "keytab"), "none", "true", true, false);
+  }
+
+  @Test(expected = KerberosInvalidConfigurationException.class)
+  public void testEnableKerberos_ManageIdentitiesTrueKdcNull() throws Exception {
+    testEnableKerberos(new KerberosCredential("principal", "password", "keytab"), null, "true", true, false);
   }
 
   @Test(expected = KerberosMissingAdminCredentialsException.class)
   public void testEnableKerberosMissingCredentials() throws Exception {
     try {
-      testEnableKerberos(null, true, false);
+      testEnableKerberos(null, "mit-kdc", "true", true, false);
     } catch (IllegalArgumentException e) {
       Assert.assertTrue(e.getMessage().startsWith("Missing KDC administrator credentials"));
       throw e;
@@ -269,7 +285,7 @@ public class KerberosHelperTest extends EasyMockSupport {
   @Test(expected = KerberosMissingAdminCredentialsException.class)
   public void testEnableKerberosInvalidCredentials() throws Exception {
     try {
-      testEnableKerberos(new KerberosCredential("invalid_principal", "password", "keytab"), true, false);
+      testEnableKerberos(new KerberosCredential("invalid_principal", "password", "keytab"), "mit-kdc", "true", true, false);
     } catch (IllegalArgumentException e) {
       Assert.assertTrue(e.getMessage().startsWith("Invalid KDC administrator credentials"));
       throw e;
@@ -278,12 +294,12 @@ public class KerberosHelperTest extends EasyMockSupport {
 
   @Test
   public void testEnableKerberos_GetKerberosDescriptorFromCluster() throws Exception {
-    testEnableKerberos(new KerberosCredential("principal", "password", "keytab"), true, false);
+    testEnableKerberos(new KerberosCredential("principal", "password", "keytab"), "mit-kdc", "true", true, false);
   }
 
   @Test
   public void testEnableKerberos_GetKerberosDescriptorFromStack() throws Exception {
-    testEnableKerberos(new KerberosCredential("principal", "password", "keytab"), false, true);
+    testEnableKerberos(new KerberosCredential("principal", "password", "keytab"), "mit-kdc", "true", false, true);
   }
 
   @Test
@@ -828,7 +844,8 @@ public class KerberosHelperTest extends EasyMockSupport {
 
 
   private void testEnableKerberos(final KerberosCredential kerberosCredential,
-                                  boolean getClusterDescriptor,
+                                  String kdcType,
+                                  String manageIdentities, boolean getClusterDescriptor,
                                   boolean getStackDescriptor) throws Exception {
 
     KerberosHelper kerberosHelper = injector.getInstance(KerberosHelper.class);
@@ -924,7 +941,8 @@ public class KerberosHelperTest extends EasyMockSupport {
     expectLastCall().once();
 
     final Map<String, String> kerberosEnvProperties = createMock(Map.class);
-    expect(kerberosEnvProperties.get("kdc_type")).andReturn("mit-kdc").anyTimes();
+    expect(kerberosEnvProperties.get("kdc_type")).andReturn(kdcType).anyTimes();
+    expect(kerberosEnvProperties.get("manage_identities")).andReturn(manageIdentities).anyTimes();
     expect(kerberosEnvProperties.get("realm")).andReturn("FOOBAR.COM").anyTimes();
 
     final Config kerberosEnvConfig = createNiceMock(Config.class);
@@ -979,9 +997,11 @@ public class KerberosHelperTest extends EasyMockSupport {
           }
         })
         .once();
-    expect(clusters.getHost("host1"))
-        .andReturn(host)
-        .once();
+    if((manageIdentities == null) || !"false".equalsIgnoreCase(manageIdentities)) {
+      expect(clusters.getHost("host1"))
+          .andReturn(host)
+          .once();
+    }
 
     final AmbariManagementController ambariManagementController = injector.getInstance(AmbariManagementController.class);
     expect(ambariManagementController.findConfigurationTagsWithOverrides(cluster, "host1"))
@@ -1089,21 +1109,23 @@ public class KerberosHelperTest extends EasyMockSupport {
 
     // This is a STRICT mock to help ensure that the end result is what we want.
     final RequestStageContainer requestStageContainer = createStrictMock(RequestStageContainer.class);
-    // Create Principals Stage
-    expect(requestStageContainer.getLastStageId()).andReturn(-1L).anyTimes();
-    expect(requestStageContainer.getId()).andReturn(1L).once();
-    requestStageContainer.addStages(anyObject(List.class));
-    expectLastCall().once();
-    // Create Keytabs Stage
-    expect(requestStageContainer.getLastStageId()).andReturn(0L).anyTimes();
-    expect(requestStageContainer.getId()).andReturn(1L).once();
-    requestStageContainer.addStages(anyObject(List.class));
-    expectLastCall().once();
-    // Distribute Keytabs Stage
-    expect(requestStageContainer.getLastStageId()).andReturn(1L).anyTimes();
-    expect(requestStageContainer.getId()).andReturn(1L).once();
-    requestStageContainer.addStages(anyObject(List.class));
-    expectLastCall().once();
+    if((manageIdentities == null) || !"false".equalsIgnoreCase(manageIdentities)) {
+      // Create Principals Stage
+      expect(requestStageContainer.getLastStageId()).andReturn(-1L).anyTimes();
+      expect(requestStageContainer.getId()).andReturn(1L).once();
+      requestStageContainer.addStages(anyObject(List.class));
+      expectLastCall().once();
+      // Create Keytabs Stage
+      expect(requestStageContainer.getLastStageId()).andReturn(0L).anyTimes();
+      expect(requestStageContainer.getId()).andReturn(1L).once();
+      requestStageContainer.addStages(anyObject(List.class));
+      expectLastCall().once();
+      // Distribute Keytabs Stage
+      expect(requestStageContainer.getLastStageId()).andReturn(1L).anyTimes();
+      expect(requestStageContainer.getId()).andReturn(1L).once();
+      requestStageContainer.addStages(anyObject(List.class));
+      expectLastCall().once();
+    }
     // Update Configs Stage
     expect(requestStageContainer.getLastStageId()).andReturn(2L).anyTimes();
     expect(requestStageContainer.getId()).andReturn(1L).once();
@@ -1121,7 +1143,7 @@ public class KerberosHelperTest extends EasyMockSupport {
     // Needed by infrastructure
     metaInfo.init();
 
-    kerberosHelper.toggleKerberos(cluster, SecurityType.KERBEROS, requestStageContainer, true);
+    kerberosHelper.toggleKerberos(cluster, SecurityType.KERBEROS, requestStageContainer, null);
 
     verifyAll();
   }

ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KDCTypeTest.java

@@ -6,9 +6,9 @@
  * to you under the Apache License, Version 2.0 (the
  * "License"); you may not use this file except in compliance
  * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
+ * <p/>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p/>
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -38,14 +38,14 @@ public class KDCTypeTest {
     Assert.assertEquals(KDCType.MIT_KDC, KDCType.translate("MIT-KDC"));
   }
 
-  @Test(expected = NullPointerException.class)
+  @Test
   public void testTranslateNull() {
-    KDCType.translate(null);
+    Assert.assertEquals(KDCType.NONE, KDCType.translate(null));
   }
 
-  @Test(expected = IllegalArgumentException.class)
+  @Test
   public void testTranslateEmptyString() {
-    KDCType.translate("");
+    Assert.assertEquals(KDCType.NONE, KDCType.translate(""));
   }
 
   @Test(expected = IllegalArgumentException.class)

ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactoryTest.java

@@ -67,8 +67,13 @@ public class KerberosOperationHandlerFactoryTest {
         injector.getInstance(KerberosOperationHandlerFactory.class).getKerberosOperationHandler(KDCType.ACTIVE_DIRECTORY).getClass());
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testForNone() {
+    Assert.assertNull(injector.getInstance(KerberosOperationHandlerFactory.class).getKerberosOperationHandler(KDCType.NONE));
+  }
+
   @Test(expected = IllegalArgumentException.class)
   public void testForNull() {
-    Assert.assertNull(new KerberosOperationHandlerFactory().getKerberosOperationHandler(null));
+    Assert.assertNull(injector.getInstance(KerberosOperationHandlerFactory.class).getKerberosOperationHandler(null));
   }
 }