Skip to content

Commit

Permalink
Use Port class instead of strings (#511)
Browse files Browse the repository at this point in the history
  • Loading branch information
TadCordle authored Jul 10, 2018
1 parent 9ac0362 commit 23777be
Show file tree
Hide file tree
Showing 22 changed files with 210 additions and 181 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ public void testSteps_forBuildToDockerRegistry()
.setMainClass("HelloWorld")
.setJavaArguments(Collections.singletonList("An argument."))
.setExposedPorts(
ExposedPortsParser.parse(
Arrays.asList("1000", "2000-2002/tcp", "3000/udp"), logger))
ExposedPortsParser.parse(Arrays.asList("1000", "2000-2002/tcp", "3000/udp")))
.setAllowHttp(true)
.build();

Expand All @@ -83,7 +82,7 @@ public void testSteps_forBuildToDockerRegistry()
new Command("docker", "inspect", imageReference).run(),
CoreMatchers.containsString(
" \"ExposedPorts\": {\n"
+ " \"1000\": {},\n"
+ " \"1000/tcp\": {},\n"
+ " \"2000/tcp\": {},\n"
+ " \"2001/tcp\": {},\n"
+ " \"2002/tcp\": {},\n"
Expand All @@ -104,8 +103,7 @@ public void testSteps_forBuildToDockerDaemon()
.setMainClass("HelloWorld")
.setJavaArguments(Collections.singletonList("An argument."))
.setExposedPorts(
ExposedPortsParser.parse(
Arrays.asList("1000", "2000-2002/tcp", "3000/udp"), logger))
ExposedPortsParser.parse(Arrays.asList("1000", "2000-2002/tcp", "3000/udp")))
.build();

