Require include glob patterns to be explicitly defined (#602)

## Changes
Before this PR we would load all yaml files matching * and \*/\*.yml
files as bundle configurations. This was problematic since this would
also load yaml files that were not meant to be a part of the bundle

## Tests
Manually, now files are no longer included unless manually specified
This commit is contained in:
shreyas-goenka 2023-07-25 10:00:46 +02:00 committed by GitHub
parent 13731e144c
commit fa37449f1f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 43 additions and 108 deletions

View File

@ -1,36 +0,0 @@
package mutator
import (
"context"
"github.com/databricks/cli/bundle"
"golang.org/x/exp/slices"
)
type defineDefaultInclude struct {
include []string
}
// DefineDefaultInclude sets the list of includes to a default if it hasn't been set.
func DefineDefaultInclude() bundle.Mutator {
return &defineDefaultInclude{
// When we support globstar we can collapse below into a single line.
include: []string{
// Load YAML files in the same directory.
"*.yml",
// Load YAML files in subdirectories.
"*/*.yml",
},
}
}
func (m *defineDefaultInclude) Name() string {
return "DefineDefaultInclude"
}
func (m *defineDefaultInclude) Apply(_ context.Context, b *bundle.Bundle) error {
if len(b.Config.Include) == 0 {
b.Config.Include = slices.Clone(m.include)
}
return nil
}

View File

@ -1,18 +0,0 @@
package mutator_test
import (
"context"
"testing"
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/mutator"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestDefaultInclude(t *testing.T) {
bundle := &bundle.Bundle{}
err := mutator.DefineDefaultInclude().Apply(context.Background(), bundle)
require.NoError(t, err)
assert.Equal(t, []string{"*.yml", "*/*.yml"}, bundle.Config.Include)
}

View File

@ -6,7 +6,6 @@ import (
func DefaultMutators() []bundle.Mutator {
return []bundle.Mutator{
DefineDefaultInclude(),
ProcessRootIncludes(),
DefineDefaultEnvironment(),
LoadGitDetails(),

View File

@ -54,11 +54,8 @@ type Root struct {
Bundle Bundle `json:"bundle"`
// Include specifies a list of patterns of file names to load and
// merge into the this configuration. If not set in `databricks.yml`,
// it defaults to loading `*.yml` and `*/*.yml`.
//
// Also see [mutator.DefineDefaultInclude].
//
// merge into the this configuration. Only includes defined in the root
// `databricks.yml` are processed. Defaults to an empty list.
Include []string `json:"include,omitempty"`
// Workspace contains details about the workspace to connect to

View File

@ -1838,7 +1838,7 @@
}
},
"include": {
"description": "A list of patterns of file names to load and merge into the this configuration. It defaults to loading `*.yml` and `*/*.yml`.",
"description": "A list of glob patterns of files to load and merge into the this configuration. Defaults to no files being included.",
"items": {
"description": ""
}

View File

@ -4,6 +4,9 @@ bundle:
workspace:
profile: test
include:
- "*.yml"
resources:
jobs:
foo:

View File

@ -3,3 +3,6 @@ bundle:
workspace:
profile: test
include:
- "*.yml"

View File

@ -1,27 +0,0 @@
package config_tests
import (
"path/filepath"
"sort"
"testing"
"github.com/stretchr/testify/assert"
"golang.org/x/exp/maps"
)
func TestIncludeDefault(t *testing.T) {
b := load(t, "./include_default")
// Test that both jobs were loaded.
keys := maps.Keys(b.Config.Resources.Jobs)
sort.Strings(keys)
assert.Equal(t, []string{"my_first_job", "my_second_job"}, keys)
first := b.Config.Resources.Jobs["my_first_job"]
assert.Equal(t, "1", first.ID)
assert.Equal(t, "include_default/my_first_job/resource.yml", filepath.ToSlash(first.ConfigFilePath))
second := b.Config.Resources.Jobs["my_second_job"]
assert.Equal(t, "2", second.ID)
assert.Equal(t, "include_default/my_second_job/resource.yml", filepath.ToSlash(second.ConfigFilePath))
}

View File

@ -0,0 +1,5 @@
bundle:
name: include_default
include:
- "*/*.yml"

View File

@ -1,4 +1,4 @@
resources:
jobs:
this_job_isnt_defined:
my_first_job:
id: 1

View File

@ -0,0 +1,4 @@
resources:
jobs:
my_second_job:
id: 2

View File

@ -1,7 +0,0 @@
bundle:
name: include_override
# Setting this explicitly means default globs are not processed.
# As a result, ./this_file_isnt_included.yml isn't included.
include:
- doesnt-exist/*.yml

View File

@ -1,12 +0,0 @@
package config_tests
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestIncludeOverride(t *testing.T) {
b := load(t, "./include_override")
assert.Empty(t, b.Config.Resources.Jobs)
}

View File

@ -32,3 +32,27 @@ func TestIncludeWithGlob(t *testing.T) {
assert.Equal(t, "1", job.ID)
assert.Equal(t, "include_with_glob/job.yml", filepath.ToSlash(job.ConfigFilePath))
}
func TestIncludeDefault(t *testing.T) {
b := load(t, "./include_default")
// No jobs should have been loaded
assert.Empty(t, b.Config.Resources.Jobs)
}
func TestIncludeForMultipleMatches(t *testing.T) {
b := load(t, "./include_multiple")
// Test that both jobs were loaded.
keys := maps.Keys(b.Config.Resources.Jobs)
sort.Strings(keys)
assert.Equal(t, []string{"my_first_job", "my_second_job"}, keys)
first := b.Config.Resources.Jobs["my_first_job"]
assert.Equal(t, "1", first.ID)
assert.Equal(t, "include_multiple/my_first_job/resource.yml", filepath.ToSlash(first.ConfigFilePath))
second := b.Config.Resources.Jobs["my_second_job"]
assert.Equal(t, "2", second.ID)
assert.Equal(t, "include_multiple/my_second_job/resource.yml", filepath.ToSlash(second.ConfigFilePath))
}