feat(kubernetes): add endpoint to list cluster manifest coordinates (#…

…4841)

* feat(kubernetes): add endopint to list cluster manifest coordinates

This will be consumed by Orca's Deploy Manifest Stage when fetching SGs to disable/delete as part of rollout strategy workflows.

* refactor(kubernetes): minimize accessibility of ManifestController and KubernetesManifestProvider

* refactor(kubernetes): throw error if account unresolvable

The only situation getClusterManifestCoordinates returns null in is when we can't resolve an account name. Let's just throw an error as soon as possible instead of throwing a less meaningful error downstream.

* refactor(kubernetes): make symmetrical changes to getClusterAndSortAscending

* fix(kubernetes): only consider same-app clusters for dynamic target selection

This preserves symmetry with the rollout strategies logic and consistency with the behavior when this endpoint used the cache instead of making live calls.

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesManifestProvider.java

@@ -26,11 +26,9 @@
 import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesCoordinates;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesPodMetric;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesPodMetric.ContainerMetric;
-import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesResourceProperties;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifestAnnotater;
-import com.netflix.spinnaker.clouddriver.kubernetes.op.handler.KubernetesHandler;
 import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesCredentials;
 import java.util.Collection;
 import java.util.List;
@@ -116,25 +114,46 @@ private ImmutableList<ContainerMetric> getPodMetrics(
         .collect(toImmutableList());
   }
 
-  @Nullable
   public List<KubernetesManifest> getClusterAndSortAscending(
-      String account, String location, String kind, String cluster, Sort sort) {
-    Optional<KubernetesCredentials> optionalCredentials = accountResolver.getCredentials(account);
-    if (!optionalCredentials.isPresent()) {
-      return null;
-    }
-    KubernetesCredentials credentials = optionalCredentials.get();
+      String account, String location, String kind, String cluster, String app, Sort sort) {
     KubernetesKind kubernetesKind = KubernetesKind.fromString(kind);
+    return accountResolver
+        .getCredentials(account)
+        .map(
+            credentials ->
+                credentials.list(kubernetesKind, location).stream()
+                    .filter(
+                        m ->
+                            cluster.equals(KubernetesManifestAnnotater.getManifestCluster(m))
+                                & app.equals(KubernetesManifestAnnotater.getManifestApplication(m)))
+                    .sorted(
+                        (m1, m2) ->
+                            credentials
+                                .getResourcePropertyRegistry()
+                                .get(kubernetesKind)
+                                .getHandler()
+                                .comparatorFor(sort)
+                                .compare(m1, m2))
+                    .collect(Collectors.toList()))
+        .orElseThrow(() -> new IllegalArgumentException("Unable to resolve account: " + account));
+  }
 
-    KubernetesResourceProperties properties =
-        credentials.getResourcePropertyRegistry().get(kubernetesKind);
-
-    KubernetesHandler handler = properties.getHandler();
-
-    return credentials.list(kubernetesKind, location).stream()
-        .filter(m -> cluster.equals(KubernetesManifestAnnotater.getManifestCluster(m)))
-        .sorted((m1, m2) -> handler.comparatorFor(sort).compare(m1, m2))
-        .collect(Collectors.toList());
+  public List<KubernetesCoordinates> getClusterManifestCoordinates(
+      String account, String location, String kind, String app, String cluster) {
+    KubernetesKind kubernetesKind = KubernetesKind.fromString(kind);
+    return accountResolver
+        .getCredentials(account)
+        .map(
+            credentials ->
+                credentials.list(kubernetesKind, location).stream()
+                    .filter(
+                        m ->
+                            cluster.equals(KubernetesManifestAnnotater.getManifestCluster(m))
+                                && app.equals(
+                                    KubernetesManifestAnnotater.getManifestApplication(m)))
+                    .map(KubernetesCoordinates::fromManifest)
+                    .collect(Collectors.toList()))
+        .orElseThrow(() -> new IllegalArgumentException("Unable to resolve account: " + account));
   }
 
   public enum Sort {

clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/controllers/ManifestController.java

@@ -41,9 +41,8 @@
 @RequestMapping("/manifests")
 public class ManifestController {
   private static final Logger log = LoggerFactory.getLogger(ManifestController.class);
-  final KubernetesManifestProvider manifestProvider;
-
-  final RequestQueue requestQueue;
+  private final KubernetesManifestProvider manifestProvider;
+  private final RequestQueue requestQueue;
 
   @Autowired
   public ManifestController(
@@ -124,16 +123,12 @@ KubernetesCoordinates getDynamicManifestFromCluster(
               account,
               () ->
                   manifestProvider.getClusterAndSortAscending(
-                      account, location, kind, cluster, criteria.getSort()));
+                      account, location, kind, cluster, app, criteria.getSort()));
     } catch (Throwable t) {
       log.warn("Failed to read {}", request, t);
       return null;
     }
 
