Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid a StackOverflow and properly fail on cyclic references in CRD generation #3652

Merged
merged 6 commits into from
Dec 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,10 @@ 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) {
final B builder = newBuilder();
Set<String> ignores =
ignore.length > 0 ? new LinkedHashSet<>(Arrays.asList(ignore)) : Collections
Expand All @@ -140,7 +139,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 +360,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 +393,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 +416,7 @@ public T internalFrom(String name, TypeRef typeRef) {
.toArray(JsonNode[]::new);
return enumProperty(enumValues);
} else {
return internalFrom(def);
return resolveNestedClass(name, def, visited);
}

}
Expand All @@ -422,6 +425,26 @@ public T internalFrom(String name, TypeRef typeRef) {
}
}

// Flag to detect cycles
private boolean resolving = false;

private T resolveNestedClass(String name, TypeDef def, Set<String> 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)
*
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,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 CyclicStatus {
private String 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
@@ -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<NoCyclicSpec, NoCyclicStatus> implements Namespaced {

}
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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 NoCyclicStatus {
private String message;
private Ref ref1;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* 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;

protected Inner inner;

public static class Inner { }

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
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;
import io.fabric8.crd.example.joke.JokeRequestSpec;
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;
Expand Down Expand Up @@ -197,6 +199,28 @@ 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(),
"An IllegalArgument Exception hasn't been thrown when generating a CRD with cyclic references"
);
}

@Test void notGeneratingACycleShouldSucceed() {
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 {
Expand Down