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

Unclear how to handle backwards-compatible changes #43

Open
paulmillar opened this issue Aug 8, 2023 · 10 comments
Open

Unclear how to handle backwards-compatible changes #43

paulmillar opened this issue Aug 8, 2023 · 10 comments

Comments

@paulmillar
Copy link
Contributor

Per page 7, the wlcg.ver, which is a required claim, denotes

the version of the WLCG token profile the relying party must understand to validate the token

The current version of the document is 1.0, thus by including the claim wlcg.ver with value 1.0, the token issuer ensures that the RP must understand the semantics of the included claims.

The document may be improved by introducing backwards compatible changes; for example, by simply fixing a simple typo. This new document would have a new version number (1.1, for example). There would then be two equivalent values for wlcg.ver claim: 1.0 and 1.1.

Under the current document definition, it is unclear which wlcg.ver claim value should be asserted.

@paulmillar
Copy link
Contributor Author

There are (at least) a couple of ways of handling this:

  1. provide an extra variable to the document's version (e.g., 1.0.0),
  2. use an errata document to manage "fixes" and clarifications.

The first approach could identify the first two digits of the document's version as the wlcg.ver claim value; for example, the document with version 1.3.2 identifies semantics identified with wlcg.ver value 1.3. This document would be the third version that defines these semantics, with document version 1.3.0 and 1.3.1 identifying earlier attempts.

The second approach is more common across standardisation. Sometimes a "bis" version is released that includes errata.

@msalle
Copy link

msalle commented Aug 10, 2023

I would probably prefer to have it defined in the doc that 1.X is backwards compatible with 1.0, and if we introduce something that breaks that, we'll go to V2.0
Furthermore, note that a client can request a version, see https://github.com/WLCG-AuthZ-WG/common-jwt-profile/blob/master/profile.md#requesting-token-versions
Also a client should ignore things that don't correspond to the specific version: https://github.com/WLCG-AuthZ-WG/common-jwt-profile/blob/master/profile.md#claim-and-token-validation
So I think most of it is already covered?
A separate errata doc while staying on 1.0 seems a bit unpractical to me.

@maarten-litmaath
Copy link
Collaborator

maarten-litmaath commented Jan 7, 2024

Here are my takes at this time:

  • It would be awkward to have to increase the profile version just to fix typos, ambiguities, omissions and other mistakes that are present e.g. in the v1.0 document. Mind that any version increase may have significant consequences for clients and/or services in the infrastructure and we thus should resort to that sparingly.
  • The profile version should reflect the set of functionality available through tokens.
    • A minor version increase from M.x to M.y indicates that functionality available in M.x still works the same way in M.y, while the latter would typically add new functionality that can just be ignored for previously existing use cases.
    • A major version increase from M.x to N.y indicates that functionality available in M.x may no longer work the same way in N.y.
  • The document should have its own version number, but that number can easily be made to extend the profile version number, as proposed above. This avoids having to know about and maintain an extra document with errata.

As a corollary, I think we should consider keeping 1.0 as the profile version while the changes stay backward-compatible with existing functionality. The document, however, would see versions 1.0.1 etc.

@DrDaveD
Copy link
Contributor

DrDaveD commented Jan 11, 2024

It seems to me that the second statement here does not follow from the first:

The profile version ... A minor version increase from M.x to M.y indicates that functionality available in M.x still works the same way in M.y, while the latter would typically add new functionality that can just be ignored for previously existing use cases.

As a corollary, I think we should consider keeping 1.0 as the profile version while the changes stay backward-compatible with existing functionality. The document, however, would see versions 1.0.1 etc.

The first statement says that new, compatible functionality should increase the profile version from 1.0 to 1.1, but the second statement says it should stay at 1.0. So which is it?

If it's super-difficult to change the wlcg.ver, we could consider adding a new claim called wlcg.revision for backward-compatible changes, but really I think that implementations should be ignoring the version number or at minimum the minor (.x) portion unless they know about existing version numbers in which they need to behave differently.

@DrDaveD
Copy link
Contributor

DrDaveD commented Jan 11, 2024

The document currently says:

Each client library implementation MUST know the versions it supports; if it encounters a token whose wlcg.ver value is not supported by the implementation, the token MUST be rejected as invalid.

Therefore we cannot change that value unless we want to introduce incompatible changes, because changing the document to say that clients need to ignore minor revision numbers is an incompatible change. So maybe a new wlcg.revision is needed for compatible changes.

@maarten-litmaath
Copy link
Collaborator

Hi @DrDaveD,
the first statement you quoted does not say that new, compatible functionality should increase the profile version!
Instead, it says what a minor version increase would imply.

The corollary rather refers to "any version increase may have significant consequences [...] should resort to that sparingly."

I agree implementations should at least ignore the minor version by default, which then would obviate the need for a wlcg.revision...

Since we are still in the early days of implementing all this, I think we can defend updating the v1.0 profile also with that requirement, while also keeping the profile on v1.0 for now, to allow time for clients and services to be upgraded as needed...

@DrDaveD
Copy link
Contributor

DrDaveD commented Jan 12, 2024

the first statement you quoted does not say that new, compatible functionality should increase the profile version! Instead, it says what a minor version increase would imply.

Huh? It says that minor version increases indicate new, compatible functionality. So naturally one would assume that new, compatible functionality would increase the minor version. I guess it doesn't say it would have to, but that's the implication.

I agree implementations should at least ignore the minor version by default, which then would obviate the need for a wlcg.revision...

It would have been nice if the 1.0 profile said that, but that's not what it says, quite clearly.

Since we are still in the early days of implementing all this, I think we can defend updating the v1.0 profile also with that requirement, while also keeping the profile on v1.0 for now, to allow time for clients and services to be upgraded as needed...

I'm afraid it's too late for that, since the official, published profile clearly requires behavior incompatible to that. I think it would require a 2.0 version to make that change.

@mambelli
Copy link

Picking a bit from the previous comments. What about formalizing a MAJOR.MINOR.PATCH versioning schema for the document and saying that the profile uses only the MAJOR.MINOR versions?

  • Patch version changes are just in the document with no changes in the schema (e.g. to fix typos or grammar, and maybe improve parts deemed unclear). This could be implemented right away since wlcg.ver talks only about 2 numbers and the patch would be an extension just for the document.
  • Minor version changes would be backward-compatible schema changes
  • Major version changes would be incompatible. Servers and clients could still support multiple versions to allow compatibility but a functionality may work differently. The same major version must be used for claim validation.
    The semantics of minor and major would have to wait for v2.x since, as @DrDaveD said the current document says otherwise. But once ready to make some changes, the next version could be 2.0.

@msalle
Copy link

msalle commented Jan 15, 2024

I'm not sure I'm following, but if we add or change functionality, we should go to a higher version. If that is new optional functionality, I'd say that would be a minor version update, if it breaks backwards compatibility it's a major version. Implementation don't need to ignore the minor version, since they might want to use the new optional features that are indicated to be supported? Not sure we need patch versions, but those should not add or change functionality, only fixing typos, clarifying things etc.

@DrDaveD
Copy link
Contributor

DrDaveD commented Apr 11, 2024

Right, they are not required to ignore the minor version, they are just required to accept any minor version and to ignore any claims or scopes they do not understand.

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

No branches or pull requests

5 participants