-    if (manifests == null) {
-      throw new NotFoundException("No manifests matching " + request + " found");
-    }
-
     try {
       switch (criteria) {
         case oldest:
@@ -152,6 +147,36 @@ KubernetesCoordinates getDynamicManifestFromCluster(
     }
   }
 
+  @RequestMapping(
+      value = "/{account:.+}/{location:.+}/{kind:.+}/cluster/{app:.+}/{cluster:.+}",
+      method = RequestMethod.GET)
+  List<KubernetesCoordinates> getClusterManifestCoordinates(
+      @PathVariable String account,
+      @PathVariable String location,
+      @PathVariable String kind,
+      @PathVariable String app,
+      @PathVariable String cluster) {
+    final String request =
+        String.format(
+            "(account: %s, location: %s, kind: %s, app %s, cluster: %s)",
+            account, location, kind, app, cluster);
+
+    List<KubernetesCoordinates> coordinates;
+    try {
+      coordinates =
+          requestQueue.execute(
+              account,
+              () ->
+                  manifestProvider.getClusterManifestCoordinates(
+                      account, location, kind, app, cluster));
+    } catch (Throwable t) {
+      log.warn("Failed to read {}", request, t);
+      return null;
+    }
+
+    return coordinates;
+  }
+
   enum Criteria {
     oldest(Sort.AGE),
     newest(Sort.AGE),

clouddriver-kubernetes/src/main/java/com/netflix/spinnaker/clouddriver/kubernetes/description/manifest/KubernetesManifestAnnotater.java

@@ -303,4 +303,8 @@ public static KubernetesManifest getLastAppliedConfiguration(KubernetesManifest
   public static String getManifestCluster(KubernetesManifest manifest) {
     return Strings.nullToEmpty(manifest.getAnnotations().get(CLUSTER));
   }
+
+  public static String getManifestApplication(KubernetesManifest manifest) {
+    return Strings.nullToEmpty(manifest.getAnnotations().get(APPLICATION));
+  }
 }

clouddriver-kubernetes/src/test/java/com/netflix/spinnaker/clouddriver/kubernetes/caching/view/provider/KubernetesDataProviderIntegrationTest.java

@@ -20,6 +20,7 @@
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyList;
 import static org.mockito.Mockito.mock;
@@ -48,6 +49,7 @@
 import com.netflix.spinnaker.clouddriver.kubernetes.config.KubernetesConfigurationProperties;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.AccountResourcePropertyRegistry;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.GlobalResourcePropertyRegistry;
+import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesCoordinates;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.KubernetesSpinnakerKindMap;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesKind;
 import com.netflix.spinnaker.clouddriver.kubernetes.description.manifest.KubernetesManifest;
@@ -525,7 +527,7 @@ void getArtifactsWrongAccount(SoftAssertions softly) {
   void getClusterAndSortAscending(SoftAssertions softly) {
     List<KubernetesManifest> manifests =
         manifestProvider.getClusterAndSortAscending(
-            ACCOUNT_NAME, "backend-ns", "replicaSet", "replicaSet backend", Sort.AGE);
+            ACCOUNT_NAME, "backend-ns", "replicaSet", "replicaSet backend", "backendapp", Sort.AGE);
     assertThat(manifests).isNotNull();
     softly
         .assertThat(
@@ -535,6 +537,66 @@ void getClusterAndSortAscending(SoftAssertions softly) {
         .containsExactly("replicaSet backend-v014", "replicaSet backend-v015");
   }
 
+  @Test
+  void getClusterAndSortAscendingBadAccount(SoftAssertions softly) {
+    assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            manifestProvider.getClusterAndSortAscending(
+                "not-an-account",
+                "backend-ns",
+                "replicaSet",
+                "replicaSet backend",
+                "backendapp",
+                Sort.AGE));
+  }
+
+  @Test
+  void getClusterManifestCoordinates(SoftAssertions softly) {
+    List<KubernetesCoordinates> coordinates =
+        manifestProvider.getClusterManifestCoordinates(
+            ACCOUNT_NAME, "backend-ns", "replicaSet", "backendapp", "replicaSet backend");
+    assertThat(coordinates).isNotNull();
+    softly
+        .assertThat(coordinates.stream().collect(toImmutableList()))
+        .containsExactlyInAnyOrder(
+            KubernetesCoordinates.builder()
+                .kind(KubernetesKind.REPLICA_SET)
+                .name("backend-v014")
+                .namespace("backend-ns")
+                .build(),
+            KubernetesCoordinates.builder()
+                .kind(KubernetesKind.REPLICA_SET)
+                .name("backend-v015")
+                .namespace("backend-ns")
+                .build());
+  }
+
+  @Test
+  void getClusterManifestCoordinatesBadAccount(SoftAssertions softly) {
+    assertThrows(
+        IllegalArgumentException.class,
+        () ->
+            manifestProvider.getClusterManifestCoordinates(
+                "not-an-account", "backend-ns", "replicaSet", "backendapp", "replicaSet backend"));
+  }
+
+  @Test
+  void getClusterManifestCoordinatesEmptyNamespace(SoftAssertions softly) {
+    List<KubernetesCoordinates> coordinates =
+        manifestProvider.getClusterManifestCoordinates(
+            ACCOUNT_NAME, "empty", "replicaSet", "backendapp", "replicaSet backend");
+    softly.assertThat(coordinates).isEmpty();
+  }
+
+  @Test
+  void getClusterManifestCoordinatesEmptyCluster(SoftAssertions softly) {
+    List<KubernetesCoordinates> coordinates =
+        manifestProvider.getClusterManifestCoordinates(
+            ACCOUNT_NAME, "empty-namespace", "replicaSet", "backendapp", "replicaSet empty");
+    softly.assertThat(coordinates).isEmpty();
+  }
+
   private static KubectlJobExecutor getJobExecutor() {
     KubectlJobExecutor jobExecutor = mock(KubectlJobExecutor.class, new ReturnsSmartNulls());
     when(jobExecutor.list(