Skip to content

Commit

Permalink
Update createOrReplace to properly propagate non-conflict exceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
@bbeaudreault
bbeaudreault committed Jun 17, 2020
1 parent 2d1a9d4 commit 1fb9e2b
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 114 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -2,6 +2,7 @@

### 4.10-SNAPSHOT
#### Bugs
* Fix #2285: Raw CustomResource API createOrReplace does not propagate exceptions from create
* Fix Raw CustomResource API path generation to not having trailing slash
* Fix #2131: Failing to parse CustomResourceDefinition with OpenAPIV3Schema using JSONSchemaPropOr\* fields
* Fix KubernetesAttributesExctractor to extract metadata from unregistered custom resources, such when using Raw CustomResource API
Expand Down
74 changes: 48 additions & 26 deletions .../main/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImpl.java
View file
Expand Up @@ -15,7 +15,17 @@
*/
package io.fabric8.kubernetes.client.dsl.internal;

import java.io.IOException;
import java.io.InputStream;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;

import com.fasterxml.jackson.databind.ObjectMapper;

import io.fabric8.kubernetes.api.model.DeleteOptions;
import io.fabric8.kubernetes.api.model.DeletionPropagation;
import io.fabric8.kubernetes.api.model.ListOptions;
Expand All @@ -38,14 +48,6 @@
import okhttp3.RequestBody;
import okhttp3.Response;

import java.io.IOException;
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;

