Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit warnings when conflicting configuration is specified #1506

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions bundle/config/validate/conflicting_configuration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package validate

import (
"context"
"fmt"
"path/filepath"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
)

// This mutator emits warnings if a configuration value is being override by another
// value in the configuration files, effectively making the configuration useless.
func ConflictingConfiguration() bundle.ReadOnlyMutator {
return &conflictingConfiguration{}
}

type conflictingConfiguration struct{}

func (v *conflictingConfiguration) Name() string {
return "validate:conflicting_configuration"
}

func isKindScalar(v dyn.Value) bool {
switch v.Kind() {
case dyn.KindString, dyn.KindInt, dyn.KindBool, dyn.KindFloat, dyn.KindTime, dyn.KindNil:
return true
default:
return false
}
}

func conflictingConfigurationWarning(v dyn.Value, p dyn.Path, rb bundle.ReadOnlyBundle) string {
loc := v.Location()
rel, err := filepath.Rel(rb.RootPath(), loc.File)
if err == nil {
loc.File = rel
}

yamlLocations := v.YamlLocations()

Check failure on line 41 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / validate-bundle-schema

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations)

Check failure on line 41 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (macos-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations)

Check failure on line 41 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (macos-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations)

Check failure on line 41 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (ubuntu-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations)

Check failure on line 41 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (ubuntu-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations)

Check failure on line 41 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (windows-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations)

Check failure on line 41 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (windows-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations)
for i, yamlLocation := range yamlLocations {
rel, err := filepath.Rel(rb.RootPath(), yamlLocation.File)
if err == nil {
yamlLocations[i].File = rel
}
}

return fmt.Sprintf("Multiple values found for the same configuration %s. Only the value from location %s will be used. Locations found: %s", p.String(), loc, yamlLocations)
}

func (v *conflictingConfiguration) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.Diagnostics {
diags := diag.Diagnostics{}

_, err := dyn.Walk(rb.Config().Value(), func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
if !isKindScalar(v) {
return v, nil
}

if len(v.YamlLocations()) >= 2 {

Check failure on line 60 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / validate-bundle-schema

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations)

Check failure on line 60 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (macos-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations) (compile)

Check failure on line 60 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (macos-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations) (compile)

Check failure on line 60 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (ubuntu-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations) (compile)

Check failure on line 60 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (ubuntu-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations) (compile)

Check failure on line 60 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (windows-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations) (compile)

Check failure on line 60 in bundle/config/validate/conflicting_configuration.go

View workflow job for this annotation

GitHub Actions / tests (windows-latest)

v.YamlLocations undefined (type dyn.Value has no field or method YamlLocations) (compile)
diags = diags.Append(diag.Diagnostic{
Severity: diag.Warning,
Summary: conflictingConfigurationWarning(v, p, rb),
Location: v.Location(),
Path: p,
})
}
return v, nil
})
diags = append(diags, diag.FromErr(err)...)
return diags
}
28 changes: 28 additions & 0 deletions bundle/config/validate/pre_initialize.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package validate

import (
"context"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
)

type preInitialize struct{}

// Apply implements bundle.Mutator.
func (v *preInitialize) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
return bundle.ApplyReadOnly(ctx, bundle.ReadOnly(b), bundle.Parallel(
ConflictingConfiguration(),
))
}

// Name implements bundle.Mutator.
func (v *preInitialize) Name() string {
return "validate:pre_initialize"
}

// Validations to perform before initialization of the bundle. These validations
// are thus applied for most bundle commands.
func PreInitialize() bundle.Mutator {
return &preInitialize{}
}
2 changes: 2 additions & 0 deletions bundle/phases/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/mutator"
"github.com/databricks/cli/bundle/config/validate"
"github.com/databricks/cli/bundle/deploy/metadata"
"github.com/databricks/cli/bundle/deploy/terraform"
"github.com/databricks/cli/bundle/permissions"
Expand All @@ -18,6 +19,7 @@ func Initialize() bundle.Mutator {
return newPhase(
"initialize",
[]bundle.Mutator{
validate.PreInitialize(),
mutator.RewriteSyncPaths(),
mutator.MergeJobClusters(),
mutator.MergeJobTasks(),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
bundle:
name: test warnings for conflicting configuration

include:
- "*.yml"

variables:
bar:
default: conflicts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
variables:
bar:
default: conflicts

baz:
default: conflicts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
variables:
baz:
default: conflicts
42 changes: 42 additions & 0 deletions bundle/tests/validate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package config_tests

import (
"context"
"path/filepath"
"testing"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/validate"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/dyn"
"github.com/stretchr/testify/assert"
)

func TestConflictingConfigurationValidate(t *testing.T) {
b := load(t, "validate/conflicting_configuration")

ctx := context.Background()
diags := bundle.ApplyReadOnly(ctx, bundle.ReadOnly(b), validate.ConflictingConfiguration())

assert.Len(t, diags, 2)
assert.Contains(t, diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Multiple values found for the same configuration variables.baz.default. Only the value from location resources2.yml:3:14 will be used. Locations found: [resources2.yml:3:14 resources1.yml:6:14]",
Location: dyn.Location{
File: filepath.FromSlash("validate/conflicting_configuration/resources2.yml"),
Line: 3,
Column: 14,
},
Path: dyn.MustPathFromString("variables.baz.default"),
})
assert.Contains(t, diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Multiple values found for the same configuration variables.bar.default. Only the value from location resources1.yml:3:14 will be used. Locations found: [resources1.yml:3:14 databricks.yml:9:14]",
Location: dyn.Location{
File: filepath.FromSlash("validate/conflicting_configuration/resources1.yml"),
Line: 3,
Column: 14,
},
Path: dyn.MustPathFromString("variables.bar.default"),
})
}
Loading