From b874c8ef0c013113c07acbea98fb4cf545482905 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 5 Jun 2020 14:54:29 -0700 Subject: [PATCH 1/6] Add connect expose CLI command --- agent/config_endpoint.go | 4 +- command/commands_oss.go | 4 +- command/connect/expose/expose.go | 236 +++++++++++++++++++++ command/connect/expose/expose_test.go | 289 ++++++++++++++++++++++++++ command/intention/create/create.go | 8 +- 5 files changed, 535 insertions(+), 6 deletions(-) create mode 100644 command/connect/expose/expose.go create mode 100644 command/connect/expose/expose_test.go diff --git a/agent/config_endpoint.go b/agent/config_endpoint.go index be8c11e334..3b1967eba3 100644 --- a/agent/config_endpoint.go +++ b/agent/config_endpoint.go @@ -9,6 +9,8 @@ import ( "github.com/hashicorp/consul/agent/structs" ) +const ConfigEntryNotFoundErr string = "Config entry not found" + // Config switches on the different CRUD operations for config entries. func (s *HTTPServer) Config(resp http.ResponseWriter, req *http.Request) (interface{}, error) { switch req.Method { @@ -48,7 +50,7 @@ func (s *HTTPServer) configGet(resp http.ResponseWriter, req *http.Request) (int setMeta(resp, &reply.QueryMeta) if reply.Entry == nil { - return nil, NotFoundError{Reason: fmt.Sprintf("Config entry not found for %q / %q", pathArgs[0], pathArgs[1])} + return nil, NotFoundError{Reason: fmt.Sprintf("%s for %q / %q", ConfigEntryNotFoundErr, pathArgs[0], pathArgs[1])} } return reply.Entry, nil diff --git a/command/commands_oss.go b/command/commands_oss.go index cf419cad4b..e7237a1884 100644 --- a/command/commands_oss.go +++ b/command/commands_oss.go @@ -51,7 +51,8 @@ import ( caget "github.com/hashicorp/consul/command/connect/ca/get" caset "github.com/hashicorp/consul/command/connect/ca/set" "github.com/hashicorp/consul/command/connect/envoy" - "github.com/hashicorp/consul/command/connect/envoy/pipe-bootstrap" + pipebootstrap "github.com/hashicorp/consul/command/connect/envoy/pipe-bootstrap" + "github.com/hashicorp/consul/command/connect/expose" "github.com/hashicorp/consul/command/connect/proxy" "github.com/hashicorp/consul/command/debug" "github.com/hashicorp/consul/command/event" @@ -169,6 +170,7 @@ func init() { Register("connect proxy", func(ui cli.Ui) (cli.Command, error) { return proxy.New(ui, MakeShutdownCh()), nil }) Register("connect envoy", func(ui cli.Ui) (cli.Command, error) { return envoy.New(ui), nil }) Register("connect envoy pipe-bootstrap", func(ui cli.Ui) (cli.Command, error) { return pipebootstrap.New(ui), nil }) + Register("connect expose", func(ui cli.Ui) (cli.Command, error) { return expose.New(ui), nil }) Register("debug", func(ui cli.Ui) (cli.Command, error) { return debug.New(ui, MakeShutdownCh()), nil }) Register("event", func(ui cli.Ui) (cli.Command, error) { return event.New(ui), nil }) Register("exec", func(ui cli.Ui) (cli.Command, error) { return exec.New(ui, MakeShutdownCh()), nil }) diff --git a/command/connect/expose/expose.go b/command/connect/expose/expose.go new file mode 100644 index 0000000000..3f0c81c08e --- /dev/null +++ b/command/connect/expose/expose.go @@ -0,0 +1,236 @@ +package expose + +import ( + "flag" + "fmt" + "strconv" + "strings" + + "github.com/hashicorp/consul/agent" + "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/command/flags" + "github.com/hashicorp/consul/command/intention/create" + "github.com/hashicorp/consul/command/intention/finder" + "github.com/mitchellh/cli" +) + +func New(ui cli.Ui) *cmd { + c := &cmd{UI: ui} + c.init() + return c +} + +type cmd struct { + UI cli.Ui + flags *flag.FlagSet + http *flags.HTTPFlags + help string + + // flags + ingressGateway string + service string + portRaw string + port int + protocol string +} + +func (c *cmd) init() { + c.flags = flag.NewFlagSet("", flag.ContinueOnError) + c.flags.StringVar(&c.ingressGateway, "ingress-gateway", "", + "The name of the ingress gateway service to use. Required.") + + c.flags.StringVar(&c.service, "service", "", + "The name of destination service to expose. Required.") + + c.flags.StringVar(&c.portRaw, "port", "", + "The listener port to use for the service on the Ingress gateway. Required.") + + c.flags.StringVar(&c.protocol, "protocol", "tcp", + "The protocol for the service. Defaults to 'tcp'. Optional.") + + c.http = &flags.HTTPFlags{} + flags.Merge(c.flags, c.http.ClientFlags()) + flags.Merge(c.flags, c.http.ServerFlags()) + c.help = flags.Usage(help, c.flags) +} + +func (c *cmd) Run(args []string) int { + if err := c.flags.Parse(args); err != nil { + if err == flag.ErrHelp { + return 0 + } + c.UI.Error(fmt.Sprintf("Failed to parse args: %v", err)) + return 1 + } + + // Set up a client. + client, err := c.http.APIClient() + if err != nil { + c.UI.Error(fmt.Sprintf("Error initializing client: %s", err)) + return 1 + } + + // Check for any missing or invalid flag values. + if c.service == "" { + c.UI.Error("A service name must be given via the -service flag.") + return 1 + } + svc, svcNamespace, err := create.ParseIntentionTarget(c.service) + if err != nil { + c.UI.Error(fmt.Sprintf("Invalid service name: %s", err)) + return 1 + } + + if c.ingressGateway == "" { + c.UI.Error("An ingress gateway service must be given via the -ingress-gateway flag.") + return 1 + } + gateway, gatewayNamespace, err := create.ParseIntentionTarget(c.ingressGateway) + if err != nil { + c.UI.Error(fmt.Sprintf("Invalid ingress gateway name: %s", err)) + return 1 + } + + if c.portRaw == "" { + c.UI.Error("A port must be provided via the -port flag.") + return 1 + } else { + c.port, err = strconv.Atoi(c.portRaw) + if err != nil { + c.UI.Error(fmt.Sprintf("Error parsing port: %s", err)) + return 1 + } + } + + // First get the config entry for the ingress gateway, if it exists. Don't error if it's a 404 as that + // just means we'll need to create a new config entry. + conf, _, err := client.ConfigEntries().Get(api.IngressGateway, gateway, nil) + if err != nil && !strings.Contains(err.Error(), agent.ConfigEntryNotFoundErr) { + c.UI.Error(fmt.Sprintf("Error fetching existing ingress gateway configuration: %s", err)) + return 1 + } + if conf == nil { + conf = &api.IngressGatewayConfigEntry{ + Kind: api.IngressGateway, + Name: gateway, + Namespace: gatewayNamespace, + } + } + + // Make sure the flags don't conflict with existing config. + ingressConf, ok := conf.(*api.IngressGatewayConfigEntry) + if !ok { + // This should never happen + c.UI.Error(fmt.Sprintf("Config entry is an invalid type: %T", conf)) + return 1 + } + + listenerIdx := -1 + for i, listener := range ingressConf.Listeners { + // Make sure the service isn't already exposed in this gateway + for _, service := range listener.Services { + if service.Name == svc { + c.UI.Error(fmt.Sprintf("Service %q already exposed through listener with port %d", svc, listener.Port)) + goto CREATE_INTENTION + } + } + + // If there's already a listener for the given port, make sure the protocol matches. + if listener.Port == c.port { + listenerIdx = i + if listener.Protocol != c.protocol { + c.UI.Error(fmt.Sprintf("Listener on port %d already configured with conflicting protocol %q", listener.Port, listener.Protocol)) + return 1 + } + } + } + + // Add a service to the existing listener for the port if one exists, or make a new listener. + if listenerIdx >= 0 { + ingressConf.Listeners[listenerIdx].Services = append(ingressConf.Listeners[listenerIdx].Services, api.IngressService{ + Name: svc, + Namespace: svcNamespace, + }) + } else { + ingressConf.Listeners = append(ingressConf.Listeners, api.IngressListener{ + Port: c.port, + Protocol: c.protocol, + Services: []api.IngressService{ + { + Name: svc, + Namespace: svcNamespace, + }, + }, + }) + } + + // Write the updated config entry using a check-and-set, so it fails if the entry + // has been changed since we looked it up. + { + succeeded, _, err := client.ConfigEntries().CAS(ingressConf, ingressConf.GetModifyIndex(), nil) + if err != nil { + c.UI.Error(fmt.Sprintf("Error writing ingress config entry: %v", err)) + return 1 + } + if !succeeded { + c.UI.Error("Ingress config entry was changed while attempting to update, please try again.") + return 1 + } + c.UI.Output(fmt.Sprintf("Successfully updated config entry for ingress service %q", gateway)) + } + +CREATE_INTENTION: + // Check for an existing intention. + ixnFinder := finder.Finder{Client: client} + existing, err := ixnFinder.Find(c.ingressGateway, c.service) + if err != nil { + c.UI.Error(fmt.Sprintf("Error looking up existing intention: %s", err)) + return 1 + } + if existing != nil && existing.Action == api.IntentionActionAllow { + c.UI.Error(fmt.Sprintf("Intention already exists for %q -> %q", c.ingressGateway, c.service)) + return 0 + } + + // Add the intention between the gateway service and the destination. + ixn := &api.Intention{ + SourceName: gateway, + SourceNS: gatewayNamespace, + DestinationName: svc, + DestinationNS: svcNamespace, + SourceType: api.IntentionSourceConsul, + Action: api.IntentionActionAllow, + } + if existing == nil { + _, _, err = client.Connect().IntentionCreate(ixn, nil) + if err != nil { + c.UI.Error(fmt.Sprintf("Error creating intention: %s", err)) + return 1 + } + } else { + _, err = client.Connect().IntentionUpdate(ixn, nil) + if err != nil { + c.UI.Error(fmt.Sprintf("Error updating intention: %s", err)) + return 1 + } + } + + c.UI.Output(fmt.Sprintf("Successfully set up intention for %q -> %q", c.ingressGateway, c.service)) + return 0 +} + +func (c *cmd) Synopsis() string { + return synopsis +} + +func (c *cmd) Help() string { + return c.help +} + +const synopsis = "Expose a Connect-enabled service through an Ingress gateway" +const help = ` +Usage: consul connect expose [options] + + Exposes a Connect-enabled service through the given ingress gateway, using the + given protocol and port. +` diff --git a/command/connect/expose/expose_test.go b/command/connect/expose/expose_test.go new file mode 100644 index 0000000000..18b650138a --- /dev/null +++ b/command/connect/expose/expose_test.go @@ -0,0 +1,289 @@ +package expose + +import ( + "testing" + + "github.com/hashicorp/consul/agent" + "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/testrpc" + "github.com/mitchellh/cli" + "github.com/stretchr/testify/require" +) + +func TestConnectExpose(t *testing.T) { + t.Parallel() + require := require.New(t) + a := agent.NewTestAgent(t, ``) + client := a.Client() + defer a.Shutdown() + + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + { + ui := cli.NewMockUi() + c := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-service=foo", + "-ingress-gateway=ingress", + "-port=8888", + "-protocol=tcp", + } + + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + } + + // Make sure the config entry and intention have been created. + entry, _, err := client.ConfigEntries().Get(api.IngressGateway, "ingress", nil) + require.NoError(err) + expected := &api.IngressGatewayConfigEntry{ + Kind: api.IngressGateway, + Name: "ingress", + Listeners: []api.IngressListener{ + { + Port: 8888, + Protocol: "tcp", + Services: []api.IngressService{ + { + Name: "foo", + }, + }, + }, + }, + } + expected.CreateIndex = entry.GetCreateIndex() + expected.ModifyIndex = entry.GetModifyIndex() + require.Equal(expected, entry) + + ixns, _, err := client.Connect().Intentions(nil) + require.NoError(err) + require.Len(ixns, 1) + require.Equal("ingress", ixns[0].SourceName) + require.Equal("foo", ixns[0].DestinationName) + + // Run the command again with a different port, make sure the config entry + // and intentions aren't modified. + { + ui := cli.NewMockUi() + c := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-service=foo", + "-ingress-gateway=ingress", + "-port=7777", + "-protocol=tcp", + } + + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + + // Make sure the config entry/intention weren't affected. + entry, _, err = client.ConfigEntries().Get(api.IngressGateway, "ingress", nil) + require.NoError(err) + require.Equal(expected, entry) + + ixns, _, err = client.Connect().Intentions(nil) + require.NoError(err) + require.Len(ixns, 1) + require.Equal("ingress", ixns[0].SourceName) + require.Equal("foo", ixns[0].DestinationName) + } + + // Run the command again with a conflicting protocol, should exit with an error and + // cause no changes to config entry/intentions. + { + ui := cli.NewMockUi() + c := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-service=bar", + "-ingress-gateway=ingress", + "-port=8888", + "-protocol=http", + } + + code := c.Run(args) + if code != 1 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + require.Contains(ui.ErrorWriter.String(), `conflicting protocol "tcp"`) + + // Make sure the config entry/intention weren't affected. + entry, _, err = client.ConfigEntries().Get(api.IngressGateway, "ingress", nil) + require.NoError(err) + require.Equal(expected, entry) + + ixns, _, err = client.Connect().Intentions(nil) + require.NoError(err) + require.Len(ixns, 1) + require.Equal("ingress", ixns[0].SourceName) + require.Equal("foo", ixns[0].DestinationName) + } +} + +func TestConnectExpose_invalidFlags(t *testing.T) { + t.Parallel() + require := require.New(t) + a := agent.NewTestAgent(t, ``) + defer a.Shutdown() + + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + t.Run("missing service", func(t *testing.T) { + ui := cli.NewMockUi() + c := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + } + + code := c.Run(args) + if code != 1 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + require.Contains(ui.ErrorWriter.String(), "A service name must be given") + }) + t.Run("missing gateway", func(t *testing.T) { + ui := cli.NewMockUi() + c := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-service=foo", + } + + code := c.Run(args) + if code != 1 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + require.Contains(ui.ErrorWriter.String(), "An ingress gateway service must be given") + }) + t.Run("missing port", func(t *testing.T) { + ui := cli.NewMockUi() + c := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-service=foo", + "-ingress-gateway=ingress", + } + + code := c.Run(args) + if code != 1 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + require.Contains(ui.ErrorWriter.String(), "A port must be provided") + }) +} + +func TestConnectExpose_existingConfig(t *testing.T) { + t.Parallel() + require := require.New(t) + a := agent.NewTestAgent(t, ``) + client := a.Client() + defer a.Shutdown() + + // Create some service config entries to set their protocol. + for _, service := range []string{"bar", "zoo"} { + _, _, err := client.ConfigEntries().Set(&api.ServiceConfigEntry{ + Kind: "service-defaults", + Name: service, + Protocol: "http", + }, nil) + require.NoError(err) + } + + // Create an existing ingress config entry with some services. + ingressConf := &api.IngressGatewayConfigEntry{ + Kind: api.IngressGateway, + Name: "ingress", + Listeners: []api.IngressListener{ + { + Port: 8888, + Protocol: "tcp", + Services: []api.IngressService{ + { + Name: "foo", + }, + }, + }, + { + Port: 9999, + Protocol: "http", + Services: []api.IngressService{ + { + Name: "bar", + }, + }, + }, + }, + } + _, _, err := client.ConfigEntries().Set(ingressConf, nil) + require.NoError(err) + + // Add a service on a new port. + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + { + ui := cli.NewMockUi() + c := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-service=baz", + "-ingress-gateway=ingress", + "-port=10000", + "-protocol=tcp", + } + + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + + // Make sure the ingress config was updated and existing services preserved. + entry, _, err := client.ConfigEntries().Get(api.IngressGateway, "ingress", nil) + require.NoError(err) + + ingressConf.Listeners = append(ingressConf.Listeners, api.IngressListener{ + Port: 10000, + Protocol: "tcp", + Services: []api.IngressService{ + { + Name: "baz", + }, + }, + }) + ingressConf.CreateIndex = entry.GetCreateIndex() + ingressConf.ModifyIndex = entry.GetModifyIndex() + require.Equal(ingressConf, entry) + } + + // Add an service on a port shared with an existing listener. + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + { + ui := cli.NewMockUi() + c := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-service=zoo", + "-ingress-gateway=ingress", + "-port=9999", + "-protocol=http", + } + + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + + // Make sure the ingress config was updated and existing services preserved. + entry, _, err := client.ConfigEntries().Get(api.IngressGateway, "ingress", nil) + require.NoError(err) + + ingressConf.Listeners[1].Services = append(ingressConf.Listeners[1].Services, api.IngressService{ + Name: "zoo", + }) + ingressConf.CreateIndex = entry.GetCreateIndex() + ingressConf.ModifyIndex = entry.GetModifyIndex() + require.Equal(ingressConf, entry) + } +} diff --git a/command/intention/create/create.go b/command/intention/create/create.go index 04ee8153e7..2f3b0b15a1 100644 --- a/command/intention/create/create.go +++ b/command/intention/create/create.go @@ -136,10 +136,10 @@ func (c *cmd) Run(args []string) int { return 0 } -// parseIntentionTarget parses a target of the form / and returns +// ParseIntentionTarget parses a target of the form / and returns // the two distinct parts. In some cases the namespace may be elided and this function // will return the empty string for the namespace then. -func parseIntentionTarget(input string) (name string, namespace string, err error) { +func ParseIntentionTarget(input string) (name string, namespace string, err error) { // Get the index to the '/'. If it doesn't exist, we have just a name // so just set that and return. idx := strings.IndexByte(input, '/') @@ -171,12 +171,12 @@ func (c *cmd) ixnsFromArgs(args []string) ([]*api.Intention, error) { return nil, fmt.Errorf("Must specify two arguments: source and destination") } - srcName, srcNamespace, err := parseIntentionTarget(args[0]) + srcName, srcNamespace, err := ParseIntentionTarget(args[0]) if err != nil { return nil, fmt.Errorf("Invalid intention source: %v", err) } - dstName, dstNamespace, err := parseIntentionTarget(args[1]) + dstName, dstNamespace, err := ParseIntentionTarget(args[1]) if err != nil { return nil, fmt.Errorf("Invalid intention destination: %v", err) } From ada9e2b3ab9d5e71468af2873a8d546e86c80003 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 5 Jun 2020 14:54:45 -0700 Subject: [PATCH 2/6] Add docs for expose command --- website/data/docs-navigation.js | 2 +- .../pages/docs/commands/connect/expose.mdx | 64 +++++++++++++++++++ website/pages/docs/commands/connect/index.mdx | 6 +- 3 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 website/pages/docs/commands/connect/expose.mdx diff --git a/website/data/docs-navigation.js b/website/data/docs-navigation.js index 1dbf4c4694..8e0931d4a6 100644 --- a/website/data/docs-navigation.js +++ b/website/data/docs-navigation.js @@ -61,7 +61,7 @@ export default [ 'agent', { category: 'catalog', content: ['datacenters', 'nodes', 'services'] }, { category: 'config', content: ['delete', 'list', 'read', 'write'] }, - { category: 'connect', content: ['ca', 'proxy', 'envoy'] }, + { category: 'connect', content: ['ca', 'proxy', 'envoy', 'expose'] }, 'debug', 'event', 'exec', diff --git a/website/pages/docs/commands/connect/expose.mdx b/website/pages/docs/commands/connect/expose.mdx new file mode 100644 index 0000000000..ce4818422f --- /dev/null +++ b/website/pages/docs/commands/connect/expose.mdx @@ -0,0 +1,64 @@ +--- +layout: docs +page_title: 'Commands: Connect Expose' +sidebar_title: expose +description: > + The connect expose subcommand is used to expose a Connect-enabled service + through an Ingress gateway by modifying the gateway's configuration and adding + an intention to allow traffic from the gateway to the service. +--- + +# Consul Connect Expose + +Command: `consul connect expose` + +The connect expose subcommand is used to expose a Connect-enabled service +through an Ingress gateway by modifying the gateway's configuration and adding +an intention to allow traffic from the gateway to the service. See the +[Ingress gateway documentation](/docs/connect/ingress-gateway) for more information +about Ingress gateways. + +```text +Usage: consul connect expose [options] + + Exposes a Connect-enabled service through the given ingress gateway, using the + given protocol and port. +``` + +#### API Options + +@include 'http_api_options_client.mdx' + +@include 'http_api_options_server.mdx' + +#### Expose Options + +- `-ingress-gateway` - The name of the ingress gateway service to use. Required. + +- `-port` - The listener port to use for the service on the Ingress gateway. + Required. + +- `-protocol` - The protocol for the service. Defaults to 'tcp'. Optional. + +- `-service` - The name of destination service to expose. Required. + +## Examples + +The example below shows using the `expose` command to make the `foo` service available +through the Ingress gateway service `ingress`. The protocol argument is optional and +defaults to `tcp` if not provided. + +```shell-session +$ consul connect expose -service=foo -ingress-gateway=ingress -port 8888 -protocol=tcp +Successfully updated config entry for ingress service "ingress" +Successfully set up intention for "ingress" -> "foo" +``` + +Running the command again when the config entry/intention are already set up will result +in a no-op: + +```shell-session +$ consul connect expose -service=foo -ingress-gateway=ingress -port 8888 -protocol=tcp +Service "foo" already exposed through listener with port 8888 +Intention already exists for "ingress" -> "foo" +``` diff --git a/website/pages/docs/commands/connect/index.mdx b/website/pages/docs/commands/connect/index.mdx index 020da9fe70..63b15c8005 100644 --- a/website/pages/docs/commands/connect/index.mdx +++ b/website/pages/docs/commands/connect/index.mdx @@ -35,8 +35,10 @@ Usage: consul connect [options] [args] For more examples, ask for subcommand help or view the documentation. Subcommands: - ca Interact with the Consul Connect Certificate Authority (CA) - proxy Runs a Consul Connect proxy + ca Interact with the Consul Connect Certificate Authority (CA) + envoy Runs or Configures Envoy as a Connect proxy + expose Expose a Connect-enabled service through an Ingress gateway + proxy Runs a Consul Connect proxy ``` For more information, examples, and usage about a subcommand, click on the name From acae044df4f73138c2e43f8f8f4fa7b523c66904 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 5 Jun 2020 15:47:03 -0700 Subject: [PATCH 3/6] Document the namespace format for expose CLI command --- command/connect/expose/expose.go | 10 ++++++---- website/pages/docs/commands/connect/expose.mdx | 14 +++++++++----- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/command/connect/expose/expose.go b/command/connect/expose/expose.go index 3f0c81c08e..3b6cd2ebb8 100644 --- a/command/connect/expose/expose.go +++ b/command/connect/expose/expose.go @@ -37,16 +37,18 @@ type cmd struct { func (c *cmd) init() { c.flags = flag.NewFlagSet("", flag.ContinueOnError) c.flags.StringVar(&c.ingressGateway, "ingress-gateway", "", - "The name of the ingress gateway service to use. Required.") + "(Required) The name of the ingress gateway service to use. A namespace "+ + "can optionally be specified as a prefix via the 'namespace/service' format.") c.flags.StringVar(&c.service, "service", "", - "The name of destination service to expose. Required.") + "(Required) The name of destination service to expose. A namespace "+ + "can optionally be specified as a prefix via the 'namespace/service' format.") c.flags.StringVar(&c.portRaw, "port", "", - "The listener port to use for the service on the Ingress gateway. Required.") + "(Required) The listener port to use for the service on the Ingress gateway.") c.flags.StringVar(&c.protocol, "protocol", "tcp", - "The protocol for the service. Defaults to 'tcp'. Optional.") + "The protocol for the service. Defaults to 'tcp'.") c.http = &flags.HTTPFlags{} flags.Merge(c.flags, c.http.ClientFlags()) diff --git a/website/pages/docs/commands/connect/expose.mdx b/website/pages/docs/commands/connect/expose.mdx index ce4818422f..88208b6d9d 100644 --- a/website/pages/docs/commands/connect/expose.mdx +++ b/website/pages/docs/commands/connect/expose.mdx @@ -33,14 +33,18 @@ Usage: consul connect expose [options] #### Expose Options -- `-ingress-gateway` - The name of the ingress gateway service to use. Required. +- `-ingress-gateway` - (Required) The name of the ingress gateway service to use. + A namespace can optionally be specified as a prefix via the + 'namespace/service' format -- `-port` - The listener port to use for the service on the Ingress gateway. - Required. +- `-port` - (Required) The listener port to use for the service on the Ingress + gateway. -- `-protocol` - The protocol for the service. Defaults to 'tcp'. Optional. +- `-protocol` - The protocol for the service. Defaults to 'tcp'. -- `-service` - The name of destination service to expose. Required. +- `-service` - (Required) The name of destination service to expose. A namespace + can optionally be specified as a prefix via the 'namespace/service' + format. ## Examples From 59583285527c662d8215d0d83668e34a7cf90409 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Mon, 8 Jun 2020 16:44:20 -0700 Subject: [PATCH 4/6] Allow multiple listeners per service via expose command --- command/connect/expose/expose.go | 18 +++++------------- command/connect/expose/expose_test.go | 13 ++++++++++++- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/command/connect/expose/expose.go b/command/connect/expose/expose.go index 3b6cd2ebb8..4a36b7ab1b 100644 --- a/command/connect/expose/expose.go +++ b/command/connect/expose/expose.go @@ -3,7 +3,6 @@ package expose import ( "flag" "fmt" - "strconv" "strings" "github.com/hashicorp/consul/agent" @@ -29,7 +28,6 @@ type cmd struct { // flags ingressGateway string service string - portRaw string port int protocol string } @@ -44,7 +42,7 @@ func (c *cmd) init() { "(Required) The name of destination service to expose. A namespace "+ "can optionally be specified as a prefix via the 'namespace/service' format.") - c.flags.StringVar(&c.portRaw, "port", "", + c.flags.IntVar(&c.port, "port", 0, "(Required) The listener port to use for the service on the Ingress gateway.") c.flags.StringVar(&c.protocol, "protocol", "tcp", @@ -93,15 +91,9 @@ func (c *cmd) Run(args []string) int { return 1 } - if c.portRaw == "" { + if c.port == 0 { c.UI.Error("A port must be provided via the -port flag.") return 1 - } else { - c.port, err = strconv.Atoi(c.portRaw) - if err != nil { - c.UI.Error(fmt.Sprintf("Error parsing port: %s", err)) - return 1 - } } // First get the config entry for the ingress gateway, if it exists. Don't error if it's a 404 as that @@ -131,8 +123,8 @@ func (c *cmd) Run(args []string) int { for i, listener := range ingressConf.Listeners { // Make sure the service isn't already exposed in this gateway for _, service := range listener.Services { - if service.Name == svc { - c.UI.Error(fmt.Sprintf("Service %q already exposed through listener with port %d", svc, listener.Port)) + if service.Name == svc && listener.Port == c.port { + c.UI.Output(fmt.Sprintf("Service %q already exposed through listener with port %d", svc, listener.Port)) goto CREATE_INTENTION } } @@ -190,7 +182,7 @@ CREATE_INTENTION: return 1 } if existing != nil && existing.Action == api.IntentionActionAllow { - c.UI.Error(fmt.Sprintf("Intention already exists for %q -> %q", c.ingressGateway, c.service)) + c.UI.Output(fmt.Sprintf("Intention already exists for %q -> %q", c.ingressGateway, c.service)) return 0 } diff --git a/command/connect/expose/expose_test.go b/command/connect/expose/expose_test.go index 18b650138a..7d8443404a 100644 --- a/command/connect/expose/expose_test.go +++ b/command/connect/expose/expose_test.go @@ -64,7 +64,7 @@ func TestConnectExpose(t *testing.T) { require.Equal("foo", ixns[0].DestinationName) // Run the command again with a different port, make sure the config entry - // and intentions aren't modified. + // is updated while intentions are unmodified. { ui := cli.NewMockUi() c := New(ui) @@ -81,9 +81,20 @@ func TestConnectExpose(t *testing.T) { t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) } + expected.Listeners = append(expected.Listeners, api.IngressListener{ + Port: 7777, + Protocol: "tcp", + Services: []api.IngressService{ + { + Name: "foo", + }, + }, + }) + // Make sure the config entry/intention weren't affected. entry, _, err = client.ConfigEntries().Get(api.IngressGateway, "ingress", nil) require.NoError(err) + expected.ModifyIndex = entry.GetModifyIndex() require.Equal(expected, entry) ixns, _, err = client.Connect().Intentions(nil) From edab5588d8be8c695b234ad15f0210696d0ead37 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Mon, 8 Jun 2020 16:59:47 -0700 Subject: [PATCH 5/6] Add -host flag to expose command --- command/connect/expose/expose.go | 6 ++++++ command/connect/expose/expose_test.go | 5 ++++- website/pages/docs/commands/connect/expose.mdx | 7 +++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/command/connect/expose/expose.go b/command/connect/expose/expose.go index 4a36b7ab1b..9714424ada 100644 --- a/command/connect/expose/expose.go +++ b/command/connect/expose/expose.go @@ -30,6 +30,7 @@ type cmd struct { service string port int protocol string + hosts flags.AppendSliceValue } func (c *cmd) init() { @@ -48,6 +49,9 @@ func (c *cmd) init() { c.flags.StringVar(&c.protocol, "protocol", "tcp", "The protocol for the service. Defaults to 'tcp'.") + c.flags.Var(&c.hosts, "host", "Additional DNS hostname to use for routing to this service."+ + "Can be specified multiple times.") + c.http = &flags.HTTPFlags{} flags.Merge(c.flags, c.http.ClientFlags()) flags.Merge(c.flags, c.http.ServerFlags()) @@ -144,6 +148,7 @@ func (c *cmd) Run(args []string) int { ingressConf.Listeners[listenerIdx].Services = append(ingressConf.Listeners[listenerIdx].Services, api.IngressService{ Name: svc, Namespace: svcNamespace, + Hosts: c.hosts, }) } else { ingressConf.Listeners = append(ingressConf.Listeners, api.IngressListener{ @@ -153,6 +158,7 @@ func (c *cmd) Run(args []string) int { { Name: svc, Namespace: svcNamespace, + Hosts: c.hosts, }, }, }) diff --git a/command/connect/expose/expose_test.go b/command/connect/expose/expose_test.go index 7d8443404a..cffbb7393e 100644 --- a/command/connect/expose/expose_test.go +++ b/command/connect/expose/expose_test.go @@ -279,6 +279,8 @@ func TestConnectExpose_existingConfig(t *testing.T) { "-ingress-gateway=ingress", "-port=9999", "-protocol=http", + "-host=foo.com", + "-host=foo.net", } code := c.Run(args) @@ -291,7 +293,8 @@ func TestConnectExpose_existingConfig(t *testing.T) { require.NoError(err) ingressConf.Listeners[1].Services = append(ingressConf.Listeners[1].Services, api.IngressService{ - Name: "zoo", + Name: "zoo", + Hosts: []string{"foo.com", "foo.net"}, }) ingressConf.CreateIndex = entry.GetCreateIndex() ingressConf.ModifyIndex = entry.GetModifyIndex() diff --git a/website/pages/docs/commands/connect/expose.mdx b/website/pages/docs/commands/connect/expose.mdx index 88208b6d9d..04373d9bad 100644 --- a/website/pages/docs/commands/connect/expose.mdx +++ b/website/pages/docs/commands/connect/expose.mdx @@ -40,12 +40,15 @@ Usage: consul connect expose [options] - `-port` - (Required) The listener port to use for the service on the Ingress gateway. -- `-protocol` - The protocol for the service. Defaults to 'tcp'. - - `-service` - (Required) The name of destination service to expose. A namespace can optionally be specified as a prefix via the 'namespace/service' format. +- `-protocol` - The protocol for the service. Defaults to 'tcp'. + +- `-host` - Additional DNS hostname to use for routing to this service. Can be + specified multiple times. + ## Examples The example below shows using the `expose` command to make the `foo` service available From e3a725c4e094e83342e0a4090939bb70b76fbcbb Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 9 Jun 2020 11:09:53 -0700 Subject: [PATCH 6/6] Always allow updating the exposed service and differentiate by namespace --- command/connect/expose/expose.go | 73 ++++++++++++++------------- command/connect/expose/expose_test.go | 29 +++++++++++ 2 files changed, 66 insertions(+), 36 deletions(-) diff --git a/command/connect/expose/expose.go b/command/connect/expose/expose.go index 9714424ada..4adf5db1fd 100644 --- a/command/connect/expose/expose.go +++ b/command/connect/expose/expose.go @@ -124,62 +124,63 @@ func (c *cmd) Run(args []string) int { } listenerIdx := -1 + serviceIdx := -1 + newService := api.IngressService{ + Name: svc, + Namespace: svcNamespace, + Hosts: c.hosts, + } for i, listener := range ingressConf.Listeners { - // Make sure the service isn't already exposed in this gateway - for _, service := range listener.Services { - if service.Name == svc && listener.Port == c.port { - c.UI.Output(fmt.Sprintf("Service %q already exposed through listener with port %d", svc, listener.Port)) - goto CREATE_INTENTION - } + // Find the listener for the specified port, if one exists. + if listener.Port != c.port { + continue } - // If there's already a listener for the given port, make sure the protocol matches. - if listener.Port == c.port { - listenerIdx = i - if listener.Protocol != c.protocol { - c.UI.Error(fmt.Sprintf("Listener on port %d already configured with conflicting protocol %q", listener.Port, listener.Protocol)) - return 1 + // Make sure the given protocol matches the existing one. + listenerIdx = i + if listener.Protocol != c.protocol { + c.UI.Error(fmt.Sprintf("Listener on port %d already configured with conflicting protocol %q", listener.Port, listener.Protocol)) + return 1 + } + + // Make sure the service isn't already exposed in this gateway + for j, service := range listener.Services { + if service.Name == svc && service.Namespace == svcNamespace { + serviceIdx = j + c.UI.Output(fmt.Sprintf("Updating service definition for %q on listener with port %d", c.service, listener.Port)) + break } } } // Add a service to the existing listener for the port if one exists, or make a new listener. if listenerIdx >= 0 { - ingressConf.Listeners[listenerIdx].Services = append(ingressConf.Listeners[listenerIdx].Services, api.IngressService{ - Name: svc, - Namespace: svcNamespace, - Hosts: c.hosts, - }) + if serviceIdx >= 0 { + ingressConf.Listeners[listenerIdx].Services[serviceIdx] = newService + } else { + ingressConf.Listeners[listenerIdx].Services = append(ingressConf.Listeners[listenerIdx].Services, newService) + } } else { ingressConf.Listeners = append(ingressConf.Listeners, api.IngressListener{ Port: c.port, Protocol: c.protocol, - Services: []api.IngressService{ - { - Name: svc, - Namespace: svcNamespace, - Hosts: c.hosts, - }, - }, + Services: []api.IngressService{newService}, }) } // Write the updated config entry using a check-and-set, so it fails if the entry // has been changed since we looked it up. - { - succeeded, _, err := client.ConfigEntries().CAS(ingressConf, ingressConf.GetModifyIndex(), nil) - if err != nil { - c.UI.Error(fmt.Sprintf("Error writing ingress config entry: %v", err)) - return 1 - } - if !succeeded { - c.UI.Error("Ingress config entry was changed while attempting to update, please try again.") - return 1 - } - c.UI.Output(fmt.Sprintf("Successfully updated config entry for ingress service %q", gateway)) + succeeded, _, err := client.ConfigEntries().CAS(ingressConf, ingressConf.GetModifyIndex(), nil) + if err != nil { + c.UI.Error(fmt.Sprintf("Error writing ingress config entry: %v", err)) + return 1 } + if !succeeded { + c.UI.Error("Ingress config entry was changed while attempting to update, please try again.") + return 1 + } + c.UI.Output(fmt.Sprintf("Successfully updated config entry for ingress service %q", gateway)) -CREATE_INTENTION: // Check for an existing intention. ixnFinder := finder.Finder{Client: client} existing, err := ixnFinder.Find(c.ingressGateway, c.service) diff --git a/command/connect/expose/expose_test.go b/command/connect/expose/expose_test.go index cffbb7393e..a509aa6460 100644 --- a/command/connect/expose/expose_test.go +++ b/command/connect/expose/expose_test.go @@ -300,4 +300,33 @@ func TestConnectExpose_existingConfig(t *testing.T) { ingressConf.ModifyIndex = entry.GetModifyIndex() require.Equal(ingressConf, entry) } + + // Update the bar service and add a custom host. + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + { + ui := cli.NewMockUi() + c := New(ui) + args := []string{ + "-http-addr=" + a.HTTPAddr(), + "-service=bar", + "-ingress-gateway=ingress", + "-port=9999", + "-protocol=http", + "-host=bar.com", + } + + code := c.Run(args) + if code != 0 { + t.Fatalf("bad: %d. %#v", code, ui.ErrorWriter.String()) + } + + // Make sure the ingress config was updated and existing services preserved. + entry, _, err := client.ConfigEntries().Get(api.IngressGateway, "ingress", nil) + require.NoError(err) + + ingressConf.Listeners[1].Services[0].Hosts = []string{"bar.com"} + ingressConf.CreateIndex = entry.GetCreateIndex() + ingressConf.ModifyIndex = entry.GetModifyIndex() + require.Equal(ingressConf, entry) + } }