remove schema from url for service instance host.

Related to smallrye#488
aureamunoz · Feb 9, 2023 · d2f60c1 · d2f60c1
1 parent c2eaf74
commit d2f60c1
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 31 deletions.
diff --git a/...ive/src/main/java/io/smallrye/stork/servicediscovery/knative/KnativeServiceDiscovery.java b/...ive/src/main/java/io/smallrye/stork/servicediscovery/knative/KnativeServiceDiscovery.java
@@ -1,5 +1,6 @@
 package io.smallrye.stork.servicediscovery.knative;
 
+import java.net.URI;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -147,9 +148,21 @@ private List<ServiceInstance> toStorkServiceInstances(List<Service> knServices,
                         : Collections.emptyMap());
 
                 Metadata<KnativeMetadataKey> knativeMetadata = Metadata.of(KnativeMetadataKey.class);
+                String host = knService.getStatus().getUrl();
+                try {
+                    URI uri = new URI(knService.getStatus().getUrl());
+                    if (uri != null && uri.getScheme() != null) {
+                        host = uri.getHost();
+                        if (host == null) { // invalid URI
+                            throw new IllegalArgumentException("Invalid URL used: '" + uri + "'");
+                        }
+                    }
+                } catch (Exception e) {
+                    e.printStackTrace();
+                }
 
                 serviceInstances
