From cdbea58cf52e93d5435a616636d4e835c2a866a8 Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Fri, 22 Dec 2023 15:38:28 +0100
Subject: [PATCH] Use resource key as name in permissions code

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.
---
 bundle/permissions/mutator.go      | 20 ++++++++++----------
 bundle/permissions/mutator_test.go | 26 ++++++++++----------------
 2 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/bundle/permissions/mutator.go b/bundle/permissions/mutator.go
index 025556f31e..54925d1c8d 100644
--- a/bundle/permissions/mutator.go
+++ b/bundle/permissions/mutator.go
@@ -72,60 +72,60 @@ func validate(b *bundle.Bundle) error {
 }
 
 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(
 			ctx,
 			b.Config.Permissions,
 			job.Permissions,
-			job.Name,
+			key,
 			levelsMap["jobs"],
 		)...)
 	}
 }
 
 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(
 			ctx,
 			b.Config.Permissions,
 			pipeline.Permissions,
-			pipeline.Name,
+			key,
 			levelsMap["pipelines"],
 		)...)
 	}
 }
 
 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(
 			ctx,
 			b.Config.Permissions,
 			experiment.Permissions,
-			experiment.Name,
+			key,
 			levelsMap["mlflow_experiments"],
 		)...)
 	}
 }
 
 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(
 			ctx,
 			b.Config.Permissions,
 			model.Permissions,
-			model.Name,
+			key,
 			levelsMap["mlflow_models"],
 		)...)
 	}
 }
 
 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(
 			ctx,
 			b.Config.Permissions,
 			model.Permissions,
-			model.Name,
+			key,
 			levelsMap["model_serving_endpoints"],
 		)...)
 	}
diff --git a/bundle/permissions/mutator_test.go b/bundle/permissions/mutator_test.go
index d9bf3efe76..62c0589d3e 100644
--- a/bundle/permissions/mutator_test.go
+++ b/bundle/permissions/mutator_test.go
@@ -7,10 +7,6 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/bundle/config"
 	"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"
 )
 
@@ -27,24 +23,24 @@ func TestApplyBundlePermissions(t *testing.T) {
 			},
 			Resources: config.Resources{
 				Jobs: map[string]*resources.Job{
-					"job_1": {JobSettings: &jobs.JobSettings{}},
-					"job_2": {JobSettings: &jobs.JobSettings{}},
+					"job_1": {},
+					"job_2": {},
 				},
 				Pipelines: map[string]*resources.Pipeline{
-					"pipeline_1": {PipelineSpec: &pipelines.PipelineSpec{}},
-					"pipeline_2": {PipelineSpec: &pipelines.PipelineSpec{}},
+					"pipeline_1": {},
+					"pipeline_2": {},
 				},
 				Models: map[string]*resources.MlflowModel{
-					"model_1": {Model: &ml.Model{}},
-					"model_2": {Model: &ml.Model{}},
+					"model_1": {},
+					"model_2": {},
 				},
 				Experiments: map[string]*resources.MlflowExperiment{
-					"experiment_1": {Experiment: &ml.Experiment{}},
-					"experiment_2": {Experiment: &ml.Experiment{}},
+					"experiment_1": {},
+					"experiment_2": {},
 				},
 				ModelServingEndpoints: map[string]*resources.ModelServingEndpoint{
-					"endpoint_1": {CreateServingEndpoint: &serving.CreateServingEndpoint{}},
-					"endpoint_2": {CreateServingEndpoint: &serving.CreateServingEndpoint{}},
+					"endpoint_1": {},
+					"endpoint_2": {},
 				},
 			},
 		},
@@ -116,13 +112,11 @@ func TestWarningOnOverlapPermission(t *testing.T) {
 						Permissions: []resources.Permission{
 							{Level: CAN_VIEW, UserName: "TestUser"},
 						},
-						JobSettings: &jobs.JobSettings{},
 					},
 					"job_2": {
 						Permissions: []resources.Permission{
 							{Level: CAN_VIEW, UserName: "TestUser2"},
 						},
-						JobSettings: &jobs.JobSettings{},
 					},
 				},
 			},