Fix: Aws secondary key issue

docs/source_aws.md

@@ -104,6 +104,16 @@ For example, the following supplemental policy enables all three:
 }
 ```
 
+The policy includes `s3:GetBucketTagging` for optional S3 tag synchronization.
+If bucket tagging is desired, grant this permission to the inventory role used
+by your AWS profile (or every selected member-account role in organization mode)
+and configure `tag_keys` to select the tags to copy to NetBox.
+
+**Current S3 discovery always requests bucket tags**, even when `tag_keys` is
+empty. Omitting `s3:GetBucketTagging` can therefore cause an authorization denial
+that skips the affected bucket and postpones orphaning/pruning for this source,
+as described below; it does not simply disable tag synchronization.
+
 Where your IAM policy supports resource-level permissions, restrict the
 resource-specific actions to the intended buckets, functions, and EKS resources.
 Listing actions still need their documented wildcard scope. No S3 object reads,

module/netbox/object_classes.py

@@ -874,6 +874,11 @@ def get_display_name(self, data=None, including_second_key=False):
                 read_from_netbox = True if secondary_key_value.get("id", 0) != 0 else False
                 secondary_key_value = self.get_display_name(data=secondary_key_value)
 
+            org_secondary_key_value = str(secondary_key_value)
+
+            if secondary_key_value is None and include_secondary_key_if_present:
+                return my_name
+
             if secondary_key_value is None and read_from_netbox is False and include_secondary_key_if_present is False:
                 log.warning(f"Unable to determine second key '{secondary_key}' for {self.name} '{my_name}', "
                             f"got: {org_secondary_key_value!r}")
@@ -1935,6 +1940,7 @@ class NBCluster(NetBoxObject):
     object_type = "virtualization.cluster"
     primary_key = "name"
     secondary_key = "scope_id"
+    include_secondary_key_if_present = True
     prune = False
 
     def __init__(self, *args, **kwargs):

module/sources/aws/connection.py

@@ -346,6 +346,23 @@ def service_object(self, object_type, identity, data):
         fields["aws_metadata"] = json_safe(fields["aws_metadata"])
         matches = [item for item in self.inventory.get_all_items(object_type)
                    if (item.data.get("custom_fields") or {}).get("aws_resource_id") == identity]
+        parent = "virtual_machine" if object_type is NBVirtualDisk else "cluster" if object_type is NBVM else "group"
+        if not matches:
+            name_matches = [item for item in items
+                            if item.data.get("name") == data["name"] and
+                            item.data.get(parent) is data.get(parent)]
+            if len(name_matches) == 1:
+                candidate = name_matches[0]
+                fields = candidate.data.get("custom_fields") or {}
+                candidate_identity = fields.get("aws_resource_id")
+                candidate_service = fields.get("aws_service")
+                identity_missing = candidate_identity is None or (
+                    isinstance(candidate_identity, str) and not candidate_identity.strip())
+                service = (data.get("custom_fields") or {}).get("aws_service")
+                service_matches = candidate_service in (None, "", service)
+                source_managed = candidate.source is self or self.source_tag in candidate.get_tags()
+                if identity_missing and service_matches and source_managed:
+                    matches = [candidate]
         if len(matches) > 1:
             raise ValueError(f"Multiple NetBox {object_type.name} objects match AWS resource '{identity}'")
         if matches:
@@ -354,9 +371,8 @@ def service_object(self, object_type, identity, data):
                 raise ValueError(f"AWS resource '{identity}' belongs to another source")
             item.update(data=data, source=self)
         else:
-            parent = "virtual_machine" if object_type is NBVirtualDisk else "cluster" if object_type is NBVM else "group"
             if any(item.data.get("name") == data["name"] and item.data.get(parent) is data.get(parent)
-                   for item in self.inventory.get_all_items(object_type)):
+                   for item in items):
                 raise ValueError(f"NetBox {object_type.name} name collision for AWS resource '{identity}'")
             item = self.inventory.add_object(object_type, data=data, source=self)
         self.reconcile_tags(item, data["tags"])

tests/test_aws_services.py

@@ -293,10 +293,13 @@ def test_s3_conflict_skips_dependencies_but_continues(make_source, inventory, ca
                 "name": "inventory-bucket", "virtual_machine": holder, "size": 0,
             })
     source.apply()
-    assert "name collision" in caplog.text
+    assert ("name collision" in caplog.text) == (stage == "bucket")
     buckets = [disk.data["name"] for disk in inventory.get_all_items(NBVirtualDisk)
                if disk.data.get("custom_fields", {}).get("aws_service") == "s3"]
-    assert buckets == (["second"] if stage == "bucket" else [])
+    assert buckets == (["second"] if stage == "bucket" else ["inventory-bucket", "second"])
+    assert cluster.data["custom_fields"]["aws_resource_id"] == identity
+    if stage == "holder":
+        assert holder.data["custom_fields"]["aws_resource_id"] == identity
     assert any(vm.data.get("custom_fields", {}).get("aws_instance_id") == INSTANCE
                for vm in inventory.get_all_items(NBVM))
 
@@ -323,6 +326,56 @@ def test_eks_conflict_skips_workers_and_continues_next_region(make_source, inven
     assert "i-next-region" in instances
 
 
+@pytest.mark.parametrize("fields", [
+    {},
+    {"aws_resource_id": "", "aws_service": ""},
+])
+def test_eks_cluster_with_missing_identity_fields_is_reconciled(make_source, inventory, fields, caplog):
+    source = make_source("sync_eks = True\nsync_private_ips = False", data=service_data())
+    group = inventory.add_object(NBClusterGroup, data={"name": f"AWS {ACCOUNT}"})
+    cluster = inventory.add_object(NBCluster, data={
+        "name": f"EKS {ACCOUNT}/{REGION}/inventory-cluster",
+        "group": group,
+        "custom_fields": fields,
+    })
+    cluster.add_tags(source.source_tag)
+
+    source.apply()
+
+    assert not any("Skipping EKS cluster 'inventory-cluster'" in message
+                   for message in caplog.messages)
+    assert len([item for item in inventory.get_all_items(NBCluster)
+                if item.data.get("name") == cluster.data["name"]]) == 1
+    assert cluster.data["custom_fields"]["aws_resource_id"] == CLUSTER_ARN
+    assert cluster.data["custom_fields"]["aws_service"] == "eks"
+    worker = next(vm for vm in inventory.get_all_items(NBVM)
+                  if vm.data.get("custom_fields", {}).get("aws_instance_id") == INSTANCE)
+    assert worker.data["cluster"] is cluster
+
+
+@pytest.mark.parametrize("fields", [
+    {"aws_resource_id": f"arn:aws:eks:{REGION}:{ACCOUNT}:cluster/another-cluster", "aws_service": "eks"},
+    {"aws_resource_id": "", "aws_service": "lambda"},
+])
+def test_eks_cluster_does_not_adopt_conflicting_identity_fields(make_source, inventory, fields, caplog):
+    source = make_source("sync_eks = True\nsync_private_ips = False", data=service_data())
+    source.custom_fields()
+    group = inventory.add_object(NBClusterGroup, data={"name": f"AWS {ACCOUNT}"})
+    cluster = inventory.add_object(NBCluster, data={
+        "name": f"EKS {ACCOUNT}/{REGION}/inventory-cluster",
+        "group": group,
+        "custom_fields": fields,
+    })
+    cluster.add_tags(source.source_tag)
+
+    source.apply()
+
+    assert "Skipping EKS cluster 'inventory-cluster'" in caplog.text
+    assert cluster.data["custom_fields"] == fields
+    assert not any(vm.data.get("custom_fields", {}).get("aws_instance_id") == INSTANCE
+                   for vm in inventory.get_all_items(NBVM))
+
+
 def test_lambda_function_conflict_continues_next_function(make_source, inventory, caplog):
     data = service_data()
     source = make_source("sync_lambda = True\nsync_private_ips = False", data=data)

tests/test_cluster_scope.py

@@ -13,10 +13,11 @@
 that object again, otherwise every run reports a change.
 """
 import logging
+from datetime import datetime
 
 import pytest
 
-from module.netbox.object_classes import NBCluster, NBClusterType, NBSite, NBSiteGroup
+from module.netbox.object_classes import NBCluster, NBClusterType, NBVM, NBSite, NBSiteGroup
 
 
 def test_cluster_scope_is_resolved_from_an_id(inventory):
@@ -46,6 +47,23 @@ def test_a_site_object_survives_resolving(inventory):
     assert cluster.data.get("scope_type") == "dcim.site", "scope type was dropped"
 
 
+def test_unscoped_cluster_has_no_missing_scope_warning(inventory, caplog):
+    cluster = inventory.add_object(NBCluster, data={"name": "global"})
+
+    assert cluster.get_display_name(including_second_key=True) == "global"
+    assert "Unable to determine second key 'scope_id'" not in caplog.text
+
+
+def test_vm_lookup_does_not_serialize_cluster_metadata(inventory):
+    cluster = inventory.add_object(NBCluster, data={
+        "name": "eks-cluster",
+        "custom_fields": {"aws_metadata": {"createdAt": datetime(2026, 10, 23)}},
+    })
+    vm = inventory.add_object(NBVM, data={"name": "worker", "cluster": cluster})
+
+    assert inventory.get_by_data(NBVM, data={"name": "worker", "cluster": cluster}) is vm
+
+
 def test_source_scopes_a_cluster_with_the_site_object(vcsim, inventory, load_config, vmware_settings):
     """A dict here is sent to NetBox as is and rejected with 'scope_id: A valid integer is required.'"""
     from module.sources import instantiate_sources