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.

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

Update createOrReplace to work more like BaseOperation #2289

Merged
merged 1 commit into from Jun 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
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
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