Skip to content
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

feat: add schema_version key to groups for calculated yaml definitions #52

Merged
merged 11 commits into from
Feb 21, 2025

Conversation

GrantBirki
Copy link
Member

@GrantBirki GrantBirki commented Feb 18, 2025

schema_version

This pull request updates the calculated groups logic to add a new key called schema_version to yaml file definitions.

This new attribute is optional and if unset it will default to entitlements/v1.

Example

description: Yo kittens
schema_version: namespace/major.minor.patch # <-- new
rules:
  or:
    - username: russianblue
    - username: BlackManx

@Copilot Copilot bot review requested due to automatic review settings February 18, 2025 18:17

Choose a reason for hiding this comment

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

PR Overview

This PR introduces a new optional key, schema_version, to YAML definitions for calculated groups. The changes include adding schema_version to sample YAML fixtures, implementing its retrieval and validation in the Ruby code, and adding tests to verify valid, default, and error cases.

Changes

File Description
spec/unit/fixtures/ldap-config/filters/no-filters-with-schema-version.yaml Adds a YAML fixture with a valid schema_version value
spec/unit/fixtures/ldap-config/filters/no-filters-with-schema-version-with-v.yaml Adds a YAML fixture with a schema_version value that has a leading 'v' to test error handling
lib/entitlements/data/groups/calculated/yaml.rb Implements schema_version method with semver validation and default fallback
spec/unit/entitlements/data/groups/calculated/yaml_spec.rb Introduces tests for valid, default, and invalid schema_version formats
spec/unit/fixtures/ldap-config/filters/no-filters-with-bad-schema-version.yaml Adds a YAML fixture with a malformed schema_version to trigger error handling

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

lib/entitlements/data/groups/calculated/yaml.rb:45

  • [nitpick] Consider updating the error message to explicitly indicate that version strings with a 'v' prefix (e.g., 'v1.2.3') are not accepted, making the error more informative for users.
unless version.match?(\A\d+\.\d+\.\d+\z)

spec/unit/entitlements/data/groups/calculated/yaml_spec.rb:45

  • [nitpick] Consider rewording this test description to clearly indicate that a version string with a 'v' prefix is expected to raise an error due to an invalid format.
it "returns the version string when one is set (with v prefix - not valid semver 2)" do

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

@GrantBirki GrantBirki added the enhancement New feature or request label Feb 18, 2025
northrup
northrup previously approved these changes Feb 20, 2025
Copy link
Member

@northrup northrup left a comment

Choose a reason for hiding this comment

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

lets do it - its go time

@northrup northrup dismissed their stale review February 20, 2025 19:39

Forgot something

@northrup
Copy link
Member

@GrantBirki you also need to bump the version of this gem 😸

@GrantBirki GrantBirki merged commit 216dcbe into main Feb 21, 2025
17 checks passed
@GrantBirki GrantBirki deleted the schema-version branch February 21, 2025 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants