From fa3c8b1017aad050757fa1074fa32f2602cb33d0 Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Fri, 22 Dec 2023 15:45:53 +0100
Subject: [PATCH] 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.
---
 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 025556f3..54925d1c 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 d9bf3efe..62c0589d 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{},
 					},
 				},
 			},