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

Proposal for extensions and protocolInfo objects versioning #1

Closed
fmvilas opened this issue Mar 30, 2019 · 19 comments
Closed

Proposal for extensions and protocolInfo objects versioning #1

fmvilas opened this issue Mar 30, 2019 · 19 comments

Comments

@fmvilas
Copy link
Member

fmvilas commented Mar 30, 2019

After thinking thoroughly about extensions and protocolInfo objects and their schemas, this is the proposal I have.

Context

AsyncAPI (and also OpenAPI) allows you to annotate your document with properties starting by x- and whose values can be anything. We now want to provide a catalog of extensions but we MUST not break this "freedom".

In version 2, we introduced "protocol info" objects. They are also treated as extensions but, unlike the other ones, they can only live inside the protocolInfo object. They consist of a key (e.g., kafka) and a value of type object. The value will contain protocol-specific information.

Problem

We'll want to iterate on extensions independently, i.e., without having to release a new AsyncAPI specification version. Therefore, when using an extension inside an AsyncAPI document, we should have the possibility to specify the version of the extension we're using.

As an example, if we have the following definition:

asyncapi: '2.0.0'
channels:
  /tweets:
    subscribe:
      protocolInfo:
        kafka:
          clientId: 'my-client-id'

We're saying that, when using Kafka, we want the code to subscribe using my-client-id as the Client Id. This kafka protocolInfo object is validated using the following (simplified) schema:

{
  "type": "object",
  "properties": {
    "clientId": {
      "type": "string"
    }
  }
}

However, after some time, our extension is growing a lot and we want to reorganize the fields in the kafka object as follows:

...
protocolInfo:
  kafka:
    client:
      id: 'my-client-id'

And the new schema is the following:

{
  "type": "object",
  "properties": {
    "client": {
      "type": "object",
      "properties": {
        "id": {
          "type": "string"
        }
      }
    }
  }
}

This is a breaking change so, suddenly, we would be breaking the workflow for these people using the Kafka protocolInfo extension. This is an undesirable side-effect we can avoid by specifying a version string in the extension:

asyncapi: '2.0.0'
channels:
  /tweets:
    subscribe:
      protocolInfo:
        kafka:
          _v: '0.1.0'
          clientId: 'my-client-id'

Note this version is the version of the extension, not the Kafka version.

It seems clear we need a version strategy, however, while this is fine for protocolInfo extensions, it might not be the case for specification extensions, because they may not be of type "object". Consider the following example:

asyncapi: '2.0.0'
info:
  x-twitter: @PermittedSoc

In this case, we can't add the version (_v: 'x.x.x'). Even more, specification extensions do not have to follow our formatting. Remember they are free-form values and they may not be in our catalog or people just don't want to validate them at all.

Solution

For specification extensions, we assume the following:
  • If the extension is not in our catalog (or provided in another way), we MUST NOT perform validation on the extension.
  • If version (_v) is omitted and the extension is in our catalog, we MUST perform validation against the "latest" version of the extension.
  • If version (_v) is omitted and the tooling provides the definition, we MUST perform validation against the provided extension definition.
  • If version (_v) is provided and the extension is in our catalog, we MUST perform validation against the provided version of the extension.
  • If version (_v) is provided and the tooling provides the definition for another version, we MUST NOT perform validation on the extension.
For protocolInfo extensions, we assume the following:
  • Version (_v) is mandatory.
  • If the protocolInfo extension is in our catalog (or provided in another way), we MUST perform validation against the provided version of the extension.
  • If the protocolInfo extension is not in our catalog and it's not provided in another way, we MUST NOT perform validation on the protocolInfo extension.
  • If the tooling provides the definition for another version, we MUST NOT perform validation on the protocolInfo extension.

Open questions

  • Should we call the version field _v? Another options: _version, version, extension_version, _extension_version. Please, take into account that protocolInfo extensions may have to use the version field for the protocol version. I personally prefer the _v name because 1) it starts with an underscore and makes it look like something special or "meta"; and 2) it's short and not similar to version.
  • Should we enforce semantic versioning?
  • Should we accept version ranges in the _v field? E.g., 1.3.x, 1.x, *, latest, etc.
@jhigginbotham
Copy link

