Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #2373: Unable to create a Template on OCP3 #2375

Merged
merged 1 commit into from Jul 31, 2020
Merged

Fix #2373: Unable to create a Template on OCP3 #2375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@

### 4.10-SNAPSHOT
#### Bugs
* Fix #2373: Unable to create a Template on OCP3
* Fix #2316: Cannot load resource from stream without apiVersion

#### Improvements
Expand Down
55 changes: 43 additions & 12 deletions ...t/src/main/java/io/fabric8/kubernetes/client/utils/BackwardsCompatibilityInterceptor.java
View file
Open in desktop
Expand Up @@ -132,8 +132,8 @@ public int hashCode() {
public Response intercept(Chain chain) throws IOException {
Request request = chain.request();
Response response = chain.proceed(request);
if (isDeprecatedOpenshiftOapiRequest(request)) {
return handleOpenshiftOapiRequests(request, response, chain);
if (isOpenshiftApiRequest(request)) {
return handleOpenshiftRequests(request, response, chain);
} else if (!response.isSuccessful() && responseCodeToTransformations.keySet().contains(response.code())) {
String url = request.url().toString();
Matcher matcher = getMatcher(url);
Expand Down Expand Up @@ -169,21 +169,28 @@ 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 handleOpenshiftOapiRequests(Request request, Response response, Chain chain) throws IOException{
private static Response handleOpenshiftRequests(Request request, Response response, Chain chain) throws IOException{
if (!response.isSuccessful()) {
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);
ResourceKey target = getResourceKeyFromRequest(request);
if (target != null) {
requestUrl = requestUrl.replace("/oapi", "/apis/" + target.getGroup());
String requestUrl = request.url().toString();
requestUrl = isOpenShift4Request(requestUrl) ?
convertToOpenShiftOapiUrl(requestUrl, target) :
convertToOpenShift4Url(requestUrl, target);
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);
Expand All @@ -206,10 +213,34 @@ private static Response handleNewRequestAndProceed(Request request, String newUr
return chain.proceed(newRequest.build());
}

private static boolean isDeprecatedOpenshiftOapiRequest(Request request) {
if (request != null && request.url() != null) {
return request.url().toString().contains("oapi");
private static boolean isOpenshiftApiRequest(Request request) {
if (request != null) {
String requestUrl = request.url().toString();
return isOpenshift3OapiRequest(requestUrl) || isOpenShift4Request(requestUrl);
}
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;
}
}
45 changes: 45 additions & 0 deletions kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/BuildConfigTest.java
View file
Open in desktop
Expand Up @@ -36,6 +36,7 @@
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.SocketTimeoutException;
import java.util.concurrent.TimeUnit;

Expand Down Expand Up @@ -200,4 +201,48 @@ 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()
.withNewSpec()
.withRunPolicy("Serial")
.addNewTrigger().withType("GitHub").withNewGithub().withSecret("secret101").endGithub().endTrigger()
.addNewTrigger().withType("Generic").withNewGeneric().withSecret("secret101").endGeneric().endTrigger()
.addNewTrigger().withType("ImageChange").endTrigger()
.withNewSource()
.withNewGit().withUri("https://github.com/openshift/ruby-hello-world").endGit()
.endSource()
.withNewStrategy()
.withNewSourceStrategy()
.withNewFrom()
.withKind("ImageStreamTag")
.withName("ruby-20-centos7:latest")
.endFrom()
.endSourceStrategy()
.endStrategy()
.withNewOutput()
.withNewTo().withKind("ImageStreamTag").withName("origin-ruby-sample:latest").endTo()
.endOutput()
.withNewPostCommit().withScript("bundle exec rake test").endPostCommit()
.endSpec()
.build();
}

}
97 changes: 59 additions & 38 deletions ...tes-tests/src/test/java/io/fabric8/openshift/client/server/mock/DeploymentConfigTest.java
View file
Open in desktop
Expand Up @@ -38,6 +38,7 @@
import io.fabric8.openshift.api.model.DeploymentConfigListBuilder;
import io.fabric8.openshift.client.OpenShiftClient;

import java.net.HttpURLConnection;
import java.util.concurrent.atomic.AtomicBoolean;

@EnableRuleMigrationSupport
Expand All @@ -46,7 +47,7 @@ public class DeploymentConfigTest {
public OpenShiftServer server = new OpenShiftServer();

@Test
public void testList() {
void testList() {
server.expect().withPath("/apis/apps.openshift.io/v1/namespaces/test/deploymentconfigs").andReturn(200, new DeploymentConfigListBuilder().build()).once();
server.expect().withPath("/apis").andReturn(200, new APIGroupListBuilder()
.addNewGroup()
Expand Down Expand Up @@ -84,7 +85,7 @@ public void testList() {
}

@Test
public void testGet() {
void testGet() {
server.expect().withPath("/apis/apps.openshift.io/v1/namespaces/test/deploymentconfigs/dc1").andReturn(200, new DeploymentConfigBuilder()
.withNewMetadata().withName("dc1").endMetadata()
.build()).once();
Expand All @@ -108,7 +109,7 @@ public void testGet() {
}

@Test
public void testDelete() throws InterruptedException {
void testDelete() throws InterruptedException {
DeploymentConfig dc1 = new DeploymentConfigBuilder()
.withNewMetadata()
.withName("dc1")
Expand Down Expand Up @@ -165,7 +166,7 @@ public void testDelete() throws InterruptedException {
}

@Test
public void testDeleteWithPropagationPolicy() throws InterruptedException {
void testDeleteWithPropagationPolicy() throws InterruptedException {
server.expect().delete()
.withPath("/apis/apps.openshift.io/v1/namespaces/test/deploymentconfigs/dc1")
.andReturn(200, new DeploymentConfigBuilder().build())
Expand All @@ -179,7 +180,7 @@ public void testDeleteWithPropagationPolicy() throws InterruptedException {
}

@Test
public void testDeployingLatest() {
void testDeployingLatest() {
server.expect().withPath("/apis/apps.openshift.io/v1/namespaces/test/deploymentconfigs/dc1")
.andReturn(200, new DeploymentConfigBuilder().withNewMetadata().withName("dc1").endMetadata()
.withNewStatus().withLatestVersion(1L).endStatus().build())
Expand All @@ -194,11 +195,11 @@ public void testDeployingLatest() {

DeploymentConfig deploymentConfig = client.deploymentConfigs().withName("dc1").deployLatest();
assertNotNull(deploymentConfig);
assertEquals(new Long(2), deploymentConfig.getStatus().getLatestVersion());
assertEquals(Long.valueOf(2), deploymentConfig.getStatus().getLatestVersion());
}

@Test
public void testDeployingLatestHandlesMissingLatestVersion() {
void testDeployingLatestHandlesMissingLatestVersion() {
server.expect().withPath("/apis/apps.openshift.io/v1/namespaces/test/deploymentconfigs/dc1")
.andReturn(200, new DeploymentConfigBuilder().withNewMetadata().withName("dc1").endMetadata()
.withNewStatus().endStatus().build())
Expand All @@ -213,43 +214,15 @@ public void testDeployingLatestHandlesMissingLatestVersion() {

DeploymentConfig deploymentConfig = client.deploymentConfigs().withName("dc1").deployLatest();
assertNotNull(deploymentConfig);
assertEquals(new Long(1), deploymentConfig.getStatus().getLatestVersion());
assertEquals(Long.valueOf(1), deploymentConfig.getStatus().getLatestVersion());
}

//This is a test that verifies a recent fix (sundrio #135).
//According to this issue when editing a list of buildables using predicates, the object visitors get overwrriten.
@Test
public void testDeploymentConfigVisitor() {
void testDeploymentConfigVisitor() {
AtomicBoolean visitedContainer = new AtomicBoolean();

DeploymentConfig dc1 = new DeploymentConfigBuilder()
.withNewMetadata()
.withName("dc1")
.endMetadata()
.withNewSpec()
.withReplicas(1)
.addToSelector("name", "dc1")
.addNewTrigger()
.withType("ImageChange")
.withNewImageChangeParams()
.withAutomatic(true)
.withContainerNames("container")
.withNewFrom()
.withKind("ImageStreamTag")
.withName("image:1.0")
.endFrom()
.endImageChangeParams()
.endTrigger()
.withNewTemplate()
.withNewSpec()
.addNewContainer()
.withName("container")
.withImage("image")
.endContainer()
.endSpec()
.endTemplate()
.endSpec()
.build();
DeploymentConfig dc1 = getDeploymentConfig().build();

DeploymentConfig dc2 = new DeploymentConfigBuilder(dc1)
.accept(new TypedVisitor<DeploymentConfigSpecFluent<?>>() {
Expand All @@ -268,6 +241,54 @@ public void visit(ContainerFluent<?> container) {

}
}).build();
assertNotNull(dc2);
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());
}

private DeploymentConfigBuilder getDeploymentConfig() {
return new DeploymentConfigBuilder()
.withNewMetadata()
.withName("dc1")
.endMetadata()
.withNewSpec()
.withReplicas(1)
.addToSelector("name", "dc1")
.addNewTrigger()
.withType("ImageChange")
.withNewImageChangeParams()
.withAutomatic(true)
.withContainerNames("container")
.withNewFrom()
.withKind("ImageStreamTag")
.withName("image:1.0")
.endFrom()
.endImageChangeParams()
.endTrigger()
.withNewTemplate()
.withNewSpec()
.addNewContainer()
.withName("container")
.withImage("image")
.endContainer()
.endSpec()
.endTemplate()
.endSpec();
}
}
47 changes: 47 additions & 0 deletions ...src/test/java/io/fabric8/openshift/client/server/mock/SecurityContextConstraintsTest.java
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@

package io.fabric8.openshift.client.server.mock;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.openshift.api.model.SecurityContextConstraints;
import io.fabric8.openshift.api.model.SecurityContextConstraintsBuilder;
import io.fabric8.openshift.api.model.SecurityContextConstraintsList;
Expand All @@ -27,6 +28,9 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport;

import java.net.HttpURLConnection;
import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
Expand All @@ -37,6 +41,49 @@ public class SecurityContextConstraintsTest {
@Rule
public OpenShiftServer server = new OpenShiftServer();

@Test
void testCreateOrReplace() {
// Given
SecurityContextConstraints scc = new SecurityContextConstraintsBuilder()
.withNewMetadata().withName("scc1").endMetadata()
.withAllowPrivilegedContainer(true)
.withNewRunAsUser().withType("RunAsAny").endRunAsUser()
.withNewSeLinuxContext().withType("RunAsAny").endSeLinuxContext()
.withUsers("admin")
.withGroups("admin-group")
.build();
server.expect().post().withPath("/apis/security.openshift.io/v1/securitycontextconstraints")
.andReturn(HttpURLConnection.HTTP_OK, scc)
.once();
OpenShiftClient client = server.getOpenshiftClient();

// When
scc = client.securityContextConstraints().createOrReplace(scc);

// Then
assertNotNull(scc);
assertEquals("scc1", scc.getMetadata().getName());
assertEquals(1, scc.getUsers().size());
assertEquals(1, scc.getGroups().size());
}

@Test
void testLoad() {
// Given
server.expect().post().withPath("/apis/security.openshift.io/v1/securitycontextconstraints")
.andReturn(HttpURLConnection.HTTP_OK, new SecurityContextConstraintsBuilder().build())
.once();
OpenShiftClient client = server.getOpenshiftClient();

// When
List<HasMetadata> items = client.load(getClass().getResourceAsStream("/test-scc.yml")).createOrReplace();

// Then
assertNotNull(items);
assertEquals(1, items.size());
assertTrue(items.get(0) instanceof SecurityContextConstraints);
}

@Test
public void testList() {
server.expect().withPath("/apis/security.openshift.io/v1/securitycontextconstraints").andReturn(200, new SecurityContextConstraintsListBuilder()
Expand Down