-
Notifications
You must be signed in to change notification settings - Fork 265
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.
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
Conversation
startswith(group.id, "registry.") | ||
attr := group.attributes[_] | ||
member := attr.type.members[_] | ||
not is_property_set(member, "deprecated") |
There was a problem hiding this comment.
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 😿
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR extends the registry policy to catch duplicate definitions and adds tests for those collision scenarios.
- Adds Rego rules to enforce unique attribute names and unique member IDs, values (excluding deprecated), and constant names
- Introduces
to_const_name
andis_property_set
helpers and corresponding tests inregistry_test.rego
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
policies/registry.rego | New deny rules for duplicate attribute names and member ID/value/const-name collisions; helper functions added |
policies_test/registry_test.rego | Tests covering member ID, member value (with/without deprecated), and const-name collision cases |
Comments suppressed due to low confidence (2)
policies_test/registry_test.rego:61
- [nitpick] We’ve added a rule to detect duplicate attribute names, but there’s no test covering this case. Please add a registry_test that defines the same attribute twice and asserts
before_resolution.deny
catches it.
test_attribute_requirement_levels if {
policies/registry.rego:97
- The
attr.name
field is not defined on attributes. You should useattr.id
or derive the full name viaget_attribute_name(attr, group)
.
name := attr.name
|
||
# 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[_] |
There was a problem hiding this comment.
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.
group := input.groups[_] | |
group := input.groups[_] | |
startswith(group.id, "registry.") |
Copilot uses AI. Check for mistakes.
replaced by #2477 |
Fixes #2448
Related to #2455
Blocked on open-telemetry/weaver#812