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

[ADDED] Websocket support #1309

Merged
merged 1 commit into from May 20, 2020
Merged

[ADDED] Websocket support #1309

merged 1 commit into from May 20, 2020

Conversation

kozlovic
Copy link
Member

@kozlovic kozlovic commented Mar 9, 2020

Websocket support can be enabled with a new websocket
configuration block:

websocket {
    # Specify a host and port to listen for websocket connections
    # listen: "host:port"

    # It can also be configured with individual parameters,
    # namely host and port.
    # host: "hostname"
    # port: 4443

    # This will optionally specify what host:port for websocket
    # connections to be advertised in the cluster
    # advertise: "host:port"

    # TLS configuration is required
    tls {
      cert_file: "/path/to/cert.pem"
      key_file: "/path/to/key.pem"
    }

    # If same_origin is true, then the Origin header of the
    # client request must match the request's Host.
    # same_origin: true

    # This list specifies the only accepted values for
    # the client's request Origin header. The scheme,
    # host and port must match. By convention, the
    # absence of port for an http:// scheme will be 80,
    # and for https:// will be 443.
    # allowed_origins [
    #    "http://www.example.com"
    #    "https://www.other-example.com"
    # ]

    # This enables support for compressed websocket frames
    # in the server. For compression to be used, both server
    # and client have to support it.
    # compression: true

    # This is the total time allowed for the server to
    # read the client request and write the response back
    # to the client. This include the time needed for the
    # TLS handshake.
    # handshake_timeout: "2s"
}

Signed-off-by: Ivan Kozlovic ivan@synadia.com

server/opts.go Outdated Show resolved Hide resolved
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally LGTM, a few comments.

server/client.go Show resolved Hide resolved
server/client.go Outdated Show resolved Hide resolved
server/opts.go Show resolved Hide resolved
server/opts.go Outdated Show resolved Hide resolved
server/server.go Outdated Show resolved Hide resolved
server/websocket.go Outdated Show resolved Hide resolved
Copy link
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wallyqs
Copy link
Member

wallyqs commented Mar 16, 2020

A couple of items to add:

  • Currently when ws is enabled and you connect to a NATS cluster on the websocket port, the connect_urls will be using the port for the clients (like "connect_urls":["172.31.10.166:4222","172.31.10.124:4222"] ). Disabling advertise would disable this for both the ws and regular connections though. Thinking that maybe there will be a use case were we want external ip gossip client enabled for both websockets and regular nats clients so we should not advertise the client port to websocket clients.

  • Something slightly different with accepting websocket connections and accepting regular NATS connections is that we do not account for websocket connection attempts, so if a client makes many bad requests (e.g. connecting without TLS) there will be no trace in the logs either for example. There is a ConnState func(net.Conn, ConnState) hook that we could implement in the http.Server that could help for debug/trace when a new websocket connection attempt is done.

@kozlovic
Copy link
Member Author

Currently when ws is enabled and you connect to a NATS cluster, the connect_urls will be using the port for the clients (like "connect_urls":["172.31.10.166:4222","172.31.10.124:4222"] ).

Keep in mind that websocket{} can define its own host so possibly not the same that the client. I was thinking that if we want to do advertise for WS clients, it would have to be a new field in the INFO. Unless we force the websocket{} to accept only port and fallback to same host than client, in which case we could have connect_urls actually contain only host and have new fields for client port, ws port, this only when sending to WS clients (otherwise it would break backward compatibility).

Something slightly different with accepting websocket connections and accepting regular NATS connections is ...

Good to know, will look into that.

@wallyqs
Copy link
Member

wallyqs commented Mar 16, 2020

Ah ok, then for example right now if you connect to a cluster on the websocket port you would get the "connect_urls" entries with the client port (4222), but since we could use a different field for advertising websockets then clients that connect using the ws port would ignore that one and use the other field with the websocket urls.

@kozlovic
Copy link
Member Author

Yes, to be clear, there is no advertise implemented for Websockets as of now, so the connect_urls is the one that you normally get for (regular) client connections.

@derekcollison
Copy link
Member

Feels like we should no? Its a ws client, so should only have connect_urls that are applicable to its client type. Or empty to start with. Most likely these will be behind a LB so might not be as critical.

@kozlovic
Copy link
Member Author

Feels like we should no? Its a ws client, so should only have connect_urls that are applicable to its client type. Or empty to start with. Most likely these will be behind a LB so might not be as critical.

Yes, but the point is that nodes in the cluster do exchange their client's listen spec, which means that this need to be implemented. As I said, if we allow user to specify a different "host" for the listen, then we have to re-implement the current client host:port gossip between servers, not only from server to WS clients.

@derekcollison
Copy link
Member

Understand regarding host.

@derekcollison
Copy link
Member

Thinking with MQTT we may need to support it..

Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general LGTM..

Websocket support can be enabled with a new websocket
configuration block:

```
websocket {
    # Specify a host and port to listen for websocket connections
    # listen: "host:port"

    # It can also be configured with individual parameters,
    # namely host and port.
    # host: "hostname"
    # port: 4443

    # This will optionally specify what host:port for websocket
    # connections to be advertised in the cluster
    # advertise: "host:port"

    # TLS configuration is required
    tls {
      cert_file: "/path/to/cert.pem"
      key_file: "/path/to/key.pem"
    }

    # If same_origin is true, then the Origin header of the
    # client request must match the request's Host.
    # same_origin: true

    # This list specifies the only accepted values for
    # the client's request Origin header. The scheme,
    # host and port must match. By convention, the
    # absence of port for an http:// scheme will be 80,
    # and for https:// will be 443.
    # allowed_origins [
    #    "http://www.example.com"
    #    "https://www.other-example.com"
    # ]

    # This enables support for compressed websocket frames
    # in the server. For compression to be used, both server
    # and client have to support it.
    # compression: true

    # This is the total time allowed for the server to
    # read the client request and write the response back
    # to the client. This include the time needed for the
    # TLS handshake.
    # handshake_timeout: "2s"
}
```

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@kozlovic kozlovic merged commit bd976e6 into master May 20, 2020
@kozlovic kozlovic deleted the websocket branch May 20, 2020 19:14
@vinsguru
Copy link

@kozlovic @wallyqs
Could you please confirm if this feature is available in 2.1.7 version? I get this error.

Error:

unknown field "websocket"

My conf:

port: 4222
websocket: {
  port: 9222
  no_tls: true
}

@aricart
Copy link
Member

aricart commented Aug 25, 2020

@vinsguru the feature is present in the master branch, and on nightly JetStream images, which you can get by following instructions here https://github.com/nats-io/jetstream#using-docker. The feature is not yet released.

@ripienaar
Copy link
Contributor

We also have nightly server images in synadia/nats-server:nightly

@vinsguru
Copy link

@aricart @ripienaar
Thank you so much for all your contribution and support!
I recently started exploring nats and It is awesome!

@fuadhasni
Copy link

fuadhasni commented Sep 22, 2020

Literally a sigh of relief, as I have been struggling to make wss work with tcp-relay.
pardon me for asking, when this feature is going to be released?

@kozlovic
Copy link
Member Author

At the next release that is targeted Q4 of this year, but the feature is already available from the main branch and nightly builds: docker pull synadia/nats-server:nightly

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

Successfully merging this pull request may close these issues.

None yet

7 participants