feat: add value parameters for websocket bindings

[Souvikns]

Description
In glee we are using WebSocket bindings for validating the headers and query in which case they work fine since current bindings say query and headers are schema objects. After this PR glee acts like a client so it actually needs to pass headers and query to the server which is something missing in the current websocket bindings. For this, we need the current bindings to support passing actual query and headers data.

I am opening this PR to add two new parameters, for passing actual headers and query values namely

• headerValues
• queryValues

Example

/notification:
  bindings:
    ws:
      headerValues:
        Authorization: "bearer token"
  publish:
    message:
      $ref: '#/components/messages/notification'

ps: @fmvilas

[fmvilas]

Left a comment. Also please bump the version in the websockets/README.md document and in the https://github.com/asyncapi/bindings/blob/master/websockets/json_schemas/channel.json file.

[Souvikns]

One question I want to ask @smoya, like when we create a new version of a protocol binding, how do we excess the old version of the binding, For example after this PR gets merged WebSocket bindings will have version 0.2.0, and there is no way to access the 0.1.0.

[smoya]

One question I want to ask @smoya, like when we create a new version of a protocol binding, how do we excess the old version of the binding, For example after this PR gets merged WebSocket bindings will have version 0.2.0, and there is no way to access the 0.1.0.

I hope this answers your question. There is a WIP PR by @jonaslagoni to handle bindings versions properly. You can see the work in asyncapi/spec-json-schemas#239 (comment).

[fmvilas]

Suggested change

	You can pass in enviornment variables to `queryValues` and `headerValues` using `$<variable>` format. Tools will resovle this value using `.env` files.
	You can pass in environment variables to `queryValues` and `headerValues` using `$<variable>` format. Tools will resolve this value using `.env` files.

[fmvilas]

A few comments on this:

1. Let's try to use more formal language when possible. For instance, use language like "All string values containing words starting with the dollar ($) sign, MUST be replaced with environment variables of the same name at runtime." instead of "You can pass" or "Tools will resolve".
2. .env files are something very typical/specific from Node.js. Also, when running in production, these values are usually retrieved from the system environment variables, not from a file.
3. Provide some examples of Do's and Dont's. For instance, Bearer $TOKEN would be ok but Bearer $ TOKEN would not.
4. Are these variables case-sensitive or not? I don't think they should be and we should probably make it mandatory to be always uppercase.
5. Can these variable names start with numbers? What symbols are allowed? You should probably check what symbols are valid for environment variables in Unix and Windows.

[Souvikns]

I couldn't find any solid document where it says for certain rule how to name env variables, but generally https://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap08.html is something we can use. We allow only uppercase, underscore, and digits but do not start with digits.

[derberg]

oh, I got here by accident from another PR.

I read it, even went to the glee PR (even though I do not know much about glee) and I really couldn't figure why we need this.

• I see the authorization use case with headerValues, but no for queryValues and looks to me like it is just added because headerValues are added
• I have to say it looks super strange to me that on the binding level we agree on some env variables and stuff like that. Is there some other use case for header other than authorization?

[Souvikns]

Query values can also be used for authentication, for example https://websockets.readthedocs.io/en/stable/topics/authentication.html#query-parameter and in glee I am trying to build a websocket client and there it has to have ways to pass in query and header as per different server requirements.

I have to say it looks super strange to me that on the binding level we agree on some env variables and stuff like that. Is there some other use case for header other than authorization?

