From a26de963eb9cad8bc6a70db72364afc6a6fd2298 Mon Sep 17 00:00:00 2001 From: Steve Sloka Date: Thu, 19 Apr 2018 16:06:38 -0400 Subject: [PATCH 1/5] Sanitize cluster-name Signed-off-by: Steve Sloka --- discovery/cmd/kubernetes-discoverer/main.go | 2 + discovery/cmd/openstack-discoverer/main.go | 5 +- discovery/pkg/util/{log_format.go => util.go} | 16 ++++- discovery/pkg/util/util_test.go | 60 +++++++++++++++++++ 4 files changed, 81 insertions(+), 2 deletions(-) rename discovery/pkg/util/{log_format.go => util.go} (71%) create mode 100644 discovery/pkg/util/util_test.go diff --git a/discovery/cmd/kubernetes-discoverer/main.go b/discovery/cmd/kubernetes-discoverer/main.go index 371c577c..e6fb350a 100644 --- a/discovery/cmd/kubernetes-discoverer/main.go +++ b/discovery/cmd/kubernetes-discoverer/main.go @@ -80,9 +80,11 @@ func main() { } // Verify cluster name is passed + clusterName = util.SanitizeClusterName(clusterName) if clusterName == "" { log.Fatalf("The Kubernetes cluster name must be provided using the `--cluster-name` flag") } + log.Infof("ClusterName is: %s", clusterName) // Discovered cluster is passed if discovererKubeCfgFile == "" { diff --git a/discovery/cmd/openstack-discoverer/main.go b/discovery/cmd/openstack-discoverer/main.go index 29ba31a1..40bb56fc 100644 --- a/discovery/cmd/openstack-discoverer/main.go +++ b/discovery/cmd/openstack-discoverer/main.go @@ -85,9 +85,12 @@ func main() { discovererMetrics = localmetrics.NewMetrics() discovererMetrics.RegisterPrometheus() + // Verify cluster name is passed + clusterName = util.SanitizeClusterName(clusterName) if clusterName == "" { - log.Fatal("The OpenStack cluster name must be provided using the --cluster-name flag") + log.Fatalf("The Kubernetes cluster name must be provided using the `--cluster-name` flag") } + log.Infof("ClusterName is: %s", clusterName) gimbalKubeClient, err := k8s.NewClient(gimbalKubeCfgFile, log) if err != nil { diff --git a/discovery/pkg/util/log_format.go b/discovery/pkg/util/util.go similarity index 71% rename from discovery/pkg/util/log_format.go rename to discovery/pkg/util/util.go index 9c4c166f..b5c06a23 100644 --- a/discovery/pkg/util/log_format.go +++ b/discovery/pkg/util/util.go @@ -13,7 +13,12 @@ limitations under the License. package util -import "github.com/sirupsen/logrus" +import ( + "log" + "regexp" + + "github.com/sirupsen/logrus" +) // GetFormatter returns a textformatter to customize logs func GetFormatter() *logrus.TextFormatter { @@ -21,3 +26,12 @@ func GetFormatter() *logrus.TextFormatter { FullTimestamp: true, } } + +// SanitizeClusterName removes special characters +func SanitizeClusterName(clustername string) string { + reg, err := regexp.Compile("[^a-zA-Z0-9-_]+") + if err != nil { + log.Fatal(err) + } + return reg.ReplaceAllString(clustername, "") +} diff --git a/discovery/pkg/util/util_test.go b/discovery/pkg/util/util_test.go new file mode 100644 index 00000000..c6d1636f --- /dev/null +++ b/discovery/pkg/util/util_test.go @@ -0,0 +1,60 @@ +// Copyright © 2018 Heptio +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package util + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestTranslateService(t *testing.T) { + tests := []struct { + name string + clusterName string + expected string + }{ + { + name: "empty string", + clusterName: "", + expected: "", + }, + { + name: "simple", + clusterName: "mycluster", + expected: "mycluster", + }, + { + name: "special chars", + clusterName: "!@!mycl^%$uster**", + expected: "mycluster", + }, + { + name: "special chars with hyphen & underscore", + clusterName: "!@!my-cl^%$ust_er**", + expected: "my-clust_er", + }, + { + name: "whitespace", + clusterName: " my cluster ", + expected: "mycluster", + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := SanitizeClusterName(tc.clusterName) + assert.EqualValues(t, tc.expected, got) + }) + } +} From 5f985ccb311447ca1a32046636b4b1b2b1a0def8 Mon Sep 17 00:00:00 2001 From: Steve Sloka Date: Thu, 19 Apr 2018 17:11:38 -0400 Subject: [PATCH 2/5] Fail if cluster-name passed is invalid Signed-off-by: Steve Sloka --- discovery/cmd/kubernetes-discoverer/main.go | 5 ++--- discovery/cmd/openstack-discoverer/main.go | 5 ++--- discovery/pkg/util/util.go | 8 +++---- discovery/pkg/util/util_test.go | 24 +++++++++++++++------ 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/discovery/cmd/kubernetes-discoverer/main.go b/discovery/cmd/kubernetes-discoverer/main.go index e6fb350a..0bc03998 100644 --- a/discovery/cmd/kubernetes-discoverer/main.go +++ b/discovery/cmd/kubernetes-discoverer/main.go @@ -80,9 +80,8 @@ func main() { } // Verify cluster name is passed - clusterName = util.SanitizeClusterName(clusterName) - if clusterName == "" { - log.Fatalf("The Kubernetes cluster name must be provided using the `--cluster-name` flag") + if util.IsInvalidClusterName(clusterName) { + log.Fatalf("The Kubernetes cluster name must be provided using the `--cluster-name` flag or the one passed is invalid") } log.Infof("ClusterName is: %s", clusterName) diff --git a/discovery/cmd/openstack-discoverer/main.go b/discovery/cmd/openstack-discoverer/main.go index 40bb56fc..d5126c2a 100644 --- a/discovery/cmd/openstack-discoverer/main.go +++ b/discovery/cmd/openstack-discoverer/main.go @@ -86,9 +86,8 @@ func main() { discovererMetrics.RegisterPrometheus() // Verify cluster name is passed - clusterName = util.SanitizeClusterName(clusterName) - if clusterName == "" { - log.Fatalf("The Kubernetes cluster name must be provided using the `--cluster-name` flag") + if util.IsInvalidClusterName(clusterName) { + log.Fatalf("The Kubernetes cluster name must be provided using the `--cluster-name` flag or the one passed is invalid") } log.Infof("ClusterName is: %s", clusterName) diff --git a/discovery/pkg/util/util.go b/discovery/pkg/util/util.go index b5c06a23..fad60a0b 100644 --- a/discovery/pkg/util/util.go +++ b/discovery/pkg/util/util.go @@ -27,11 +27,11 @@ func GetFormatter() *logrus.TextFormatter { } } -// SanitizeClusterName removes special characters -func SanitizeClusterName(clustername string) string { - reg, err := regexp.Compile("[^a-zA-Z0-9-_]+") +// IsInvalidClusterName returns true if valid cluster name +func IsInvalidClusterName(clustername string) bool { + matched, err := regexp.MatchString("^[a-zA-Z0-9-_]+$", clustername) if err != nil { log.Fatal(err) } - return reg.ReplaceAllString(clustername, "") + return !matched } diff --git a/discovery/pkg/util/util_test.go b/discovery/pkg/util/util_test.go index c6d1636f..1c0c962e 100644 --- a/discovery/pkg/util/util_test.go +++ b/discovery/pkg/util/util_test.go @@ -23,37 +23,47 @@ func TestTranslateService(t *testing.T) { tests := []struct { name string clusterName string - expected string + expected bool }{ { name: "empty string", clusterName: "", - expected: "", + expected: true, }, { name: "simple", clusterName: "mycluster", - expected: "mycluster", + expected: false, + }, + { + name: "hyphen", + clusterName: "my-cluster", + expected: false, + }, + { + name: "underscore", + clusterName: "my_cluster", + expected: false, }, { name: "special chars", clusterName: "!@!mycl^%$uster**", - expected: "mycluster", + expected: true, }, { name: "special chars with hyphen & underscore", clusterName: "!@!my-cl^%$ust_er**", - expected: "my-clust_er", + expected: true, }, { name: "whitespace", clusterName: " my cluster ", - expected: "mycluster", + expected: true, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - got := SanitizeClusterName(tc.clusterName) + got := IsInvalidClusterName(tc.clusterName) assert.EqualValues(t, tc.expected, got) }) } From ffe56291f88af91f1626bb5c83668bd0669f58b7 Mon Sep 17 00:00:00 2001 From: Steve Sloka Date: Thu, 19 Apr 2018 20:26:41 -0400 Subject: [PATCH 3/5] update regex to match k8s service restrictions Signed-off-by: Steve Sloka --- discovery/pkg/util/util.go | 2 +- discovery/pkg/util/util_test.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/discovery/pkg/util/util.go b/discovery/pkg/util/util.go index fad60a0b..360a636e 100644 --- a/discovery/pkg/util/util.go +++ b/discovery/pkg/util/util.go @@ -29,7 +29,7 @@ func GetFormatter() *logrus.TextFormatter { // IsInvalidClusterName returns true if valid cluster name func IsInvalidClusterName(clustername string) bool { - matched, err := regexp.MatchString("^[a-zA-Z0-9-_]+$", clustername) + matched, err := regexp.MatchString("^[a-zA-Z0-9]+[a-zA-Z0-9-]*[a-zA-Z0-9]+$", clustername) if err != nil { log.Fatal(err) } diff --git a/discovery/pkg/util/util_test.go b/discovery/pkg/util/util_test.go index 1c0c962e..37583654 100644 --- a/discovery/pkg/util/util_test.go +++ b/discovery/pkg/util/util_test.go @@ -43,8 +43,23 @@ func TestTranslateService(t *testing.T) { { name: "underscore", clusterName: "my_cluster", + expected: true, + }, + { + name: "multiple underscores", + clusterName: "my----cluster", expected: false, }, + { + name: "can't start with underscores", + clusterName: "-mycluster", + expected: true, + }, + { + name: "can't end with underscores", + clusterName: "mycluster-", + expected: true, + }, { name: "special chars", clusterName: "!@!mycl^%$uster**", From 6bec96d33a431b5d755b9ea2557e1cc6fe7501f1 Mon Sep 17 00:00:00 2001 From: Steve Sloka Date: Thu, 19 Apr 2018 20:29:40 -0400 Subject: [PATCH 4/5] Add note to docs to describe rules for cluster-name Signed-off-by: Steve Sloka --- docs/kubernetes-discoverer.md | 2 +- docs/openstack-discoverer.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/kubernetes-discoverer.md b/docs/kubernetes-discoverer.md index ac3491cf..906455c4 100644 --- a/docs/kubernetes-discoverer.md +++ b/docs/kubernetes-discoverer.md @@ -22,7 +22,7 @@ Arguments are available to customize the discoverer, most have defaults but othe | num-threads | 2 | Specify number of threads to use when processing queue items | gimbal-kubecfg-file | "" | Location of kubecfg file for access to Kubernetes cluster hosting Gimbal | discover-kubecfg-file | "" | Location of kubecfg file for access to remote Kubernetes cluster to watch for services / endpoints -| cluster-name | "" | Name of cluster scraping for services & endpoints +| cluster-name | "" | Name of cluster scraping for services & endpoints (Cannot start or end with a hyphen and must be alpha-numeric) | debug | false | Enable debug logging ### Credentials diff --git a/docs/openstack-discoverer.md b/docs/openstack-discoverer.md index c2d47d44..f7812d2c 100644 --- a/docs/openstack-discoverer.md +++ b/docs/openstack-discoverer.md @@ -21,7 +21,7 @@ Arguments are available to customize the discoverer, most have defaults but othe | version | false | Show version, build information and quit | num-threads | 2 | Specify number of threads to use when processing queue items | gimbal-kubecfg-file | "" | Location of kubecfg file for access to Kubernetes cluster hosting Gimbal -| cluster-name | "" | Name of cluster scraping for services & endpoints +| cluster-name | "" | Name of cluster scraping for services & endpoints (Cannot start or end with a hyphen and must be alpha-numeric) | debug | false | Enable debug logging | reconciliation-period | 30s | The interval of time between reconciliation loop runs | http-client-timeout | 5s | The HTTP client request timeout From 877ecb8faac224d1d55878ad997aa5a82c73bed6 Mon Sep 17 00:00:00 2001 From: Steve Sloka Date: Fri, 20 Apr 2018 07:10:44 -0400 Subject: [PATCH 5/5] Update regex to match k8s docs Signed-off-by: Steve Sloka --- discovery/pkg/util/util.go | 2 +- docs/kubernetes-discoverer.md | 2 +- docs/openstack-discoverer.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/discovery/pkg/util/util.go b/discovery/pkg/util/util.go index 360a636e..705b7d48 100644 --- a/discovery/pkg/util/util.go +++ b/discovery/pkg/util/util.go @@ -29,7 +29,7 @@ func GetFormatter() *logrus.TextFormatter { // IsInvalidClusterName returns true if valid cluster name func IsInvalidClusterName(clustername string) bool { - matched, err := regexp.MatchString("^[a-zA-Z0-9]+[a-zA-Z0-9-]*[a-zA-Z0-9]+$", clustername) + matched, err := regexp.MatchString("^[a-z]([-a-z0-9]*[a-z0-9])?$", clustername) if err != nil { log.Fatal(err) } diff --git a/docs/kubernetes-discoverer.md b/docs/kubernetes-discoverer.md index 906455c4..c75c3b57 100644 --- a/docs/kubernetes-discoverer.md +++ b/docs/kubernetes-discoverer.md @@ -22,7 +22,7 @@ Arguments are available to customize the discoverer, most have defaults but othe | num-threads | 2 | Specify number of threads to use when processing queue items | gimbal-kubecfg-file | "" | Location of kubecfg file for access to Kubernetes cluster hosting Gimbal | discover-kubecfg-file | "" | Location of kubecfg file for access to remote Kubernetes cluster to watch for services / endpoints -| cluster-name | "" | Name of cluster scraping for services & endpoints (Cannot start or end with a hyphen and must be alpha-numeric) +| cluster-name | "" | Name of cluster scraping for services & endpoints (Cannot start or end with a hyphen and must be lowercase alpha-numeric) | debug | false | Enable debug logging ### Credentials diff --git a/docs/openstack-discoverer.md b/docs/openstack-discoverer.md index f7812d2c..1083a04c 100644 --- a/docs/openstack-discoverer.md +++ b/docs/openstack-discoverer.md @@ -21,7 +21,7 @@ Arguments are available to customize the discoverer, most have defaults but othe | version | false | Show version, build information and quit | num-threads | 2 | Specify number of threads to use when processing queue items | gimbal-kubecfg-file | "" | Location of kubecfg file for access to Kubernetes cluster hosting Gimbal -| cluster-name | "" | Name of cluster scraping for services & endpoints (Cannot start or end with a hyphen and must be alpha-numeric) +| cluster-name | "" | Name of cluster scraping for services & endpoints (Cannot start or end with a hyphen and must be lowercase alpha-numeric) | debug | false | Enable debug logging | reconciliation-period | 30s | The interval of time between reconciliation loop runs | http-client-timeout | 5s | The HTTP client request timeout