You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apex.apache.org by PramodSSImmaneni <gi...@git.apache.org> on 2016/03/19 18:14:28 UTC

[GitHub] incubator-apex-core pull request: APEXCORE-397 Allow configurabili...

GitHub user PramodSSImmaneni opened a pull request:

    https://github.com/apache/incubator-apex-core/pull/277

    APEXCORE-397 Allow configurability of stram web services authentication

    @davidyan74 please review

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/PramodSSImmaneni/incubator-apex-core APEXCORE-397

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-apex-core/pull/277.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #277
    
----
commit 990dd04a98fd367ef6d9317ee93f66d6876f2c8c
Author: Pramod Immaneni <pr...@datatorrent.com>
Date:   2016-03-19T02:19:25Z

    Initial web services auth

commit aeec294dd785026407e10be993506d62ab0cce29
Author: Pramod Immaneni <pr...@datatorrent.com>
Date:   2016-03-19T16:43:09Z

    Updates and test

commit 31fe02585babdae32a6e84d76fe3373617ff5903
Author: Pramod Immaneni <pr...@datatorrent.com>
Date:   2016-03-19T17:02:23Z

    Fixed checkstyle

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: APEXCORE-397 Allow configurabili...

Posted by davidyan74 <gi...@git.apache.org>.
Github user davidyan74 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/277#discussion_r57084367
  
    --- Diff: engine/src/test/java/com/datatorrent/stram/util/SecurityUtilsTest.java ---
    @@ -0,0 +1,64 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * 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
    + *
    + * 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.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package com.datatorrent.stram.util;
    +
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +import org.apache.hadoop.conf.Configuration;
    +
    +import com.datatorrent.api.Context;
    +
    +/**
    + *
    + */
    +public class SecurityUtilsTest
    +{
    +  @Test
    +  public void testStramWebSecurity()
    +  {
    +    checkWebSecurity(false, false);
    +    Configuration conf = new Configuration();
    +    checkSecurityConfiguration(conf, new boolean[][] {{false, false}, {false, true}, {false, false}, {false, false},
    +        {false, false}});
    +    conf.set(SecurityUtils.HADOOP_HTTP_AUTH_PROP, "kerberos");
    +    checkSecurityConfiguration(conf, new boolean[][] {{true, false}, {true, true}, {true, false}, {true, false},
    +        {true, true}});
    +  }
    +
    +  private void checkSecurityConfiguration(Configuration conf, boolean[][] securityConf) {
    --- End diff --
    
    curly brace :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: APEXCORE-397 Allow configurabili...

Posted by davidyan74 <gi...@git.apache.org>.
Github user davidyan74 commented on the pull request:

    https://github.com/apache/incubator-apex-core/pull/277#issuecomment-199938346
  
    I'm good with the changes. Please squash the commits when ready to be merged.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: APEXCORE-397 Allow configurabili...

Posted by davidyan74 <gi...@git.apache.org>.
Github user davidyan74 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/277#discussion_r57067551
  
    --- Diff: api/src/main/java/com/datatorrent/api/Context.java ---
    @@ -101,6 +101,20 @@
         String getJVMOptions(List<DAG.OperatorMeta> operatorMetaList);
       }
     
    +  /**
    +   * The streaming application master web service authentication strategy.
    +   * ENABLE - Enable authentication for web service access
    +   * DISABLE - Disable authentication for web services
    +   * FOLLOW_HADOOP_AUTH - Follow Hadoop authentication, if hadoop authentication is enabled, i.e., if it is set to something
    +   *                other than "simple", enable authentication for web services as well
    +   * FOLLOW_HADOOP_HTTP_AUTH - Follow Hadoop HTTP authentication, if hadoop authentication is enabled, i.e., if it is
    +   *                set to something other than "simple", enable authentication for web services as well
    +   */
    +  enum StramHTTPAuthentication
    +  {
    +    ENABLE, DISABLE, FOLLOW_HADOOP_AUTH, FOLLOW_HADOOP_HTTP_AUTH
    --- End diff --
    
    Hmmm... how about ALWAYS_ENABLE?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: APEXCORE-397 Allow configurabili...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-apex-core/pull/277


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: APEXCORE-397 Allow configurabili...

Posted by davidyan74 <gi...@git.apache.org>.
Github user davidyan74 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/277#discussion_r57036460
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/util/SecurityUtils.java ---
    @@ -0,0 +1,74 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * 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
    + *
    + * 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.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package com.datatorrent.stram.util;
    +
    +import org.apache.hadoop.conf.Configuration;
    +import org.apache.hadoop.security.UserGroupInformation;
    +
    +import com.datatorrent.api.Context;
    +import com.datatorrent.api.Context.StramHTTPAuthentication;
    +
    +/**
    + *
    + */
    +public class SecurityUtils
    +{
    +
    +  public static final String HADOOP_HTTP_AUTH_PROP = "hadoop.http.authentication.type";
    +  private static final String HADOOP_HTTP_AUTH_VALUE_SIMPLE = "simple";
    +
    +  private static boolean stramWebSecurityEnabled;
    +  private static boolean hadoopWebSecurityEnabled;
    +
    +  // If not initialized explicitly default to Hadoop auth
    +  static {
    +    hadoopWebSecurityEnabled = stramWebSecurityEnabled = UserGroupInformation.isSecurityEnabled();
    +  }
    +
    +  public static void init(Configuration configuration, StramHTTPAuthentication stramHTTPAuth)
    +  {
    +    hadoopWebSecurityEnabled = false;
    +    String authValue = configuration.get(HADOOP_HTTP_AUTH_PROP);
    +    if ((authValue != null) && !authValue.equals(HADOOP_HTTP_AUTH_VALUE_SIMPLE)) {
    +      hadoopWebSecurityEnabled = true;
    +    }
    +    // Stram http auth may not be specified and is null but still set a default
    +    boolean authDefault = false;
    +    if (stramHTTPAuth != null) {
    +      if (stramHTTPAuth == Context.StramHTTPAuthentication.FOLLOW_HADOOP_HTTP_AUTH) {
    +        stramWebSecurityEnabled = hadoopWebSecurityEnabled;
    +      } else if (stramHTTPAuth == StramHTTPAuthentication.FOLLOW_HADOOP_AUTH) {
    +        stramWebSecurityEnabled = UserGroupInformation.isSecurityEnabled();
    +      } else if (stramHTTPAuth == StramHTTPAuthentication.ENABLE) {
    +        stramWebSecurityEnabled = true;
    +      } else if (stramHTTPAuth == StramHTTPAuthentication.DISABLE) {
    +        stramWebSecurityEnabled = false;
    +      }
    +    }
    +  }
    +
    +  public static boolean isHadoopWebSecurityEnabled() {
    --- End diff --
    
    open curly brace following definition should be on a newline


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: APEXCORE-397 Allow configurabili...

Posted by davidyan74 <gi...@git.apache.org>.
Github user davidyan74 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/277#discussion_r57036221
  
    --- Diff: api/src/main/java/com/datatorrent/api/Context.java ---
    @@ -471,6 +485,12 @@
          */
         Attribute<ContainerOptConfigurator> CONTAINER_OPTS_CONFIGURATOR = new Attribute<ContainerOptConfigurator>(new Object2String<ContainerOptConfigurator>());
         /**
    +     * The strategy to use for stram web services authentication.
    +     * See {@link StramHTTPAuthentication}
    +     * Default value is StramHTTPAuthentication.FOLLOW_HADOOP_AUTH
    +     */
    +    Attribute<StramHTTPAuthentication> STRAM_HTTP_AUTHENTICATION = new Attribute<StramHTTPAuthentication>(StramHTTPAuthentication.FOLLOW_HADOOP_AUTH, new StringCodec.Enum2String<>(StramHTTPAuthentication.class));
    --- End diff --
    
    Should we take advantage of the diamond operator here: Attribute<> instead of Attribute<StramHTTPAuthentication>


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: APEXCORE-397 Allow configurabili...

Posted by davidyan74 <gi...@git.apache.org>.
Github user davidyan74 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/277#discussion_r57036051
  
    --- Diff: api/src/main/java/com/datatorrent/api/Context.java ---
    @@ -101,6 +101,20 @@
         String getJVMOptions(List<DAG.OperatorMeta> operatorMetaList);
       }
     
    +  /**
    +   * The streaming application master web service authentication strategy.
    +   * ENABLE - Enable authentication for web service access
    +   * DISABLE - Disable authentication for web services
    +   * FOLLOW_HADOOP_AUTH - Follow Hadoop authentication, if hadoop authentication is enabled, i.e., if it is set to something
    +   *                other than "simple", enable authentication for web services as well
    +   * FOLLOW_HADOOP_HTTP_AUTH - Follow Hadoop HTTP authentication, if hadoop authentication is enabled, i.e., if it is
    +   *                set to something other than "simple", enable authentication for web services as well
    +   */
    +  enum StramHTTPAuthentication
    +  {
    +    ENABLE, DISABLE, FOLLOW_HADOOP_AUTH, FOLLOW_HADOOP_HTTP_AUTH
    --- End diff --
    
    I think the name "ENABLE" is a little misleading. How about something like "BUILTIN"?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-apex-core pull request: APEXCORE-397 Allow configurabili...

Posted by PramodSSImmaneni <gi...@git.apache.org>.
Github user PramodSSImmaneni commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/277#discussion_r57061658
  
    --- Diff: api/src/main/java/com/datatorrent/api/Context.java ---
    @@ -101,6 +101,20 @@
         String getJVMOptions(List<DAG.OperatorMeta> operatorMetaList);
       }
     
    +  /**
    +   * The streaming application master web service authentication strategy.
    +   * ENABLE - Enable authentication for web service access
    +   * DISABLE - Disable authentication for web services
    +   * FOLLOW_HADOOP_AUTH - Follow Hadoop authentication, if hadoop authentication is enabled, i.e., if it is set to something
    +   *                other than "simple", enable authentication for web services as well
    +   * FOLLOW_HADOOP_HTTP_AUTH - Follow Hadoop HTTP authentication, if hadoop authentication is enabled, i.e., if it is
    +   *                set to something other than "simple", enable authentication for web services as well
    +   */
    +  enum StramHTTPAuthentication
    +  {
    +    ENABLE, DISABLE, FOLLOW_HADOOP_AUTH, FOLLOW_HADOOP_HTTP_AUTH
    --- End diff --
    
    I was thinking about this, all options still use the builtin authentication. The follow_ options determine how this auth is enabled or disable based on whether hadoop auth is enabled. I understand your concern, what would be a good substitute?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---