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

feature: add command for exporting service to peer or partition #15654

Merged
merged 57 commits into from
May 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
ea11fa1
Sujata's peering-cli branch
20sr20 Oct 14, 2022
1a2a456
Added error message for connecting to cluster
20sr20 Oct 27, 2022
3adedfe
We can export service to peer
20sr20 Nov 3, 2022
59ee6d6
export handling multiple peers
20sr20 Nov 4, 2022
241a0ec
export handles multiple peers
20sr20 Nov 4, 2022
fcbb526
export now can handle multiple services
20sr20 Nov 4, 2022
b17b61c
Export after 1st cleanup
20sr20 Nov 4, 2022
14551aa
Successful export
20sr20 Nov 4, 2022
d155ed6
Added the namespace option
20sr20 Dec 2, 2022
43ef742
Add .changelog entry
20sr20 Dec 12, 2022
fe889aa
go mod tidy
20sr20 Dec 12, 2022
f87110a
Stub unit tests for peering export command
nathancoleman Dec 12, 2022
0f68ffe
added export in peering.go
20sr20 Dec 12, 2022
bf173d8
Adding export_test
20sr20 Dec 19, 2022
7f73c1f
Moved the code to services from peers and cleaned the serviceNamespace
20sr20 Jan 27, 2023
47d7311
Added support for exporting to partitions
20sr20 Jan 27, 2023
e5f69bc
Fixed partition bug
20sr20 Jan 30, 2023
82ff42c
Added unit tests for export command
20sr20 Jan 31, 2023
efce0da
Add multi-tenancy flags
nathancoleman Jan 31, 2023
8f6294d
gofmt
nathancoleman Jan 31, 2023
1b0c5f1
Add some helpful comments
nathancoleman Jan 31, 2023
cd052d8
Exclude namespace + partition flags when running OSS
nathancoleman Feb 1, 2023
baa7910
cleaned up partition stuff
20sr20 Feb 1, 2023
436dc06
Validate required flags differently for OSS vs. ENT
nathancoleman Feb 1, 2023
0410b19
Update success output to include only the requested consumers
nathancoleman Feb 1, 2023
076644b
cleaned up
20sr20 Feb 1, 2023
657ba30
fixed broken test
20sr20 Feb 1, 2023
dd855b8
gofmt
nathancoleman Feb 1, 2023
c2d29c0
Include all flags in OSS build
nathancoleman Feb 2, 2023
5d29866
Remove example previously added to peering command
nathancoleman Feb 2, 2023
c853fe5
Move stray import into correct block
nathancoleman Feb 2, 2023
e0cd160
Update changelog entry to include support for exporting to a partition
nathancoleman Feb 2, 2023
a1b77a4
Add required-ness label to consumer-peers flag description
nathancoleman Feb 2, 2023
3362d68
Update command/services/export/export.go
nathancoleman Feb 2, 2023
642c60b
Add docs placeholder for new services export command
nathancoleman Feb 2, 2023
c952806
Moved piece of code to OSS
20sr20 Feb 3, 2023
06ce0d6
Break config entry init + update into separate functions
nathancoleman Feb 3, 2023
6679bac
fixed
20sr20 Feb 3, 2023
3260e74
Vary existing service export comparison for OSS vs. ENT
nathancoleman Feb 3, 2023
65d6cb8
Move OSS-specific test to export_oss_test.go
nathancoleman Feb 3, 2023
6560df3
Set config entry name based on partition being exported from
nathancoleman Feb 8, 2023
7ceb7d9
Set namespace on added services
nathancoleman Feb 8, 2023
8823919
Adding namespace
20sr20 Apr 10, 2023
f800794
Merge branch 'export-peering-cli' of https://github.com/hashicorp/con…
20sr20 Apr 10, 2023
81e2e31
Remove export documentation
nathancoleman Apr 10, 2023
02581a3
Consolidate code from export_oss into export.go
nathancoleman Apr 10, 2023
705c08e
Merge branch 'main' into export-peering-cli
nathancoleman Apr 10, 2023
f8d53be
Consolidated export_oss_test.go and export_test.go
20sr20 Apr 11, 2023
542d58b
Merge branch 'main' into export-peering-cli
nathancoleman May 1, 2023
5aff944
Add example of partition export to command synopsis
nathancoleman May 1, 2023
27c5a0e
Allow empty peers flag if partitions flag provided
nathancoleman May 1, 2023
905de0a
Add test coverage for -consumer-partitions flag
nathancoleman May 1, 2023
151a461
Update command/services/export/export.go
nathancoleman May 30, 2023
563cc5a
Update command/services/export/export.go
nathancoleman May 30, 2023
447a89d
Update changelog entry
nathancoleman May 30, 2023
119cc4e
Use "cluster peers" to clear up any possible confusion
nathancoleman May 30, 2023
2e4da1b
Update test assertions
nathancoleman May 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/15654.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:feature
cli: Adds new command - `consul services export` - for exporting a service to a peer or partition
```
4 changes: 4 additions & 0 deletions command/flags/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ func (f *HTTPFlags) Datacenter() string {
return f.datacenter.String()
}

func (f *HTTPFlags) Namespace() string {
return f.namespace.String()
}

func (f *HTTPFlags) Partition() string {
return f.partition.String()
}
Expand Down
4 changes: 3 additions & 1 deletion command/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ import (
"github.com/hashicorp/consul/command/rtt"
"github.com/hashicorp/consul/command/services"
svcsderegister "github.com/hashicorp/consul/command/services/deregister"
svcsexport "github.com/hashicorp/consul/command/services/export"
svcsregister "github.com/hashicorp/consul/command/services/register"
"github.com/hashicorp/consul/command/snapshot"
snapinspect "github.com/hashicorp/consul/command/snapshot/inspect"
Expand All @@ -121,7 +122,7 @@ import (
tlscacreate "github.com/hashicorp/consul/command/tls/ca/create"
tlscert "github.com/hashicorp/consul/command/tls/cert"
tlscertcreate "github.com/hashicorp/consul/command/tls/cert/create"
troubleshoot "github.com/hashicorp/consul/command/troubleshoot"
"github.com/hashicorp/consul/command/troubleshoot"
troubleshootproxy "github.com/hashicorp/consul/command/troubleshoot/proxy"
troubleshootupstreams "github.com/hashicorp/consul/command/troubleshoot/upstreams"
"github.com/hashicorp/consul/command/validate"
Expand Down Expand Up @@ -241,6 +242,7 @@ func RegisteredCommands(ui cli.Ui) map[string]mcli.CommandFactory {
entry{"services", func(cli.Ui) (cli.Command, error) { return services.New(), nil }},
entry{"services register", func(ui cli.Ui) (cli.Command, error) { return svcsregister.New(ui), nil }},
entry{"services deregister", func(ui cli.Ui) (cli.Command, error) { return svcsderegister.New(ui), nil }},
entry{"services export", func(ui cli.Ui) (cli.Command, error) { return svcsexport.New(ui), nil }},
entry{"snapshot", func(cli.Ui) (cli.Command, error) { return snapshot.New(), nil }},
entry{"snapshot inspect", func(ui cli.Ui) (cli.Command, error) { return snapinspect.New(ui), nil }},
entry{"snapshot restore", func(ui cli.Ui) (cli.Command, error) { return snaprestore.New(ui), nil }},
Expand Down
260 changes: 260 additions & 0 deletions command/services/export/export.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,260 @@
package export

import (
"errors"
"flag"
"fmt"
"strings"

"github.com/mitchellh/cli"

"github.com/hashicorp/consul/agent"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/command/flags"
)

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

serviceName string
peerNames string
partitionNames string
}

func (c *cmd) init() {
c.flags = flag.NewFlagSet("", flag.ContinueOnError)

c.flags.StringVar(&c.serviceName, "name", "", "(Required) Specify the name of the service you want to export.")
c.flags.StringVar(&c.peerNames, "consumer-peers", "", "(Required) A comma-separated list of cluster peers to export the service to. In Consul Enterprise, this flag is optional if -consumer-partitions is specified.")
c.flags.StringVar(&c.partitionNames, "consumer-partitions", "", "(Enterprise only) A comma-separated list of admin partitions within the same datacenter to export the service to. This flag is optional if -consumer-peers is specified.")

c.http = &flags.HTTPFlags{}
flags.Merge(c.flags, c.http.ClientFlags())
flags.Merge(c.flags, c.http.MultiTenancyFlags())
c.help = flags.Usage(help, c.flags)
}

func (c *cmd) Run(args []string) int {
if err := c.flags.Parse(args); err != nil {
return 1
}

if err := c.validateFlags(); err != nil {
c.UI.Error(err.Error())
return 1
}

peerNames, err := c.getPeerNames()
if err != nil {
c.UI.Error(err.Error())
return 1
}

partitionNames, err := c.getPartitionNames()
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if partitions are specified in OSS?

Copy link
Member Author

Choose a reason for hiding this comment

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

This follows the same theme as other enterprise features in this PR. The command with partitions specified will go through even for OSS users, same as if you use $ consul config write today (since this is really all that's happening behind the scenes).

Since the established pattern is to allow Enterprise features in the OSS CLI, the correct thing to do in the longer term IMO is to add server validation for this config entry that returns an error in this case.

if err != nil {
c.UI.Error(err.Error())
return 1
}

client, err := c.http.APIClient()
if err != nil {
c.UI.Error(fmt.Sprintf("Error connect to Consul agent: %s", err))
return 1
}

// Name matches partition, so "default" if none specified
cfgName := "default"
if c.http.Partition() != "" {
cfgName = c.http.Partition()
}

entry, _, err := client.ConfigEntries().Get(api.ExportedServices, cfgName, &api.QueryOptions{Namespace: ""})
if err != nil && !strings.Contains(err.Error(), agent.ConfigEntryNotFoundErr) {
c.UI.Error(fmt.Sprintf("Error reading config entry %s/%s: %v", "exported-services", "default", err))
return 1
}

var cfg *api.ExportedServicesConfigEntry
if entry == nil {
cfg = c.initializeConfigEntry(cfgName, peerNames, partitionNames)
} else {
existingCfg, ok := entry.(*api.ExportedServicesConfigEntry)
if !ok {
c.UI.Error(fmt.Sprintf("Existing config entry has incorrect type: %t", entry))
return 1
}

cfg = c.updateConfigEntry(existingCfg, peerNames, partitionNames)
}

ok, _, err := client.ConfigEntries().CAS(cfg, cfg.GetModifyIndex(), nil)
if err != nil {
c.UI.Error(fmt.Sprintf("Error writing config entry: %s", err))
return 1
} else if !ok {
c.UI.Error(fmt.Sprintf("Config entry was changed during update. Please try again"))
return 1
}
Comment on lines +99 to +106
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice due diligence in handling this case :)


switch {
case len(c.peerNames) > 0 && len(c.partitionNames) > 0:
c.UI.Info(fmt.Sprintf("Successfully exported service %q to cluster peers %q and to partitions %q", c.serviceName, c.peerNames, c.partitionNames))
case len(c.peerNames) > 0:
c.UI.Info(fmt.Sprintf("Successfully exported service %q to cluster peers %q", c.serviceName, c.peerNames))
case len(c.partitionNames) > 0:
c.UI.Info(fmt.Sprintf("Successfully exported service %q to partitions %q", c.serviceName, c.partitionNames))
}

return 0
}

func (c *cmd) validateFlags() error {
if c.serviceName == "" {
return errors.New("Missing the required -name flag")
}

if c.peerNames == "" && c.partitionNames == "" {
return errors.New("Missing the required -consumer-peers or -consumer-partitions flag")
}

return nil
}

func (c *cmd) getPeerNames() ([]string, error) {
var peerNames []string
if c.peerNames != "" {
peerNames = strings.Split(c.peerNames, ",")
for _, peerName := range peerNames {
if peerName == "" {
return nil, fmt.Errorf("Invalid peer %q", peerName)
}
}
}
return peerNames, nil
}

func (c *cmd) getPartitionNames() ([]string, error) {
var partitionNames []string
if c.partitionNames != "" {
partitionNames = strings.Split(c.partitionNames, ",")
for _, partitionName := range partitionNames {
if partitionName == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any other validation of peer or partition names beyond "is it empty" that the backend does?

I guess the possible harm is that someone could write a config entry for a non-existent partition, or one with a mistyped/garbled name?

That said, we output a string like Successfully exported service %q to peers %q afterwards, so that would hopefully show whether something was wrong.

So maybe additional validation isn't important (unless it was really easy to do).

Copy link
Member Author

@nathancoleman nathancoleman May 30, 2023

Choose a reason for hiding this comment

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

@DanStough and I talked at one point about adding validation in the backend for this as a separate PR.

Today, you can write a config entry that exports to a non-existent peer or partition using consul config write - this command is no different since it uses the same config entry mechanism. Based on that, I consider adding more validation on the server side to be outside the scope of this PR. It would be good to get on someone's radar though. Thoughts?

return nil, fmt.Errorf("Invalid partition %q", partitionName)
}
}
}
return partitionNames, nil
}

func (c *cmd) initializeConfigEntry(cfgName string, peerNames, partitionNames []string) *api.ExportedServicesConfigEntry {
return &api.ExportedServicesConfigEntry{
Name: cfgName,
Services: []api.ExportedService{
{
Name: c.serviceName,
Namespace: c.http.Namespace(),
Consumers: buildConsumers(peerNames, partitionNames),
},
},
}
}

func (c *cmd) updateConfigEntry(cfg *api.ExportedServicesConfigEntry, peerNames, partitionNames []string) *api.ExportedServicesConfigEntry {
serviceExists := false

for i, service := range cfg.Services {
if service.Name == c.serviceName && service.Namespace == c.http.Namespace() {
serviceExists = true

// Add a consumer for each peer where one doesn't already exist
for _, peerName := range peerNames {
peerExists := false
for _, consumer := range service.Consumers {
if consumer.Peer == peerName {
peerExists = true
break
}
}
if !peerExists {
cfg.Services[i].Consumers = append(cfg.Services[i].Consumers, api.ServiceConsumer{Peer: peerName})
}
}

// Add a consumer for each partition where one doesn't already exist
for _, partitionName := range partitionNames {
partitionExists := false

for _, consumer := range service.Consumers {
if consumer.Partition == partitionName {
partitionExists = true
break
}
}
if !partitionExists {
cfg.Services[i].Consumers = append(cfg.Services[i].Consumers, api.ServiceConsumer{Partition: partitionName})
}
}
}
}

if !serviceExists {
cfg.Services = append(cfg.Services, api.ExportedService{
Name: c.serviceName,
Namespace: c.http.Namespace(),
Consumers: buildConsumers(peerNames, partitionNames),
})
}

return cfg
}

func buildConsumers(peerNames []string, partitionNames []string) []api.ServiceConsumer {
var consumers []api.ServiceConsumer
for _, peer := range peerNames {
consumers = append(consumers, api.ServiceConsumer{
Peer: peer,
})
}
for _, partition := range partitionNames {
consumers = append(consumers, api.ServiceConsumer{
Partition: partition,
})
}
return consumers
}

//========

func (c *cmd) Synopsis() string {
return synopsis
}

func (c *cmd) Help() string {
return flags.Usage(c.help, nil)
}

const (
synopsis = "Export a service from one peer or admin partition to another"
help = `
Usage: consul services export [options] -name <service name> -consumer-peers <other cluster name>

Export a service to a peered cluster.

$ consul services export -name=web -consumer-peers=other-cluster

Use the -consumer-partitions flag instead of -consumer-peers to export to a different partition in the same cluster.

$ consul services export -name=web -consumer-partitions=other-partition

Additional flags and more advanced use cases are detailed below.
`
)
Loading
Loading