From 3685eb16f4041b20a97da0ec15f2ea1a7fe4e935 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 29 Sep 2023 10:38:06 +0200 Subject: [PATCH] Run tests to verify backend tag validation behavior (#814) ## Changes Validation rules on tags are different per cloud (they are passed through to the underlying clusters and as such must comply with cloud-specific validation rules). This change adds tests to confirm the current behavior to ensure the normalization we can apply is in line with how the backend behaves. ## Tests The new integration tests pass (tested locally). --- internal/tags_test.go | 259 +++++++++++++++++++++++++++++++ internal/testutil/cloud.go | 48 ++++++ internal/testutil/env.go | 9 ++ internal/testutil/requirement.go | 19 +++ 4 files changed, 335 insertions(+) create mode 100644 internal/tags_test.go create mode 100644 internal/testutil/cloud.go create mode 100644 internal/testutil/requirement.go diff --git a/internal/tags_test.go b/internal/tags_test.go new file mode 100644 index 00000000..2dd3759a --- /dev/null +++ b/internal/tags_test.go @@ -0,0 +1,259 @@ +package internal + +import ( + "context" + "strings" + "testing" + + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/require" +) + +func testTags(t *testing.T, tags map[string]string) error { + var nodeTypeId string + switch testutil.GetCloud(t) { + case testutil.AWS: + nodeTypeId = "i3.xlarge" + case testutil.Azure: + nodeTypeId = "Standard_DS4_v2" + case testutil.GCP: + nodeTypeId = "n1-standard-4" + } + + w, err := databricks.NewWorkspaceClient() + require.NoError(t, err) + + ctx := context.Background() + resp, err := w.Jobs.Create(ctx, jobs.CreateJob{ + Name: RandomName("test-tags-"), + Tasks: []jobs.Task{ + { + TaskKey: "test", + NewCluster: &compute.ClusterSpec{ + SparkVersion: "13.3.x-scala2.12", + NumWorkers: 1, + NodeTypeId: nodeTypeId, + }, + SparkPythonTask: &jobs.SparkPythonTask{ + PythonFile: "/doesnt_exist.py", + }, + }, + }, + Tags: tags, + }) + + if resp != nil { + t.Cleanup(func() { + w.Jobs.DeleteByJobId(ctx, resp.JobId) + }) + } + + return err +} + +func testTagKey(t *testing.T, key string) error { + return testTags(t, map[string]string{ + key: "value", + }) +} + +func testTagValue(t *testing.T, value string) error { + return testTags(t, map[string]string{ + "key": value, + }) +} + +type tagTestCase struct { + name string + value string + fn func(t *testing.T, value string) error + err string +} + +func runTagTestCases(t *testing.T, cases []tagTestCase) { + for i := range cases { + tc := cases[i] + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + err := tc.fn(t, tc.value) + if tc.err == "" { + require.NoError(t, err) + } else { + require.Error(t, err) + msg := strings.ReplaceAll(err.Error(), "\n", " ") + require.Contains(t, msg, tc.err) + } + }) + } +} + +func TestAccTagKeyAWS(t *testing.T) { + testutil.Require(t, testutil.AWS) + t.Parallel() + + runTagTestCases(t, []tagTestCase{ + { + name: "invalid", + value: "café", + fn: testTagKey, + err: ` The key must match the regular expression ^[\d \w\+\-=\.:\/@]*$.`, + }, + { + name: "unicode", + value: "🍎", + fn: testTagKey, + err: ` contains non-latin1 characters.`, + }, + { + name: "empty", + value: "", + fn: testTagKey, + err: ` the minimal length is 1, and the maximum length is 127.`, + }, + { + name: "valid", + value: "cafe", + fn: testTagKey, + err: ``, + }, + }) +} + +func TestAccTagValueAWS(t *testing.T) { + testutil.Require(t, testutil.AWS) + t.Parallel() + + runTagTestCases(t, []tagTestCase{ + { + name: "invalid", + value: "café", + fn: testTagValue, + err: ` The value must match the regular expression ^[\d \w\+\-=\.:/@]*$.`, + }, + { + name: "unicode", + value: "🍎", + fn: testTagValue, + err: ` contains non-latin1 characters.`, + }, + { + name: "valid", + value: "cafe", + fn: testTagValue, + err: ``, + }, + }) +} + +func TestAccTagKeyAzure(t *testing.T) { + testutil.Require(t, testutil.Azure) + t.Parallel() + + runTagTestCases(t, []tagTestCase{ + { + name: "invalid", + value: "café?", + fn: testTagKey, + err: ` The key must match the regular expression ^[^<>\*&%;\\\/\+\?]*$.`, + }, + { + name: "unicode", + value: "🍎", + fn: testTagKey, + err: ` contains non-latin1 characters.`, + }, + { + name: "empty", + value: "", + fn: testTagKey, + err: ` the minimal length is 1, and the maximum length is 512.`, + }, + { + name: "valid", + value: "cafe", + fn: testTagKey, + err: ``, + }, + }) +} + +func TestAccTagValueAzure(t *testing.T) { + testutil.Require(t, testutil.Azure) + t.Parallel() + + runTagTestCases(t, []tagTestCase{ + { + name: "unicode", + value: "🍎", + fn: testTagValue, + err: ` contains non-latin1 characters.`, + }, + { + name: "valid", + value: "cafe", + fn: testTagValue, + err: ``, + }, + }) +} + +func TestAccTagKeyGCP(t *testing.T) { + testutil.Require(t, testutil.GCP) + t.Parallel() + + runTagTestCases(t, []tagTestCase{ + { + name: "invalid", + value: "café?", + fn: testTagKey, + err: ` The key must match the regular expression ^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$.`, + }, + { + name: "unicode", + value: "🍎", + fn: testTagKey, + err: ` contains non-latin1 characters.`, + }, + { + name: "empty", + value: "", + fn: testTagKey, + err: ` the minimal length is 1, and the maximum length is 63.`, + }, + { + name: "valid", + value: "cafe", + fn: testTagKey, + err: ``, + }, + }) +} + +func TestAccTagValueGCP(t *testing.T) { + testutil.Require(t, testutil.GCP) + t.Parallel() + + runTagTestCases(t, []tagTestCase{ + { + name: "invalid", + value: "café", + fn: testTagValue, + err: ` The value must match the regular expression ^(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])?$.`, + }, + { + name: "unicode", + value: "🍎", + fn: testTagValue, + err: ` contains non-latin1 characters.`, + }, + { + name: "valid", + value: "cafe", + fn: testTagValue, + err: ``, + }, + }) +} diff --git a/internal/testutil/cloud.go b/internal/testutil/cloud.go new file mode 100644 index 00000000..50bbf67f --- /dev/null +++ b/internal/testutil/cloud.go @@ -0,0 +1,48 @@ +package testutil + +import ( + "testing" +) + +type Cloud int + +const ( + AWS Cloud = iota + Azure + GCP +) + +// Implement [Requirement]. +func (c Cloud) Verify(t *testing.T) { + if c != GetCloud(t) { + t.Skipf("Skipping %s-specific test", c) + } +} + +func (c Cloud) String() string { + switch c { + case AWS: + return "AWS" + case Azure: + return "Azure" + case GCP: + return "GCP" + default: + return "unknown" + } +} + +func GetCloud(t *testing.T) Cloud { + env := GetEnvOrSkipTest(t, "CLOUD_ENV") + switch env { + case "aws": + return AWS + case "azure": + return Azure + case "gcp": + return GCP + default: + t.Fatalf("Unknown cloud environment: %s", env) + } + return -1 +} diff --git a/internal/testutil/env.go b/internal/testutil/env.go index 11a61018..39201c5b 100644 --- a/internal/testutil/env.go +++ b/internal/testutil/env.go @@ -35,3 +35,12 @@ func CleanupEnvironment(t *testing.T) { t.Setenv("USERPROFILE", pwd) } } + +// GetEnvOrSkipTest proceeds with test only with that env variable +func GetEnvOrSkipTest(t *testing.T, name string) string { + value := os.Getenv(name) + if value == "" { + t.Skipf("Environment variable %s is missing", name) + } + return value +} diff --git a/internal/testutil/requirement.go b/internal/testutil/requirement.go new file mode 100644 index 00000000..53855e0b --- /dev/null +++ b/internal/testutil/requirement.go @@ -0,0 +1,19 @@ +package testutil + +import ( + "testing" +) + +// Requirement is the interface for test requirements. +type Requirement interface { + Verify(t *testing.T) +} + +// Require should be called at the beginning of a test to ensure that all +// requirements are met before running the test. +// If any requirement is not met, the test will be skipped. +func Require(t *testing.T, requirements ...Requirement) { + for _, r := range requirements { + r.Verify(t) + } +}