Skip to content

Commit

Permalink
fix fabric8io#3993: further restricting / refining type resolution
Browse files Browse the repository at this point in the history
  • Loading branch information
shawkins committed Apr 5, 2022
1 parent db396dc commit 2bfc938
Show file tree
Hide file tree
Showing 20 changed files with 24 additions and 306 deletions.
8 changes: 7 additions & 1 deletion doc/MIGRATION-v6.md
Expand Up @@ -89,7 +89,13 @@ To use it, exclude the kubernetes-httpclient-okhttp dependency and add the kuber

## Deserialization Resolution

The group on the object being deserialized is not required to match the prospective class - even for built-in types. This prevents the unintentional parsing of custom types without a registered class as a built-in type of the same name. This also means that you must ensure the apiVersion values are correct on the objects you are deserializing as they will no longer resolve to built-in type of the same name when there is a mistake.
The apiVersion on an resource being deserialized is required.

If a version only is specified as the apiVersion, it may match kubernetes built-in types with an empty group (for example Pod), or OpenShift built in types (for example kind: Template apiVersion: v1 is allowed to match apiVersion: template.openshift.io/v1).

Otherwise with group and version specified, the resource match uniquely with a class. This prevents the unintentional parsing of custom types without a registered class as a built-in type of the same name.

This means that you must ensure the apiVersion values are correct on the objects you are deserializing as they will no longer resolve to built-in type of the same name when there is a mistake.

## Deprecation Removals

Expand Down
Expand Up @@ -16,6 +16,7 @@

---
kind: ReplicationController
apiVersion: v1
metadata:
name: rc-get
spec:
Expand All @@ -35,6 +36,7 @@ spec:
- containerPort: 80
---
kind: ReplicationController
apiVersion: v1
metadata:
name: rc-list
spec:
Expand All @@ -54,6 +56,7 @@ spec:
- containerPort: 80
---
kind: ReplicationController
apiVersion: v1
metadata:
name: rc-update
spec:
Expand All @@ -73,6 +76,7 @@ spec:
- containerPort: 80
---
kind: ReplicationController
apiVersion: v1
metadata:
name: rc-delete
spec:
Expand Down
8 changes: 4 additions & 4 deletions kubernetes-itests/src/test/resources/secret-it.yml
Expand Up @@ -15,7 +15,7 @@
#

---
apiversion: v1
apiVersion: v1
kind: Secret
metadata:
name: secret-get
Expand All @@ -24,7 +24,7 @@ data:
username: "guccifer"
password: "shadowgovernment"
---
apiversion: v1
apiVersion: v1
kind: Secret
metadata:
name: secret-list
Expand All @@ -33,7 +33,7 @@ data:
username: "guccifer"
password: "shadowgovernment"
---
apiversion: v1
apiVersion: v1
kind: Secret
metadata:
name: secret-update
Expand All @@ -42,7 +42,7 @@ data:
username: "guccifer"
password: "shadowgovernment"
---
apiversion: v1
apiVersion: v1
kind: Secret
metadata:
name: secret-delete
Expand Down
Expand Up @@ -282,8 +282,10 @@ public Class<? extends KubernetesResource> getForKey(TypeKey key) {
}
bestMatch = typeKey;
break;
} else if (typeKey.apiGroup != null && !typeKey.apiGroup.endsWith(".openshift.io")) {
continue;
}
if (key.version != null && key.version.equals(typeKey.apiGroup)) {
if (key.version != null && key.version.equals(typeKey.version)) {
bestMatch = typeKey;
break;
}
Expand Down
Expand Up @@ -708,21 +708,6 @@ void testDeploymentGetLogMultiContainer() {
assertEquals("hello", log);
}

@Test
void testDeploymentLoadWithoutApiVersion() {
// Given

// 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());
}

private DeploymentBuilder createDeploymentBuilder() {
return new DeploymentBuilder()
.withNewMetadata()
Expand Down
Expand Up @@ -181,19 +181,6 @@ void testCreateWithNameMismatch() {
Assertions.assertThrows(KubernetesClientException.class, () -> ingressOp.create(ingress1));
}

@Test
void testIngressLoadWithoutApiVersion() {
// Given

// 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 io.fabric8.kubernetes.api.model.networking.v1.Ingress);
}

@Test
void testCreateOrReplaceWhenAnnotationUpdated() {
// Given
Expand Down
Expand Up @@ -178,19 +178,6 @@ void testCreateWithNameMismatch() {
});
}

@Test
void testIngressLoadWithoutApiVersion() {
// Given

// 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 io.fabric8.kubernetes.api.model.networking.v1.Ingress);
}

@Test
void testCreateOrReplaceWhenAnnotationUpdated() {
// Given
Expand Down
Expand Up @@ -25,7 +25,6 @@
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.server.mock.EnableKubernetesMockClient;
import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -120,17 +119,6 @@ void testDelete() {
assertTrue(deleted);
}

@Test
void testCustomResourceDefinitionTest() {
// 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
Expand Up @@ -189,17 +189,4 @@ void testBuild() {
assertNotNull(horizontalPodAutoscaler);
}

@Test
void testHorizontalPodAutoscalerLoadWithNoApiVersion() {
// Given

// 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);
}

}
Expand Up @@ -26,8 +26,6 @@
import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer;
import org.junit.jupiter.api.Test;

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 @@ -101,19 +99,6 @@ public void delete() {
assertTrue(isDeleted);
}

@Test
void testMutatingWebhookConfigurationLoadWithNoApiVersion() {
// Given

// 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
Expand Up @@ -26,8 +26,6 @@
import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer;
import org.junit.jupiter.api.Test;

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 @@ -110,18 +108,6 @@ public void delete() {
assertTrue(isDeleted);
}

@Test
void testValidatingWebhookConfigurationLoadWithNoApiVersion() {

// 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
31 changes: 0 additions & 31 deletions kubernetes-tests/src/test/resources/test-crd-no-apiversion.yml

This file was deleted.

2 changes: 1 addition & 1 deletion kubernetes-tests/src/test/resources/test-cronjob.yml
Expand Up @@ -14,7 +14,7 @@
# limitations under the License.
#

apiVersion: batch/v2alpha1
apiVersion: batch/v1beta1
kind: CronJob
metadata:
name: pi
Expand Down
34 changes: 0 additions & 34 deletions kubernetes-tests/src/test/resources/test-hpa-no-apiversion.yml

This file was deleted.

30 changes: 0 additions & 30 deletions kubernetes-tests/src/test/resources/test-ingress-no-apiversion.yml

This file was deleted.

30 changes: 0 additions & 30 deletions kubernetes-tests/src/test/resources/test-mwc-no-apiversion.yml

This file was deleted.

0 comments on commit 2bfc938

Please sign in to comment.