Skip to content

Conversation

lovisaberggren
Copy link
Collaborator

@lovisaberggren lovisaberggren commented Aug 14, 2025

Proposed changes

New IPA rule validates that a discriminator property is always accompanied by oneOf, allOf or anyOf.

Note: The exception level is on the schema itself, since we can't put extensions on the discriminator object.

Current violations: 23. Will add exceptions/corrections in follow-up

Jira ticket: CLOUDP-308542

@lovisaberggren lovisaberggren marked this pull request as ready for review August 14, 2025 10:31
@lovisaberggren lovisaberggren requested a review from a team as a code owner August 14, 2025 10:31
@lovisaberggren lovisaberggren changed the title feat(ipa): new IPA rule xgen-IPA-125-oneOf-must-accompany-oneOf-anyOf-allOf feat(ipa): new IPA rule xgen-IPA-125-discriminator-must-accompany-oneOf-anyOf-allOf Aug 15, 2025
},
oneOf: [{}],
},
SchemaAnuOf: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

[nit] SchemaAnyOf

return evaluateAndCollectAdoptionStatus(errors, rule.name, siblings, path);
};

function checkViolationsAndReturnErrors(input, path, siblingKeys) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Which Swagger component annotation level are we able to define exceptions? Is it Schema?
Should we update the component level for metrics collection?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the schema. The exceptions are siblings to the discriminator since discriminators cannot have extensions.

Now the metric path is ['components', 'schemas', 'SchemaName', 'discriminator'] - no strong opinion to have it or remove the discriminator

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can remove the discriminator

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

@lovisaberggren lovisaberggren merged commit 34ca221 into main Aug 15, 2025
11 checks passed
@lovisaberggren lovisaberggren deleted the CLOUDP-308542 branch August 15, 2025 12:37
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.

3 participants