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

Fix CRD generation #2996

Merged
merged 36 commits into from
Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
80a0fed
Fix #2910: Move crd-generator tests out of kubernetes-itests
rohanKanojia Apr 6, 2021
455497e
refactor: extract output name generation
metacosm Apr 9, 2021
9bd4cc7
fix: CRDOutput.outputFor shouldn't expect a file name
metacosm Apr 9, 2021
1365453
chore: use HasMetadata methods instead of deprecated versions
metacosm Apr 9, 2021
254dd28
refactor: move type-related methods to Types
metacosm Apr 9, 2021
2561722
chore: add tests
metacosm Apr 9, 2021
43ab31f
fix: format license
metacosm Apr 9, 2021
5f4d404
chore: remove unused constant
metacosm Apr 11, 2021
c35e433
chore: additional status detection test
metacosm Apr 11, 2021
3773ba9
fix: properly add status subresource decorator when needed
metacosm Apr 11, 2021
815ed2f
chore(tests): additional JsonSchema tests
metacosm Apr 11, 2021
df42463
fix: typo
metacosm Apr 11, 2021
b197d59
chore(tests): check that unrollHierarchy produces the expected result
metacosm Apr 11, 2021
2358610
chore(tests): add isNamespaced test
metacosm Apr 11, 2021
d423b42
refactor: use Types.isNamespaced
metacosm Apr 12, 2021
7220bb6
refactor: unify status/spec resolution
metacosm Apr 12, 2021
aebe081
misc changes
iocanel Apr 12, 2021
963f5ac
test: assert the crd apiVersion used
iocanel Apr 13, 2021
b6694fc
fix: test expectations
metacosm Apr 12, 2021
ec3d05d
fix: CRDOutput implementations were fragile and order-dependent
metacosm Apr 13, 2021
ab69481
chore(tests): add more tests + clean up
metacosm Apr 13, 2021
51d53d3
fix: do not "find" status if type is Void
metacosm Apr 13, 2021
ce04856
feat: do not add spec or status decorators if type is void
metacosm Apr 13, 2021
eca9df3
fix: avoid projecting properties twice
metacosm Apr 13, 2021
854bd47
fix: avoid unnecessary class processing
metacosm Apr 13, 2021
f7d7a7a
fix: avoid infinite loop with self-referential static properties
metacosm Apr 14, 2021
a4003d5
fix: projectProperties is now recursive. Filter out statics
iocanel Apr 14, 2021
b336bce
refactor: no need to check for already excluded static properties
metacosm Apr 14, 2021
b989cc1
feat: add function that describes types
iocanel Apr 14, 2021
70f45e4
fix: jokerequest test now looks at the right place for enum constnats
iocanel Apr 14, 2021
a6633d3
chore(deps): upgrade to sundrio 0.30.3
metacosm Apr 15, 2021
f951b03
fix: address license/format/Sonatype issues
metacosm Apr 15, 2021
cb219fc
refactor: use Types.isNamespaced and clean up code
metacosm Apr 15, 2021
4bde098
feat: make it possible to output type description
metacosm Apr 15, 2021
e8a948d
chore: CRDGeneratorTest now prints the generated crd
iocanel Apr 16, 2021
f75f98e
chore(docs): updated CHANGELOG
metacosm Apr 16, 2021
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
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
java: [8, 11]
java: [8,11]
steps:
- name: Checkout
uses: actions/checkout@v2
Expand Down
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
### 5.4-SNAPSHOT

#### Bugs

* Fix #2989: serialization will generate valid yaml when using subtypes
* Generating CRDs from the API should now properly work

#### Improvements
* Fix #2910: Move crd-generator tests from kubernetes-itests to kubernetes-tests

#### Dependency Upgrade

Expand All @@ -28,7 +29,7 @@
* Fix #2950: RawCustomResourceOperationsImpl should also work with standard resources
* Fix #2938: Make it possible to manage Tekton Triggers directly
* Fix #2921: Kubernetes server mock will generate missing metadata fields
* Fix #2946: Kubernetes server mock watch will generate initial ADDED events
* Fix #2946: Kubernetes server mock watch will generate initial `ADDED` events
* Fix #2925: Add CustomResource.getShortNames(Class) method

