Removal of public method in a minor release breaking semantic versioning with utter lack of warnings or anything

[olliefreeman]

Describe the bug

869c7ba#diff-8af2ccae2405932ff99db333a8b3932c5df400268993c83c38210a1e18fd762eL204 removes the method decodeJwtContent from OIDCUtils class.
There was no depreciation warning, and with the standards of semantic versioning you cant make changes like this in a MINOR version, in a major version sure, but the expectation would be the method would be tagged with deprecation annotation to warn us to make changes.
We have minor version dependency updating in place and this broke our system.

Expected behavior

Deprecation warnings on methods before removal, or clear documentation that this is happening. Minor semantic versions should never introduce breaking changes

Actual behavior

Complete removal of multiple methods from a class in a MINOR semantic version change, and zero documentation as to what to use to replace it

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

No response

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

[olliefreeman]

Also the method was moved into OidcCommonUtils, but it didnt exist in there prior to 3.18 which means we have to make any library changes across all our services at EXACTLY the same time to prevent any further breaking

[gsmet]

So first, I don't think we said anywhere we were following semantic versioning. We aren't.

Second this class isn't considered API so even if we were following semantic versioning, it wouldn't have been considered an issue to move this method.

I would recommend you make your case as to what you are using this method for and why you would like it to be exposed as API and we could make progress from there.

[sberyozkin]

We can't really treat all public methods inside the runtime package as public API, I can restore this method though, delegating to its new location

[sberyozkin]

@olliefreeman I've proposed restoring a couple of methods in #45949, but, as Guillaume explained, classes in the runtime are not considered API classes, with some exceptions, for example, when we ended up documenting how to use these runtime package classes in the quickstart or reference guides.

In general, only the classes in the io,quarkus.oidc package can be considered part of the public Quarkus OIDC API.

[olliefreeman]

TBH I dont mind that they're being removed, its just the lack of protections around people using your code, I've moved over to the com.auth0:java-jwt package for decoding the token as its actually better and what I would use anyway. Whoever originally coded our code obviously assumed a class called OidcUtils with public methods would be api level.

Useful to know that only stuff directly inside io.quarkus.oidc is public api. Although arguably the utils class is inside that package and its part of the oidc extension jar file, it's not part of some dependency of it, maybe some docs to intimate this, or extract the runtime package into a separate jar with a clear "runtime" dependency defined to prevent this sort of thing happening.

One of the points of using an x.y.z format is that its semantic and with java based (and maven stored) dependencies everyone assumes its following semantic versioning, maybe not down to the letter but closely enough that minor and patch changes can be trusted to not introduce breaking changes. So you may not have said you're following it, but people will assume you are as everyone else does. Again now we know you're not, we will take steps to assume all changes are breaking.

Thank you.

[sberyozkin]

@olliefreeman This particular utility code is a very simple code which avoids reg-exs and just decodes the specific parts of the token, I'm not sure switching to some 3rd party library makes sense or what does it really mean that the Base64url decoding can be done better elsewhere, but it is your code, so I'm fine with it...

In any case, like we said, the code in the runtime package is not a public API...

[gsmet]

One of the points of using an x.y.z format is that its semantic and with java based (and maven stored) dependencies everyone assumes its following semantic versioning, maybe not down to the letter but closely enough that minor and patch changes can be trusted to not introduce breaking changes.

I think there is a misunderstanding here: the x.y.z format for versions existed far before semantic versioning was even a thing.

As for our API, here is the rule we try to have: everything under the .runtime. package (and the packages under it) is considered internal. You can use it but at your own risk.
Now these are internal rules, we should probably make them explicit somewhere.

As for our versioning:

• in major, we introduce major breakages, such as the Jakarta package change, Java 17 as minimal requirement
• in minor, we introduce new features and various changes
• in micro, we push only bugfixes - but bugfixes might break things that are under the .runtime. package as it is not considered API.

Now, we are very open to discussion so if you need something in particular for a very common task and would like us to provide it as a public util, please let us know.
Sometimes we might point you in another direction to use an external library, and sometimes we will happily agree and we will discuss the API and expose it.

But I would recommend to start the discussion with "we need a utility to do X in most of our projects, here is why we need it and the features we need it to cover, it would be nice to include it out of the box in the Quarkus API as it's a very common use case".
Because I think we all agree that Quarkus needs to expose what you need for the common use cases of its extensions.

Hope it clarifies things and happy to discuss with you about how to best make progress on this particular use case.

[quarkus-bot]

/cc @pedroigor (bearer-token,oidc)

[geoand]

I'll just add to @gsmet's excellent summary above, that if we considered every public class part of the API, our ability to innovate and improve would be severely hampered.