feat: support secure Hubble monitoring targets#3094
Closed
imbajin wants to merge 3 commits into
Closed
Conversation
- exclude the PD secret key from generated toString output - preserve secret access for authentication behavior - cover the logging boundary in the PD core test suite
- derive the REST authority from the registered host and rest.port label - share IPv4 and IPv6 validation with service discovery - preserve the legacy discovery fallback for missing labels - cover additive DTO output and malformed address handling
- propagate explicit schema and serializer limits to PD graph configs - preserve values supplied by PD or callers with putIfAbsent - exclude aliases, implicit defaults, and unrelated sensitive settings - cover propagation and compatibility in the server unit suite
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds compatibility and security improvements to support Hubble’s native monitoring targets across HugeGraph Server and PD, while preserving existing authentication and service-discovery fallback behavior.
Changes:
- Redacts
PDConfig.secretKeyfromtoString()output while keeping runtime access intact. - Introduces validated Store REST target derivation (
rest.portlabel + store host) with IPv4/hostname/IPv6 handling and shares the logic with service discovery (with legacy fallback). - Ensures only explicitly configured
schema.cache_capacityandserializer.buffer_max_capacityare propagated into PD graph configs, without overriding values provided by PD/callers.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java | Registers the new GraphManagerConfigTest in the server unit suite. |
| hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java | Adds unit coverage for explicitly attaching local cache/buffer limits and preserving caller configs. |
| hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java | Attaches schema.cache_capacity and serializer.buffer_max_capacity only when explicitly configured locally. |
| hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/util/StoreRestAddressUtilTest.java | Adds tests for REST address construction/validation across IPv4/IPv6/URI forms and invalid inputs. |
| hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/StoreAPIStoreStatisticsTest.java | Verifies StoreStatistics.restAddress exposure behavior via the REST API model. |
| hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java | Includes the new REST-oriented tests in the PD REST suite. |
| hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java | Includes PDConfigTest in the PD core suite. |
| hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDConfigTest.java | Adds coverage to ensure PDConfig.toString() doesn’t leak the secret key. |
| hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/util/StoreRestAddressUtil.java | Introduces a shared, validated REST-address derivation helper for Store registrations. |
| hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/SDConfigService.java | Uses the shared REST-address derivation with a legacy gRPC-address fallback. |
| hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java | Exposes the derived REST address on StoreStatistics. |
| hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/config/PDConfig.java | Excludes secretKey from Lombok-generated toString(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
Author
|
Closing because this change was opened at the wrong publication stage. It must first be based on and reviewed in the hugegraph/hugegraph repository; an Apache upstream PR will only be considered after that review and CI are complete. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Provide the small HugeGraph Server and PD compatibility boundaries required by Hubble native monitoring while keeping existing authentication and discovery behavior intact.
Changes
PDConfig.secretKeyfrom generated log output without changing authentication accessStoreStatistics.restAddress, derived from the registered Store host and validatedrest.port, with IPv4/hostname/IPv6 handlingschema.cache_capacityandserializer.buffer_max_capacityinto PD graph configs, preserving caller/PD valuesVerification
git diff --checkpassed/v1/storesreturned the validated REST authority, Prometheus target discovery remained available, and four Server graphs started with the explicit cache limitsThis is intentionally limited to compatibility, secret redaction, and bounded local test configuration support; it does not add an alerting engine or production sizing policy.