Skip to content

Commit

Permalink
Add node name validation to the catalog sync process
Browse files Browse the repository at this point in the history
  • Loading branch information
adilyse committed Aug 26, 2020
1 parent 9ad0b5d commit 00bd66e
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 1 deletion.
35 changes: 34 additions & 1 deletion subcommand/sync-catalog/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"net/http"
"os"
"os/signal"
"regexp"
"sync"
"time"

Expand Down Expand Up @@ -97,7 +98,8 @@ func (c *Command) init() {
c.flags.StringVar(&c.flagConsulK8STag, "consul-k8s-tag", "k8s",
"Tag value for K8S services registered in Consul")
c.flags.StringVar(&c.flagConsulNodeName, "consul-node-name", "k8s-sync",
"The Consul node name to register for k8s-sync. Defaults to k8s-sync.")
"The Consul node name to register for catalog sync. Defaults to k8s-sync. To be discoverable "+
"via DNS, the name should only contain alpha-numerics and dashes.")
c.flags.DurationVar(&c.flagConsulWritePeriod, "consul-write-interval", 30*time.Second,
"The interval to perform syncing operations creating Consul services, formatted "+
"as a time.Duration. All changes are merged and write calls are only made "+
Expand Down Expand Up @@ -162,6 +164,12 @@ func (c *Command) Run(args []string) int {
return 1
}

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

// Create the k8s clientset
if c.clientset == nil {
config, err := subcommand.K8SConfig(c.k8s.KubeConfig())
Expand Down Expand Up @@ -380,6 +388,31 @@ func (c *Command) interrupt() {
c.sigCh <- os.Interrupt
}

func (c *Command) validateFlags() error {
// For the Consul node name to be discoverable via DNS, it must contain only
// dashes and alphanumeric characters. Length is also constrained.
// These restrictions match those defined in Consul's agent definition.
var InvalidDnsRe = regexp.MustCompile(`[^A-Za-z0-9\\-]+`)
const MaxDNSLabelLength = 63

if InvalidDnsRe.MatchString(c.flagConsulNodeName) {
return fmt.Errorf("Node name will not be discoverable "+
"via DNS due to invalid characters. Valid characters include "+
"all alpha-numerics and dashes. consul-node-name=%s",
c.flagConsulNodeName,
)
}
if len(c.flagConsulNodeName) > MaxDNSLabelLength {
return fmt.Errorf("Node name will not be discoverable "+
"via DNS due to it being too long. Valid lengths are between "+
"1 and 63 bytes. consul-node-name=%s",
c.flagConsulNodeName,
)
}

return nil
}

const synopsis = "Sync Kubernetes services and Consul services."
const help = `
Usage: consul-k8s sync-catalog [options]
Expand Down
33 changes: 33 additions & 0 deletions subcommand/sync-catalog/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,39 @@ import (
"k8s.io/client-go/kubernetes/fake"
)

// Test flag validation
func TestRun_FlagValidation(t *testing.T) {
t.Parallel()

cases := []struct {
Flags []string
ExpErr string
}{
{
Flags: []string{"-consul-node-name=Speci@l_Chars"},
ExpErr: "Node name will not be discoverable via DNS due to invalid characters. Valid characters include " +
"all alpha-numerics and dashes. consul-node-name=Speci@l_Chars",
},
{
Flags: []string{"-consul-node-name=5r9OPGfSRXUdGzNjBdAwmhCBrzHDNYs4XjZVR4wp7lSLIzqwS0ta51nBLIN0TMPV-too-long"},
ExpErr: "Node name will not be discoverable via DNS due to it being too long. Valid lengths are between " +
"1 and 63 bytes. consul-node-name=5r9OPGfSRXUdGzNjBdAwmhCBrzHDNYs4XjZVR4wp7lSLIzqwS0ta51nBLIN0TMPV-too-long",
},
}

for _, c := range cases {
t.Run(c.ExpErr, func(t *testing.T) {
ui := cli.NewMockUi()
cmd := Command{
UI: ui,
}
responseCode := cmd.Run(c.Flags)
require.Equal(t, 1, responseCode, ui.ErrorWriter.String())
require.Contains(t, ui.ErrorWriter.String(), c.ExpErr)
})
}
}

// Test that the default consul service is synced to k8s
func TestRun_Defaults_SyncsConsulServiceToK8s(t *testing.T) {
t.Parallel()
Expand Down

0 comments on commit 00bd66e

Please sign in to comment.