You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by sm...@apache.org on 2018/07/27 17:26:30 UTC

[ambari] branch trunk updated: [AMBARI-24351] Using Predicate based evaluation when determining if SSO is enabled for a service (#1890)

This is an automated email from the ASF dual-hosted git repository.

smolnar pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/trunk by this push:
     new c43f3c6  [AMBARI-24351] Using Predicate based evaluation when determining if SSO is enabled for a service (#1890)
c43f3c6 is described below

commit c43f3c6b93666876874479dce0b98cd93a8dfc28
Author: Sandor Molnar <sm...@apache.org>
AuthorDate: Fri Jul 27 19:26:28 2018 +0200

    [AMBARI-24351] Using Predicate based evaluation when determining if SSO is enabled for a service (#1890)
---
 .../server/controller/KerberosHelperImpl.java      | 43 +---------------
 .../apache/ambari/server/state/ConfigHelper.java   | 57 ++++++++++++++++++++++
 .../apache/ambari/server/state/ServiceImpl.java    | 42 +++++++++++-----
 .../apache/ambari/server/state/ServiceInfo.java    |  7 +++
 .../ambari/server/state/SingleSignOnInfo.java      | 47 +++++++++++++++++-
 .../server/controller/KerberosHelperTest.java      |  6 +--
 6 files changed, 144 insertions(+), 58 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java
index 13cb8fa..b08ddbe 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java
@@ -467,7 +467,7 @@ public class KerberosHelperImpl implements KerberosHelper {
         }
       });
 
-    Map<String, Map<String, String>> existingConfigurations = calculateExistingConfigurations(cluster, null);
+    Map<String, Map<String, String>> existingConfigurations = configHelper.calculateExistingConfigurations(ambariManagementController, cluster, null);
     Map<String, Map<String, String>> updates = getServiceConfigurationUpdates(cluster,
       existingConfigurations, installedServices, serviceFilter, previouslyExistingServices, true, true);
 
@@ -1742,7 +1742,7 @@ public class KerberosHelperImpl implements KerberosHelper {
 
     Map<String, Map<String, String>> calculatedConfigurations = addAdditionalConfigurations(
       cluster,
-      calculateExistingConfigurations(cluster, hostname),
+      configHelper.calculateExistingConfigurations(ambariManagementController, cluster, hostname),
       hostname,
       (kerberosDescriptor == null) ? null : kerberosDescriptor.getProperties());
 
@@ -2928,45 +2928,6 @@ public class KerberosHelperImpl implements KerberosHelper {
   }
 
   /**
-   * Determines the existing configurations for the cluster, related to a given hostname (if provided)
-   *
-   * @param cluster  the cluster
-   * @param hostname a hostname
-   * @return a map of the existing configurations
-   * @throws AmbariException
-   */
-  private Map<String, Map<String, String>> calculateExistingConfigurations(Cluster cluster, String hostname) throws AmbariException {
-    // For a configuration type, both tag and an actual configuration can be stored
-    // Configurations from the tag is always expanded and then over-written by the actual
-    // global:version1:{a1:A1,b1:B1,d1:D1} + global:{a1:A2,c1:C1,DELETED_d1:x} ==>
-    // global:{a1:A2,b1:B1,c1:C1}
-    Map<String, Map<String, String>> configurations = new HashMap<>();
-    Map<String, Map<String, String>> configurationTags = ambariManagementController.findConfigurationTagsWithOverrides(cluster, hostname);
-
-    Map<String, Map<String, String>> configProperties = configHelper.getEffectiveConfigProperties(cluster, configurationTags);
-
-    // Apply the configurations saved with the Execution Cmd on top of
-    // derived configs - This will take care of all the hacks
-    for (Map.Entry<String, Map<String, String>> entry : configProperties.entrySet()) {
-      String type = entry.getKey();
-      Map<String, String> allLevelMergedConfig = entry.getValue();
-      Map<String, String> configuration = configurations.get(type);
-
-      if (configuration == null) {
-        configuration = new HashMap<>(allLevelMergedConfig);
-      } else {
-        Map<String, String> mergedConfig = configHelper.getMergedConfig(allLevelMergedConfig, configuration);
-        configuration.clear();
-        configuration.putAll(mergedConfig);
-      }
-
-      configurations.put(type, configuration);
-    }
-
-    return configurations;
-  }
-
-  /**
    * Add configurations related to Kerberos, to a previously created map of configurations.
    * <p/>
    * The supplied map of configurations is expected to be mutable and will be altered.
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
index ba8c3d6..c685c79 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
@@ -2122,4 +2122,61 @@ public class ConfigHelper {
     return configurationAttributesTreeMap;
   }
 
+  /**
+   * Determines the existing configurations for the cluster
+   *
+   * @param ambariManagementController
+   *          the Ambari management controller
+   * @param cluster
+   *          the cluster
+   * @return a map of the existing configurations
+   * @throws AmbariException
+   */
+  public Map<String, Map<String, String>> calculateExistingConfigurations(AmbariManagementController ambariManagementController, Cluster cluster) throws AmbariException {
+    final Map<String, Map<String, String>> configurations = new HashMap<>();
+    for (Host host : cluster.getHosts()) {
+      configurations.putAll(calculateExistingConfigurations(ambariManagementController, cluster, host.getHostName()));
+    }
+    return configurations;
+  }
+
+  /**
+   * Determines the existing configurations for the cluster, related to a given hostname (if provided)
+   *
+   * @param ambariManagementController the Ambari management controller
+   * @param cluster  the cluster
+   * @param hostname a hostname
+   * @return a map of the existing configurations
+   * @throws AmbariException
+   */
+  public Map<String, Map<String, String>> calculateExistingConfigurations(AmbariManagementController ambariManagementController, Cluster cluster, String hostname) throws AmbariException {
+    // For a configuration type, both tag and an actual configuration can be stored
+    // Configurations from the tag is always expanded and then over-written by the actual
+    // global:version1:{a1:A1,b1:B1,d1:D1} + global:{a1:A2,c1:C1,DELETED_d1:x} ==>
+    // global:{a1:A2,b1:B1,c1:C1}
+    final Map<String, Map<String, String>> configurations = new HashMap<>();
+    final Map<String, Map<String, String>> configurationTags = ambariManagementController.findConfigurationTagsWithOverrides(cluster, hostname);
+    final Map<String, Map<String, String>> configProperties = getEffectiveConfigProperties(cluster, configurationTags);
+
+    // Apply the configurations saved with the Execution Cmd on top of
+    // derived configs - This will take care of all the hacks
+    for (Map.Entry<String, Map<String, String>> entry : configProperties.entrySet()) {
+      String type = entry.getKey();
+      Map<String, String> allLevelMergedConfig = entry.getValue();
+      Map<String, String> configuration = configurations.get(type);
+
+      if (configuration == null) {
+        configuration = new HashMap<>(allLevelMergedConfig);
+      } else {
+        Map<String, String> mergedConfig = getMergedConfig(allLevelMergedConfig, configuration);
+        configuration.clear();
+        configuration.putAll(mergedConfig);
+      }
+
+      configurations.put(type, configuration);
+    }
+
+    return configurations;
+  }
+
 }
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java
index eaf9206..52d4a23 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceImpl.java
@@ -32,9 +32,13 @@ import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.ambari.server.AmbariException;
+import org.apache.ambari.server.AmbariRuntimeException;
 import org.apache.ambari.server.ObjectNotFoundException;
 import org.apache.ambari.server.ServiceComponentNotFoundException;
 import org.apache.ambari.server.api.services.AmbariMetaInfo;
+import org.apache.ambari.server.collections.Predicate;
+import org.apache.ambari.server.collections.PredicateUtils;
+import org.apache.ambari.server.controller.AmbariManagementController;
 import org.apache.ambari.server.controller.ServiceResponse;
 import org.apache.ambari.server.controller.internal.AmbariServerSSOConfigurationHandler;
 import org.apache.ambari.server.controller.internal.DeleteHostComponentStatusMetaData;
@@ -82,7 +86,7 @@ public class ServiceImpl implements Service {
   private boolean isCredentialStoreSupported;
   private boolean isCredentialStoreRequired;
   private final boolean ssoIntegrationSupported;
-  private final String ssoEnabledConfiguration;
+  private final Predicate ssoEnabledConfiguration;
   private final boolean ssoRequiresKerberos;
   private AmbariMetaInfo ambariMetaInfo;
   private AtomicReference<MaintenanceState> maintenanceState = new AtomicReference<>();
@@ -91,6 +95,9 @@ public class ServiceImpl implements Service {
   private ServiceConfigDAO serviceConfigDAO;
 
   @Inject
+  private AmbariManagementController ambariManagementController;
+
+  @Inject
   private ConfigHelper configHelper;
 
   @Inject
@@ -150,12 +157,26 @@ public class ServiceImpl implements Service {
     isCredentialStoreSupported = sInfo.isCredentialStoreSupported();
     isCredentialStoreRequired = sInfo.isCredentialStoreRequired();
     ssoIntegrationSupported = sInfo.isSingleSignOnSupported();
-    ssoEnabledConfiguration = sInfo.getSingleSignOnEnabledConfiguration();
+    ssoEnabledConfiguration = compileSsoEnabledPredicate(sInfo);
     ssoRequiresKerberos = sInfo.isKerberosRequiredForSingleSignOnIntegration();
 
     persist(serviceEntity);
   }
 
+  private Predicate compileSsoEnabledPredicate(ServiceInfo sInfo) {
+    if (StringUtils.isNotBlank(sInfo.getSingleSignOnEnabledTest())) {
+      if (StringUtils.isNotBlank(sInfo.getSingleSignOnEnabledConfiguration())) {
+        LOG.warn("Both <ssoEnabledTest> and <enabledConfiguration> have been declared within <sso> for {}; using <ssoEnabledTest>", serviceName);
+      }
+      return PredicateUtils.fromJSON(sInfo.getSingleSignOnEnabledTest());
+    } else if (StringUtils.isNotBlank(sInfo.getSingleSignOnEnabledConfiguration())) {
+      LOG.warn("Only <enabledConfiguration> have been declared  within <sso> for {}; converting its value to an equals predicate", serviceName);
+      final String equalsPredicateJson = "{\"equals\": [\"" + sInfo.getSingleSignOnEnabledConfiguration() + "\", \"true\"]}";
+      return PredicateUtils.fromJSON(equalsPredicateJson);
+    }
+    return null;
+  }
+
   @AssistedInject
   ServiceImpl(@Assisted Cluster cluster, @Assisted ClusterServiceEntity serviceEntity,
       ClusterDAO clusterDAO, ClusterServiceDAO clusterServiceDAO,
@@ -200,7 +221,7 @@ public class ServiceImpl implements Service {
     isCredentialStoreRequired = sInfo.isCredentialStoreRequired();
     displayName = sInfo.getDisplayName();
     ssoIntegrationSupported = sInfo.isSingleSignOnSupported();
-    ssoEnabledConfiguration = sInfo.getSingleSignOnEnabledConfiguration();
+    ssoEnabledConfiguration = compileSsoEnabledPredicate(sInfo);
     ssoRequiresKerberos = sInfo.isKerberosRequiredForSingleSignOnIntegration();
   }
 
@@ -713,20 +734,15 @@ public class ServiceImpl implements Service {
   }
 
   public boolean isSsoIntegrationEnabled() {
-    return ssoIntegrationSupported && ssoEnabledConfigValid() && "true".equalsIgnoreCase(ssoEnabledConfigValue());
-  }
-
-  private boolean ssoEnabledConfigValid() {
-    return ssoEnabledConfiguration != null && ssoEnabledConfiguration.split("/").length == 2;
+    try {
+      return ssoIntegrationSupported && ssoEnabledConfiguration != null && ssoEnabledConfiguration.evaluate(configHelper.calculateExistingConfigurations(ambariManagementController, cluster));
+    } catch (AmbariException e) {
+      throw new AmbariRuntimeException("Error while evaulating if SSO integration is enabled", e);
+    }
   }
 
   private boolean isKerberosRequredForSsoIntegration() {
     return ssoRequiresKerberos;
   }
 
-  private String ssoEnabledConfigValue() {
-    String configType = ssoEnabledConfiguration.split("/")[0];
-    String propertyName = ssoEnabledConfiguration.split("/")[1];
-    return configHelper.getValueFromDesiredConfigurations(cluster, configType, propertyName);
-  }
 }
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java
index 2636a39..938f73b 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java
@@ -653,10 +653,17 @@ public class ServiceInfo implements Validable {
     return (singleSignOnInfo != null) && singleSignOnInfo.isSupported();
   }
 
+  /**
+   * @deprecated Use {@link #getSingleSignOnEnabledTest()} instead
+   */
   public String getSingleSignOnEnabledConfiguration() {
     return singleSignOnInfo != null ? singleSignOnInfo.getEnabledConfiguration() : null;
   }
 
+  public String getSingleSignOnEnabledTest() {
+    return singleSignOnInfo != null ? singleSignOnInfo.getSsoEnabledTest() : null;
+  }
+
   /**
    * @return the boolean flag is Kerberos is required for SSO integration
    */
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/SingleSignOnInfo.java b/ambari-server/src/main/java/org/apache/ambari/server/state/SingleSignOnInfo.java
index d78fc86..0a5fd36 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/SingleSignOnInfo.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/SingleSignOnInfo.java
@@ -22,6 +22,8 @@ import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
 import javax.xml.bind.annotation.XmlElement;
 
+import org.apache.ambari.server.collections.PredicateUtils;
+
 import com.google.common.base.MoreObjects;
 
 /**
@@ -32,7 +34,14 @@ import com.google.common.base.MoreObjects;
  * <pre>
  *   &lt;sso&gt;
  *     &lt;supported&gt;true&lt;/supported&gt;
- *     &lt;enabledConfiguration&gt;config-type/sso.enabled.property_name&lt;/enabledConfiguration&gt;
+ *     &lt;ssoEnabledTest&gt;
+ *         {
+ *           "equals": [
+ *             "config-type/sso.enabled.property_name",
+ *             "true"
+ *           ]
+ *         }
+ *     &lt;/ssoEnabledTest&gt;
  *   &lt;/sso&gt;
  * </pre>
  */
@@ -49,11 +58,21 @@ public class SingleSignOnInfo {
    * The configuration that can be used to determine if SSO integration has been enabled.
    * <p>
    * It is expected that this value is in the form of <code>configuration-type/property_name</code>
+   *
+   * @deprecated Use {@link #ssoEnabledTest} instead
    */
   @XmlElement(name = "enabledConfiguration")
   private String enabledConfiguration = null;
 
   /**
+   * The configuration that can be used to determine if SSO integration has been enabled.
+   * <p>
+   * It is expected that this value is in the form of a valid JSON predicate ({@link PredicateUtils#fromJSON(String)}
+   */
+  @XmlElement(name = "ssoEnabledTest")
+  private String ssoEnabledTest = null;
+
+  /**
    * Indicates if Kerberos is required for SSO integration (<code>true</code>) or not (<code>false</code>)
    */
   @XmlElement(name = "kerberosRequired")
@@ -109,6 +128,8 @@ public class SingleSignOnInfo {
   }
 
   /**
+   * @deprecated USe {@link #getSsoEnabledTest()} instead
+   * <p>
    * Gets the configuration specification that can be used to determine if SSO has been enabled or not.
    *
    * @return a configuration specification (config-type/property_name)
@@ -118,6 +139,8 @@ public class SingleSignOnInfo {
   }
 
   /**
+   * @deprecated Use {@link #setSsoEnabledTest(String)} instead
+   * <p>
    * Sets the configuration specification that can be used to determine if SSO has been enabled or not.
    *
    * @param enabledConfiguration a configuration specification (config-type/property_name)
@@ -127,6 +150,27 @@ public class SingleSignOnInfo {
   }
 
   /**
+   * Gets the configuration specification that can be used to determine if SSO has
+   * been enabled or not.
+   *
+   * @return a configuration specification (a valid JSON predicate)
+   */
+  public String getSsoEnabledTest() {
+    return ssoEnabledTest;
+  }
+
+  /**
+   * Sets the configuration specification that can be used to determine if SSO has
+   * been enabled or not.
+   *
+   * @param enabledConfiguration
+   *          a configuration specification (a valid JSON predicate)
+   */
+  public void setSsoEnabledTest(String ssoEnabledTest) {
+    this.ssoEnabledTest = ssoEnabledTest;
+  }
+
+  /**
    * @return the flag is Kerberos is required for SSO integration
    */
   public boolean isKerberosRequired() {
@@ -150,6 +194,7 @@ public class SingleSignOnInfo {
     return MoreObjects.toStringHelper(this)
         .add("supported", supported)
         .add("enabledConfiguration", enabledConfiguration)
+        .add("ssoEnabledTest", ssoEnabledTest)
         .add("kerberosRequired", kerberosRequired)
         .toString();
   }
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
index 09d5d4c..9209cae 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
@@ -3537,7 +3537,7 @@ public class KerberosHelperTest extends EasyMockSupport {
           .once();
 
       final ConfigHelper configHelper = injector.getInstance(ConfigHelper.class);
-      expect(configHelper.getEffectiveConfigProperties(anyObject(Cluster.class), EasyMock.anyObject()))
+      expect(configHelper.calculateExistingConfigurations(eq(ambariManagementController), anyObject(Cluster.class), EasyMock.anyObject()))
           .andReturn(new HashMap<String, Map<String, String>>() {
             {
               put("cluster-env", new HashMap<String, String>() {{
@@ -3710,7 +3710,7 @@ public class KerberosHelperTest extends EasyMockSupport {
         .once();
 
     final ConfigHelper configHelper = injector.getInstance(ConfigHelper.class);
-    expect(configHelper.getEffectiveConfigProperties(anyObject(Cluster.class), EasyMock.anyObject()))
+    expect(configHelper.calculateExistingConfigurations(eq(ambariManagementController), anyObject(Cluster.class), EasyMock.anyObject()))
         .andReturn(new HashMap<String, Map<String, String>>() {
           {
             put("cluster-env", new HashMap<String, String>() {
@@ -3927,7 +3927,7 @@ public class KerberosHelperTest extends EasyMockSupport {
         .anyTimes();
 
     final ConfigHelper configHelper = injector.getInstance(ConfigHelper.class);
-    expect(configHelper.getEffectiveConfigProperties(anyObject(Cluster.class), EasyMock.anyObject()))
+    expect(configHelper.calculateExistingConfigurations(eq(ambariManagementController), anyObject(Cluster.class), EasyMock.anyObject()))
         .andReturn(new HashMap<String, Map<String, String>>() {
           {
             put("cluster-env", new HashMap<String, String>() {{