Skip to content

Commit 93bc9c4

Browse files
csvirimetacosm
csviri
and
metacosm
committed
improve: mapping from annotation depends on type (#2606)
* improve: mapping from annotation depends on type Signed-off-by: Attila Mészáros <[email protected]> compiles Signed-off-by: Attila Mészáros <[email protected]> * refactor: avoid creating useless strings Signed-off-by: Chris Laprun <[email protected]> * refactor: ensure roundtrip works Signed-off-by: Chris Laprun <[email protected]> * refactor: rename toSimpleString to toGVKString for greater clarity Signed-off-by: Chris Laprun <[email protected]> * refactor: minor improvements Signed-off-by: Chris Laprun <[email protected]> --------- Signed-off-by: Attila Mészáros <[email protected]> Signed-off-by: Chris Laprun <[email protected]> Co-authored-by: Chris Laprun <[email protected]>
1 parent a0273f8 commit 93bc9c4

File tree

7 files changed

+92
-33
lines changed

7 files changed

+92
-33
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/GroupVersionKind.java‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import io.fabric8.kubernetes.api.model.HasMetadata;
88

99
public class GroupVersionKind {
10+
private static final String SEPARATOR = "/";
1011
private final String group;
1112
private final String version;
1213
private final String kind;
@@ -16,7 +17,7 @@ public class GroupVersionKind {
1617

1718
public GroupVersionKind(String apiVersion, String kind) {
1819
this.kind = kind;
19-
String[] groupAndVersion = apiVersion.split("/");
20+
String[] groupAndVersion = apiVersion.split(SEPARATOR);
2021
if (groupAndVersion.length == 1) {
2122
this.group = null;
2223
this.version = groupAndVersion[0];
@@ -40,24 +41,24 @@ public GroupVersionKind(String group, String version, String kind) {
4041
this.group = group;
4142
this.version = version;
4243
this.kind = kind;
43-
this.apiVersion = (group == null || group.isBlank()) ? version : group + "/" + version;
44+
this.apiVersion = (group == null || group.isBlank()) ? version : group + SEPARATOR + version;
4445
}
4546

4647
/**
4748
* Parse GVK from a String representation. Expected format is: [group]/[version]/[kind]
48-
*
49+
*
4950
* <pre>
5051
* Sample: "apps/v1/Deployment"
5152
* </pre>
52-
*
53+
*
5354
* or: [version]/[kind]
54-
*
55+
*
5556
* <pre>
5657
* Sample: v1/ConfigMap
5758
* </pre>
5859
**/
5960
public static GroupVersionKind fromString(String gvk) {
60-
String[] parts = gvk.split("/");
61+
String[] parts = gvk.split(SEPARATOR);
6162
if (parts.length == 3) {
6263
return new GroupVersionKind(parts[0], parts[1], parts[2]);
6364
} else if (parts.length == 2) {
@@ -68,6 +69,19 @@ public static GroupVersionKind fromString(String gvk) {
6869
}
6970
}
7071

72+
/**
73+
* Reverse to {@link #fromString(String)}.
74+
*
75+
* @return gvk encoded in simple string.
76+
*/
77+
public String toGVKString() {
78+
if (group != null) {
79+
return group + SEPARATOR + version + SEPARATOR + kind;
80+
} else {
81+
return version + SEPARATOR + kind;
82+
}
83+
}
84+
7185
public String getGroup() {
7286
return group;
7387
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import io.javaoperatorsdk.operator.api.reconciler.Ignore;
2020
import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected;
2121
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ConfiguredDependentResource;
22+
import io.javaoperatorsdk.operator.processing.GroupVersionKind;
2223
import io.javaoperatorsdk.operator.processing.dependent.AbstractEventSourceHolderDependentResource;
2324
import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
2425
import io.javaoperatorsdk.operator.processing.event.ResourceID;
@@ -208,17 +209,18 @@ private boolean useNonOwnerRefBasedSecondaryToPrimaryMapping() {
208209

209210
protected void addSecondaryToPrimaryMapperAnnotations(R desired, P primary) {
210211
addSecondaryToPrimaryMapperAnnotations(desired, primary, Mappers.DEFAULT_ANNOTATION_FOR_NAME,
211-
Mappers.DEFAULT_ANNOTATION_FOR_NAMESPACE);
212+
Mappers.DEFAULT_ANNOTATION_FOR_NAMESPACE, Mappers.DEFAULT_ANNOTATION_FOR_PRIMARY_TYPE);
212213
}
213214

214215
protected void addSecondaryToPrimaryMapperAnnotations(R desired, P primary, String nameKey,
215-
String namespaceKey) {
216+
String namespaceKey, String typeKey) {
216217
var annotations = desired.getMetadata().getAnnotations();
217218
annotations.put(nameKey, primary.getMetadata().getName());
218219
var primaryNamespaces = primary.getMetadata().getNamespace();
219220
if (primaryNamespaces != null) {
220221
annotations.put(namespaceKey, primary.getMetadata().getNamespace());
221222
}
223+
annotations.put(typeKey, GroupVersionKind.gvkFor(primary.getClass()).toGVKString());
222224
}
223225

224226
@Override
@@ -274,7 +276,7 @@ protected Optional<SecondaryToPrimaryMapper<R>> getSecondaryToPrimaryMapper(
274276
return Optional
275277
.of(Mappers.fromOwnerReferences(context.getPrimaryResourceClass(), clustered));
276278
} else if (isCreatable()) {
277-
return Optional.of(Mappers.fromDefaultAnnotations());
279+
return Optional.of(Mappers.fromDefaultAnnotations(context.getPrimaryResourceClass()));
278280
}
279281
}
280282
return Optional.empty();

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/Mappers.java‎

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import java.util.stream.Collectors;
66

77
import io.fabric8.kubernetes.api.model.HasMetadata;
8+
import io.javaoperatorsdk.operator.processing.GroupVersionKind;
89
import io.javaoperatorsdk.operator.processing.event.ResourceID;
910
import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper;
1011

@@ -13,33 +14,41 @@ public class Mappers {
1314
public static final String DEFAULT_ANNOTATION_FOR_NAME = "io.javaoperatorsdk/primary-name";
1415
public static final String DEFAULT_ANNOTATION_FOR_NAMESPACE =
1516
"io.javaoperatorsdk/primary-namespace";
17+
public static final String DEFAULT_ANNOTATION_FOR_PRIMARY_TYPE =
18+
"io.javaoperatorsdk/primary-type";
1619

1720
private Mappers() {}
1821

1922
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromAnnotation(
20-
String nameKey) {
21-
return fromMetadata(nameKey, null, false);
23+
String nameKey, String typeKey, Class<? extends HasMetadata> primaryResourceType) {
24+
return fromAnnotation(nameKey, null, typeKey, primaryResourceType);
2225
}
2326

2427
@SuppressWarnings("unused")
2528
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromAnnotation(
26-
String nameKey, String namespaceKey) {
27-
return fromMetadata(nameKey, namespaceKey, false);
29+
String nameKey, String namespaceKey, String typeKey,
30+
Class<? extends HasMetadata> primaryResourceType) {
31+
return fromMetadata(nameKey, namespaceKey, typeKey, primaryResourceType, false);
2832
}
2933

3034
@SuppressWarnings("unused")
31-
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(String nameKey) {
32-
return fromMetadata(nameKey, null, true);
35+
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(String nameKey,
36+
String typeKey,
37+
Class<? extends HasMetadata> primaryResourceType) {
38+
return fromLabel(nameKey, null, typeKey, primaryResourceType);
3339
}
3440

35-
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromDefaultAnnotations() {
36-
return fromMetadata(DEFAULT_ANNOTATION_FOR_NAME, DEFAULT_ANNOTATION_FOR_NAMESPACE, false);
41+
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromDefaultAnnotations(
42+
Class<? extends HasMetadata> primaryResourceType) {
43+
return fromAnnotation(DEFAULT_ANNOTATION_FOR_NAME, DEFAULT_ANNOTATION_FOR_NAMESPACE,
44+
DEFAULT_ANNOTATION_FOR_PRIMARY_TYPE, primaryResourceType);
3745
}
3846

3947
@SuppressWarnings("unused")
4048
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(
41-
String nameKey, String namespaceKey) {
42-
return fromMetadata(nameKey, namespaceKey, true);
49+
String nameKey, String namespaceKey, String typeKey,
50+
Class<? extends HasMetadata> primaryResourceType) {
51+
return fromMetadata(nameKey, namespaceKey, typeKey, primaryResourceType, true);
4352
}
4453

4554
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
@@ -78,7 +87,8 @@ public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerRefer
7887
}
7988

8089
private static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromMetadata(
81-
String nameKey, String namespaceKey, boolean isLabel) {
90+
String nameKey, String namespaceKey, String typeKey,
91+
Class<? extends HasMetadata> primaryResourceType, boolean isLabel) {
8292
return resource -> {
8393
final var metadata = resource.getMetadata();
8494
if (metadata == null) {
@@ -94,6 +104,15 @@ private static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromMetadata(
94104
}
95105
var namespace =
96106
namespaceKey == null ? resource.getMetadata().getNamespace() : map.get(namespaceKey);
107+
108+
String gvkSimple = map.get(typeKey);
109+
110+
if (gvkSimple != null &&
111+
!GroupVersionKind.fromString(gvkSimple)
112+
.equals(GroupVersionKind.gvkFor(primaryResourceType))) {
113+
return Set.of();
114+
}
115+
97116
return Set.of(new ResourceID(name, namespace));
98117
}
99118
};
@@ -105,14 +124,11 @@ public static ResourceID fromString(String cacheKey) {
105124
}
106125

107126
final String[] split = cacheKey.split("/");
108-
switch (split.length) {
109-
case 1:
110-
return new ResourceID(split[0]);
111-
case 2:
112-
return new ResourceID(split[1], split[0]);
113-
default:
114-
throw new IllegalArgumentException("Cannot extract a ResourceID from " + cacheKey);
115-
}
127+
return switch (split.length) {
128+
case 1 -> new ResourceID(split[0]);
129+
case 2 -> new ResourceID(split[1], split[0]);
130+
default -> throw new IllegalArgumentException("Cannot extract a ResourceID from " + cacheKey);
131+
};
116132
}
117133

118134
/**
@@ -139,9 +155,17 @@ public static <OWNER extends HasMetadata, T extends HasMetadata> SecondaryToPrim
139155

140156
public static class SecondaryToPrimaryFromDefaultAnnotation
141157
implements SecondaryToPrimaryMapper<HasMetadata> {
158+
159+
private final Class<? extends HasMetadata> primaryResourceType;
160+
161+
public SecondaryToPrimaryFromDefaultAnnotation(
162+
Class<? extends HasMetadata> primaryResourceType) {
163+
this.primaryResourceType = primaryResourceType;
164+
}
165+
142166
@Override
143167
public Set<ResourceID> toPrimaryResourceIDs(HasMetadata resource) {
144-
return Mappers.fromDefaultAnnotations().toPrimaryResourceIDs(resource);
168+
return Mappers.fromDefaultAnnotations(primaryResourceType).toPrimaryResourceIDs(resource);
145169
}
146170
}
147171

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,4 +75,16 @@ void pluralShouldOverrideDefaultComputedVersionIfProvided() {
7575
"MyPlural");
7676
assertThat(gvk.getPlural()).hasValue("MyPlural");
7777
}
78+
79+
@Test
80+
void encodesToGVKString() {
81+
final var deploymentGVK = "apps/v1/Deployment";
82+
var gvk = GroupVersionKind.fromString(deploymentGVK);
83+
assertThat(gvk.toGVKString()).isEqualTo(deploymentGVK);
84+
assertThat(gvk).isEqualTo(GroupVersionKind.fromString(gvk.toGVKString()));
85+
86+
gvk = GroupVersionKind.fromString("v1/ConfigMap");
87+
assertThat(gvk.toGVKString()).isEqualTo("v1/ConfigMap");
88+
assertThat(gvk).isEqualTo(GroupVersionKind.fromString(gvk.toGVKString()));
89+
}
7890
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informereventsource/InformerEventSourceTestCustomReconciler.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public class InformerEventSourceTestCustomReconciler
2626
LoggerFactory.getLogger(InformerEventSourceTestCustomReconciler.class);
2727

2828
public static final String RELATED_RESOURCE_NAME = "relatedResourceName";
29+
public static final String RELATED_RESOURCE_TYPE = "relatedResourceType";
2930
public static final String TARGET_CONFIG_MAP_KEY = "targetStatus";
3031
public static final String MISSING_CONFIG_MAP = "Missing Config Map";
3132

@@ -38,7 +39,9 @@ public List<EventSource<?, InformerEventSourceTestCustomResource>> prepareEventS
3839
InformerEventSourceConfiguration<ConfigMap> config =
3940
InformerEventSourceConfiguration
4041
.from(ConfigMap.class, InformerEventSourceTestCustomResource.class)
41-
.withSecondaryToPrimaryMapper(Mappers.fromAnnotation(RELATED_RESOURCE_NAME))
42+
.withSecondaryToPrimaryMapper(
43+
Mappers.fromAnnotation(RELATED_RESOURCE_NAME, RELATED_RESOURCE_TYPE,
44+
InformerEventSourceTestCustomResource.class))
4245
.build();
4346

4447
return List.of(new InformerEventSource<>(config, context));

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerremotecluster/InformerRemoteClusterReconciler.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ public List<EventSource<?, InformerRemoteClusterCustomResource>> prepareEventSou
5353
.from(ConfigMap.class, InformerRemoteClusterCustomResource.class)
5454
// owner references do not work cross cluster, using
5555
// annotations here to reference primary resource
56-
.withSecondaryToPrimaryMapper(Mappers.fromDefaultAnnotations())
56+
.withSecondaryToPrimaryMapper(
57+
Mappers.fromDefaultAnnotations(InformerRemoteClusterCustomResource.class))
5758
// setting remote client for informer
5859
.withKubernetesClient(remoteClient)
5960
.withInformerConfiguration(

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/dependentcustommappingannotation/CustomMappingConfigMapDependentResource.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@ public class CustomMappingConfigMapDependentResource
2020

2121
public static final String CUSTOM_NAME_KEY = "customNameKey";
2222
public static final String CUSTOM_NAMESPACE_KEY = "customNamespaceKey";
23+
public static final String CUSTOM_TYPE_KEY = "customTypeKey";
2324
public static final String KEY = "key";
2425

2526
private static final SecondaryToPrimaryMapper<ConfigMap> mapper =
26-
Mappers.fromAnnotation(CUSTOM_NAME_KEY, CUSTOM_NAMESPACE_KEY);
27+
Mappers.fromAnnotation(CUSTOM_NAME_KEY, CUSTOM_NAMESPACE_KEY, CUSTOM_TYPE_KEY,
28+
DependentCustomMappingCustomResource.class);
2729

2830
public CustomMappingConfigMapDependentResource() {
2931
super(ConfigMap.class);
@@ -44,7 +46,8 @@ protected ConfigMap desired(DependentCustomMappingCustomResource primary,
4446
@Override
4547
protected void addSecondaryToPrimaryMapperAnnotations(ConfigMap desired,
4648
DependentCustomMappingCustomResource primary) {
47-
addSecondaryToPrimaryMapperAnnotations(desired, primary, CUSTOM_NAME_KEY, CUSTOM_NAMESPACE_KEY);
49+
addSecondaryToPrimaryMapperAnnotations(desired, primary, CUSTOM_NAME_KEY, CUSTOM_NAMESPACE_KEY,
50+
CUSTOM_TYPE_KEY);
4851
}
4952

5053

0 commit comments

Comments
 (0)