#### Dependency Upgrade
Expand Down
10 changes: 10 additions & 0 deletions crd-generator/api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,14 @@
<scope>test</scope>
</dependency>
</dependencies>
<build>
<plugins>
<plugin>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<compilerArgument>-proc:none</compilerArgument>
</configuration>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import io.sundr.codegen.model.Property;
import io.sundr.codegen.model.TypeDef;
import io.sundr.codegen.model.TypeDefBuilder;
import io.sundr.codegen.model.TypeRef;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -56,37 +55,50 @@ public void handle(CustomResourceInfo config) {
LabelSelectorPathDetector labelSelectorPathDetector = new LabelSelectorPathDetector();
AdditionalPrinterColumnDetector additionalPrinterColumnDetector = new AdditionalPrinterColumnDetector();

boolean statusExists = config.statusClassName().isPresent();
if (statusExists) {
TypeDefBuilder builder = new TypeDefBuilder(def);
Optional<Property> statusProperty = findStatusProperty(def);
if (statusProperty.isPresent()) {
Property p = statusProperty.get();
builder.removeFromProperties(p);

def = builder
.addNewProperty()
.withName("status")
.withTypeRef(p.getTypeRef())
.endProperty()
.build();
} else {
statusExists = false;
}
}

TypeDefBuilder builder = new TypeDefBuilder(def);
Optional<Property> statusProperty = findStatusProperty(def);

if (statusProperty.isPresent()) {
Property p = statusProperty.get();
builder.removeFromProperties(p);

def = builder
.addNewProperty()
.withName("status")
.withTypeRef(p.getTypeRef())
.endProperty()
.build();
if (config.specClassName().isPresent()) {
builder.accept(specReplicasPathDetector);
}

if (statusExists) {
builder.accept(statusReplicasPathDetector);
}

def = new TypeDefBuilder(def)
.accept(specReplicasPathDetector)
.accept(statusReplicasPathDetector)
def = builder
.accept(labelSelectorPathDetector)
.accept(additionalPrinterColumnDetector)
.build();

addDecorators(config, def, specReplicasPathDetector.getPath(), statusReplicasPathDetector.getPath(), labelSelectorPathDetector.getPath());
addDecorators(config, def, specReplicasPathDetector.getPath(),
statusReplicasPathDetector.getPath(), labelSelectorPathDetector.getPath());

Map<String, Property> additionalPrinterColumns = new HashMap<>();
additionalPrinterColumns.putAll(additionalPrinterColumnDetector.getProperties());
additionalPrinterColumns.forEach((path, property) -> {
Map<String, Object> parameters = property.getAnnotations().stream()
.filter(a -> a.getClassRef().getName().equals("PrinterColumn")).map(a -> a.getParameters())
.findFirst().orElse(Collections.emptyMap());
String type = AbstractJsonSchema.COMMON_MAPPINGS.getOrDefault(property.getTypeRef(), "object");
String type = AbstractJsonSchema.COMMON_MAPPINGS
.getOrDefault(property.getTypeRef(), "object");
String column = (String) parameters.get("name");
if (Utils.isNullOrEmpty(column)) {
column = property.getName().toUpperCase();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ protected T internalFrom(TypeDef definition, String... ignore) {
.emptySet();
List<String> required = new ArrayList<>();

for (Property property : Types.allProperties(definition)) {
for (Property property : Types.projectProperties(definition)) {
final String name = property.getName();

if (property.isStatic() || ignores.contains(name)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@
import java.io.IOException;
import java.io.OutputStream;
import java.net.URI;
import java.util.HashMap;
import java.util.Map;

public class CRDGenerator {

private final Resources resources;
private final CustomResourceHandler v1Handler;
private final io.fabric8.crd.generator.v1beta1.CustomResourceHandler v1beta1Handler;
private CRDOutput output;
private CRDOutput<? extends OutputStream> output;
private boolean hasResources;

private static final ObjectMapper YAML_MAPPER = new ObjectMapper(
Expand All @@ -62,7 +64,7 @@ public CRDGenerator inOutputDir(File outputDir) {
return this;
}

public CRDGenerator withOutput(CRDOutput output) {
public CRDGenerator withOutput(CRDOutput<? extends OutputStream> output) {
this.output = output;
return this;
}
Expand Down Expand Up @@ -100,10 +102,11 @@ public void generate() {
list.getItems().forEach(crd -> {
final String version = ApiVersionUtil.trimVersion(crd.getApiVersion());
try {
try (final OutputStream outputStream = output.outputFor(crd.getMetadata().getName() + "-" + version + ".yml")){
String outputName = getOutputName(crd.getMetadata().getName(), version);
try (final OutputStream outputStream = output.outputFor(outputName)){
outputStream.write("# Generated by Fabric8 CRDGenerator, manual edits might get overwritten!\n".getBytes());
YAML_MAPPER.writeValue(outputStream, crd);
System.out.println("Created " + output.crdURI());
System.out.println("Created " + output.crdURI(outputName));
}
} catch (IOException e) {
throw new RuntimeException(e);
Expand All @@ -115,15 +118,43 @@ public void generate() {
}
}

public interface CRDOutput extends Closeable {
OutputStream outputFor(String crdFileName) throws IOException;
URI crdURI();
public static String getOutputName(String crdName, String crdSpecVersion) {
return crdName + "-" + crdSpecVersion;
}

static class DirCRDOutput implements CRDOutput {
public interface CRDOutput<T extends OutputStream> extends Closeable {
T outputFor(String crdName) throws IOException;
default URI crdURI(String crdName) {
return URI.create(crdName);
}
}

public abstract static class AbstractCRDOutput<T extends OutputStream> implements CRDOutput<T> {
private final Map<String, T> crds = new HashMap<>(7);

@Override
public T outputFor(String crdName) throws IOException {
final T outputStream = createStreamFor(crdName);
crds.put(crdName, outputStream);
return outputStream;
}

protected abstract T createStreamFor(String crdName) throws IOException;

protected T getStreamFor(String crdName) {
return crds.get(crdName);
}

@Override
public void close() throws IOException {
for (T stream : crds.values()) {
stream.close();
}
}
}

static class DirCRDOutput extends AbstractCRDOutput<FileOutputStream> {
private final File dir;
private OutputStream output;
private URI uri;

public DirCRDOutput(File dir) {
if (!dir.isDirectory() || !dir.canWrite() || !dir.exists()) {
Expand All @@ -133,21 +164,18 @@ public DirCRDOutput(File dir) {
}

@Override
public OutputStream outputFor(String crdFileName) throws IOException {
final File file = new File(dir, crdFileName);
uri = file.toURI();
output = new FileOutputStream(file);
return output;
protected FileOutputStream createStreamFor(String crdName) throws IOException {
final File file = getCRDFile(crdName);
return new FileOutputStream(file);
}

@Override
public URI crdURI() {
return uri;
private File getCRDFile(String crdName) {
return new File(dir, crdName + ".yml");
}

@Override
public void close() throws IOException {
output.close();
public URI crdURI(String crdName) {
return URI.create("file://" + getCRDFile(crdName));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@
*/
package io.fabric8.crd.generator;

import io.fabric8.kubernetes.api.model.Namespaced;
import io.fabric8.crd.generator.utils.Types;
import io.fabric8.crd.generator.utils.Types.SpecAndStatus;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.KubernetesClientException;
import io.fabric8.kubernetes.model.Scope;
import io.sundr.codegen.functions.ClassTo;
import io.sundr.codegen.model.TypeDef;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Optional;

public class CustomResourceInfo {
public static final boolean DESCRIBE_TYPE_DEFS = false;
private final String group;
private final String version;
private final String kind;
Expand Down Expand Up @@ -121,43 +121,29 @@ public TypeDef definition() {
return definition;
}

private final static String TYPE_NAME = CustomResource.class.getTypeName();
private final static String VOID_TYPE_NAME = Void.class.getTypeName();


public static CustomResourceInfo fromClass(Class<? extends CustomResource> customResource) {
try {
final CustomResource instance = customResource.getDeclaredConstructor().newInstance();

final String[] shortNames = CustomResource.getShortNames(customResource);

final Scope scope = Arrays.stream(customResource.getInterfaces())
.filter(t -> t.getTypeName().equals(Namespaced.class.getTypeName()))
.findFirst().map(t -> Scope.NAMESPACED).orElse(Scope.CLUSTER);

final TypeDef definition = ClassTo.TYPEDEF.apply(customResource);

// walk the type hierarchy until we reach CustomResource or a ParameterizedType
Type genericSuperclass = customResource.getGenericSuperclass();
String typeName = genericSuperclass.getTypeName();
while (!TYPE_NAME.equals(typeName) && !(genericSuperclass instanceof ParameterizedType)) {
genericSuperclass = ((Class) genericSuperclass).getGenericSuperclass();
typeName = genericSuperclass.getTypeName();
if (DESCRIBE_TYPE_DEFS) {
manusa marked this conversation as resolved.
Show resolved Hide resolved
Types.describeType(definition, "", new HashSet<>());
}

// this works because CustomResource is an abstract class
String specClassName = null, statusClassName = null;
if (genericSuperclass instanceof ParameterizedType) {
final Type[] types = ((ParameterizedType) genericSuperclass).getActualTypeArguments();
if (types.length == 2) {
specClassName = types[0].getTypeName();
statusClassName = types[1].getTypeName();
}
final Scope scope = Types.isNamespaced(definition) ? Scope.NAMESPACED : Scope.CLUSTER;

SpecAndStatus specAndStatus = Types.resolveSpecAndStatusTypes(definition);
if (specAndStatus.isUnreliable()) {
System.out.println("Cannot reliably determine status types for " + customResource.getCanonicalName()
+ " because it isn't parameterized with only spec and status types. Status replicas detection will be deactivated.");
}

return new CustomResourceInfo(instance.getGroup(), instance.getVersion(), instance.getKind(),
instance.getSingular(), instance.getPlural(), shortNames, instance.isStorage(), instance.isServed(), scope, definition,
customResource.getCanonicalName(), specClassName, statusClassName);
customResource.getCanonicalName(), specAndStatus.getSpecClassName(),
specAndStatus.getStatusClassName());
} catch (InstantiationException | IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
throw KubernetesClientException.launderThrowable(e);
}
Expand Down
Loading