diff --git a/.nextchanges/bundles/run-as-group.md b/.nextchanges/bundles/run-as-group.md new file mode 100644 index 00000000000..a6a39a92c5f --- /dev/null +++ b/.nextchanges/bundles/run-as-group.md @@ -0,0 +1 @@ +* Add support for `run_as.group_name` at the bundle and target levels for jobs using the direct deployment engine. ([#6676](https://github.com/databricks/cli/pull/6676)) diff --git a/acceptance/bundle/run_as/empty_run_as_dict/output.txt b/acceptance/bundle/run_as/empty_run_as_dict/output.txt index 63762765fb5..88363707b5c 100644 --- a/acceptance/bundle/run_as/empty_run_as_dict/output.txt +++ b/acceptance/bundle/run_as/empty_run_as_dict/output.txt @@ -1,4 +1,4 @@ -Error: run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified +Error: run_as section must specify exactly one non-empty identity: user_name, service_principal_name, or group_name in databricks.yml:4:9 "run_as": {}, diff --git a/acceptance/bundle/run_as/empty_sp/output.txt b/acceptance/bundle/run_as/empty_sp/output.txt index f5808a912e8..2664376966e 100644 --- a/acceptance/bundle/run_as/empty_sp/output.txt +++ b/acceptance/bundle/run_as/empty_sp/output.txt @@ -1,4 +1,4 @@ -Error: run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified +Error: run_as section must specify exactly one non-empty identity: user_name, service_principal_name, or group_name in databricks.yml:5:3 { diff --git a/acceptance/bundle/run_as/empty_user/output.txt b/acceptance/bundle/run_as/empty_user/output.txt index 77aa2920022..509819dff0a 100644 --- a/acceptance/bundle/run_as/empty_user/output.txt +++ b/acceptance/bundle/run_as/empty_user/output.txt @@ -1,4 +1,4 @@ -Error: run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified +Error: run_as section must specify exactly one non-empty identity: user_name, service_principal_name, or group_name in databricks.yml:5:3 { diff --git a/acceptance/bundle/run_as/empty_user_and_sp/output.txt b/acceptance/bundle/run_as/empty_user_and_sp/output.txt index 31712da26ba..0d330606fee 100644 --- a/acceptance/bundle/run_as/empty_user_and_sp/output.txt +++ b/acceptance/bundle/run_as/empty_user_and_sp/output.txt @@ -1,4 +1,4 @@ -Error: run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified +Error: run_as section must specify exactly one non-empty identity: user_name, service_principal_name, or group_name in databricks.yml:5:3 { diff --git a/acceptance/bundle/run_as/invalid_both_sp_and_user/output.txt b/acceptance/bundle/run_as/invalid_both_sp_and_user/output.txt index 78188b8260c..3634b730576 100644 --- a/acceptance/bundle/run_as/invalid_both_sp_and_user/output.txt +++ b/acceptance/bundle/run_as/invalid_both_sp_and_user/output.txt @@ -1,4 +1,4 @@ -Error: run_as section cannot specify both user_name and service_principal_name +Error: run_as section must specify exactly one non-empty identity: user_name, service_principal_name, or group_name in databricks.yml:6:3 Name: run_as diff --git a/bundle/config/mutator/resourcemutator/run_as.go b/bundle/config/mutator/resourcemutator/run_as.go index 4f5e3ce9036..48f69c9ce14 100644 --- a/bundle/config/mutator/resourcemutator/run_as.go +++ b/bundle/config/mutator/resourcemutator/run_as.go @@ -44,31 +44,21 @@ func reportRunAsNotSupported(resourceType string, location dyn.Location, current func validateRunAs(b *bundle.Bundle) diag.Diagnostics { diags := diag.Diagnostics{} - neitherSpecifiedErr := diag.Diagnostics{{ - Summary: "run_as section must specify exactly one identity. Neither service_principal_name nor user_name is specified", - Locations: []dyn.Location{b.Config.GetLocation("run_as")}, - Severity: diag.Error, - }} - - // Fail fast if neither service_principal_name nor user_name are specified, but the - // run_as section is present. - if b.Config.Value().Get("run_as").Kind() == dyn.KindNil { - return neitherSpecifiedErr - } - - // Fail fast if one or both of service_principal_name and user_name are specified, - // but with empty values. + identityCount := 0 runAs := b.Config.RunAs - if runAs.ServicePrincipalName == "" && runAs.UserName == "" { - return neitherSpecifiedErr + if runAs != nil { + for _, identity := range []string{runAs.UserName, runAs.ServicePrincipalName, runAs.GroupName} { + if identity != "" { + identityCount++ + } + } } - - if runAs.UserName != "" && runAs.ServicePrincipalName != "" { - diags = diags.Extend(diag.Diagnostics{{ - Summary: "run_as section cannot specify both user_name and service_principal_name", + if identityCount != 1 { + return diag.Diagnostics{{ + Summary: "run_as section must specify exactly one non-empty identity: user_name, service_principal_name, or group_name", Locations: []dyn.Location{b.Config.GetLocation("run_as")}, Severity: diag.Error, - }}) + }} } identity := runAs.ServicePrincipalName @@ -77,10 +67,24 @@ func validateRunAs(b *bundle.Bundle) diag.Diagnostics { } // All resources are supported if the run_as identity is the same as the current deployment identity. - if identity == b.Config.Workspace.CurrentUser.UserName { + if runAs.GroupName == "" && identity == b.Config.Workspace.CurrentUser.UserName { return diags } + if runAs.GroupName != "" { + identity = fmt.Sprintf("group %q", runAs.GroupName) + for _, pipeline := range b.Config.Resources.Pipelines { + if pipeline.RunAs == nil { + return diag.Errorf("this CLI version cannot configure run_as.group_name for pipelines; set run_as.user_name or run_as.service_principal_name on each pipeline") + } + } + for _, alert := range b.Config.Resources.Alerts { + if alert.RunAs == nil { + return diag.Errorf("alerts do not support run_as.group_name; set run_as.user_name or run_as.service_principal_name on each alert") + } + } + } + // Model serving endpoints do not support run_as in the API. if len(b.Config.Resources.ModelServingEndpoints) > 0 { diags = diags.Extend(reportRunAsNotSupported( @@ -141,6 +145,7 @@ func setRunAsForJobs(b *bundle.Bundle) { continue } job.RunAs = &jobs.JobRunAs{ + GroupName: runAs.GroupName, ServicePrincipalName: runAs.ServicePrincipalName, UserName: runAs.UserName, } @@ -228,6 +233,9 @@ func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { // User has opted to use the legacy behavior of run_as with the // experimental.use_legacy_run_as flag. if b.Config.Experimental != nil && b.Config.Experimental.UseLegacyRunAs { + if b.Config.Value().Get("run_as").Get("group_name").Kind() != dyn.KindInvalid { + return diag.Errorf("run_as.group_name is not supported with experimental.use_legacy_run_as; disable experimental.use_legacy_run_as to use a group identity") + } setPipelineOwnersToRunAsIdentity(b) setRunAsForJobs(b) return diag.Diagnostics{ diff --git a/bundle/config/mutator/resourcemutator/run_as_test.go b/bundle/config/mutator/resourcemutator/run_as_test.go index 13a3491a9f3..dfd64eec079 100644 --- a/bundle/config/mutator/resourcemutator/run_as_test.go +++ b/bundle/config/mutator/resourcemutator/run_as_test.go @@ -1,6 +1,7 @@ package resourcemutator import ( + "fmt" "slices" "testing" @@ -316,3 +317,121 @@ func TestRunAsNoErrorForSupportedResources(t *testing.T) { require.NoError(t, diags.Error()) } } + +func TestRunAsIdentities(t *testing.T) { + for _, tc := range []struct { + runAs string + wantError bool + }{ + {`null`, true}, + {`{}`, true}, + {`{user_name: ""}`, true}, + {`{service_principal_name: ""}`, true}, + {`{group_name: ""}`, true}, + {`{user_name: user}`, false}, + {`{service_principal_name: sp}`, false}, + {`{group_name: group}`, false}, + {`{user_name: "", service_principal_name: "", group_name: ""}`, true}, + {`{user_name: "", service_principal_name: "", group_name: group}`, false}, + {`{user_name: user, service_principal_name: sp}`, true}, + {`{user_name: user, group_name: group}`, true}, + {`{service_principal_name: sp, group_name: group}`, true}, + {`{user_name: user, service_principal_name: sp, group_name: group}`, true}, + } { + t.Run(tc.runAs, func(t *testing.T) { + yaml := "workspace: {current_user: {userName: deployer}}\nrun_as: " + tc.runAs + r, diags := config.LoadFromBytes("databricks.yml", []byte(yaml)) + require.NoError(t, diags.Error()) + b := &bundle.Bundle{Config: *r} + diags = bundle.Apply(t.Context(), b, SetRunAs()) + if tc.wantError { + require.ErrorContains(t, diags.Error(), "run_as section must specify exactly one non-empty identity: user_name, service_principal_name, or group_name") + assert.Equal(t, []dyn.Location{r.GetLocation("run_as")}, diags[0].Locations) + } else { + require.NoError(t, diags.Error()) + } + }) + } +} + +func TestRunAsLegacyGroup(t *testing.T) { + for _, runAs := range []string{`{group_name: group}`, `{group_name: ""}`} { + t.Run(runAs, func(t *testing.T) { + yaml := "workspace: {current_user: {userName: deployer}}\nexperimental: {use_legacy_run_as: true}\nrun_as: " + runAs + r, diags := config.LoadFromBytes("databricks.yml", []byte(yaml)) + require.NoError(t, diags.Error()) + b := &bundle.Bundle{Config: *r} + diags = bundle.Apply(t.Context(), b, SetRunAs()) + require.ErrorContains(t, diags.Error(), "run_as.group_name is not supported with experimental.use_legacy_run_as") + }) + } +} + +func TestRunAsGroupResources(t *testing.T) { + for _, tc := range []struct { + name string + resource string + wantError string + }{ + {name: "pipeline", resource: `pipelines: {test: {}}`, wantError: "this CLI version cannot configure run_as.group_name for pipelines"}, + {name: "alert", resource: `alerts: {test: {}}`, wantError: "alerts do not support run_as.group_name"}, + {name: "model serving", resource: `model_serving_endpoints: {test: {}}`, wantError: "Run as identity: group \"group\""}, + {name: "pipeline user override", resource: `pipelines: {test: {run_as: {user_name: user}}}`}, + {name: "alert sp override", resource: `alerts: {test: {run_as: {service_principal_name: sp}}}`}, + } { + t.Run(tc.name, func(t *testing.T) { + yaml := "run_as: {group_name: group}\nworkspace: {current_user: {userName: group}}\nresources:\n " + tc.resource + r, diags := config.LoadFromBytes("databricks.yml", []byte(yaml)) + require.NoError(t, diags.Error()) + b := &bundle.Bundle{Config: *r} + before := b.Config.Value().Get("resources") + diags = bundle.Apply(t.Context(), b, SetRunAs()) + if tc.wantError != "" { + require.Error(t, diags.Error()) + assert.Contains(t, diags.Error().Error(), tc.wantError) + } else { + require.NoError(t, diags.Error()) + assert.Equal(t, before, b.Config.Value().Get("resources")) + } + }) + } +} + +func TestRunAsGroupInheritance(t *testing.T) { + for _, tc := range []struct { + name string + root string + target string + want jobs.JobRunAs + }{ + {name: "root group", root: `{group_name: group}`, target: `{}`, want: jobs.JobRunAs{GroupName: "group"}}, + {name: "target group replaces user", root: `{user_name: user}`, target: `{run_as: {group_name: group}}`, want: jobs.JobRunAs{GroupName: "group"}}, + {name: "target user replaces group", root: `{group_name: group}`, target: `{run_as: {user_name: user}}`, want: jobs.JobRunAs{UserName: "user"}}, + } { + t.Run(tc.name, func(t *testing.T) { + yaml := fmt.Sprintf(` +workspace: {current_user: {userName: deployer}} +run_as: %s +targets: + test: %s +resources: + jobs: + inherited: {} + user: {run_as: {user_name: other_user}} + sp: {run_as: {service_principal_name: other_sp}} + group: {run_as: {group_name: other_group}} +`, tc.root, tc.target) + r, diags := config.LoadFromBytes("databricks.yml", []byte(yaml)) + require.NoError(t, diags.Error()) + require.NoError(t, r.MergeTargetOverrides("test")) + b := &bundle.Bundle{Config: *r} + diags = bundle.Apply(t.Context(), b, SetRunAs()) + require.NoError(t, diags.Error()) + assert.Equal(t, &tc.want, b.Config.RunAs) + assert.Equal(t, &tc.want, b.Config.Resources.Jobs["inherited"].RunAs) + assert.Equal(t, &jobs.JobRunAs{UserName: "other_user"}, b.Config.Resources.Jobs["user"].RunAs) + assert.Equal(t, &jobs.JobRunAs{ServicePrincipalName: "other_sp"}, b.Config.Resources.Jobs["sp"].RunAs) + assert.Equal(t, &jobs.JobRunAs{GroupName: "other_group"}, b.Config.Resources.Jobs["group"].RunAs) + }) + } +}