Revert BackwardCompatibilityInterceptor's behavior of changing /apis to /oapi URLs

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/BackwardsCompatibilityInterceptor.java

@@ -128,24 +128,24 @@ public int hashCode() {
     openshiftOAPITransformations.put("imagestreams", new ResourceKey("ImageStream", "imagestreams", "image.openshift.io", "v1"));
     openshiftOAPITransformations.put("imagestreamtags", new ResourceKey("ImageStream", "imagestreamtags", "image.openshift.io", "v1"));
     openshiftOAPITransformations.put("securitycontextconstraints", new ResourceKey("SecurityContextConstraints", "securitycontextconstraints", "security.openshift.io", "v1"));
-}
+  }
 
   public Response intercept(Chain chain) throws IOException {
     Request request = chain.request();
     Response response = chain.proceed(request);
-    if (isOpenshiftApiRequest(request)) {
-      return handleOpenshiftRequests(request, response, chain);
-    } else if (!response.isSuccessful() && responseCodeToTransformations.containsKey(response.code())) {
+    if (isDeprecatedOpenshiftOapiRequest(request)) {
+      return handleOpenshiftOapiRequests(request, response, chain);
+    } else if (!response.isSuccessful() && responseCodeToTransformations.keySet().contains(response.code())) {
       String url = request.url().toString();
       Matcher matcher = getMatcher(url);
       ResourceKey key = getKey(matcher);
       ResourceKey target = responseCodeToTransformations.get(response.code()).get(key);
       if (target != null) {
         response.close(); // At this point, we know we won't reuse or return the response; so close it to avoid a connection leak.
         String newUrl = new StringBuilder(url)
-            .replace(matcher.start(API_VERSION), matcher.end(API_VERSION), target.version) // Order matters: We need to substitute right to left, so that former substitution don't affect the indexes of later.
-            .replace(matcher.start(API_GROUP), matcher.end(API_GROUP), target.group)
-            .toString();
+          .replace(matcher.start(API_VERSION), matcher.end(API_VERSION), target.version) // Order matters: We need to substitute right to left, so that former substitution don't affect the indexes of later.
+          .replace(matcher.start(API_GROUP), matcher.end(API_GROUP), target.group)
+          .toString();
 
         return handleNewRequestAndProceed(request, newUrl, target, chain);
       }
@@ -170,28 +170,21 @@ private static ResourceKey getKey(Matcher m) {
     return m != null ? new ResourceKey(null, m.group(PATH), m.group(API_GROUP), m.group(API_VERSION)) : null;
   }
 
-  private static Response handleOpenshiftRequests(Request request, Response response, Chain chain) throws IOException{
+  private static Response handleOpenshiftOapiRequests(Request request, Response response, Chain chain) throws IOException{
     if (!response.isSuccessful() && response.code() == HttpURLConnection.HTTP_NOT_FOUND) {
-      ResourceKey target = getResourceKeyFromRequest(request);
+      String requestUrl = request.url().toString();
+      // handle case when /oapi is not available
+      String[] parts = requestUrl.split("/");
+      String resourcePath = parts[parts.length - 1];
+      ResourceKey target = openshiftOAPITransformations.get(resourcePath);
       if (target != null) {
-        String requestUrl = request.url().toString();
-        requestUrl = isOpenShift4Request(requestUrl) ?
-          convertToOpenShiftOapiUrl(requestUrl, target) :
-          convertToOpenShift4Url(requestUrl, target);
+        requestUrl = requestUrl.replace("/oapi", "/apis/" + target.getGroup());
         return handleNewRequestAndProceed(request, requestUrl, target, chain);
       }
     }
     return response;
   }
 
-  private static String convertToOpenShift4Url(String requestUrl, ResourceKey target) {
-    return requestUrl.replace("/oapi", "/apis/" + target.getGroup());
-  }
-
-  private static String convertToOpenShiftOapiUrl(String requestUrl, ResourceKey target) {
-    return requestUrl.replace("/apis/" + target.getGroup() + "/" + target.getVersion(), "/oapi/v1");
-  }
-
   private static Response handleNewRequestAndProceed(Request request, String newUrl, ResourceKey target, Chain chain) throws IOException {
     Request.Builder newRequest = request.newBuilder()
       .url(newUrl);
@@ -214,34 +207,10 @@ private static Response handleNewRequestAndProceed(Request request, String newUr
     return chain.proceed(newRequest.build());
   }
 
-  private static boolean isOpenshiftApiRequest(Request request) {
-    if (request != null) {
-      String requestUrl = request.url().toString();
-      return isOpenshift3OapiRequest(requestUrl) || isOpenShift4Request(requestUrl);
+  private static boolean isDeprecatedOpenshiftOapiRequest(Request request) {
+    if (request != null && request.url() != null) {
+      return request.url().toString().contains("oapi");
     }
     return false;
   }
-
-  private static boolean isOpenShift4Request(String requestUrl) {
-    return requestUrl.contains(".openshift.io");
-  }
-
-  private static boolean isOpenshift3OapiRequest(String requestUrl) {
-    return requestUrl.contains("oapi");
-  }
-
-  private static ResourceKey getResourceKeyFromRequest(Request request) {
-    String requestUrl = request.url().toString();
-    String resourcePath;
-    String[] parts = requestUrl.split("/");
-    if (parts.length > 2) {
-      if (request.method().equalsIgnoreCase("POST")) {
-        resourcePath = parts[parts.length - 1];
-      } else {
-        resourcePath = parts[parts.length - 2];
-      }
-      return openshiftOAPITransformations.get(resourcePath);
-    }
-    return null;
-  }
 }

kubernetes-itests/src/test/java/io/fabric8/openshift/DeploymentConfigIT.java

@@ -89,14 +89,6 @@ public void delete() {
     assertTrue(bDeleted);
   }
 
-  @Test
-  public void testWaitUntilReady() throws InterruptedException {
-    DeploymentConfig deploymentConfig = client.deploymentConfigs().inNamespace(session.getNamespace()).withName("dc-waituntilready").waitUntilReady(2, TimeUnit.MINUTES);
-    assertNotNull(deploymentConfig);
-    assertEquals(1, deploymentConfig.getStatus().getAvailableReplicas().intValue());
-    assertTrue(deploymentConfig.getStatus().getConditions().stream().anyMatch(c -> c.getType().equals("Available")));
-  }
-
   @Test
   public void createOrReplace() {
     // Given

kubernetes-itests/src/test/java/io/fabric8/openshift/TemplateIT.java

@@ -18,7 +18,10 @@
 
 import io.fabric8.commons.ClusterEntity;
 import io.fabric8.commons.ReadyEntity;
+import io.fabric8.kubernetes.api.model.Pod;
+import io.fabric8.kubernetes.api.model.PodBuilder;
 import io.fabric8.openshift.api.model.Template;
+import io.fabric8.openshift.api.model.TemplateBuilder;
 import io.fabric8.openshift.api.model.TemplateList;
 import io.fabric8.openshift.client.OpenShiftClient;
 import org.arquillian.cube.kubernetes.api.Session;
@@ -84,4 +87,45 @@ public void delete() {
     boolean bDeleted = client.templates().inNamespace(session.getNamespace()).withName("template-delete").delete();
     assertTrue(bDeleted);
   }
+
+  @Test
+  public void testCreateWithVersionV1() {
+    // Given
+    Pod pod = new PodBuilder()
+      .withNewMetadata().withName("redis-master").endMetadata()
+      .withNewSpec()
+      .addNewContainer()
+      .addNewEnv().withName("REDIS_PASSWORD").withValue("${REDIS_PASSWORD}").endEnv()
+      .withImage("dockerfile/redis")
+      .addNewPort()
+      .withContainerPort(6379)
+      .withProtocol("TCP")
+      .endPort()
+      .endContainer()
+      .endSpec()
+      .build();
+    Template template =  new TemplateBuilder()
+      .withNewMetadata()
+      .withName("templateit-createv1")
+      .addToAnnotations("description", "Description")
+      .addToAnnotations("iconClass", "icon-redis")
+      .addToAnnotations("tags", "database,nosql")
+      .endMetadata()
+      .addToObjects(pod)
+      .addNewParameter()
+      .withDescription("Password used for Redis authentication")
+      .withFrom("[A-Z0-9]{8}")
+      .withGenerate("expression")
+      .withName("REDIS_PASSWORD")
+      .endParameter()
+      .addToLabels("redis", "master")
+      .build();
+
+    // When
+    // Set v1 Api
+    template.setApiVersion("v1");
+    template = client.templates().inNamespace(session.getNamespace()).create(template);
+    assertNotNull(template);
+    assertEquals("template.openshift.io/v1", template.getApiVersion());
+  }
 }

kubernetes-itests/src/test/resources/deploymentconfig-it.yml

@@ -174,25 +174,3 @@ spec:
           name: "origin-ruby-sample:latest"
   strategy:
     type: "Rolling"
----
-kind: "DeploymentConfig"
-apiVersion: "v1"
-metadata:
-  name: dc-waituntilready
-spec:
-  template:
-    metadata:
-      labels:
-        name: "frontend"
-    spec:
-      containers:
-        - name: "helloworld"
-          image: "openshift/hello-openshift:latest"
-          ports:
-            - containerPort: 8080
-              protocol: "TCP"
-  replicas: 1
-  selector:
-    name: "frontend"
-  triggers:
-    - type: ConfigChange

kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/BuildConfigTest.java

@@ -201,23 +201,6 @@ public void testDelete() {
     assertTrue(deleted);
   }
 
-  @Test
-  void testCreateOrReplaceOpenShift3() {
-    // Given
-    BuildConfig buildConfig = getBuildConfig();
-    server.expect().post().withPath("/oapi/v1/namespaces/ns1/buildconfigs")
-      .andReturn(HttpURLConnection.HTTP_OK, buildConfig)
-      .once();
-    OpenShiftClient client = server.getOpenshiftClient();
-
-    // When
-    buildConfig = client.buildConfigs().inNamespace("ns1").createOrReplace(buildConfig);
-
-    // Then
-    assertNotNull(buildConfig);
-    assertEquals("ruby-sample-build", buildConfig.getMetadata().getName());
-  }
-
   private BuildConfig getBuildConfig() {
     return new BuildConfigBuilder()
       .withNewMetadata().withName("ruby-sample-build").endMetadata()

kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/DeploymentConfigTest.java

@@ -29,6 +29,7 @@
 import io.fabric8.kubernetes.api.model.DeletionPropagation;
 import io.fabric8.kubernetes.api.model.PodBuilder;
 import io.fabric8.kubernetes.api.model.PodListBuilder;
+import io.fabric8.kubernetes.api.model.WatchEvent;
 import io.fabric8.kubernetes.client.dsl.LogWatch;
 import io.fabric8.kubernetes.client.utils.Utils;
 import io.fabric8.openshift.api.model.DeploymentConfigSpecFluent;
@@ -49,7 +50,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 
 @EnableRuleMigrationSupport
-public class DeploymentConfigTest {
+class DeploymentConfigTest {
   @Rule
   public OpenShiftServer server = new OpenShiftServer();
 
@@ -252,23 +253,6 @@ public void visit(ContainerFluent<?> container) {
    assertTrue(visitedContainer.get());
   }
 
-  @Test
-  void testCreateOrReplaceOnOpenShift3() {
-    // Given
-    DeploymentConfig deploymentConfig = getDeploymentConfig().build();
-    server.expect().post().withPath("/oapi/v1/namespaces/ns1/deploymentconfigs")
-      .andReturn(HttpURLConnection.HTTP_OK, deploymentConfig)
-      .once();
-    OpenShiftClient client = server.getOpenshiftClient();
-
-    // When
-    deploymentConfig = client.deploymentConfigs().inNamespace("ns1").createOrReplace(deploymentConfig);
-
-    // Then
-    assertNotNull(deploymentConfig);
-    assertEquals("dc1", deploymentConfig.getMetadata().getName());
-  }
-
   @Test
   void testGetLog() {
     // Given
@@ -318,6 +302,29 @@ void testWatchLog() {
     logWatch.close();
   }
 
+  @Test
+  void testWaitUntilReady() throws InterruptedException {
+    // Given
+    DeploymentConfig deploymentConfig = getDeploymentConfig().withNewStatus()
+      .addNewCondition()
+      .withType("Available")
+      .endCondition()
+      .withReplicas(1).withAvailableReplicas(1)
+      .endStatus().build();
+    server.expect().get().withPath("/apis/apps.openshift.io/v1/namespaces/ns1/deploymentconfigs/dc1")
+      .andReturn(HttpURLConnection.HTTP_OK, deploymentConfig)
+      .always();
+    OpenShiftClient client = server.getOpenshiftClient();
+
+    // When
+    deploymentConfig = client.deploymentConfigs().inNamespace("ns1").withName("dc1").waitUntilReady(10, TimeUnit.SECONDS);
+
+    // Then
+    assertNotNull(deploymentConfig);
+    assertEquals(1, deploymentConfig.getStatus().getAvailableReplicas().intValue());
+    assertTrue(deploymentConfig.getStatus().getConditions().stream().anyMatch(c -> c.getType().equals("Available")));
+  }
+
   private DeploymentConfigBuilder getDeploymentConfig() {
     return new DeploymentConfigBuilder()
       .withNewMetadata()

kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/TemplateTest.java

@@ -262,23 +262,6 @@ void testEmptyParameterMapValueShouldNotThrowNullPointerException() {
     assertNotNull(list);
   }
 
-  @Test
-  void testCreateOrReplaceOpenShift3() {
-    // Given
-    Template template = getTemplateBuilder().build();
-    server.expect().post().withPath("/oapi/v1/namespaces/ns1/templates")
-      .andReturn(HttpURLConnection.HTTP_OK, template)
-      .once();
-
-    OpenShiftClient client = server.getOpenshiftClient();
-
-    // When
-    template = client.templates().inNamespace("ns1").createOrReplace(template);
-
-    // Then
-    assertNotNull(template);
-  }
-
   @Test
   void testCreateOrReplaceOpenShif4() {
     // Given

openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/OpenShiftOperation.java

@@ -41,6 +41,7 @@ public class OpenShiftOperation<T extends HasMetadata, L extends KubernetesResou
 
   public OpenShiftOperation(OperationContext ctx) {
     super(wrap(ctx));
+    updateApiVersion();
   }
 
   static OperationContext wrap(OperationContext context) {
@@ -106,7 +107,15 @@ public T waitUntilReady(long amount, TimeUnit timeUnit) throws InterruptedExcept
     return waitUntilCondition(resource -> Objects.nonNull(resource) && OpenShiftReadiness.isReady(resource), amount, timeUnit);
   }
 
-  protected Class<? extends Config> getConfigType()  {
+  private void updateApiVersion() {
+    if (apiGroupName != null && apiGroupVersion != null) {
+      this.apiVersion = apiGroupName + "/" + apiGroupVersion;
+    } else if (apiGroupVersion != null) {
+      this.apiVersion = apiGroupVersion;
+    }
+  }
+
+  protected Class<? extends Config> getConfigType() {
     return OpenShiftConfig.class;
   }
 }