feat(entitytags): Support indexing / searching by application

If `entityRef.application` is not explicitly provided, it will be parsed from `entityRef.entityId` using frigga conventions. Searching by application can be accomplished by `/tags?application=my_application`
spinnaker · Nov 16, 2017 · 3e2682b · 3e2682b
1 parent ee1819e
commit 3e2682b
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 20 deletions.
diff --git a/clouddriver-core/src/main/groovy/com/netflix/spinnaker/clouddriver/model/EntityTags.groovy b/clouddriver-core/src/main/groovy/com/netflix/spinnaker/clouddriver/model/EntityTags.groovy
@@ -76,6 +76,7 @@ class EntityTags {
     private Map<String, Object> attributes = new HashMap<String, Object>()
 
     String cloudProvider
+    String application
     String accountId
     String account
     String region

diff --git a/...iver-core/src/main/groovy/com/netflix/spinnaker/clouddriver/model/EntityTagsProvider.java b/...iver-core/src/main/groovy/com/netflix/spinnaker/clouddriver/model/EntityTagsProvider.java
@@ -26,6 +26,7 @@ public interface EntityTagsProvider {
    * Fetch EntityTags by any combination of {@code cloudProvider}/{@code type}/{@code idPrefix}/{@code tags}
    */
   Collection<EntityTags> getAll(String cloudProvider,
+                                String application,
                                 String entityType,
                                 List<String> entityIds,
                                 String idPrefix,

diff --git a/.../java/com/netflix/spinnaker/clouddriver/elasticsearch/ElasticSearchServerGroupTagger.java b/.../java/com/netflix/spinnaker/clouddriver/elasticsearch/ElasticSearchServerGroupTagger.java
@@ -86,7 +86,16 @@ public Collection<EntityTags> taggedEntities(String cloudProvider,
                                                String tagName,
                                                int maxResults) {
     return entityTagsProvider.getAll(
-      cloudProvider, ENTITY_TYPE, null, null, accountId, null, null, Collections.singletonMap(tagName, "*"), maxResults
+      cloudProvider,
+      null,
+      ENTITY_TYPE,
+      null,
+      null,
+      accountId,
+      null,
+      null,
+      Collections.singletonMap(tagName, "*"),
+      maxResults
     );
   }
 

diff --git a/...om/netflix/spinnaker/clouddriver/elasticsearch/model/ElasticSearchEntityTagsProvider.java b/...om/netflix/spinnaker/clouddriver/elasticsearch/model/ElasticSearchEntityTagsProvider.java
@@ -18,6 +18,7 @@
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.Lists;
+import com.netflix.frigga.Names;
 import com.netflix.spinnaker.clouddriver.core.services.Front50Service;
 import com.netflix.spinnaker.clouddriver.helpers.OperationPoller;
 import com.netflix.spinnaker.clouddriver.model.EntityTags;
@@ -79,6 +80,7 @@ public ElasticSearchEntityTagsProvider(ObjectMapper objectMapper,
 
   @Override
   public Collection<EntityTags> getAll(String cloudProvider,
+                                       String application,
                                        String entityType,
                                        List<String> entityIds,
                                        String idPrefix,
@@ -94,6 +96,11 @@ public Collection<EntityTags> getAll(String cloudProvider,
       queryBuilder = queryBuilder.must(QueryBuilders.termQuery("entityRef.cloudProvider", cloudProvider));
     }
 
+    if (application != null) {
+      // restrict to a specific application (optional)
+      queryBuilder = queryBuilder.must(QueryBuilders.termQuery("entityRef.application", application));
+    }
+
     if (entityIds != null && !entityIds.isEmpty()) {
       // restrict to a specific set of entityIds (optional)
       queryBuilder = queryBuilder.must(QueryBuilders.termsQuery("entityRef.entityId", entityIds));
@@ -461,6 +468,20 @@ private static EntityTags prepareForWrite(ObjectMapper objectMapper, EntityTags
 
     copyOfEntityTags.getTags().forEach(entityTag -> entityTag.setValue(entityTag.getValueForWrite(objectMapper)));
 
+    String application = copyOfEntityTags.getEntityRef().getApplication();
+    if (application == null || application.trim().isEmpty()) {
+      try {
+        Names names = Names.parseName(copyOfEntityTags.getEntityRef().getEntityId());
+        copyOfEntityTags.getEntityRef().setApplication(names.getApp());
+      } catch (Exception e) {
+        log.error(
+          "Unable to extract application name (entityId: {})",
+          copyOfEntityTags.getEntityRef().getEntityId(),
+          e
+        );
+      }
+    }
+
     return copyOfEntityTags;
   }
 

diff --git a/.../netflix/spinnaker/clouddriver/elasticsearch/ops/BulkUpsertEntityTagsAtomicOperation.java b/.../netflix/spinnaker/clouddriver/elasticsearch/ops/BulkUpsertEntityTagsAtomicOperation.java
@@ -71,7 +71,12 @@ public BulkUpsertEntityTagsAtomicOperationResult operate(List priorOutputs) {
       Map<String, EntityTags> existingTags = retrieveExistingTags(tags);
 
       getTask().updateStatus(BASE_PHASE, "Merging existing tags and metadata");
-      tags.forEach(tag -> mergeExistingTagsAndMetadata(now, existingTags.get(tag.getId()), tag, bulkUpsertEntityTagsDescription.isPartial));
+      tags.forEach(tag -> mergeExistingTagsAndMetadata(
+        now,
+        existingTags.get(tag.getId()),
+        tag,
+        bulkUpsertEntityTagsDescription.isPartial
+      ));
 
       getTask().updateStatus(BASE_PHASE, "Performing batch update to durable tagging service");
       Map<String, EntityTags> durableTags = front50Service.batchUpdate(new ArrayList<>(tags))
@@ -111,7 +116,9 @@ BASE_PHASE, format("Failed to build tag id for %s, reason: %s", tag.getId(), e.g
     entityTags.removeAll(failed);
   }
 
-  private void updateMetadataFromDurableTagsAndIndex(List<EntityTags> entityTags, Map<String, EntityTags> durableTags, BulkUpsertEntityTagsAtomicOperationResult result) {
+  private void updateMetadataFromDurableTagsAndIndex(List<EntityTags> entityTags,
+                                                     Map<String, EntityTags> durableTags,
+                                                     BulkUpsertEntityTagsAtomicOperationResult result) {
     Collection<EntityTags> failed = new ArrayList<>();
     entityTags.forEach(tag -> {
       try {
@@ -153,14 +160,22 @@ public static EntityRefIdBuilder.EntityRefId entityRefId(AccountCredentialsProvi
 
     if (entityRefAccount != null && entityRefAccountId == null) {
       // add `accountId` if not explicitly provided
-      AccountCredentials accountCredentials = lookupAccountCredentialsByAccountIdOrName(accountCredentialsProvider, entityRefAccount, "accountName");
+      AccountCredentials accountCredentials = lookupAccountCredentialsByAccountIdOrName(
+        accountCredentialsProvider,
+        entityRefAccount,
+        "accountName"
+      );
       entityRefAccountId = accountCredentials.getAccountId();
       entityRef.setAccountId(entityRefAccountId);
     }
 
     if (entityRefAccount == null && entityRefAccountId != null) {
       // add `account` if not explicitly provided
-      AccountCredentials accountCredentials = lookupAccountCredentialsByAccountIdOrName(accountCredentialsProvider, entityRefAccountId, "accountId");
+      AccountCredentials accountCredentials = lookupAccountCredentialsByAccountIdOrName(
+        accountCredentialsProvider,
+        entityRefAccountId,
+        "accountId"
+      );
       if (accountCredentials != null) {
         entityRefAccount = accountCredentials.getName();
         entityRef.setAccount(entityRefAccount);
@@ -252,7 +267,9 @@ private static AccountCredentials lookupAccountCredentialsByAccountIdOrName(Acco
     return accountCredentialsProvider.getAll().stream()
       .filter(c -> entityRefAccountIdOrName.equals(c.getAccountId()) || entityRefAccountIdOrName.equals(c.getName()))
       .findFirst()
-      .orElseThrow(() -> new IllegalArgumentException(String.format("No credentials found for %s: %s", type, entityRefAccountIdOrName)));
+      .orElseThrow(() -> new IllegalArgumentException(
+        String.format("No credentials found for %s: %s", type, entityRefAccountIdOrName)
+      ));
   }
 
   private static Task getTask() {

diff --git a/.../com/netflix/spinnaker/clouddriver/elasticsearch/ops/UpsertEntityTagsAtomicOperation.java b/.../com/netflix/spinnaker/clouddriver/elasticsearch/ops/UpsertEntityTagsAtomicOperation.java
@@ -47,8 +47,14 @@ public Void operate(List priorOutputs) {
     BulkUpsertEntityTagsDescription bulkDescription = new BulkUpsertEntityTagsDescription();
     bulkDescription.isPartial = entityTagsDescription.isPartial;
     bulkDescription.entityTags = Collections.singletonList(entityTagsDescription);
-    new BulkUpsertEntityTagsAtomicOperation(front50Service, accountCredentialsProvider, entityTagsProvider,
-      bulkDescription).operate(priorOutputs);
+
+    new BulkUpsertEntityTagsAtomicOperation(
+      front50Service,
+      accountCredentialsProvider,
+      entityTagsProvider,
+      bulkDescription
+    ).operate(priorOutputs);
+
     return null;
   }
 

diff --git a/...flix/spinnaker/clouddriver/elasticsearch/model/ElasticSearchEntityTagsProviderSpec.groovy b/...flix/spinnaker/clouddriver/elasticsearch/model/ElasticSearchEntityTagsProviderSpec.groovy
@@ -120,6 +120,25 @@ class ElasticSearchEntityTagsProviderSpec extends Specification {
     !entityTagsProvider.get(entityTags.id, ["tag3": "value3"]).isPresent()
   }
 
+  def "should assign entityRef.application if not specified"() {
+    given:
+    def entityTags = buildEntityTags("aws:cluster:front50-main:myaccount:*", ["tag1": "value1", "tag2": "value2"])
+    entityTags.entityRef.application = application
+
+    entityTagsProvider.index(entityTags)
+    entityTagsProvider.verifyIndex(entityTags)
+
+    expect:
+    entityTagsProvider.get(entityTags.id).get().entityRef.application == expectedApplication
+
+    where:
+    application      || expectedApplication
+    null             || "front50"
+    ""               || "front50"
+    " "              || "front50"
+    "my_application" || "my_application"
+  }
+
   def "should support multi result retrieval by `cloudProvider, `entityType`, `idPrefix` and `tags`"() {
     given:
     def entityTags = buildEntityTags("aws:cluster:clouddriver-main:myaccount:*", ["tag3": "value3"])
@@ -132,37 +151,40 @@ class ElasticSearchEntityTagsProviderSpec extends Specification {
 
     expect:
     // fetch everything
-    entityTagsProvider.getAll(null, null, null, null, null, null, null, null, 2)*.id.sort() == [entityTags.id, moreEntityTags.id].sort()
+    entityTagsProvider.getAll(null, null, null, null, null, null, null, null, null, 2)*.id.sort() == [entityTags.id, moreEntityTags.id].sort()
+
+    // fetch everything for an application
+    entityTagsProvider.getAll(null, "front50", null, null, null, null, null, null, null, 2)*.id.sort() == [moreEntityTags.id].sort()
 
     // fetch everything for a single `entityId`
-    entityTagsProvider.getAll(null, null, [entityTags.entityRef.entityId], null, null, null, null, null, 2)*.id.sort() == [entityTags.id].sort()
+    entityTagsProvider.getAll(null, null, null, [entityTags.entityRef.entityId], null, null, null, null, null, 2)*.id.sort() == [entityTags.id].sort()
 
     // fetch everything for a multiple `entityId`
-    entityTagsProvider.getAll(null, null, [
+    entityTagsProvider.getAll(null, null, null, [
       entityTags.entityRef.entityId, moreEntityTags.entityRef.entityId
     ], null, null, null, null, null, 2)*.id.sort() == [entityTags.id, moreEntityTags.id].sort()
 
     // fetch everything for `cloudprovider`
-    entityTagsProvider.getAll("aws", null, null, null, null, null, null, null, 2)*.id.sort() == [entityTags.id, moreEntityTags.id].sort()
+    entityTagsProvider.getAll("aws", null, null, null, null, null, null, null, null, 2)*.id.sort() == [entityTags.id, moreEntityTags.id].sort()
 
     // fetch everything for `cloudprovider` and `cluster`
-    entityTagsProvider.getAll("aws", "cluster", null, null, null, null, null, null, 2)*.id.sort() == [entityTags.id, moreEntityTags.id].sort()
+    entityTagsProvider.getAll("aws", null, "cluster", null, null, null, null, null, null, 2)*.id.sort() == [entityTags.id, moreEntityTags.id].sort()
 
     // fetch everything for `cloudprovider`, `cluster` and `idPrefix`
-    entityTagsProvider.getAll("aws", "cluster", null, "aws:cluster:clouddriver*", null, null, null, null, 2)*.id == [entityTags.id]
+    entityTagsProvider.getAll("aws", null, "cluster", null, "aws:cluster:clouddriver*", null, null, null, null, 2)*.id == [entityTags.id]
 
     // fetch everything for `cloudprovider`, `cluster`, `idPrefix` and `tags`
-    entityTagsProvider.getAll("aws", "cluster", null, "aws*", null, null, null, ["tag3": "value3"], 2)*.id == [entityTags.id]
+    entityTagsProvider.getAll("aws", null, "cluster", null, "aws*", null, null, null, ["tag3": "value3"], 2)*.id == [entityTags.id]
 
     // verify that globbing by tags works (with and without a namespace specified)
-    entityTagsProvider.getAll("aws", "cluster", null, "aws*", null, null, "default", ["tag3": "*"], 2)*.id == [entityTags.id]
-    entityTagsProvider.getAll("aws", "cluster", null, "aws*", null, null, null, ["tag3": "*"], 2)*.id == [entityTags.id]
+    entityTagsProvider.getAll("aws", null, "cluster", null, "aws*", null, null, "default", ["tag3": "*"], 2)*.id == [entityTags.id]
+    entityTagsProvider.getAll("aws", null, "cluster", null, "aws*", null, null, null, ["tag3": "*"], 2)*.id == [entityTags.id]
 
     // namespace 'not_default' does not exist and should negate the matched tags
-    entityTagsProvider.getAll("aws", "cluster", null, "aws*", null, null, "not_default", ["tag3": "*"], 2).isEmpty()
+    entityTagsProvider.getAll("aws", null, "cluster", null, "aws*", null, null, "not_default", ["tag3": "*"], 2).isEmpty()
 
     // verify that `maxResults` works
-    entityTagsProvider.getAll("aws", "cluster", null, null, null, null, null, null, 0).isEmpty()
+    entityTagsProvider.getAll("aws", null, "cluster", null, null, null, null, null, null, 0).isEmpty()
 
   }
 

diff --git a/...b/src/main/groovy/com/netflix/spinnaker/clouddriver/controllers/EntityTagsController.java b/...b/src/main/groovy/com/netflix/spinnaker/clouddriver/controllers/EntityTagsController.java
@@ -51,6 +51,7 @@ public EntityTagsController(MessageSource messageSource,
 
   @RequestMapping(method = RequestMethod.GET)
   public Collection<EntityTags> list(@RequestParam(value = "cloudProvider", required = false) String cloudProvider,
+                                     @RequestParam(value = "application", required = false) String application,
                                      @RequestParam(value = "entityType", required = false) String entityType,
                                      @RequestParam(value = "entityId", required = false) String entityId,
                                      @RequestParam(value = "idPrefix", required = false) String idPrefix,
@@ -66,6 +67,7 @@ public Collection<EntityTags> list(@RequestParam(value = "cloudProvider", requir
 
     return tagProvider.getAll(
       cloudProvider,
+      application,
       entityType,
       entityId != null ? Arrays.asList(entityId.split(",")) : null,
       idPrefix,
@@ -74,7 +76,7 @@ public Collection<EntityTags> list(@RequestParam(value = "cloudProvider", requir
       namespace,
       tags,
       maxResults
-      );
+    );
   }
 
   @RequestMapping(value = "/**", method = RequestMethod.GET)