Add OpenAPI compatibility checker

[simonnepomuk]

Hi guys,

this would be useful for many projects I'm facing with enterprise customers. Would be happy if someone would be able to review.

This should close #15

[apicurio-bot]

Thank you for creating a pull request!

Pinging @jsenko to respond or triage.

[EricWittmann]

Thanks @simonnepomuk - we'll have a look at this asap and discuss. Using the diff as the basis for the compatibility check makes sense, and this could be a nice first step for OpenAPI compatibility.

The problem with full OpenAPI compatibility checking is that there are lots of ways the API definition could change, and it's actually pretty hard to determine which of those changes are incompatible. I'll give you an example:

If you add a new required property to a schema type, is that a compatible change? It probably depends on whether that schema type is used as an input to an operation (INCOMPATIBLE) or an output (COMPATIBLE).

This is the kind of thing that has made us hesitate to implement the feature. That said, it could be argued that having at least some compatibility checking is better than none!

[EricWittmann]

Quick note to @simonnepomuk : could you please run mvn spotless:apply and then push the resulting changes? That should retrigger all of the CI tests.

[simonnepomuk]

@EricWittmann I've tried to apply the codestyles, hopefully the CI will run now :)

Regarding your general concerns: I get your hesitation, thats why I delegated the "hard" work to the library provided through OpenAPI which also comes with compatibility checks. I'm not sure wether the tricky situations you described are picked up through the library or not, I would argue that the best way to go about it would be to resolve them over there though. Glancing through the library code gave me a solid impression though.

If you have a list of hard to determine compatibility issues, I'd be happy to assist in analysing the OpenAPI diff library.

Still, I would also agree with your last argument, that some checks are better than none. That should be noted in the documentation though, so people don't put too much trust in the compatibility checking mechanism.

[simonnepomuk]

@EricWittmann Could you maybe retrigger the failed integration test workflow? Looks like some CI issue to me.

I would need help in addressing the native image build errors as I have no clue whats going on there.

[EricWittmann]

Yeah we are having some issues with inconsistent CI results. We are working on that.

As for the native image issues, I'll have @carlesarnal take a look. 😁

[simonnepomuk]

@carlesarnal Could you rerun the workflows? I think I fixed the native build issue, at least it runs on my machine now.

[simonnepomuk]

Sorry, formatting was off again. Maybe we could add a pre-commit hook to run mvn spotless:apply?

[EricWittmann]

Sorry, formatting was off again. Maybe we could add a pre-commit hook to run mvn spotless:apply?

I was thinking the same thing recently actually. @andreaTP wdyt?

[EricWittmann]

@simonnepomuk Update: CI is failing for a new reason. Give us a day or two more, we're actively working on fixing CI. Once we do you can Update branch and the workflows should all run properly.

[carlesarnal]

@simonnepomuk CI should be more reliable now, I recommend rebasing main (and I will approve the execution).

[simonnepomuk]

@simonnepomuk CI should be more reliable now, I recommend rebasing main (and I will approve the execution).

Done :)

[EricWittmann]

All green. Noice.

@jsenko Any concerns?

[EricWittmann]

Thanks for the contribution @simonnepomuk - apologies it took so long to get merged. :)