From d3d70837865c0b3fbe9d86afe704a64ebff3b202 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 9 May 2024 17:58:07 +0000 Subject: [PATCH] fix(jenkins): Wrong Job name encoding in query params for Artifacts/Properties (#4722) (#4724) * fix(jenkins): Wrong Job name encoding in query params for Artifacts/Properties * fix(jenkins): Wrong Job name encoding in query params - added Wiremock tests * fix(jenkins): Wrong Job name encoding in query params - fix Wiremock tests imports (cherry picked from commit b2dba5935bfdb3674961d97ae776fd8eaa90318f) Co-authored-by: Christos Arvanitis --- orca-igor/orca-igor.gradle | 1 + .../spinnaker/orca/igor/IgorService.java | 4 +- .../orca/igor/BuildServiceSpecMock.groovy | 239 ++++++++++++++++++ 3 files changed, 242 insertions(+), 2 deletions(-) create mode 100644 orca-igor/src/test/groovy/com/netflix/spinnaker/orca/igor/BuildServiceSpecMock.groovy diff --git a/orca-igor/orca-igor.gradle b/orca-igor/orca-igor.gradle index 823dca391b..a907cd8ff4 100644 --- a/orca-igor/orca-igor.gradle +++ b/orca-igor/orca-igor.gradle @@ -32,6 +32,7 @@ dependencies { testImplementation(project(":orca-test-groovy")) testImplementation("org.springframework:spring-test") testImplementation("io.spinnaker.fiat:fiat-core:$fiatVersion") + testImplementation("com.github.tomakehurst:wiremock-jre8-standalone") } sourceSets { diff --git a/orca-igor/src/main/java/com/netflix/spinnaker/orca/igor/IgorService.java b/orca-igor/src/main/java/com/netflix/spinnaker/orca/igor/IgorService.java index d1d37512e5..94832e5fd7 100644 --- a/orca-igor/src/main/java/com/netflix/spinnaker/orca/igor/IgorService.java +++ b/orca-igor/src/main/java/com/netflix/spinnaker/orca/igor/IgorService.java @@ -84,7 +84,7 @@ Map getPropertyFileWithJobAsQueryParam( @Path("buildNumber") Integer buildNumber, @Path("fileName") String fileName, @Path("master") String master, - @Query(value = "job") String job); + @Query(encodeValue = false, value = "job") String job); @GET("/{repoType}/{projectKey}/{repositorySlug}/compareCommits") List compareCommits( @@ -105,7 +105,7 @@ List getArtifactsWithJobAsQueryParam( @Path("buildNumber") Integer buildNumber, @Query("propertyFile") String propertyFile, @Path("master") String master, - @Query(value = "job") String job); + @Query(value = "job", encodeValue = false) String job); @POST("/gcb/builds/create/{account}") GoogleCloudBuild createGoogleCloudBuild( diff --git a/orca-igor/src/test/groovy/com/netflix/spinnaker/orca/igor/BuildServiceSpecMock.groovy b/orca-igor/src/test/groovy/com/netflix/spinnaker/orca/igor/BuildServiceSpecMock.groovy new file mode 100644 index 0000000000..dfa4fe3c44 --- /dev/null +++ b/orca-igor/src/test/groovy/com/netflix/spinnaker/orca/igor/BuildServiceSpecMock.groovy @@ -0,0 +1,239 @@ +/* + * Copyright 2024 Harness, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.netflix.spinnaker.orca.igor + +import com.fasterxml.jackson.databind.ObjectMapper +import com.github.tomakehurst.wiremock.WireMockServer +import com.jakewharton.retrofit.Ok3Client +import com.netflix.spinnaker.kork.artifacts.model.Artifact +import retrofit.RestAdapter +import retrofit.client.Response +import retrofit.converter.JacksonConverter +import spock.lang.Specification +import spock.lang.Subject + +import static com.github.tomakehurst.wiremock.client.WireMock.configureFor +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor +import static com.github.tomakehurst.wiremock.client.WireMock.okJson +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse +import static com.github.tomakehurst.wiremock.client.WireMock.get +import static com.github.tomakehurst.wiremock.client.WireMock.put +import static retrofit.Endpoints.newFixedEndpoint + + +class BuildServiceSpecMock extends Specification{ + + static WireMockServer wireMockServer = new WireMockServer(0) + + @Subject BuildService buildService + + private static final MASTER = 'MASTER' + private static final BUILD_NUMBER = 123 + private static final JOB_NAME = "name/with/slashes and spaces" + private static final JOB_NAME_ENCODED = "name/with/slashes%20and%20spaces" + private static final PARAMS = ['key': 'value'] + private static final FILENAME = 'build.properties' + + def setupSpec() { + wireMockServer.start() + configureFor(wireMockServer.port()) + } + def mapper = new ObjectMapper() + IgorService igorService + def setup() { + igorService = new RestAdapter.Builder() + .setEndpoint(newFixedEndpoint(wireMockServer.url("/"))) + .setClient(new Ok3Client()) + .setConverter(new JacksonConverter(mapper)) + .build() + .create(IgorService) + buildService = new BuildService(igorService, new IgorFeatureFlagProperties(jobNameAsQueryParameter: false)) + } + + def cleanupSpec() { + wireMockServer.stop() + } + + def "build starts a Jenkins job"() { + String queryMap = PARAMS.collect { k, v -> "$k=$v" }.join('&') + String uriPath = "/masters/$MASTER/jobs/$JOB_NAME_ENCODED?$queryMap" + stubFor(put(uriPath) + .willReturn( + aResponse() + .withStatus(200) + .withBody(BUILD_NUMBER.toString()) + ) + ) + + when: + Response response = buildService.build(MASTER, JOB_NAME, PARAMS) + + then: + response.status == 200 + } + + + + def "getBuild returns the build details - jobNameAsQueryParameter:false"() { + given: + String uriPath = "/builds/status/$BUILD_NUMBER/$MASTER/$JOB_NAME_ENCODED" + stubFor(get(urlEqualTo(uriPath)) + .willReturn(okJson(getBuildResponse()))) + + when: + Map response = buildService.getBuild(BUILD_NUMBER,MASTER,JOB_NAME) + then: + response.get("name") == JOB_NAME + } + + def "getBuild returns the build details - jobNameAsQueryParameter:true"() { + given: + String uriPath = "/builds/status/$BUILD_NUMBER/$MASTER?job=$JOB_NAME_ENCODED" + IgorFeatureFlagProperties igorFeatureFlagProperties = new IgorFeatureFlagProperties() + igorFeatureFlagProperties.setJobNameAsQueryParameter(true) + buildService = new BuildService(igorService, igorFeatureFlagProperties) + stubFor(get(urlEqualTo(uriPath)) + .willReturn(okJson(getBuildResponse()))) + + when: + Map response = buildService.getBuild(BUILD_NUMBER,MASTER,JOB_NAME) + then: + response.get("name") == JOB_NAME + } + + def "getPropertyFile returns a property file - jobNameAsQueryParameter:false"() { + given: + String uriPath = "/builds/properties/$BUILD_NUMBER/$FILENAME/$MASTER/$JOB_NAME_ENCODED" + stubFor(get(urlEqualTo(uriPath)) + .willReturn( + okJson(getBuildPropertiesResponse()) + ) + ) + + when: + Map response = buildService.getPropertyFile(BUILD_NUMBER, FILENAME, MASTER, JOB_NAME) + + then: + response.get("randomKey") == "randomValue" + } + + def "getPropertyFile returns a property file - jobNameAsQueryParameter:true"() { + given: + String uriPath = "/builds/properties/$BUILD_NUMBER/$FILENAME/$MASTER?job=$JOB_NAME_ENCODED" + IgorFeatureFlagProperties igorFeatureFlagProperties = new IgorFeatureFlagProperties() + igorFeatureFlagProperties.setJobNameAsQueryParameter(true) + buildService = new BuildService(igorService, igorFeatureFlagProperties) + stubFor(get(urlEqualTo(uriPath)) + .willReturn( + okJson(getBuildPropertiesResponse()) + ) + ) + + when: + Map response = buildService.getPropertyFile(BUILD_NUMBER, FILENAME, MASTER, JOB_NAME) + + then: + response.get("randomKey") == "randomValue" + + } + + def "getArtifacts returns build artifacts - jobNameAsQueryParameter:false"() { + given: + String uriPath = "/builds/artifacts/$BUILD_NUMBER/$MASTER/$JOB_NAME_ENCODED?propertyFile=$FILENAME" + stubFor(get(urlEqualTo(uriPath)) + .willReturn( + okJson(getArtifactsResponse()) + ) + ) + + when: + List response = buildService.getArtifacts(BUILD_NUMBER, FILENAME, MASTER, JOB_NAME) + + then: + response.toString() == "[]" + } + + def "getArtifacts returns build artifacts - jobNameAsQueryParameter:true"() { + given: + String uriPath = "/builds/artifacts/$BUILD_NUMBER/$MASTER?propertyFile=$FILENAME&job=$JOB_NAME_ENCODED" + IgorFeatureFlagProperties igorFeatureFlagProperties = new IgorFeatureFlagProperties() + igorFeatureFlagProperties.setJobNameAsQueryParameter(true) + buildService = new BuildService(igorService, igorFeatureFlagProperties) + stubFor(get(urlEqualTo(uriPath)) + .willReturn( + okJson(getArtifactsResponse()) + ) + ) + + when: + List response = buildService.getArtifacts(BUILD_NUMBER, FILENAME, MASTER, JOB_NAME) + + then: + response.toString() == "[]" + } + + + private static String getBuildResponse() { + return "{\n" + + " \"building\": false,\n" + + " \"fullDisplayName\": \"$JOB_NAME #$BUILD_NUMBER\",\n" + + " \"name\": \"$JOB_NAME\",\n" + + " \"number\": $BUILD_NUMBER,\n" + + " \"duration\": 778,\n" + + " \"timestamp\": \"1714730108196\",\n" + + " \"result\": \"SUCCESS\",\n" + + " \"artifacts\": [\n" + + " {\n" + + " \"fileName\": \"$FILENAME\",\n" + + " \"displayPath\": \"$FILENAME\",\n" + + " \"relativePath\": \"$FILENAME\",\n" + + " \"reference\": \"$FILENAME\",\n" + + " \"name\": \"$JOB_NAME\",\n" + + " \"type\": \"jenkins/file\",\n" + + " \"version\": \"6\",\n" + + " \"decorated\": false\n" + + " }\n" + + " ],\n" + + " \"testResults\": [\n" + + " {\n" + + " \"failCount\": 0,\n" + + " \"skipCount\": 0,\n" + + " \"totalCount\": 0,\n" + + " \"urlName\": null,\n" + + " \"cause\": {\n" + + " \"shortDescription\": \"Started by user Spinnaker\",\n" + + " \"upstreamBuild\": null,\n" + + " \"upstreamProject\": null,\n" + + " \"upstreamUrl\": null\n" + + " }\n" + + " }\n" + + " ],\n" + + " \"url\": \"https://jenkins.somedomain.com/job/$JOB_NAME_ENCODED/$BUILD_NUMBER/\"\n" + + "}" + } + + private static String getBuildPropertiesResponse() { + return "{\"randomKey\":\"randomValue\"}" + } + + private static String getArtifactsResponse() { + return "[]" + } + + +}