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

[crd-generator] Fix fallback value of Default annotation in presence of multiple accessors #5503

Merged
merged 5 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### 6.10-SNAPSHOT

#### Bugs
* Fix #5501: [crd-generator] Fix fallback value of `Default` annotation in presence of multiple accessors

#### Improvements

Expand Down
5 changes: 5 additions & 0 deletions crd-generator/api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@
<artifactId>validation-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<scope>test</scope>
</dependency>
</dependencies>
<build>
<plugins>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ public Property process() {
LOGGER.debug("Description for property {} has already been contributed by: {}", name, descriptionContributedBy);
}
}
defaultValue = p.getDefault().orElse(null);
defaultValue = p.getDefault().orElse(defaultValue);
Copy link
Member Author

Choose a reason for hiding this comment

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

for the records, this is the bug.

min = p.getMin().orElse(min);
max = p.getMax().orElse(max);
pattern = p.getPattern().orElse(pattern);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
import io.fabric8.generator.annotation.Nullable;
import io.fabric8.generator.annotation.Pattern;
import io.fabric8.generator.annotation.Required;
import lombok.Data;

@Data
public class AnnotatedSpec {
@JsonProperty("from-field")
@JsonPropertyDescription("from-field-description")
Expand All @@ -37,6 +39,8 @@ public class AnnotatedSpec {
private String singleDigit;
private String nullable;
private String defaultValue;
@Default("my-value2")
private String defaultValue2;
@Required
private boolean emptySetter;
@Required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ void shouldAugmentPropertiesSchemaFromAnnotations() throws JsonProcessingExcepti
assertNotNull(schema);
Map<String, JSONSchemaProps> properties = assertSchemaHasNumberOfProperties(schema, 2);
final JSONSchemaProps specSchema = properties.get("spec");
Map<String, JSONSchemaProps> spec = assertSchemaHasNumberOfProperties(specSchema, 12);
Map<String, JSONSchemaProps> spec = assertSchemaHasNumberOfProperties(specSchema, 13);

// check descriptions are present
assertTrue(spec.containsKey("from-field"));
Expand Down Expand Up @@ -154,6 +154,13 @@ void shouldAugmentPropertiesSchemaFromAnnotations() throws JsonProcessingExcepti
assertNull(defaultValue.getMaximum());
assertNull(defaultValue.getPattern());

final JSONSchemaProps defaultValue2 = spec.get("defaultValue2");
assertEquals("my-value2", YAML_MAPPER.writeValueAsString(defaultValue2.getDefault()).trim());
assertNull(defaultValue.getNullable());
assertNull(defaultValue.getMinimum());
assertNull(defaultValue.getMaximum());
assertNull(defaultValue.getPattern());
andreaTP marked this conversation as resolved.
Show resolved Hide resolved

// check required list, should register properties with their modified name if needed
final List<String> required = specSchema.getRequired();
assertEquals(3, required.size());
Expand Down
6 changes: 6 additions & 0 deletions crd-generator/test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@
<groupId>io.fabric8</groupId>
<artifactId>crd-generator-apt</artifactId>
</dependency>

<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.junit.jupiter</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ void testCrdv1beta1() {
assertEquals(1, v.getAdditionalPrinterColumns().get(0).getPriority());
assertEquals("UPTIME", v.getAdditionalPrinterColumns().get(1).getName());
assertEquals(0, v.getAdditionalPrinterColumns().get(1).getPriority());
assertEquals("false", props.getProperties().get("ephemeral").getDefault().asText());
});

Optional<CustomResourceDefinitionVersion> v1alpha1 = d.getSpec().getVersions().stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@
*/
package io.fabric8.crd.generator.zookeeper.v1;

import io.fabric8.generator.annotation.Default;
import lombok.Data;
import io.fabric8.generator.annotation.Required;
import io.fabric8.kubernetes.model.annotation.SpecReplicas;

@Data
public class ZookeeperSpec {

@SpecReplicas
private int size;
@Required
private String version;
@Default("false")
Copy link
Member Author

Choose a reason for hiding this comment

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

Please, take note that the issue was reproducible only here.

private boolean ephemeral;
}
Loading