@fmvilas Regarding non-object extensions, such as your x-twitter example, would it be better to have an extensions section in the asyncapi definition that provides the extension prefix(es) used, e.g. x-twitter, the version of the extension, and the URL to the extension spec? That way, you don't have to provide it inline (especially if you use the extension more than once within a given asyncapi file).

+1 for expanded names, such as protocolVersion, extensionVersion, etc. whenever possible to avoid confusion by authors and consumers of the definition file.

@fmvilas
Copy link
Member Author

fmvilas commented Apr 1, 2019

That sounds good. I wonder if we lose reusability (across documents) if we have an extensions section. Can you provide an example of your vision?

@RobertDiebels
Copy link

RobertDiebels commented Apr 1, 2019

Versioning
I would add versioning in this manner for extensions:

asyncapi: '2.0.0'
channels:
  /tweets:
    subscribe:
      protocolInfo:
        kafka:
          metadata: 
            version: 1.2.0
          extensions:
            x-name:
              metadata: 
                version:  0.1.0
              clientId: 'my-client-id'

If a new version of x-name gets released:

asyncapi: '2.0.0'
channels:
  /tweets:
    subscribe:
      protocolInfo:
        kafka:
          metadata: 
            version: 1.2.0
          extensions:
            x-name:
              metadata: 
                version:  0.2.0
              client:
                id: 'my-client-id'

This makes it clear that the kafka protocol has version 1.2.0 and that you're defining one extension 'x-name' which has version 0.1.0.

The extensions property MAY be added, but is not required.
The metadata property MAY be added, but is not required.
The metadata property is a keyword. If it is used it is subject to limitations defined in the spec.

Re-use
This way we preserve free-form of an extension and a user can re-use an extension by doing something like this:

asyncapi: '2.0.0'
channels:
  /tweets:
    subscribe:
      protocolInfo:
        metadata: 
          version: 1.2.0
        kafka:
          extensions:
            x-name:
              $ref: /some/reference

Literals
As for the x-twitter example. I don't think you can avoid placing some minor restrictions if you want to allow versioning on an extension.

In this case an extension would always have to be an object since you want to allow for the possibility of versioning. Like so:

asyncapi: '2.0.0'
info:
  extentions:
   x-twitter: 
     value: @PermittedSoc

Then with versioning:

asyncapi: '2.0.0'
info:
  extentions:
   x-twitter: 
     metadata:
       version: 0.1.0
     key-name: @PermittedSoc

Benefits/Costs
Pros

  • Avoids long naming like extentionVersion, protocolVersion etc,
  • Avoids issues with in-lining of extensions.
  • Enables a user to see in one glance which version belongs to which property.
  • Enables the spec to add other properties as metadata.
  • Makes it clear that you've added extensions and which ones.

Cons

  • You no longer have the option of doing something like x-twitter: @PermittedSoc. Since an extension should allow for the possibility of a metadata property, we are now forcing it to be defined as an object.

Personally I believe that giving up the ability to define extensions as literals is minor compared to what is gained through versioning.

@fmvilas
Copy link
Member Author

fmvilas commented Apr 1, 2019

@RobertDiebels I see what you mean but let me clarify something:

Specification extensions (x-something) are not meant to be inside a protocolInfo object but in (almost) any other part of the AsyncAPI document. For instance:

asyncapi: '2.0.0'
info:
  x-twitter: '@PermittedSoc'

Then we have the protocolInfo extensions which are protocol-specific and they don't start by x-. Instead, we identify them because they are defined inside the protocolInfo object. See my Kafka example.

That said, protocolInfo extensions are easy to design because they didn't exist in previous versions and they're constrained to protocolInfo. The "problem" comes with specification extensions because their value can be of any type, i.e., they may not be objects and therefore they can't contain a metadata key. An example is the Twitter one that I mentioned earlier. Its value is a string, so there's no way to add version unless we "annotate" the name or, as @jhigginbotham suggested, we create an extensions section where we define which version a given extension is using.

Hope it helps clarify the challenge. Thanks!

@fmvilas
Copy link
Member Author

fmvilas commented Apr 1, 2019

