Skip to content

Commit 7a944e7

Browse files
csviricsviri
committed
additional tests and improvements
1 parent b379178 commit 7a944e7

File tree

6 files changed

+400
-32
lines changed

6 files changed

+400
-32
lines changed

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

+73-31
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,17 @@
1414
import com.fasterxml.jackson.core.type.TypeReference;
1515
import com.fasterxml.jackson.databind.ObjectMapper;
1616

17+
/**
18+
* Matches the actual state on the server vs the desired state. Based on the managedFields of SSA.
19+
*
20+
* The ide of algorithm is basically trivial, we convert resources to Map/List composition.
21+
* The actual resource (from the server) is pruned, all the fields which are not mentioed in managedFields
22+
* of the target manager is removed. Some irrelevant fields are also removed from desired. And the
23+
* two resulted Maps are compared for equality. The implementation is a bit nasty since have to deal with
24+
* some specific cases of managedFields format.
25+
*
26+
* @param <R> matched resource type
27+
*/
1728
// https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#fieldsv1-v1-meta
1829
// https://github.com/kubernetes-sigs/structured-merge-diff
1930
// https://docs.aws.amazon.com/eks/latest/userguide/kubernetes-field-management.html
@@ -69,7 +80,7 @@ public boolean matches(R actual, R desired, Context<?> context) {
6980

7081
removeIrrelevantValues(desiredMap);
7182

72-
log.trace("Pruned actual: \n {} \n desired: \n {} ", prunedActual, desiredMap);
83+
log.debug("Pruned actual: \n {} \n desired: \n {} ", prunedActual, desiredMap);
7384

7485
return prunedActual.equals(desiredMap);
7586
} catch (JsonProcessingException e) {
@@ -99,22 +110,24 @@ private void pruneActualAccordingManagedFields(Map<String, Object> result,
99110
for (Map.Entry<String, Object> entry : managedFields.entrySet()) {
100111
String key = entry.getKey();
101112
if (key.startsWith(F_PREFIX)) {
102-
String keyInActual = key.substring(2);
113+
String keyInActual = keyWithoutPrefix(key);
103114
var managedFieldValue = entry.getValue();
104-
// basically if we should travers further
105115
if (isNestedValue(managedFieldValue)) {
106116
var managedEntrySet = ((Map<String, Object>) managedFieldValue).entrySet();
107117

118+
// two special cases "k:" and "v:" prefixes
108119
if (isListKeyEntrySet(managedEntrySet)) {
109120
handleListKeyEntrySet(result, actualMap, objectMapper, keyInActual, managedEntrySet);
110121
} else if (isSetValueField(managedEntrySet)) {
111122
handleSetValues(result, actualMap, objectMapper, keyInActual, managedEntrySet);
112123
} else {
124+
// basically if we should travers further
113125
fillResultsAndTraversFurther(result, actualMap, managedFields, objectMapper, key,
114126
keyInActual, managedFieldValue);
115127
}
116128
} else {
117-
// this should handle the case when the value is complex in the actual map (not just a simple value).
129+
// this should handle the case when the value is complex in the actual map (not just a
130+
// simple value).
118131
result.put(keyInActual, actualMap.get(keyInActual));
119132
}
120133
} else {
@@ -124,7 +137,6 @@ private void pruneActualAccordingManagedFields(Map<String, Object> result,
124137
}
125138
}
126139
}
127-
128140
}
129141

130142
private void fillResultsAndTraversFurther(Map<String, Object> result,
@@ -146,31 +158,38 @@ private static boolean isNestedValue(Object managedFieldValue) {
146158

147159
// list entries referenced by key, or when "k:" prefix is used
148160
private void handleListKeyEntrySet(Map<String, Object> result, Map<String, Object> actualMap,
149-
ObjectMapper objectMapper, String keyInActual, Set<Map.Entry<String, Object>> managedEntrySet)
150-
throws JsonProcessingException {
161+
ObjectMapper objectMapper, String keyInActual, Set<Map.Entry<String, Object>> managedEntrySet) {
151162
var valueList = new ArrayList<>();
152163
result.put(keyInActual, valueList);
153-
for (Map.Entry<String, Object> listEntry : managedEntrySet) {
154-
var actualListEntry = selectListEntryBasedOnKey(listEntry.getKey().substring(2),
155-
(List<Map<String, Object>>) actualMap.get(keyInActual), objectMapper);
156-
157-
if (listEntry.getValue() instanceof Map
158-
&& ((Map<String, Object>) listEntry.getValue()).isEmpty()) {
159-
var map = new HashMap<String, Object>();
160-
valueList.add(map);
161-
pruneActualAccordingManagedFields(map, actualListEntry,
162-
(Map<String, Object>) listEntry.getValue(), objectMapper);
163-
continue;
164-
}
164+
var actualValueList = (List<Map<String, Object>>) actualMap.get(keyInActual);
165+
166+
Map<Integer, Map<String, Object>> targetValuesByIndex = new HashMap<>();
167+
Map<Integer, Map<String, Object>> mangedEntryByIndex = new HashMap<>();
165168

169+
for (Map.Entry<String, Object> listEntry : managedEntrySet) {
166170
if (DOT_KEY.equals(listEntry.getKey())) {
167171
continue;
168172
}
169-
var emptyResMapValue = new HashMap<String, Object>();
170-
valueList.add(emptyResMapValue);
171-
pruneActualAccordingManagedFields(emptyResMapValue, actualListEntry,
172-
(Map<String, Object>) listEntry.getValue(), objectMapper);
173+
var actualListEntry = selectListEntryBasedOnKey(keyWithoutPrefix(listEntry.getKey()),
174+
actualValueList, objectMapper);
175+
targetValuesByIndex.put(actualListEntry.getKey(), actualListEntry.getValue());
176+
mangedEntryByIndex.put(actualListEntry.getKey(), (Map<String, Object>) listEntry.getValue());
173177
}
178+
179+
targetValuesByIndex.entrySet()
180+
.stream()
181+
// list is sorted according to the value in actual
182+
.sorted(Map.Entry.comparingByKey())
183+
.forEach(e -> {
184+
var emptyResMapValue = new HashMap<String, Object>();
185+
valueList.add(emptyResMapValue);
186+
try {
187+
pruneActualAccordingManagedFields(emptyResMapValue, e.getValue(),
188+
mangedEntryByIndex.get(e.getKey()), objectMapper);
189+
} catch (JsonProcessingException ex) {
190+
throw new IllegalStateException(ex);
191+
}
192+
});
174193
}
175194

176195
// set values, the "v:" prefix
@@ -184,15 +203,14 @@ private static void handleSetValues(Map<String, Object> result, Map<String, Obje
184203
if (DOT_KEY.equals(valueEntry.getKey())) {
185204
continue;
186205
}
187-
188206
Class<?> targetClass = null;
189207
List values = (List) actualMap.get(keyInActual);
190208
if (!(values.get(0) instanceof Map)) {
191209
targetClass = values.get(0).getClass();
192210
}
193211

194212
var value =
195-
parseKeyValue(valueEntry.getKey().substring(2), targetClass, objectMapper);
213+
parseKeyValue(keyWithoutPrefix(valueEntry.getKey()), targetClass, objectMapper);
196214
valueList.add(value);
197215
}
198216
}
@@ -223,21 +241,26 @@ private boolean isSetValueField(Set<Map.Entry<String, Object>> managedEntrySet)
223241
private boolean isListKeyEntrySet(Set<Map.Entry<String, Object>> managedEntrySet) {
224242
var iterator = managedEntrySet.iterator();
225243
var managedFieldEntry = iterator.next();
226-
// todo unit test
227244
if (managedFieldEntry.getKey().equals(DOT_KEY)) {
228245
managedFieldEntry = iterator.next();
229246
}
230247
return managedFieldEntry.getKey().startsWith(K_PREFIX);
231248
}
232249

233-
private Map<String, Object> selectListEntryBasedOnKey(String key,
250+
private java.util.Map.Entry<Integer, Map<String, Object>> selectListEntryBasedOnKey(String key,
234251
List<Map<String, Object>> values,
235252
ObjectMapper objectMapper) {
236253
try {
237254
Map<String, Object> ids = objectMapper.readValue(key, typeRef);
238-
var possibleTargets =
239-
values.stream().filter(v -> v.entrySet().containsAll(ids.entrySet()))
240-
.collect(Collectors.toList());
255+
List<Map<String, Object>> possibleTargets = new ArrayList<>(1);
256+
int index = -1;
257+
for (int i = 0; i < values.size(); i++) {
258+
var v = values.get(i);
259+
if (v.entrySet().containsAll(ids.entrySet())) {
260+
possibleTargets.add(v);
261+
index = i;
262+
}
263+
}
241264
if (possibleTargets.isEmpty()) {
242265
throw new IllegalStateException(
243266
"Cannot find list element for key:" + key + ", in map: " + values);
@@ -246,8 +269,23 @@ private Map<String, Object> selectListEntryBasedOnKey(String key,
246269
throw new IllegalStateException(
247270
"More targets found in list element for key:" + key + ", in map: " + values);
248271
}
272+
final var finalIndex = index;
273+
return new Map.Entry<>() {
274+
@Override
275+
public Integer getKey() {
276+
return finalIndex;
277+
}
278+
279+
@Override
280+
public Map<String, Object> getValue() {
281+
return possibleTargets.get(0);
282+
}
249283

250-
return possibleTargets.get(0);
284+
@Override
285+
public Map<String, Object> setValue(Map<String, Object> stringObjectMap) {
286+
throw new IllegalStateException("should not be called");
287+
}
288+
};
251289
} catch (JsonProcessingException e) {
252290
throw new IllegalStateException(e);
253291
}
@@ -276,4 +314,8 @@ private Optional<ManagedFieldsEntry> checkIfFieldManagerExists(R actual, String
276314
return Optional.of(targetManagedFields.get(0));
277315
}
278316

317+
private static String keyWithoutPrefix(String key) {
318+
return key.substring(2);
319+
}
320+
279321
}

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java

+24-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ void checksIfAddsNotAddedByController() {
4242
assertThat(matcher.matches(actual, desired, mockedContext)).isTrue();
4343
}
4444

45+
// In the example the owner reference in a list is referenced by "k:", while all the fields are
46+
// managed but not listed
4547
@Test
4648
void emptyListElementMatchesAllFields() {
4749
var desiredConfigMap = loadResource("configmap.empty-owner-reference-desired.yaml",
@@ -52,7 +54,28 @@ void emptyListElementMatchesAllFields() {
5254
assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isTrue();
5355
}
5456

55-
// todo test lists with more entries
57+
58+
// the whole "rules:" part is just implicitly managed
59+
@Test
60+
void wholeComplexFieldManaged() {
61+
var desiredConfigMap = loadResource("sample-whole-complex-part-managed-desired.yaml",
62+
ConfigMap.class);
63+
var actualConfigMap = loadResource("sample-whole-complex-part-managed.yaml",
64+
ConfigMap.class);
65+
66+
assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isTrue();
67+
}
68+
69+
70+
@Test
71+
void multiItemList() {
72+
var desiredConfigMap = loadResource("multi-container-pod-desired.yaml",
73+
ConfigMap.class);
74+
var actualConfigMap = loadResource("multi-container-pod.yaml",
75+
ConfigMap.class);
76+
77+
assertThat(matcher.matches(actualConfigMap, desiredConfigMap, mockedContext)).isTrue();
78+
}
5679

5780
private <R> R loadResource(String fileName, Class<R> clazz) {
5881
return ReconcilerUtils.loadYaml(clazz, SSABasedGenericKubernetesResourceMatcherTest.class,

‎operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/multi-container-pod-desired.yaml

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
apiVersion: v1
2+
kind: Pod
3+
metadata:
4+
name: shared-storage
5+
spec:
6+
volumes:
7+
- name: shared-data
8+
emptyDir: {}
9+
containers:
10+
- name: nginx-container
11+
image: nginx
12+
volumeMounts:
13+
- name: shared-data
14+
mountPath: /usr/share/nginx/html
15+
- name: debian-container
16+
image: debian
17+
volumeMounts:
18+
- name: shared-data
19+
mountPath: /data
20+
command: ["/bin/sh"]
21+
args: ["-c", "echo Level Up Blue Team! > /data/index.html"]

0 commit comments

Comments
 (0)