From a277bf1d1b47e6f01ac97d9596d6a66777ca8e05 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 16 Nov 2018 11:16:22 -0500 Subject: [PATCH] Moves metadata properties to an inner class. This makes the properties grouped better (rather than in the name) and easier to read. properties.getMetadata().isAddLabels() vs properties.isEnabledAdditionOfLabelsAsMetadata() --- .../discovery/KubernetesDiscoveryClient.java | 15 +- .../KubernetesDiscoveryProperties.java | 188 +++++++++--------- ...etesDiscoveryClientFilterMetadataTest.java | 71 ++++--- 3 files changed, 147 insertions(+), 127 deletions(-) diff --git a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClient.java b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClient.java index f85ea6e5..dce420d4 100644 --- a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClient.java +++ b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClient.java @@ -21,10 +21,8 @@ import io.fabric8.kubernetes.api.model.EndpointPort; import io.fabric8.kubernetes.api.model.EndpointSubset; import io.fabric8.kubernetes.api.model.Endpoints; import io.fabric8.kubernetes.api.model.Service; -import io.fabric8.kubernetes.api.model.ServicePort; import io.fabric8.kubernetes.client.KubernetesClient; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -87,28 +85,29 @@ public class KubernetesDiscoveryClient implements DiscoveryClient { final Service service = client.services().withName(serviceId).get(); final Map serviceMetadata = new HashMap<>(); - if(properties.isEnabledAdditionOfLabelsAsMetadata()) { + KubernetesDiscoveryProperties.Metadata metadataProps = properties.getMetadata(); + if(metadataProps.isAddLabels()) { serviceMetadata.putAll( getMapWithPrefixedKeys( - service.getMetadata().getLabels(), properties.getLabelKeysPrefix()) + service.getMetadata().getLabels(), metadataProps.getLabelsPrefix()) ); } - if(properties.isEnabledAdditionOfAnnotationsAsMetadata()) { + if(metadataProps.isAddAnnotations()) { serviceMetadata.putAll( getMapWithPrefixedKeys( - service.getMetadata().getAnnotations(), properties.getAnnotationKeysPrefix()) + service.getMetadata().getAnnotations(), metadataProps.getAnnotationsPrefix()) ); } for (EndpointSubset s : subsets) { // Extend the service metadata map with per-endpoint port information (if requested) Map endpointMetadata = new HashMap<>(serviceMetadata); - if(properties.isEnabledAdditionOfPortsAsMetadata()) { + if(metadataProps.isAddPorts()) { Map ports = s.getPorts().stream() .filter(port -> !StringUtils.isEmpty(port.getName())) .collect(toMap(EndpointPort::getName, port -> Integer.toString(port.getPort()))); endpointMetadata.putAll( - getMapWithPrefixedKeys(ports, properties.getPortKeysPrefix()) + getMapWithPrefixedKeys(ports, metadataProps.getPortsPrefix()) ); } diff --git a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryProperties.java b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryProperties.java index 228c1ec9..dfd89ae4 100644 --- a/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryProperties.java +++ b/spring-cloud-kubernetes-discovery/src/main/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryProperties.java @@ -20,56 +20,22 @@ package org.springframework.cloud.kubernetes.discovery; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.cloud.client.serviceregistry.AutoServiceRegistrationProperties; +import org.springframework.core.style.ToStringCreator; @ConfigurationProperties("spring.cloud.kubernetes.discovery") public class KubernetesDiscoveryProperties extends AutoServiceRegistrationProperties { + /** If Kubernetes Discovery is enabled. */ private boolean enabled = true; + /** The service name of the local instance. */ @Value("${spring.application.name:unknown}") private String serviceName = "unknown"; - /** - * SpEL expression to filter services - **/ + /** SpEL expression to filter services. */ private String filter; - /** - * When set, the Kubernetes labels of the services will be included as metadata - * of the returned ServiceInstance - */ - private boolean enabledAdditionOfLabelsAsMetadata = true; - - /** - * When enabledAdditionOfLabelsAsMetadata is set, then the value labelKeysPrefix - * will be used as a prefix to the key names in the metadata map - */ - private String labelKeysPrefix; - - - /** - * When set, the Kubernetes annotations of the services will be included as metadata - * of the returned ServiceInstance - */ - private boolean enabledAdditionOfAnnotationsAsMetadata = true; - - /** - * When enabledAdditionOfAnnotationsAsMetadata is set, then the value annotationKeysPrefix - * will be used as a prefix to the key names in the metadata map - */ - private String annotationKeysPrefix; - - /** - * When set, any named Kubernetes service ports will be included as metadata - * of the returned ServiceInstance - */ - private boolean enabledAdditionOfPortsAsMetadata = true; - - /** - * When enabledAdditionOfPortsAsMetadata is set, then the value portKeysPrefix - * will be used as a prefix to the key names in the metadata map - */ - private String portKeysPrefix = "port."; + private Metadata metadata = new Metadata(); public boolean isEnabled() { return enabled; @@ -83,6 +49,10 @@ public class KubernetesDiscoveryProperties extends AutoServiceRegistrationProper return serviceName; } + public void setServiceName(String serviceName) { + this.serviceName = serviceName; + } + public String getFilter() { return filter; } @@ -91,61 +61,101 @@ public class KubernetesDiscoveryProperties extends AutoServiceRegistrationProper this.filter = filter; } - public boolean isEnabledAdditionOfLabelsAsMetadata() { - return enabledAdditionOfLabelsAsMetadata; + public Metadata getMetadata() { + return metadata; } - public void setEnabledAdditionOfLabelsAsMetadata(boolean enabledAdditionOfLabelsAsMetadata) { - this.enabledAdditionOfLabelsAsMetadata = enabledAdditionOfLabelsAsMetadata; - } - - public String getLabelKeysPrefix() { - return labelKeysPrefix; - } - - public void setLabelKeysPrefix(String labelKeysPrefix) { - this.labelKeysPrefix = labelKeysPrefix; - } - - public boolean isEnabledAdditionOfAnnotationsAsMetadata() { - return enabledAdditionOfAnnotationsAsMetadata; - } - - public void setEnabledAdditionOfAnnotationsAsMetadata( - boolean enabledAdditionOfAnnotationsAsMetadata) { - this.enabledAdditionOfAnnotationsAsMetadata = enabledAdditionOfAnnotationsAsMetadata; - } - - public String getAnnotationKeysPrefix() { - return annotationKeysPrefix; - } - - public void setAnnotationKeysPrefix(String annotationKeysPrefix) { - this.annotationKeysPrefix = annotationKeysPrefix; - } - - public boolean isEnabledAdditionOfPortsAsMetadata() { - return enabledAdditionOfPortsAsMetadata; - } - - public void setEnabledAdditionOfPortsAsMetadata(boolean enabledAdditionOfPortsAsMetadata) { - this.enabledAdditionOfPortsAsMetadata = enabledAdditionOfPortsAsMetadata; - } - - public String getPortKeysPrefix() { - return portKeysPrefix; - } - - public void setPortKeysPrefix(String portKeysPrefix) { - this.portKeysPrefix = portKeysPrefix; + public void setMetadata(Metadata metadata) { + this.metadata = metadata; } @Override public String toString() { - return "KubernetesDiscoveryProperties{" + - "enabled=" + enabled + - ", serviceName='" + serviceName + '\'' + - ", filter='" + filter + '\'' + - '}'; + return new ToStringCreator(this) + .append("enabled", enabled) + .append("serviceName", serviceName) + .append("filter", filter) + .append("metadata", metadata) + .toString(); + } + + public class Metadata { + /** When set, the Kubernetes labels of the services will be included as metadata of the returned ServiceInstance. */ + private boolean addLabels = true; + + /** When addLabels is set, then this will be used as a prefix to the key names in the metadata map. */ + private String labelsPrefix; + + /** When set, the Kubernetes annotations of the services will be included as metadata of the returned ServiceInstance. */ + private boolean addAnnotations = true; + + /** When addAnnotations is set, then this will be used as a prefix to the key names in the metadata map. */ + private String annotationsPrefix; + + /** When set, any named Kubernetes service ports will be included as metadata of the returned ServiceInstance. */ + private boolean addPorts = true; + + /** When addPorts is set, then this will be used as a prefix to the key names in the metadata map. */ + private String portsPrefix = "port."; + + public boolean isAddLabels() { + return addLabels; + } + + public void setAddLabels(boolean addLabels) { + this.addLabels = addLabels; + } + + public String getLabelsPrefix() { + return labelsPrefix; + } + + public void setLabelsPrefix(String labelsPrefix) { + this.labelsPrefix = labelsPrefix; + } + + public boolean isAddAnnotations() { + return addAnnotations; + } + + public void setAddAnnotations(boolean addAnnotations) { + this.addAnnotations = addAnnotations; + } + + public String getAnnotationsPrefix() { + return annotationsPrefix; + } + + public void setAnnotationsPrefix(String annotationsPrefix) { + this.annotationsPrefix = annotationsPrefix; + } + + public boolean isAddPorts() { + return addPorts; + } + + public void setAddPorts(boolean addPorts) { + this.addPorts = addPorts; + } + + public String getPortsPrefix() { + return portsPrefix; + } + + public void setPortsPrefix(String portsPrefix) { + this.portsPrefix = portsPrefix; + } + + @Override + public String toString() { + return new ToStringCreator(this) + .append("addLabels", addLabels) + .append("labelsPrefix", labelsPrefix) + .append("addAnnotations", addAnnotations) + .append("annotationsPrefix", annotationsPrefix) + .append("addPorts", addPorts) + .append("portsPrefix", portsPrefix) + .toString(); + } } } diff --git a/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientFilterMetadataTest.java b/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientFilterMetadataTest.java index efe160d2..178b21a7 100644 --- a/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientFilterMetadataTest.java +++ b/spring-cloud-kubernetes-discovery/src/test/java/org/springframework/cloud/kubernetes/discovery/KubernetesDiscoveryClientFilterMetadataTest.java @@ -57,6 +57,9 @@ public class KubernetesDiscoveryClientFilterMetadataTest { @Mock private KubernetesDiscoveryProperties properties; + @Mock + private KubernetesDiscoveryProperties.Metadata metadata; + @Mock private MixedOperation> serviceOperation; @@ -76,9 +79,10 @@ public class KubernetesDiscoveryClientFilterMetadataTest { public void testAllExtraMetadataDisabled() { final String serviceId = "s"; - when(properties.isEnabledAdditionOfLabelsAsMetadata()).thenReturn(false); - when(properties.isEnabledAdditionOfAnnotationsAsMetadata()).thenReturn(false); - when(properties.isEnabledAdditionOfPortsAsMetadata()).thenReturn(false); + when(properties.getMetadata()).thenReturn(metadata); + when(metadata.isAddLabels()).thenReturn(false); + when(metadata.isAddAnnotations()).thenReturn(false); + when(metadata.isAddPorts()).thenReturn(false); setupServiceWithLabelsAndAnnotationsAndPorts( serviceId, @@ -103,9 +107,10 @@ public class KubernetesDiscoveryClientFilterMetadataTest { public void testLabelsEnabled() { final String serviceId = "s"; - when(properties.isEnabledAdditionOfLabelsAsMetadata()).thenReturn(true); - when(properties.isEnabledAdditionOfAnnotationsAsMetadata()).thenReturn(false); - when(properties.isEnabledAdditionOfPortsAsMetadata()).thenReturn(false); + when(properties.getMetadata()).thenReturn(metadata); + when(metadata.isAddLabels()).thenReturn(true); + when(metadata.isAddAnnotations()).thenReturn(false); + when(metadata.isAddPorts()).thenReturn(false); setupServiceWithLabelsAndAnnotationsAndPorts( serviceId, @@ -131,10 +136,11 @@ public class KubernetesDiscoveryClientFilterMetadataTest { public void testLabelsEnabledWithPrefix() { final String serviceId = "s"; - when(properties.isEnabledAdditionOfLabelsAsMetadata()).thenReturn(true); - when(properties.getLabelKeysPrefix()).thenReturn("l_"); - when(properties.isEnabledAdditionOfAnnotationsAsMetadata()).thenReturn(false); - when(properties.isEnabledAdditionOfPortsAsMetadata()).thenReturn(false); + when(properties.getMetadata()).thenReturn(metadata); + when(metadata.isAddLabels()).thenReturn(true); + when(metadata.getLabelsPrefix()).thenReturn("l_"); + when(metadata.isAddAnnotations()).thenReturn(false); + when(metadata.isAddPorts()).thenReturn(false); setupServiceWithLabelsAndAnnotationsAndPorts( serviceId, @@ -159,9 +165,10 @@ public class KubernetesDiscoveryClientFilterMetadataTest { public void testAnnotationsEnabled() { final String serviceId = "s"; - when(properties.isEnabledAdditionOfLabelsAsMetadata()).thenReturn(false); - when(properties.isEnabledAdditionOfAnnotationsAsMetadata()).thenReturn(true); - when(properties.isEnabledAdditionOfPortsAsMetadata()).thenReturn(false); + when(properties.getMetadata()).thenReturn(metadata); + when(metadata.isAddLabels()).thenReturn(false); + when(metadata.isAddAnnotations()).thenReturn(true); + when(metadata.isAddPorts()).thenReturn(false); setupServiceWithLabelsAndAnnotationsAndPorts( serviceId, @@ -187,10 +194,11 @@ public class KubernetesDiscoveryClientFilterMetadataTest { public void testAnnotationsEnabledWithPrefix() { final String serviceId = "s"; - when(properties.isEnabledAdditionOfLabelsAsMetadata()).thenReturn(false); - when(properties.isEnabledAdditionOfAnnotationsAsMetadata()).thenReturn(true); - when(properties.getAnnotationKeysPrefix()).thenReturn("a_"); - when(properties.isEnabledAdditionOfPortsAsMetadata()).thenReturn(false); + when(properties.getMetadata()).thenReturn(metadata); + when(metadata.isAddLabels()).thenReturn(false); + when(metadata.isAddAnnotations()).thenReturn(true); + when(metadata.getAnnotationsPrefix()).thenReturn("a_"); + when(metadata.isAddPorts()).thenReturn(false); setupServiceWithLabelsAndAnnotationsAndPorts( serviceId, @@ -216,9 +224,10 @@ public class KubernetesDiscoveryClientFilterMetadataTest { public void testPortsEnabled() { final String serviceId = "s"; - when(properties.isEnabledAdditionOfLabelsAsMetadata()).thenReturn(false); - when(properties.isEnabledAdditionOfAnnotationsAsMetadata()).thenReturn(false); - when(properties.isEnabledAdditionOfPortsAsMetadata()).thenReturn(true); + when(properties.getMetadata()).thenReturn(metadata); + when(metadata.isAddLabels()).thenReturn(false); + when(metadata.isAddAnnotations()).thenReturn(false); + when(metadata.isAddPorts()).thenReturn(true); setupServiceWithLabelsAndAnnotationsAndPorts( serviceId, @@ -244,10 +253,11 @@ public class KubernetesDiscoveryClientFilterMetadataTest { public void testPortsEnabledWithPrefix() { final String serviceId = "s"; - when(properties.isEnabledAdditionOfLabelsAsMetadata()).thenReturn(false); - when(properties.isEnabledAdditionOfAnnotationsAsMetadata()).thenReturn(false); - when(properties.isEnabledAdditionOfPortsAsMetadata()).thenReturn(true); - when(properties.getPortKeysPrefix()).thenReturn("p_"); + when(properties.getMetadata()).thenReturn(metadata); + when(metadata.isAddLabels()).thenReturn(false); + when(metadata.isAddAnnotations()).thenReturn(false); + when(metadata.isAddPorts()).thenReturn(true); + when(metadata.getPortsPrefix()).thenReturn("p_"); setupServiceWithLabelsAndAnnotationsAndPorts( serviceId, @@ -273,12 +283,13 @@ public class KubernetesDiscoveryClientFilterMetadataTest { public void testLabelsAndAnnotationsAndPortsEnabledWithPrefix() { final String serviceId = "s"; - when(properties.isEnabledAdditionOfLabelsAsMetadata()).thenReturn(true); - when(properties.getLabelKeysPrefix()).thenReturn("l_"); - when(properties.isEnabledAdditionOfAnnotationsAsMetadata()).thenReturn(true); - when(properties.getAnnotationKeysPrefix()).thenReturn("a_"); - when(properties.isEnabledAdditionOfPortsAsMetadata()).thenReturn(true); - when(properties.getPortKeysPrefix()).thenReturn("p_"); + when(properties.getMetadata()).thenReturn(metadata); + when(metadata.isAddLabels()).thenReturn(true); + when(metadata.getLabelsPrefix()).thenReturn("l_"); + when(metadata.isAddAnnotations()).thenReturn(true); + when(metadata.getAnnotationsPrefix()).thenReturn("a_"); + when(metadata.isAddPorts()).thenReturn(true); + when(metadata.getPortsPrefix()).thenReturn("p_"); setupServiceWithLabelsAndAnnotationsAndPorts( serviceId,