/**
* This class simple does basic operations for custom defined resources without
* demanding the POJOs for custom resources. It is serializing/deserializing
Expand All @@ -55,6 +57,9 @@
*
*/
public class RawCustomResourceOperationsImpl extends OperationSupport {

private static final String METADATA = "metadata";
private static final String RESOURCE_VERSION = "resourceVersion";
private OkHttpClient client;
private Config config;
private CustomResourceDefinitionContext customResourceDefinition;
Expand Down Expand Up @@ -174,7 +179,7 @@ public Map<String, Object> create(String namespace, Map<String, Object> object)
* @throws IOException in case of network/serializiation failures or failures from Kuberntes API
*/
public Map<String, Object> createOrReplace(String objectAsString) throws IOException {
return createOrReplaceJsonStringObject(null, objectAsString);
return createOrReplaceObject(null, load(objectAsString));
}

/**
Expand All @@ -185,7 +190,7 @@ public Map<String, Object> createOrReplace(String objectAsString) throws IOExcep
* @throws IOException in case of network/serialization failures or failures from Kubernetes API
*/
public Map<String, Object> createOrReplace(Map<String, Object> customResourceObject) throws IOException {
return createOrReplace(objectMapper.writeValueAsString(customResourceObject));
return createOrReplaceObject(null, customResourceObject);
}

/**
Expand All @@ -196,7 +201,7 @@ public Map<String, Object> createOrReplace(Map<String, Object> customResourceObj
* @throws IOException in case of network/serialization failures or failures from Kubernetes API
*/
public Map<String, Object> createOrReplace(InputStream inputStream) throws IOException {
return createOrReplace(IOHelpers.readFully(inputStream));
return createOrReplaceObject(null, load(inputStream));
}

/**
Expand All @@ -208,7 +213,7 @@ public Map<String, Object> createOrReplace(InputStream inputStream) throws IOExc
* @throws IOException in case of network/serialization failures or failures from Kubernetes API
*/
public Map<String, Object> createOrReplace(String namespace, String objectAsString) throws IOException {
return createOrReplaceJsonStringObject(namespace, objectAsString);
return createOrReplaceObject(namespace, load(objectAsString));
}

/**
Expand All @@ -220,19 +225,19 @@ public Map<String, Object> createOrReplace(String namespace, String objectAsStri
* @throws IOException in case of network/serialization failures or failures from Kubernetes API
*/
public Map<String, Object> createOrReplace(String namespace, Map<String, Object> customResourceObject) throws IOException {
return createOrReplace(namespace, objectMapper.writeValueAsString(customResourceObject));
return createOrReplaceObject(namespace, customResourceObject);
}

/**
* Create or replace a custom resource which is namespaced object.
*
* @param namespace desired namespace
* @param objectAsString object as file input stream
* @param objectAsStream object as file input stream
* @return Object as HashMap
* @throws IOException in case of network/serialization failures or failures from Kubernetes API
*/
public Map<String, Object> createOrReplace(String namespace, InputStream objectAsString) throws IOException {
return createOrReplace(namespace, IOHelpers.readFully(objectAsString));
public Map<String, Object> createOrReplace(String namespace, InputStream objectAsStream) throws IOException {
return createOrReplaceObject(namespace, load(objectAsStream));
}

/**
Expand Down Expand Up @@ -680,22 +685,39 @@ public Watch watch(String namespace, String name, Map<String, String> labels, Li

}

private Map<String, Object> createOrReplaceJsonStringObject(String namespace, String objectAsString) throws IOException {
private Map<String, Object> createOrReplaceObject(String namespace, Map<String, Object> objectAsMap) throws IOException {
Map<String, Object> metadata = (Map<String, Object>) objectAsMap.get(METADATA);
if (metadata == null) {
throw KubernetesClientException.launderThrowable(new IllegalStateException("Invalid object provided -- metadata is required."));
}

Map<String, Object> ret;

// can't include resourceVersion in create calls
String originalResourceVersion = (String) metadata.get(RESOURCE_VERSION);
metadata.remove(RESOURCE_VERSION);

try {
if(namespace != null) {
ret = create(namespace, objectAsString);
ret = create(namespace, objectAsMap);
} else {
ret = create(objectAsString);
ret = create(objectAsMap);
}
} catch (KubernetesClientException exception) {
if (exception.getCode() != HttpURLConnection.HTTP_CONFLICT) {
throw exception;
}

try {
Map<String, Object> objectMap = load(objectAsString);
String name = ((Map<String, Object>) objectMap.get("metadata")).get("name").toString();
ret = namespace != null ?
edit(namespace, name, objectAsString) : edit(name, objectAsString);
// re-add for edit call
if (originalResourceVersion != null) {
metadata.put(RESOURCE_VERSION, originalResourceVersion);
}
String name = (String) metadata.get("name");
ret = namespace != null ?
edit(namespace, name, objectAsMap) : edit(name, objectAsMap);
} catch (NullPointerException nullPointerException) {
throw KubernetesClientException.launderThrowable(new IllegalStateException("Invalid json string provided."));
throw KubernetesClientException.launderThrowable(new IllegalStateException("Invalid object provided -- metadata.name is required."));
}
}
return ret;
Expand Down Expand Up @@ -832,10 +854,10 @@ private Request getRequest(String url, String body, HttpCallMethod httpCallMetho

private String appendResourceVersionInObject(String namespace, String customResourceName, String customResourceAsJsonString) throws IOException {
Map<String, Object> oldObject = get(namespace, customResourceName);
String resourceVersion = ((Map<String, Object>)oldObject.get("metadata")).get("resourceVersion").toString();
String resourceVersion = ((Map<String, Object>)oldObject.get(METADATA)).get(RESOURCE_VERSION).toString();

Map<String, Object> newObject = convertJsonOrYamlStringToMap(customResourceAsJsonString);
((Map<String, Object>)newObject.get("metadata")).put("resourceVersion", resourceVersion);
((Map<String, Object>)newObject.get(METADATA)).put(RESOURCE_VERSION, resourceVersion);

return objectMapper.writeValueAsString(newObject);
}
Expand Down
73 changes: 52 additions & 21 deletions ...t/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImplTest.java
View file
Expand Up @@ -15,37 +15,41 @@
*/
package io.fabric8.kubernetes.client.dsl.internal;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.Mockito.*;

import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.util.HashMap;
import java.util.Map;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;

import io.fabric8.kubernetes.api.model.ListOptionsBuilder;
import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.ConfigBuilder;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.client.dsl.base.CustomResourceDefinitionContext;
import io.fabric8.kubernetes.client.utils.Utils;
import okhttp3.Call;
import okhttp3.HttpUrl;
import okhttp3.MediaType;
import okhttp3.OkHttpClient;
import okhttp3.Protocol;
import okhttp3.Request;
import okhttp3.Response;
import okhttp3.ResponseBody;

import org.bouncycastle.cert.ocsp.Req;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;

import java.io.IOException;
import java.net.MalformedURLException;
import java.util.HashMap;
import java.util.Map;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.*;

public class RawCustomResourceOperationsImplTest {
private OkHttpClient mockClient;
private Config config;
private CustomResourceDefinitionContext customResourceDefinitionContext;
private Response mockSuccessResponse;

@BeforeEach
public void setUp() throws IOException {
Expand All @@ -60,11 +64,9 @@ public void setUp() throws IOException {
.build();

Call mockCall = mock(Call.class);
Response mockResponse = mock(Response.class);
when(mockResponse.isSuccessful()).thenReturn(true);
when(mockResponse.body()).thenReturn(ResponseBody.create(MediaType.get("application/json"), ""));
mockSuccessResponse = mockResponse(HttpURLConnection.HTTP_OK);
when(mockCall.execute())
.thenReturn(mockResponse);
.thenReturn(mockSuccessResponse);
when(mockClient.newCall(any())).thenReturn(mockCall);
}

Expand All @@ -75,15 +77,44 @@ void testCreateOrReplaceUrl() throws IOException {
String resourceAsString = "{\"metadata\":{\"name\":\"myresource\",\"namespace\":\"myns\"}, \"kind\":\"raw\", \"apiVersion\":\"v1\"}";
ArgumentCaptor<Request> captor = ArgumentCaptor.forClass(Request.class);

Call mockCall = mock(Call.class);
Response mockErrorResponse = mockResponse(HttpURLConnection.HTTP_INTERNAL_ERROR);
Response mockConflictResponse = mockResponse(HttpURLConnection.HTTP_CONFLICT);
when(mockCall.execute())
.thenReturn(mockErrorResponse, mockConflictResponse, mockSuccessResponse);
when(mockClient.newCall(any())).thenReturn(mockCall);

// When
try {
rawCustomResourceOperations.createOrReplace(resourceAsString);
fail("expected first call to createOrReplace to throw exception due to 500 response");
} catch (KubernetesClientException e) {
assertEquals(HttpURLConnection.HTTP_INTERNAL_ERROR, e.getCode());
}
rawCustomResourceOperations.createOrReplace(resourceAsString);
rawCustomResourceOperations.createOrReplace("myns", resourceAsString);

// Then
verify(mockClient, times(2)).newCall(captor.capture());
assertEquals(2, captor.getAllValues().size());
verify(mockClient, times(4)).newCall(captor.capture());
assertEquals(4, captor.getAllValues().size());
assertEquals("/apis/test.fabric8.io/v1alpha1/hellos", captor.getAllValues().get(0).url().encodedPath());
assertEquals("/apis/test.fabric8.io/v1alpha1/namespaces/myns/hellos", captor.getAllValues().get(1).url().encodedPath());
assertEquals("POST", captor.getAllValues().get(0).method());
assertEquals("/apis/test.fabric8.io/v1alpha1/hellos", captor.getAllValues().get(1).url().encodedPath());
assertEquals("POST", captor.getAllValues().get(1).method());
assertEquals("/apis/test.fabric8.io/v1alpha1/hellos/myresource", captor.getAllValues().get(2).url().encodedPath());
assertEquals("PUT", captor.getAllValues().get(2).method());
assertEquals("/apis/test.fabric8.io/v1alpha1/namespaces/myns/hellos", captor.getAllValues().get(3).url().encodedPath());
assertEquals("POST", captor.getAllValues().get(3).method());
}

private Response mockResponse(int code) {
return new Response.Builder()
.request(new Request.Builder().url("http://mock").build())
.protocol(Protocol.HTTP_1_1)
.code(code)
.body(ResponseBody.create(MediaType.get("application/json"), ""))
.message("mock")
.build();
}

@Test
Expand Down
32 changes: 21 additions & 11 deletions kubernetes-itests/src/test/java/io/fabric8/kubernetes/RawCustomResourceIT.java
View file
Expand Up @@ -15,10 +15,14 @@
*/
package io.fabric8.kubernetes;

import com.fasterxml.jackson.databind.ObjectMapper;
import io.fabric8.kubernetes.api.model.apiextensions.CustomResourceDefinition;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.dsl.base.CustomResourceDefinitionContext;
import static org.assertj.core.api.Assertions.assertThat;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import org.arquillian.cube.kubernetes.api.Session;
import org.arquillian.cube.kubernetes.impl.requirement.RequiresKubernetes;
import org.arquillian.cube.requirement.ArquillianConditionalRunner;
Expand All @@ -28,13 +32,11 @@
import org.junit.Test;
import org.junit.runner.RunWith;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import com.fasterxml.jackson.databind.ObjectMapper;

import static org.assertj.core.api.Assertions.assertThat;
import io.fabric8.kubernetes.api.model.apiextensions.CustomResourceDefinition;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.dsl.base.CustomResourceDefinitionContext;

@RunWith(ArquillianConditionalRunner.class)
@RequiresKubernetes
Expand Down Expand Up @@ -85,12 +87,20 @@ public void testCrud() throws IOException {
// Test Create via file
Map<String, Object> object = client.customResource(customResourceDefinitionContext).create(currentNamespace, getClass().getResourceAsStream("/test-rawcustomresource.yml"));
assertThat(((HashMap<String, String>)object.get("metadata")).get("name")).isEqualTo("otter");

// Test Create via raw json string
String rawJsonCustomResourceObj = "{\"apiVersion\":\"jungle.example.com/v1\"," +
"\"kind\":\"Animal\",\"metadata\": {\"name\": \"walrus\"}," +
"\"spec\": {\"image\": \"my-awesome-walrus-image\"}}";
object = client.customResource(customResourceDefinitionContext).create(currentNamespace, rawJsonCustomResourceObj);
object = client.customResource(customResourceDefinitionContext).createOrReplace(currentNamespace, rawJsonCustomResourceObj);
assertThat(((HashMap<String, String>)object.get("metadata")).get("name")).isEqualTo("walrus");
assertThat(((HashMap<String, String>)object.get("spec")).get("image")).isEqualTo("my-awesome-walrus-image");

// Test replace with object
((HashMap<String, String>)object.get("spec")).put("image", "new-walrus-image");
object = client.customResource(customResourceDefinitionContext).createOrReplace(currentNamespace, object);
assertThat(((HashMap<String, String>)object.get("metadata")).get("name")).isEqualTo("walrus");
assertThat(((HashMap<String, String>)object.get("spec")).get("image")).isEqualTo("new-walrus-image");

// Test Get:
object = client.customResource(customResourceDefinitionContext).get(currentNamespace, "otter");
Expand Down

0 comments on commit 1fb9e2b

Please sign in to comment.