You no longer have the option of doing something like x-twitter: @PermittedSoc. Since an extension should allow for the possibility of a metadata property, we are now forcing it to be defined as an object.

This cons must be avoided to keep compatibility between OpenAPI and AsyncAPI extensions, and also backward compatibility with AsyncAPI 1.x extensions.

@fmvilas
Copy link
Member Author

fmvilas commented Apr 1, 2019

I'd rather prefer to have something like the following: "if your extension doesn't have a metadata key (and it includes strings, like the Twitter one) then we assume you're using the latest definition available."

If you want to avoid the (sometimes undesirable) effect of getting the latest version, change the extension format to an object. That's the trade-off.

@RobertDiebels
Copy link

RobertDiebels commented Apr 1, 2019

@fmvilas

Specification extensions (x-something) are not meant to be inside a protocolInfo object but in (almost) any other part of the AsyncAPI document.

Understood. My proposal would be to add the extensions property to all definitions.

This cons must be avoided to keep compatibility between OpenAPI and AsyncAPI extensions

Can you elaborate on the need to keep compatibility with OpenAPI extensions?

and also backward compatibility with AsyncAPI 1.x extensions

I thought this change to the spec concerns 2.0 so adding a breaking change wouldn't be an issue.

I'd rather prefer to have something like the following: "if your extension doesn't have a metadata key (and it includes strings, like the Twitter one) then we assume you're using the latest definition available."

Seems fair.

If you want to avoid the (sometimes undesirable) effect of getting the latest version, change the extension format to an object. That's the trade-off.

So that would be off-loading the decision to the extensions' maintainer. Seems good to me 👍 .

One other pro I could think of like this is that the parser can avoid having to check each key for the x- prefix. It would simply read the properties in the extensions property if it exists and move on if it doesn't.
However if OpenAPI compatibility requires the x- prefix for extensions and we can't deviate from that then that's a no-go.

@fmvilas
Copy link
Member Author

fmvilas commented Apr 1, 2019

Can you elaborate on the need to keep compatibility with OpenAPI extensions?

Companies and tooling are already using their own extensions in OpenAPI. We want people to use their existing extensions on AsyncAPI too, if that makes sense for them. For instance, the Twitter one is one of these cases.

I thought this change to the spec concerns 2.0 so adding a breaking change wouldn't be an issue.

Yes, we can add breaking changes, and we could provide migration scripts to migrate from version 1.x to version 2. However, this decision is causing breaking changes on user extensions and not much on our side. If we think this is something we really need to do, that's fine, but I think we can avoid it.

However if OpenAPI compatibility requires the x- prefix for extensions and we can't deviate from that then that's a no-go.

It's not that "we can't" but let's think on the user experience first, and they will want to reuse as much as possible. That's why we keep the compatibility with OpenAPI schemas in version 2.

@RobertDiebels
Copy link

RobertDiebels commented Apr 1, 2019

So that would be off-loading the decision to the extensions' maintainer. Seems good to me 👍 .

On second thought. That might not be the best. Consider this: If you're working with a team and team-member A uses the same spec team-member B. However, team-member B is performing a fresh tooling run and suddenly nothing is working due to the using the latest version of a literal extension. I don't think the user should be bothered with that.

Maybe adding an extensions section would be best.

Companies and tooling are already using their own extensions in OpenAPI. We want people to use their existing extensions on AsyncAPI too, if that makes sense for them. For instance, the Twitter one is one of these cases.

So it's not so much about adding a extensions property. It's about whether or not the user can define a literal extension?

If we think this is something we really need to do, that's fine, but I think we can avoid it.

Seems good to me 👍 .

It's not that "we can't" but let's think on the user experience first, and they will want to reuse as much as possible.

In this case that would be the extensions being defined as a literal correct? Not so much where they are placed within the spec?

@fmvilas
Copy link
Member Author

fmvilas commented Apr 1, 2019

Maybe adding an extensions section would be best.

I don't see how an extensions section would solve this problem. You still have to create another extension that's not just a string. Am I missing something?

So it's not so much about adding a extensions property. It's about whether or not the user can define a literal extension?

Users can define extensions with the value they'd prefer. It's an opaque value for us and should remain the same unless the user explicitly wants to validate some of them. Another example could be an extension called x-rating: 5 whose value is a number. Or another one called x-documented-in-company-website: true whose value is a boolean.

