From 229c264180ed811c39faf822c346d0137b6781eb Mon Sep 17 00:00:00 2001 From: Mike Verbanic Date: Tue, 10 Aug 2021 16:42:53 -0400 Subject: [PATCH] Refactored common code --- pkg/skaffold/instrumentation/export.go | 15 ++---- pkg/skaffold/user/user.go | 39 +++++++++++++++ pkg/skaffold/user/user_test.go | 68 ++++++++++++++++++++++++++ pkg/skaffold/version/version.go | 18 ++----- 4 files changed, 114 insertions(+), 26 deletions(-) create mode 100644 pkg/skaffold/user/user.go create mode 100644 pkg/skaffold/user/user_test.go diff --git a/pkg/skaffold/instrumentation/export.go b/pkg/skaffold/instrumentation/export.go index 95e69d4049c..90087b8ac4e 100644 --- a/pkg/skaffold/instrumentation/export.go +++ b/pkg/skaffold/instrumentation/export.go @@ -25,7 +25,6 @@ import ( "math/rand" "os" "path/filepath" - "regexp" "strconv" "time" @@ -49,6 +48,7 @@ import ( "github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/statik" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/user" "github.com/GoogleContainerTools/skaffold/proto/v1" ) @@ -181,17 +181,8 @@ func createMetrics(ctx context.Context, meter skaffoldMeter) { randLabel, } - // check each allowed user key for pattern match - for allowedUser := range constants.AllowedUsers { - matched, err := regexp.MatchString(fmt.Sprintf(constants.AllowedUserPattern, allowedUser), meter.User) - if err != nil { - panic(fmt.Sprintf("error matching allowed user: %v", err)) - } - - if matched || allowedUser == meter.User { - sharedLabels = append(sharedLabels, attribute.String("user", meter.User)) - break - } + if allowedUser := user.IsAllowedUser(meter.User); allowedUser { + sharedLabels = append(sharedLabels, attribute.String("user", meter.User)) } labels = append(labels, sharedLabels...) diff --git a/pkg/skaffold/user/user.go b/pkg/skaffold/user/user.go new file mode 100644 index 00000000000..072be95227a --- /dev/null +++ b/pkg/skaffold/user/user.go @@ -0,0 +1,39 @@ +/* +Copyright 2021 The Skaffold Authors + +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 user + +import ( + "fmt" + "regexp" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" +) + +func IsAllowedUser(user string) bool { + for allowedUser := range constants.AllowedUsers { + matched, err := regexp.MatchString(fmt.Sprintf(constants.AllowedUserPattern, allowedUser), user) + if err != nil { + panic(fmt.Sprintf("error matching allowed user: %v", err)) + } + + if matched { + return true + } + } + + return false +} diff --git a/pkg/skaffold/user/user_test.go b/pkg/skaffold/user/user_test.go new file mode 100644 index 00000000000..96d6ead1c70 --- /dev/null +++ b/pkg/skaffold/user/user_test.go @@ -0,0 +1,68 @@ +/* +Copyright 2021 The Skaffold Authors + +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 user + +import ( + "testing" + + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func TestAllowedUser(t *testing.T) { + tests := []struct { + description string + user string + expected bool + }{ + { + description: "user in allowed list", + user: "vsc", + expected: true, + }, + { + description: "user in allowed list", + user: "cloud-deploy", + expected: true, + }, + { + description: "user in allowed list with valid pattern", + user: "cloud-deploy/dev", + expected: true, + }, + { + description: "user in allowed list with invalid pattern (only slash)", + user: "cloud-deploy|dev", + expected: false, + }, + { + description: "user in allowed list with invalid pattern (suffix required)", + user: "cloud-deploy/", + expected: false, + }, + { + description: "user not in allowed list", + user: "test-user", + expected: false, + }, + } + for _, test := range tests { + testutil.Run(t, "", func(t *testutil.T) { + allowedUser := IsAllowedUser(test.user) + t.CheckDeepEqual(test.expected, allowedUser) + }) + } +} diff --git a/pkg/skaffold/version/version.go b/pkg/skaffold/version/version.go index 8c2710ba3f1..0d5f434de09 100644 --- a/pkg/skaffold/version/version.go +++ b/pkg/skaffold/version/version.go @@ -18,14 +18,13 @@ package version import ( "fmt" - "regexp" "runtime" "strings" "github.com/blang/semver" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/user" ) var version, gitCommit, buildDate, client string @@ -58,18 +57,9 @@ var Get = func() *Info { } } -var SetClient = func(user string) { - // check each allowed user key for pattern match - for allowedUser := range constants.AllowedUsers { - matched, err := regexp.MatchString(fmt.Sprintf(constants.AllowedUserPattern, allowedUser), user) - if err != nil { - panic(fmt.Sprintf("error matching allowed user: %v", err)) - } - - if matched || allowedUser == user { - client = user - break - } +var SetClient = func(userAgent string) { + if allowedUser := user.IsAllowedUser(userAgent); allowedUser { + client = userAgent } }