imbajin commented on code in PR #2962:
URL: https://github.com/apache/hugegraph/pull/2962#discussion_r2909802340
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/auth/IpAuthHandler.java:
##########
@@ -65,7 +71,24 @@ private static String getClientIp(ChannelHandlerContext ctx)
{
}
private boolean isIpAllowed(String ip) {
- return allowedIps.isEmpty() || allowedIps.contains(ip);
+ Set<String> resolved = this.resolvedIps;
+ return resolved.isEmpty() || resolved.contains(ip);
+ }
+
+ private static Set<String> resolveAll(Set<String> entries) {
Review Comment:
⚠️ This change introduces new hostname-resolution behavior, but the PR still
has no regression coverage around it. Please add at least one test for a
hostname allowlist entry and one for a resolution failure/refresh scenario,
otherwise it will be very easy to regress this path again.
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/auth/IpAuthHandler.java:
##########
@@ -30,11 +33,14 @@
@ChannelHandler.Sharable
public class IpAuthHandler extends ChannelDuplexHandler {
+ // Retained for potential refresh of resolvedIps on membership changes
private final Set<String> allowedIps;
+ private volatile Set<String> resolvedIps;
private static volatile IpAuthHandler instance;
private IpAuthHandler(Set<String> allowedIps) {
this.allowedIps = Collections.unmodifiableSet(allowedIps);
+ this.resolvedIps = resolveAll(allowedIps);
Review Comment:
‼️ `resolvedIps` is only built once in the constructor, but `IpAuthHandler`
is still a JVM-wide singleton. `RaftEngine#changePeerList()` can replace the
peer set after startup, and hostname-based peers can also resolve to a
different IP later. In both cases this handler keeps the original resolved
addresses forever, so a valid PD peer can be blocked until the whole process
restarts. Please add a refresh path for membership/DNS changes, or defer
hostname resolution to validation time with a bounded cache/TTL.
--
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]