Specification extensions (x-something) can be anything and that can't be changed.

@RobertDiebels
Copy link

I don't see how an extensions section would solve this problem. You still have to create another extension that's not just a string. Am I missing something?

No you're correct. I meant an extensions section such as @jhigginbotham proposed. I should have been clearer about that.

Users can define extensions with the value they'd prefer.

What I meant was that in essence you have 2 options, an object or a literal. In language parsing a literal means any token representing a concrete value. Like you said: true, false, 5, a-string-of-characters etc.

Specification extensions (x-something) can be anything and that can't be changed.

In that case adding a metadata or _v property can't be done either since an extension may have defined it themselves, correct?

That would leave only @jhigginbotham 's proposal of adding an extensions section and adding any metadata there.

@fmvilas
Copy link
Member Author

fmvilas commented Apr 1, 2019

In that case adding a metadata or _v property can't be done either since an extension may have defined it themselves, correct?

Correct. That's why I wanted to use a name like _v because it's very unlikely anyone is using it. We could also use the following:

x-my-extension:
  _meta:
    version: 0.1.0

That would leave only @jhigginbotham 's proposal of adding an extensions section and adding any metadata there.

Yes. My fear with this solution is that we may reduce the "portability" of extensions but it's a great solution in any case. We can consider that version is "latest" if it's not found in the extensions section. Or we just ignore them. We have to define the behavior in this case.

@RobertDiebels
Copy link

RobertDiebels commented Apr 1, 2019

@fmvilas I'd suggest to try and avoid using latest tags as much as possible or not at all. Using implicit versions can lead to issues for users where none are expected. Explicit versioning would be preferable. I'd even go as far as having the parser throw an exception but that's my personal preference. In addition I believe we should enforce explicit semantic-versioning.

As an example, in the past I've had some problems with implicit versioning where I couldn't find the cause of an issue in an NPM package. It ended up being caused by a "version": "x.x" which allowed the package to be updated on a fresh install. Allowing implicit versioning assumes that you can trust other peoples code to be stable at all times, which is not the case most of the time.

@fmvilas
Copy link
Member Author

fmvilas commented Apr 1, 2019

I understand your concern but this is a problem that appears when you have dependencies of dependencies. In this case, there's only one level and you're the one specifying what versions you want. If you're concerned about it, just don't use wildcards ("x.x" or anything similar), but it can be useful many other times.

@RobertDiebels
Copy link

@fmvilas This is a problem when you have dependencies. If the spec contains a dependency on an extension and suddenly the extensions' version is increased by a major version, due to the latest tag being moved, users will have a different output even-though they didn't alter their spec.

It's not so much about using wildcards. It's about implicit versions causing unexpected behavior. That's why I suggested the warning or error message if a version hasn't been provided.

@fmvilas
Copy link
Member Author

fmvilas commented Apr 2, 2019

Sorry for the confusion. I meant the problem with "x.x" you mentioned. If the parser should trigger an error, ignore it or download the latest version, is something that we can put in the parser as an option, and we let people decide.

@RobertDiebels
Copy link

@fmvilas

Sorry for the confusion. I meant the problem with "x.x" you mentioned.

No problem. I could have been a tad clearer.

If the parser should trigger an error, ignore it or download the latest version, is something that we can put in the parser as an option, and we let people decide.

That would help tremendously. In the end users would benefit from it since their specs won't have any unexpected behavior. Or at the very least they would know about the possibility of the spec possibly exhibiting unexpected behavior.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 30 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

@github-actions github-actions bot added the stale label Mar 12, 2020
@fmvilas fmvilas added keep-open and removed stale labels Mar 13, 2020
@github-actions github-actions bot added the stale label Oct 5, 2021
@derberg derberg removed the stale label Oct 5, 2021
@asyncapi asyncapi deleted a comment from github-actions bot Oct 5, 2021
@derberg
Copy link
Member

derberg commented Apr 18, 2024

closing for now

catalog is super simplified now, as binding, any other improvements can be pushed but when there is really a dedicated person that can work on it, own discussion and lead till the end

@derberg derberg closed this as completed Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants