Skip to content

Conversation

lmolkova
Copy link
Member

@lmolkova lmolkova commented Jul 1, 2025

Fixes #2448

Related to #2455

Blocked on open-telemetry/weaver#812

@lmolkova lmolkova added the Skip Changelog Label to skip the changelog check label Jul 1, 2025
startswith(group.id, "registry.")
attr := group.attributes[_]
member := attr.type.members[_]
not is_property_set(member, "deprecated")
Copy link
Member Author

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 😿

Copy link

@Copilot Copilot AI left a 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 and is_property_set helpers and corresponding tests in registry_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 use attr.id or derive the full name via get_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[_]
Copy link
Preview

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.

@lmolkova
Copy link
Member Author

lmolkova commented Jul 4, 2025

replaced by #2477

@lmolkova lmolkova closed this Jul 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog Label to skip the changelog check
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Duplicate enum members are not being identified
1 participant