Skip to content

Commit

Permalink
Store 'http1' in routing table if HTTP/2 is disabled
Browse files Browse the repository at this point in the history
- When routes are registered and HTTP/2 is disabled, persist the
protocol for endpoints as 'http1', even if the message has protocol
'http2'.
- It was confusing that the route registry and gorouter logs would
show protocol 'http2', even if it would really use HTTP/1.1 when
communicating with backends.
- Enabling HTTP/2 will require restarting the gorouter, so the route
registry will be regenerated with 'http2'.

[cloudfoundry/routing-release#217]
[cloudfoundry/routing-release#200]
  • Loading branch information
Gerg authored and geofffranks committed Sep 30, 2021
1 parent a9449a9 commit 5838b6b
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 5 deletions.
10 changes: 6 additions & 4 deletions mbus/subscriber.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type RegistryMessage struct {
EndpointUpdatedAtNs int64 `json:"endpoint_updated_at_ns"`
}

func (rm *RegistryMessage) makeEndpoint() (*route.Endpoint, error) {
func (rm *RegistryMessage) makeEndpoint(http2Enabled bool) (*route.Endpoint, error) {
port, useTLS, err := rm.port()
if err != nil {
return nil, err
Expand All @@ -53,7 +53,7 @@ func (rm *RegistryMessage) makeEndpoint() (*route.Endpoint, error) {
}

protocol := rm.Protocol
if protocol == "" {
if protocol == "" || (!http2Enabled && protocol == "http2") {
protocol = "http1"
}

Expand Down Expand Up @@ -95,6 +95,7 @@ type Subscriber struct {
subscription *nats.Subscription
reconnected <-chan Signal
natsPendingLimit int
http2Enabled bool

params startMessageParams

Expand Down Expand Up @@ -131,6 +132,7 @@ func NewSubscriber(
reconnected: reconnected,
natsPendingLimit: c.NatsClientMessageBufferSize,
logger: l,
http2Enabled: c.EnableHTTP2,
}
}

Expand Down Expand Up @@ -233,7 +235,7 @@ func (s *Subscriber) subscribeRoutes() (*nats.Subscription, error) {
}

func (s *Subscriber) registerEndpoint(msg *RegistryMessage) {
endpoint, err := msg.makeEndpoint()
endpoint, err := msg.makeEndpoint(s.http2Enabled)
if err != nil {
s.logger.Error("Unable to register route",
zap.Error(err),
Expand All @@ -248,7 +250,7 @@ func (s *Subscriber) registerEndpoint(msg *RegistryMessage) {
}

func (s *Subscriber) unregisterEndpoint(msg *RegistryMessage) {
endpoint, err := msg.makeEndpoint()
endpoint, err := msg.makeEndpoint(s.http2Enabled)
if err != nil {
s.logger.Error("Unable to unregister route",
zap.Error(err),
Expand Down
32 changes: 31 additions & 1 deletion mbus/subscriber_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ var _ = Describe("Subscriber", func() {
})

Context("when the message contains a protocol", func() {
BeforeEach(func() {
JustBeforeEach(func() {
sub = mbus.NewSubscriber(natsClient, registry, cfg, reconnected, l)
process = ifrit.Invoke(sub)
Eventually(process.Ready()).Should(BeClosed())
Expand Down Expand Up @@ -410,6 +410,36 @@ var _ = Describe("Subscriber", func() {

Expect(originalEndpoint).To(Equal(expectedEndpoint))
})

Context("when HTTP/2 is disabled and the protocol is http2", func() {
BeforeEach(func() {
cfg.EnableHTTP2 = false
})
It("constructs the endpoint with protocol 'http1'", func() {
msg := mbus.RegistryMessage{
Host: "host",
App: "app",
Protocol: "http2",
Uris: []route.Uri{"test.example.com"},
}

data, err := json.Marshal(msg)
Expect(err).NotTo(HaveOccurred())

err = natsClient.Publish("router.register", data)
Expect(err).ToNot(HaveOccurred())

Eventually(registry.RegisterCallCount).Should(Equal(1))
_, originalEndpoint := registry.RegisterArgsForCall(0)
expectedEndpoint := route.NewEndpoint(&route.EndpointOpts{
Host: "host",
AppId: "app",
Protocol: "http1",
})

Expect(originalEndpoint).To(Equal(expectedEndpoint))
})
})
})

Context("when the message contains a tls port for route", func() {
Expand Down

0 comments on commit 5838b6b

Please sign in to comment.