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

Enable configuration of whether quotes are used in generated yaml #6773

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

#### Improvements

* Fix #6763: CRDGenerator: YAML output customization

#### Dependency Upgrade

#### New Features
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public class CRDGenerator {
private ObjectMapper objectMapper;
private KubernetesSerialization kubernetesSerialization;
private Map<String, CustomResourceInfo> infos;
private boolean minQuotes = false;

public CRDGenerator inOutputDir(File outputDir) {
output = new DirCRDOutput(outputDir);
Expand All @@ -70,6 +71,11 @@ public CRDGenerator withImplicitPreserveUnknownFields(boolean implicitPreserveUn
return this;
}

public CRDGenerator withMinQuotes(boolean minQuotes) {
this.minQuotes = minQuotes;
return this;
}

public CRDGenerator withParallelGenerationEnabled(boolean parallel) {
this.parallel = parallel;
return this;
Expand Down Expand Up @@ -212,7 +218,7 @@ public void emitCrd(HasMetadata crd, Set<String> dependentClassNames, CRDGenerat
final String outputName = getOutputName(crdName, version);
try (final OutputStreamWriter writer = new OutputStreamWriter(output.outputFor(outputName), StandardCharsets.UTF_8)) {
writer.write("# Generated by Fabric8 CRDGenerator, manual edits might get overwritten!\n");
String yaml = kubernetesSerialization.asYaml(crd);
String yaml = kubernetesSerialization.asYaml(crd, minQuotes);
// strip the explicit start added by default
writer.write(yaml.substring(4));
final URI fileURI = output.crdURI(outputName);
Expand Down Expand Up @@ -284,4 +290,5 @@ public URI crdURI(String crdName) {
return getCRDFile(crdName).toURI();
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@

import static io.fabric8.crdv2.generator.CRDGeneratorAssertions.assertFileEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
Expand Down Expand Up @@ -103,6 +104,7 @@ void choosingCRDVersionsShouldWork() {
@Test
void addingCustomResourceInfosShouldWork() {
CRDGenerator generator = newCRDGenerator();

assertTrue(generator.getCustomResourceInfos().isEmpty());

generator.customResourceClasses();
Expand Down Expand Up @@ -477,6 +479,66 @@ void checkGenerationIsDeterministic() throws Exception {
assertTrue(outputDir.delete());
}

@Test
void checkMinQuotesDefault() throws Exception {
final File outputDir = Files.createTempDirectory("crd-").toFile();
final String crdName = CustomResourceInfo.fromClass(Complex.class).crdName();
CRDGenerationInfo crdInfo = newCRDGenerator()
.inOutputDir(outputDir)
.forCRDVersions("v1")
.customResourceClasses(Complex.class)
.detailedGenerate();

File crdFile = new File(crdInfo.getCRDInfos(crdName).get("v1").getFilePath());
String crd = Files.readString(crdFile.toPath());
assertTrue(crd.contains("\"complexkinds.example.com\""));

// only delete the generated files if the test is successful
assertTrue(crdFile.delete());
assertTrue(outputDir.delete());
}

@Test
void checkMinQuotesFalse() throws Exception {
final File outputDir = Files.createTempDirectory("crd-").toFile();
final String crdName = CustomResourceInfo.fromClass(Complex.class).crdName();
CRDGenerationInfo crdInfo = newCRDGenerator()
.inOutputDir(outputDir)
.forCRDVersions("v1")
.customResourceClasses(Complex.class)
.withMinQuotes(false)
.detailedGenerate();

File crdFile = new File(crdInfo.getCRDInfos(crdName).get("v1").getFilePath());
String crd = Files.readString(crdFile.toPath());
assertTrue(crd.contains("\"complexkinds.example.com\""));

// only delete the generated files if the test is successful
assertTrue(crdFile.delete());
assertTrue(outputDir.delete());
}

@Test
void checkMinQuotesTrue() throws Exception {
final File outputDir = Files.createTempDirectory("crd-").toFile();
final String crdName = CustomResourceInfo.fromClass(Complex.class).crdName();
CRDGenerationInfo crdInfo = newCRDGenerator()
.inOutputDir(outputDir)
.forCRDVersions("v1")
.customResourceClasses(Complex.class)
.withMinQuotes(true)
.detailedGenerate();

File crdFile = new File(crdInfo.getCRDInfos(crdName).get("v1").getFilePath());
String crd = Files.readString(crdFile.toPath());
assertTrue(crd.contains("complexkinds.example.com"));
assertFalse(crd.contains("\"complexkinds.example.com\""));

// only delete the generated files if the test is successful
assertTrue(crdFile.delete());
assertTrue(outputDir.delete());
}

@RepeatedTest(value = 10)
void checkGenerationMultipleVersionsOfCRDsIsDeterministic() throws Exception {
// generated CRD
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@ public class CrdGeneratorMojo extends AbstractMojo {
@Parameter(property = "fabric8.crd-generator.skip", defaultValue = "false")
boolean skip;

/**
* If {@code true}, quotes will only be included where necessary
*/
@Parameter(property = "fabric8.crd-generator.minimizeQuotes", defaultValue = "false")
boolean minimizeQuotes;

private final CustomResourceCollector customResourceCollector;
private final CRDGenerator crdGenerator;

Expand Down Expand Up @@ -178,6 +184,7 @@ public void execute() throws MojoExecutionException {
.customResourceClasses(customResourceClassesLoaded)
.withParallelGenerationEnabled(parallel)
.withImplicitPreserveUnknownFields(implicitPreserveUnknownFields)
.withMinQuotes(minimizeQuotes)
.inOutputDir(outputDirectory);

CRDGenerationInfo crdGenerationInfo = crdGenerator.detailedGenerate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,23 @@ public <T> String asJson(T object) {
* @return a String containing a JSON representation of the provided object.
*/
public <T> String asYaml(T object) {
return asYaml(object, false);
}

/**
* Returns a YAML representation of the given object.
*
* <p>
* If the provided object contains a JsonAnyGetter annotated method with a Map that contains an entry that
* overrides a field of the provided object, the Map entry will take precedence upon serialization. Properties won't
* be duplicated.
*
* @param object the object to serialize.
* @param minQuotes whether strings will be rendered without quotes (true) or with quotes (false).
* @param <T> the type of the object being serialized.
* @return a String containing a JSON representation of the provided object.
*/
public <T> String asYaml(T object, boolean minQuotes) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change looks good, but wanted to bring up that based upon the DumpSettings there's a lot that could be overriden about the generation style.

Min quoting is roughly defaultScalarStyle=ScalarStyle.PLAIN - it's just that we need to still quote keys that look like boolean values to kube. Another possibliity is to create our own "DumpSettings" builder and have that as an argument so that this can be expanded in the future without adding additional signatures to asYaml.

DumpSettings settings = DumpSettings.builder()
.setExplicitStart(true).setDefaultFlowStyle(FlowStyle.BLOCK).build();
final Dump yaml = new Dump(settings, new StandardRepresenter(settings) {
Expand All @@ -207,7 +224,7 @@ protected NodeTuple representMappingEntry(java.util.Map.Entry<?, ?> entry) {
}
}
org.snakeyaml.engine.v2.nodes.Node nodeKey = representData(key);
quote = true;
quote = !minQuotes;
return new NodeTuple(nodeKey, representData(entry.getValue()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.apiextensions.v1beta1.CustomResourceDefinition;
import io.fabric8.kubernetes.model.annotation.Group;
import io.fabric8.kubernetes.model.annotation.Version;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -33,6 +34,11 @@
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -65,6 +71,30 @@ void withRegisteredKubernetesResourceShouldDeserializeToPod() {
.isInstanceOf(io.fabric8.kubernetes.api.model.Pod.class);
}

@Test
void asYaml() throws Exception {
final String input = readYamlToString("/serialization/test-crd-schema.yml");
final CustomResourceDefinition crd = Serialization.unmarshal(input, CustomResourceDefinition.class);

String result = kubernetesSerialization.asYaml(crd);
assertThat(result).asString().contains("\"widgets.test.fabric8.io\"");

result = kubernetesSerialization.asYaml(crd, false);
assertThat(result).asString().contains("\"widgets.test.fabric8.io\"");

result = kubernetesSerialization.asYaml(crd, true);
assertThat(result).asString().doesNotContain("\"widgets.test.fabric8.io\"");
assertThat(result).asString().contains("widgets.test.fabric8.io");
}

private String readYamlToString(String path) throws IOException {
return Files.readAllLines(
new File(KubernetesSerializationTest.class.getResource(path).getFile()).toPath(), StandardCharsets.UTF_8)
.stream()
.filter(line -> !line.startsWith("#"))
.collect(Collectors.joining("\n"));
}

@ParameterizedTest(name = "{index}: {0} {1} deserializes to {2}")
@MethodSource("sameGVK")
void withCollidingRegisteredKubernetesResourceShouldDeserializeAppropriate(
Expand Down
Loading