Copilot commented on code in PR #6802:
URL: https://github.com/apache/hive/pull/6802#discussion_r4051743544
##########
packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/dependent/HiveDependentResource.java:
##########
@@ -360,13 +369,82 @@ 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; got: " + db.type());
+ }
+ validateOptValue("spec.metastore.database.url", db.url());
+ validateOptValue("spec.metastore.database.driver", db.driver());
+ validateOptValue("spec.metastore.database.username", db.username());
Review Comment:
This guard is only applied to the metastore database fields, but other
CR-controlled values are still appended directly to `SERVICE_OPTS`:
`metastore.externalUri`, `llap.serviceHosts`, and `zookeeper.quorum` in
`HiveServer2DeploymentDependent.java:112-139`. That env var is later folded
into the JVM option string by the image entrypoint, so whitespace can add
unintended JVM arguments and Kubernetes `$(...)` references can still be
expanded. Apply the same safe-value validation to every value assembled into
`SERVICE_OPTS` (or avoid embedding raw CR text there).
##########
packaging/src/kubernetes/src/java/org/apache/hive/kubernetes/operator/reconciler/HiveClusterReconciler.java:
##########
@@ -387,6 +391,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);
Review Comment:
Validation failures can contain the full CR-provided JAR URL (the exceptions
in `validateJarUrl` append `jarUrl`). This path logs `errorMessage` and copies
it into the `Ready` condition, so credentials embedded in a URL are exposed in
operator logs/status and control characters can forge their contents. Keep
these errors field-only or redact the URL in every branch.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
