Skip to content

Reverse check on model types validation - #837

Open
shipperizer wants to merge 1 commit into
mainfrom
fix/authz-check
Open

Reverse check on model types validation#837
shipperizer wants to merge 1 commit into
mainfrom
fix/authz-check

Conversation

@shipperizer

Copy link
Copy Markdown
Collaborator

No description provided.

@shipperizer
shipperizer requested a review from a team as a code owner January 27, 2026 14:07
Copilot AI review requested due to automatic review settings January 27, 2026 14:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes a logic error in the model validation function by reversing the condition check for comparing type definitions.

Changes:

  • Corrected the comparison logic in CompareModel to return false when type definitions do not match, instead of when they do match.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +187 to 190
if !reflect.DeepEqual(authModel.TypeDefinitions, model.TypeDefinitions) {
c.logger.Errorf("invalid authorization model type definitions")
span.SetStatus(codes.Error, "invalid authorization model type definitions")
return false, nil

Copilot AI Jan 27, 2026

Copy link

Choose a reason for hiding this comment

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

The logic has been inverted to return false when type definitions don't match. However, the error message states 'invalid authorization model type definitions' which suggests a validation failure. If type definitions not matching is expected to return false (indicating models are different), consider whether this is truly an error condition that warrants logging as an error and setting an error status on the span. If this is expected behavior for non-matching models, use Info level logging instead of Error, or remove the logging/span status entirely.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants