consul/connect: Add support for Connect terminating gateways #9829

shoenig · 2021-01-15T16:01:36Z

This PR implements Nomad built-in support for running Consul Connect
terminating gateways. Such a gateway can be used by services running
inside the service mesh to access "legacy" services outside the mesh
while still making use of Consul's service identity based networking and
ACL policies.

https://www.consul.io/docs/connect/gateways/terminating-gateway

These gateways are declared as part of a task group level service
definition within the connect stanza.

service {
  connect {
    gateway {
      proxy {
        // envoy proxy configuration
      }
      terminating {
        // terminating-gateway configuration entry
      }
    }
  }
}

Currently Envoy is the only supported gateway implementation in
Consul. The gateway task can be customized by configuring the
connect.sidecar_task block or meta.connect.gateway_image
client attribute.

When the gateway.terminating field is set, Nomad will write/update
the Configuration Entry into Consul on job submission. Because CEs
are global in scope and there may be more than one Nomad cluster
communicating with Consul, there is an assumption that any terminating
gateway defined in Nomad for a particular service will be the same
among Nomad clusters.

Gateways require Consul 1.8.0+, checked by a node constraint.

Closes #9445

drewbailey · 2021-01-21T14:53:10Z

nomad/consul.go

+	return c.setCE(ctx, convertTerminatingCE(service, entry))
+}
+
+// also mesh


Suggested change

// also mesh

tgross

This looks great @shoenig! A heck of a lot of work to plumb this all in!

I've left a few comments and questions, but nothing that should be blocking.

tgross · 2021-01-21T17:59:14Z

contributing/checklist-jobspec.md

@@ -2,21 +2,25 @@

 ## Code


Thanks for bringing this up-to-date!

nomad/structs/connect.go

api/services.go

tgross · 2021-01-21T18:52:55Z

nomad/job_endpoint.go

 			return err
 		}
 	}
+	for service, entry := range entries.Terminating {
+		fmt.Println("SH JE set terminating CE", service)


Looks like we missed a leftover debug message

command/agent/consul/connect.go

tgross · 2021-01-21T19:08:52Z

nomad/job_endpoint_hook_connect.go

+// networking is being used the network_mode driver configuration is set here.
+func connectGatewayDriverConfig(hostNetwork bool) map[string]interface{} {
+	m := map[string]interface{}{
+		"image": envoy.GatewayConfigVar,


This looks like the only value besides the network mode that's different between the gateway task config and the sidecar task config... but the docs suggest that the same sidecar_task {} block is used for either. So I can't figure out where this value is coming from?

The config var variables represent meta.sidecar_image and meta.gateway_image for when configuring a sidecar or gateway image through client configuration (leaving room for them to be different). When setting the task in jobspec, sidecar_task serves double duty since only 1 of sidecar_service or gateway can be configured per connect block. In retrospect, sidecar_task would have been better named connect_task or similar . (The other alternative would be duplicating the sidecar_task plumbing just to have another field named gateway_task).

Aye, that all makes sense. It's probably worth spelling some of these details out in the documentation before we ship, just so that the relationship between those configuration values is super-clear.

nickethier

This looks really good and I especially like the code cleanup/tidiness done throughout.

One general question I had that shouldn't hold up this PR is if we could show an example of integrating with vault's pki backend to pull the ca and maybe the cert and key too for mutual tls. Maybe thats better saved for a learn guide though. Thoughts?

This PR implements Nomad built-in support for running Consul Connect terminating gateways. Such a gateway can be used by services running inside the service mesh to access "legacy" services running outside the service mesh while still making use of Consul's service identity based networking and ACL policies. https://www.consul.io/docs/connect/gateways/terminating-gateway These gateways are declared as part of a task group level service definition within the connect stanza. service { connect { gateway { proxy { // envoy proxy configuration } terminating { // terminating-gateway configuration entry } } } } Currently Envoy is the only supported gateway implementation in Consul. The gateay task can be customized by configuring the connect.sidecar_task block. When the gateway.terminating field is set, Nomad will write/update the Configuration Entry into Consul on job submission. Because CEs are global in scope and there may be more than one Nomad cluster communicating with Consul, there is an assumption that any terminating gateway defined in Nomad for a particular service will be the same among Nomad clusters. Gateways require Consul 1.8.0+, checked by a node constraint. Closes #9445

This parameter is now supposed to be non-nil even if empty, and the Copy method should also maintain that invariant.

shoenig · 2021-01-25T16:56:14Z

Yeah @nickethier we definitely need learn guides around this and also ingress gateways. It's on my list to get some first drafts written around 1.1

github-actions · 2022-11-30T02:19:15Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

shoenig force-pushed the f-terminating-gateway branch from c22ab5a to 36b0969 Compare January 15, 2021 16:06

vercel bot temporarily deployed to Preview – nomad-storybook-and-ui January 15, 2021 16:06 Inactive

vercel bot deployed to Preview – nomad January 15, 2021 16:06 View deployment

shoenig marked this pull request as ready for review January 15, 2021 17:29

shoenig requested review from nickethier and schmichael January 15, 2021 17:29

shoenig force-pushed the f-terminating-gateway branch from 36b0969 to 04bf502 Compare January 21, 2021 14:12

vercel bot deployed to Preview – nomad January 21, 2021 14:12 View deployment

vercel bot temporarily deployed to Preview – nomad-storybook-and-ui January 21, 2021 14:12 Inactive

shoenig requested review from tgross and drewbailey January 21, 2021 14:41

drewbailey reviewed Jan 21, 2021

View reviewed changes

nomad/consul.go

return c.setCE(ctx, convertTerminatingCE(service, entry))

}

// also mesh

Copy link

Contributor

drewbailey Jan 21, 2021

Choose a reason for hiding this comment

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

Suggested change

// also mesh

tgross approved these changes Jan 21, 2021

View reviewed changes

vercel bot temporarily deployed to Preview – nomad-storybook-and-ui January 22, 2021 16:19 Inactive

vercel bot temporarily deployed to Preview – nomad January 22, 2021 16:19 Inactive

shoenig force-pushed the f-terminating-gateway branch from 63eb6e7 to 6730de3 Compare January 22, 2021 16:41

vercel bot temporarily deployed to Preview – nomad-storybook-and-ui January 22, 2021 16:41 Inactive

vercel bot temporarily deployed to Preview – nomad January 22, 2021 16:41 Inactive

nickethier approved these changes Jan 25, 2021

View reviewed changes

vercel bot temporarily deployed to Preview – nomad January 25, 2021 16:34 Inactive

vercel bot temporarily deployed to Preview – nomad-storybook-and-ui January 25, 2021 16:34 Inactive

shoenig added 3 commits January 25, 2021 10:36

consul/connect: remove debug line

37544ca

consul/connect: copy bind address map if empty

74780f0

This parameter is now supposed to be non-nil even if empty, and the Copy method should also maintain that invariant.

shoenig force-pushed the f-terminating-gateway branch from 10a1eb2 to 74780f0 Compare January 25, 2021 16:36

vercel bot temporarily deployed to Preview – nomad-storybook-and-ui January 25, 2021 16:36 Inactive

vercel bot temporarily deployed to Preview – nomad January 25, 2021 16:36 Inactive

shoenig merged commit 9db4663 into master Jan 25, 2021

shoenig deleted the f-terminating-gateway branch January 25, 2021 16:56

github-actions bot locked as resolved and limited conversation to collaborators Nov 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

consul/connect: Add support for Connect terminating gateways #9829

consul/connect: Add support for Connect terminating gateways #9829

shoenig commented Jan 15, 2021

drewbailey Jan 21, 2021

tgross left a comment

tgross Jan 21, 2021

tgross Jan 21, 2021

shoenig Jan 22, 2021

tgross Jan 21, 2021

shoenig Jan 22, 2021 •

edited

Loading

tgross Jan 22, 2021

nickethier left a comment

shoenig commented Jan 25, 2021

github-actions bot commented Nov 30, 2022

consul/connect: Add support for Connect terminating gateways #9829

consul/connect: Add support for Connect terminating gateways #9829

Conversation

shoenig commented Jan 15, 2021

drewbailey Jan 21, 2021

Choose a reason for hiding this comment

tgross left a comment

Choose a reason for hiding this comment

tgross Jan 21, 2021

Choose a reason for hiding this comment

tgross Jan 21, 2021

Choose a reason for hiding this comment

shoenig Jan 22, 2021

Choose a reason for hiding this comment

tgross Jan 21, 2021

Choose a reason for hiding this comment

shoenig Jan 22, 2021 • edited Loading

Choose a reason for hiding this comment

tgross Jan 22, 2021

Choose a reason for hiding this comment

nickethier left a comment

Choose a reason for hiding this comment

shoenig commented Jan 25, 2021

github-actions bot commented Nov 30, 2022

shoenig Jan 22, 2021 •

edited

Loading