Set KubernetesServiceInstance.secure based on sane defaults
Fixes: #272
This commit is contained in:
committed by
Spencer Gibb
parent
3409ad81a5
commit
eb73e05ccb
@@ -0,0 +1,120 @@
|
||||
/*
|
||||
* Copyright 2013-2018 the original author or authors.
|
||||
*
|
||||
* 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 org.springframework.cloud.kubernetes.discovery;
|
||||
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
|
||||
import java.util.HashMap;
|
||||
import java.util.HashSet;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
/**
|
||||
* TODO break up into delegates if the implementation get's more complicated
|
||||
* <p>
|
||||
* Returns true if one of the following conditions apply:
|
||||
* <p>
|
||||
* spring.cloud.kubernetes.discovery.secured has been set to true
|
||||
* the service contains a label or an annotation named 'secured' that is truthy
|
||||
* the port is one of the known ports used for secure communication
|
||||
*/
|
||||
class DefaultIsServicePortSecureResolver {
|
||||
|
||||
private static final Log log = LogFactory.getLog(DefaultIsServicePortSecureResolver.class);
|
||||
|
||||
|
||||
private static final Set<String> TRUTHY_STRINGS = new HashSet<String>() {{
|
||||
add("true");
|
||||
add("on");
|
||||
add("yes");
|
||||
add("1");
|
||||
}};
|
||||
|
||||
private final KubernetesDiscoveryProperties properties;
|
||||
|
||||
public DefaultIsServicePortSecureResolver(KubernetesDiscoveryProperties properties) {
|
||||
this.properties = properties;
|
||||
}
|
||||
|
||||
boolean resolve(Input input) {
|
||||
final String securedLabelValue = input.getServiceLabels().getOrDefault("secured", "false");
|
||||
if (TRUTHY_STRINGS.contains(securedLabelValue)) {
|
||||
if (log.isDebugEnabled()) {
|
||||
log.debug("Considering service with name: " + input.getServiceName() + " and port " + input.getPort()
|
||||
+ " is secure since the service contains a true value for the 'secured' label");
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
final String securedAnnotationValue = input.getServiceAnnotations().getOrDefault("secured", "false");
|
||||
if (TRUTHY_STRINGS.contains(securedAnnotationValue)) {
|
||||
if (log.isDebugEnabled()) {
|
||||
log.debug("Considering service with name: " + input.getServiceName() + " and port " + input.getPort()
|
||||
+ " is secure since the service contains a true value for the 'secured' annotation");
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
if (input.getPort() != null && properties.getKnownSecurePorts().contains(input.getPort())) {
|
||||
if (log.isDebugEnabled()) {
|
||||
log.debug("Considering service with name: " + input.getServiceName() + " and port " + input.getPort()
|
||||
+ " is secure due to the port being a known https port");
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
static class Input {
|
||||
private final Integer port;
|
||||
private final String serviceName;
|
||||
private final Map<String, String> serviceLabels;
|
||||
private final Map<String, String> serviceAnnotations;
|
||||
|
||||
//used only for testing
|
||||
Input(Integer port, String serviceName) {
|
||||
this(port, serviceName, null, null);
|
||||
}
|
||||
|
||||
Input(Integer port, String serviceName,
|
||||
Map<String, String> serviceLabels, Map<String, String> serviceAnnotations) {
|
||||
this.port = port;
|
||||
this.serviceName = serviceName;
|
||||
this.serviceLabels = serviceLabels == null ? new HashMap<>() : serviceLabels;
|
||||
this.serviceAnnotations = serviceAnnotations == null ? new HashMap<>() : serviceAnnotations;
|
||||
}
|
||||
|
||||
public String getServiceName() {
|
||||
return serviceName;
|
||||
}
|
||||
|
||||
public Map<String, String> getServiceLabels() {
|
||||
return serviceLabels;
|
||||
}
|
||||
|
||||
public Map<String, String> getServiceAnnotations() {
|
||||
return serviceAnnotations;
|
||||
}
|
||||
|
||||
public Integer getPort() {
|
||||
return port;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -48,6 +48,8 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
|
||||
|
||||
private KubernetesClient client;
|
||||
private final KubernetesDiscoveryProperties properties;
|
||||
private final DefaultIsServicePortSecureResolver isServicePortSecureResolver;
|
||||
|
||||
private final SpelExpressionParser parser = new SpelExpressionParser();
|
||||
private final SimpleEvaluationContext evalCtxt = SimpleEvaluationContext
|
||||
.forReadOnlyDataBinding()
|
||||
@@ -55,10 +57,18 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
|
||||
.build();
|
||||
|
||||
public KubernetesDiscoveryClient(KubernetesClient client,
|
||||
KubernetesDiscoveryProperties kubernetesDiscoveryProperties) {
|
||||
KubernetesDiscoveryProperties kubernetesDiscoveryProperties) {
|
||||
|
||||
this(client, kubernetesDiscoveryProperties, new DefaultIsServicePortSecureResolver(kubernetesDiscoveryProperties));
|
||||
}
|
||||
|
||||
KubernetesDiscoveryClient(KubernetesClient client,
|
||||
KubernetesDiscoveryProperties kubernetesDiscoveryProperties,
|
||||
DefaultIsServicePortSecureResolver isServicePortSecureResolver) {
|
||||
|
||||
this.client = client;
|
||||
this.properties = kubernetesDiscoveryProperties;
|
||||
this.isServicePortSecureResolver = isServicePortSecureResolver;
|
||||
}
|
||||
|
||||
public KubernetesClient getClient() {
|
||||
@@ -120,12 +130,21 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
|
||||
}
|
||||
|
||||
List<EndpointAddress> addresses = s.getAddresses();
|
||||
for (EndpointAddress a : addresses) {
|
||||
for (EndpointAddress endpointAddress : addresses) {
|
||||
final EndpointPort endpointPort =
|
||||
s.getPorts().stream().findFirst().orElseThrow(IllegalStateException::new);
|
||||
instances.add(new KubernetesServiceInstance(serviceId,
|
||||
a,
|
||||
s.getPorts().stream().findFirst().orElseThrow(IllegalStateException::new),
|
||||
endpointAddress,
|
||||
endpointPort,
|
||||
endpointMetadata,
|
||||
false));
|
||||
isServicePortSecureResolver.resolve(
|
||||
new DefaultIsServicePortSecureResolver.Input(
|
||||
endpointPort.getPort(),
|
||||
service.getMetadata().getName(),
|
||||
service.getMetadata().getLabels(),
|
||||
service.getMetadata().getAnnotations()
|
||||
)
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,7 +18,6 @@
|
||||
package org.springframework.cloud.kubernetes.discovery;
|
||||
|
||||
import io.fabric8.kubernetes.client.KubernetesClient;
|
||||
|
||||
import org.springframework.boot.autoconfigure.AutoConfigureBefore;
|
||||
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
|
||||
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
|
||||
@@ -35,12 +34,19 @@ import org.springframework.context.annotation.Configuration;
|
||||
CommonsClientAutoConfiguration.class, })
|
||||
public class KubernetesDiscoveryClientAutoConfiguration {
|
||||
|
||||
@Bean
|
||||
@ConditionalOnMissingBean
|
||||
public DefaultIsServicePortSecureResolver isServicePortSecureResolver(KubernetesDiscoveryProperties properties) {
|
||||
return new DefaultIsServicePortSecureResolver(properties);
|
||||
}
|
||||
|
||||
@Bean
|
||||
@ConditionalOnMissingBean
|
||||
@ConditionalOnProperty(name = "spring.cloud.kubernetes.discovery.enabled",matchIfMissing = true)
|
||||
public KubernetesDiscoveryClient kubernetesDiscoveryClient(KubernetesClient client,
|
||||
KubernetesDiscoveryProperties properties) {
|
||||
return new KubernetesDiscoveryClient(client, properties);
|
||||
public KubernetesDiscoveryClient kubernetesDiscoveryClient(
|
||||
KubernetesClient client, KubernetesDiscoveryProperties properties,
|
||||
DefaultIsServicePortSecureResolver isServicePortSecureResolver) {
|
||||
return new KubernetesDiscoveryClient(client, properties, isServicePortSecureResolver);
|
||||
}
|
||||
|
||||
@Bean
|
||||
|
||||
@@ -21,6 +21,11 @@ import org.springframework.beans.factory.annotation.Value;
|
||||
import org.springframework.boot.context.properties.ConfigurationProperties;
|
||||
import org.springframework.core.style.ToStringCreator;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
|
||||
@ConfigurationProperties("spring.cloud.kubernetes.discovery")
|
||||
public class KubernetesDiscoveryProperties {
|
||||
|
||||
@@ -34,6 +39,13 @@ public class KubernetesDiscoveryProperties {
|
||||
/** SpEL expression to filter services. */
|
||||
private String filter;
|
||||
|
||||
|
||||
/** Set the port numbers that are considered secure and use HTTPS */
|
||||
private Set<Integer> knownSecurePorts = new HashSet<Integer>() {{
|
||||
add(443);
|
||||
add(8443);
|
||||
}};
|
||||
|
||||
private Metadata metadata = new Metadata();
|
||||
|
||||
public boolean isEnabled() {
|
||||
@@ -60,6 +72,14 @@ public class KubernetesDiscoveryProperties {
|
||||
this.filter = filter;
|
||||
}
|
||||
|
||||
public Set<Integer> getKnownSecurePorts() {
|
||||
return knownSecurePorts;
|
||||
}
|
||||
|
||||
public void setKnownSecurePorts(Set<Integer> knownSecurePorts) {
|
||||
this.knownSecurePorts = knownSecurePorts;
|
||||
}
|
||||
|
||||
public Metadata getMetadata() {
|
||||
return metadata;
|
||||
}
|
||||
|
||||
@@ -28,75 +28,38 @@ import spock.lang.Specification
|
||||
|
||||
class KubernetesDiscoveryClientTest extends Specification {
|
||||
|
||||
private static KubernetesMockServer mockServer = new KubernetesMockServer()
|
||||
private static KubernetesClient mockClient
|
||||
private static KubernetesMockServer mockServer = new KubernetesMockServer()
|
||||
private static KubernetesClient mockClient
|
||||
|
||||
|
||||
def setupSpec() {
|
||||
mockServer.init()
|
||||
mockClient = mockServer.createClient()
|
||||
def setupSpec() {
|
||||
mockServer.init()
|
||||
mockClient = mockServer.createClient()
|
||||
|
||||
//Configure the kubernetes master url to point to the mock server
|
||||
System.setProperty(Config.KUBERNETES_MASTER_SYSTEM_PROPERTY, mockClient.getConfiguration().getMasterUrl())
|
||||
System.setProperty(Config.KUBERNETES_TRUST_CERT_SYSTEM_PROPERTY, "true")
|
||||
System.setProperty(Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false")
|
||||
System.setProperty(Config.KUBERNETES_AUTH_TRYSERVICEACCOUNT_SYSTEM_PROPERTY, "false")
|
||||
}
|
||||
//Configure the kubernetes master url to point to the mock server
|
||||
System.setProperty(Config.KUBERNETES_MASTER_SYSTEM_PROPERTY, mockClient.getConfiguration().getMasterUrl())
|
||||
System.setProperty(Config.KUBERNETES_TRUST_CERT_SYSTEM_PROPERTY, "true")
|
||||
System.setProperty(Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false")
|
||||
System.setProperty(Config.KUBERNETES_AUTH_TRYSERVICEACCOUNT_SYSTEM_PROPERTY, "false")
|
||||
}
|
||||
|
||||
def cleanupSpec() {
|
||||
mockServer.destroy();
|
||||
}
|
||||
def cleanupSpec() {
|
||||
mockServer.destroy()
|
||||
}
|
||||
|
||||
def "Should be able to handle endpoints single address"() {
|
||||
given:
|
||||
mockServer.expect().get().withPath("/api/v1/namespaces/test/endpoints/endpoint").andReturn(200, new EndpointsBuilder()
|
||||
.withNewMetadata()
|
||||
.withName("endpoint")
|
||||
.endMetadata()
|
||||
.addNewSubset()
|
||||
.addNewAddress()
|
||||
.withIp("ip1")
|
||||
.endAddress()
|
||||
.addNewPort("http",80,"TCP")
|
||||
.endSubset()
|
||||
.build()).once()
|
||||
and:
|
||||
mockServer.expect().get().withPath("/api/v1/namespaces/test/services/endpoint").andReturn(200, new ServiceBuilder()
|
||||
.withNewMetadata()
|
||||
.withName("endpoint")
|
||||
.withLabels(new HashMap<String, String>() {{
|
||||
put("l", "v")
|
||||
}})
|
||||
.endMetadata()
|
||||
.build()).once()
|
||||
|
||||
DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, new KubernetesDiscoveryProperties())
|
||||
when:
|
||||
List<ServiceInstance> instances = discoveryClient.getInstances("endpoint")
|
||||
then:
|
||||
instances != null
|
||||
instances.size() == 1
|
||||
instances.find({s -> s.host == "ip1"})
|
||||
}
|
||||
|
||||
|
||||
|
||||
def "Should be able to handle endpoints multiple addresses"() {
|
||||
given:
|
||||
mockServer.expect().get().withPath("/api/v1/namespaces/test/endpoints/endpoint").andReturn(200, new EndpointsBuilder()
|
||||
.withNewMetadata()
|
||||
.withName("endpoint")
|
||||
.endMetadata()
|
||||
.addNewSubset()
|
||||
.addNewAddress()
|
||||
.withIp("ip1")
|
||||
.endAddress()
|
||||
.addNewAddress()
|
||||
.withIp("ip2")
|
||||
.endAddress()
|
||||
.addNewPort("http",80,"TCP")
|
||||
.endSubset()
|
||||
.build()).once()
|
||||
def "Should be able to handle endpoints single address"() {
|
||||
given:
|
||||
mockServer.expect().get().withPath("/api/v1/namespaces/test/endpoints/endpoint").andReturn(200, new EndpointsBuilder()
|
||||
.withNewMetadata()
|
||||
.withName("endpoint")
|
||||
.endMetadata()
|
||||
.addNewSubset()
|
||||
.addNewAddress()
|
||||
.withIp("ip1")
|
||||
.endAddress()
|
||||
.addNewPort("http",80,"TCP")
|
||||
.endSubset()
|
||||
.build()).once()
|
||||
|
||||
and:
|
||||
mockServer.expect().get().withPath("/api/v1/namespaces/test/services/endpoint").andReturn(200, new ServiceBuilder()
|
||||
@@ -108,14 +71,60 @@ class KubernetesDiscoveryClientTest extends Specification {
|
||||
.endMetadata()
|
||||
.build()).once()
|
||||
|
||||
DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, new KubernetesDiscoveryProperties())
|
||||
when:
|
||||
List<ServiceInstance> instances = discoveryClient.getInstances("endpoint")
|
||||
then:
|
||||
instances != null
|
||||
instances.size() == 2
|
||||
instances.find({s -> s.host == "ip1"})
|
||||
instances.find({s -> s.host == "ip2"})
|
||||
final properties = new KubernetesDiscoveryProperties()
|
||||
DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(
|
||||
mockClient, properties, new DefaultIsServicePortSecureResolver(properties))
|
||||
|
||||
}
|
||||
when:
|
||||
List<ServiceInstance> instances = discoveryClient.getInstances("endpoint")
|
||||
|
||||
then:
|
||||
instances != null
|
||||
instances.size() == 1
|
||||
instances.find({s -> s.host == "ip1" && !s.secure})
|
||||
}
|
||||
|
||||
|
||||
|
||||
def "Should be able to handle endpoints multiple addresses"() {
|
||||
given:
|
||||
mockServer.expect().get().withPath("/api/v1/namespaces/test/endpoints/endpoint").andReturn(200, new EndpointsBuilder()
|
||||
.withNewMetadata()
|
||||
.withName("endpoint")
|
||||
.endMetadata()
|
||||
.addNewSubset()
|
||||
.addNewAddress()
|
||||
.withIp("ip1")
|
||||
.endAddress()
|
||||
.addNewAddress()
|
||||
.withIp("ip2")
|
||||
.endAddress()
|
||||
.addNewPort("https",443,"TCP")
|
||||
.endSubset()
|
||||
.build()).once()
|
||||
|
||||
and:
|
||||
mockServer.expect().get().withPath("/api/v1/namespaces/test/services/endpoint").andReturn(200, new ServiceBuilder()
|
||||
.withNewMetadata()
|
||||
.withName("endpoint")
|
||||
.withLabels(new HashMap<String, String>() {{
|
||||
put("l", "v")
|
||||
}})
|
||||
.endMetadata()
|
||||
.build()).once()
|
||||
|
||||
final properties = new KubernetesDiscoveryProperties()
|
||||
DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(
|
||||
mockClient, properties, new DefaultIsServicePortSecureResolver(properties))
|
||||
|
||||
when:
|
||||
List<ServiceInstance> instances = discoveryClient.getInstances("endpoint")
|
||||
|
||||
then:
|
||||
instances != null
|
||||
instances.size() == 2
|
||||
instances.find({s -> s.host == "ip1" && s.secure})
|
||||
instances.find({s -> s.host == "ip2" && s.secure})
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
/*
|
||||
* Copyright 2013-2018 the original author or authors.
|
||||
*
|
||||
* 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 org.springframework.cloud.kubernetes.discovery;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
import java.util.HashMap;
|
||||
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
public class DefaultIsServicePortSecureResolverTest {
|
||||
|
||||
|
||||
|
||||
@Test
|
||||
public void testPortNumbersOnly() {
|
||||
final KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties();
|
||||
properties.getKnownSecurePorts().add(12345);
|
||||
|
||||
final DefaultIsServicePortSecureResolver sut = new DefaultIsServicePortSecureResolver(properties);
|
||||
|
||||
assertFalse(sut.resolve(new DefaultIsServicePortSecureResolver.Input(null, "dummy")));
|
||||
assertFalse(sut.resolve(new DefaultIsServicePortSecureResolver.Input(8080, "dummy")));
|
||||
assertFalse(sut.resolve(new DefaultIsServicePortSecureResolver.Input(1234, "dummy")));
|
||||
|
||||
assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input(443, "dummy")));
|
||||
assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input(8443, "dummy")));
|
||||
assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input(12345, "dummy")));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLabelsAndAnnotations() {
|
||||
final DefaultIsServicePortSecureResolver sut
|
||||
= new DefaultIsServicePortSecureResolver(new KubernetesDiscoveryProperties());
|
||||
|
||||
assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input(
|
||||
8080,
|
||||
"dummy",
|
||||
new HashMap<String, String>() {{
|
||||
put("secured", "true");
|
||||
put("other", "value");
|
||||
}},
|
||||
new HashMap<>()))
|
||||
);
|
||||
assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input(
|
||||
1234,
|
||||
"dummy",
|
||||
new HashMap<String, String>() {{
|
||||
put("other", "value");
|
||||
put("secured", "1");
|
||||
}},
|
||||
new HashMap<>()))
|
||||
);
|
||||
assertTrue(sut.resolve(new DefaultIsServicePortSecureResolver.Input(
|
||||
4321,
|
||||
"dummy",
|
||||
new HashMap<>(),
|
||||
new HashMap<String, String>() {{
|
||||
put("other1", "value1");
|
||||
put("secured", "yes");
|
||||
put("other2", "value2");
|
||||
}}))
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -58,6 +58,9 @@ public class KubernetesDiscoveryClientFilterMetadataTest {
|
||||
@Mock
|
||||
private KubernetesDiscoveryProperties properties;
|
||||
|
||||
@Mock
|
||||
private DefaultIsServicePortSecureResolver isServicePortSecureResolver;
|
||||
|
||||
@Mock
|
||||
private KubernetesDiscoveryProperties.Metadata metadata;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user