Skip to content

Commit 1b4af7a

Browse files
authored
fix: move all annotations from CSVMetadata to metadata annotations (#607)
It seems that that's where they should go and it's not obvious at all what the spec.annotations field is for in the CSV. Also add support for `skipRange` and add tests.
1 parent 8a94ceb commit 1b4af7a

File tree

7 files changed

+121
-54
lines changed

7 files changed

+121
-54
lines changed

bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/BundleProcessor.java

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
import java.io.IOException;
77
import java.nio.file.Path;
88
import java.util.ArrayList;
9+
import java.util.Collections;
910
import java.util.HashMap;
1011
import java.util.LinkedList;
1112
import java.util.List;
13+
import java.util.Map;
1214
import java.util.Objects;
1315
import java.util.Optional;
1416
import java.util.function.Function;
@@ -203,6 +205,16 @@ private CSVMetadataHolder createMetadataHolder(AnnotationInstance csvMetadata, C
203205
CSVMetadataHolder.Annotations annotations;
204206
if (annotationsField != null) {
205207
final var annotationsAnn = annotationsField.asNested();
208+
209+
final var othersAnn = annotationsAnn.value("others");
210+
Map<String, String> others = Collections.emptyMap();
211+
if (othersAnn != null) {
212+
final var othersArray = othersAnn.asNestedArray();
213+
others = new HashMap<>(othersArray.length);
214+
for (AnnotationInstance other : othersArray) {
215+
others.put(other.value("name").asString(), other.value("value").asString());
216+
}
217+
}
206218
annotations = new CSVMetadataHolder.Annotations(
207219
ConfigurationUtils.annotationValueOrDefault(annotationsAnn, "containerImage",
208220
AnnotationValue::asString, () -> null),
@@ -215,7 +227,10 @@ private CSVMetadataHolder createMetadataHolder(AnnotationInstance csvMetadata, C
215227
ConfigurationUtils.annotationValueOrDefault(annotationsAnn, "certified",
216228
AnnotationValue::asBoolean, () -> false),
217229
ConfigurationUtils.annotationValueOrDefault(annotationsAnn, "almExamples",
218-
AnnotationValue::asString, () -> null));
230+
AnnotationValue::asString, () -> null),
231+
ConfigurationUtils.annotationValueOrDefault(annotationsAnn, "skipRange",
232+
AnnotationValue::asString, () -> null),
233+
others);
219234
} else {
220235
annotations = mh.annotations;
221236
}
@@ -335,6 +350,8 @@ private CSVMetadataHolder createMetadataHolder(AnnotationInstance csvMetadata, C
335350
providerName, providerURL,
336351
ConfigurationUtils.annotationValueOrDefault(csvMetadata, "replaces",
337352
AnnotationValue::asString, () -> mh.replaces),
353+
ConfigurationUtils.annotationValueOrDefault(csvMetadata, "skips",
354+
AnnotationValue::asStringArray, () -> mh.skips),
338355
ConfigurationUtils.annotationValueOrDefault(csvMetadata, "version",
339356
AnnotationValue::asString, () -> mh.version),
340357
ConfigurationUtils.annotationValueOrDefault(csvMetadata, "maturity",

bundle-generator/deployment/src/main/java/io/quarkiverse/operatorsdk/bundle/deployment/builders/CsvManifestsBuilder.java

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public class CsvManifestsBuilder extends ManifestsBuilder {
3434

3535
private static final String IMAGE_PNG = "image/png";
3636

37-
private final ClusterServiceVersionBuilder csvBuilder;
37+
private ClusterServiceVersionBuilder csvBuilder;
3838
private final SortedSet<String> ownedCRs = new TreeSet<>();
3939
private final SortedSet<String> requiredCRs = new TreeSet<>();
4040
private final Path kubernetesResources;
@@ -44,8 +44,24 @@ public CsvManifestsBuilder(CSVMetadataHolder metadata, List<ReconcilerAugmentedC
4444
super(metadata);
4545
this.kubernetesResources = mainSourcesRoot != null ? mainSourcesRoot.resolve("kubernetes") : null;
4646

47-
csvBuilder = new ClusterServiceVersionBuilder()
48-
.withNewMetadata().withName(getName()).endMetadata();
47+
csvBuilder = new ClusterServiceVersionBuilder();
48+
49+
final var metadataBuilder = csvBuilder.withNewMetadata().withName(getName());
50+
if (metadata.annotations != null) {
51+
metadataBuilder.addToAnnotations("olm.skipRange", metadata.annotations.skipRange);
52+
metadataBuilder.addToAnnotations("containerImage", metadata.annotations.containerImage);
53+
metadataBuilder.addToAnnotations("repository", metadata.annotations.repository);
54+
metadataBuilder.addToAnnotations("capabilities", metadata.annotations.capabilities);
55+
metadataBuilder.addToAnnotations("categories", metadata.annotations.categories);
56+
metadataBuilder.addToAnnotations("certified",
57+
String.valueOf(metadata.annotations.certified));
58+
metadataBuilder.addToAnnotations("alm-examples", metadata.annotations.almExamples);
59+
if (metadata.annotations.others != null) {
60+
metadata.annotations.others.forEach(metadataBuilder::addToAnnotations);
61+
}
62+
}
63+
csvBuilder = metadataBuilder.endMetadata();
64+
4965
final var csvSpecBuilder = csvBuilder
5066
.editOrNewSpec()
5167
.withDescription(metadata.description)
@@ -63,15 +79,6 @@ public CsvManifestsBuilder(CSVMetadataHolder metadata, List<ReconcilerAugmentedC
6379
.endProvider();
6480
}
6581

66-
if (metadata.annotations != null) {
67-
csvSpecBuilder.addToAnnotations("containerImage", metadata.annotations.containerImage);
68-
csvSpecBuilder.addToAnnotations("repository", metadata.annotations.repository);
69-
csvSpecBuilder.addToAnnotations("capabilities", metadata.annotations.capabilities);
70-
csvSpecBuilder.addToAnnotations("categories", metadata.annotations.categories);
71-
csvSpecBuilder.addToAnnotations("certified", String.valueOf(metadata.annotations.certified));
72-
csvSpecBuilder.addToAnnotations("alm-examples", metadata.annotations.almExamples);
73-
}
74-
7582
if (metadata.maintainers != null) {
7683
for (CSVMetadataHolder.Maintainer maintainer : metadata.maintainers) {
7784
csvSpecBuilder.addNewMaintainer(maintainer.email, maintainer.name);
@@ -216,8 +223,7 @@ private String readIconAsBase64(String fileName) {
216223
public byte[] getManifestData(List<ServiceAccount> serviceAccounts, List<ClusterRoleBinding> clusterRoleBindings,
217224
List<ClusterRole> clusterRoles, List<RoleBinding> roleBindings, List<Role> roles,
218225
List<Deployment> deployments) throws IOException {
219-
final var csvSpecBuilder = csvBuilder
220-
.editOrNewSpec();
226+
final var csvSpecBuilder = csvBuilder.editOrNewSpec();
221227

222228
String defaultServiceAccountName = NO_SERVICE_ACCOUNT;
223229
if (!serviceAccounts.isEmpty()) {

bundle-generator/deployment/src/test/java/io/quarkiverse/operatorsdk/bundle/MultipleOperatorsBundleTest.java

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
package io.quarkiverse.operatorsdk.bundle;
22

3+
import static io.quarkiverse.operatorsdk.bundle.Utils.assertFileExistsIn;
4+
import static io.quarkiverse.operatorsdk.bundle.Utils.checkBundleFor;
5+
import static io.quarkiverse.operatorsdk.bundle.Utils.getCRDNameFor;
36
import static org.junit.jupiter.api.Assertions.assertEquals;
47

58
import java.io.IOException;
69
import java.nio.file.Files;
7-
import java.nio.file.Path;
810

9-
import org.junit.jupiter.api.Assertions;
1011
import org.junit.jupiter.api.Test;
1112
import org.junit.jupiter.api.extension.RegisterExtension;
1213

@@ -51,37 +52,9 @@ public void shouldWriteBundleForTheOperators() throws IOException {
5152
// CRDs should be alphabetically ordered
5253
assertEquals(HasMetadata.getFullResourceName(External.class), crds.getRequired().get(0).getName());
5354
assertEquals(HasMetadata.getFullResourceName(SecondExternal.class), crds.getRequired().get(1).getName());
55+
assertEquals("1.0.0", csv.getSpec().getReplaces());
56+
assertEquals(">=1.0.0 <1.0.3", csv.getMetadata().getAnnotations().get("olm.skipRange"));
57+
assertEquals("Test", csv.getMetadata().getAnnotations().get("capabilities"));
58+
assertEquals("bar", csv.getMetadata().getAnnotations().get("foo"));
5459
}
55-
56-
private void checkBundleFor(Path bundle, String operatorName, Class<? extends HasMetadata> resourceClass) {
57-
final var operatorManifests = bundle.resolve(operatorName);
58-
assertFileExistsIn(operatorManifests, bundle);
59-
assertFileExistsIn(operatorManifests.resolve("bundle.Dockerfile"), bundle);
60-
final var manifests = operatorManifests.resolve("manifests");
61-
assertFileExistsIn(manifests, bundle);
62-
assertFileExistsIn(manifests.resolve(operatorName + ".clusterserviceversion.yaml"), manifests);
63-
assertFileExistsIn(manifests.resolve(getCRDNameFor(resourceClass)), manifests);
64-
final var metadata = operatorManifests.resolve("metadata");
65-
assertFileExistsIn(metadata, bundle);
66-
assertFileExistsIn(metadata.resolve("annotations.yaml"), metadata);
67-
}
68-
69-
private static String getCRDNameFor(Class<? extends HasMetadata> resourceClass) {
70-
return HasMetadata.getFullResourceName(resourceClass) + "-v1.crd.yml";
71-
}
72-
73-
private static void assertFileExistsIn(Path file, Path parent) {
74-
final var exists = Files.exists(file);
75-
if (!exists) {
76-
System.out.println("Couldn't find " + file.getFileName() + " in " + parent);
77-
System.out.println("Known files: ");
78-
try (final var list = Files.list(parent)) {
79-
list.forEach(f -> System.out.println("\t" + f));
80-
} catch (IOException e) {
81-
throw new RuntimeException(e);
82-
}
83-
}
84-
Assertions.assertTrue(exists);
85-
}
86-
8760
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package io.quarkiverse.operatorsdk.bundle;
2+
3+
import java.io.IOException;
4+
import java.nio.file.Files;
5+
import java.nio.file.Path;
6+
7+
import org.junit.jupiter.api.Assertions;
8+
9+
import io.fabric8.kubernetes.api.model.HasMetadata;
10+
11+
public class Utils {
12+
13+
static void checkBundleFor(Path bundle, String operatorName,
14+
Class<? extends HasMetadata> resourceClass) {
15+
final var operatorManifests = bundle.resolve(operatorName);
16+
assertFileExistsIn(operatorManifests, bundle);
17+
assertFileExistsIn(operatorManifests.resolve("bundle.Dockerfile"), bundle);
18+
final var manifests = operatorManifests.resolve("manifests");
19+
assertFileExistsIn(manifests, bundle);
20+
assertFileExistsIn(manifests.resolve(operatorName + ".clusterserviceversion.yaml"), manifests);
21+
if (resourceClass != null) {
22+
assertFileExistsIn(manifests.resolve(getCRDNameFor(resourceClass)), manifests);
23+
}
24+
final var metadata = operatorManifests.resolve("metadata");
25+
assertFileExistsIn(metadata, bundle);
26+
assertFileExistsIn(metadata.resolve("annotations.yaml"), metadata);
27+
}
28+
29+
static String getCRDNameFor(Class<? extends HasMetadata> resourceClass) {
30+
return HasMetadata.getFullResourceName(resourceClass) + "-v1.crd.yml";
31+
}
32+
33+
static void assertFileExistsIn(Path file, Path parent) {
34+
final var exists = Files.exists(file);
35+
if (!exists) {
36+
System.out.println("Couldn't find " + file.getFileName() + " in " + parent);
37+
System.out.println("Known files: ");
38+
try (final var list = Files.list(parent)) {
39+
list.forEach(f -> System.out.println("\t" + f));
40+
} catch (IOException e) {
41+
throw new RuntimeException(e);
42+
}
43+
}
44+
Assertions.assertTrue(exists);
45+
}
46+
47+
}

bundle-generator/deployment/src/test/java/io/quarkiverse/operatorsdk/bundle/sources/ThirdReconciler.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
77
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
88
import io.quarkiverse.operatorsdk.bundle.runtime.CSVMetadata;
9+
import io.quarkiverse.operatorsdk.bundle.runtime.CSVMetadata.Annotations;
10+
import io.quarkiverse.operatorsdk.bundle.runtime.CSVMetadata.Annotations.Annotation;
911
import io.quarkiverse.operatorsdk.bundle.runtime.CSVMetadata.RequiredCRD;
1012

1113
@CSVMetadata(name = "third-operator", requiredCRDs = @RequiredCRD(kind = SecondExternal.KIND, name = "externalagains."
12-
+ SecondExternal.GROUP, version = SecondExternal.VERSION))
14+
+ SecondExternal.GROUP, version = SecondExternal.VERSION), replaces = "1.0.0", annotations = @Annotations(skipRange = ">=1.0.0 <1.0.3", capabilities = "Test", others = @Annotation(name = "foo", value = "bar")))
1315
@ControllerConfiguration(dependents = @Dependent(type = ExternalDependentResource.class))
1416
public class ThirdReconciler implements Reconciler<Third> {
1517

bundle-generator/runtime/src/main/java/io/quarkiverse/operatorsdk/bundle/runtime/CSVMetadata.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,16 @@
5353
boolean certified() default false;
5454

5555
String almExamples() default "";
56+
57+
String skipRange() default "";
58+
59+
Annotation[] others() default {};
60+
61+
@interface Annotation {
62+
String name();
63+
64+
String value();
65+
}
5666
}
5767

5868
@interface Icon {

bundle-generator/runtime/src/main/java/io/quarkiverse/operatorsdk/bundle/runtime/CSVMetadataHolder.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package io.quarkiverse.operatorsdk.bundle.runtime;
22

3+
import java.util.Collections;
4+
import java.util.Map;
35
import java.util.Objects;
46

57
public class CSVMetadataHolder {
@@ -11,6 +13,7 @@ public class CSVMetadataHolder {
1113
public final String providerName;
1214
public final String providerURL;
1315
public final String replaces;
16+
public final String[] skips;
1417
public final String version;
1518
public final String maturity;
1619
public final String minKubeVersion;
@@ -38,15 +41,19 @@ public static class Annotations {
3841
public final String categories;
3942
public final boolean certified;
4043
public final String almExamples;
44+
public final String skipRange;
45+
public final Map<String, String> others;
4146

4247
public Annotations(String containerImage, String repository, String capabilities, String categories, boolean certified,
43-
String almExamples) {
48+
String almExamples, String skipRange, Map<String, String> others) {
4449
this.containerImage = containerImage;
4550
this.repository = repository;
4651
this.capabilities = capabilities;
4752
this.categories = categories;
4853
this.certified = certified;
4954
this.almExamples = almExamples;
55+
this.skipRange = skipRange;
56+
this.others = Collections.unmodifiableMap(others);
5057
}
5158
}
5259

@@ -109,20 +116,24 @@ public RequiredCRD(String kind, String name, String version) {
109116

110117
}
111118

119+
@SuppressWarnings("unused")
112120
public CSVMetadataHolder(CSVMetadataHolder other) {
113121
this(other.name, other.description, other.displayName, other.annotations, other.keywords, other.providerName,
114122
other.providerURL,
115-
other.replaces, other.version, other.maturity, other.minKubeVersion, other.maintainers, other.links, other.icon,
123+
other.replaces, other.skips, other.version, other.maturity, other.minKubeVersion,
124+
other.maintainers,
125+
other.links, other.icon,
116126
other.installModes, other.permissionRules, other.requiredCRDs);
117127
}
118128

119129
public CSVMetadataHolder(String name) {
120-
this(name, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null);
130+
this(name, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null);
121131
}
122132

123133
public CSVMetadataHolder(String name, String description, String displayName, Annotations annotations, String[] keywords,
124134
String providerName,
125-
String providerURL, String replaces, String version, String maturity, String minKubeVersion,
135+
String providerURL, String replaces, String[] skips, String version, String maturity,
136+
String minKubeVersion,
126137
Maintainer[] maintainers, Link[] links, Icon[] icon,
127138
InstallMode[] installModes, PermissionRule[] permissionRules, RequiredCRD[] requiredCRDs) {
128139
this.name = name;
@@ -133,6 +144,7 @@ public CSVMetadataHolder(String name, String description, String displayName, An
133144
this.providerName = providerName;
134145
this.providerURL = providerURL;
135146
this.replaces = replaces;
147+
this.skips = skips;
136148
this.version = version;
137149
this.maturity = maturity;
138150
this.minKubeVersion = minKubeVersion;

0 commit comments

Comments
 (0)