From 160c8c27e6a8732647c54d48eeff693a4117b890 Mon Sep 17 00:00:00 2001 From: kirangodishala Date: Fri, 9 May 2025 12:27:54 +0530 Subject: [PATCH 1/4] test(jenkins): add a test to demonstrate the issue of Retrofit2EncodeCorrectionInterceptor getting added to retrofit1 client of JenkinsClient --- .../spinnaker/igor/JenkinsClientSpec.groovy | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy new file mode 100644 index 000000000..43d964814 --- /dev/null +++ b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy @@ -0,0 +1,105 @@ +/* + * Copyright 2025 OpsMx, 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.igor + +import com.netflix.spectator.api.NoopRegistry +import com.netflix.spectator.api.Registry +import com.netflix.spinnaker.igor.config.JenkinsConfig +import com.netflix.spinnaker.igor.config.client.JenkinsOkHttpClientProvider +import com.netflix.spinnaker.igor.service.BuildServices +import com.netflix.spinnaker.okhttp.Retrofit2EncodeCorrectionInterceptor +import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry +import io.github.resilience4j.circuitbreaker.internal.InMemoryCircuitBreakerRegistry; +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.task.TaskExecutorBuilder; +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.context.annotation.Bean; +import com.netflix.spinnaker.config.okhttp3.RawOkHttpClientConfiguration; +import com.netflix.spinnaker.igor.config.JenkinsProperties +import com.netflix.spinnaker.config.OkHttpClientComponents +import retrofit.RestAdapter +import spock.lang.Specification; + +@SpringBootTest(classes = [JenkinsConfig, RawOkHttpClientConfiguration, OkHttpClientComponents, TestConfiguration], + properties = ["jenkins.enabled=true"]) +class JenkinsClientSpec extends Specification { + + @Autowired + JenkinsOkHttpClientProvider jenkinsOkHttpClientProvider + + @Autowired + JenkinsProperties.JenkinsHost jenkinsHost + + def "test if jenkins retrofit1 client has retrofit2 interceptor - Retrofit2EncodeCorrectionInterceptor"() { + when: + def client = jenkinsOkHttpClientProvider.provide(jenkinsHost) + def interceptors = client.interceptors() + + then: + interceptors.size() != 0 + //TODO: Fix this. Retrofit2EncodeCorrectionInterceptor should not be added to a retrofit1 client + interceptors.any { it instanceof Retrofit2EncodeCorrectionInterceptor } + } +} + +class TestConfiguration { + @Bean + TaskExecutorBuilder taskExecutorBuilder() { + return new TaskExecutorBuilder() + } + + @Bean + JenkinsProperties.JenkinsHost jenkinsHost() { + def host = new JenkinsProperties.JenkinsHost() + host.address = "http://localhost:8080" + host.name = "jenkins-dev" + return host + } + + @Bean + JenkinsProperties jenkinsProperties(JenkinsProperties.JenkinsHost jenkinsHost) { + def jenkinsProperties = new JenkinsProperties() + jenkinsProperties.masters = [jenkinsHost] + return jenkinsProperties + } + + @Bean + BuildServices buildServices() { + def buildServices = new BuildServices() + return buildServices + } + + @Bean + IgorConfigurationProperties igorConfigurationProperties() { + new IgorConfigurationProperties() + } + + @Bean + Registry registry() { + new NoopRegistry() + } + + @Bean + CircuitBreakerRegistry circuitBreakerRegistry() { + new InMemoryCircuitBreakerRegistry() + } + + @Bean + RestAdapter.LogLevel retrofitLogLevel() { + return RestAdapter.LogLevel.BASIC + } +} From 48b240d90004858f2606a4eb0a13dee14324dce4 Mon Sep 17 00:00:00 2001 From: kirangodishala Date: Fri, 9 May 2025 12:43:27 +0530 Subject: [PATCH 2/4] fix(jenkins): fix the issue of Retrofit2EncodeCorrectionInterceptor getting added to retrofit1 client of JenkinsClient. More details on this issue: https://github.com/spinnaker/spinnaker/issues/7059 --- .../igor/config/JenkinsConfig.groovy | 6 ++++++ .../spinnaker/igor/JenkinsClientSpec.groovy | 20 ++++++++++++------- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/JenkinsConfig.groovy b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/JenkinsConfig.groovy index 4d55f1060..51fc1a3b6 100644 --- a/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/JenkinsConfig.groovy +++ b/igor-web/src/main/groovy/com/netflix/spinnaker/igor/config/JenkinsConfig.groovy @@ -22,6 +22,7 @@ import com.fasterxml.jackson.dataformat.xml.XmlMapper import com.fasterxml.jackson.module.jaxb.JaxbAnnotationModule import com.jakewharton.retrofit.Ok3Client import com.netflix.spectator.api.Registry +import com.netflix.spinnaker.config.OkHttp3ClientConfiguration import com.netflix.spinnaker.fiat.model.resources.Permissions import com.netflix.spinnaker.igor.IgorConfigurationProperties import com.netflix.spinnaker.igor.config.client.DefaultJenkinsOkHttpClientProvider @@ -69,6 +70,11 @@ import java.util.concurrent.TimeUnit @EnableConfigurationProperties(JenkinsProperties) class JenkinsConfig { + @Bean + OkHttpClient okHttpClient(OkHttp3ClientConfiguration okHttpClientConfig) { + return okHttpClientConfig.create().build(); + } + @Bean @ConditionalOnMissingBean JenkinsOkHttpClientProvider jenkinsOkHttpClientProvider(OkHttpClient okHttpClient) { diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy index 43d964814..17565e913 100644 --- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy +++ b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy @@ -22,19 +22,21 @@ import com.netflix.spinnaker.igor.config.JenkinsConfig import com.netflix.spinnaker.igor.config.client.JenkinsOkHttpClientProvider import com.netflix.spinnaker.igor.service.BuildServices import com.netflix.spinnaker.okhttp.Retrofit2EncodeCorrectionInterceptor +import com.netflix.spinnaker.config.OkHttp3ClientConfiguration +import com.netflix.spinnaker.config.okhttp3.RawOkHttpClientConfiguration; +import com.netflix.spinnaker.igor.config.JenkinsProperties +import com.netflix.spinnaker.config.OkHttpClientComponents import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry -import io.github.resilience4j.circuitbreaker.internal.InMemoryCircuitBreakerRegistry; +import io.github.resilience4j.circuitbreaker.internal.InMemoryCircuitBreakerRegistry +import okhttp3.logging.HttpLoggingInterceptor; import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.task.TaskExecutorBuilder; import org.springframework.boot.test.context.SpringBootTest import org.springframework.context.annotation.Bean; -import com.netflix.spinnaker.config.okhttp3.RawOkHttpClientConfiguration; -import com.netflix.spinnaker.igor.config.JenkinsProperties -import com.netflix.spinnaker.config.OkHttpClientComponents import retrofit.RestAdapter import spock.lang.Specification; -@SpringBootTest(classes = [JenkinsConfig, RawOkHttpClientConfiguration, OkHttpClientComponents, TestConfiguration], +@SpringBootTest(classes = [JenkinsConfig, RawOkHttpClientConfiguration, OkHttpClientComponents, TestConfiguration, OkHttp3ClientConfiguration], properties = ["jenkins.enabled=true"]) class JenkinsClientSpec extends Specification { @@ -51,8 +53,7 @@ class JenkinsClientSpec extends Specification { then: interceptors.size() != 0 - //TODO: Fix this. Retrofit2EncodeCorrectionInterceptor should not be added to a retrofit1 client - interceptors.any { it instanceof Retrofit2EncodeCorrectionInterceptor } + !interceptors.any { it instanceof Retrofit2EncodeCorrectionInterceptor } } } @@ -102,4 +103,9 @@ class TestConfiguration { RestAdapter.LogLevel retrofitLogLevel() { return RestAdapter.LogLevel.BASIC } + + @Bean + HttpLoggingInterceptor.Level retrofitInterceptorLogLevel() { + return HttpLoggingInterceptor.Level.BODY + } } From f388cc0c58117aedf3fa6f743f1d370c3971645f Mon Sep 17 00:00:00 2001 From: kirangodishala Date: Fri, 9 May 2025 23:57:43 +0530 Subject: [PATCH 3/4] fix(jenkins): address review comments --- .../com/netflix/spinnaker/igor/JenkinsClientSpec.groovy | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy index 17565e913..ca552f16e 100644 --- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy +++ b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy @@ -28,13 +28,13 @@ import com.netflix.spinnaker.igor.config.JenkinsProperties import com.netflix.spinnaker.config.OkHttpClientComponents import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry import io.github.resilience4j.circuitbreaker.internal.InMemoryCircuitBreakerRegistry -import okhttp3.logging.HttpLoggingInterceptor; +import okhttp3.logging.HttpLoggingInterceptor import org.springframework.beans.factory.annotation.Autowired -import org.springframework.boot.task.TaskExecutorBuilder; +import org.springframework.boot.task.TaskExecutorBuilder import org.springframework.boot.test.context.SpringBootTest -import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Bean import retrofit.RestAdapter -import spock.lang.Specification; +import spock.lang.Specification @SpringBootTest(classes = [JenkinsConfig, RawOkHttpClientConfiguration, OkHttpClientComponents, TestConfiguration, OkHttp3ClientConfiguration], properties = ["jenkins.enabled=true"]) From 3ff78ca7623f9af8a9f2b5145c44ebd9a47fc8e7 Mon Sep 17 00:00:00 2001 From: kirangodishala Date: Sat, 10 May 2025 00:45:11 +0530 Subject: [PATCH 4/4] test(jenkins): Convert JenkinsClientSpec groovy test to java --- .../spinnaker/igor/JenkinsClientSpec.groovy | 111 ---------------- .../igor/config/JenkinsClientTest.java | 118 ++++++++++++++++++ 2 files changed, 118 insertions(+), 111 deletions(-) delete mode 100644 igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy create mode 100644 igor-web/src/test/java/com/netflix/spinnaker/igor/config/JenkinsClientTest.java diff --git a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy b/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy deleted file mode 100644 index ca552f16e..000000000 --- a/igor-web/src/test/groovy/com/netflix/spinnaker/igor/JenkinsClientSpec.groovy +++ /dev/null @@ -1,111 +0,0 @@ -/* - * Copyright 2025 OpsMx, 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.igor - -import com.netflix.spectator.api.NoopRegistry -import com.netflix.spectator.api.Registry -import com.netflix.spinnaker.igor.config.JenkinsConfig -import com.netflix.spinnaker.igor.config.client.JenkinsOkHttpClientProvider -import com.netflix.spinnaker.igor.service.BuildServices -import com.netflix.spinnaker.okhttp.Retrofit2EncodeCorrectionInterceptor -import com.netflix.spinnaker.config.OkHttp3ClientConfiguration -import com.netflix.spinnaker.config.okhttp3.RawOkHttpClientConfiguration; -import com.netflix.spinnaker.igor.config.JenkinsProperties -import com.netflix.spinnaker.config.OkHttpClientComponents -import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry -import io.github.resilience4j.circuitbreaker.internal.InMemoryCircuitBreakerRegistry -import okhttp3.logging.HttpLoggingInterceptor -import org.springframework.beans.factory.annotation.Autowired -import org.springframework.boot.task.TaskExecutorBuilder -import org.springframework.boot.test.context.SpringBootTest -import org.springframework.context.annotation.Bean -import retrofit.RestAdapter -import spock.lang.Specification - -@SpringBootTest(classes = [JenkinsConfig, RawOkHttpClientConfiguration, OkHttpClientComponents, TestConfiguration, OkHttp3ClientConfiguration], - properties = ["jenkins.enabled=true"]) -class JenkinsClientSpec extends Specification { - - @Autowired - JenkinsOkHttpClientProvider jenkinsOkHttpClientProvider - - @Autowired - JenkinsProperties.JenkinsHost jenkinsHost - - def "test if jenkins retrofit1 client has retrofit2 interceptor - Retrofit2EncodeCorrectionInterceptor"() { - when: - def client = jenkinsOkHttpClientProvider.provide(jenkinsHost) - def interceptors = client.interceptors() - - then: - interceptors.size() != 0 - !interceptors.any { it instanceof Retrofit2EncodeCorrectionInterceptor } - } -} - -class TestConfiguration { - @Bean - TaskExecutorBuilder taskExecutorBuilder() { - return new TaskExecutorBuilder() - } - - @Bean - JenkinsProperties.JenkinsHost jenkinsHost() { - def host = new JenkinsProperties.JenkinsHost() - host.address = "http://localhost:8080" - host.name = "jenkins-dev" - return host - } - - @Bean - JenkinsProperties jenkinsProperties(JenkinsProperties.JenkinsHost jenkinsHost) { - def jenkinsProperties = new JenkinsProperties() - jenkinsProperties.masters = [jenkinsHost] - return jenkinsProperties - } - - @Bean - BuildServices buildServices() { - def buildServices = new BuildServices() - return buildServices - } - - @Bean - IgorConfigurationProperties igorConfigurationProperties() { - new IgorConfigurationProperties() - } - - @Bean - Registry registry() { - new NoopRegistry() - } - - @Bean - CircuitBreakerRegistry circuitBreakerRegistry() { - new InMemoryCircuitBreakerRegistry() - } - - @Bean - RestAdapter.LogLevel retrofitLogLevel() { - return RestAdapter.LogLevel.BASIC - } - - @Bean - HttpLoggingInterceptor.Level retrofitInterceptorLogLevel() { - return HttpLoggingInterceptor.Level.BODY - } -} diff --git a/igor-web/src/test/java/com/netflix/spinnaker/igor/config/JenkinsClientTest.java b/igor-web/src/test/java/com/netflix/spinnaker/igor/config/JenkinsClientTest.java new file mode 100644 index 000000000..da8238141 --- /dev/null +++ b/igor-web/src/test/java/com/netflix/spinnaker/igor/config/JenkinsClientTest.java @@ -0,0 +1,118 @@ +/* + * Copyright 2025 OpsMx, 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.igor.config; + +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; + +import com.netflix.spectator.api.NoopRegistry; +import com.netflix.spectator.api.Registry; +import com.netflix.spinnaker.config.OkHttp3ClientConfiguration; +import com.netflix.spinnaker.config.OkHttpClientComponents; +import com.netflix.spinnaker.igor.IgorConfigurationProperties; +import com.netflix.spinnaker.igor.config.client.JenkinsOkHttpClientProvider; +import com.netflix.spinnaker.igor.service.BuildServices; +import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties; +import com.netflix.spinnaker.okhttp.Retrofit2EncodeCorrectionInterceptor; +import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry; +import io.github.resilience4j.circuitbreaker.internal.InMemoryCircuitBreakerRegistry; +import java.util.List; +import okhttp3.OkHttpClient; +import okhttp3.logging.HttpLoggingInterceptor; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.task.TaskExecutorBuilder; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Primary; +import retrofit.RestAdapter; + +@SpringBootTest( + classes = { + JenkinsConfig.class, + OkHttp3ClientConfiguration.class, + OkHttpClientComponents.class, + JenkinsClientTest.TestConfiguration.class, + TaskExecutorBuilder.class + }, + properties = {"jenkins.enabled=true"}) +public class JenkinsClientTest { + + @Autowired private JenkinsOkHttpClientProvider jenkinsOkHttpClientProvider; + + @Autowired private JenkinsProperties.JenkinsHost jenkinsHost; + + @Test + public void test_if_Jenkins_Retrofit1_client_Has_Retrofit2Interceptor() { + OkHttpClient client = jenkinsOkHttpClientProvider.provide(jenkinsHost); + assertThat(client.interceptors()) + .noneMatch(interceptor -> interceptor instanceof Retrofit2EncodeCorrectionInterceptor); + } + + static class TestConfiguration { + @Bean + public JenkinsProperties.JenkinsHost jenkinsHost() { + JenkinsProperties.JenkinsHost jenkinsHost = new JenkinsProperties.JenkinsHost(); + jenkinsHost.setAddress("http://localhost:8080"); + jenkinsHost.setName("jenkins-dev"); + return jenkinsHost; + } + + @Bean + public JenkinsProperties jenkinsProperties(JenkinsProperties.JenkinsHost jenkinsHost) { + JenkinsProperties jenkinsProperties = new JenkinsProperties(); + jenkinsProperties.setMasters(List.of(jenkinsHost)); + return jenkinsProperties; + } + + @Bean + public BuildServices buildServices() { + return Mockito.mock(BuildServices.class); + } + + @Bean + public HttpLoggingInterceptor.Level retrofitInterceptorLogLevel() { + return HttpLoggingInterceptor.Level.BODY; + } + + @Primary + @Bean + public OkHttpClientConfigurationProperties okHttpClientConfigurationProperties() { + return new OkHttpClientConfigurationProperties(); + } + + @Bean + public IgorConfigurationProperties igorConfigurationProperties() { + return new IgorConfigurationProperties(); + } + + @Bean + Registry registry() { + return new NoopRegistry(); + } + + @Bean + CircuitBreakerRegistry circuitBreakerRegistry() { + return new InMemoryCircuitBreakerRegistry(); + } + + @Bean + RestAdapter.LogLevel retrofitLogLevel() { + return RestAdapter.LogLevel.BASIC; + } + } +}