You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/08/23 09:34:45 UTC

[GitHub] [hive] ShubhamChaurasia opened a new pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

ShubhamChaurasia opened a new pull request #1418:
URL: https://github.com/apache/hive/pull/1418


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] ShubhamChaurasia commented on a change in pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

Posted by GitBox <gi...@apache.org>.
ShubhamChaurasia commented on a change in pull request #1418:
URL: https://github.com/apache/hive/pull/1418#discussion_r479979149



##########
File path: llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java
##########
@@ -474,6 +478,10 @@ public void serviceStart() throws Exception {
       getConfig().setInt(ConfVars.LLAP_DAEMON_WEB_PORT.varname, webServices.getPort());
     }
     getConfig().setInt(ConfVars.LLAP_DAEMON_OUTPUT_SERVICE_PORT.varname, LlapOutputFormatService.get().getPort());
+    if (LlapUtil.isCloudDeployment()) {
+      getConfig().setInt(ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_RPC_PORT.varname,

Review comment:
       done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] ShubhamChaurasia commented on a change in pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

Posted by GitBox <gi...@apache.org>.
ShubhamChaurasia commented on a change in pull request #1418:
URL: https://github.com/apache/hive/pull/1418#discussion_r479978966



##########
File path: llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java
##########
@@ -157,12 +153,12 @@ public LlapBaseInputFormat() {
     HiveConf.setVar(job, HiveConf.ConfVars.LLAP_ZK_REGISTRY_USER, llapSplit.getLlapUser());
     SubmitWorkInfo submitWorkInfo = SubmitWorkInfo.fromBytes(llapSplit.getPlanBytes());
 
-    LlapServiceInstance serviceInstance = getServiceInstance(job, llapSplit);
-    String host = serviceInstance.getHost();
-    int llapSubmitPort = serviceInstance.getRpcPort();
+    final LlapDaemonInfo llapDaemonInfo = llapSplit.getLlapDaemonInfos()[0];

Review comment:
       no, it does not as of now. Same has been validated in get_splits. Added a comment about it in new patch.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] ShubhamChaurasia commented on a change in pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

Posted by GitBox <gi...@apache.org>.
ShubhamChaurasia commented on a change in pull request #1418:
URL: https://github.com/apache/hive/pull/1418#discussion_r479982596



##########
File path: llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java
##########
@@ -342,6 +352,33 @@ public SubmitWorkResponseProto submitWork(SubmitWorkRequestProto request) throws
         .build();
   }
 
+  // if request is coming from llap external client, verify the JWT
+  // as of now, JWT contains applicationId

Review comment:
       added a comment in code which explains this - 
   
   In GenericUDTFGetSplits
   
   // 6. Generate JWT for external clients if it's a cloud deployment
           // we inject extClientAppId in JWT which is same as what fragment contains.
           // extClientAppId in JWT and in fragment are compared on LLAP when a fragment is submitted.
           // see method ContainerRunnerImpl#verifyJwtForExternalClient
   
       
   In ContainerRunnerImpl#verifyJwtForExternalClient
   
   
   // extClientAppId is injected in JWT and fragment request by initial get_splits() call.
         // so both of these - extClientAppIdFromJwt and extClientAppIdFromSplit should be equal eventually if the signed JWT is valid for this request.
         // In get_splits, this extClientAppId is obtained via LlapCoordinator#createExtClientAppId which generates a
         // application Id to be used by external clients.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] ShubhamChaurasia commented on a change in pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

Posted by GitBox <gi...@apache.org>.
ShubhamChaurasia commented on a change in pull request #1418:
URL: https://github.com/apache/hive/pull/1418#discussion_r481088959



