From 2f8df81159e2e20de06ae49d5aa290324100fd2c Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 19 Dec 2024 14:00:06 +0100 Subject: [PATCH] Added same source validation and made thread safe --- bundle/apps/upload_config.go | 14 ++++++-------- bundle/apps/validate.go | 13 ++++++++++++- bundle/apps/validate_test.go | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/bundle/apps/upload_config.go b/bundle/apps/upload_config.go index 4eb90f905..b37f06351 100644 --- a/bundle/apps/upload_config.go +++ b/bundle/apps/upload_config.go @@ -6,6 +6,7 @@ import ( "fmt" "path" "strings" + "sync" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/resources" @@ -25,8 +26,7 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos var diags diag.Diagnostics errGroup, ctx := errgroup.WithContext(ctx) - diagsPerApp := make(map[string]diag.Diagnostic) - + mu := &sync.Mutex{} for key, app := range b.Config.Resources.Apps { // If the app has a config, we need to deploy it first. // It means we need to write app.yml file with the content of the config field @@ -59,12 +59,14 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos errGroup.Go(func() error { err = f.Write(ctx, path.Join(appPath, "app.yml"), buf, filer.OverwriteIfExists) if err != nil { - diagsPerApp[key] = diag.Diagnostic{ + mu.Lock() + diags = append(diags, diag.Diagnostic{ Severity: diag.Error, Summary: "Failed to save config", Detail: fmt.Sprintf("Failed to write %s file: %s", path.Join(app.SourceCodePath, "app.yml"), err), Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s", key)), - } + }) + mu.Unlock() } return nil }) @@ -75,10 +77,6 @@ func (u *uploadConfig) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos return diags.Extend(diag.FromErr(err)) } - for _, diag := range diagsPerApp { - diags = append(diags, diag) - } - return diags } diff --git a/bundle/apps/validate.go b/bundle/apps/validate.go index dfb3fffad..2579db73e 100644 --- a/bundle/apps/validate.go +++ b/bundle/apps/validate.go @@ -14,8 +14,19 @@ type validate struct{} func (v *validate) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { var diags diag.Diagnostics possibleConfigFiles := []string{"app.yml", "app.yaml"} + usedSourceCodePaths := make(map[string]string) + + for key, app := range b.Config.Resources.Apps { + if _, ok := usedSourceCodePaths[app.SourceCodePath]; ok { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: "Duplicate app source code path", + Detail: fmt.Sprintf("app resource '%s' has the same source code path as app resource '%s'", key, usedSourceCodePaths[app.SourceCodePath]), + Locations: b.Config.GetLocations(fmt.Sprintf("resources.apps.%s.source_code_path", key)), + }) + } + usedSourceCodePaths[app.SourceCodePath] = key - for _, app := range b.Config.Resources.Apps { for _, configFile := range possibleConfigFiles { cf := path.Join(app.SourceCodePath, configFile) if _, err := b.SyncRoot.Stat(cf); err == nil { diff --git a/bundle/apps/validate_test.go b/bundle/apps/validate_test.go index d14d23731..6ae0fd806 100644 --- a/bundle/apps/validate_test.go +++ b/bundle/apps/validate_test.go @@ -53,3 +53,39 @@ func TestAppsValidate(t *testing.T) { require.Equal(t, "app.yml detected", diags[0].Summary) require.Contains(t, diags[0].Detail, "app.yml and use 'config' property for app resource") } + +func TestAppsValidateSameSourcePath(t *testing.T) { + tmpDir := t.TempDir() + testutil.Touch(t, tmpDir, "app1", "app.py") + + b := &bundle.Bundle{ + BundleRootPath: tmpDir, + SyncRootPath: tmpDir, + SyncRoot: vfs.MustNew(tmpDir), + Config: config.Root{ + Resources: config.Resources{ + Apps: map[string]*resources.App{ + "app1": { + App: &apps.App{ + Name: "app1", + }, + SourceCodePath: "./app1", + }, + "app2": { + App: &apps.App{ + Name: "app2", + }, + SourceCodePath: "./app1", + }, + }, + }, + }, + } + + bundletest.SetLocation(b, ".", []dyn.Location{{File: filepath.Join(tmpDir, "databricks.yml")}}) + + diags := bundle.Apply(context.Background(), b, bundle.Seq(mutator.TranslatePaths(), Validate())) + require.Len(t, diags, 1) + require.Equal(t, "Duplicate app source code path", diags[0].Summary) + require.Contains(t, diags[0].Detail, "has the same source code path as app resource") +}