SecurePortResolver clean-up (#711)

* SecurePortResolver clean-up

* SecurePortResolver clean-up tests

* trying to fix 2.0.1-SNAPSHOT in IT

* no need for getters

* any sane implementation does not need a check against isDebugLevel...

* removed the bean reference

* reverted two accidental bumps

* make ServicePortSecureResolver as a non-bean
This commit is contained in:
erabii
2021-02-02 09:36:21 -05:00
committed by GitHub
parent 6cebd1d2e5
commit 696627ee47
8 changed files with 194 additions and 224 deletions

View File

@@ -1,128 +0,0 @@
/*
* Copyright 2013-2019 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
*
* 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 org.springframework.cloud.kubernetes.fabric8.discovery;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
/**
* 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;
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 && this.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 this.serviceName;
}
public Map<String, String> getServiceLabels() {
return this.serviceLabels;
}
public Map<String, String> getServiceAnnotations() {
return this.serviceAnnotations;
}
public Integer getPort() {
return this.port;
}
}
}

View File

@@ -57,7 +57,7 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
private final KubernetesDiscoveryProperties properties;
private final DefaultIsServicePortSecureResolver isServicePortSecureResolver;
private final ServicePortSecureResolver servicePortSecureResolver;
private final KubernetesClientServicesFunction kubernetesClientServicesFunction;
@@ -73,17 +73,17 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
KubernetesClientServicesFunction kubernetesClientServicesFunction) {
this(client, kubernetesDiscoveryProperties, kubernetesClientServicesFunction,
new DefaultIsServicePortSecureResolver(kubernetesDiscoveryProperties));
new ServicePortSecureResolver(kubernetesDiscoveryProperties));
}
KubernetesDiscoveryClient(KubernetesClient client, KubernetesDiscoveryProperties kubernetesDiscoveryProperties,
KubernetesClientServicesFunction kubernetesClientServicesFunction,
DefaultIsServicePortSecureResolver isServicePortSecureResolver) {
ServicePortSecureResolver servicePortSecureResolver) {
this.client = client;
this.properties = kubernetesDiscoveryProperties;
this.kubernetesClientServicesFunction = kubernetesClientServicesFunction;
this.isServicePortSecureResolver = isServicePortSecureResolver;
this.servicePortSecureResolver = servicePortSecureResolver;
}
public KubernetesClient getClient() {
@@ -171,7 +171,7 @@ public class KubernetesDiscoveryClient implements DiscoveryClient {
EndpointPort endpointPort = findEndpointPort(s);
instances.add(new KubernetesServiceInstance(instanceId, serviceId, endpointAddress.getIp(),
endpointPort.getPort(), endpointMetadata,
this.isServicePortSecureResolver.resolve(new DefaultIsServicePortSecureResolver.Input(
this.servicePortSecureResolver.resolve(new ServicePortSecureResolver.Input(
endpointPort.getPort(), service.getMetadata().getName(),
service.getMetadata().getLabels(), service.getMetadata().getAnnotations()))));
}

View File

@@ -46,12 +46,6 @@ import org.springframework.context.annotation.Configuration;
@AutoConfigureAfter({ Fabric8AutoConfiguration.class })
public class KubernetesDiscoveryClientAutoConfiguration {
@Bean
@ConditionalOnMissingBean
public DefaultIsServicePortSecureResolver isServicePortSecureResolver(KubernetesDiscoveryProperties properties) {
return new DefaultIsServicePortSecureResolver(properties);
}
@Bean
public KubernetesClientServicesFunction servicesFunction(KubernetesDiscoveryProperties properties) {
if (properties.getServiceLabels().isEmpty()) {
@@ -96,10 +90,9 @@ public class KubernetesDiscoveryClientAutoConfiguration {
@ConditionalOnMissingBean
public KubernetesDiscoveryClient kubernetesDiscoveryClient(KubernetesClient client,
KubernetesDiscoveryProperties properties,
KubernetesClientServicesFunction kubernetesClientServicesFunction,
DefaultIsServicePortSecureResolver isServicePortSecureResolver) {
KubernetesClientServicesFunction kubernetesClientServicesFunction) {
return new KubernetesDiscoveryClient(client, properties, kubernetesClientServicesFunction,
isServicePortSecureResolver);
new ServicePortSecureResolver(properties));
}
}

View File

@@ -0,0 +1,103 @@
/*
* Copyright 2013-2019 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
*
* 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 org.springframework.cloud.kubernetes.fabric8.discovery;
import java.util.Collections;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
class ServicePortSecureResolver {
private static final Log LOG = LogFactory.getLog(ServicePortSecureResolver.class);
private static final Set<String> TRUTHY_STRINGS = Stream.of("true", "on", "yes", "1").collect(Collectors.toSet());
private final KubernetesDiscoveryProperties properties;
ServicePortSecureResolver(KubernetesDiscoveryProperties properties) {
this.properties = properties;
}
/**
* <p>
* Returns true if any of the following conditions apply.
* <p>
* <ul>
* <li>service contains a label named 'secured' that is truthy</li>
* <li>service contains an annotation named 'secured' that is truthy</li>
* <li>the port is one of the known ports used for secure communication</li>
* </ul>
*
*/
boolean resolve(Input input) {
String securedLabelValue = input.serviceLabels.getOrDefault("secured", "false");
if (TRUTHY_STRINGS.contains(securedLabelValue)) {
LOG.debug("Considering service with name: " + input.serviceName + " and port " + input.port
+ " is secure since the service contains a true value for the 'secured' label");
return true;
}
String securedAnnotationValue = input.serviceAnnotations.getOrDefault("secured", "false");
if (TRUTHY_STRINGS.contains(securedAnnotationValue)) {
LOG.debug("Considering service with name: " + input.serviceName + " and port " + input.port
+ " is secure since the service contains a true value for the 'secured' annotation");
return true;
}
if (input.port != null && this.properties.getKnownSecurePorts().contains(input.port)) {
LOG.debug("Considering service with name: " + input.serviceName + " and port " + input.port
+ " is secure due to the port being a known https port");
return true;
}
return false;
}
static final 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 ? Collections.emptyMap() : serviceLabels;
this.serviceAnnotations = serviceAnnotations == null ? Collections.emptyMap() : serviceAnnotations;
}
}
}

