Skip to content

Commit bbb326b

Browse files
myakovemyakove
authored
fix: identify_missing_resources uses (group, kind) pairs for comparison (#2667)
* fix: identify_missing_resources uses (group, kind) pairs for comparison Fixes #2666 * fix: Append missing group variants when allow_updates=False * fix: Guard group normalization and check all GVK entries in variant dedup
1 parent b0dcf28 commit bbb326b

2 files changed

Lines changed: 259 additions & 44 deletions

File tree

‎class_generator/core/schema.py‎

Lines changed: 86 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ def identify_missing_resources(client: str, existing_resources_mapping: dict[Any
163163
"""
164164
Identify resources that exist in the cluster but are missing from our mapping.
165165
166+
Compares using (group, kind) pairs so that resources with the same kind but
167+
different API groups (e.g. velero.io/Backup vs postgresql.cnpg.noobaa.io/Backup)
168+
are tracked independently.
169+
166170
Args:
167171
client: Path to kubectl/oc client binary
168172
existing_resources_mapping: Current resources mapping
@@ -172,57 +176,83 @@ def identify_missing_resources(client: str, existing_resources_mapping: dict[Any
172176
"""
173177
missing_resources: set[str] = set()
174178

175-
# Get all available resources from cluster
179+
# Get all available resources from cluster with their API groups
176180
cmd = f"{client} api-resources --no-headers"
177181
success, output, _ = run_command(command=shlex.split(cmd), check=False, log_errors=False)
178182

179183
if not success or not output:
180184
LOGGER.debug("Failed to get api-resources from cluster")
181185
return missing_resources
182186

183-
# Extract kinds from cluster resources
184-
cluster_resources = set()
187+
# Build kind -> set of groups from cluster resources
188+
cluster_kind_groups: dict[str, set[str]] = {}
185189
lines = output.strip().split("\n")
186190

187191
for line in lines:
188192
parts = [p for p in line.split() if p]
189-
if parts:
190-
kind = parts[-1] # KIND is the last column
191-
cluster_resources.add(kind)
193+
if len(parts) < 3:
194+
continue
192195

193-
# Find missing resources by comparing with existing mapping
194-
existing_kinds = set()
195-
for _, resource_schemas in existing_resources_mapping.items():
196-
# Verify resource_schemas is a non-empty list
197-
if not isinstance(resource_schemas, list) or not resource_schemas:
196+
# Find APIVERSION column by looking for version patterns
197+
# (same logic as build_dynamic_resource_to_api_mapping)
198+
apiversion_idx = None
199+
for i, part in enumerate(parts):
200+
if ("/" in part and ("v" in part or "alpha" in part or "beta" in part)) or (
201+
part.startswith("v") and part[1:].replace(".", "").replace("alpha", "").replace("beta", "").isdigit()
202+
):
203+
apiversion_idx = i
204+
break
205+
206+
if apiversion_idx is None:
198207
continue
199208

200-
# Verify first_schema is a dict
201-
first_schema = resource_schemas[0]
202-
if not isinstance(first_schema, dict):
203-
LOGGER.debug(f"Skipping malformed schema: first_schema is not a dict, got {type(first_schema)}")
209+
# KIND is 2 columns after APIVERSION (APIVERSION, NAMESPACED, KIND)
210+
if len(parts) <= apiversion_idx + 2:
204211
continue
205212

206-
# Get gvk_list via .get and ensure it is a list
207-
gvk_list = first_schema.get("x-kubernetes-group-version-kind")
208-
if not isinstance(gvk_list, list) or not gvk_list:
209-
LOGGER.debug(
210-
f"Skipping malformed schema: x-kubernetes-group-version-kind is not a non-empty list, got {type(gvk_list)}"
211-
)
213+
apiversion = parts[apiversion_idx]
214+
kind = parts[apiversion_idx + 2]
215+
216+
# Extract group from apiversion: "velero.io/v1" -> "velero.io", "v1" -> ""
217+
group = apiversion.rsplit("/", 1)[0] if "/" in apiversion else ""
218+
219+
cluster_kind_groups.setdefault(kind, set()).add(group)
220+
221+
# Build kind -> set of groups from existing mappings
222+
existing_kind_groups: dict[str, set[str]] = {}
223+
for _, resource_schemas in existing_resources_mapping.items():
224+
if not isinstance(resource_schemas, list) or not resource_schemas:
212225
continue
213226

214-
# Iterate gvk_list items and for each item confirm it's a dict before calling .get("kind")
215-
for gvk_item in gvk_list:
216-
if not isinstance(gvk_item, dict):
217-
LOGGER.debug(f"Skipping malformed gvk item: not a dict, got {type(gvk_item)}")
227+
# Check ALL schemas in the list, not just the first one
228+
for schema in resource_schemas:
229+
if not isinstance(schema, dict):
218230
continue
219231

220-
kind = gvk_item.get("kind")
221-
# Check that kind is a non-empty string before adding to existing_kinds
222-
if isinstance(kind, str) and kind.strip():
223-
existing_kinds.add(kind.strip())
232+
gvk_list = schema.get("x-kubernetes-group-version-kind")
233+
if not isinstance(gvk_list, list) or not gvk_list:
234+
continue
235+
236+
for gvk_item in gvk_list:
237+
if not isinstance(gvk_item, dict):
238+
continue
239+
240+
kind = gvk_item.get("kind")
241+
if not isinstance(kind, str) or not kind.strip():
242+
continue
243+
244+
kind = kind.strip()
245+
group_raw = gvk_item.get("group", "")
246+
group = group_raw.strip() if isinstance(group_raw, str) else ""
247+
248+
existing_kind_groups.setdefault(kind, set()).add(group)
249+
250+
# Find missing resources: kind is missing if any cluster group is not in existing groups
251+
for kind, cluster_groups in cluster_kind_groups.items():
252+
existing_groups = existing_kind_groups.get(kind, set())
253+
if not cluster_groups.issubset(existing_groups):
254+
missing_resources.add(kind)
224255

225-
missing_resources = cluster_resources - existing_kinds
226256
LOGGER.info(f"Found {len(missing_resources)} missing resources: {sorted(list(missing_resources)[:10])}")
227257

228258
return missing_resources
@@ -661,9 +691,32 @@ def process_schema_definitions(
661691
f"Added new schema version for existing resource: {kind_lower} ({group or 'core'}/{version})"
662692
)
663693
else:
664-
# Don't update existing resources when cluster version is older
665-
LOGGER.debug(f"Skipping update for existing resource: {kind_lower}")
666-
continue
694+
# Older cluster: keep existing schemas untouched, but still add
695+
# missing group/version variants for existing kinds.
696+
existing_schemas = resources_mapping.get(kind_lower, [])
697+
variant_exists = False
698+
for existing_schema in existing_schemas:
699+
if not isinstance(existing_schema, dict):
700+
continue
701+
existing_gvk_list = existing_schema.get("x-kubernetes-group-version-kind") or []
702+
for existing_gvk in existing_gvk_list:
703+
if not isinstance(existing_gvk, dict):
704+
continue
705+
if existing_gvk.get("group") == group and existing_gvk.get("version") == version:
706+
variant_exists = True
707+
break
708+
if variant_exists:
709+
break
710+
711+
if variant_exists:
712+
LOGGER.debug(f"Skipping update for existing resource: {kind_lower}")
713+
continue
714+
715+
existing_schemas.append(schema_data)
716+
updated_resources += 1
717+
LOGGER.debug(
718+
f"Added missing schema variant for existing resource: {kind_lower} ({group or 'core'}/{version})"
719+
)
667720

668721
# Also store in definitions for separate definitions file
669722
definitions[schema_name] = schema_data

‎class_generator/tests/test_schema_new_functions.py‎

Lines changed: 173 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -306,16 +306,16 @@ class TestIdentifyMissingResources:
306306
@patch("class_generator.core.schema.run_command")
307307
def test_successful_missing_resources_identification(self, mock_run_command):
308308
"""Test successful identification of missing resources."""
309-
api_resources_output = """pods po v1 Pod
310-
services svc v1 Service
311-
deployments deploy apps/v1 Deployment
312-
configmaps cm v1 ConfigMap"""
309+
api_resources_output = """pods po v1 true Pod
310+
services svc v1 true Service
311+
deployments deploy apps/v1 true Deployment
312+
configmaps cm v1 true ConfigMap"""
313313

314314
mock_run_command.return_value = (True, api_resources_output, "")
315315

316316
existing_mapping = {
317-
"pod": [{"x-kubernetes-group-version-kind": [{"kind": "Pod"}]}],
318-
"service": [{"x-kubernetes-group-version-kind": [{"kind": "Service"}]}],
317+
"pod": [{"x-kubernetes-group-version-kind": [{"group": "", "version": "v1", "kind": "Pod"}]}],
318+
"service": [{"x-kubernetes-group-version-kind": [{"group": "", "version": "v1", "kind": "Service"}]}],
319319
}
320320

321321
result = identify_missing_resources("kubectl", existing_mapping)
@@ -333,14 +333,14 @@ def test_api_resources_command_failure(self, mock_run_command):
333333
@patch("class_generator.core.schema.run_command")
334334
def test_no_missing_resources(self, mock_run_command):
335335
"""Test when no resources are missing."""
336-
api_resources_output = """pods po v1 Pod
337-
services svc v1 Service"""
336+
api_resources_output = """pods po v1 true Pod
337+
services svc v1 true Service"""
338338

339339
mock_run_command.return_value = (True, api_resources_output, "")
340340

341341
existing_mapping = {
342-
"pod": [{"x-kubernetes-group-version-kind": [{"kind": "Pod"}]}],
343-
"service": [{"x-kubernetes-group-version-kind": [{"kind": "Service"}]}],
342+
"pod": [{"x-kubernetes-group-version-kind": [{"group": "", "version": "v1", "kind": "Pod"}]}],
343+
"service": [{"x-kubernetes-group-version-kind": [{"group": "", "version": "v1", "kind": "Service"}]}],
344344
}
345345

346346
result = identify_missing_resources("kubectl", existing_mapping)
@@ -349,7 +349,9 @@ def test_no_missing_resources(self, mock_run_command):
349349
@patch("class_generator.core.schema.run_command")
350350
def test_malformed_existing_mapping_handled(self, mock_run_command):
351351
"""Test that malformed existing mapping is handled gracefully."""
352-
api_resources_output = "pods po v1 Pod"
352+
api_resources_output = (
353+
"pods po v1 true Pod"
354+
)
353355
mock_run_command.return_value = (True, api_resources_output, "")
354356

355357
# Malformed mapping - missing required structure
@@ -359,6 +361,166 @@ def test_malformed_existing_mapping_handled(self, mock_run_command):
359361
assert "Pod" in result
360362

361363

364+
class TestIdentifyMissingResourcesDuplicateKinds:
365+
"""Test identify_missing_resources with duplicate kinds across API groups."""
366+
367+
CLUSTER_OUTPUT_DUPLICATE_BACKUP = (
368+
"configmaps cm v1 true ConfigMap\n"
369+
"backups velero.io/v1 true Backup\n"
370+
"backups postgresql.cnpg.noobaa.io/v1 true Backup\n"
371+
"widgets example.com/v1 true Widget"
372+
)
373+
374+
@pytest.mark.parametrize(
375+
"test_id, existing_mapping, expected_missing",
376+
[
377+
pytest.param(
378+
"duplicate_kind_one_group_covered",
379+
{
380+
"backup": [
381+
{
382+
"x-kubernetes-group-version-kind": [
383+
{"group": "postgresql.cnpg.noobaa.io", "version": "v1", "kind": "Backup"}
384+
],
385+
"namespaced": True,
386+
}
387+
],
388+
"configmap": [
389+
{
390+
"x-kubernetes-group-version-kind": [{"group": "", "version": "v1", "kind": "ConfigMap"}],
391+
"namespaced": True,
392+
}
393+
],
394+
},
395+
{"Backup", "Widget"},
396+
id="duplicate_kind_one_group_covered",
397+
),
398+
pytest.param(
399+
"duplicate_kind_all_groups_covered",
400+
{
401+
"backup_velero": [
402+
{
403+
"x-kubernetes-group-version-kind": [
404+
{"group": "velero.io", "version": "v1", "kind": "Backup"}
405+
],
406+
"namespaced": True,
407+
}
408+
],
409+
"backup_cnpg": [
410+
{
411+
"x-kubernetes-group-version-kind": [
412+
{"group": "postgresql.cnpg.noobaa.io", "version": "v1", "kind": "Backup"}
413+
],
414+
"namespaced": True,
415+
}
416+
],
417+
"configmap": [
418+
{
419+
"x-kubernetes-group-version-kind": [{"group": "", "version": "v1", "kind": "ConfigMap"}],
420+
"namespaced": True,
421+
}
422+
],
423+
"widget": [
424+
{
425+
"x-kubernetes-group-version-kind": [
426+
{"group": "example.com", "version": "v1", "kind": "Widget"}
427+
],
428+
"namespaced": True,
429+
}
430+
],
431+
},
432+
set(),
433+
id="duplicate_kind_all_groups_covered",
434+
),
435+
pytest.param(
436+
"completely_new_kind",
437+
{
438+
"configmap": [
439+
{
440+
"x-kubernetes-group-version-kind": [{"group": "", "version": "v1", "kind": "ConfigMap"}],
441+
"namespaced": True,
442+
}
443+
],
444+
},
445+
{"Backup", "Widget"},
446+
id="completely_new_kind",
447+
),
448+
pytest.param(
449+
"core_api_resource_covered",
450+
{
451+
"backup_velero": [
452+
{
453+
"x-kubernetes-group-version-kind": [
454+
{"group": "velero.io", "version": "v1", "kind": "Backup"}
455+
],
456+
"namespaced": True,
457+
}
458+
],
459+
"backup_cnpg": [
460+
{
461+
"x-kubernetes-group-version-kind": [
462+
{"group": "postgresql.cnpg.noobaa.io", "version": "v1", "kind": "Backup"}
463+
],
464+
"namespaced": True,
465+
}
466+
],
467+
"configmap": [
468+
{
469+
"x-kubernetes-group-version-kind": [{"group": "", "version": "v1", "kind": "ConfigMap"}],
470+
"namespaced": True,
471+
}
472+
],
473+
},
474+
{"Widget"},
475+
id="core_api_resource_covered",
476+
),
477+
],
478+
)
479+
@patch("class_generator.core.schema.run_command")
480+
def test_identify_missing_resources_duplicate_kinds(
481+
self,
482+
mock_run_command,
483+
test_id,
484+
existing_mapping,
485+
expected_missing,
486+
):
487+
"""Test identify_missing_resources correctly handles duplicate kinds across API groups."""
488+
mock_run_command.return_value = (True, self.CLUSTER_OUTPUT_DUPLICATE_BACKUP, "")
489+
490+
result = identify_missing_resources("oc", existing_mapping)
491+
assert result == expected_missing, f"[{test_id}] Expected {expected_missing}, got {result}"
492+
493+
def test_allow_updates_false_appends_missing_group_variant(self):
494+
"""Test that allow_updates=False still appends missing group variants for existing kinds."""
495+
schemas = {
496+
"apis/postgresql.cnpg.noobaa.io/v1": {
497+
"components": {
498+
"schemas": {
499+
"io.example.Backup": {
500+
"type": "object",
501+
"x-kubernetes-group-version-kind": [
502+
{"group": "postgresql.cnpg.noobaa.io", "version": "v1", "kind": "Backup"}
503+
],
504+
}
505+
}
506+
}
507+
}
508+
}
509+
existing_mapping = {
510+
"backup": [{"x-kubernetes-group-version-kind": [{"group": "velero.io", "version": "v1", "kind": "Backup"}]}]
511+
}
512+
513+
resources_mapping, _ = process_schema_definitions(
514+
schemas=schemas,
515+
namespacing_dict={"Backup": True},
516+
existing_resources_mapping=existing_mapping,
517+
allow_updates=False,
518+
)
519+
520+
groups = {item["x-kubernetes-group-version-kind"][0].get("group", "") for item in resources_mapping["backup"]}
521+
assert groups == {"velero.io", "postgresql.cnpg.noobaa.io"}
522+
523+
362524
class TestBuildDynamicResourceToApiMapping:
363525
"""Test build_dynamic_resource_to_api_mapping function."""
364526

0 commit comments

Comments
 (0)