Skip to content

Add advertisedPlaintextPort to DruidNode for sidecar proxy support #19598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
saithal-confluent wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add advertisedPlaintextPort to DruidNode for sidecar proxy support #19598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
719c3b2
Add advertisedPlaintextPort to DruidNode for sidecar proxy support
saithal-confluent Jun 9, 2026
b98173f
Fix DruidNode constructor source compatibility
saithal-confluent Jun 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/main/java/org/apache/druid/rpc/ServiceLocation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public ServiceLocation(final String host, final int plaintextPort, final int tls
*/
public static ServiceLocation fromDruidNode(final DruidNode druidNode)
{
return new ServiceLocation(druidNode.getHost(), druidNode.getPlaintextPort(), druidNode.getTlsPort(), "");
return new ServiceLocation(druidNode.getHost(), druidNode.getAdvertisedPlaintextPort(), druidNode.getTlsPort(), "");
}

/**
Expand Down
63 changes: 55 additions & 8 deletions server/src/main/java/org/apache/druid/server/DruidNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ public class DruidNode
@JsonProperty
private Map<String, String> labels;

@JsonProperty
@Max(0xffff)
private int advertisedPlaintextPort = -1;

public DruidNode(
String serviceName,
String host,
Expand All @@ -110,7 +114,36 @@ public DruidNode(
boolean enableTlsPort
)
{
this(serviceName, host, bindOnHost, plaintextPort, null, tlsPort, enablePlaintextPort, enableTlsPort, null);
this(serviceName, host, bindOnHost, plaintextPort, null, tlsPort, enablePlaintextPort, enableTlsPort, null, null);
}

public DruidNode(
String serviceName,
String host,
boolean bindOnHost,
Integer plaintextPort,
Integer port,
Integer tlsPort,
Boolean enablePlaintextPort,
boolean enableTlsPort
)
{
this(serviceName, host, bindOnHost, plaintextPort, port, tlsPort, enablePlaintextPort, enableTlsPort, null, null);
}

public DruidNode(
String serviceName,
String host,
boolean bindOnHost,
Integer plaintextPort,
Integer port,
Integer tlsPort,
Boolean enablePlaintextPort,
boolean enableTlsPort,
Map<String, String> labels
)
{
this(serviceName, host, bindOnHost, plaintextPort, port, tlsPort, enablePlaintextPort, enableTlsPort, labels, null);
}

/**
Expand Down Expand Up @@ -139,7 +172,8 @@ public DruidNode(
@JacksonInject(useInput = OptBoolean.TRUE) @Named("tlsServicePort") @JsonProperty("tlsPort") Integer tlsPort,
@JsonProperty("enablePlaintextPort") Boolean enablePlaintextPort,
@JsonProperty("enableTlsPort") boolean enableTlsPort,
@JsonProperty("labels") @Nullable Map<String, String> labels
@JsonProperty("labels") @Nullable Map<String, String> labels,
@JsonProperty("advertisedPlaintextPort") Integer advertisedPlaintextPort

@FrankChen021 FrankChen021 Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve the labels-taking DruidNode constructor

Adding advertisedPlaintextPort to the existing @JsonCreator constructor removes the previous nine-argument overload that accepted labels. Existing source in DruidNodeTest still calls new DruidNode(..., labels), for example the equality and serde tests, and those calls no longer match any constructor, so the server module will not compile. Please keep a nine-argument overload that delegates with a null advertisedPlaintextPort, or update all local call sites while preserving source compatibility intentionally.

@saithal-confluent saithal-confluent Jun 19, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restored the 9-arg overload so existing call sites keep compiling. The new field is reachable via the 10-arg constructor instead of 9-arg one as it would make existing null-literal calls ambiguous.
Thanks for the review @FrankChen021 , could you please take a look again?

)
{
init(
Expand All @@ -150,7 +184,8 @@ public DruidNode(
tlsPort,
enablePlaintextPort == null || enablePlaintextPort.booleanValue(),
enableTlsPort,
labels
labels,
advertisedPlaintextPort
);
}

Expand All @@ -162,7 +197,8 @@ private void init(
Integer tlsPort,
boolean enablePlaintextPort,
boolean enableTlsPort,
Map<String, String> labels
Map<String, String> labels,
Integer advertisedPlaintextPort
)
{
Preconditions.checkNotNull(serviceName);
Expand Down Expand Up @@ -210,8 +246,12 @@ private void init(
}
}
this.plaintextPort = plainTextPort;
this.advertisedPlaintextPort = advertisedPlaintextPort != null && advertisedPlaintextPort > 0
? advertisedPlaintextPort
: this.plaintextPort;
} else {
this.plaintextPort = -1;
this.advertisedPlaintextPort = -1;
}
if (enableTlsPort) {
this.tlsPort = tlsPort;
Expand Down Expand Up @@ -276,9 +316,14 @@ public String getBuildRevision()
return buildRevision;
}

public int getAdvertisedPlaintextPort()
{
return advertisedPlaintextPort;
}

public DruidNode withService(String service)
{
return new DruidNode(service, host, bindOnHost, plaintextPort, tlsPort, enablePlaintextPort, enableTlsPort);
return new DruidNode(service, host, bindOnHost, plaintextPort, null, tlsPort, enablePlaintextPort, enableTlsPort, labels, advertisedPlaintextPort);
}

public String getServiceScheme()
Expand All @@ -292,10 +337,10 @@ public String getServiceScheme()
public String getHostAndPort()
{
if (enablePlaintextPort) {
if (plaintextPort < 0) {
if (advertisedPlaintextPort < 0) {
return HostAndPort.fromString(host).toString();
} else {
return HostAndPort.fromParts(host, plaintextPort).toString();
return HostAndPort.fromParts(host, advertisedPlaintextPort).toString();
}
}
return null;
Expand Down Expand Up @@ -359,6 +404,7 @@ public boolean equals(Object o)
enablePlaintextPort == druidNode.enablePlaintextPort &&
tlsPort == druidNode.tlsPort &&
enableTlsPort == druidNode.enableTlsPort &&
advertisedPlaintextPort == druidNode.advertisedPlaintextPort &&
Objects.equals(serviceName, druidNode.serviceName) &&
Objects.equals(host, druidNode.host) &&
Objects.equals(labels, druidNode.labels);
Expand All @@ -367,7 +413,7 @@ public boolean equals(Object o)
@Override
public int hashCode()
{
return Objects.hash(serviceName, host, port, plaintextPort, enablePlaintextPort, tlsPort, enableTlsPort, labels);
return Objects.hash(serviceName, host, port, plaintextPort, enablePlaintextPort, tlsPort, enableTlsPort, labels, advertisedPlaintextPort);
}

@Override
Expand All @@ -379,6 +425,7 @@ public String toString()
", bindOnHost=" + bindOnHost +
", port=" + port +
", plaintextPort=" + plaintextPort +
", advertisedPlaintextPort=" + advertisedPlaintextPort +
", enablePlaintextPort=" + enablePlaintextPort +
", tlsPort=" + tlsPort +
", enableTlsPort=" + enableTlsPort +
Expand Down
112 changes: 112 additions & 0 deletions server/src/test/java/org/apache/druid/server/DruidNodeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,118 @@ public void testDefaultsAndSanity()
Assert.assertEquals(-1, node.getTlsPort());
}

@Test
public void testAdvertisedPlaintextPort()
{
// When not set, advertisedPlaintextPort defaults to plaintextPort
DruidNode node = new DruidNode("test", "host", false, 8082, null, true, false);
Assert.assertEquals(8082, node.getPlaintextPort());
Assert.assertEquals(8082, node.getAdvertisedPlaintextPort());
Assert.assertEquals("host:8082", node.getHostAndPort());
Assert.assertEquals("host:8082", node.getHostAndPortToUse());

// When set, advertisedPlaintextPort overrides in getHostAndPort() but not getPlaintextPort()
node = new DruidNode("test", "host", false, 8082, null, 9443, true, false, null, 9443);
Assert.assertEquals(8082, node.getPlaintextPort());
Assert.assertEquals(9443, node.getAdvertisedPlaintextPort());
Assert.assertEquals("host:9443", node.getHostAndPort());
Assert.assertEquals("host:9443", node.getHostAndPortToUse());
// getPortToUse() still returns the bind port
Assert.assertEquals(8082, node.getPortToUse());
}

@Test
public void testAdvertisedPlaintextPortWithTls()
{
// advertisedPlaintextPort with TLS enabled — getHostAndPortToUse() prefers TLS
DruidNode node = new DruidNode("test", "host", false, 8082, null, 8443, true, true, null, 9443);
Assert.assertEquals(8082, node.getPlaintextPort());
Assert.assertEquals(9443, node.getAdvertisedPlaintextPort());
Assert.assertEquals(8443, node.getTlsPort());
Assert.assertEquals("host:9443", node.getHostAndPort());
Assert.assertEquals("host:8443", node.getHostAndTlsPort());
Assert.assertEquals("host:8443", node.getHostAndPortToUse());
}

@Test
public void testAdvertisedPlaintextPortDisabledPlaintext()
{
// When plaintext is disabled, advertisedPlaintextPort is -1 regardless
DruidNode node = new DruidNode("test", "host", false, null, null, 8443, false, true, null, 9443);
Assert.assertEquals(-1, node.getPlaintextPort());
Assert.assertEquals(-1, node.getAdvertisedPlaintextPort());
Assert.assertNull(node.getHostAndPort());
}

@Test
public void testAdvertisedPlaintextPortSerde() throws Exception
{
// Serialization roundtrip preserves advertisedPlaintextPort
DruidNode original = new DruidNode("service", "host", true, 8082, null, 5678, true, true, null, 9443);
DruidNode actual = mapper.readValue(mapper.writeValueAsString(original), DruidNode.class);
Assert.assertEquals(8082, actual.getPlaintextPort());
Assert.assertEquals(9443, actual.getAdvertisedPlaintextPort());
Assert.assertEquals(5678, actual.getTlsPort());
Assert.assertEquals("host:9443", actual.getHostAndPort());
}

@Test
public void testAdvertisedPlaintextPortBackwardCompatDeserialization() throws Exception
{
// Old JSON without advertisedPlaintextPort — should default to plaintextPort
String json = "{\n"
+ " \"service\":\"service\",\n"
+ " \"host\":\"host\",\n"
+ " \"plaintextPort\":8082,\n"
+ " \"enablePlaintextPort\":true,\n"
+ " \"enableTlsPort\":false\n"
+ "}\n";
DruidNode actual = mapper.readValue(json, DruidNode.class);
Assert.assertEquals(8082, actual.getPlaintextPort());
Assert.assertEquals(8082, actual.getAdvertisedPlaintextPort());
Assert.assertEquals("host:8082", actual.getHostAndPort());
}

@Test
public void testAdvertisedPlaintextPortDeserialization() throws Exception
{
// JSON with advertisedPlaintextPort set
String json = "{\n"
+ " \"service\":\"service\",\n"
+ " \"host\":\"host\",\n"
+ " \"plaintextPort\":8082,\n"
+ " \"advertisedPlaintextPort\":9443,\n"
+ " \"enablePlaintextPort\":true,\n"
+ " \"enableTlsPort\":false\n"
+ "}\n";
DruidNode actual = mapper.readValue(json, DruidNode.class);
Assert.assertEquals(8082, actual.getPlaintextPort());
Assert.assertEquals(9443, actual.getAdvertisedPlaintextPort());
Assert.assertEquals("host:9443", actual.getHostAndPort());
Assert.assertEquals("host:9443", actual.getHostAndPortToUse());
}

@Test
public void testAdvertisedPlaintextPortWithService()
{
DruidNode node = new DruidNode("test", "host", false, 8082, null, 9443, true, false, null, 9443);
DruidNode copy = node.withService("other");
Assert.assertEquals("other", copy.getServiceName());
Assert.assertEquals(8082, copy.getPlaintextPort());
Assert.assertEquals(9443, copy.getAdvertisedPlaintextPort());
}

@Test
public void testAdvertisedPlaintextPortEquality()
{
DruidNode a = new DruidNode("test", "host", false, 8082, null, 9443, true, false, null, 9443);
DruidNode b = new DruidNode("test", "host", false, 8082, null, 9443, true, false, null, 9443);
DruidNode c = new DruidNode("test", "host", false, 8082, null, true, false);
Assert.assertEquals(a, b);
Assert.assertNotEquals(a, c);
Assert.assertEquals(a.hashCode(), b.hashCode());
}

@Test(expected = IllegalArgumentException.class)
public void testConflictingPorts()
{
Expand Down