Fix #3152: Retry only Create-only resources in OpenShiftOAuthInterceptor

fabric8io · May 20, 2021 · 19e57dc · 19e57dc
1 parent ed08377
commit 19e57dc
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 9 deletions.
diff --git a/...-tests/src/test/java/io/fabric8/openshift/client/server/mock/SubjectAccessReviewTest.java b/...-tests/src/test/java/io/fabric8/openshift/client/server/mock/SubjectAccessReviewTest.java
@@ -50,7 +50,7 @@ class SubjectAccessReviewTest {
   void testCreate() {
     server.expect().withPath("/apis/authorization.openshift.io/v1/subjectaccessreviews").andReturn(201, new SubjectAccessReviewResponseBuilder()
       .withReason("r1")
-      .build()).once();
+      .build()).times(2);
     SubjectAccessReviewResponse response = client.inAnyNamespace().subjectAccessReviews().create(new SubjectAccessReviewBuilder()
       .build());
     assertNotNull(response);
@@ -62,7 +62,7 @@ void testCreate() {
   void testCreateInLine() {
     server.expect().withPath("/apis/authorization.openshift.io/v1/subjectaccessreviews").andReturn(201, new SubjectAccessReviewResponseBuilder()
       .withReason("r2")
-      .build()).once();
+      .build()).times(2);
 
     SubjectAccessReviewResponse response = client.inAnyNamespace().subjectAccessReviews()
       .create(new SubjectAccessReviewBuilder().build());
@@ -75,7 +75,7 @@ void testCreateInLine() {
   void testCreateLocal() {
    server.expect().withPath("/apis/authorization.openshift.io/v1/namespaces/test/localsubjectaccessreviews").andReturn(201, new SubjectAccessReviewResponseBuilder()
       .withReason("r1")
-      .build()).once();
+      .build()).times(2);
 
 
     SubjectAccessReviewResponse response = client.localSubjectAccessReviews().inNamespace("test").create(new LocalSubjectAccessReviewBuilder()
@@ -92,7 +92,7 @@ void testCreateLocal() {
   void testCreateLocalInLine() {
    server.expect().withPath("/apis/authorization.openshift.io/v1/namespaces/test/localsubjectaccessreviews").andReturn( 201, new SubjectAccessReviewResponseBuilder()
       .withReason("r2")
-      .build()).once();
+      .build()).times(2);
 
 
     SubjectAccessReviewResponse response = client.localSubjectAccessReviews().inNamespace("test").create(new LocalSubjectAccessReviewBuilder()
@@ -110,7 +110,7 @@ void createResourceAccessReview() {
       .andReturn( HTTP_CREATED, new ResourceAccessReviewResponseBuilder()
         .addToGroups("system:cluster-admins", "system:masters")
         .addToUsers("kubeadmin", "system:admin")
-      .build()).once();
+      .build()).times(2);
 
     // When
     ResourceAccessReviewResponse response = client.resourceAccessReviews().create(new ResourceAccessReviewBuilder()
@@ -132,7 +132,7 @@ void createLocalResourceAccessReview() {
         .withNamespace("ns1")
         .addToGroups("system:cluster-admins", "system:masters")
         .addToUsers("kubeadmin", "system:admin")
-        .build()).once();
+        .build()).times(2);
 
     // When
     ResourceAccessReviewResponse response = client.localResourceAccessReviews().inNamespace("ns1").create(new LocalResourceAccessReviewBuilder()
@@ -160,7 +160,7 @@ void createSelfSubjectRulesReviews() {
         .withResources("buildconfigs/webhooks")
         .endRule()
         .endStatus()
-        .build()).once();
+        .build()).times(2);
 
     // When
     SelfSubjectRulesReview response = client.selfSubjectRulesReviews().inNamespace("test").create(new SelfSubjectRulesReviewBuilder()
@@ -187,7 +187,7 @@ void createSubjectRulesReviews() {
         .withResources("imagestreams")
         .endRule()
         .endStatus()
-        .build()).once();
+        .build()).times(2);
 
     // When
     SubjectRulesReview response = client.subjectRulesReviews().inNamespace("test").create(new SubjectRulesReviewBuilder()

diff --git a/...-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java b/...-client/src/main/java/io/fabric8/openshift/client/internal/OpenShiftOAuthInterceptor.java
@@ -17,10 +17,18 @@
 package io.fabric8.openshift.client.internal;
 
 import com.fasterxml.jackson.databind.JsonNode;
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectAccessReview;
 import io.fabric8.kubernetes.client.KubernetesClientException;
 import io.fabric8.kubernetes.client.utils.Serialization;
 import io.fabric8.kubernetes.client.utils.URLUtils;
 import io.fabric8.kubernetes.client.utils.Utils;
+import io.fabric8.openshift.api.model.LocalResourceAccessReview;
+import io.fabric8.openshift.api.model.LocalSubjectAccessReview;
+import io.fabric8.openshift.api.model.ResourceAccessReview;
+import io.fabric8.openshift.api.model.SelfSubjectRulesReview;
+import io.fabric8.openshift.api.model.SubjectAccessReview;
+import io.fabric8.openshift.api.model.SubjectRulesReview;
 import io.fabric8.openshift.client.OpenShiftConfig;
 import okhttp3.Credentials;
 import okhttp3.Interceptor;
@@ -30,7 +38,10 @@
 
 import java.io.IOException;
 import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Predicate;
 
 import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
 import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED;
@@ -46,6 +57,11 @@ public class OpenShiftOAuthInterceptor implements Interceptor {
   private static final String AFTER_TOKEN = "&expires";
   private static final String K8S_AUTHORIZATION = "authorization.k8s.io";
   private static final String OPENSHIFT_AUTHORIZATION = "authorization.openshift.io";
+  private static final Predicate<String> isAuthorizationApiGroupRequest = s -> s.contains(K8S_AUTHORIZATION) || s.contains(OPENSHIFT_AUTHORIZATION);
+  private static final List<String> createOnlyResources = new ArrayList<>();
+  static {{
+    initCreateOnlyResources();
+  }}
 
   private final OkHttpClient client;
   private final OpenShiftConfig config;
@@ -151,9 +167,23 @@ private boolean isResponseSuccessful(Request request, Response response) {
     String url = request.url().toString();
     // always retry in case of authorization endpoints; since they also return 200 when no
     // authorization header is provided
-    if (url.contains(K8S_AUTHORIZATION) || url.contains(OPENSHIFT_AUTHORIZATION)) {
+    if (isAuthorizationApiGroupRequest.test(url) && isCreateOnlyResourcePluralInUrl(url)) {
       return false;
     }
     return response.code() != HTTP_UNAUTHORIZED && response.code() != HTTP_FORBIDDEN;
   }
+
+  private boolean isCreateOnlyResourcePluralInUrl(String url) {
+    return createOnlyResources.stream().anyMatch(url::contains);
+  }
+
+  private static void initCreateOnlyResources() {
+    createOnlyResources.add(HasMetadata.getPlural(LocalSubjectAccessReview.class));
+    createOnlyResources.add(HasMetadata.getPlural(LocalResourceAccessReview.class));
+    createOnlyResources.add(HasMetadata.getPlural(ResourceAccessReview.class));
+    createOnlyResources.add(HasMetadata.getPlural(SelfSubjectRulesReview.class));
+    createOnlyResources.add(HasMetadata.getPlural(SubjectRulesReview.class));
+    createOnlyResources.add(HasMetadata.getPlural(SubjectAccessReview.class));
+    createOnlyResources.add(HasMetadata.getPlural(SelfSubjectAccessReview.class));
+  }
 }