-                        .add(new DefaultServiceInstance(ServiceInstanceIds.next(), knService.getStatus().getUrl(), 8080, secure,
+                        .add(new DefaultServiceInstance(ServiceInstanceIds.next(), host, 8080, secure,
                                 labels,
                                 knativeMetadata
                                         .with(KnativeMetadataKey.META_KNATIVE_SERVICE_ID, knService.getFullResourceName())

diff --git a/.../test/java/io/smallrye/stork/servicediscovery/knative/KnativeServiceDiscoveryCDITest.java b/.../test/java/io/smallrye/stork/servicediscovery/knative/KnativeServiceDiscoveryCDITest.java
@@ -71,7 +71,7 @@ void shouldDiscoverNamespacedKnativeServices() {
 
         String knSvcName = "my-knservice";
 
-        registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null);
+        registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null);
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -84,7 +84,7 @@ void shouldDiscoverNamespacedKnativeServices() {
                 .until(() -> instances.get() != null);
 
         assertThat(instances.get()).hasSize(1);
-        assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
+        assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
         assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
         Map<String, String> labels = instances.get().get(0).getLabels();
         assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
@@ -123,8 +123,8 @@ void shouldDiscoverKnativeServicesInAllNs() {
 
         String knativeService = "my-knservice";
 
-        registerKnativeServices(knativeService, "http://hello.ns1.127.0.0.1.sslip.io", "ns1");
-        registerKnativeServices(knativeService, "http://hello.ns2.127.0.0.1.sslip.io", "ns2");
+        registerKnativeServices(knativeService, "hello.ns1.127.0.0.1.sslip.io", "ns1");
+        registerKnativeServices(knativeService, "hello.ns2.127.0.0.1.sslip.io", "ns2");
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -138,7 +138,7 @@ void shouldDiscoverKnativeServicesInAllNs() {
 
         assertThat(instances.get()).hasSize(2);
         assertThat(instances.get().stream().map(ServiceInstance::getHost))
-                .containsExactlyInAnyOrder("http://hello.ns1.127.0.0.1.sslip.io", "http://hello.ns2.127.0.0.1.sslip.io");
+                .containsExactlyInAnyOrder("hello.ns1.127.0.0.1.sslip.io", "hello.ns2.127.0.0.1.sslip.io");
     }
 
     @Test
@@ -149,7 +149,7 @@ void shouldGetServiceFromK8sDefaultNamespaceUsingProgrammaticAPI() {
 
         String knativeService = "my-knservice";
 
-        registerKnativeServices(knativeService, "http://hello.test.127.0.0.1.sslip.io", null);
+        registerKnativeServices(knativeService, "hello.test.127.0.0.1.sslip.io", null);
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -162,7 +162,7 @@ void shouldGetServiceFromK8sDefaultNamespaceUsingProgrammaticAPI() {
                 .until(() -> instances.get() != null);
 
         assertThat(instances.get()).hasSize(1);
-        assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
+        assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
         assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
         Map<String, String> labels = instances.get().get(0).getLabels();
         assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
@@ -179,7 +179,7 @@ void shouldHandleSecureAttribute() {
 
         String knSvcName = "my-knservice";
 
-        registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null);
+        registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null);
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -192,7 +192,7 @@ void shouldHandleSecureAttribute() {
                 .until(() -> instances.get() != null);
 
         assertThat(instances.get()).hasSize(1);
-        assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
+        assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
         assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
         Map<String, String> labels = instances.get().get(0).getLabels();
         assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
@@ -218,7 +218,7 @@ void shouldFetchInstancesFromTheClusterWhenCacheIsInvalidated() {
 
         String knSvcName = "my-knservice";
 
-        registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null);
+        registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null);
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -231,7 +231,7 @@ void shouldFetchInstancesFromTheClusterWhenCacheIsInvalidated() {
                 .until(() -> instances.get() != null);
 
         assertThat(instances.get()).hasSize(1);
-        assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
+        assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
         assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
         Map<String, String> labels = instances.get().get(0).getLabels();
         assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
@@ -266,7 +266,7 @@ void shouldFetchInstancesFromTheCache() throws InterruptedException {
         server.expect().get().withPath("/apis/serving.knative.dev/v1/namespaces/test/services/my-knservice")
                 .andReply(200, r -> {
                     serverHit.incrementAndGet();
-                    return buildKnService(knSvcName, "http://hello.test.127.0.0.1.sslip.io", "test");
+                    return buildKnService(knSvcName, "hello.test.127.0.0.1.sslip.io", "test");
                 }).always();
 
         config.addServiceConfig("my-knservice", null, "knative",

diff --git a/...src/test/java/io/smallrye/stork/servicediscovery/knative/KnativeServiceDiscoveryTest.java b/...src/test/java/io/smallrye/stork/servicediscovery/knative/KnativeServiceDiscoveryTest.java
@@ -58,7 +58,7 @@ void shouldDiscoverNamespacedKnativeServices() {
 
         String knSvcName = "my-knservice";
 
-        registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null, null);
+        registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null, null);
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -71,7 +71,7 @@ void shouldDiscoverNamespacedKnativeServices() {
                 .until(() -> instances.get() != null);
 
         assertThat(instances.get()).hasSize(1);
-        assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
+        assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
         assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
         Map<String, String> labels = instances.get().get(0).getLabels();
         assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
@@ -88,7 +88,7 @@ void shouldDiscoverNamespacedKnativeServicesWithApp() {
         String knSvcName = "my-knservice";
         String applicationName = "greetingApp";
 
-        registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null, applicationName);
+        registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null, applicationName);
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -101,7 +101,7 @@ void shouldDiscoverNamespacedKnativeServicesWithApp() {
                 .until(() -> instances.get() != null);
 
         assertThat(instances.get()).hasSize(1);
-        assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
+        assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
         assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
         Map<String, String> labels = instances.get().get(0).getLabels();
         assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
@@ -142,8 +142,8 @@ void shouldDiscoverKnativeServicesInAllNs() {
 
         String knativeService = "my-knservice";
 
-        registerKnativeServices(knativeService, "http://hello.ns1.127.0.0.1.sslip.io", "ns1", null);
-        registerKnativeServices(knativeService, "http://hello.ns2.127.0.0.1.sslip.io", "ns2", null);
+        registerKnativeServices(knativeService, "hello.ns1.127.0.0.1.sslip.io", "ns1", null);
+        registerKnativeServices(knativeService, "hello.ns2.127.0.0.1.sslip.io", "ns2", null);
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -157,7 +157,7 @@ void shouldDiscoverKnativeServicesInAllNs() {
 
         assertThat(instances.get()).hasSize(2);
         assertThat(instances.get().stream().map(ServiceInstance::getHost))
-                .containsExactlyInAnyOrder("http://hello.ns1.127.0.0.1.sslip.io", "http://hello.ns2.127.0.0.1.sslip.io");
+                .containsExactlyInAnyOrder("hello.ns1.127.0.0.1.sslip.io", "hello.ns2.127.0.0.1.sslip.io");
     }
 
     @Test
@@ -168,7 +168,7 @@ void shouldGetServiceFromK8sDefaultNamespaceUsingProgrammaticAPI() {
 
         String knativeService = "my-knservice";
 
-        registerKnativeServices(knativeService, "http://hello.test.127.0.0.1.sslip.io", null, null);
+        registerKnativeServices(knativeService, "hello.test.127.0.0.1.sslip.io", null, null);
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -181,7 +181,7 @@ void shouldGetServiceFromK8sDefaultNamespaceUsingProgrammaticAPI() {
                 .until(() -> instances.get() != null);
 
         assertThat(instances.get()).hasSize(1);
-        assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
+        assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
         assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
         Map<String, String> labels = instances.get().get(0).getLabels();
         assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
@@ -198,7 +198,7 @@ void shouldHandleSecureAttribute() {
 
         String knSvcName = "my-knservice";
 
-        registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null, null);
+        registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null, null);
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -211,7 +211,7 @@ void shouldHandleSecureAttribute() {
                 .until(() -> instances.get() != null);
 
         assertThat(instances.get()).hasSize(1);
-        assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
+        assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
         assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
         Map<String, String> labels = instances.get().get(0).getLabels();
         assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
@@ -237,7 +237,7 @@ void shouldFetchInstancesFromTheClusterWhenCacheIsInvalidated() {
 
         String knSvcName = "my-knservice";
 
-        registerKnativeServices(knSvcName, "http://hello.test.127.0.0.1.sslip.io", null, null);
+        registerKnativeServices(knSvcName, "hello.test.127.0.0.1.sslip.io", null, null);
 
         AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
 
@@ -250,7 +250,7 @@ void shouldFetchInstancesFromTheClusterWhenCacheIsInvalidated() {
                 .until(() -> instances.get() != null);
 
         assertThat(instances.get()).hasSize(1);
-        assertThat(instances.get().get(0).getHost()).isEqualTo("http://hello.test.127.0.0.1.sslip.io");
+        assertThat(instances.get().get(0).getHost()).isEqualTo("hello.test.127.0.0.1.sslip.io");
         assertThat(instances.get().get(0).getPort()).isEqualTo(8080);
         Map<String, String> labels = instances.get().get(0).getLabels();
         assertThat(labels).contains(entry("serving.knative.dev/creator", "kubernetes-admin"),
@@ -285,7 +285,7 @@ void shouldFetchInstancesFromTheCache() throws InterruptedException {
         server.expect().get().withPath("/apis/serving.knative.dev/v1/namespaces/test/services/my-knservice")
                 .andReply(200, r -> {
                     serverHit.incrementAndGet();
-                    return buildKnService(knSvcName, "http://hello.test.127.0.0.1.sslip.io", "test", null);
+                    return buildKnService(knSvcName, "hello.test.127.0.0.1.sslip.io", "test", null);
                 }).always();
 
         TestConfigProvider.addServiceConfig("my-knservice", null, "knative",

diff --git a/...st/java/io/smallrye/stork/servicediscovery/kubernetes/KubernetesServiceDiscoveryTest.java b/...st/java/io/smallrye/stork/servicediscovery/kubernetes/KubernetesServiceDiscoveryTest.java
@@ -98,6 +98,44 @@ void shouldGetServiceFromK8sDefaultNamespace() {
         assertThat(instances.get()).allSatisfy(si -> assertThat(si.isSecure()).isFalse());
     }
 
+    @Test
+    void shouldGetServiceFromK8sWithApplicationNameConfig() {
+        TestConfigProvider.addServiceConfig("svc", null, "kubernetes",
+                null, Map.of("k8s-host", k8sMasterUrl, "k8s-namespace", defaultNamespace, "application", "greetingApp"));
+        Stork stork = StorkTestUtils.getNewStorkInstance();
+
+        String serviceName = "svc";
+        String[] ips = { "10.96.96.231", "10.96.96.232", "10.96.96.233" };
+
+        registerKubernetesResources("greetingApp", defaultNamespace, ips);
+
+        AtomicReference<List<ServiceInstance>> instances = new AtomicReference<>();
+
+        Service service = stork.getService(serviceName);
+        service.getServiceDiscovery().getServiceInstances()
+                .onFailure().invoke(th -> fail("Failed to get service instances from Kubernetes", th))
+                .subscribe().with(instances::set);
+
+        await().atMost(Duration.ofSeconds(5))
+                .until(() -> instances.get() != null);
+
+        assertThat(instances.get()).hasSize(3);
+        assertThat(instances.get().stream().map(ServiceInstance::getPort)).allMatch(p -> p == 8080);
+        assertThat(instances.get().stream().map(ServiceInstance::getHost)).containsExactlyInAnyOrder("10.96.96.231",
+                "10.96.96.232", "10.96.96.233");
+        for (ServiceInstance serviceInstance : instances.get()) {
+            Map<String, String> labels = serviceInstance.getLabels();
+            assertThat(labels).contains(entry("app.kubernetes.io/name", "greetingApp"),
+                    entry("app.kubernetes.io/version", "1.0"),
+                    entry("ui", "ui-" + ipAsSuffix(serviceInstance.getHost())));
+        }
+        instances.get().stream().map(ServiceInstance::getMetadata).forEach(metadata -> {
+            Metadata<KubernetesMetadataKey> k8sMetadata = (Metadata<KubernetesMetadataKey>) metadata;
+            assertThat(k8sMetadata.getMetadata()).containsKey(META_K8S_SERVICE_ID);
+        });
+        assertThat(instances.get()).allSatisfy(si -> assertThat(si.isSecure()).isFalse());
+    }
+
     @Test
     void shouldGetServiceFromK8sDefaultNamespaceUsingProgrammaticAPI() {
         Stork stork = StorkTestUtils.getNewStorkInstance();
@@ -472,23 +510,23 @@ private Map<String, Long> mapHostnameToIds(List<ServiceInstance> serviceInstance
         return result;
     }
 
-    private Endpoints buildAndRegisterKubernetesService(String serviceName, String namespace, boolean register,
+    private Endpoints buildAndRegisterKubernetesService(String applicationName, String namespace, boolean register,
             String... ipAdresses) {
 
         Map<String, String> serviceLabels = new HashMap<>();
-        serviceLabels.put("app.kubernetes.io/name", serviceName);
+        serviceLabels.put("app.kubernetes.io/name", applicationName);
         serviceLabels.put("app.kubernetes.io/version", "1.0");
 
         List<EndpointAddress> endpointAddresses = Arrays.stream(ipAdresses)
                 .map(ipAddress -> {
                     ObjectReference targetRef = new ObjectReference(null, null, "Pod",
-                            serviceName + "-" + ipAsSuffix(ipAddress), namespace, null, UUID.randomUUID().toString());
+                            applicationName + "-" + ipAsSuffix(ipAddress), namespace, null, UUID.randomUUID().toString());
                     EndpointAddress endpointAddress = new EndpointAddressBuilder().withIp(ipAddress).withTargetRef(targetRef)
                             .build();
                     return endpointAddress;
                 }).collect(Collectors.toList());
         Endpoints endpoint = new EndpointsBuilder()
-                .withNewMetadata().withName(serviceName).withLabels(serviceLabels).endMetadata()
+                .withNewMetadata().withName(applicationName).withLabels(serviceLabels).endMetadata()
                 .addToSubsets(new EndpointSubsetBuilder().withAddresses(endpointAddresses)
                         .addToPorts(new EndpointPortBuilder().withPort(8080).withProtocol("TCP").build())
                         .build())