Use resource key as name in permissions code (#1087)

## Changes

The code relied on the `Name` property being accessible for every
resource. This is generally true, but because these property structs are
embedded as pointer, they can be nil. This is also why the tests had to
initialize the embedded struct to pass. This changes the approach to use
the keys from the resource map instead, so that we no longer rely on the
non-nil embedded struct.

Note: we should evaluate whether we should turn these into values
instead of pointers. I don't recall if we get value from them being
pointers.

## Tests

Unit tests pass.
This commit is contained in:
Pieter Noordhuis 2023-12-22 15:45:53 +01:00 committed by GitHub
parent 938eb1600c
commit fa3c8b1017
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 20 additions and 26 deletions

View File

@ -72,60 +72,60 @@ func validate(b *bundle.Bundle) error {
} }
func applyForJobs(ctx context.Context, b *bundle.Bundle) { func applyForJobs(ctx context.Context, b *bundle.Bundle) {
for _, job := range b.Config.Resources.Jobs { for key, job := range b.Config.Resources.Jobs {
job.Permissions = append(job.Permissions, convert( job.Permissions = append(job.Permissions, convert(
ctx, ctx,
b.Config.Permissions, b.Config.Permissions,
job.Permissions, job.Permissions,
job.Name, key,
levelsMap["jobs"], levelsMap["jobs"],
)...) )...)
} }
} }
func applyForPipelines(ctx context.Context, b *bundle.Bundle) { func applyForPipelines(ctx context.Context, b *bundle.Bundle) {
for _, pipeline := range b.Config.Resources.Pipelines { for key, pipeline := range b.Config.Resources.Pipelines {
pipeline.Permissions = append(pipeline.Permissions, convert( pipeline.Permissions = append(pipeline.Permissions, convert(
ctx, ctx,
b.Config.Permissions, b.Config.Permissions,
pipeline.Permissions, pipeline.Permissions,
pipeline.Name, key,
levelsMap["pipelines"], levelsMap["pipelines"],
)...) )...)
} }
} }
func applyForMlExperiments(ctx context.Context, b *bundle.Bundle) { func applyForMlExperiments(ctx context.Context, b *bundle.Bundle) {
for _, experiment := range b.Config.Resources.Experiments { for key, experiment := range b.Config.Resources.Experiments {
experiment.Permissions = append(experiment.Permissions, convert( experiment.Permissions = append(experiment.Permissions, convert(
ctx, ctx,
b.Config.Permissions, b.Config.Permissions,
experiment.Permissions, experiment.Permissions,
experiment.Name, key,
levelsMap["mlflow_experiments"], levelsMap["mlflow_experiments"],
)...) )...)
} }
} }
func applyForMlModels(ctx context.Context, b *bundle.Bundle) { func applyForMlModels(ctx context.Context, b *bundle.Bundle) {
for _, model := range b.Config.Resources.Models { for key, model := range b.Config.Resources.Models {
model.Permissions = append(model.Permissions, convert( model.Permissions = append(model.Permissions, convert(
ctx, ctx,
b.Config.Permissions, b.Config.Permissions,
model.Permissions, model.Permissions,
model.Name, key,
levelsMap["mlflow_models"], levelsMap["mlflow_models"],
)...) )...)
} }
} }
func applyForModelServiceEndpoints(ctx context.Context, b *bundle.Bundle) { func applyForModelServiceEndpoints(ctx context.Context, b *bundle.Bundle) {
for _, model := range b.Config.Resources.ModelServingEndpoints { for key, model := range b.Config.Resources.ModelServingEndpoints {
model.Permissions = append(model.Permissions, convert( model.Permissions = append(model.Permissions, convert(
ctx, ctx,
b.Config.Permissions, b.Config.Permissions,
model.Permissions, model.Permissions,
model.Name, key,
levelsMap["model_serving_endpoints"], levelsMap["model_serving_endpoints"],
)...) )...)
} }

View File

@ -7,10 +7,6 @@ import (
"github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/databricks-sdk-go/service/jobs"
"github.com/databricks/databricks-sdk-go/service/ml"
"github.com/databricks/databricks-sdk-go/service/pipelines"
"github.com/databricks/databricks-sdk-go/service/serving"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
@ -27,24 +23,24 @@ func TestApplyBundlePermissions(t *testing.T) {
}, },
Resources: config.Resources{ Resources: config.Resources{
Jobs: map[string]*resources.Job{ Jobs: map[string]*resources.Job{
"job_1": {JobSettings: &jobs.JobSettings{}}, "job_1": {},
"job_2": {JobSettings: &jobs.JobSettings{}}, "job_2": {},
}, },
Pipelines: map[string]*resources.Pipeline{ Pipelines: map[string]*resources.Pipeline{
"pipeline_1": {PipelineSpec: &pipelines.PipelineSpec{}}, "pipeline_1": {},
"pipeline_2": {PipelineSpec: &pipelines.PipelineSpec{}}, "pipeline_2": {},
}, },
Models: map[string]*resources.MlflowModel{ Models: map[string]*resources.MlflowModel{
"model_1": {Model: &ml.Model{}}, "model_1": {},
"model_2": {Model: &ml.Model{}}, "model_2": {},
}, },
Experiments: map[string]*resources.MlflowExperiment{ Experiments: map[string]*resources.MlflowExperiment{
"experiment_1": {Experiment: &ml.Experiment{}}, "experiment_1": {},
"experiment_2": {Experiment: &ml.Experiment{}}, "experiment_2": {},
}, },
ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{ ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{
"endpoint_1": {CreateServingEndpoint: &serving.CreateServingEndpoint{}}, "endpoint_1": {},
"endpoint_2": {CreateServingEndpoint: &serving.CreateServingEndpoint{}}, "endpoint_2": {},
}, },
}, },
}, },
@ -116,13 +112,11 @@ func TestWarningOnOverlapPermission(t *testing.T) {
Permissions: []resources.Permission{ Permissions: []resources.Permission{
{Level: CAN_VIEW, UserName: "TestUser"}, {Level: CAN_VIEW, UserName: "TestUser"},
}, },
JobSettings: &jobs.JobSettings{},
}, },
"job_2": { "job_2": {
Permissions: []resources.Permission{ Permissions: []resources.Permission{
{Level: CAN_VIEW, UserName: "TestUser2"}, {Level: CAN_VIEW, UserName: "TestUser2"},
}, },
JobSettings: &jobs.JobSettings{},
}, },
}, },
}, },