smengcl commented on code in PR #10763:
URL: https://github.com/apache/ozone/pull/10763#discussion_r4280557601
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneCluster.java:
##########
@@ -714,6 +715,12 @@ private int configureS3Gateway(OzoneConfiguration conf,
PersistedPortState persi
if (!config.isS3gEnabled()) {
return -1;
}
+ // getS3Endpoint() advertises the HTTP listener, which a policy without
HTTP leaves unbound.
+ HttpConfig.Policy httpPolicy =
Review Comment:
Moving the HTTP-policy check here drops the existing validation for
`--no-s3g --recon`. With `ozone.http.policy=HTTPS_ONLY`,
`prepareConfiguration()` now accepts the configuration even though
`getReconEndpoint()` and the new readiness check use HTTP and Recon only opens
its HTTPS listener. Startup can therefore never satisfy that check.
I reproduced an earlier failure in the preparation stage too: with format
mode `ALWAYS`, an existing marker in the data directory is deleted before this
incompatible configuration fails. Restoring the shared check rejects the
setting before formatting and preserves the marker.
Please keep the `(config.isS3gEnabled() || config.isReconEnabled())` guard
in `configureLocalDefaults()` and restore the removed
`prepareConfigurationChecksHttpPolicyForRecon` test. This is the HTTPS-only
issue already referenced in the earlier review.
```diff
---
a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneCluster.java
+++
b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneCluster.java
@@ -465,6 +465,13 @@
}
private void configureLocalDefaults(OzoneConfiguration conf) throws
IOException {
+ if (config.isS3gEnabled() || config.isReconEnabled()) {
+ HttpConfig.Policy httpPolicy =
+ HttpConfig.Policy.fromString(conf.get(OZONE_HTTP_POLICY_KEY,
OZONE_HTTP_POLICY_DEFAULT));
+ if (httpPolicy != null && !httpPolicy.isHttpEnabled()) {
+ rejectUserConfigured(conf, OZONE_HTTP_POLICY_KEY, "HTTP_ONLY or
HTTP_AND_HTTPS");
+ }
+ }
conf.set(OZONE_METADATA_DIRS, metadataDir().toString());
// SCM and OM share one configuration and JVM, so the Jetty base dir is
a
// single service-neutral location; Jetty keeps per-context temp dirs.
@@ -714,12 +721,6 @@
PortAllocator portAllocator) throws IOException {
if (!config.isS3gEnabled()) {
return -1;
- }
- // getS3Endpoint() advertises the HTTP listener, which a policy without
HTTP leaves unbound.
- HttpConfig.Policy httpPolicy =
- HttpConfig.Policy.fromString(conf.get(OZONE_HTTP_POLICY_KEY,
OZONE_HTTP_POLICY_DEFAULT));
- if (httpPolicy != null && !httpPolicy.isHttpEnabled()) {
- rejectUserConfigured(conf, OZONE_HTTP_POLICY_KEY, "HTTP_ONLY or
HTTP_AND_HTTPS");
}
// getS3gPort() reads the port back off this listener.
setLocalOverride(conf, OZONE_S3G_HTTP_ENABLED_KEY, true);
```
--
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]