Glee has to support the passing of env variables, but in the binding, we are just mentioning that you can pass env variables and the tools will support it. And if it is too specific we could just use extensions (I think there is an issue about supporting extensions on bindings, I couldn't find it 😞 )

[fmvilas]

A few things here that we may have to revise or think about further:

1. As discussed offline today, it's weird that bindings make a reference to environment variables. There may not be environment variables when processing the AsyncAPI document. E.g., if we parse the document in the browser. So tightly coupling this binding to a specific environment doesn't seem like a good idea after all.
2. What happens if someone specifies headers and headerValues (or query and queryValues)? Should the value be valid against the header definition and match exactly the value? In a WS server, what if we find a value like Authorization $TOKEN? The token may vary per user (or other business logic) and is not strictly related to an environment variable as we could think on a non-browser client. We can always consider that it's a responsibility of the author but it's definitely worth mentioning somewhere if we go down this path. There's this PR taking the same approach but using either a Schema Object or a string but not both, and not adding anything about variables.
3. With the env var syntax, I'm starting to have the feeling that we're trying to solve a Glee problem on the binding. If a value is dynamic, it shouldn't probably be on the document. Or maybe we should come up with a feature for the whole spec. Something like "variables" that will be needed when parsing an AsyncAPI document. We definitely have another similar case here: https://github.com/asyncapi/avro-schema-parser#hardcoded-key-and-secret. Pointing to a schema registry often requires authentication and hardcoding the username and password is not really an option.

[derberg]

The thing is that in the case of client generation for REST API for example, using OpenAPI, you also do not generate anything authentication specific. You just enable user to pass token or whatever authorization is needed. Shouldn't Glee do the same.

although I do not know much about Glee use case. I just do not fully understand how is this helping to have these specific environment variables provided by the user in the AsyncAPI document.

@Souvikns, maybe you wanna drop that topic on one of community meetings, to talk about it and brainstorm in a group. Don't know, just throwing ideas.

[Souvikns]

Nice Idea @derberg I will add this in agenda for next spec 3.0 meeeting asyncapi/community#476

[jonaslagoni]

Just gonna leave my 5 cents here since we discussed it in the last spec 3.0 meeting.

After asyncapi/glee#319 glee acts like a client so it actually needs to pass headers and query to the server which is something missing in the current websocket bindings

I actually disagree here tbh. Something is telling me that runtime variables that are defined somewhere in your implementation is not something the spec or bindings should try and map to 🤔 That really comes down to implementation which the document in principle has no authority over it just defines the interface for interaction.

I guess I agree with @fmvilas comment here #144 (comment).

Some additional comments in case you pursue it 😄:

• I would not recommend splitting the values from their structure definition unless it is strictly necessary to allow complex properties such as value templating. Otherwise, I see no reason why it should not be part of the schema definition:

/notification:
  bindings:
    ws:
      headers:
        type: object
        properties:
          Authorization: 
            type: string
            const: "bearer token"
            enum: ["bearer token", "bearer token2"] 

• Is there a reason behind not utilizing a security object for defining the Authorization header? I would assume it would be part of that 🤔
• I would really be careful about defining custom templating syntax for bindings cause remember, someone has to understand it. Not just for your solution, in glee where you have control over it, but any time tooling support ws it needs to be aware of this. So the more core concept it can be the better.
• Since this feels a lot like a glee problem, maybe it makes sense to introduce a x-glee extension specifically for glee use cases?

That was just what came to mind 😄

[Souvikns]

Since this feels a lot like a glee problem, maybe it makes sense to introduce a x-glee extension specifically for glee use cases?

I totally agree with this, going the extension route was our first idea, looking at this https://github.com/asyncapi/spec-json-schemas/blob/master/definitions/2.5.0/bindingsObject.json I think there is no check for extensions in bindings. So can we use bindings in extensions? @jonaslagoni

One more update here in the meeting @derberg suggested for using server bindings for making connections, I researched it and I don't think we can use that for our use case we are creating a new client connection using the server URL and the channel path to connect to a specific ws server. So ws://localhost:3000/ and ws://localhost:3000/user are two separate ws servers so it has to be channel bindings.

[derberg]

looking at this https://github.com/asyncapi/spec-json-schemas/blob/master/definitions/2.5.0/bindingsObject.json I think there is no check for extensions in bindings. So can we use bindings in extensions? @jonaslagoni

Yeah, so current JSON Schema for AsyncAPI has no integration with bindings schema files. Thus anything is accepted by validation based on schema.

There is no clear answer to your questions, neither in main spec or binding specs. We only have it defined in bindings schemas, like here for example: https://github.com/asyncapi/bindings/blob/master/googlepubsub/json_schemas/channel.json#L10

@Souvikns would be awesome if you could open an issue, probably in spec repo, as a question to get it clarified and described, maybe in spec as general rule, I'm not 100% sure

[KhudaDad414]

@Souvikns Hey, can we close this or are you still working on it?

[Souvikns]

Yeah we can close this