From d35933ee4581b018545c681bd0b2aec4884f6e2e Mon Sep 17 00:00:00 2001 From: andreaTP Date: Wed, 15 Dec 2021 15:50:09 +0000 Subject: [PATCH 1/6] Avoid a StackOverflow and properly fail on cyclic references in CRD definition #3651 --- .../crd/generator/AbstractJsonSchema.java | 25 +++++++++++------ .../fabric8/crd/generator/v1/JsonSchema.java | 1 + .../crd/generator/v1beta1/JsonSchema.java | 1 + .../io/fabric8/crd/example/cyclic/Cyclic.java | 27 ++++++++++++++++++ .../crd/example/cyclic/CyclicSpec.java | 20 +++++++++++++ .../crd/example/cyclic/CyclicStatus.java | 28 +++++++++++++++++++ .../io/fabric8/crd/example/cyclic/Ref.java | 22 +++++++++++++++ .../crd/generator/CRDGeneratorTest.java | 14 ++++++++++ .../zookeeper/v1alpha1/Zookeeper.java | 3 +- 9 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/Cyclic.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicSpec.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicStatus.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/Ref.java diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java index de7ac69cf27..3534b4d0355 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java @@ -29,19 +29,14 @@ import java.util.*; -import static io.sundr.model.utils.Types.BOOLEAN; import static io.sundr.model.utils.Types.BOOLEAN_REF; -import static io.sundr.model.utils.Types.STRING; import static io.sundr.model.utils.Types.STRING_REF; -import static io.sundr.model.utils.Types.INT; import static io.sundr.model.utils.Types.INT_REF; -import static io.sundr.model.utils.Types.LONG; import static io.sundr.model.utils.Types.LONG_REF; -import static io.sundr.model.utils.Types.DOUBLE; import static io.sundr.model.utils.Types.DOUBLE_REF; /** @@ -117,6 +112,11 @@ public static String getSchemaTypeFor(TypeRef typeRef) { * @return The schema. */ protected T internalFrom(TypeDef definition, String... ignore) { + return internalFromImpl(definition, new HashSet<>(), ignore); + } + + private T internalFromImpl(TypeDef definition, Set visited, String... ignore) { + visited.add(definition.getFullyQualifiedName()); final B builder = newBuilder(); Set ignores = ignore.length > 0 ? new LinkedHashSet<>(Arrays.asList(ignore)) : Collections @@ -140,7 +140,7 @@ protected T internalFrom(TypeDef definition, String... ignore) { if (facade.required) { required.add(name); } - final T schema = internalFrom(name, possiblyRenamedProperty.getTypeRef()); + final T schema = internalFromImpl(name, possiblyRenamedProperty.getTypeRef(), visited); // if we got a description from the field or an accessor, use it final String description = facade.description; final T possiblyUpdatedSchema; @@ -361,12 +361,16 @@ private String extractUpdatedNameFromJacksonPropertyIfPresent(Property property) * @return the structural schema associated with the specified property */ public T internalFrom(String name, TypeRef typeRef) { + return internalFromImpl(name, typeRef, new HashSet<>()); + } + + private T internalFromImpl(String name, TypeRef typeRef, Set visited) { // Note that ordering of the checks here is meaningful: we need to check for complex types last // in case some "complex" types are handled specifically if (typeRef.getDimensions() > 0 || io.sundr.model.utils.Collections.isCollection(typeRef)) { // Handle Collections & Arrays final TypeRef collectionType = TypeAs.combine(TypeAs.UNWRAP_ARRAY_OF, TypeAs.UNWRAP_COLLECTION_OF) .apply(typeRef); - final T schema = internalFrom(name, collectionType); + final T schema = internalFromImpl(name, collectionType, visited); return arrayLikeProperty(schema); } else if (io.sundr.model.utils.Collections.IS_MAP.apply(typeRef)) { // Handle Maps final TypeRef keyType = TypeAs.UNWRAP_MAP_KEY_OF.apply(typeRef); @@ -390,7 +394,7 @@ public T internalFrom(String name, TypeRef typeRef) { } return mapLikeProperty(); } else if (io.sundr.model.utils.Optionals.isOptional(typeRef)) { // Handle Optionals - return internalFrom(name, TypeAs.UNWRAP_OPTIONAL_OF.apply(typeRef)); + return internalFromImpl(name, TypeAs.UNWRAP_OPTIONAL_OF.apply(typeRef), visited); } else { final String typeName = COMMON_MAPPINGS.get(typeRef); if (typeName != null) { // we have a type that we handle specifically @@ -413,7 +417,10 @@ public T internalFrom(String name, TypeRef typeRef) { .toArray(JsonNode[]::new); return enumProperty(enumValues); } else { - return internalFrom(def); + if (!def.getFullyQualifiedName().startsWith("java") && visited.contains(def.getFullyQualifiedName())) { + throw new IllegalArgumentException("Found a cyclic reference involving " + def.getFullyQualifiedName()); + } + return internalFromImpl(def, visited); } } diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1/JsonSchema.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1/JsonSchema.java index 8839c60a3ab..b1cf296c90e 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1/JsonSchema.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1/JsonSchema.java @@ -22,6 +22,7 @@ import io.sundr.model.Property; import io.sundr.model.TypeDef; import io.sundr.model.TypeRef; + import java.util.List; public class JsonSchema extends AbstractJsonSchema { diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1beta1/JsonSchema.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1beta1/JsonSchema.java index cf27eff054b..a48cbf9adac 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1beta1/JsonSchema.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1beta1/JsonSchema.java @@ -22,6 +22,7 @@ import io.sundr.model.Property; import io.sundr.model.TypeDef; import io.sundr.model.TypeRef; + import java.util.List; public class JsonSchema extends AbstractJsonSchema { diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/Cyclic.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/Cyclic.java new file mode 100644 index 00000000000..481712742af --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/Cyclic.java @@ -0,0 +1,27 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.cyclic; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.fabric8.io") +@Version("v1alpha1") +public class Cyclic extends CustomResource implements Namespaced { + +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicSpec.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicSpec.java new file mode 100644 index 00000000000..764a885f83b --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicSpec.java @@ -0,0 +1,20 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.cyclic; + +public class CyclicSpec { + private Ref ref; +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicStatus.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicStatus.java new file mode 100644 index 00000000000..255292e2e8b --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicStatus.java @@ -0,0 +1,28 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.cyclic; + +public class CyclicStatus { + private String message; + + public String getMessage() { + return message; + } + + public void setMessage(String message) { + this.message = message; + } +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/Ref.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/Ref.java new file mode 100644 index 00000000000..1dd1b2a03b4 --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/Ref.java @@ -0,0 +1,22 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.cyclic; + +public class Ref { + + private Ref ref; + +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java b/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java index edb9cd16bbd..b976a2239f1 100644 --- a/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java +++ b/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java @@ -18,6 +18,7 @@ import io.fabric8.crd.example.basic.Basic; import io.fabric8.crd.example.basic.BasicSpec; import io.fabric8.crd.example.basic.BasicStatus; +import io.fabric8.crd.example.cyclic.Cyclic; import io.fabric8.crd.example.inherited.*; import io.fabric8.crd.example.joke.Joke; import io.fabric8.crd.example.joke.JokeRequest; @@ -197,6 +198,19 @@ void shouldProperlyGenerateMultipleVersionsOfCRDs() { assertEquals(0, generator.generate()); } + @Test void generatingACycleShouldFail() { + final CRDGenerator generator = new CRDGenerator() + .customResourceClasses(Cyclic.class) + .forCRDVersions("v1", "v1beta1") + .withOutput(output); + + assertThrows( + IllegalArgumentException.class, + () -> generator.detailedGenerate(), + "Found a cyclic reference involving io.fabric8.crd.example.cyclic.Ref" + ); + } + @FunctionalInterface private interface CRTest { diff --git a/kubernetes-tests/src/test/java/io/fabric8/crd/generator/zookeeper/v1alpha1/Zookeeper.java b/kubernetes-tests/src/test/java/io/fabric8/crd/generator/zookeeper/v1alpha1/Zookeeper.java index 265bf98ef50..315961ae0d1 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/crd/generator/zookeeper/v1alpha1/Zookeeper.java +++ b/kubernetes-tests/src/test/java/io/fabric8/crd/generator/zookeeper/v1alpha1/Zookeeper.java @@ -24,7 +24,6 @@ @Version(value="v1alpha1", storage=false) @Group("io.zookeeper") public class Zookeeper extends CustomResource implements Namespaced { - private ZookeeperSpec spec; - private ZookeeperStatus status; + } From 6b4c15705896668c25feb00e1cc449a5e9bcfc02 Mon Sep 17 00:00:00 2001 From: andreaTP Date: Thu, 16 Dec 2021 13:41:29 +0000 Subject: [PATCH 2/6] minor --- .../src/main/java/io/fabric8/crd/generator/v1/JsonSchema.java | 1 - .../main/java/io/fabric8/crd/generator/v1beta1/JsonSchema.java | 1 - 2 files changed, 2 deletions(-) diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1/JsonSchema.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1/JsonSchema.java index b1cf296c90e..8839c60a3ab 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1/JsonSchema.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1/JsonSchema.java @@ -22,7 +22,6 @@ import io.sundr.model.Property; import io.sundr.model.TypeDef; import io.sundr.model.TypeRef; - import java.util.List; public class JsonSchema extends AbstractJsonSchema { diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1beta1/JsonSchema.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1beta1/JsonSchema.java index a48cbf9adac..cf27eff054b 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1beta1/JsonSchema.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/v1beta1/JsonSchema.java @@ -22,7 +22,6 @@ import io.sundr.model.Property; import io.sundr.model.TypeDef; import io.sundr.model.TypeRef; - import java.util.List; public class JsonSchema extends AbstractJsonSchema { From ab4b62a5059a198f25a195193f2f10a2bdff50ad Mon Sep 17 00:00:00 2001 From: andreaTP Date: Mon, 20 Dec 2021 18:51:55 +0000 Subject: [PATCH 3/6] Add more test and edge cases --- .../crd/generator/AbstractJsonSchema.java | 7 +++-- .../crd/example/nocyclic/NoCyclic.java | 27 +++++++++++++++++ .../crd/example/nocyclic/NoCyclicSpec.java | 21 ++++++++++++++ .../crd/example/nocyclic/NoCyclicStatus.java | 29 +++++++++++++++++++ .../io/fabric8/crd/example/nocyclic/Ref.java | 22 ++++++++++++++ .../crd/generator/CRDGeneratorTest.java | 12 +++++++- 6 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclic.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicSpec.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicStatus.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/Ref.java diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java index 3534b4d0355..283006f9cba 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java @@ -116,7 +116,6 @@ protected T internalFrom(TypeDef definition, String... ignore) { } private T internalFromImpl(TypeDef definition, Set visited, String... ignore) { - visited.add(definition.getFullyQualifiedName()); final B builder = newBuilder(); Set ignores = ignore.length > 0 ? new LinkedHashSet<>(Arrays.asList(ignore)) : Collections @@ -417,9 +416,11 @@ private T internalFromImpl(String name, TypeRef typeRef, Set visited) { .toArray(JsonNode[]::new); return enumProperty(enumValues); } else { - if (!def.getFullyQualifiedName().startsWith("java") && visited.contains(def.getFullyQualifiedName())) { - throw new IllegalArgumentException("Found a cyclic reference involving " + def.getFullyQualifiedName()); + String visitedName = def.getFullyQualifiedName() + "-" + name; + if (!def.getFullyQualifiedName().startsWith("java") && visited.contains(visitedName)) { + throw new IllegalArgumentException("Found a cyclic reference involving the field " + name + " of type " + def.getFullyQualifiedName()); } + visited.add(visitedName); return internalFromImpl(def, visited); } diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclic.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclic.java new file mode 100644 index 00000000000..16f7ffba9c2 --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclic.java @@ -0,0 +1,27 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.nocyclic; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.fabric8.io") +@Version("v1alpha1") +public class NoCyclic extends CustomResource implements Namespaced { + +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicSpec.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicSpec.java new file mode 100644 index 00000000000..4d65bec923f --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicSpec.java @@ -0,0 +1,21 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.nocyclic; + +public class NoCyclicSpec { + private Ref ref1; + private Ref ref2; +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicStatus.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicStatus.java new file mode 100644 index 00000000000..fa68c249813 --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicStatus.java @@ -0,0 +1,29 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.nocyclic; + +public class NoCyclicStatus { + private String message; + // private Ref ref1; + + public String getMessage() { + return message; + } + + public void setMessage(String message) { + this.message = message; + } +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/Ref.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/Ref.java new file mode 100644 index 00000000000..908ceb90a1b --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/Ref.java @@ -0,0 +1,22 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.nocyclic; + +public class Ref { + + private int ref; + +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java b/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java index b976a2239f1..6edd8d9e75c 100644 --- a/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java +++ b/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java @@ -26,6 +26,7 @@ import io.fabric8.crd.example.joke.JokeRequestStatus; import io.fabric8.crd.example.multiple.v1.Multiple; import io.fabric8.crd.example.multiple.v1.MultipleSpec; +import io.fabric8.crd.example.nocyclic.NoCyclic; import io.fabric8.crd.example.simplest.Simplest; import io.fabric8.crd.example.simplest.SimplestSpec; import io.fabric8.crd.example.simplest.SimplestStatus; @@ -207,10 +208,19 @@ void shouldProperlyGenerateMultipleVersionsOfCRDs() { assertThrows( IllegalArgumentException.class, () -> generator.detailedGenerate(), - "Found a cyclic reference involving io.fabric8.crd.example.cyclic.Ref" + "An IllegalArgument Exception hasn't been thrown when generating a CRD with cyclic references" ); } + @Test void notGeneratingACycleButReusingShouldSucceed() { + final CRDGenerator generator = new CRDGenerator() + .customResourceClasses(NoCyclic.class) + .forCRDVersions("v1", "v1beta1") + .withOutput(output); + + CRDGenerationInfo info = generator.detailedGenerate(); + assertEquals(2, info.numberOfGeneratedCRDs()); + } @FunctionalInterface private interface CRTest { From 46f940fae65ee5ed8b4ddff0447002b05770a5e0 Mon Sep 17 00:00:00 2001 From: andreaTP Date: Tue, 21 Dec 2021 18:27:58 +0000 Subject: [PATCH 4/6] more edge cases --- .../crd/generator/AbstractJsonSchema.java | 21 +- .../crd/example/cyclic/CyclicStatus.java | 8 - .../crd/example/nocyclic/NoCyclicStatus.java | 10 +- .../io/fabric8/crd/example/nocyclic/Ref.java | 4 + .../crd/generator/CRDGeneratorTest.java | 338 +++++++++--------- 5 files changed, 190 insertions(+), 191 deletions(-) diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java index 283006f9cba..5f77ee775bb 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java @@ -416,12 +416,20 @@ private T internalFromImpl(String name, TypeRef typeRef, Set visited) { .toArray(JsonNode[]::new); return enumProperty(enumValues); } else { - String visitedName = def.getFullyQualifiedName() + "-" + name; - if (!def.getFullyQualifiedName().startsWith("java") && visited.contains(visitedName)) { - throw new IllegalArgumentException("Found a cyclic reference involving the field " + name + " of type " + def.getFullyQualifiedName()); + if (!resolving) { + visited.clear(); + resolving = true; + } else { + String visitedName = name + ":" + classRef; + if (!def.getFullyQualifiedName().startsWith("java") && visited.contains(visitedName)) { + throw new IllegalArgumentException("Found a cyclic reference involving the field " + name + " of type " + classRef.getFullyQualifiedName()); + } + visited.add(visitedName); } - visited.add(visitedName); - return internalFromImpl(def, visited); + + T res = internalFromImpl(def, visited); + resolving = false; + return res; } } @@ -430,6 +438,9 @@ private T internalFromImpl(String name, TypeRef typeRef, Set visited) { } } + // Flag to detect cycles + private boolean resolving = false; + /** * Builds the schema for specifically handled property types (e.g. intOrString properties) * diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicStatus.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicStatus.java index 255292e2e8b..b857cd44dd5 100644 --- a/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicStatus.java +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicStatus.java @@ -17,12 +17,4 @@ public class CyclicStatus { private String message; - - public String getMessage() { - return message; - } - - public void setMessage(String message) { - this.message = message; - } } diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicStatus.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicStatus.java index fa68c249813..21d1ae22fb2 100644 --- a/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicStatus.java +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/NoCyclicStatus.java @@ -17,13 +17,5 @@ public class NoCyclicStatus { private String message; - // private Ref ref1; - - public String getMessage() { - return message; - } - - public void setMessage(String message) { - this.message = message; - } + private Ref ref1; } diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/Ref.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/Ref.java index 908ceb90a1b..76d80b25d95 100644 --- a/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/Ref.java +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/nocyclic/Ref.java @@ -19,4 +19,8 @@ public class Ref { private int ref; + protected Inner inner; + + public static class Inner { } + } diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java b/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java index 6edd8d9e75c..dba281f7b3f 100644 --- a/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java +++ b/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java @@ -56,149 +56,149 @@ class CRDGeneratorTest { private final TestCRDOutput output = new TestCRDOutput(); - @Test - void choosingCRDVersionsShouldWork() { - CRDGenerator generator = new CRDGenerator(); - assertTrue(generator.getHandlers().isEmpty()); - - generator.forCRDVersions(); - assertTrue(generator.getHandlers().isEmpty()); - - generator.forCRDVersions((List) null); - assertTrue(generator.getHandlers().isEmpty()); - - generator.forCRDVersions((String[]) null); - assertTrue(generator.getHandlers().isEmpty()); - - generator.forCRDVersions("v2"); - assertTrue(generator.getHandlers().isEmpty()); - - String version = "v1"; - generator.forCRDVersions(version); - Map handlers = generator.getHandlers(); - assertEquals(1, handlers.size()); - assertTrue(handlers.containsKey(version)); - - generator.forCRDVersions(version, "v1beta1", version, "v3", null); - handlers = generator.getHandlers(); - assertEquals(2, handlers.size()); - assertTrue(handlers.containsKey(version)); - assertTrue(handlers.containsKey("v1beta1")); - } - - @Test - void addingCustomResourceInfosShouldWork() { - CRDGenerator generator = new CRDGenerator(); - assertTrue(generator.getCustomResourceInfos().isEmpty()); - - generator.customResourceClasses(); - assertTrue(generator.getCustomResourceInfos().isEmpty()); - - generator.customResources(); - assertTrue(generator.getCustomResourceInfos().isEmpty()); - - generator.customResources(null); - assertTrue(generator.getCustomResourceInfos().isEmpty()); - - generator.customResources(null, null); - assertTrue(generator.getCustomResourceInfos().isEmpty()); - - generator.customResourceClasses(Simplest.class); - assertEquals(1, generator.getCustomResourceInfos().size()); - assertTrue(generator.getCustomResourceInfos().stream().allMatch(cri -> cri.crClassName().equals(Simplest.class.getName()))); - - generator.customResourceClasses(Child.class); - assertEquals(2, generator.getCustomResourceInfos().size()); - CustomResourceInfo simplest = CustomResourceInfo.fromClass(Simplest.class); - assertTrue(generator.getCustomResourceInfos().contains(simplest)); - CustomResourceInfo child = CustomResourceInfo.fromClass(Child.class); - assertTrue(generator.getCustomResourceInfos().contains(child)); - - generator.customResources(CustomResourceInfo.fromClass(Child.class)); - assertEquals(2, generator.getCustomResourceInfos().size()); - - CustomResourceInfo joke = CustomResourceInfo.fromClass(Joke.class); - CustomResourceInfo jr = CustomResourceInfo.fromClass(JokeRequest.class); - generator.customResources(joke, jr); - Set infos = generator.getCustomResourceInfos(); - assertEquals(4, infos.size()); - assertTrue(infos.contains(simplest)); - assertTrue(infos.contains(child)); - assertTrue(infos.contains(joke)); - assertTrue(infos.contains(jr)); - } - - @Test - void shouldProperlyRecordNumberOfGeneratedCRDs() { - CRDGenerator generator = new CRDGenerator(); - assertEquals(0, generator.generate()); - assertEquals(0, generator.detailedGenerate().numberOfGeneratedCRDs()); - - final CRDGenerationInfo info = generator - .customResourceClasses(Simplest.class, Child.class, Joke.class, JokeRequest.class) - .forCRDVersions("v1", "v1beta1") - .withOutput(output).detailedGenerate(); - - assertEquals(4 * 2, info.numberOfGeneratedCRDs()); - final Map> details = info.getCRDDetailsPerNameAndVersion(); - assertEquals(4, details.size()); - assertTrue(details.containsKey(CustomResource.getCRDName(Simplest.class))); - assertTrue(details.containsKey(CustomResource.getCRDName(Child.class))); - assertTrue(details.containsKey(CustomResource.getCRDName(Joke.class))); - final String crdName = CustomResource.getCRDName(JokeRequest.class); - assertTrue(details.containsKey(crdName)); - final Map jokeRequestInfos = info.getCRDInfos(crdName); - assertEquals(2, jokeRequestInfos.size()); - assertTrue(jokeRequestInfos.containsKey("v1")); - assertTrue(jokeRequestInfos.containsKey("v1beta1")); - } - - @Test - void shouldProperlyGenerateMultipleVersionsOfCRDs() { - CRDGenerator generator = new CRDGenerator(); - final String specVersion = "v1beta1"; - final CRDGenerationInfo info = generator - .customResourceClasses(Multiple.class, io.fabric8.crd.example.multiple.v2.Multiple.class) - .forCRDVersions(specVersion) - .withOutput(output).detailedGenerate(); - - assertEquals(1, info.numberOfGeneratedCRDs()); - final Map> details = info.getCRDDetailsPerNameAndVersion(); - assertEquals(1, details.size()); - // check multiple versions for same CR - final String crdName = CustomResource.getCRDName(Multiple.class); - assertTrue(details.containsKey(crdName)); - final Map infos = info.getCRDInfos(crdName); - assertEquals(1, infos.size()); - assertTrue(infos.containsKey(specVersion)); - - final String outputName = CRDGenerator.getOutputName(crdName, specVersion); - CustomResourceDefinition definition = output.definition(outputName); - assertNotNull(definition); - assertEquals("apiextensions.k8s.io/" + specVersion, definition.getApiVersion()); - - CustomResourceDefinitionSpec spec = definition.getSpec(); - final List versions = spec.getVersions(); - assertEquals(2, versions.size()); - assertTrue(versions.stream().filter(v -> v.getName().equals("v1")).count() == 1); - assertTrue(versions.stream().filter(v -> v.getName().equals("v2")).count() == 1); - - - Class[] mustContainTraversedClasses = {Multiple.class, MultipleSpec.class, io.fabric8.crd.example.multiple.v2.Multiple.class, io.fabric8.crd.example.multiple.v2.MultipleSpec.class}; - final Set dependentClassNames = infos.get(specVersion).getDependentClassNames(); - Arrays.stream(mustContainTraversedClasses).map(Class::getCanonicalName).forEach(c -> assertTrue(dependentClassNames.contains(c), "should contain " + c)); - } - - @Test void notDefiningOutputShouldNotGenerateAnything() { - CRDGenerator generator = new CRDGenerator(); - assertEquals(0, generator.generate()); - - CustomResourceInfo joke = CustomResourceInfo.fromClass(Joke.class); - CustomResourceInfo jr = CustomResourceInfo.fromClass(JokeRequest.class); - generator.customResources(joke, jr); - assertEquals(0, generator.generate()); - } - +// @Test +// void choosingCRDVersionsShouldWork() { +// CRDGenerator generator = new CRDGenerator(); +// assertTrue(generator.getHandlers().isEmpty()); +// +// generator.forCRDVersions(); +// assertTrue(generator.getHandlers().isEmpty()); +// +// generator.forCRDVersions((List) null); +// assertTrue(generator.getHandlers().isEmpty()); +// +// generator.forCRDVersions((String[]) null); +// assertTrue(generator.getHandlers().isEmpty()); +// +// generator.forCRDVersions("v2"); +// assertTrue(generator.getHandlers().isEmpty()); +// +// String version = "v1"; +// generator.forCRDVersions(version); +// Map handlers = generator.getHandlers(); +// assertEquals(1, handlers.size()); +// assertTrue(handlers.containsKey(version)); +// +// generator.forCRDVersions(version, "v1beta1", version, "v3", null); +// handlers = generator.getHandlers(); +// assertEquals(2, handlers.size()); +// assertTrue(handlers.containsKey(version)); +// assertTrue(handlers.containsKey("v1beta1")); +// } +// +// @Test +// void addingCustomResourceInfosShouldWork() { +// CRDGenerator generator = new CRDGenerator(); +// assertTrue(generator.getCustomResourceInfos().isEmpty()); +// +// generator.customResourceClasses(); +// assertTrue(generator.getCustomResourceInfos().isEmpty()); +// +// generator.customResources(); +// assertTrue(generator.getCustomResourceInfos().isEmpty()); +// +// generator.customResources(null); +// assertTrue(generator.getCustomResourceInfos().isEmpty()); +// +// generator.customResources(null, null); +// assertTrue(generator.getCustomResourceInfos().isEmpty()); +// +// generator.customResourceClasses(Simplest.class); +// assertEquals(1, generator.getCustomResourceInfos().size()); +// assertTrue(generator.getCustomResourceInfos().stream().allMatch(cri -> cri.crClassName().equals(Simplest.class.getName()))); +// +// generator.customResourceClasses(Child.class); +// assertEquals(2, generator.getCustomResourceInfos().size()); +// CustomResourceInfo simplest = CustomResourceInfo.fromClass(Simplest.class); +// assertTrue(generator.getCustomResourceInfos().contains(simplest)); +// CustomResourceInfo child = CustomResourceInfo.fromClass(Child.class); +// assertTrue(generator.getCustomResourceInfos().contains(child)); +// +// generator.customResources(CustomResourceInfo.fromClass(Child.class)); +// assertEquals(2, generator.getCustomResourceInfos().size()); +// +// CustomResourceInfo joke = CustomResourceInfo.fromClass(Joke.class); +// CustomResourceInfo jr = CustomResourceInfo.fromClass(JokeRequest.class); +// generator.customResources(joke, jr); +// Set infos = generator.getCustomResourceInfos(); +// assertEquals(4, infos.size()); +// assertTrue(infos.contains(simplest)); +// assertTrue(infos.contains(child)); +// assertTrue(infos.contains(joke)); +// assertTrue(infos.contains(jr)); +// } +// +// @Test +// void shouldProperlyRecordNumberOfGeneratedCRDs() { +// CRDGenerator generator = new CRDGenerator(); +// assertEquals(0, generator.generate()); +// assertEquals(0, generator.detailedGenerate().numberOfGeneratedCRDs()); +// +// final CRDGenerationInfo info = generator +// .customResourceClasses(Simplest.class, Child.class, Joke.class, JokeRequest.class) +// .forCRDVersions("v1", "v1beta1") +// .withOutput(output).detailedGenerate(); +// +// assertEquals(4 * 2, info.numberOfGeneratedCRDs()); +// final Map> details = info.getCRDDetailsPerNameAndVersion(); +// assertEquals(4, details.size()); +// assertTrue(details.containsKey(CustomResource.getCRDName(Simplest.class))); +// assertTrue(details.containsKey(CustomResource.getCRDName(Child.class))); +// assertTrue(details.containsKey(CustomResource.getCRDName(Joke.class))); +// final String crdName = CustomResource.getCRDName(JokeRequest.class); +// assertTrue(details.containsKey(crdName)); +// final Map jokeRequestInfos = info.getCRDInfos(crdName); +// assertEquals(2, jokeRequestInfos.size()); +// assertTrue(jokeRequestInfos.containsKey("v1")); +// assertTrue(jokeRequestInfos.containsKey("v1beta1")); +// } +// +// @Test +// void shouldProperlyGenerateMultipleVersionsOfCRDs() { +// CRDGenerator generator = new CRDGenerator(); +// final String specVersion = "v1beta1"; +// final CRDGenerationInfo info = generator +// .customResourceClasses(Multiple.class, io.fabric8.crd.example.multiple.v2.Multiple.class) +// .forCRDVersions(specVersion) +// .withOutput(output).detailedGenerate(); +// +// assertEquals(1, info.numberOfGeneratedCRDs()); +// final Map> details = info.getCRDDetailsPerNameAndVersion(); +// assertEquals(1, details.size()); +// // check multiple versions for same CR +// final String crdName = CustomResource.getCRDName(Multiple.class); +// assertTrue(details.containsKey(crdName)); +// final Map infos = info.getCRDInfos(crdName); +// assertEquals(1, infos.size()); +// assertTrue(infos.containsKey(specVersion)); +// +// final String outputName = CRDGenerator.getOutputName(crdName, specVersion); +// CustomResourceDefinition definition = output.definition(outputName); +// assertNotNull(definition); +// assertEquals("apiextensions.k8s.io/" + specVersion, definition.getApiVersion()); +// +// CustomResourceDefinitionSpec spec = definition.getSpec(); +// final List versions = spec.getVersions(); +// assertEquals(2, versions.size()); +// assertTrue(versions.stream().filter(v -> v.getName().equals("v1")).count() == 1); +// assertTrue(versions.stream().filter(v -> v.getName().equals("v2")).count() == 1); +// +// +// Class[] mustContainTraversedClasses = {Multiple.class, MultipleSpec.class, io.fabric8.crd.example.multiple.v2.Multiple.class, io.fabric8.crd.example.multiple.v2.MultipleSpec.class}; +// final Set dependentClassNames = infos.get(specVersion).getDependentClassNames(); +// Arrays.stream(mustContainTraversedClasses).map(Class::getCanonicalName).forEach(c -> assertTrue(dependentClassNames.contains(c), "should contain " + c)); +// } +// +// @Test void notDefiningOutputShouldNotGenerateAnything() { +// CRDGenerator generator = new CRDGenerator(); +// assertEquals(0, generator.generate()); +// +// CustomResourceInfo joke = CustomResourceInfo.fromClass(Joke.class); +// CustomResourceInfo jr = CustomResourceInfo.fromClass(JokeRequest.class); +// generator.customResources(joke, jr); +// assertEquals(0, generator.generate()); +// } +// @Test void generatingACycleShouldFail() { final CRDGenerator generator = new CRDGenerator() .customResourceClasses(Cyclic.class) @@ -212,7 +212,7 @@ void shouldProperlyGenerateMultipleVersionsOfCRDs() { ); } - @Test void notGeneratingACycleButReusingShouldSucceed() { + @Test void notGeneratingACycleShouldSucceed() { final CRDGenerator generator = new CRDGenerator() .customResourceClasses(NoCyclic.class) .forCRDVersions("v1", "v1beta1") @@ -237,31 +237,31 @@ private void outputCRDIfFailed(Class> customResou throw e; } } - @Test - void simplestCRDShouldWork() { - outputCRDIfFailed(Simplest.class, (customResource) -> { - final CustomResourceDefinitionVersion version = checkCRD(customResource, "Simplest", "simplests", - Scope.CLUSTER, SimplestSpec.class, SimplestStatus.class); - assertNotNull(version.getSubresources()); - }); - - } - - @Test - void inheritedCRDShouldWork() { - outputCRDIfFailed(Child.class, (customResource) -> { - final CustomResourceDefinitionVersion version = checkCRD(customResource, "Child", "children", - Scope.NAMESPACED, ChildSpec.class, ChildStatus.class, BaseSpec.class, BaseStatus.class); - assertNotNull(version.getSubresources()); - final Map specProps = version.getSchema().getOpenAPIV3Schema() - .getProperties().get("spec").getProperties(); - assertEquals(4, specProps.size()); - assertEquals("integer", specProps.get("baseInt").getType()); - checkMapProp(specProps, "unsupported"); - checkMapProp(specProps, "unsupported2"); - checkMapProp(specProps, "supported"); - }); - } +// @Test +// void simplestCRDShouldWork() { +// outputCRDIfFailed(Simplest.class, (customResource) -> { +// final CustomResourceDefinitionVersion version = checkCRD(customResource, "Simplest", "simplests", +// Scope.CLUSTER, SimplestSpec.class, SimplestStatus.class); +// assertNotNull(version.getSubresources()); +// }); +// +// } +// +// @Test +// void inheritedCRDShouldWork() { +// outputCRDIfFailed(Child.class, (customResource) -> { +// final CustomResourceDefinitionVersion version = checkCRD(customResource, "Child", "children", +// Scope.NAMESPACED, ChildSpec.class, ChildStatus.class, BaseSpec.class, BaseStatus.class); +// assertNotNull(version.getSubresources()); +// final Map specProps = version.getSchema().getOpenAPIV3Schema() +// .getProperties().get("spec").getProperties(); +// assertEquals(4, specProps.size()); +// assertEquals("integer", specProps.get("baseInt").getType()); +// checkMapProp(specProps, "unsupported"); +// checkMapProp(specProps, "unsupported2"); +// checkMapProp(specProps, "supported"); +// }); +// } private void checkMapProp(Map specProps, String name) { final JSONSchemaProps props = specProps.get(name); From 6cfd03ad054f7388891437a53f288f3d279ab062 Mon Sep 17 00:00:00 2001 From: andreaTP Date: Tue, 21 Dec 2021 18:34:18 +0000 Subject: [PATCH 5/6] less complexity --- .../crd/generator/AbstractJsonSchema.java | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java index 5f77ee775bb..b1463c32b2c 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/AbstractJsonSchema.java @@ -416,20 +416,7 @@ private T internalFromImpl(String name, TypeRef typeRef, Set visited) { .toArray(JsonNode[]::new); return enumProperty(enumValues); } else { - if (!resolving) { - visited.clear(); - resolving = true; - } else { - String visitedName = name + ":" + classRef; - if (!def.getFullyQualifiedName().startsWith("java") && visited.contains(visitedName)) { - throw new IllegalArgumentException("Found a cyclic reference involving the field " + name + " of type " + classRef.getFullyQualifiedName()); - } - visited.add(visitedName); - } - - T res = internalFromImpl(def, visited); - resolving = false; - return res; + return resolveNestedClass(name, def, visited); } } @@ -441,6 +428,23 @@ private T internalFromImpl(String name, TypeRef typeRef, Set visited) { // Flag to detect cycles private boolean resolving = false; + private T resolveNestedClass(String name, TypeDef def, Set visited) { + if (!resolving) { + visited.clear(); + resolving = true; + } else { + String visitedName = name + ":" + def.getFullyQualifiedName(); + if (!def.getFullyQualifiedName().startsWith("java") && visited.contains(visitedName)) { + throw new IllegalArgumentException("Found a cyclic reference involving the field " + name + " of type " + def.getFullyQualifiedName()); + } + visited.add(visitedName); + } + + T res = internalFromImpl(def, visited); + resolving = false; + return res; + } + /** * Builds the schema for specifically handled property types (e.g. intOrString properties) * From d7cd8466930dd02dae60810cb54f8d1188a48417 Mon Sep 17 00:00:00 2001 From: andreaTP Date: Tue, 21 Dec 2021 18:36:50 +0000 Subject: [PATCH 6/6] more --- .../crd/generator/CRDGeneratorTest.java | 336 +++++++++--------- .../zookeeper/v1alpha1/Zookeeper.java | 3 +- 2 files changed, 170 insertions(+), 169 deletions(-) diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java b/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java index dba281f7b3f..a7258a166b5 100644 --- a/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java +++ b/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java @@ -56,149 +56,149 @@ class CRDGeneratorTest { private final TestCRDOutput output = new TestCRDOutput(); -// @Test -// void choosingCRDVersionsShouldWork() { -// CRDGenerator generator = new CRDGenerator(); -// assertTrue(generator.getHandlers().isEmpty()); -// -// generator.forCRDVersions(); -// assertTrue(generator.getHandlers().isEmpty()); -// -// generator.forCRDVersions((List) null); -// assertTrue(generator.getHandlers().isEmpty()); -// -// generator.forCRDVersions((String[]) null); -// assertTrue(generator.getHandlers().isEmpty()); -// -// generator.forCRDVersions("v2"); -// assertTrue(generator.getHandlers().isEmpty()); -// -// String version = "v1"; -// generator.forCRDVersions(version); -// Map handlers = generator.getHandlers(); -// assertEquals(1, handlers.size()); -// assertTrue(handlers.containsKey(version)); -// -// generator.forCRDVersions(version, "v1beta1", version, "v3", null); -// handlers = generator.getHandlers(); -// assertEquals(2, handlers.size()); -// assertTrue(handlers.containsKey(version)); -// assertTrue(handlers.containsKey("v1beta1")); -// } -// -// @Test -// void addingCustomResourceInfosShouldWork() { -// CRDGenerator generator = new CRDGenerator(); -// assertTrue(generator.getCustomResourceInfos().isEmpty()); -// -// generator.customResourceClasses(); -// assertTrue(generator.getCustomResourceInfos().isEmpty()); -// -// generator.customResources(); -// assertTrue(generator.getCustomResourceInfos().isEmpty()); -// -// generator.customResources(null); -// assertTrue(generator.getCustomResourceInfos().isEmpty()); -// -// generator.customResources(null, null); -// assertTrue(generator.getCustomResourceInfos().isEmpty()); -// -// generator.customResourceClasses(Simplest.class); -// assertEquals(1, generator.getCustomResourceInfos().size()); -// assertTrue(generator.getCustomResourceInfos().stream().allMatch(cri -> cri.crClassName().equals(Simplest.class.getName()))); -// -// generator.customResourceClasses(Child.class); -// assertEquals(2, generator.getCustomResourceInfos().size()); -// CustomResourceInfo simplest = CustomResourceInfo.fromClass(Simplest.class); -// assertTrue(generator.getCustomResourceInfos().contains(simplest)); -// CustomResourceInfo child = CustomResourceInfo.fromClass(Child.class); -// assertTrue(generator.getCustomResourceInfos().contains(child)); -// -// generator.customResources(CustomResourceInfo.fromClass(Child.class)); -// assertEquals(2, generator.getCustomResourceInfos().size()); -// -// CustomResourceInfo joke = CustomResourceInfo.fromClass(Joke.class); -// CustomResourceInfo jr = CustomResourceInfo.fromClass(JokeRequest.class); -// generator.customResources(joke, jr); -// Set infos = generator.getCustomResourceInfos(); -// assertEquals(4, infos.size()); -// assertTrue(infos.contains(simplest)); -// assertTrue(infos.contains(child)); -// assertTrue(infos.contains(joke)); -// assertTrue(infos.contains(jr)); -// } -// -// @Test -// void shouldProperlyRecordNumberOfGeneratedCRDs() { -// CRDGenerator generator = new CRDGenerator(); -// assertEquals(0, generator.generate()); -// assertEquals(0, generator.detailedGenerate().numberOfGeneratedCRDs()); -// -// final CRDGenerationInfo info = generator -// .customResourceClasses(Simplest.class, Child.class, Joke.class, JokeRequest.class) -// .forCRDVersions("v1", "v1beta1") -// .withOutput(output).detailedGenerate(); -// -// assertEquals(4 * 2, info.numberOfGeneratedCRDs()); -// final Map> details = info.getCRDDetailsPerNameAndVersion(); -// assertEquals(4, details.size()); -// assertTrue(details.containsKey(CustomResource.getCRDName(Simplest.class))); -// assertTrue(details.containsKey(CustomResource.getCRDName(Child.class))); -// assertTrue(details.containsKey(CustomResource.getCRDName(Joke.class))); -// final String crdName = CustomResource.getCRDName(JokeRequest.class); -// assertTrue(details.containsKey(crdName)); -// final Map jokeRequestInfos = info.getCRDInfos(crdName); -// assertEquals(2, jokeRequestInfos.size()); -// assertTrue(jokeRequestInfos.containsKey("v1")); -// assertTrue(jokeRequestInfos.containsKey("v1beta1")); -// } -// -// @Test -// void shouldProperlyGenerateMultipleVersionsOfCRDs() { -// CRDGenerator generator = new CRDGenerator(); -// final String specVersion = "v1beta1"; -// final CRDGenerationInfo info = generator -// .customResourceClasses(Multiple.class, io.fabric8.crd.example.multiple.v2.Multiple.class) -// .forCRDVersions(specVersion) -// .withOutput(output).detailedGenerate(); -// -// assertEquals(1, info.numberOfGeneratedCRDs()); -// final Map> details = info.getCRDDetailsPerNameAndVersion(); -// assertEquals(1, details.size()); -// // check multiple versions for same CR -// final String crdName = CustomResource.getCRDName(Multiple.class); -// assertTrue(details.containsKey(crdName)); -// final Map infos = info.getCRDInfos(crdName); -// assertEquals(1, infos.size()); -// assertTrue(infos.containsKey(specVersion)); -// -// final String outputName = CRDGenerator.getOutputName(crdName, specVersion); -// CustomResourceDefinition definition = output.definition(outputName); -// assertNotNull(definition); -// assertEquals("apiextensions.k8s.io/" + specVersion, definition.getApiVersion()); -// -// CustomResourceDefinitionSpec spec = definition.getSpec(); -// final List versions = spec.getVersions(); -// assertEquals(2, versions.size()); -// assertTrue(versions.stream().filter(v -> v.getName().equals("v1")).count() == 1); -// assertTrue(versions.stream().filter(v -> v.getName().equals("v2")).count() == 1); -// -// -// Class[] mustContainTraversedClasses = {Multiple.class, MultipleSpec.class, io.fabric8.crd.example.multiple.v2.Multiple.class, io.fabric8.crd.example.multiple.v2.MultipleSpec.class}; -// final Set dependentClassNames = infos.get(specVersion).getDependentClassNames(); -// Arrays.stream(mustContainTraversedClasses).map(Class::getCanonicalName).forEach(c -> assertTrue(dependentClassNames.contains(c), "should contain " + c)); -// } -// -// @Test void notDefiningOutputShouldNotGenerateAnything() { -// CRDGenerator generator = new CRDGenerator(); -// assertEquals(0, generator.generate()); -// -// CustomResourceInfo joke = CustomResourceInfo.fromClass(Joke.class); -// CustomResourceInfo jr = CustomResourceInfo.fromClass(JokeRequest.class); -// generator.customResources(joke, jr); -// assertEquals(0, generator.generate()); -// } -// + @Test + void choosingCRDVersionsShouldWork() { + CRDGenerator generator = new CRDGenerator(); + assertTrue(generator.getHandlers().isEmpty()); + + generator.forCRDVersions(); + assertTrue(generator.getHandlers().isEmpty()); + + generator.forCRDVersions((List) null); + assertTrue(generator.getHandlers().isEmpty()); + + generator.forCRDVersions((String[]) null); + assertTrue(generator.getHandlers().isEmpty()); + + generator.forCRDVersions("v2"); + assertTrue(generator.getHandlers().isEmpty()); + + String version = "v1"; + generator.forCRDVersions(version); + Map handlers = generator.getHandlers(); + assertEquals(1, handlers.size()); + assertTrue(handlers.containsKey(version)); + + generator.forCRDVersions(version, "v1beta1", version, "v3", null); + handlers = generator.getHandlers(); + assertEquals(2, handlers.size()); + assertTrue(handlers.containsKey(version)); + assertTrue(handlers.containsKey("v1beta1")); + } + + @Test + void addingCustomResourceInfosShouldWork() { + CRDGenerator generator = new CRDGenerator(); + assertTrue(generator.getCustomResourceInfos().isEmpty()); + + generator.customResourceClasses(); + assertTrue(generator.getCustomResourceInfos().isEmpty()); + + generator.customResources(); + assertTrue(generator.getCustomResourceInfos().isEmpty()); + + generator.customResources(null); + assertTrue(generator.getCustomResourceInfos().isEmpty()); + + generator.customResources(null, null); + assertTrue(generator.getCustomResourceInfos().isEmpty()); + + generator.customResourceClasses(Simplest.class); + assertEquals(1, generator.getCustomResourceInfos().size()); + assertTrue(generator.getCustomResourceInfos().stream().allMatch(cri -> cri.crClassName().equals(Simplest.class.getName()))); + + generator.customResourceClasses(Child.class); + assertEquals(2, generator.getCustomResourceInfos().size()); + CustomResourceInfo simplest = CustomResourceInfo.fromClass(Simplest.class); + assertTrue(generator.getCustomResourceInfos().contains(simplest)); + CustomResourceInfo child = CustomResourceInfo.fromClass(Child.class); + assertTrue(generator.getCustomResourceInfos().contains(child)); + + generator.customResources(CustomResourceInfo.fromClass(Child.class)); + assertEquals(2, generator.getCustomResourceInfos().size()); + + CustomResourceInfo joke = CustomResourceInfo.fromClass(Joke.class); + CustomResourceInfo jr = CustomResourceInfo.fromClass(JokeRequest.class); + generator.customResources(joke, jr); + Set infos = generator.getCustomResourceInfos(); + assertEquals(4, infos.size()); + assertTrue(infos.contains(simplest)); + assertTrue(infos.contains(child)); + assertTrue(infos.contains(joke)); + assertTrue(infos.contains(jr)); + } + + @Test + void shouldProperlyRecordNumberOfGeneratedCRDs() { + CRDGenerator generator = new CRDGenerator(); + assertEquals(0, generator.generate()); + assertEquals(0, generator.detailedGenerate().numberOfGeneratedCRDs()); + + final CRDGenerationInfo info = generator + .customResourceClasses(Simplest.class, Child.class, Joke.class, JokeRequest.class) + .forCRDVersions("v1", "v1beta1") + .withOutput(output).detailedGenerate(); + + assertEquals(4 * 2, info.numberOfGeneratedCRDs()); + final Map> details = info.getCRDDetailsPerNameAndVersion(); + assertEquals(4, details.size()); + assertTrue(details.containsKey(CustomResource.getCRDName(Simplest.class))); + assertTrue(details.containsKey(CustomResource.getCRDName(Child.class))); + assertTrue(details.containsKey(CustomResource.getCRDName(Joke.class))); + final String crdName = CustomResource.getCRDName(JokeRequest.class); + assertTrue(details.containsKey(crdName)); + final Map jokeRequestInfos = info.getCRDInfos(crdName); + assertEquals(2, jokeRequestInfos.size()); + assertTrue(jokeRequestInfos.containsKey("v1")); + assertTrue(jokeRequestInfos.containsKey("v1beta1")); + } + + @Test + void shouldProperlyGenerateMultipleVersionsOfCRDs() { + CRDGenerator generator = new CRDGenerator(); + final String specVersion = "v1beta1"; + final CRDGenerationInfo info = generator + .customResourceClasses(Multiple.class, io.fabric8.crd.example.multiple.v2.Multiple.class) + .forCRDVersions(specVersion) + .withOutput(output).detailedGenerate(); + + assertEquals(1, info.numberOfGeneratedCRDs()); + final Map> details = info.getCRDDetailsPerNameAndVersion(); + assertEquals(1, details.size()); + // check multiple versions for same CR + final String crdName = CustomResource.getCRDName(Multiple.class); + assertTrue(details.containsKey(crdName)); + final Map infos = info.getCRDInfos(crdName); + assertEquals(1, infos.size()); + assertTrue(infos.containsKey(specVersion)); + + final String outputName = CRDGenerator.getOutputName(crdName, specVersion); + CustomResourceDefinition definition = output.definition(outputName); + assertNotNull(definition); + assertEquals("apiextensions.k8s.io/" + specVersion, definition.getApiVersion()); + + CustomResourceDefinitionSpec spec = definition.getSpec(); + final List versions = spec.getVersions(); + assertEquals(2, versions.size()); + assertTrue(versions.stream().filter(v -> v.getName().equals("v1")).count() == 1); + assertTrue(versions.stream().filter(v -> v.getName().equals("v2")).count() == 1); + + + Class[] mustContainTraversedClasses = {Multiple.class, MultipleSpec.class, io.fabric8.crd.example.multiple.v2.Multiple.class, io.fabric8.crd.example.multiple.v2.MultipleSpec.class}; + final Set dependentClassNames = infos.get(specVersion).getDependentClassNames(); + Arrays.stream(mustContainTraversedClasses).map(Class::getCanonicalName).forEach(c -> assertTrue(dependentClassNames.contains(c), "should contain " + c)); + } + + @Test void notDefiningOutputShouldNotGenerateAnything() { + CRDGenerator generator = new CRDGenerator(); + assertEquals(0, generator.generate()); + + CustomResourceInfo joke = CustomResourceInfo.fromClass(Joke.class); + CustomResourceInfo jr = CustomResourceInfo.fromClass(JokeRequest.class); + generator.customResources(joke, jr); + assertEquals(0, generator.generate()); + } + @Test void generatingACycleShouldFail() { final CRDGenerator generator = new CRDGenerator() .customResourceClasses(Cyclic.class) @@ -237,31 +237,31 @@ private void outputCRDIfFailed(Class> customResou throw e; } } -// @Test -// void simplestCRDShouldWork() { -// outputCRDIfFailed(Simplest.class, (customResource) -> { -// final CustomResourceDefinitionVersion version = checkCRD(customResource, "Simplest", "simplests", -// Scope.CLUSTER, SimplestSpec.class, SimplestStatus.class); -// assertNotNull(version.getSubresources()); -// }); -// -// } -// -// @Test -// void inheritedCRDShouldWork() { -// outputCRDIfFailed(Child.class, (customResource) -> { -// final CustomResourceDefinitionVersion version = checkCRD(customResource, "Child", "children", -// Scope.NAMESPACED, ChildSpec.class, ChildStatus.class, BaseSpec.class, BaseStatus.class); -// assertNotNull(version.getSubresources()); -// final Map specProps = version.getSchema().getOpenAPIV3Schema() -// .getProperties().get("spec").getProperties(); -// assertEquals(4, specProps.size()); -// assertEquals("integer", specProps.get("baseInt").getType()); -// checkMapProp(specProps, "unsupported"); -// checkMapProp(specProps, "unsupported2"); -// checkMapProp(specProps, "supported"); -// }); -// } + @Test + void simplestCRDShouldWork() { + outputCRDIfFailed(Simplest.class, (customResource) -> { + final CustomResourceDefinitionVersion version = checkCRD(customResource, "Simplest", "simplests", + Scope.CLUSTER, SimplestSpec.class, SimplestStatus.class); + assertNotNull(version.getSubresources()); + }); + + } + + @Test + void inheritedCRDShouldWork() { + outputCRDIfFailed(Child.class, (customResource) -> { + final CustomResourceDefinitionVersion version = checkCRD(customResource, "Child", "children", + Scope.NAMESPACED, ChildSpec.class, ChildStatus.class, BaseSpec.class, BaseStatus.class); + assertNotNull(version.getSubresources()); + final Map specProps = version.getSchema().getOpenAPIV3Schema() + .getProperties().get("spec").getProperties(); + assertEquals(4, specProps.size()); + assertEquals("integer", specProps.get("baseInt").getType()); + checkMapProp(specProps, "unsupported"); + checkMapProp(specProps, "unsupported2"); + checkMapProp(specProps, "supported"); + }); + } private void checkMapProp(Map specProps, String name) { final JSONSchemaProps props = specProps.get(name); diff --git a/kubernetes-tests/src/test/java/io/fabric8/crd/generator/zookeeper/v1alpha1/Zookeeper.java b/kubernetes-tests/src/test/java/io/fabric8/crd/generator/zookeeper/v1alpha1/Zookeeper.java index 315961ae0d1..265bf98ef50 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/crd/generator/zookeeper/v1alpha1/Zookeeper.java +++ b/kubernetes-tests/src/test/java/io/fabric8/crd/generator/zookeeper/v1alpha1/Zookeeper.java @@ -24,6 +24,7 @@ @Version(value="v1alpha1", storage=false) @Group("io.zookeeper") public class Zookeeper extends CustomResource implements Namespaced { - + private ZookeeperSpec spec; + private ZookeeperStatus status; }