morningman opened a new pull request, #70472:
URL: https://github.com/apache/doris/pull/70472

   ### Background
   
   FE REST endpoints such as `get_load_state` forward follower requests to the 
master. Internal FE clients also serve checkpoint synchronization, manager 
operations and audit loading. HTTPS deployments need those clients to select a 
matching protocol, port and trust store.
   
   ### What problem does this PR solve?
   
   Issue Number: #70425
   Related PR: #60921 (original FE HTTPS support), #64697 (required follow-up 
fixes), #63918 (companion BE small-file support), #64670 (port helper already 
present)
   PR Type: Bugfix
   
   Problem Summary:
   
   Backport #60921 and its required companion fixes #64697 and #63918 to 
branch-4.1. An HTTPS request to a follower currently forwards to the master's 
HTTP port and returns an error envelope. The patch selects the HTTPS endpoint, 
trusts certificates from the FE HTTPS keystore, keeps BE stream-load redirects 
on HTTP, and allows BE small-file downloads to use the HTTPS port advertised by 
the FE.
   
   ### Details
   
   - **Change map:** 371 changed lines in production code and 303 in tests; no 
generated files or documentation. The production changes cover internal FE URLs 
and clients, FE-keystore trust, FE-to-BE redirect selection, audit loading, and 
the companion BE small-file retry.
   - **Frequency:** HTTPS requests that require follower-to-master forwarding 
fail when the HTTP and HTTPS listeners use different ports.
   - **Root cause:** The original forwarding code combines the incoming HTTPS 
scheme with `getMasterHttpPort()`, which advertises the HTTP port on this 
branch.
   - **Why existing tests missed it:** The forwarding unit-test shortcut uses 
the request's own endpoint. The upstream URL, client-selection and redirect 
tests cover the individual behaviors, but there is no real multi-FE HTTPS 
integration test in this backport.
   - **Fix:** Reuse `HttpURLUtil.buildInternalFeUrl()` and the upstream 
internal TLS client. Include #64697 so FE HTTPS trust does not depend on the 
separate MySQL truststore and HTTP BE requests keep using a plain client. 
Include #63918 because the heartbeat now advertises `https_port` to BEs.
   - **Other paths and branches:** The same patch is being prepared for 
branch-4.2. Master already contains the upstream fixes. This PR does not target 
branch-4.0 or branch-4.1.4.
   
   ### Testing
   
   - `mvn -B -pl fe-core checkstyle:check` (from `fe/`): passed, zero 
violations.
   - `bash run-fe-ut.sh --run 
'org.apache.doris.common.util.HttpURLUtilTest,org.apache.doris.common.util.InternalHttpsUtilsTest,org.apache.doris.httpv2.rest.RestBaseControllerTest,org.apache.doris.httpv2.rest.manager.HttpUtilsTest,org.apache.doris.system.HeartbeatMgrTest,org.apache.doris.nereids.minidump.MinidumpUtTest'`:
 passed, 20 tests reported, 19 passed and one existing skipped test; no 
failures or errors.
   - `build-support/clang-format.sh` with clang-format 16: passed.
   - `build-support/run-clang-tidy.sh --base HEAD --build-dir be/build_ASAN`: 
no warnings on the changed C++ lines. This branch predates the helper, so the 
master helper was run from a temporary file with its repository-root path set 
explicitly.
   - `git diff --cached --check`: passed.
   - `bash build.sh --be --output /tmp/doris-70425-build41 -j4`: configuration 
succeeded, but the full build failed in unchanged ANN/runtime-profile code 
because LLVM 22 diagnoses `__COUNTER__` as a C2y extension under `-Werror`. 
Full BE linking and runtime tests are not verified.
   - No live cluster, multi-FE HTTPS integration, or BE HTTPS download test was 
run. The header-hygiene helper is absent from this release branch.
   
   ### Notes for Reviewers
   
   Conflict resolution preserves the branch's existing FE metadata 
authentication token, log redaction and JMockit-based heartbeat tests. The 
existing `getHttpPort()` helper is retained once; upstream URL tests are 
appended to the existing token tests. In the BE retry, failure to truncate the 
partial download returns an error instead of continuing with an invalid 
temporary file.
   
   Review checks:
   
   - Goal and scope: the forwarding defect is addressed through the upstream 
shared URL/client path; companion changes prevent known regressions from a 
partial pick.
   - Concurrency and lifecycle: the upstream volatile, synchronized SSL-context 
cache and per-request client ownership are preserved; cached certificates 
require an FE restart after rotation.
   - Configuration: no new options or defaults. Internal FE URLs use the local 
`enable_https` and listener port, so FE protocol/port settings must be uniform 
and the HTTPS keystore must contain the relevant trusted certificates or CA 
chain.
   - Compatibility: no Thrift/protobuf IDs, persisted formats or transaction 
semantics change. Deploy the companion BE support before relying on HTTPS-port 
heartbeats during a rolling upgrade.
   - Parallel paths and conditions: FE forwarding, metadata synchronization and 
manager clients use the shared helpers; BE redirects and BE manager requests 
retain HTTP.
   - Tests and expected output: upstream unit tests are retained, with no 
handwritten regression output. Real follower/master HTTPS and BE HTTPS download 
integration tests remain unexecuted locally.
   - Observability and performance: existing exception logging is retained; 
SSL-context initialization remains cached, and plain HTTP does not construct 
the HTTPS client.
   - Data writes and FE/BE propagation: label-state and transaction logic are 
unchanged; the existing heartbeat port field carries the selected FE listener, 
paired with the BE fallback.
   
   ### Release note
   
   Fix FE HTTPS forwarding from followers to the master, including load-state 
queries. Internal FE communication and audit loading use the configured HTTPS 
listener and keystore; BE stream-load redirects continue to use HTTP, and 
small-file downloads support HTTPS-enabled FEs.
   
   ### Check List (For Author)
   
   - Behavior changed: Yes (see Details).
   - Does this need documentation: No new configuration; deployment assumptions 
are recorded above.
   - Pre-submission status: Partially verified: FE unit tests, Java style, C++ 
formatting and changed-line static analysis pass; full BE build is blocked by 
the local LLVM 22 warning above, and live HTTPS integration remains unverified.
   
   ### AI Disclosure
   
   - Tool: Codex (GPT-6).
   - Generated: Backport conflict resolutions, a BE error-propagation 
adjustment, and this description. Other code and tests originate in the linked 
upstream PRs.
   - Human review: Pending maintainer review.
   


-- 
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]

Reply via email to