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

@@ -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 {

executors/docker/machine/name_test.go

 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{

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 {

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())
 			},
 		},
 	}
@@ -1451,7 +1452,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())
 			},
 		},
 	}

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
-}

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)
-		})
-	}
-}

helpers/dns/test/test.go

+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")
+}

helpers/dns/utils.go

+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
+}

helpers/dns/utils_test.go

+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)
+		})
+	}
+}