Path cacheDirectory = temporaryCacheDirectory.newFolder().toPath();
Expand All @@ -120,7 +118,7 @@ public void testSteps_forBuildToDockerDaemon()
new Command("docker", "inspect", "testdocker").run(),
CoreMatchers.containsString(
" \"ExposedPorts\": {\n"
+ " \"1000\": {},\n"
+ " \"1000/tcp\": {},\n"
+ " \"2000/tcp\": {},\n"
+ " \"2001/tcp\": {},\n"
+ " \"2002/tcp\": {},\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.cloud.tools.jib.configuration.CacheConfiguration;
import com.google.cloud.tools.jib.configuration.LayerConfiguration;
import com.google.cloud.tools.jib.configuration.Port;
import com.google.cloud.tools.jib.image.ImageReference;
import com.google.cloud.tools.jib.image.json.BuildableManifestTemplate;
import com.google.cloud.tools.jib.image.json.V22ManifestTemplate;
Expand Down Expand Up @@ -48,7 +49,7 @@ public static class Builder {
private ImmutableList<String> javaArguments = ImmutableList.of();
private ImmutableList<String> jvmFlags = ImmutableList.of();
private ImmutableMap<String, String> environmentMap = ImmutableMap.of();
private ImmutableList<String> exposedPorts = ImmutableList.of();
private ImmutableList<Port> exposedPorts = ImmutableList.of();
private Class<? extends BuildableManifestTemplate> targetFormat = V22ManifestTemplate.class;
@Nullable private CacheConfiguration applicationLayersCacheConfiguration;
@Nullable private CacheConfiguration baseImageLayersCacheConfiguration;
Expand Down Expand Up @@ -123,7 +124,7 @@ public Builder setEnvironment(@Nullable Map<String, String> environmentMap) {
return this;
}

public Builder setExposedPorts(@Nullable List<String> exposedPorts) {
public Builder setExposedPorts(@Nullable List<Port> exposedPorts) {
if (exposedPorts != null) {
Preconditions.checkArgument(!exposedPorts.contains(null));
this.exposedPorts = ImmutableList.copyOf(exposedPorts);
Expand Down Expand Up @@ -279,7 +280,7 @@ public static Builder builder(BuildLogger buildLogger) {
private final ImmutableList<String> javaArguments;
private final ImmutableList<String> jvmFlags;
private final ImmutableMap<String, String> environmentMap;
private final ImmutableList<String> exposedPorts;
private final ImmutableList<Port> exposedPorts;
private final Class<? extends BuildableManifestTemplate> targetFormat;
@Nullable private final CacheConfiguration applicationLayersCacheConfiguration;
@Nullable private final CacheConfiguration baseImageLayersCacheConfiguration;
Expand All @@ -299,7 +300,7 @@ private BuildConfiguration(
ImmutableList<String> javaArguments,
ImmutableList<String> jvmFlags,
ImmutableMap<String, String> environmentMap,
ImmutableList<String> exposedPorts,
ImmutableList<Port> exposedPorts,
Class<? extends BuildableManifestTemplate> targetFormat,
@Nullable CacheConfiguration applicationLayersCacheConfiguration,
@Nullable CacheConfiguration baseImageLayersCacheConfiguration,
Expand Down Expand Up @@ -396,7 +397,7 @@ public ImmutableMap<String, String> getEnvironment() {
return environmentMap;
}

public ImmutableList<String> getExposedPorts() {
public ImmutableList<Port> getExposedPorts() {
return exposedPorts;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright 2018 Google LLC. All rights reserved.
*
* Licensed 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
*
* http://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.google.cloud.tools.jib.configuration;

import java.util.Objects;

/** Holds port number and protocol for an exposed port. */
public class Port {

/** Represents the protocol portion of the port. */
public enum Protocol {
TCP("tcp"),
UDP("udp");

private final String stringRepresentation;

Protocol(String stringRepresentation) {
this.stringRepresentation = stringRepresentation;
}

@Override
public String toString() {
return stringRepresentation;
}
}

private final int port;
private final Protocol protocol;

public Port(int port, Protocol protocol) {
this.port = port;
this.protocol = protocol;
}

public int getPort() {
return port;
}

public Protocol getProtocol() {
return protocol;
}

@Override
public boolean equals(Object other) {
if (other == this) {
return true;
}
if (other == null || !(other instanceof Port)) {
return false;
}
Port otherPort = (Port) other;
return port == otherPort.port && protocol == otherPort.protocol;
}

@Override
public int hashCode() {
return Objects.hash(port, protocol);
}

@Override
public String toString() {
return port + "/" + protocol;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -16,41 +16,37 @@

package com.google.cloud.tools.jib.frontend;

import com.google.cloud.tools.jib.builder.BuildLogger;
import com.google.cloud.tools.jib.configuration.PortsWithProtocol;
import com.google.common.annotations.VisibleForTesting;
import com.google.cloud.tools.jib.configuration.Port;
import com.google.cloud.tools.jib.configuration.Port.Protocol;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

/** Utility for parsing exposed ports from plugin configuration */
public class ExposedPortsParser {

/**
* Pattern used for parsing information out of exposed port configurations.
*
* <p>Examples: 100, 200-210, 1000/tcp, 2000/udp, 500-600/tcp
* <p>Example matches: 100, 200-210, 1000/tcp, 2000/udp, 500-600/tcp
*/
private static final Pattern portPattern = Pattern.compile("(\\d+)(?:-(\\d+))?(/tcp|/udp)?");
private static final Pattern portPattern = Pattern.compile("(\\d+)(?:-(\\d+))?(?:/(tcp|udp))?");

/**
* TODO: Return list of {@link PortsWithProtocol}s instead of strings
* Converts/validates a list of strings representing port ranges to an expanded list of {@link
* Port}s.
*
* <p>Converts/validates a list of ports with ranges to an expanded form without ranges.
*
* <p>Example: {@code ["1000/tcp", "2000-2002/tcp"] -> ["1000/tcp", "2000/tcp", "2001/tcp",
* "2002/tcp"]}
* <p>For example: ["1000", "2000-2002"] will expand to a list of {@link Port}s with the port
* numbers [1000, 2000, 2001, 2002]
*
* @param ports the list of port numbers/ranges
* @param buildLogger used to log warning messages
* @return the ports as a list of integers
* @return the ports as a list of {@link Port}
* @throws NumberFormatException if any of the ports are in an invalid format or out of range
*/
@VisibleForTesting
public static ImmutableList<String> parse(List<String> ports, BuildLogger buildLogger)
throws NumberFormatException {
ImmutableList.Builder<String> result = new ImmutableList.Builder<>();
public static ImmutableList<Port> parse(List<String> ports) throws NumberFormatException {
ImmutableList.Builder<Port> result = new ImmutableList.Builder<>();

for (String port : ports) {
Matcher matcher = portPattern.matcher(port);
Expand All @@ -70,7 +66,8 @@ public static ImmutableList<String> parse(List<String> ports, BuildLogger buildL
if (!Strings.isNullOrEmpty(matcher.group(2))) {
max = Integer.parseInt(matcher.group(2));
}
String protocol = matcher.group(3);
Protocol protocol =
Protocol.UDP.toString().equals(matcher.group(3)) ? Protocol.UDP : Protocol.TCP;

// Error if configured as 'max-min' instead of 'min-max'
if (min > max) {
Expand All @@ -80,14 +77,12 @@ public static ImmutableList<String> parse(List<String> ports, BuildLogger buildL

// Warn for possibly invalid port numbers
if (min < 1 || max > 65535) {
// TODO: Add details/use HelpfulSuggestions for these warnings
buildLogger.warn("Port number '" + port + "' is out of usual range (1-65535).");
throw new NumberFormatException(
"Port number '" + port + "' is out of usual range (1-65535).");
}

// Add all numbers in range to list
String portString = (protocol == null ? "" : protocol);
for (int portNum = min; portNum <= max; portNum++) {
result.add(portNum + portString);
for (int portNumber = min; portNumber <= max; portNumber++) {
result.add(new Port(portNumber, protocol));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.google.cloud.tools.jib.image;

import com.google.cloud.tools.jib.configuration.Port;
import com.google.common.collect.ImmutableList;
import java.util.List;
import java.util.Map;
Expand All @@ -31,7 +32,7 @@ public static class Builder<T extends Layer> {

private ImmutableList<String> entrypoint = ImmutableList.of();
private ImmutableList<String> javaArguments = ImmutableList.of();
private ImmutableList<String> exposedPorts = ImmutableList.of();
private ImmutableList<Port> exposedPorts = ImmutableList.of();

/**
* Sets the environment with a map from environment variable names to values.
Expand Down Expand Up @@ -94,11 +95,10 @@ public Builder<T> setJavaArguments(List<String> javaArguments) {
/**
* Sets the items in the "ExposedPorts" field in the container configuration.
*
* @param exposedPorts the map of exposed ports to add, with the key in the format it would
* appear in the configuration json (e.g. "portNum/tcp")
* @param exposedPorts the list of exposed ports to add
* @return this
*/
public Builder<T> setExposedPorts(ImmutableList<String> exposedPorts) {
public Builder<T> setExposedPorts(ImmutableList<Port> exposedPorts) {
this.exposedPorts = exposedPorts;
return this;
}
Expand Down Expand Up @@ -142,14 +142,14 @@ public static <T extends Layer> Builder<T> builder() {
private final ImmutableList<String> javaArguments;

/** Ports that the container listens on. */
private final ImmutableList<String> exposedPorts;
private final ImmutableList<Port> exposedPorts;

private Image(
ImageLayers<T> layers,
ImmutableList<String> environment,
ImmutableList<String> entrypoint,
ImmutableList<String> javaArguments,
ImmutableList<String> exposedPorts) {
ImmutableList<Port> exposedPorts) {
this.layers = layers;
this.environmentBuilder = environment;
this.entrypoint = entrypoint;
Expand All @@ -169,7 +169,7 @@ public ImmutableList<String> getJavaArguments() {
return javaArguments;
}

public ImmutableList<String> getExposedPorts() {
public ImmutableList<Port> getExposedPorts() {
return exposedPorts;
}

Expand Down
Loading

0 comments on commit 23777be

Please sign in to comment.