From 9e40068103e7eec7cc52f67c48e5acd742ce7304 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= <ayufan@ayufan.eu> Date: Tue, 15 Jan 2019 12:17:35 +0000 Subject: [PATCH] Merge branch '3883-fix-dns-1123-with-docker-machine' into 'master' Make new runner tokens compatible with docker-machine executor Closes #3883 See merge request gitlab-org/gitlab-runner!1144 --- executors/docker/machine/name.go | 3 +- executors/docker/machine/name_test.go | 38 ++++++++++++++++++- executors/kubernetes/executor_kubernetes.go | 3 +- .../kubernetes/executor_kubernetes_test.go | 7 ++-- executors/kubernetes/util.go | 24 ------------ executors/kubernetes/util_test.go | 29 -------------- helpers/dns/test/test.go | 16 ++++++++ helpers/dns/utils.go | 28 ++++++++++++++ helpers/dns/utils_test.go | 29 ++++++++++++++ 9 files changed, 117 insertions(+), 60 deletions(-) create mode 100644 helpers/dns/test/test.go create mode 100644 helpers/dns/utils.go create mode 100644 helpers/dns/utils_test.go diff --git a/executors/docker/machine/name.go b/executors/docker/machine/name.go index 6f748b77c..0cafa2f4d 100644 --- a/executors/docker/machine/name.go +++ b/executors/docker/machine/name.go @@ -7,6 +7,7 @@ import ( "time" "gitlab.com/gitlab-org/gitlab-runner/common" + "gitlab.com/gitlab-org/gitlab-runner/helpers/dns" ) func machineFormat(runner string, template string) string { @@ -17,7 +18,7 @@ func machineFormat(runner string, template string) string { } func machineFilter(config *common.RunnerConfig) string { - return machineFormat(config.ShortDescription(), config.Machine.MachineName) + return machineFormat(dns.MakeRFC1123Compatible(config.ShortDescription()), config.Machine.MachineName) } func matchesMachineFilter(name, filter string) bool { diff --git a/executors/docker/machine/name_test.go b/executors/docker/machine/name_test.go index e002307a1..106bd07a4 100644 --- a/executors/docker/machine/name_test.go +++ b/executors/docker/machine/name_test.go @@ -1,12 +1,46 @@ package machine import ( + "testing" + "github.com/stretchr/testify/assert" + "gitlab.com/gitlab-org/gitlab-runner/common" - "testing" + dns_test "gitlab.com/gitlab-org/gitlab-runner/helpers/dns/test" ) -func TestMachineNewName(t *testing.T) { +func TestNewMachineName(t *testing.T) { + testCases := map[string]struct { + token string + }{ + "DNS-1123 compatible token": { + token: "token-of", + }, + "non DNS-1123 compatible token": { + token: "ToK3_?OF", + }, + } + + for name, testCase := range testCases { + t.Run(name, func(t *testing.T) { + config := &common.RunnerConfig{ + RunnerCredentials: common.RunnerCredentials{ + Token: testCase.token, + }, + RunnerSettings: common.RunnerSettings{ + Machine: &common.DockerMachine{ + MachineName: "test-machine-%s", + }, + }, + } + + name := newMachineName(config) + dns_test.AssertRFC1123Compatibility(t, name) + }) + } +} + +func TestNewMachineNameIsUnique(t *testing.T) { config := &common.RunnerConfig{ RunnerSettings: common.RunnerSettings{ Machine: &common.DockerMachine{ diff --git a/executors/kubernetes/executor_kubernetes.go b/executors/kubernetes/executor_kubernetes.go index dba659910..d4b48ded1 100644 --- a/executors/kubernetes/executor_kubernetes.go +++ b/executors/kubernetes/executor_kubernetes.go @@ -20,6 +20,7 @@ import ( "gitlab.com/gitlab-org/gitlab-runner/common" "gitlab.com/gitlab-org/gitlab-runner/executors" + "gitlab.com/gitlab-org/gitlab-runner/helpers/dns" terminalsession "gitlab.com/gitlab-org/gitlab-runner/session/terminal" ) @@ -394,7 +395,7 @@ type dockerConfigEntry struct { } func (s *executor) projectUniqueName() string { - return makeDNS1123Compatible(s.Build.ProjectUniqueName()) + return dns.MakeRFC1123Compatible(s.Build.ProjectUniqueName()) } func (s *executor) setupCredentials() error { diff --git a/executors/kubernetes/executor_kubernetes_test.go b/executors/kubernetes/executor_kubernetes_test.go index 10a3f1482..d343e1e26 100644 --- a/executors/kubernetes/executor_kubernetes_test.go +++ b/executors/kubernetes/executor_kubernetes_test.go @@ -20,7 +20,6 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "gitlab.com/gitlab-org/gitlab-runner/session" api "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -30,6 +29,8 @@ import ( "gitlab.com/gitlab-org/gitlab-runner/common" "gitlab.com/gitlab-org/gitlab-runner/executors" "gitlab.com/gitlab-org/gitlab-runner/helpers" + dns_test "gitlab.com/gitlab-org/gitlab-runner/helpers/dns/test" + "gitlab.com/gitlab-org/gitlab-runner/session" ) var ( @@ -940,7 +941,7 @@ func TestSetupCredentials(t *testing.T) { }, }, VerifyFn: func(t *testing.T, test testDef, secret *api.Secret) { - assertDNS1123Compatibility(t, secret.GetGenerateName()) + dns_test.AssertRFC1123Compatibility(t, secret.GetGenerateName()) }, }, } @@ -1408,7 +1409,7 @@ func TestSetupBuildPod(t *testing.T) { }, }, VerifyFn: func(t *testing.T, test setupBuildPodTestDef, pod *api.Pod) { - assertDNS1123Compatibility(t, pod.GetGenerateName()) + dns_test.AssertRFC1123Compatibility(t, pod.GetGenerateName()) }, }, } diff --git a/executors/kubernetes/util.go b/executors/kubernetes/util.go index 83af91588..e78648c61 100644 --- a/executors/kubernetes/util.go +++ b/executors/kubernetes/util.go @@ -5,8 +5,6 @@ import ( "fmt" "io" "net/http" - "regexp" - "strings" "time" "golang.org/x/net/context" @@ -20,12 +18,6 @@ import ( "gitlab.com/gitlab-org/gitlab-runner/common" ) -const ( - DNS1123NameMaximumLength = 63 - DNS1123NotAllowedCharacters = "[^-a-z0-9]" - DNS1123NotAllowedStartCharacters = "^[^a-z0-9]+" -) - type kubeConfigProvider func() (*restclient.Config, error) var ( @@ -264,19 +256,3 @@ func buildVariables(bv common.JobVariables) []api.EnvVar { } return e } - -func makeDNS1123Compatible(name string) string { - name = strings.ToLower(name) - - nameNotAllowedChars := regexp.MustCompile(DNS1123NotAllowedCharacters) - name = nameNotAllowedChars.ReplaceAllString(name, "") - - nameNotAllowedStartChars := regexp.MustCompile(DNS1123NotAllowedStartCharacters) - name = nameNotAllowedStartChars.ReplaceAllString(name, "") - - if len(name) > DNS1123NameMaximumLength { - name = name[0:DNS1123NameMaximumLength] - } - - return name -} diff --git a/executors/kubernetes/util_test.go b/executors/kubernetes/util_test.go index e5bb7c68a..980605e2d 100644 --- a/executors/kubernetes/util_test.go +++ b/executors/kubernetes/util_test.go @@ -6,7 +6,6 @@ import ( "io" "io/ioutil" "net/http" - "regexp" "strings" "testing" @@ -401,31 +400,3 @@ func testVersionAndCodec() (version string, codec runtime.Codec) { return } - -func assertDNS1123Compatibility(t *testing.T, name string) { - dns1123MaxLength := 63 - dns1123FormatRegexp := regexp.MustCompile("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") - - assert.True(t, len(name) <= dns1123MaxLength, "Name length needs to be shorter than %d", dns1123MaxLength) - assert.Regexp(t, dns1123FormatRegexp, name, "Name needs to be in DNS-1123 allowed format") -} - -func TestMakeDNS1123Compatible(t *testing.T) { - examples := []struct { - name string - expected string - }{ - {name: "tOk3_?ofTHE-Runner", expected: "tok3ofthe-runner"}, - {name: "----tOk3_?ofTHE-Runner", expected: "tok3ofthe-runner"}, - {name: "very-long-token-----------------------------------------------end", expected: "very-long-token-----------------------------------------------e"}, - } - - for _, example := range examples { - t.Run(example.name, func(t *testing.T) { - name := makeDNS1123Compatible(example.name) - - assert.Equal(t, example.expected, name) - assertDNS1123Compatibility(t, name) - }) - } -} diff --git a/helpers/dns/test/test.go b/helpers/dns/test/test.go new file mode 100644 index 000000000..d51927b93 --- /dev/null +++ b/helpers/dns/test/test.go @@ -0,0 +1,16 @@ +package test + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/assert" +) + +func AssertRFC1123Compatibility(t *testing.T, name string) { + dns1123MaxLength := 63 + dns1123FormatRegexp := regexp.MustCompile("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + + assert.True(t, len(name) <= dns1123MaxLength, "Name length needs to be shorter than %d", dns1123MaxLength) + assert.Regexp(t, dns1123FormatRegexp, name, "Name needs to be in RFC-1123 allowed format") +} diff --git a/helpers/dns/utils.go b/helpers/dns/utils.go new file mode 100644 index 000000000..f8201b26c --- /dev/null +++ b/helpers/dns/utils.go @@ -0,0 +1,28 @@ +package dns + +import ( + "regexp" + "strings" +) + +const ( + RFC1123NameMaximumLength = 63 + RFC1123NotAllowedCharacters = "[^-a-z0-9]" + RFC1123NotAllowedStartCharacters = "^[^a-z0-9]+" +) + +func MakeRFC1123Compatible(name string) string { + name = strings.ToLower(name) + + nameNotAllowedChars := regexp.MustCompile(RFC1123NotAllowedCharacters) + name = nameNotAllowedChars.ReplaceAllString(name, "") + + nameNotAllowedStartChars := regexp.MustCompile(RFC1123NotAllowedStartCharacters) + name = nameNotAllowedStartChars.ReplaceAllString(name, "") + + if len(name) > RFC1123NameMaximumLength { + name = name[0:RFC1123NameMaximumLength] + } + + return name +} diff --git a/helpers/dns/utils_test.go b/helpers/dns/utils_test.go new file mode 100644 index 000000000..3c0c60eac --- /dev/null +++ b/helpers/dns/utils_test.go @@ -0,0 +1,29 @@ +package dns + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "gitlab.com/gitlab-org/gitlab-runner/helpers/dns/test" +) + +func TestMakeRFC1123Compatible(t *testing.T) { + examples := []struct { + name string + expected string + }{ + {name: "tOk3_?ofTHE-Runner", expected: "tok3ofthe-runner"}, + {name: "----tOk3_?ofTHE-Runner", expected: "tok3ofthe-runner"}, + {name: "very-long-token-----------------------------------------------end", expected: "very-long-token-----------------------------------------------e"}, + } + + for _, example := range examples { + t.Run(example.name, func(t *testing.T) { + name := MakeRFC1123Compatible(example.name) + + assert.Equal(t, example.expected, name) + test.AssertRFC1123Compatibility(t, name) + }) + } +} -- GitLab