Skip to content

Commit

Permalink
Avoid a StackOverflow and properly fail on cyclic references in CRD d…
Browse files Browse the repository at this point in the history
…efinition #3651
  • Loading branch information
andreaTP committed Dec 16, 2021
1 parent 6480899 commit d35933e
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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<String> visited, String... ignore) {
visited.add(definition.getFullyQualifiedName());
final B builder = newBuilder();
Set<String> ignores =
ignore.length > 0 ? new LinkedHashSet<>(Arrays.asList(ignore)) : Collections
Expand All @@ -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;
Expand Down Expand Up @@ -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<String> 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);
Expand All @@ -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
Expand All @@ -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);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<JSONSchemaProps, JSONSchemaPropsBuilder> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<JSONSchemaProps, JSONSchemaPropsBuilder> {
Expand Down
Original file line number Diff line number Diff line change
@@ -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<CyclicSpec, CyclicStatus> implements Namespaced {

}
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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;

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
@Version(value="v1alpha1", storage=false)
@Group("io.zookeeper")
public class Zookeeper extends CustomResource<ZookeeperSpec, ZookeeperStatus> implements Namespaced {
private ZookeeperSpec spec;
private ZookeeperStatus status;

}

0 comments on commit d35933e

Please sign in to comment.