##########
File path: llap-common/src/java/org/apache/hadoop/hive/llap/security/DefaultJwtSharedSecretProvider.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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 org.apache.hadoop.hive.llap.security;
+
+import com.google.common.base.Preconditions;
+import io.jsonwebtoken.security.Keys;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.conf.HiveConf;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+import java.nio.charset.StandardCharsets;
+import java.security.Key;
+
+import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_DEPLOYMENT_SETUP_ENABLED;
+import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET;
+
+/**
+ * Default implementation of {@link JwtSecretProvider}.
+ *
+ * 1. It first tries to get shared secret from conf {@link HiveConf.ConfVars#LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET}
+ * using {@link Configuration#getPassword(String)}.
+ *
+ * 2. If not found, it tries to read from env var {@link #LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR}.
+ *
+ * If secret is not found even after 1) and 2), {@link #init(Configuration)} methods throws {@link NullPointerException}.
+ *
+ * It uses the same encryption and decryption secret which can be used to sign and verify JWT.
+ */
+public class DefaultJwtSharedSecretProvider implements JwtSecretProvider {
+
+  public static final String LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR =
+      "LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR";
+
+  private Key jwtEncryptionKey;
+
+  @Override public Key getEncryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public Key getDecryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public void init(final Configuration conf) {
+    char[] sharedSecret;
+    byte[] sharedSecretBytes = null;
+
+    // try getting secret from conf first
+    // if not found, get from env var - LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR
+    try {
+      sharedSecret = conf.getPassword(LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET.varname);
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to get password [hive.llap.external.client.cloud.jwt.shared.secret] - "
+          + e.getMessage(), e);
+    }
+    if (sharedSecret != null) {
+      ByteBuffer bb = StandardCharsets.UTF_8.encode(CharBuffer.wrap(sharedSecret));
+      sharedSecretBytes = new byte[bb.remaining()];
+      bb.get(sharedSecretBytes);
+    } else {
+      String sharedSecredFromEnv = System.getenv(LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR);
+      if (sharedSecredFromEnv != null) {
+        sharedSecretBytes = sharedSecredFromEnv.getBytes();
+      }
+    }
+
+    Preconditions.checkNotNull(sharedSecretBytes,

Review comment:
       done

##########
File path: llap-common/src/java/org/apache/hadoop/hive/llap/security/DefaultJwtSharedSecretProvider.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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 org.apache.hadoop.hive.llap.security;
+
+import com.google.common.base.Preconditions;
+import io.jsonwebtoken.security.Keys;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.conf.HiveConf;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+import java.nio.charset.StandardCharsets;
+import java.security.Key;
+
+import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_DEPLOYMENT_SETUP_ENABLED;
+import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET;
+
+/**
+ * Default implementation of {@link JwtSecretProvider}.
+ *
+ * 1. It first tries to get shared secret from conf {@link HiveConf.ConfVars#LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET}
+ * using {@link Configuration#getPassword(String)}.
+ *
+ * 2. If not found, it tries to read from env var {@link #LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR}.
+ *
+ * If secret is not found even after 1) and 2), {@link #init(Configuration)} methods throws {@link NullPointerException}.
+ *
+ * It uses the same encryption and decryption secret which can be used to sign and verify JWT.
+ */
+public class DefaultJwtSharedSecretProvider implements JwtSecretProvider {
+
+  public static final String LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR =
+      "LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR";
+
+  private Key jwtEncryptionKey;
+
+  @Override public Key getEncryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public Key getDecryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public void init(final Configuration conf) {
+    char[] sharedSecret;
+    byte[] sharedSecretBytes = null;
+
+    // try getting secret from conf first
+    // if not found, get from env var - LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR
+    try {
+      sharedSecret = conf.getPassword(LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET.varname);
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to get password [hive.llap.external.client.cloud.jwt.shared.secret] - "
+          + e.getMessage(), e);
+    }
+    if (sharedSecret != null) {
+      ByteBuffer bb = StandardCharsets.UTF_8.encode(CharBuffer.wrap(sharedSecret));
+      sharedSecretBytes = new byte[bb.remaining()];
+      bb.get(sharedSecretBytes);
+    } else {
+      String sharedSecredFromEnv = System.getenv(LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR);
+      if (sharedSecredFromEnv != null) {

Review comment:
       done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kgyrtkirk closed pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

Posted by GitBox <gi...@apache.org>.
kgyrtkirk closed pull request #1418:
URL: https://github.com/apache/hive/pull/1418


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] ShubhamChaurasia commented on a change in pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

Posted by GitBox <gi...@apache.org>.
ShubhamChaurasia commented on a change in pull request #1418:
URL: https://github.com/apache/hive/pull/1418#discussion_r479982693



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java
##########
@@ -559,12 +566,22 @@ private SplitResult getSplits(JobConf job, TezWork work, Schema schema, Applicat
         // 4. Make location hints.
         SplitLocationInfo[] locations = makeLocationHints(hints.get(i));
 
+        // 5. populate info about llap daemons(to help client submit request and read data)
+        LlapDaemonInfo[] llapDaemonInfos = populateLlapDaemonInfos(job, locations);
+
+        // 6. Generate JWT for external clients if it's a cloud deployment
+        String jwt = "";
+        if (LlapUtil.isCloudDeployment()) {
+          JwtHelper jwtHelper = new JwtHelper(SessionState.getSessionConf());
+          jwt = jwtHelper.buildJwtForLlap(applicationId);

Review comment:
       done

##########
File path: llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java
##########
@@ -342,6 +352,33 @@ public SubmitWorkResponseProto submitWork(SubmitWorkRequestProto request) throws
         .build();
   }
 
+  // if request is coming from llap external client, verify the JWT
+  // as of now, JWT contains applicationId
+  private void verifyJwtForExternalClient(SubmitWorkRequestProto request, String applicationIdString,
+      String fragmentIdString) {
+    LOG.info("Checking if request[{}] is from llap external client in a cloud based deployment", applicationIdString);
+    if (request.getIsExternalClientRequest() && LlapUtil.isCloudDeployment()) {
+      LOG.info("Llap external client request - {}, verifying JWT", applicationIdString);
+      Preconditions.checkState(request.hasJwt(), "JWT not found in request, fragmentId: " + fragmentIdString);
+
+      JwtHelper jwtHelper = new JwtHelper(getConfig());
+      Jws<Claims> claimsJws;
+      try {
+        claimsJws = jwtHelper.parseClaims(request.getJwt());
+      } catch (JwtException e) {
+        LOG.error("Cannot verify JWT provided with the request, fragmentId: {}, {}", fragmentIdString, e);
+        throw e;
+      }
+
+      String appIdInJwt = (String) claimsJws.getBody().get(JwtHelper.LLAP_EXT_CLIENT_APP_ID);
+      // this should never happen ideally.
+      Preconditions.checkState(appIdInJwt.equals(applicationIdString),

Review comment:
       done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] ShubhamChaurasia commented on a change in pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

Posted by GitBox <gi...@apache.org>.
ShubhamChaurasia commented on a change in pull request #1418:
URL: https://github.com/apache/hive/pull/1418#discussion_r479978466



##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -4880,6 +4880,22 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
     LLAP_EXTERNAL_CLIENT_USE_HYBRID_CALENDAR("hive.llap.external.client.use.hybrid.calendar",
         false,
         "Whether to use hybrid calendar for parsing of data/timestamps."),
+
+    // confs for llap-external-client cloud deployment
+    LLAP_EXTERNAL_CLIENT_CLOUD_RPC_PORT("hive.llap.external.client.cloud.rpc.port", 30004,
+        "The LLAP daemon RPC port for external clients when llap is running in cloud environment."),
+    LLAP_EXTERNAL_CLIENT_CLOUD_OUTPUT_SERVICE_PORT("hive.llap.external.client.cloud.output.service.port", 30005,
+                "LLAP output service port when llap is running in cloud environment"),
+    LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_PROVIDER(
+        "hive.llap.external.client.cloud.jwt.shared.secret.provider",
+        "org.apache.hadoop.hive.llap.security.ConfBasedJwtSharedSecretProvider",

Review comment:
       It's now changed to DefaultJwtSharedSecretProvider which reads from conf.getPassword, if not found, then tries ENV variable.

##########
File path: llap-client/src/java/org/apache/hadoop/hive/llap/registry/LlapServiceInstance.java
##########
@@ -47,6 +49,24 @@
    */
   public int getOutputFormatPort();
 
+  /**
+   * External host, usually needed in cloud envs where we cannot access internal host from outside
+   *
+   * @return
+   */
+  String getExternalHost();

Review comment:
       done

##########
File path: llap-common/src/java/org/apache/hadoop/hive/llap/security/ConfBasedJwtSharedSecretProvider.java
##########
@@ -0,0 +1,52 @@
+/*
+ * 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 org.apache.hadoop.hive.llap.security;
+
+import com.google.common.base.Preconditions;
+import io.jsonwebtoken.security.Keys;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.conf.HiveConf;
+
+import java.security.Key;
+
+/**
+ * Default implementation of {@link JwtSecretProvider}.
+ * It uses the same encryption and decryption secret which can be used to sign and verify JWT.
+ */
+public class ConfBasedJwtSharedSecretProvider implements JwtSecretProvider {
+
+  private Key jwtEncryptionKey;
+
+  @Override public Key getEncryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public Key getDecryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public void init(final Configuration conf) {
+    final String sharedSecret = HiveConf.getVar(conf, HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET);

Review comment:
       done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] prasanthj commented on a change in pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

Posted by GitBox <gi...@apache.org>.
prasanthj commented on a change in pull request #1418:
URL: https://github.com/apache/hive/pull/1418#discussion_r477463702



##########
File path: llap-common/src/java/org/apache/hadoop/hive/llap/security/ConfBasedJwtSharedSecretProvider.java
##########
@@ -0,0 +1,52 @@
+/*
+ * 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 org.apache.hadoop.hive.llap.security;
+
+import com.google.common.base.Preconditions;
+import io.jsonwebtoken.security.Keys;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.conf.HiveConf;
+
+import java.security.Key;
+
+/**
+ * Default implementation of {@link JwtSecretProvider}.
+ * It uses the same encryption and decryption secret which can be used to sign and verify JWT.
+ */
+public class ConfBasedJwtSharedSecretProvider implements JwtSecretProvider {
+
+  private Key jwtEncryptionKey;
+
+  @Override public Key getEncryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public Key getDecryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public void init(final Configuration conf) {
+    final String sharedSecret = HiveConf.getVar(conf, HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET);

Review comment:
       Use conf.getPassword(). This should be fetched from jceks file. 

##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -4880,6 +4880,22 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
     LLAP_EXTERNAL_CLIENT_USE_HYBRID_CALENDAR("hive.llap.external.client.use.hybrid.calendar",
         false,
         "Whether to use hybrid calendar for parsing of data/timestamps."),
+
+    // confs for llap-external-client cloud deployment
+    LLAP_EXTERNAL_CLIENT_CLOUD_RPC_PORT("hive.llap.external.client.cloud.rpc.port", 30004,
+        "The LLAP daemon RPC port for external clients when llap is running in cloud environment."),
+    LLAP_EXTERNAL_CLIENT_CLOUD_OUTPUT_SERVICE_PORT("hive.llap.external.client.cloud.output.service.port", 30005,
+                "LLAP output service port when llap is running in cloud environment"),
+    LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_PROVIDER(
+        "hive.llap.external.client.cloud.jwt.shared.secret.provider",
+        "org.apache.hadoop.hive.llap.security.ConfBasedJwtSharedSecretProvider",
+        "Shared secret provider to be used to sign JWT"),
+    LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET("hive.llap.external.client.cloud.jwt.shared.secret",
+        "Let me give you this secret and you will get the access!",

Review comment:
       may be keep the default value empty. The system deploying this would have to randomly generate and store it in jceks file. empty default value and fail early instead of falling back to default. 

##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -4880,6 +4880,22 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
     LLAP_EXTERNAL_CLIENT_USE_HYBRID_CALENDAR("hive.llap.external.client.use.hybrid.calendar",
         false,
         "Whether to use hybrid calendar for parsing of data/timestamps."),
+
+    // confs for llap-external-client cloud deployment
+    LLAP_EXTERNAL_CLIENT_CLOUD_RPC_PORT("hive.llap.external.client.cloud.rpc.port", 30004,
+        "The LLAP daemon RPC port for external clients when llap is running in cloud environment."),
+    LLAP_EXTERNAL_CLIENT_CLOUD_OUTPUT_SERVICE_PORT("hive.llap.external.client.cloud.output.service.port", 30005,
+                "LLAP output service port when llap is running in cloud environment"),
+    LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_PROVIDER(
+        "hive.llap.external.client.cloud.jwt.shared.secret.provider",
+        "org.apache.hadoop.hive.llap.security.ConfBasedJwtSharedSecretProvider",

Review comment:
       I would also provide environment variable based secret provider. Conf based ones have the potential of plain text secret if jceks is not used. Also will be good to have a config to enable the ports explicitly. If the config is false, then none of these ports, jwt secret etc. should be initialized. 

##########
File path: llap-common/src/java/org/apache/hadoop/hive/llap/security/JwtSecretProvider.java
##########
@@ -0,0 +1,71 @@
+/*
+ * 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 org.apache.hadoop.hive.llap.security;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.conf.HiveConf;
+
+import java.security.Key;
+
+/**
+ * JwtSecretProvider
+ *
+ * - provides encryption and decryption secrets for generating and parsing JWTs.
+ *
+ * - Hive internally uses method initAndGet() which initializes providers based on the value of config
+ *   {@link HiveConf.ConfVars#LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_PROVIDER}.
+ *   It expects implementations to provide default constructor and {@link #init(Configuration)} method.
+ */
+public interface JwtSecretProvider {
+
+  /**
+   * returns secret for signing JWT.
+   */
+  Key getEncryptionSecret();
+
+  /**
+   * returns secret for parsing JWT.
+   */
+  Key getDecryptionSecret();
+
+  /**
+   * Initializes the provider.
+   * @param conf configuration
+   */
+  void init(Configuration conf);
+
+  /**
+   *  Hive internally uses this method to obtain instance of {@link JwtSecretProvider}
+   *
+   * @param conf configuration
+   * @return implementation of {@link HiveConf.ConfVars#LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_PROVIDER}
+   */
+  static JwtSecretProvider initAndGet(Configuration conf) {
+    final String providerClass =
+        HiveConf.getVar(conf, HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_PROVIDER);

Review comment:
       same here. conf.getPassword

##########
File path: llap-client/src/java/org/apache/hadoop/hive/llap/registry/LlapServiceInstance.java
##########
@@ -47,6 +49,24 @@
    */
   public int getOutputFormatPort();
 
+  /**
+   * External host, usually needed in cloud envs where we cannot access internal host from outside
+   *
+   * @return
+   */
+  String getExternalHost();

Review comment:
       nit: getExternalHostname()

##########
File path: llap-common/src/java/org/apache/hadoop/hive/llap/LlapUtil.java
##########
@@ -401,4 +401,24 @@ public static Credentials credentialsFromByteArray(byte[] binaryCredentials)
     credentials.readTokenStorageStream(dib);
     return credentials;
   }
+
+  /**
+   * @return returns the value of IS_CLOUD_DEPLOYMENT from either environment variable or system properties
+   */
+  public static boolean isCloudDeployment() {
+    return "true".equalsIgnoreCase(System.getenv("IS_CLOUD_DEPLOYMENT"))

Review comment:
       alternative you can use is to default the external ports to 0. If port number is set to 0 then it is not cloud deployment. 

##########
File path: llap-ext-client/src/java/org/apache/hadoop/hive/llap/LlapBaseInputFormat.java
##########
@@ -157,12 +153,12 @@ public LlapBaseInputFormat() {
     HiveConf.setVar(job, HiveConf.ConfVars.LLAP_ZK_REGISTRY_USER, llapSplit.getLlapUser());
     SubmitWorkInfo submitWorkInfo = SubmitWorkInfo.fromBytes(llapSplit.getPlanBytes());
 
-    LlapServiceInstance serviceInstance = getServiceInstance(job, llapSplit);
-    String host = serviceInstance.getHost();
-    int llapSubmitPort = serviceInstance.getRpcPort();
+    final LlapDaemonInfo llapDaemonInfo = llapSplit.getLlapDaemonInfos()[0];

Review comment:
       can getLlapDaemonInfos() return empty array? 

##########
File path: llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java
##########
@@ -342,6 +352,33 @@ public SubmitWorkResponseProto submitWork(SubmitWorkRequestProto request) throws
         .build();
   }
 
+  // if request is coming from llap external client, verify the JWT
+  // as of now, JWT contains applicationId

Review comment:
       which applicationId is it? I assume llap application ID? can you explain a little bit more about how/who injects the llap application ID into jwt and how llap validate it against its own app ID. 

##########
File path: llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/LlapDaemon.java
##########
@@ -474,6 +478,10 @@ public void serviceStart() throws Exception {
       getConfig().setInt(ConfVars.LLAP_DAEMON_WEB_PORT.varname, webServices.getPort());
     }
     getConfig().setInt(ConfVars.LLAP_DAEMON_OUTPUT_SERVICE_PORT.varname, LlapOutputFormatService.get().getPort());
+    if (LlapUtil.isCloudDeployment()) {
+      getConfig().setInt(ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_RPC_PORT.varname,

Review comment:
       i think somewhere here we have to do a validation.. if the port is non-zero and if the jwt shared secret cannot be obtained then fail the daemon.. 

##########
File path: llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java
##########
@@ -342,6 +352,33 @@ public SubmitWorkResponseProto submitWork(SubmitWorkRequestProto request) throws
         .build();
   }
 
+  // if request is coming from llap external client, verify the JWT
+  // as of now, JWT contains applicationId
+  private void verifyJwtForExternalClient(SubmitWorkRequestProto request, String applicationIdString,
+      String fragmentIdString) {
+    LOG.info("Checking if request[{}] is from llap external client in a cloud based deployment", applicationIdString);
+    if (request.getIsExternalClientRequest() && LlapUtil.isCloudDeployment()) {
+      LOG.info("Llap external client request - {}, verifying JWT", applicationIdString);
+      Preconditions.checkState(request.hasJwt(), "JWT not found in request, fragmentId: " + fragmentIdString);
+
+      JwtHelper jwtHelper = new JwtHelper(getConfig());
+      Jws<Claims> claimsJws;
+      try {
+        claimsJws = jwtHelper.parseClaims(request.getJwt());
+      } catch (JwtException e) {
+        LOG.error("Cannot verify JWT provided with the request, fragmentId: {}, {}", fragmentIdString, e);
+        throw e;
+      }
+
+      String appIdInJwt = (String) claimsJws.getBody().get(JwtHelper.LLAP_EXT_CLIENT_APP_ID);
+      // this should never happen ideally.
+      Preconditions.checkState(appIdInJwt.equals(applicationIdString),

Review comment:
       .equals(selfAppIdString) where selfAppIdString is llap daemon's app id.. just for better clarity.. 

##########
File path: ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDTFGetSplits.java
##########
@@ -559,12 +566,22 @@ private SplitResult getSplits(JobConf job, TezWork work, Schema schema, Applicat
         // 4. Make location hints.
         SplitLocationInfo[] locations = makeLocationHints(hints.get(i));
 
+        // 5. populate info about llap daemons(to help client submit request and read data)
+        LlapDaemonInfo[] llapDaemonInfos = populateLlapDaemonInfos(job, locations);
+
+        // 6. Generate JWT for external clients if it's a cloud deployment
+        String jwt = "";
+        if (LlapUtil.isCloudDeployment()) {
+          JwtHelper jwtHelper = new JwtHelper(SessionState.getSessionConf());
+          jwt = jwtHelper.buildJwtForLlap(applicationId);

Review comment:
       leave a comment about which application's id we are injecting here and how the flow of jwt validation works.. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] prasanthj commented on a change in pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

Posted by GitBox <gi...@apache.org>.
prasanthj commented on a change in pull request #1418:
URL: https://github.com/apache/hive/pull/1418#discussion_r480371903



##########
File path: llap-common/src/java/org/apache/hadoop/hive/llap/security/DefaultJwtSharedSecretProvider.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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 org.apache.hadoop.hive.llap.security;
+
+import com.google.common.base.Preconditions;
+import io.jsonwebtoken.security.Keys;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.conf.HiveConf;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+import java.nio.charset.StandardCharsets;
+import java.security.Key;
+
+import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_DEPLOYMENT_SETUP_ENABLED;
+import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET;
+
+/**
+ * Default implementation of {@link JwtSecretProvider}.
+ *
+ * 1. It first tries to get shared secret from conf {@link HiveConf.ConfVars#LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET}
+ * using {@link Configuration#getPassword(String)}.
+ *
+ * 2. If not found, it tries to read from env var {@link #LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR}.
+ *
+ * If secret is not found even after 1) and 2), {@link #init(Configuration)} methods throws {@link NullPointerException}.
+ *
+ * It uses the same encryption and decryption secret which can be used to sign and verify JWT.
+ */
+public class DefaultJwtSharedSecretProvider implements JwtSecretProvider {
+
+  public static final String LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR =
+      "LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR";
+
+  private Key jwtEncryptionKey;
+
+  @Override public Key getEncryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public Key getDecryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public void init(final Configuration conf) {
+    char[] sharedSecret;
+    byte[] sharedSecretBytes = null;
+
+    // try getting secret from conf first
+    // if not found, get from env var - LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR
+    try {
+      sharedSecret = conf.getPassword(LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET.varname);
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to get password [hive.llap.external.client.cloud.jwt.shared.secret] - "
+          + e.getMessage(), e);
+    }
+    if (sharedSecret != null) {
+      ByteBuffer bb = StandardCharsets.UTF_8.encode(CharBuffer.wrap(sharedSecret));
+      sharedSecretBytes = new byte[bb.remaining()];
+      bb.get(sharedSecretBytes);
+    } else {
+      String sharedSecredFromEnv = System.getenv(LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR);
+      if (sharedSecredFromEnv != null) {
+        sharedSecretBytes = sharedSecredFromEnv.getBytes();
+      }
+    }
+
+    Preconditions.checkNotNull(sharedSecretBytes,

Review comment:
       nit: IllegalStateException is more appropriate I guess. check state instead? 

##########
File path: llap-common/src/java/org/apache/hadoop/hive/llap/security/DefaultJwtSharedSecretProvider.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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 org.apache.hadoop.hive.llap.security;
+
+import com.google.common.base.Preconditions;
+import io.jsonwebtoken.security.Keys;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.conf.HiveConf;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+import java.nio.charset.StandardCharsets;
+import java.security.Key;
+
+import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_DEPLOYMENT_SETUP_ENABLED;
+import static org.apache.hadoop.hive.conf.HiveConf.ConfVars.LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET;
+
+/**
+ * Default implementation of {@link JwtSecretProvider}.
+ *
+ * 1. It first tries to get shared secret from conf {@link HiveConf.ConfVars#LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET}
+ * using {@link Configuration#getPassword(String)}.
+ *
+ * 2. If not found, it tries to read from env var {@link #LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR}.
+ *
+ * If secret is not found even after 1) and 2), {@link #init(Configuration)} methods throws {@link NullPointerException}.
+ *
+ * It uses the same encryption and decryption secret which can be used to sign and verify JWT.
+ */
+public class DefaultJwtSharedSecretProvider implements JwtSecretProvider {
+
+  public static final String LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR =
+      "LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR";
+
+  private Key jwtEncryptionKey;
+
+  @Override public Key getEncryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public Key getDecryptionSecret() {
+    return jwtEncryptionKey;
+  }
+
+  @Override public void init(final Configuration conf) {
+    char[] sharedSecret;
+    byte[] sharedSecretBytes = null;
+
+    // try getting secret from conf first
+    // if not found, get from env var - LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR
+    try {
+      sharedSecret = conf.getPassword(LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET.varname);
+    } catch (IOException e) {
+      throw new RuntimeException("Unable to get password [hive.llap.external.client.cloud.jwt.shared.secret] - "
+          + e.getMessage(), e);
+    }
+    if (sharedSecret != null) {
+      ByteBuffer bb = StandardCharsets.UTF_8.encode(CharBuffer.wrap(sharedSecret));
+      sharedSecretBytes = new byte[bb.remaining()];
+      bb.get(sharedSecretBytes);
+    } else {
+      String sharedSecredFromEnv = System.getenv(LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_ENV_VAR);
+      if (sharedSecredFromEnv != null) {

Review comment:
       also check for empty string?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] ShubhamChaurasia commented on a change in pull request #1418: HIVE-24059: Llap external client - Initial changes for running in cloud environment

Posted by GitBox <gi...@apache.org>.
ShubhamChaurasia commented on a change in pull request #1418:
URL: https://github.com/apache/hive/pull/1418#discussion_r479977303



##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -4880,6 +4880,22 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
     LLAP_EXTERNAL_CLIENT_USE_HYBRID_CALENDAR("hive.llap.external.client.use.hybrid.calendar",
         false,
         "Whether to use hybrid calendar for parsing of data/timestamps."),
+
+    // confs for llap-external-client cloud deployment
+    LLAP_EXTERNAL_CLIENT_CLOUD_RPC_PORT("hive.llap.external.client.cloud.rpc.port", 30004,
+        "The LLAP daemon RPC port for external clients when llap is running in cloud environment."),
+    LLAP_EXTERNAL_CLIENT_CLOUD_OUTPUT_SERVICE_PORT("hive.llap.external.client.cloud.output.service.port", 30005,
+                "LLAP output service port when llap is running in cloud environment"),
+    LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET_PROVIDER(
+        "hive.llap.external.client.cloud.jwt.shared.secret.provider",
+        "org.apache.hadoop.hive.llap.security.ConfBasedJwtSharedSecretProvider",
+        "Shared secret provider to be used to sign JWT"),
+    LLAP_EXTERNAL_CLIENT_CLOUD_JWT_SHARED_SECRET("hive.llap.external.client.cloud.jwt.shared.secret",
+        "Let me give you this secret and you will get the access!",

Review comment:
       Done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org