Skip to content

Commit

Permalink
Fix fabric8io#2316: Cannot load resource from stream without apiVersion
Browse files Browse the repository at this point in the history 
In case of duplicate Kubernetes resources found, when no apiVersion
is provided return first item found instead of null
  • Loading branch information
@rohanKanojia
rohanKanojia committed Jul 28, 2020
1 parent a0a4516 commit 522b07b
Show file tree
Hide file tree
Showing 14 changed files with 323 additions and 25 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Expand Up @@ -2,12 +2,13 @@

### 4.10-SNAPSHOT
#### Bugs
* Fix #2316: Cannot load resource from stream without apiVersion

#### Improvements

#### Dependency Upgrade
* Fix #2355: bump jandex from 2.1.3.Final to 2.2.0.Final
* Fix #2353: bump workflow action-setup-* versions + kubernetes to 1.18.6
* Fix #2353: bump workflow action-setup- versions + kubernetes to 1.18.6

#### New Features
* Fix #2287: Add support for V1 and V1Beta1 CustomResourceDefinition
Expand Down
21 changes: 13 additions & 8 deletions ...netes-model-core/src/main/java/io/fabric8/kubernetes/internal/KubernetesDeserializer.java
View file
Expand Up @@ -126,6 +126,9 @@ public static void registerProvider(KubernetesResourceMappingProvider provider)
static class Mapping {

private static final String KEY_SEPARATOR = "#";

// n.b. Packages sorted in order of precedence, deserialization of resources with no
// specific version will default to first available Class in one of these packages:
private static final String[] PACKAGES = {
"io.fabric8.kubernetes.api.model.",
"io.fabric8.kubernetes.api.model.admission",
Expand All @@ -145,7 +148,6 @@ static class Mapping {
"io.fabric8.kubernetes.api.model.coordination.",
"io.fabric8.kubernetes.api.model.coordination.v1.",
"io.fabric8.kubernetes.api.model.discovery.",
"io.fabric8.kubernetes.api.model.extensions.",
"io.fabric8.kubernetes.api.model.events.",
"io.fabric8.kubernetes.api.model.networking.",
"io.fabric8.kubernetes.api.model.networking.v1beta1.",
Expand All @@ -154,7 +156,8 @@ static class Mapping {
"io.fabric8.kubernetes.api.model.storage.",
"io.fabric8.kubernetes.api.model.scheduling.",
"io.fabric8.kubernetes.api.model.settings.",
"io.fabric8.openshift.api.model."
"io.fabric8.openshift.api.model.",
"io.fabric8.kubernetes.api.model.extensions."
};

private Map<String, Class<? extends KubernetesResource>> mappings = new ConcurrentHashMap<>();
Expand Down Expand Up @@ -248,16 +251,18 @@ private Class<? extends KubernetesResource> getInternalTypeForName(String key) {
// If only one class found, return it
if (possibleResults.size() == 1) {
return possibleResults.get(0);
}

// Compare with apiVersions being compared for set of classes found
for (Class<? extends KubernetesResource> result : possibleResults) {
} else if (possibleResults.size() > 1) {
// Compare with apiVersions being compared for set of classes found
for (Class<? extends KubernetesResource> result : possibleResults) {
String defaultKeyFromClass = getKeyFromClass(result);
if (key.equals(defaultKeyFromClass)) {
return result;
return result;
}
}
return possibleResults.get(0);
} else {
return null;
}
return null;
}

private String getKeyFromClass(Class<? extends KubernetesResource> clazz) {
Expand Down
17 changes: 17 additions & 0 deletions kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/DeploymentTest.java
View file
Expand Up @@ -41,6 +41,7 @@
import io.fabric8.kubernetes.client.server.mock.KubernetesServer;

import io.fabric8.kubernetes.client.utils.Utils;
import io.fabric8.kubernetes.model.util.Helper;
import okhttp3.mockwebserver.RecordedRequest;
import org.junit.Rule;
import org.junit.jupiter.api.DisplayName;
Expand Down Expand Up @@ -772,4 +773,20 @@ private DeploymentBuilder getDeploymentBuilder() {
.endSpec();
}

@Test
void testDeploymentLoadWithoutApiVersion() {
// Given
KubernetesClient client = server.getClient();

// When
List<HasMetadata> list = client.load(getClass().getResourceAsStream("/valid-deployment-without-apiversion.json")).get();
Deployment deployment = (Deployment) list.get(0);

// Then
assertNotNull(deployment);
assertEquals("test", deployment.getMetadata().getName());
assertEquals(1, deployment.getSpec().getReplicas());
assertEquals(1, deployment.getSpec().getTemplate().getSpec().getContainers().size());
}

}
28 changes: 21 additions & 7 deletions ...s-tests/src/test/java/io/fabric8/kubernetes/client/mock/NetworkingV1beta1IngressTest.java
View file
Expand Up @@ -54,7 +54,7 @@ void testLoad() {
}

@Test
public void testList() {
void testList() {
server.expect().withPath("/apis/networking.k8s.io/v1beta1/namespaces/test/ingresses").andReturn(200, new IngressListBuilder().build()).once();
server.expect().withPath("/apis/networking.k8s.io/v1beta1/namespaces/ns1/ingresses").andReturn(200, new IngressListBuilder()
.addNewItem().and()
Expand Down Expand Up @@ -82,7 +82,7 @@ public void testList() {
}

@Test
public void testListWithLables() {
void testListWithLables() {
server.expect().withPath("/apis/networking.k8s.io/v1beta1/namespaces/test/ingresses?labelSelector=" + Utils.toUrlEncoded("key1=value1,key2=value2,key3=value3")).andReturn(200, new IngressListBuilder().build()).always();
server.expect().withPath("/apis/networking.k8s.io/v1beta1/namespaces/test/ingresses?labelSelector=" + Utils.toUrlEncoded("key1=value1,key2=value2")).andReturn(200, new IngressListBuilder()
.addNewItem().and()
Expand Down Expand Up @@ -112,7 +112,7 @@ public void testListWithLables() {


@Test
public void testGet() {
void testGet() {
server.expect().withPath("/apis/networking.k8s.io/v1beta1/namespaces/test/ingresses/ingress1").andReturn(200, new IngressBuilder().build()).once();
server.expect().withPath("/apis/networking.k8s.io/v1beta1/namespaces/ns1/ingresses/ingress2").andReturn(200, new IngressBuilder().build()).once();

Expand All @@ -130,7 +130,7 @@ public void testGet() {


@Test
public void testDelete() {
void testDelete() {
server.expect().withPath("/apis/networking.k8s.io/v1beta1/namespaces/test/ingresses/ingress1").andReturn(200, new IngressBuilder().build()).once();
server.expect().withPath("/apis/networking.k8s.io/v1beta1/namespaces/ns1/ingresses/ingress2").andReturn(200, new IngressBuilder().build()).once();

Expand All @@ -148,7 +148,7 @@ public void testDelete() {


@Test
public void testDeleteMulti() {
void testDeleteMulti() {
Ingress ingress1 = new IngressBuilder().withNewMetadata().withName("ingress1").withNamespace("test").and().build();
Ingress ingress2 = new IngressBuilder().withNewMetadata().withName("ingress2").withNamespace("ns1").and().build();
Ingress ingress3 = new IngressBuilder().withNewMetadata().withName("ingress3").withNamespace("any").and().build();
Expand All @@ -166,7 +166,7 @@ public void testDeleteMulti() {
}

@Test
public void testDeleteWithNamespaceMismatch() {
void testDeleteWithNamespaceMismatch() {
Assertions.assertThrows(KubernetesClientException.class, () -> {
Ingress ingress1 = new IngressBuilder().withNewMetadata().withName("ingress1").withNamespace("test").and().build();
Ingress ingress2 = new IngressBuilder().withNewMetadata().withName("ingress2").withNamespace("ns1").and().build();
Expand All @@ -178,7 +178,7 @@ public void testDeleteWithNamespaceMismatch() {
}

@Test
public void testCreateWithNameMismatch() {
void testCreateWithNameMismatch() {
Assertions.assertThrows(KubernetesClientException.class, () -> {
Ingress ingress1 = new IngressBuilder().withNewMetadata().withName("ingress1").withNamespace("test").and().build();
Ingress ingress2 = new IngressBuilder().withNewMetadata().withName("ingress2").withNamespace("ns1").and().build();
Expand All @@ -188,4 +188,18 @@ public void testCreateWithNameMismatch() {
});
}

@Test
void testIngressLoadWithoutApiVersion() {
// Given
KubernetesClient client = server.getClient();

// When
List<HasMetadata> items = client.load(getClass().getResourceAsStream("/test-ingress-no-apiversion.yml")).get();

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

}
15 changes: 15 additions & 0 deletions ...tests/src/test/java/io/fabric8/kubernetes/client/mock/V1CustomResourceDefinitionTest.java
View file
Expand Up @@ -25,6 +25,7 @@
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.server.mock.KubernetesServer;
import org.junit.Rule;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport;
Expand Down Expand Up @@ -124,6 +125,20 @@ void testDelete() {
assertTrue(deleted);
}

@Test
void testCustomResourceDefinitionTest() {
// Given
KubernetesClient client = server.getClient();

// When
List<HasMetadata> items = client.load(getClass().getResourceAsStream("/test-crd-no-apiversion.yml")).get();

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

JSONSchemaProps readSchema() throws IOException {
ObjectMapper mapper = new ObjectMapper();
final URL resource = getClass().getResource("/test-crd-validation-schema.json");
Expand Down
32 changes: 23 additions & 9 deletions ...-tests/src/test/java/io/fabric8/kubernetes/client/mock/V1HorizontalPodAutoscalerTest.java
View file
Expand Up @@ -57,7 +57,7 @@ void testLoadAsPlainList() {
}

@Test
public void testList() {
void testList() {
server.expect().withPath("/apis/autoscaling/v1/namespaces/test/horizontalpodautoscalers").andReturn(200, new HorizontalPodAutoscalerListBuilder().build()).once();
server.expect().withPath("/apis/autoscaling/v1/namespaces/ns1/horizontalpodautoscalers").andReturn(200, new HorizontalPodAutoscalerListBuilder()
.addNewItem().and()
Expand All @@ -75,7 +75,7 @@ public void testList() {
}

@Test
public void testListWithLabels() {
void testListWithLabels() {
server.expect().withPath("/apis/autoscaling/v1/namespaces/test/horizontalpodautoscalers?labelSelector=" + Utils.toUrlEncoded("key1=value1,key2=value2,key3=value3")).andReturn(200, new HorizontalPodAutoscalerListBuilder().build()).once();
server.expect().withPath("/apis/autoscaling/v1/namespaces/ns1/horizontalpodautoscalers?labelSelector=" + Utils.toUrlEncoded("key1=value1,key2=value2")).andReturn(200, new HorizontalPodAutoscalerListBuilder()
.addNewItem().and()
Expand All @@ -101,7 +101,7 @@ public void testListWithLabels() {
}

@Test
public void testGet() {
void testGet() {
server.expect().withPath("/apis/autoscaling/v1/namespaces/test/horizontalpodautoscalers/horizontalpodautoscaler1").andReturn(200, new HorizontalPodAutoscalerBuilder().build()).once();
server.expect().withPath("/apis/autoscaling/v1/namespaces/ns1/horizontalpodautoscalers/horizontalpodautoscaler2").andReturn(200, new HorizontalPodAutoscalerBuilder().build()).once();

Expand All @@ -117,7 +117,7 @@ public void testGet() {
}

@Test
public void testEditMissing() {
void testEditMissing() {

Assertions.assertThrows(KubernetesClientException.class, () -> {
server.expect().withPath("/apis/autoscaling/v1/namespaces/test/horizontalpodautoscalers/horizontalpodautoscaler").andReturn(404, "error message from kubernetes").always();
Expand All @@ -128,7 +128,7 @@ public void testEditMissing() {
}

@Test
public void testDelete() {
void testDelete() {
server.expect().withPath("/apis/autoscaling/v1/namespaces/test/horizontalpodautoscalers/horizontalpodautoscaler1").andReturn(200, new HorizontalPodAutoscalerBuilder().build()).once();
server.expect().withPath("/apis/autoscaling/v1/namespaces/ns1/horizontalpodautoscalers/horizontalpodautoscaler2").andReturn(200, new HorizontalPodAutoscalerBuilder().build()).once();

Expand All @@ -144,7 +144,7 @@ public void testDelete() {
}

@Test
public void testDeleteMulti() {
void testDeleteMulti() {
HorizontalPodAutoscaler horizontalPodAutoscaler1 = new HorizontalPodAutoscalerBuilder().withNewMetadata().withName("horizontalpodautoscaler1").withNamespace("test").endMetadata().build();
HorizontalPodAutoscaler horizontalPodAutoscaler2 = new HorizontalPodAutoscalerBuilder().withNewMetadata().withName("horizontalpodautoscaler2").withNamespace("ns1").endMetadata().build();
HorizontalPodAutoscaler horizontalPodAutoscaler3 = new HorizontalPodAutoscalerBuilder().withNewMetadata().withName("horizontalpodautoscaler3").withNamespace("any").endMetadata().build();
Expand All @@ -161,7 +161,7 @@ public void testDeleteMulti() {
}

@Test
public void testCreateWithNameMismatch() {
void testCreateWithNameMismatch() {
Assertions.assertThrows(KubernetesClientException.class, () -> {
HorizontalPodAutoscaler horizontalPodAutoscaler1 = new HorizontalPodAutoscalerBuilder().withNewMetadata().withName("horizontalpodautoscaler1").withNamespace("test").endMetadata().build();
KubernetesClient client = server.getClient();
Expand All @@ -170,14 +170,14 @@ public void testCreateWithNameMismatch() {
}

@Test
public void testLoadFromFile() {
void testLoadFromFile() {
KubernetesClient client = server.getClient();
HorizontalPodAutoscaler horizontalPodAutoscaler = client.autoscaling().v1().horizontalPodAutoscalers().load(getClass().getResourceAsStream("/test-horizontalpodautoscaler.yml")).get();
assertEquals("php-apache", horizontalPodAutoscaler.getMetadata().getName());
}

@Test
public void testBuild() {
void testBuild() {
HorizontalPodAutoscaler horizontalPodAutoscaler = new HorizontalPodAutoscalerBuilder()
.withNewMetadata().withName("test-hpa").withNamespace("test").endMetadata()
.withNewSpec()
Expand All @@ -198,4 +198,18 @@ public void testBuild() {
assertNotNull(horizontalPodAutoscaler);
}

@Test
void testHorizontalPodAutoscalerLoadWithNoApiVersion() {
// Given
KubernetesClient client = server.getClient();

// When
List<HasMetadata> items = client.load(getClass().getResourceAsStream("/test-hpa-no-apiversion.yml")).get();

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

}
16 changes: 16 additions & 0 deletions ...s/src/test/java/io/fabric8/kubernetes/client/mock/V1MutatingWebhookConfigurationTest.java
View file
Expand Up @@ -27,6 +27,8 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport;

import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -104,6 +106,20 @@ public void delete() {
assertTrue(isDeleted);
}

@Test
void testMutatingWebhookConfigurationLoadWithNoApiVersion() {
// Given
KubernetesClient client = server.getClient();

// When
List<HasMetadata> items = client.load(getClass().getResourceAsStream("/test-mwc-no-apiversion.yml")).get();

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

public MutatingWebhookConfiguration getMutatingWebhookConfigurationSample() {
return new MutatingWebhookConfigurationBuilder()
.withNewMetadata().withName("mutatingWebhookConfiguration1").endMetadata()
Expand Down
17 changes: 17 additions & 0 deletions ...src/test/java/io/fabric8/kubernetes/client/mock/V1ValidatingWebhookConfigurationTest.java
View file
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/
package io.fabric8.kubernetes.client.mock;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.admissionregistration.v1.ValidatingWebhookBuilder;
import io.fabric8.kubernetes.api.model.admissionregistration.v1.ValidatingWebhookConfiguration;
Expand All @@ -26,6 +27,8 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.migrationsupport.rules.EnableRuleMigrationSupport;

import java.util.List;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -112,6 +115,20 @@ public void delete() {
assertTrue(isDeleted);
}

@Test
void testValidatingWebhookConfigurationLoadWithNoApiVersion() {
// Given
KubernetesClient client = server.getClient();

// When
List<HasMetadata> items = client.load(getClass().getResourceAsStream("/test-vwc-no-apiversion.yml")).get();

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

public ValidatingWebhookConfiguration getValidatingWebhookConfigurationSample() {
return new ValidatingWebhookConfigurationBuilder()
.withNewMetadata().withName("validatingWebhookConfiguration1").endMetadata()
Expand Down

0 comments on commit 522b07b

Please sign in to comment.