This is an automated email from the ASF dual-hosted git repository.
ayushtkn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 828f668ee1a HIVE-29999: HiveCluster CR string fields should be
validated before being injected into k8s operator generated bash commands
(#6802)
828f668ee1a is described below
commit 828f668ee1a3c98e74356a9a1fdb66c33a1d9847
Author: Tanishq Chugh <[email protected]>
AuthorDate: Mon Sep 21 10:50:36 2026 +0530
HIVE-29999: HiveCluster CR string fields should be validated before being
injected into k8s operator generated bash commands (#6802)
---
.../crds/hiveclusters.hive.apache.org-v1.yml | 1 +
.../operator/dependent/HiveDependentResource.java | 103 +++++++++++++++++++++
.../dependent/HiveServer2DeploymentDependent.java | 1 +
.../operator/model/spec/DatabaseConfig.java | 2 +
.../operator/reconciler/HiveClusterReconciler.java | 47 +++++++++-
5 files changed, 151 insertions(+), 3 deletions(-)
diff --git
a/packaging/src/kubernetes/helm/hive-operator/crds/hiveclusters.hive.apache.org-v1.yml
b/packaging/src/kubernetes/helm/hive-operator/crds/hiveclusters.hive.apache.org-v1.yml
index 516e4309d28..1f5bd05e2ce 100644
---
a/packaging/src/kubernetes/helm/hive-operator/crds/hiveclusters.hive.apache.org-v1.yml
+++
b/packaging/src/kubernetes/helm/hive-operator/crds/hiveclusters.hive.apache.org-v1.yml
@@ -648,6 +648,7 @@ spec:
default: derby
description: "Database type: derby, mysql, postgres,
mssql,\
\ or oracle"
+ pattern: ^(derby|mysql|postgres|mssql|oracle)$
type: string
url:
description: JDBC connection URL
diff --git
a/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/dependent/HiveDependentResource.java
b/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/dependent/HiveDependentResource.java
index bb031b50ac2..a64440acab8 100644
---
a/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/dependent/HiveDependentResource.java
+++
b/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/dependent/HiveDependentResource.java
@@ -19,6 +19,10 @@
package org.apache.hive.kubernetes.operator.dependent;
+import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.security.MessageDigest;
import java.util.ArrayList;
@@ -26,6 +30,8 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.regex.Pattern;
+
import io.fabric8.kubernetes.api.model.AffinityBuilder;
import io.fabric8.kubernetes.api.model.Container;
import io.fabric8.kubernetes.api.model.ContainerBuilder;
@@ -49,7 +55,9 @@
import org.apache.hive.kubernetes.operator.autoscaling.HiveClusterAutoscaler;
import org.apache.hive.kubernetes.operator.model.HiveCluster;
import org.apache.hive.kubernetes.operator.model.spec.AutoscalingSpec;
+import org.apache.hive.kubernetes.operator.model.HiveClusterSpec;
import org.apache.hive.kubernetes.operator.model.spec.DatabaseConfig;
+import org.apache.hive.kubernetes.operator.model.spec.LlapSpec;
import org.apache.hive.kubernetes.operator.model.spec.SecretKeyRef;
import org.apache.hive.kubernetes.operator.model.spec.ProbeSpec;
@@ -74,6 +82,9 @@ public abstract class HiveDependentResource<R extends
HasMetadata,
private static final Logger LOG =
LoggerFactory.getLogger(HiveDependentResource.class);
+ /** Closed set of valid schematool -dbType values. */
+ private static final Pattern DB_TYPE_PATTERN =
Pattern.compile("derby|mysql|postgres|mssql|oracle");
+
protected static final String CONF_MOUNT_PATH = "/etc/hive/conf";
protected static final String HIVE_CONF_DIR = "/opt/hive/conf";
protected static final String EXT_JARS_PATH = "/tmp/ext-jars";
@@ -360,6 +371,96 @@ protected static String sha256(String... inputs) {
}
}
+ /**
+ * Validates CR-provided database fields before they are embedded into
+ * shell command lines (schematool -dbType in the schema-init Job) and
+ * into SERVICE_OPTS, which the image entrypoint expands into JVM arguments.
+ */
+ protected static void validateDatabaseConfig(DatabaseConfig db) {
+ if (!DB_TYPE_PATTERN.matcher(db.type()).matches()) {
+ throw new IllegalArgumentException(
+ "spec.metastore.database.type must be one of derby, mysql, postgres,
mssql, oracle");
+ }
+ validateOptValue("spec.metastore.database.url", db.url());
+ validateOptValue("spec.metastore.database.driver", db.driver());
+ validateOptValue("spec.metastore.database.username", db.username());
+ }
+
+ /**
+ * Validates CR-provided string fields before they are embedded into
+ * HiveServer2 SERVICE_OPTS and related env vars.
+ */
+ protected static void validateHiveServer2EmbeddedValues(HiveClusterSpec
spec) {
+ if (!spec.metastore().isEnabled()) {
+ validateOptValue("spec.metastore.externalUri",
spec.metastore().externalUri());
+ }
+ if (spec.tezAm().isEnabled()) {
+ validateOptValue("spec.zookeeper.quorum", spec.zookeeper().quorum());
+ }
+ spec.llapClusters().stream()
+ .filter(LlapSpec::isEnabled)
+ .forEach(llap -> validateOptValue("spec.llapClusters.serviceHosts",
llap.serviceHosts()));
+ }
+
+ /**
+ * Validates CR-provided string fields before they are embedded into
+ * LLAP / TezAM env vars of llap cluster.
+ */
+ public static void validateLlapEmbeddedValues(HiveClusterSpec spec, LlapSpec
llap) {
+ validateOptValue("spec.zookeeper.quorum", spec.zookeeper().quorum());
+ validateOptValue("spec.llapClusters.serviceHosts", llap.serviceHosts());
+ }
+
+ private static void validateOptValue(String field, String value) {
+ if (containsUnsafeShellChars(value) || (value != null &&
value.contains("$("))) {
+ throw new IllegalArgumentException(field + " must not contain
whitespace, quotes, backslashes, "
+ + "control characters or Kubernetes variable references");
+ }
+ }
+
+ /**
+ * Validates a CR-provided external JAR location before it is embedded
+ * into a bash download command.
+ */
+ protected static void validateJarUrl(String jarUrl) {
+ validateOptValue("external JAR location", jarUrl);
+
+ if (jarUrl != null && (jarUrl.startsWith("http://";) ||
jarUrl.startsWith("https://";))) {
+ // Normalizing parse: rejects malformed URLs early instead of
+ // letting wget interpret them, preventing the init containers
+ // from crashing and the spun-up pods from entering CrashLoopBackOff.
+ try {
+ URL url = new URI(jarUrl).toURL();
+
+ String host = url.getHost();
+ if (host == null || host.isEmpty()) {
+ throw new IllegalArgumentException("HTTP/HTTPS external JAR URL must
specify a host");
+ }
+
+ int port = url.getPort();
+ if (port != -1 && (port < 1 || port > 65535)) {
+ throw new IllegalArgumentException("HTTP/HTTPS external JAR URL has
invalid port");
+ }
+
+ } catch (MalformedURLException | URISyntaxException e) {
+ throw new IllegalArgumentException("Malformed HTTP/HTTPS external JAR
URL");
+ }
+ }
+ }
+
+ private static boolean containsUnsafeShellChars(String value) {
+ if (value == null) {
+ return false;
+ }
+ for (int i = 0; i < value.length(); i++) {
+ char c = value.charAt(i);
+ if (Character.isWhitespace(c) || c == '\'' || c == '"' || c == '\\' || c
== '`' || Character.isISOControl(c)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
/**
* Builds the database connection env vars: DB_DRIVER, DBPASSWORD
* (from SecretKeyRef), and SERVICE_OPTS with javax.jdo connection
@@ -367,6 +468,7 @@ protected static String sha256(String... inputs) {
* SchemaInitJobDependent.
*/
protected static List<EnvVar> buildDbEnvVars(DatabaseConfig db) {
+ validateDatabaseConfig(db);
List<EnvVar> envVars = new ArrayList<>();
envVars.add(new EnvVar("DB_DRIVER", db.type(), null));
@@ -511,6 +613,7 @@ protected static Container buildExternalJarsInitContainer(
cmd.append("export HADOOP_CONF_DIR=").append(CONF_MOUNT_PATH).append(" &&
");
for (String jarUrl : externalJars) {
+ validateJarUrl(jarUrl);
if (jarUrl.startsWith("http://";) || jarUrl.startsWith("https://";)) {
cmd.append("wget -q --tries=3 --waitretry=5 -P ").append(targetDir)
.append(" '").append(jarUrl).append("' && ");
diff --git
a/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/dependent/HiveServer2DeploymentDependent.java
b/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/dependent/HiveServer2DeploymentDependent.java
index aab6c890897..6bc6291fd1f 100644
---
a/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/dependent/HiveServer2DeploymentDependent.java
+++
b/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/dependent/HiveServer2DeploymentDependent.java
@@ -72,6 +72,7 @@ protected Deployment desired(HiveCluster hiveCluster,
Context<HiveCluster> context) {
HiveClusterSpec spec = hiveCluster.getSpec();
HiveServer2Spec hs2 = spec.hiveServer2();
+ validateHiveServer2EmbeddedValues(spec);
Map<String, String> selectorLabels =
Labels.selectorForComponent(hiveCluster, COMPONENT);
diff --git
a/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/model/spec/DatabaseConfig.java
b/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/model/spec/DatabaseConfig.java
index cd60f80f857..9e0a79d80d1 100644
---
a/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/model/spec/DatabaseConfig.java
+++
b/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/model/spec/DatabaseConfig.java
@@ -21,11 +21,13 @@
import com.fasterxml.jackson.annotation.JsonPropertyDescription;
import io.fabric8.generator.annotation.Default;
+import io.fabric8.generator.annotation.Pattern;
/** JDBC database connection configuration for the Hive Metastore backend. */
public record DatabaseConfig(
@JsonPropertyDescription("Database type: derby, mysql, postgres, mssql, or
oracle")
@Default("derby")
+ @Pattern("^(derby|mysql|postgres|mssql|oracle)$")
String type,
@JsonPropertyDescription("JDBC connection URL")
String url,
diff --git
a/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/reconciler/HiveClusterReconciler.java
b/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/reconciler/HiveClusterReconciler.java
index ec37d6820ad..557ebe99d54 100644
---
a/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/reconciler/HiveClusterReconciler.java
+++
b/packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/reconciler/HiveClusterReconciler.java
@@ -49,6 +49,7 @@
import org.apache.hive.kubernetes.operator.autoscaling.MetricsCache;
import org.apache.hive.kubernetes.operator.autoscaling.MetricsScraper;
import org.apache.hive.kubernetes.operator.autoscaling.PodMetrics;
+import org.apache.hive.kubernetes.operator.dependent.HiveDependentResource;
import org.apache.hive.kubernetes.operator.dependent.LlapResourceBuilder;
import org.apache.hive.kubernetes.operator.model.HiveCluster;
import org.apache.hive.kubernetes.operator.model.HiveClusterSpec;
@@ -72,6 +73,8 @@ public class HiveClusterReconciler
private static final Logger LOG =
LoggerFactory.getLogger(HiveClusterReconciler.class);
+ private static final String CONDITION_READY_LITERAL = "Ready";
+ private static final String RECONCILIATION_ERROR_LITERAL =
"ReconciliationError";
private volatile HiveClusterAutoscaler autoscaler;
private volatile BackgroundMetricsScraper bgScraper;
@@ -178,9 +181,11 @@ public UpdateControl<HiveCluster> reconcile(HiveCluster
resource, Context<HiveCl
? 2 : getMinScrapeInterval(resource.getSpec());
}
+ boolean workflowError = applyWorkflowDependentErrors(resource, context,
newStatus, existingStatus);
+
// --- Single exit point for status update ---
boolean statusNowChanged = !statusEqualsIgnoringTimestamps(existingStatus,
newStatus);
- if (!statusNowChanged && rescheduleSeconds == 0) {
+ if (!statusNowChanged && rescheduleSeconds == 0 && !workflowError) {
return UpdateControl.noUpdate();
}
resource.setStatus(newStatus);
@@ -217,7 +222,7 @@ public ErrorStatusUpdateControl<HiveCluster>
updateErrorStatus(HiveCluster resou
status.getConditions() != null ? status.getConditions() :
Collections.emptyList();
status.setConditions(List.of(
- buildCondition("Ready", "False", "ReconciliationError",
+ buildCondition(CONDITION_READY_LITERAL, "False",
RECONCILIATION_ERROR_LITERAL,
e.getMessage(), existingConditions)
));
status.setObservedGeneration(resource.getMetadata().getGeneration());
@@ -324,7 +329,7 @@ private HiveClusterStatus buildStatus(HiveCluster resource,
// Overall Ready condition
boolean allReady = schemaReady && metastoreReady && hs2Ready;
- conditions.add(buildCondition("Ready", allReady ? "True" : "False",
+ conditions.add(buildCondition(CONDITION_READY_LITERAL, allReady ? "True" :
"False",
allReady ? "AllComponentsReady" : "ComponentsNotReady",
allReady ? "All Hive components are ready" : "One or more components
are not ready",
existingConditions));
@@ -387,6 +392,41 @@ private <T extends HasMetadata> ComponentStatus
buildComponentStatus(
return cs;
}
+ /**
+ * When the managed workflow dependents incur failures, update the Ready
+ * condition with the incurred error, while preserving component conditions.
+ */
+ private boolean applyWorkflowDependentErrors(HiveCluster resource,
Context<HiveCluster> context,
+ HiveClusterStatus newStatus, HiveClusterStatus existingStatus) {
+ var workflowResult =
context.managedWorkflowAndDependentResourceContext().getWorkflowReconcileResult();
+ if (workflowResult.isEmpty() ||
!workflowResult.get().erroredDependentsExist()) {
+ return false;
+ }
+
+ Exception error =
workflowResult.get().getErroredDependents().values().iterator().next();
+ String errorMessage = error.getMessage();
+ LOG.error("Error reconciling HiveCluster: {}/{} - {}",
resource.getMetadata().getNamespace(),
+ resource.getMetadata().getName(), errorMessage, error);
+
+ List<Condition> existingConditions = existingStatus != null &&
existingStatus.getConditions() != null
+ ? existingStatus.getConditions() : Collections.emptyList();
+ boolean alreadyReported = existingConditions.stream()
+ .anyMatch(c -> CONDITION_READY_LITERAL.equals(c.getType())
+ && "False".equals(c.getStatus())
+ && RECONCILIATION_ERROR_LITERAL.equals(c.getReason())
+ && Objects.equals(errorMessage, c.getMessage()));
+
+ List<Condition> conditions = newStatus.getConditions();
+ if (conditions == null) {
+ conditions = new ArrayList<>();
+ newStatus.setConditions(conditions);
+ }
+ conditions.removeIf(c -> CONDITION_READY_LITERAL.equals(c.getType()));
+ conditions.add(buildCondition(CONDITION_READY_LITERAL, "False",
RECONCILIATION_ERROR_LITERAL,
+ errorMessage, existingConditions));
+ return !alreadyReported;
+ }
+
private Condition buildCondition(String type, String conditionStatus,
String reason, String message, List<Condition> existingConditions) {
@@ -612,6 +652,7 @@ private void reconcileLlapClusters(HiveCluster resource,
KubernetesClient client
if (!llapSpec.isEnabled()) {
continue;
}
+ HiveDependentResource.validateLlapEmbeddedValues(resource.getSpec(),
llapSpec);
desiredNames.add(llapSpec.name());
int replicas = resolveLlapReplicaCount(resource, llapSpec, ns,
clusterName);