View File

@@ -1,74 +0,0 @@
/*
* Copyright 2013-2019 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
*
* 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 org.springframework.cloud.kubernetes.fabric8.discovery;
import java.util.HashMap;
import org.junit.Test;
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
import static org.assertj.core.api.Assertions.assertThat;
public class DefaultIsServicePortSecureResolverTest {
@Test
public void testPortNumbersOnly() {
final KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties();
properties.getKnownSecurePorts().add(12345);
final DefaultIsServicePortSecureResolver sut = new DefaultIsServicePortSecureResolver(properties);
assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(null, "dummy"))).isFalse();
assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(8080, "dummy"))).isFalse();
assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(1234, "dummy"))).isFalse();
assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(443, "dummy"))).isTrue();
assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(8443, "dummy"))).isTrue();
assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(12345, "dummy"))).isTrue();
}
@Test
public void testLabelsAndAnnotations() {
final DefaultIsServicePortSecureResolver sut = new DefaultIsServicePortSecureResolver(
new KubernetesDiscoveryProperties());
assertThat(
sut.resolve(new DefaultIsServicePortSecureResolver.Input(8080, "dummy", new HashMap<String, String>() {
{
put("secured", "true");
put("other", "value");
}
}, new HashMap<>()))).isTrue();
assertThat(
sut.resolve(new DefaultIsServicePortSecureResolver.Input(1234, "dummy", new HashMap<String, String>() {
{
put("other", "value");
put("secured", "1");
}
}, new HashMap<>()))).isTrue();
assertThat(sut.resolve(new DefaultIsServicePortSecureResolver.Input(4321, "dummy", new HashMap<>(),
new HashMap<String, String>() {
{
put("other1", "value1");
put("secured", "yes");
put("other2", "value2");
}
}))).isTrue();
}
}

View File

@@ -69,7 +69,7 @@ public class KubernetesDiscoveryClientFilterMetadataTest {
private KubernetesDiscoveryProperties properties;
@Mock
private DefaultIsServicePortSecureResolver isServicePortSecureResolver;
private ServicePortSecureResolver isServicePortSecureResolver;
@Mock
private KubernetesDiscoveryProperties.Metadata metadata;

View File

@@ -100,7 +100,7 @@ public class KubernetesDiscoveryClientTest {
properties.getMetadata().setAddAnnotations(false);
final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties,
KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties));
KubernetesClient::services, new ServicePortSecureResolver(properties));
final List<ServiceInstance> instances = discoveryClient.getInstances("endpoint");
@@ -141,7 +141,7 @@ public class KubernetesDiscoveryClientTest {
properties.setPrimaryPortName("http_tcp");
final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties,
KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties));
KubernetesClient::services, new ServicePortSecureResolver(properties));
final List<ServiceInstance> instances = discoveryClient.getInstances("endpoint");
@@ -175,7 +175,7 @@ public class KubernetesDiscoveryClientTest {
properties.setServiceLabels(labels);
final KubernetesDiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties,
KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties));
KubernetesClient::services, new ServicePortSecureResolver(properties));
final List<Endpoints> result_endpoints = discoveryClient.getEndPointsList("endpoint");
@@ -218,7 +218,7 @@ public class KubernetesDiscoveryClientTest {
properties.getMetadata().setAddLabels(false);
final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties,
KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties));
KubernetesClient::services, new ServicePortSecureResolver(properties));
final List<ServiceInstance> instances = discoveryClient.getInstances("endpoint");
@@ -244,7 +244,7 @@ public class KubernetesDiscoveryClientTest {
final KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties();
final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties,
KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties));
KubernetesClient::services, new ServicePortSecureResolver(properties));
final List<String> services = discoveryClient.getServices();
@@ -274,7 +274,7 @@ public class KubernetesDiscoveryClientTest {
{
put("label", "value");
}
}), new DefaultIsServicePortSecureResolver(properties));
}), new ServicePortSecureResolver(properties));
final List<String> services = discoveryClient.getServices();
@@ -341,7 +341,7 @@ public class KubernetesDiscoveryClientTest {
properties.setAllNamespaces(true);
final DiscoveryClient discoveryClient = new KubernetesDiscoveryClient(mockClient, properties,
KubernetesClient::services, new DefaultIsServicePortSecureResolver(properties));
KubernetesClient::services, new ServicePortSecureResolver(properties));
final List<ServiceInstance> instances = discoveryClient.getInstances("endpoint");

View File

@@ -0,0 +1,76 @@
/*
* Copyright 2013-2019 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
*
* 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 org.springframework.cloud.kubernetes.fabric8.discovery;
import java.util.Collections;
import java.util.Map;
import org.junit.Test;
import org.springframework.cloud.kubernetes.commons.discovery.KubernetesDiscoveryProperties;
import static org.assertj.core.api.Assertions.assertThat;
public class ServicePortSecureResolverTest {
private static final Map<String, String> SECURED_TRUE_MAP = Collections.singletonMap("secured", "true");
private static final Map<String, String> SECURED_1_MAP = Collections.singletonMap("secured", "1");
private static final Map<String, String> SECURED_YES_MAP = Collections.singletonMap("secured", "yes");
private static final Map<String, String> SECURED_ON_MAP = Collections.singletonMap("secured", "on");
private static final ServicePortSecureResolver.Input SECURED_TRUE = new ServicePortSecureResolver.Input(8080,
"dummy", SECURED_TRUE_MAP, Collections.emptyMap());
private static final ServicePortSecureResolver.Input SECURED_1 = new ServicePortSecureResolver.Input(1234, "dummy",
SECURED_1_MAP, Collections.emptyMap());
private static final ServicePortSecureResolver.Input SECURED_YES = new ServicePortSecureResolver.Input(4321,
"dummy", SECURED_YES_MAP, Collections.emptyMap());
private static final ServicePortSecureResolver.Input SECURED_ON = new ServicePortSecureResolver.Input(4321, "dummy",
SECURED_ON_MAP, Collections.emptyMap());
@Test
public void testPortNumbersOnly() {
KubernetesDiscoveryProperties properties = new KubernetesDiscoveryProperties();
properties.getKnownSecurePorts().add(12345);
ServicePortSecureResolver secureResolver = new ServicePortSecureResolver(properties);
assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(null, "dummy"))).isFalse();
assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(8080, "dummy"))).isFalse();
assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(1234, "dummy"))).isFalse();
assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(443, "dummy"))).isTrue();
assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(8443, "dummy"))).isTrue();
assertThat(secureResolver.resolve(new ServicePortSecureResolver.Input(12345, "dummy"))).isTrue();
}
@Test
public void testLabelsAndAnnotations() {
ServicePortSecureResolver secureResolver = new ServicePortSecureResolver(new KubernetesDiscoveryProperties());
assertThat(secureResolver.resolve(SECURED_TRUE)).isTrue();
assertThat(secureResolver.resolve(SECURED_1)).isTrue();
assertThat(secureResolver.resolve(SECURED_YES)).isTrue();
assertThat(secureResolver.resolve(SECURED_ON)).isTrue();
}
}