Skip to content

Member collisions policies: id, value, const name #2456

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lmolkova wants to merge 2 commits into from
Closed

Member collisions policies: id, value, const name #2456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5850c6f
Add policies that check member name, value, and const name collisions
Jul 1, 2025
6926460
oops
Jul 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions policies/registry.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,82 @@ deny contains attr_registry_violation(description, group.id, "") if {
description := sprintf("Semconv group '%s' does not contain stability field. All semconv definitions must include stability level.", [group.id])
}


# check that attribute is not defined or referenced more than once within the same group
deny contains attr_registry_violation(description, group.id, name) if {
group := input.groups[_]
Copy link

Copilot AI Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attribute-duplicate rule applies to every group. To match other member-collision rules, add a filter like startswith(group.id, "registry.") so non-registry groups aren’t inadvertently flagged.

Suggested change
group := input.groups[_]
group := input.groups[_]
startswith(group.id, "registry.")

Copilot uses AI. Check for mistakes.
attr := group.attributes[_]
name := attr.name

collisions := [n | n := group.attributes[_].name; n == name ]
count(collisions) > 1

description := sprintf("Attribute '%s' is already defined in the group '%s'. Attributes must be unique.", [name, group.id])
}

# check that member ids do not collide within the same attribute
deny contains attr_registry_violation(description, group.id, attr.id) if {
group := input.groups[_]
startswith(group.id, "registry.")

attr := group.attributes[_]
member := attr.type.members[_]

collisions := [n | n := attr.type.members[_].id; n == member.id ]
count(collisions) > 1

description := sprintf("Member with id '%s' is already defined on the attribute '%s' in the group '%s'. Member id must be unique.", [member.id, attr.id, group.id])
}

# check that member values do not collide within the same attribute
deny contains attr_registry_violation(description, group.id, attr.id) if {
group := input.groups[_]
startswith(group.id, "registry.")
attr := group.attributes[_]
member := attr.type.members[_]
not is_property_set(member, "deprecated")
Copy link
Copy Markdown
Member Author

@lmolkova lmolkova Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can never understand the difference between not obj.prop vs obj.prop == null vs this hack. For whatever reason this works and not obj.prop, obj.prop == null don't 😿


collisions := [m
| m := attr.type.members[_]
not is_property_set(m, "deprecated")
m.value == member.value
]
count(collisions) > 1

description := sprintf("Member with value '%s' (id '%s') is already defined on the attribute '%s' in the group '%s'. Member value must be unique.", [member.value, member.id, attr.id, group.id])
}

# check that member const names do not collide within the same attribute
deny contains attr_registry_violation(description, group.id, attr.id) if {
group := input.groups[_]
startswith(group.id, "registry.")
attr := group.attributes[_]
member := attr.type.members[_]
not member.annotations["code_generation"]["exclude"]

const_name := to_const_name(member.id)

collisions := [m
| m := attr.type.members[_]
to_const_name(m.id) == const_name
not m.annotations["code_generation"]["exclude"]
]
count(collisions) > 1

description := sprintf("Member with const name '%s' (id '%s'), is already defined on the attribute '%s' in the group '%s'. Member const names must be unique.", [const_name, member.id, attr.id, group.id])
}

get_attribute_name(attr, group) := name if {
full_name := concat(".", [group.prefix, attr.id])

# if there was no prefix, we have a leading dot
name := trim(full_name, ".")
}

to_const_name(name) = const_name if {
const_name := replace(name, ".", "_")
}

is_property_set(obj, property) = true if {
obj[property] != null
} else = false
40 changes: 40 additions & 0 deletions policies_test/registry_test.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,43 @@ test_attribute_requirement_levels if {
count(before_resolution.deny) > 0 with input as {"groups": [{"id": "registry.foo", "attributes": [{"id": "foo", "requirement_level": {"recommended": "if available"}, "stability": "rc"}]}]}
count(before_resolution.deny) == 0 with input as {"groups": [{"id": "not_registry", "attributes": [{"ref": "foo", "requirement_level": "required"}]}]}
}

test_fails_on_member_id_collision if {
collision := {"groups": [
{"id": "registry.test", "prefix": "", "attributes": [{"id": "foo.bar.baz", "type": {"members": [
{"id": "member", "value": "value1", "brief": "brief", "stability": "stable"},
{"id": "member", "value": "value2", "brief": "brief", "stability": "stable"},
]}, "stability": "stable"}]},
]}
count(before_resolution.deny) == 2 with input as collision
}

test_fails_on_member_const_name_collision if {
collision := {"groups": [
{"id": "registry.test", "prefix": "", "attributes": [{"id": "foo.bar.baz", "type": {"members": [
{"id": "member_id", "value": "member_id", "brief": "brief", "stability": "stable"},
{"id": "member.id", "value": "member.id", "brief": "brief", "stability": "stable"},
]}, "stability": "stable"}]},
]}
count(before_resolution.deny) == 2 with input as collision
}

test_fails_on_member_value_collision if {
collision := {"groups": [
{"id": "registry.test", "prefix": "", "attributes": [{"id": "foo.bar.baz", "type": {"members": [
{"id": "member1", "value": "member", "brief": "brief", "stability": "stable"},
{"id": "member2", "value": "member", "brief": "brief", "stability": "stable"},
]}, "stability": "stable"}]},
]}
count(before_resolution.deny) == 2 with input as collision
}

test_passes_on_member_value_collision_with_deprecated if {
collision := {"groups": [
{"id": "registry.test", "prefix": "", "attributes": [{"id": "foo.bar.baz", "type": {"members": [
{"id": "member1", "value": "member", "brief": "brief", "stability": "stable", "deprecated": "renamed to member2"},
{"id": "member2", "value": "member", "brief": "brief", "stability": "stable"},
]}, "stability": "stable"}]},
]}
count(before_resolution.deny) == 0 with input as collision
}
Loading