Skip to content

Commit

Permalink
Deprecate ArmeriaSettings.ip and add ArmeriaSettings.address (#4598)
Browse files Browse the repository at this point in the history
Motivation:

Spring allows Users to specify a hostname because Spring converts the input to an InetAddress.
So we need to also allow users to specify a hostname.

Modifications:

- adding ArmeriaSettings.address whose type is InetAddress
- deprecating ArmeriaSettings.ip

Result:

- Closes #4597
  • Loading branch information
kojilin authored Jan 12, 2023
1 parent 76044df commit 76c50fe
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ armeria:
ports:
- port: 0
protocol: HTTP
- ip: 127.0.0.1
- address: 127.0.0.1
port: 0
protocol: HTTP
- ip: 0.0.0.0
- address: 0.0.0.0
port: 0
protocol: HTTP

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ armeria:
ports:
- port: 0
protocol: HTTP
- ip: 127.0.0.1
- address: 127.0.0.1
port: 0
protocol: HTTP
- ip: 0.0.0.0
- address: 0.0.0.0
port: 0
protocol: HTTP
metrics-path: ''
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,12 @@ public static void configurePorts(ServerBuilder server, List<Port> ports) {
requireNonNull(server, "server");
requireNonNull(ports, "ports");
ports.forEach(p -> {
final String ip = p.getIp();
final String iface = p.getIface();
final int port = p.getPort();
final List<SessionProtocol> protocols = firstNonNull(p.getProtocols(),
ImmutableList.of(SessionProtocol.HTTP));

if (ip == null) {
final InetSocketAddress socketAddress = createSocketAddress(p);
if (socketAddress == null) {
if (iface == null) {
server.port(new ServerPort(port, protocols));
} else {
Expand All @@ -71,24 +70,42 @@ public static void configurePorts(ServerBuilder server, List<Port> ports) {
}
}
} else if (iface == null) {
if (NetUtil.isValidIpV4Address(ip) || NetUtil.isValidIpV6Address(ip)) {
final byte[] bytes = NetUtil.createByteArrayFromIpAddressString(ip);
try {
server.port(new ServerPort(new InetSocketAddress(
InetAddress.getByAddress(bytes), port), protocols));
} catch (UnknownHostException e) {
// Should never happen.
throw new Error(e);
}
} else {
throw new IllegalStateException("invalid IP address: " + ip);
}
server.port(new ServerPort(socketAddress, protocols));
} else {
throw new IllegalStateException("A port cannot have both IP and iface: " + p);
}
});
}

@Nullable
private static InetSocketAddress createSocketAddress(Port p) {
final InetAddress address = p.getAddress();
final String ip = p.getIp();
final int port = p.getPort();
if (ip != null && address != null) {
throw new IllegalStateException("A port cannot have both IP and address: " + p);
}
final InetSocketAddress targetAddress;
if (ip != null) {
if (NetUtil.isValidIpV4Address(ip) || NetUtil.isValidIpV6Address(ip)) {
final byte[] bytes = NetUtil.createByteArrayFromIpAddressString(ip);
try {
targetAddress = new InetSocketAddress(InetAddress.getByAddress(bytes), port);
} catch (UnknownHostException e) {
// Should never happen.
throw new Error(e);
}
} else {
throw new IllegalStateException("invalid IP address: " + ip);
}
} else if (address != null) {
targetAddress = new InetSocketAddress(address, port);
} else {
targetAddress = null;
}
return targetAddress;
}

/**
* Returns a newly created {@link Port}.
* {@code null} if the specified {@code code} is either {@code null} or a negative number.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.linecorp.armeria.spring;

import java.net.InetAddress;
import java.time.Duration;
import java.util.ArrayList;
import java.util.List;
Expand All @@ -24,6 +25,7 @@
import org.springframework.validation.annotation.Validated;

import com.codahale.metrics.json.MetricsModule;
import com.google.common.base.MoreObjects;
import com.google.common.collect.ImmutableList;

import com.linecorp.armeria.common.SessionProtocol;
Expand All @@ -48,7 +50,7 @@
* ports:
* - port: 8080
* protocol: HTTP
* - ip: 127.0.0.1
* - address: 127.0.0.1
* port: 8081
* protocol:HTTP
* - port: 8443
Expand Down Expand Up @@ -82,10 +84,19 @@ public class ArmeriaSettings {
public static class Port {
/**
* IP address to bind to. If not set, will bind to all addresses, e.g. {@code 0.0.0.0}.
*
* @deprecated Use {@link #address} instead.
*/
@Deprecated
@Nullable
private String ip;

/**
* Network address to bind to. If not set, will bind to all addresses, e.g. {@code 0.0.0.0}.
*/
@Nullable
private InetAddress address;

/**
* Network interface to bind to. If not set, will bind to the first detected network interface.
*/
Expand All @@ -105,20 +116,42 @@ public static class Port {

/**
* Returns the IP address that the {@link Server} uses.
*
* @deprecated Use {@link #getAddress()} instead.
*/
@Deprecated
@Nullable
public String getIp() {
return ip;
}

/**
* Registers an IP address that the {@link Server} uses.
*
* @deprecated Use {@link #setAddress(InetAddress)} instead.
*/
@Deprecated
public Port setIp(String ip) {
this.ip = ip;
return this;
}

/**
* Returns the network address that the {@link Server} uses.
*/
@Nullable
public InetAddress getAddress() {
return address;
}

/**
* Registers a network address that the {@link Server} uses.
*/
public Port setAddress(InetAddress address) {
this.address = address;
return this;
}

/**
* Returns the network interface that the {@link Server} uses.
*/
Expand Down Expand Up @@ -173,6 +206,17 @@ public Port setProtocol(SessionProtocol protocol) {
protocols = ImmutableList.of(protocol);
return this;
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this).omitNullValues()
.add("ip", ip)
.add("address", address)
.add("iface", iface)
.add("port", port)
.add("protocols", protocols)
.toString();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright 2023 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package com.linecorp.armeria.spring;

import static org.assertj.core.api.Assertions.assertThat;

import java.net.InetAddress;
import java.util.Collection;

import javax.inject.Inject;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.junit4.SpringRunner;

import com.linecorp.armeria.server.Server;
import com.linecorp.armeria.server.ServerPort;
import com.linecorp.armeria.spring.ArmeriaSettings.Port;
import com.linecorp.armeria.spring.DeprecatedIpTest.TestConfiguration;

/**
* Tests for keeping the behavior of deprecated {@link Port#getIp()}.
*/
@RunWith(SpringRunner.class)
@SpringBootTest(classes = TestConfiguration.class)
@ActiveProfiles({ "local", "deprecatedIpTest" })
@DirtiesContext
public class DeprecatedIpTest {

@SpringBootApplication
static class TestConfiguration {}

@Inject
private Server server;

@Test
public void testIpCanBeUsed() {
final Collection<ServerPort> serverPorts = server.activePorts().values();
for (ServerPort sp : serverPorts) {
final InetAddress address = sp.localAddress().getAddress();
if ("127.0.0.1".equals(address.getHostAddress())) {
assertThat(address.isLoopbackAddress()).isTrue();
} else {
// Setting 0.0.0.0 at properties
assertThat(address.isAnyLocalAddress()).isTrue();
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright 2023 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package com.linecorp.armeria.spring;

import static org.assertj.core.api.Assertions.assertThat;

import java.net.InetAddress;
import java.util.Collection;

import javax.inject.Inject;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.junit4.SpringRunner;

import com.linecorp.armeria.server.Server;
import com.linecorp.armeria.server.ServerPort;
import com.linecorp.armeria.spring.ArmeriaSettings.Port;
import com.linecorp.armeria.spring.LocalhostAddressTest.TestConfiguration;

/**
* Tests for keeping the behavior of {@link Port#getIp()}.
*/
@RunWith(SpringRunner.class)
@SpringBootTest(classes = TestConfiguration.class)
@ActiveProfiles({ "local", "localhostAddressTest" })
@DirtiesContext
public class LocalhostAddressTest {

@SpringBootApplication
static class TestConfiguration {}

@Inject
private Server server;

@Test
public void testLocalhostAddressCanBeUsed() {
final Collection<ServerPort> serverPorts = server.activePorts().values();
assertThat(serverPorts).hasSize(1);
for (ServerPort sp : serverPorts) {
final InetAddress address = sp.localAddress().getAddress();
assertThat(address.isLoopbackAddress()).isTrue();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ armeria:
ports:
- port: 0
protocol: HTTP
- ip: 127.0.0.1
- address: 127.0.0.1
port: 0
protocol: HTTP
- ip: 0.0.0.0
- address: 0.0.0.0
port: 0
protocol: HTTP
enable-auto-injection: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
armeria:
ports:
- ip: 127.0.0.1
port: 0
protocol: HTTP
- ip: 0.0.0.0
port: 0
protocol: HTTP
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
armeria:
ports:
- address: localhost
port: 0
protocol: HTTP

0 comments on commit 76c50fe

Please sign in to comment.