feat(web): add ability to skip X-SPINNAKER-ACCOUNTS in SpinnakerRequestInterceptor and SpinnakerRequestHeaderInterceptor

kork-retrofit/src/test/kotlin/com/netflix/spinnaker/kork/retrofit/RetrofitServiceProviderTest.kt

@@ -119,5 +119,3 @@ interface Retrofit1Service {
   fun getSomething(@Path("user") user: String?, callback: Callback<List<*>?>?)
 
 }
-
-

kork-retrofit2/src/test/kotlin/com/netflix/spinnaker/kork/retrofit/Retrofit2ServiceProviderTest.kt

@@ -102,7 +102,7 @@ private open class TestConfiguration {
 
   @Bean
   open fun spinnakerRequestInterceptor(): SpinnakerRequestInterceptor {
-    return SpinnakerRequestInterceptor(OkHttpClientConfigurationProperties())
+    return SpinnakerRequestInterceptor(true)
   }
 
   @Bean

kork-web/kork-web.gradle

@@ -43,8 +43,11 @@ dependencies {
   runtimeOnly "org.hibernate.validator:hibernate-validator"
 
   testImplementation project(":kork-test")
+  testImplementation "com.github.tomakehurst:wiremock-jre8-standalone"
   testImplementation "ch.qos.logback:logback-classic"
   testImplementation "ch.qos.logback:logback-core"
+  testImplementation "com.squareup.retrofit2:retrofit"
+  testImplementation "org.junit.jupiter:junit-jupiter-params"
   testImplementation "org.spockframework:spock-core"
   testImplementation "org.spockframework:spock-spring"
   testImplementation "org.springframework.boot:spring-boot-starter-test"

kork-web/src/main/groovy/com/netflix/spinnaker/okhttp/SpinnakerRequestInterceptor.groovy

@@ -16,24 +16,37 @@
 
 package com.netflix.spinnaker.okhttp
 
+import com.netflix.spinnaker.kork.common.Header
 import com.netflix.spinnaker.security.AuthenticatedRequest
 import retrofit.RequestInterceptor
 
 class SpinnakerRequestInterceptor implements RequestInterceptor {
-  private final OkHttpClientConfigurationProperties okHttpClientConfigurationProperties
+  private final boolean propagateSpinnakerHeaders;
 
-  SpinnakerRequestInterceptor(OkHttpClientConfigurationProperties okHttpClientConfigurationProperties) {
-    this.okHttpClientConfigurationProperties = okHttpClientConfigurationProperties
+  /**
+   * Don't propagate X-SPINNAKER-ACCOUNTS.  Only relevant when propagateSpinnakerHeaders is true.
+   */
+  private final boolean skipAccountsHeader;
+
+  SpinnakerRequestInterceptor(boolean propagateSpinnakerHeaders) {
+    this.propagateSpinnakerHeaders = propagateSpinnakerHeaders
+    this.skipAccountsHeader = false
+  }
+
+  SpinnakerRequestInterceptor(boolean propagateSpinnakerHeaders,
+                              boolean skipAccountsHeader) {
+    this.propagateSpinnakerHeaders = propagateSpinnakerHeaders
+    this.skipAccountsHeader = skipAccountsHeader
   }
 
   void intercept(RequestInterceptor.RequestFacade request) {
-    if (!okHttpClientConfigurationProperties.propagateSpinnakerHeaders) {
+    if (!propagateSpinnakerHeaders) {
       // noop
       return
     }
 
     AuthenticatedRequest.authenticationHeaders.each { String key, Optional<String> value ->
-      if (value.present) {
+      if (value.present && (!skipAccountsHeader || !Header.ACCOUNTS.getHeader().equals(key))) {
         request.addHeader(key, value.get())
       }
     }

kork-web/src/main/groovy/com/netflix/spinnaker/config/OkHttpClientComponents.java → kork-web/src/main/java/com/netflix/spinnaker/config/OkHttpClientComponents.java

@@ -82,12 +82,12 @@ public class OkHttpClientComponents {
 
   @Bean
   public SpinnakerRequestInterceptor spinnakerRequestInterceptor() {
-    return new SpinnakerRequestInterceptor(clientProperties);
+    return new SpinnakerRequestInterceptor(clientProperties.getPropagateSpinnakerHeaders());
   }
 
   @Bean
   public SpinnakerRequestHeaderInterceptor spinnakerRequestHeaderInterceptor() {
-    return new SpinnakerRequestHeaderInterceptor(clientProperties);
+    return new SpinnakerRequestHeaderInterceptor(clientProperties.getPropagateSpinnakerHeaders());
   }
 
   @Bean

kork-web/src/main/java/com/netflix/spinnaker/okhttp/SpinnakerRequestHeaderInterceptor.java

@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.okhttp;
 
+import com.netflix.spinnaker.kork.common.Header;
 import com.netflix.spinnaker.security.AuthenticatedRequest;
 import java.io.IOException;
 import okhttp3.Interceptor;
@@ -28,24 +29,34 @@
  */
 public class SpinnakerRequestHeaderInterceptor implements Interceptor {
 
-  private final OkHttpClientConfigurationProperties okHttpClientConfigurationProperties;
+  private final boolean propagateSpinnakerHeaders;
+
+  /** Don't propagate X-SPINNAKER-ACCOUNTS. Only relevant when propagateSpinnakerHeaders is true. */
+  private final boolean skipAccountsHeader;
+
+  public SpinnakerRequestHeaderInterceptor(boolean propagateSpinnakerHeaders) {
+    this.propagateSpinnakerHeaders = propagateSpinnakerHeaders;
+    this.skipAccountsHeader = false;
+  }
 
   public SpinnakerRequestHeaderInterceptor(
-      OkHttpClientConfigurationProperties okHttpClientConfigurationProperties) {
-    this.okHttpClientConfigurationProperties = okHttpClientConfigurationProperties;
+      boolean propagateSpinnakerHeaders, boolean skipAccountsHeader) {
+    this.propagateSpinnakerHeaders = propagateSpinnakerHeaders;
+    this.skipAccountsHeader = skipAccountsHeader;
   }
 
   @Override
   public Response intercept(Chain chain) throws IOException {
     Request.Builder builder = chain.request().newBuilder();
-    if (!okHttpClientConfigurationProperties.getPropagateSpinnakerHeaders()) {
+    if (!propagateSpinnakerHeaders) {
       return chain.proceed(builder.build());
     }
 
     AuthenticatedRequest.getAuthenticationHeaders()
         .forEach(
             (key, value) -> {
-              if (value.isPresent()) {
+              if (value.isPresent()
+                  && (!skipAccountsHeader || !Header.ACCOUNTS.getHeader().equals(key))) {
                 builder.addHeader(key, value.get());
               }
             });

kork-web/src/test/java/com/netflix/spinnaker/config/OkHttpClientComponentsTest.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright 2024 Salesforce, 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.config;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.netflix.spinnaker.okhttp.OkHttp3MetricsInterceptor;
+import com.netflix.spinnaker.okhttp.OkHttpMetricsInterceptor;
+import com.netflix.spinnaker.okhttp.SpinnakerRequestHeaderInterceptor;
+import com.netflix.spinnaker.okhttp.SpinnakerRequestInterceptor;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
+import org.springframework.boot.context.annotation.UserConfigurations;
+import org.springframework.boot.task.TaskExecutorBuilder;
+import org.springframework.boot.test.context.runner.ApplicationContextRunner;
+
+class OkHttpClientComponentsTest {
+
+  private final ApplicationContextRunner runner =
+      new ApplicationContextRunner()
+          .withBean(TaskExecutorBuilder.class)
+          .withConfiguration(UserConfigurations.of(OkHttpClientComponents.class));
+
+  @BeforeEach
+  void init(TestInfo testInfo) {
+    System.out.println("--------------- Test " + testInfo.getDisplayName());
+  }
+
+  @Test
+  void verifyValidConfiguration() {
+    runner.run(
+        ctx -> {
+          assertThat(ctx).hasSingleBean(SpinnakerRequestInterceptor.class);
+          assertThat(ctx).hasSingleBean(SpinnakerRequestHeaderInterceptor.class);
+          assertThat(ctx).hasSingleBean(OkHttpMetricsInterceptor.class);
+          assertThat(ctx).hasSingleBean(OkHttp3MetricsInterceptor.class);
+        });
+  }
+}