Remove plugin id from elastic agent profiles API (gocd#5538)

GaneshSPatil · Apr 22, 2019 · 9053d22 · 9053d22
1 parent c590f5d
commit 9053d22
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 81 deletions.
diff --git a/...v2/src/main/java/com/thoughtworks/go/apiv2/elasticprofile/ElasticProfileControllerV2.java b/...v2/src/main/java/com/thoughtworks/go/apiv2/elasticprofile/ElasticProfileControllerV2.java
@@ -25,9 +25,11 @@
 import com.thoughtworks.go.apiv2.elasticprofile.representers.ElasticProfileRepresenter;
 import com.thoughtworks.go.apiv2.elasticprofile.representers.ElasticProfilesRepresenter;
 import com.thoughtworks.go.config.PluginProfiles;
+import com.thoughtworks.go.config.elastic.ClusterProfile;
 import com.thoughtworks.go.config.elastic.ElasticProfile;
 import com.thoughtworks.go.config.exceptions.EntityType;
 import com.thoughtworks.go.config.exceptions.HttpException;
+import com.thoughtworks.go.server.service.ClusterProfilesService;
 import com.thoughtworks.go.server.service.ElasticProfileService;
 import com.thoughtworks.go.server.service.EntityHashingService;
 import com.thoughtworks.go.server.service.result.HttpLocalizedOperationResult;
@@ -53,13 +55,15 @@ public class ElasticProfileControllerV2 extends ApiController implements SparkSp
     private final ElasticProfileService elasticProfileService;
     private final ApiAuthenticationHelper apiAuthenticationHelper;
     private final EntityHashingService entityHashingService;
+    private ClusterProfilesService clusterProfilesService;
 
     @Autowired
-    public ElasticProfileControllerV2(ElasticProfileService elasticProfileService, ApiAuthenticationHelper apiAuthenticationHelper, EntityHashingService entityHashingService) {
+    public ElasticProfileControllerV2(ElasticProfileService elasticProfileService, ApiAuthenticationHelper apiAuthenticationHelper, EntityHashingService entityHashingService, ClusterProfilesService clusterProfilesService) {
         super(ApiVersion.v2);
         this.elasticProfileService = elasticProfileService;
         this.apiAuthenticationHelper = apiAuthenticationHelper;
         this.entityHashingService = entityHashingService;
+        this.clusterProfilesService = clusterProfilesService;
     }
 
     @Override
@@ -106,13 +110,21 @@ public String show(Request request, Response response) throws IOException {
     public String create(Request request, Response response) {
         final ElasticProfile elasticProfileToCreate = buildEntityFromRequestBody(request);
         haltIfEntityWithSameIdExists(elasticProfileToCreate);
+        populatePluginIdFromReferencedClusterProfile(elasticProfileToCreate);
 
         final HttpLocalizedOperationResult operationResult = new HttpLocalizedOperationResult();
         elasticProfileService.create(currentUsername(), elasticProfileToCreate, operationResult);
 
         return handleCreateOrUpdateResponse(request, response, elasticProfileToCreate, operationResult);
     }
 
+    private void populatePluginIdFromReferencedClusterProfile(ElasticProfile elasticProfileToCreate) {
+        ClusterProfile associatedClusterProfile = clusterProfilesService.findProfile(elasticProfileToCreate.getClusterProfileId());
+        haltIfSpecifiedClusterProfileDoesntExists(associatedClusterProfile, elasticProfileToCreate);
+        elasticProfileToCreate.setPluginId(associatedClusterProfile.getPluginId());
+        elasticProfileToCreate.encryptSecureConfigurations();
+    }
+
     public String update(Request request, Response response) {
         final String profileId = request.params(PROFILE_ID_PARAM);
         final ElasticProfile existingElasticProfile = fetchEntityFromConfig(profileId);
@@ -126,6 +138,8 @@ public String update(Request request, Response response) {
             throw haltBecauseEtagDoesNotMatch("elasticProfile", existingElasticProfile.getId());
         }
 
+        populatePluginIdFromReferencedClusterProfile(newElasticProfile);
+
         final HttpLocalizedOperationResult operationResult = new HttpLocalizedOperationResult();
         elasticProfileService.update(currentUsername(), etagFor(existingElasticProfile), newElasticProfile, operationResult);
 
@@ -179,4 +193,15 @@ private void haltIfEntityWithSameIdExists(ElasticProfile elasticProfile) {
         elasticProfile.addError("id", format("Elastic profile ids should be unique. Elastic profile with id '%s' already exists.", elasticProfile.getId()));
         throw haltBecauseEntityAlreadyExists(jsonWriter(elasticProfile), "elasticProfile", elasticProfile.getId());
     }
+
+    private void haltIfSpecifiedClusterProfileDoesntExists(ClusterProfile clusterProfile, ElasticProfile elasticProfile) {
+        if (clusterProfile != null) {
+            return;
+        }
+
+        String errorMsg = format("No Cluster Profile exists with the specified cluster_profile_id '%s'.", elasticProfile.getClusterProfileId());
+        elasticProfile.addError("cluster_profile_id", errorMsg);
+
+        throw haltBecauseOfReason(errorMsg);
+    }
 }
diff --git a/...java/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfileRepresenter.java b/...java/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfileRepresenter.java
@@ -34,7 +34,6 @@ public static void toJSON(OutputWriter outputWriter, ElasticProfile elasticProfi
                         .addAbsoluteLink("doc", Routes.ElasticProfileAPI.DOC)
                         .addLink("find", Routes.ElasticProfileAPI.find()))
                 .add("id", elasticProfile.getId())
-                .add("plugin_id", elasticProfile.getPluginId())
                 .add("cluster_profile_id", elasticProfile.getClusterProfileId())
                 .addChildList("properties", listWriter ->
                         elasticProfile.forEach(property ->
@@ -49,7 +48,7 @@ public static void toJSON(OutputWriter outputWriter, ElasticProfile elasticProfi
     public static ElasticProfile fromJSON(JsonReader jsonReader) {
         ElasticProfile elasticProfile = new ElasticProfile(
                 jsonReader.getString("id"),
-                jsonReader.getString("plugin_id"),
+                null,
                 jsonReader.getString("cluster_profile_id"));
         elasticProfile.addConfigurations(ConfigurationPropertyRepresenter.fromJSONArray(jsonReader, "properties"));
         return elasticProfile;

diff --git a/...est/groovy/com/thoughtworks/go/apiv2/elasticprofile/ElasticProfileControllerV2Test.groovy b/...est/groovy/com/thoughtworks/go/apiv2/elasticprofile/ElasticProfileControllerV2Test.groovy
@@ -20,10 +20,12 @@ import com.thoughtworks.go.api.SecurityTestTrait
 import com.thoughtworks.go.api.spring.ApiAuthenticationHelper
 import com.thoughtworks.go.apiv2.elasticprofile.representers.ElasticProfileRepresenter
 import com.thoughtworks.go.apiv2.elasticprofile.representers.ElasticProfilesRepresenter
+import com.thoughtworks.go.config.elastic.ClusterProfile
 import com.thoughtworks.go.config.elastic.ElasticProfile
 import com.thoughtworks.go.config.elastic.ElasticProfiles
 import com.thoughtworks.go.i18n.LocalizedMessage
 import com.thoughtworks.go.server.domain.Username
+import com.thoughtworks.go.server.service.ClusterProfilesService
 import com.thoughtworks.go.server.service.ElasticProfileService
 import com.thoughtworks.go.server.service.EntityHashingService
 import com.thoughtworks.go.server.service.result.HttpLocalizedOperationResult
@@ -55,14 +57,17 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
   @Mock
   private EntityHashingService entityHashingService
 
+  @Mock
+  private ClusterProfilesService clusterProfileService
+
   @BeforeEach
   void setup() {
     initMocks(this)
   }
 
   @Override
   ElasticProfileControllerV2 createControllerInstance() {
-    return new ElasticProfileControllerV2(elasticProfileService, new ApiAuthenticationHelper(securityService, goConfigService), entityHashingService)
+    return new ElasticProfileControllerV2(elasticProfileService, new ApiAuthenticationHelper(securityService, goConfigService), entityHashingService, clusterProfileService)
   }
 
   @Nested
@@ -220,7 +225,6 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
       void 'should create elastic profile from given json payload'() {
         def jsonPayload = [
           id                : 'docker',
-          plugin_id         : 'cd.go.docker',
           cluster_profile_id: "prod-cluster",
           properties        : [
             [
@@ -229,6 +233,7 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
             ]
           ]]
 
+        when(clusterProfileService.findProfile("prod-cluster")).thenReturn(new ClusterProfile("prod-cluster", "cd.go.docker"))
         when(entityHashingService.md5ForEntity(Mockito.any() as ElasticProfile)).thenReturn('some-md5')
 
         postWithApiHeader(controller.controllerPath(), jsonPayload)
@@ -240,12 +245,31 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
           .hasBodyWithJsonObject(new ElasticProfile("docker", "cd.go.docker", "prod-cluster", create("DockerURI", false, "http://foo")), ElasticProfileRepresenter)
       }
 
+      @Test
+      void 'should not create elastic profile from given json payload when specified cluster profile does not exists'() {
+        def jsonPayload = [
+          id                : 'docker',
+          cluster_profile_id: "prod-cluster",
+          properties        : [
+            [
+              "key"  : "DockerURI",
+              "value": "http://foo"
+            ]
+          ]]
+
+        when(entityHashingService.md5ForEntity(Mockito.any() as ElasticProfile)).thenReturn('some-md5')
+
+        postWithApiHeader(controller.controllerPath(), jsonPayload)
+
+        assertThatResponse()
+          .isUnprocessableEntity()
+          .hasJsonMessage("No Cluster Profile exists with the specified cluster_profile_id 'prod-cluster'.")
+      }
 
       @Test
       void 'should not create elastic profile in case of validation error and return the profile with errors'() {
         def jsonPayload = [
           id                : 'docker',
-          plugin_id         : 'cd.go.docker',
           cluster_profile_id: "prod-cluster",
           properties        : [
             [
@@ -254,6 +278,7 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
             ]
           ]]
 
+        when(clusterProfileService.findProfile("prod-cluster")).thenReturn(new ClusterProfile("prod-cluster", "cd.go.docker"))
         when(elasticProfileService.create(Mockito.any() as Username, Mockito.any() as ElasticProfile, Mockito.any() as LocalizedOperationResult))
           .then({ InvocationOnMock invocation ->
           ElasticProfile elasticProfile = invocation.getArguments()[1]
@@ -268,7 +293,6 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
           message: "validation failed",
           data   : [
             id                : "docker",
-            plugin_id         : "cd.go.docker",
             cluster_profile_id: "prod-cluster",
             properties        : [[key: "DockerURI", value: "http://foo"]],
             errors            : [plugin_id: ["Plugin not installed."]]
@@ -286,7 +310,6 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
         def existingElasticProfile = new ElasticProfile("docker", "cd.go.docker", 'foo', create("DockerURI", false, "http://foo"))
         def jsonPayload = [
           id                : 'docker',
-          plugin_id         : 'cd.go.docker',
           cluster_profile_id: "prod-cluster",
           properties        : [
             [
@@ -300,12 +323,10 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
 
         postWithApiHeader(controller.controllerPath(), jsonPayload)
 
-
         def expectedResponseBody = [
           message: "Failed to add elasticProfile 'docker'. Another elasticProfile with the same name already exists.",
           data   : [
             id                : "docker",
-            plugin_id         : "cd.go.docker",
             cluster_profile_id: "prod-cluster",
             properties        : [[key: "DockerURI", value: "http://foo"]],
             errors            : [id: ["Elastic profile ids should be unique. Elastic profile with id 'docker' already exists."]]
@@ -351,7 +372,6 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
         def updatedProfile = new ElasticProfile("docker", "cd.go.docker", "prod-cluster", create("DockerURI", false, "http://new-uri"))
         def jsonPayload = [
           id                : 'docker',
-          plugin_id         : 'cd.go.docker',
           cluster_profile_id: "prod-cluster",
           properties        : [
             [
@@ -360,6 +380,7 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
             ]
           ]]
 
+        when(clusterProfileService.findProfile("prod-cluster")).thenReturn(new ClusterProfile("prod-cluster", "cd.go.docker"))
         when(entityHashingService.md5ForEntity(existingProfile)).thenReturn('some-md5')
         when(entityHashingService.md5ForEntity(updatedProfile)).thenReturn('new-md5')
         when(elasticProfileService.findProfile("docker")).thenReturn(existingProfile)
@@ -373,12 +394,36 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
           .hasBodyWithJsonObject(updatedProfile, ElasticProfileRepresenter)
       }
 
+      @Test
+      void 'should not update elastic profile when specified cluster profile does not exists'() {
+        def existingProfile = new ElasticProfile("docker", "cd.go.docker", "prod-cluster", create("DockerURI", false, "http://foo"))
+        def updatedProfile = new ElasticProfile("docker", "cd.go.docker", "prod-cluster", create("DockerURI", false, "http://new-uri"))
+        def jsonPayload = [
+          id                : 'docker',
+          cluster_profile_id: "prod-cluster",
+          properties        : [
+            [
+              "key"  : "DockerURI",
+              "value": "http://new-uri"
+            ]
+          ]]
+
+        when(entityHashingService.md5ForEntity(existingProfile)).thenReturn('some-md5')
+        when(entityHashingService.md5ForEntity(updatedProfile)).thenReturn('new-md5')
+        when(elasticProfileService.findProfile("docker")).thenReturn(existingProfile)
+
+        putWithApiHeader(controller.controllerPath("/docker"), ['if-match': 'some-md5'], jsonPayload)
+
+        assertThatResponse()
+          .isUnprocessableEntity()
+          .hasJsonMessage("No Cluster Profile exists with the specified cluster_profile_id 'prod-cluster'.")
+      }
+
       @Test
       void 'should not update elastic profile if etag does not match'() {
         def existingProfile = new ElasticProfile("docker", "cd.go.docker", "prod-cluster", create("DockerURI", false, "http://foo"))
         def jsonPayload = [
           id                : 'docker',
-          plugin_id         : 'cd.go.docker',
           cluster_profile_id: "prod-cluster",
           properties        : [
             [
@@ -414,7 +459,6 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
         def existingProfile = new ElasticProfile("docker", "cd.go.docker", "prod-cluster", create("DockerURI", false, "http://foo"))
         def jsonPayload = [
           id                : 'docker-new',
-          plugin_id         : 'cd.go.docker',
           cluster_profile_id: "prod-cluster",
           properties        : [
             [
@@ -439,7 +483,6 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
         def existingProfile = new ElasticProfile("docker", "cd.go.docker", "prod-cluster", create("DockerURI", false, "http://foo"))
         def jsonPayload = [
           id                : 'docker',
-          plugin_id         : 'cd.go.docker',
           cluster_profile_id: "prod-cluster",
           properties        : [
             [
@@ -448,6 +491,7 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
             ]
           ]]
 
+        when(clusterProfileService.findProfile("prod-cluster")).thenReturn(new ClusterProfile("prod-cluster", "cd.go.docker"))
         when(entityHashingService.md5ForEntity(existingProfile)).thenReturn('some-md5')
         when(elasticProfileService.findProfile("docker")).thenReturn(existingProfile)
 
@@ -465,7 +509,6 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller
           message: "validation failed",
           data   : [
             id                : "docker",
-            plugin_id         : "cd.go.docker",
             cluster_profile_id: "prod-cluster",
             properties        : [[key: "DockerURI", value: "http://foo"]],
             errors            : [plugin_id: ["Plugin not installed."]]