diff --git a/api/api-elastic-profile-v2/src/main/java/com/thoughtworks/go/apiv2/elasticprofile/ElasticProfileControllerV2.java b/api/api-elastic-profile-v2/src/main/java/com/thoughtworks/go/apiv2/elasticprofile/ElasticProfileControllerV2.java index 518b18dbe84e..ff4d321d38fd 100644 --- a/api/api-elastic-profile-v2/src/main/java/com/thoughtworks/go/apiv2/elasticprofile/ElasticProfileControllerV2.java +++ b/api/api-elastic-profile-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,6 +110,7 @@ public String show(Request request, Response response) throws IOException { public String create(Request request, Response response) { final ElasticProfile elasticProfileToCreate = buildEntityFromRequestBody(request); haltIfEntityWithSameIdExists(elasticProfileToCreate); + populateSpecifiedPluginIdOnTheElasticAgentProfile(elasticProfileToCreate); final HttpLocalizedOperationResult operationResult = new HttpLocalizedOperationResult(); elasticProfileService.create(currentUsername(), elasticProfileToCreate, operationResult); @@ -113,6 +118,13 @@ public String create(Request request, Response response) { return handleCreateOrUpdateResponse(request, response, elasticProfileToCreate, operationResult); } + private void populateSpecifiedPluginIdOnTheElasticAgentProfile(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()); } + populateSpecifiedPluginIdOnTheElasticAgentProfile(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/api/api-elastic-profile-v2/src/main/java/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfileRepresenter.java b/api/api-elastic-profile-v2/src/main/java/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfileRepresenter.java index 9f0f0a70ac23..bf3d8b91944f 100644 --- a/api/api-elastic-profile-v2/src/main/java/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfileRepresenter.java +++ b/api/api-elastic-profile-v2/src/main/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/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/ElasticProfileControllerV2Test.groovy b/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/ElasticProfileControllerV2Test.groovy index f8fa09597201..9e788c70a4ee 100644 --- a/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/ElasticProfileControllerV2Test.groovy +++ b/api/api-elastic-profile-v2/src/test/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,6 +57,9 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller @Mock private EntityHashingService entityHashingService + @Mock + private ClusterProfilesService clusterProfileService + @BeforeEach void setup() { initMocks(this) @@ -62,7 +67,7 @@ class ElasticProfileControllerV2Test implements SecurityServiceTrait, Controller @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."]] diff --git a/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfileRepresenterTest.groovy b/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfileRepresenterTest.groovy index fef60bea07f7..5c118a5b1357 100644 --- a/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfileRepresenterTest.groovy +++ b/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfileRepresenterTest.groovy @@ -18,12 +18,7 @@ package com.thoughtworks.go.apiv2.elasticprofile.representers import com.thoughtworks.go.api.util.GsonTransformer import com.thoughtworks.go.config.elastic.ElasticProfile -import com.thoughtworks.go.plugin.access.elastic.ElasticAgentMetadataStore import com.thoughtworks.go.plugin.api.info.PluginDescriptor -import com.thoughtworks.go.plugin.domain.common.Metadata -import com.thoughtworks.go.plugin.domain.common.PluggableInstanceSettings -import com.thoughtworks.go.plugin.domain.common.PluginConfiguration -import com.thoughtworks.go.plugin.domain.elastic.ElasticAgentPluginInfo import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test import spark.HaltException @@ -44,7 +39,6 @@ class ElasticProfileRepresenterTest { void shouldCreateObjectFromJson() { def elasticProfile = [ id : 'docker', - plugin_id : 'cd.go.docker', cluster_profile_id: 'foo', properties : [ [ @@ -54,7 +48,7 @@ class ElasticProfileRepresenterTest { ] ] - def expectedObject = new ElasticProfile('docker', 'cd.go.docker', 'foo', create('DockerURI', false, 'http://foo')) + def expectedObject = new ElasticProfile('docker', null, 'foo', create('DockerURI', false, 'http://foo')) def jsonReader = GsonTransformer.instance.jsonReaderFrom(elasticProfile) def object = ElasticProfileRepresenter.fromJSON(jsonReader) @@ -64,7 +58,7 @@ class ElasticProfileRepresenterTest { @Test void shouldAddErrorsToJson() { - def elasticProfile = new ElasticProfile('docker', 'cd.go.docker', 'foo', create('DockerURI', false, 'http://foo')) + def elasticProfile = new ElasticProfile('docker', null, 'foo', create('DockerURI', false, 'http://foo')) elasticProfile.addError("pluginId", "Invalid Plugin Id") def expectedJson = [ @@ -74,7 +68,6 @@ class ElasticProfileRepresenterTest { find: [href: 'http://test.host/go/api/elastic/profiles/:profile_id'], ], id : 'docker', - plugin_id : 'cd.go.docker', cluster_profile_id: 'foo', properties : [ [ @@ -92,66 +85,41 @@ class ElasticProfileRepresenterTest { assertThatJson(json).isEqualTo(expectedJson) } - @Test - void shouldEncryptSecureValues() { - def elasticProfile = [ - id : 'docker', - plugin_id : 'cd.go.docker', - cluster_profile_id: 'foo', - properties : [ - [ - "key" : "Password", - "value": "passw0rd1" - ] - ] - ] - def elasticAgentMetadataStore = ElasticAgentMetadataStore.instance() - PluggableInstanceSettings pluggableInstanceSettings = new PluggableInstanceSettings(Arrays.asList( - new PluginConfiguration("Password", new Metadata(true, true)))) - elasticAgentMetadataStore.setPluginInfo(new ElasticAgentPluginInfo(pluginDescriptor(), pluggableInstanceSettings, pluggableInstanceSettings, null, null, null)) - def jsonReader = GsonTransformer.instance.jsonReaderFrom(elasticProfile) - - def object = ElasticProfileRepresenter.fromJSON(jsonReader) - - assertThat(object.getProperty("Password").isSecure()).isTrue() - } - } - - @Nested - class WithoutClusterProfileId { - @Test - void shouldThrowHaltExceptionForNotSpecifyingClusterProfileId() { - def elasticProfile = [ - id : 'docker', - plugin_id : 'cd.go.docker', - properties: [ - [ - "key" : "DockerURI", - "value": "http://foo" + @Nested + class WithoutClusterProfileId { + @Test + void shouldThrowHaltExceptionForNotSpecifyingClusterProfileId() { + def elasticProfile = [ + id : 'docker', + properties: [ + [ + "key" : "DockerURI", + "value": "http://foo" + ] ] ] - ] - def jsonReader = GsonTransformer.instance.jsonReaderFrom(elasticProfile) - assertThrows(HaltException.class, { ElasticProfileRepresenter.fromJSON(jsonReader) }) - } - } - - private static PluginDescriptor pluginDescriptor() { - return new PluginDescriptor() { - @Override - String id() { - return "cd.go.docker" - } - - @Override - String version() { - return null + def jsonReader = GsonTransformer.instance.jsonReaderFrom(elasticProfile) + assertThrows(HaltException.class, { ElasticProfileRepresenter.fromJSON(jsonReader) }) } + } - @Override - PluginDescriptor.About about() { - return null + private static PluginDescriptor pluginDescriptor() { + return new PluginDescriptor() { + @Override + String id() { + return "cd.go.docker" + } + + @Override + String version() { + return null + } + + @Override + PluginDescriptor.About about() { + return null + } } } } diff --git a/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfilesRepresenterTest.groovy b/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfilesRepresenterTest.groovy index dc3742bf5da7..cad097f36381 100644 --- a/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfilesRepresenterTest.groovy +++ b/api/api-elastic-profile-v2/src/test/groovy/com/thoughtworks/go/apiv2/elasticprofile/representers/ElasticProfilesRepresenterTest.groovy @@ -42,7 +42,6 @@ class ElasticProfilesRepresenterTest { find: [href: 'http://test.host/go/api/elastic/profiles/:profile_id'], ], id : 'docker', - plugin_id : 'cd.go.docker', cluster_profile_id: 'foo', "properties" : [ [ @@ -58,7 +57,6 @@ class ElasticProfilesRepresenterTest { find: [href: 'http://test.host/go/api/elastic/profiles/:profile_id'], ], id : 'ecs', - plugin_id : 'cd.go.ecs', cluster_profile_id: "bar", "properties" : [ [