diff --git a/echo-core/src/test/groovy/com/netflix/spinnaker/echo/services/Front50ServiceSpec.groovy b/echo-core/src/test/groovy/com/netflix/spinnaker/echo/services/Front50ServiceSpec.groovy index 894bac016..cf2abd612 100644 --- a/echo-core/src/test/groovy/com/netflix/spinnaker/echo/services/Front50ServiceSpec.groovy +++ b/echo-core/src/test/groovy/com/netflix/spinnaker/echo/services/Front50ServiceSpec.groovy @@ -4,16 +4,19 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.github.tomakehurst.wiremock.WireMockServer import com.github.tomakehurst.wiremock.core.WireMockConfiguration import com.github.tomakehurst.wiremock.client.WireMock +import com.google.common.collect.ImmutableList import com.netflix.spinnaker.config.DefaultServiceEndpoint import com.netflix.spinnaker.config.okhttp3.OkHttpClientProvider import com.netflix.spinnaker.config.okhttp3.InsecureOkHttpClientBuilderProvider +import com.netflix.spinnaker.echo.model.Trigger import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall import okhttp3.OkHttpClient import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.test.context.SpringBootTest import retrofit2.Retrofit -import retrofit2.converter.jackson.JacksonConverterFactory; +import retrofit2.converter.jackson.JacksonConverterFactory +import spock.lang.Ignore; import spock.lang.Specification import spock.util.concurrent.BlockingVariable @@ -109,6 +112,22 @@ class Front50ServiceSpec extends Specification { pipelines.first().parallel } + @Ignore + def "list properties are immutable"() { + given: + def pipelines = front50Service.getPipelines() + def pipeline = pipelines.find { it.application == "kato" } + + expect: + pipeline.triggers instanceof ImmutableList + + when: + pipeline.triggers << Trigger.builder().enabled(false).type('jenkins').master('foo').job('bar').propertyFile('baz').build() + + then: + thrown UnsupportedOperationException + } + private Front50Service front50Service(String baseUrl){ new Retrofit.Builder() .baseUrl(baseUrl) diff --git a/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/notification/InteractiveNotificationCallbackHandler.groovy b/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/notification/InteractiveNotificationCallbackHandler.groovy index 8a7a1f014..449f1e64c 100644 --- a/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/notification/InteractiveNotificationCallbackHandler.groovy +++ b/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/notification/InteractiveNotificationCallbackHandler.groovy @@ -97,6 +97,7 @@ class InteractiveNotificationCallbackHandler { // TODO(lfp): error handling (retries?). I'd like to respond to the message in a thread, but // have been unable to make that work. Troubleshooting with Slack support. + // TODO(lfp): need to retrieve user's accounts to pass in X-SPINNAKER-ACCOUNTS final ResponseBody response = Retrofit2SyncCall.execute(spinnakerService.notificationCallback(callback, callback.getUser())); log.debug("Received callback response from downstream Spinnaker service: " + response.string()); diff --git a/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/pagerduty/PagerDutyNotificationService.groovy b/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/pagerduty/PagerDutyNotificationService.groovy index d320fcc97..f692aa929 100644 --- a/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/pagerduty/PagerDutyNotificationService.groovy +++ b/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/pagerduty/PagerDutyNotificationService.groovy @@ -19,10 +19,12 @@ package com.netflix.spinnaker.echo.pagerduty import com.fasterxml.jackson.databind.ObjectMapper import com.netflix.spinnaker.echo.api.Notification +import com.netflix.spinnaker.echo.config.PagerDutyConfigurationProperties import com.netflix.spinnaker.echo.controller.EchoResponse import com.netflix.spinnaker.echo.notification.NotificationService import com.netflix.spinnaker.echo.services.Front50Service import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerHttpException import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException import groovy.transform.InheritConstructors import groovy.util.logging.Slf4j @@ -46,8 +48,8 @@ class PagerDutyNotificationService implements NotificationService { @Autowired PagerDutyService pagerDuty - @Value('${pager-duty.token:}') - String token + @Autowired + PagerDutyConfigurationProperties pagerDutyConfigurationProperties @Autowired Front50Service front50Service @@ -65,7 +67,7 @@ class PagerDutyNotificationService implements NotificationService { notification.to.each { serviceKey -> try { Map response = Retrofit2SyncCall.execute(pagerDuty.createEvent( - "Token token=${token}", + "Token token=${pagerDutyConfigurationProperties.token}", new PagerDutyService.PagerDutyCreateEvent( service_key: serviceKey, client: "Spinnaker (${notification.source.user})", @@ -82,10 +84,20 @@ class PagerDutyNotificationService implements NotificationService { pdErrors.put(serviceKey, response.message) } } catch (SpinnakerServerException e){ + def errorMessage = null + if (e instanceof SpinnakerHttpException){ + Map errorResponse = ((SpinnakerHttpException) e).responseBody + if (errorResponse != null) { + if (errorResponse.errors && errorResponse.errors.size() > 0) { + errorMessage = errorResponse.errors.join(", ") + } + } + } + errorMessage = errorMessage == null ? e.message : errorMessage log.error('Failed to send page {} {} {}', kv('serviceKey', serviceKey), kv('message', notification.additionalContext.message), - kv('error', e.message) + kv('error', errorMessage) ) errors.put(serviceKey, e.message) } diff --git a/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/slack/SlackNotificationService.groovy b/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/slack/SlackNotificationService.groovy index 3c830ad64..d90da6421 100644 --- a/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/slack/SlackNotificationService.groovy +++ b/echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/slack/SlackNotificationService.groovy @@ -22,7 +22,6 @@ import com.netflix.spinnaker.echo.controller.EchoResponse import com.netflix.spinnaker.echo.notification.NotificationService import com.netflix.spinnaker.echo.notification.NotificationTemplateEngine import groovy.util.logging.Slf4j -import okhttp3.ResponseBody import org.springframework.beans.factory.annotation.Qualifier import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty import org.springframework.stereotype.Component @@ -63,7 +62,12 @@ class SlackNotificationService implements NotificationService { new SlackAttachment(subject, text, (InteractiveActions)notification.interactiveActions), address, true) } - log.trace("Received response from Slack: {}", response?.string()) + try { + log.trace("Received response from Slack: {}", response?.string()) + } catch (IOException e) { + log.trace("Received response from Slack but unable to deserialize : {}", e.message) + } + } new EchoResponse.Void() diff --git a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/config/PagerDutyConfig.java b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/config/PagerDutyConfig.java index b115a92dc..fcaf1e067 100644 --- a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/config/PagerDutyConfig.java +++ b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/config/PagerDutyConfig.java @@ -21,31 +21,27 @@ import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import retrofit2.Retrofit; import retrofit2.converter.jackson.JacksonConverterFactory; @Configuration +@EnableConfigurationProperties(PagerDutyConfigurationProperties.class) @ConditionalOnProperty("pager-duty.enabled") public class PagerDutyConfig { private static final Logger log = LoggerFactory.getLogger(PagerDutyConfig.class); - private final String endpoint; - - public PagerDutyConfig( - @Value("${pager-duty.endpoint:https://events.pagerduty.com}") String endpoint) { - this.endpoint = endpoint; - } - @Bean - PagerDutyService pagerDutyService(OkHttp3ClientConfiguration okHttpClientConfig) { + PagerDutyService pagerDutyService( + OkHttp3ClientConfiguration okHttpClientConfig, + PagerDutyConfigurationProperties pagerDutyProps) { log.info("Pager Duty service loaded"); return new Retrofit.Builder() - .baseUrl(endpoint) + .baseUrl(pagerDutyProps.getEndpoint()) .client(okHttpClientConfig.createForRetrofit2().build()) .addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance()) .addConverterFactory(JacksonConverterFactory.create()) diff --git a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/config/PagerDutyConfigurationProperties.java b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/config/PagerDutyConfigurationProperties.java new file mode 100644 index 000000000..d585afc6c --- /dev/null +++ b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/config/PagerDutyConfigurationProperties.java @@ -0,0 +1,29 @@ +/* + * 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.echo.config; + +import lombok.Getter; +import lombok.Setter; +import org.springframework.boot.context.properties.ConfigurationProperties; + +@Setter +@Getter +@ConfigurationProperties(prefix = "pager-duty") +public class PagerDutyConfigurationProperties { + private String endpoint = "https://events.pagerduty.com"; + private String token; +} diff --git a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/jira/JiraNotificationService.java b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/jira/JiraNotificationService.java index d4b8020b3..317594372 100644 --- a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/jira/JiraNotificationService.java +++ b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/jira/JiraNotificationService.java @@ -189,11 +189,7 @@ private Map issueRequestBody(Notification notification) { private Map errors(Exception exception) { if (exception instanceof SpinnakerHttpException) { - try { - return ((SpinnakerHttpException) exception).getResponseBody(); - } catch (Exception e) { - LOGGER.warn("failed retrieving error messages {}", e.getMessage()); - } + return ((SpinnakerHttpException) exception).getResponseBody(); } return ImmutableMap.of("errors", exception.getMessage()); diff --git a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/microsoftteams/MicrosoftTeamsService.java b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/microsoftteams/MicrosoftTeamsService.java index 807d7b57a..cdbcc750f 100644 --- a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/microsoftteams/MicrosoftTeamsService.java +++ b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/microsoftteams/MicrosoftTeamsService.java @@ -17,6 +17,7 @@ package com.netflix.spinnaker.echo.microsoftteams; import com.netflix.spinnaker.config.OkHttp3ClientConfiguration; +import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory; import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall; import com.netflix.spinnaker.kork.web.exceptions.InvalidRequestException; import java.net.MalformedURLException; @@ -42,6 +43,7 @@ public ResponseBody sendMessage(String webhookUrl, MicrosoftTeamsMessage message new Retrofit.Builder() .baseUrl(webhookUrl) .client(okHttp3ClientConfiguration.createForRetrofit2().build()) + .addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance()) .addConverterFactory(JacksonConverterFactory.create()) .build() .create(MicrosoftTeamsClient.class); diff --git a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/CDEventsNotificationAgent.java b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/CDEventsNotificationAgent.java index 6bf26ca6a..5d08e8949 100644 --- a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/CDEventsNotificationAgent.java +++ b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/CDEventsNotificationAgent.java @@ -82,10 +82,12 @@ public void sendNotifications( response.body() != null ? response.body().string() : ""); } catch (IOException e) { log.info( - "Received response from events broker : {} {} for execution id {}", + "Received response from events broker : {} {} for execution id {} " + + "but unable to serialize the response body: {}", response.code(), response.message(), - executionId); + executionId, + e.getMessage()); } } } diff --git a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/GithubNotificationAgent.java b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/GithubNotificationAgent.java index 30ef9dad4..ddf2098e9 100644 --- a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/GithubNotificationAgent.java +++ b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/GithubNotificationAgent.java @@ -24,6 +24,7 @@ import com.netflix.spinnaker.echo.github.GithubStatus; import com.netflix.spinnaker.kork.core.RetrySupport; import com.netflix.spinnaker.kork.retrofit.Retrofit2SyncCall; +import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerServerException; import java.util.Map; import java.util.Optional; import java.util.regex.Matcher; @@ -127,9 +128,12 @@ public void sendNotifications( } private String getBranchCommit(String repo, String sha) { - GithubCommit message = - Retrofit2SyncCall.execute(githubService.getCommit("token " + token, repo, sha)); - + GithubCommit message; + try { + message = Retrofit2SyncCall.execute(githubService.getCommit("token " + token, repo, sha)); + } catch (SpinnakerServerException e) { + return sha; + } Pattern pattern = Pattern.compile( "Merge (?[0-9a-f]{5,40}) into (?[0-9a-f]{5,40})"); @@ -137,6 +141,7 @@ private String getBranchCommit(String repo, String sha) { if (matcher.matches()) { return matcher.group("branchCommit"); } + return sha; } diff --git a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/MicrosoftTeamsNotificationAgent.java b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/MicrosoftTeamsNotificationAgent.java index ccf17e3bb..e7db1753f 100644 --- a/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/MicrosoftTeamsNotificationAgent.java +++ b/echo-notifications/src/main/java/com/netflix/spinnaker/echo/notification/MicrosoftTeamsNotificationAgent.java @@ -150,7 +150,7 @@ public void sendNotifications( response.string()); } catch (IOException e) { log.info( - "Received response from Microsoft Teams Webhook for execution id {} but failed to deserialize", + "Received response from Microsoft Teams Webhook for execution id {} but failed to deserialize", executionId); } } diff --git a/echo-rest/echo-rest.gradle b/echo-rest/echo-rest.gradle index a55f515fe..9b780f711 100644 --- a/echo-rest/echo-rest.gradle +++ b/echo-rest/echo-rest.gradle @@ -19,7 +19,6 @@ dependencies { implementation project(':echo-model') implementation project(':echo-api') implementation project(':echo-core') - implementation project(':echo-test') implementation "commons-codec:commons-codec" @@ -29,6 +28,7 @@ dependencies { implementation "javax.validation:validation-api" implementation "org.apache.commons:commons-lang3" + testImplementation project(':echo-test') testImplementation "com.github.tomakehurst:wiremock-jre8" testImplementation "org.springframework.boot:spring-boot-starter-test" } diff --git a/echo-telemetry/src/main/java/com/netflix/spinnaker/echo/config/TelemetryConfig.kt b/echo-telemetry/src/main/java/com/netflix/spinnaker/echo/config/TelemetryConfig.kt index b5f96539d..f2a19647d 100644 --- a/echo-telemetry/src/main/java/com/netflix/spinnaker/echo/config/TelemetryConfig.kt +++ b/echo-telemetry/src/main/java/com/netflix/spinnaker/echo/config/TelemetryConfig.kt @@ -15,11 +15,15 @@ */ package com.netflix.spinnaker.echo.config +import com.netflix.spinnaker.config.DefaultServiceEndpoint import com.netflix.spinnaker.config.OkHttp3ClientConfiguration +import com.netflix.spinnaker.config.okhttp3.DefaultOkHttpClientBuilderProvider import com.netflix.spinnaker.echo.config.TelemetryConfig.TelemetryConfigProps import com.netflix.spinnaker.echo.telemetry.TelemetryService import com.netflix.spinnaker.kork.retrofit.ErrorHandlingExecutorCallAdapterFactory +import com.netflix.spinnaker.okhttp.OkHttpClientConfigurationProperties import de.huxhorn.sulky.ulid.ULID +import okhttp3.OkHttpClient import org.slf4j.LoggerFactory import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty import org.springframework.boot.context.properties.ConfigurationProperties @@ -41,12 +45,15 @@ open class TelemetryConfig { @Bean open fun telemetryService( configProps: TelemetryConfigProps, - okHttpClientConfig: OkHttp3ClientConfiguration + okHttpClientConfig: OkHttp3ClientConfiguration, + okHttpClient: OkHttpClient ): TelemetryService { + val clientProps = OkHttpClientConfigurationProperties(configProps.connectionTimeoutMillis.toLong(), configProps.readTimeoutMillis.toLong()) + val clientProvider = DefaultOkHttpClientBuilderProvider(okHttpClient, clientProps) log.info("Telemetry service loaded") return Retrofit.Builder() .baseUrl(configProps.endpoint) - .client(okHttpClientConfig.createForRetrofit2().build()) + .client(clientProvider.get(DefaultServiceEndpoint("telemetry", configProps.endpoint)).build()) .addCallAdapterFactory(ErrorHandlingExecutorCallAdapterFactory.getInstance()) .addConverterFactory(JacksonConverterFactory.create()) .build() @@ -65,6 +72,8 @@ open class TelemetryConfig { var instanceId = ULID().nextULID() var spinnakerVersion = "unknown" var deploymentMethod = DeploymentMethod() + var connectionTimeoutMillis = 3000 + var readTimeoutMillis = 5000 class DeploymentMethod { var type: String? = null diff --git a/echo-telemetry/src/test/kotlin/com/netflix/spinnaker/echo/telemetry/TelemetryEventListenerTest.kt b/echo-telemetry/src/test/kotlin/com/netflix/spinnaker/echo/telemetry/TelemetryEventListenerTest.kt index 56ff48cf5..d5c52163e 100644 --- a/echo-telemetry/src/test/kotlin/com/netflix/spinnaker/echo/telemetry/TelemetryEventListenerTest.kt +++ b/echo-telemetry/src/test/kotlin/com/netflix/spinnaker/echo/telemetry/TelemetryEventListenerTest.kt @@ -22,9 +22,13 @@ import com.netflix.spinnaker.echo.api.events.Event import com.netflix.spinnaker.echo.api.events.Metadata import com.netflix.spinnaker.kork.retrofit.exceptions.SpinnakerNetworkException import io.github.resilience4j.circuitbreaker.CircuitBreakerRegistry -import io.mockk.* +import io.mockk.Called +import io.mockk.CapturingSlot +import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.junit5.MockKExtension +import io.mockk.slot +import io.mockk.verify import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.Request import okhttp3.RequestBody @@ -144,7 +148,7 @@ class TelemetryEventListenerTest { val event = createLoggableEvent() val request: Request = Request.Builder().url("http://url").build() every { telemetryService.log(any()) } throws - SpinnakerNetworkException(IOException("timeout"), request) + SpinnakerNetworkException(IOException("network error"), request) expectCatching { telemetryEventListener.processEvent(event) }.isSuccess()