direct: Fix permissions for resources.models

NEXT_CHANGELOG.md

@@ -11,6 +11,7 @@
 
 ### Bundles
 * Added support for lifecycle.started option for apps ([#4672](https://github.com/databricks/cli/pull/4672))
+* engine/direct: Fix permissions for resources.models ([#4941](https://github.com/databricks/cli/pull/4941))
 
 ### Dependency updates
 

acceptance/bundle/deploy/mlops-stacks/test.toml

@@ -1,20 +1,12 @@
 Cloud=true
-Local=false
+Local=true
 
 Badness = "the newly initialized bundle from the 'mlops-stacks' template contains two validation warnings in the configuration"
 
 Ignore = [
     "config.json"
 ]
 
-EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform"]
-# On direct, this fails with
-#        +Endpoint: PUT [DATABRICKS_URL]/api/2.0/permissions/registered-models/%5Bdev%20[USERNAME]%5D%20dev-project_name_[UNIQUE_NAME]-model
-#        +HTTP Status: 400 Bad Request
-#        +API error_code: INVALID_PARAMETER_VALUE
-#        +API message: '[dev [USERNAME]] dev-project_name_[UNIQUE_NAME]-model' is not a valid registered model ID.
-
-
 [[Repls]]
 Old = "aws|azure|gcp"
 New = "[CLOUD_ENV_BASE]"

acceptance/bundle/invariant/configs/model_with_permissions.yml.tmpl

@@ -0,0 +1,10 @@
+bundle:
+  name: test-bundle-$UNIQUE_NAME
+
+resources:
+  models:
+    foo:
+      name: test-model-$UNIQUE_NAME
+      permissions:
+        - level: CAN_READ
+          group_name: users

acceptance/bundle/invariant/test.toml

@@ -38,6 +38,7 @@ EnvMatrix.INPUT_CONFIG = [
     "job_with_permissions.yml.tmpl",
     "job_with_task.yml.tmpl",
     "model.yml.tmpl",
+    "model_with_permissions.yml.tmpl",
     "model_serving_endpoint.yml.tmpl",
     "pipeline.yml.tmpl",
     "postgres_branch.yml.tmpl",

acceptance/bundle/refschema/out.fields.txt

@@ -2105,6 +2105,7 @@ resources.models.*.latest_versions[*].user_id	string	REMOTE
 resources.models.*.latest_versions[*].version	string	REMOTE
 resources.models.*.lifecycle	resources.Lifecycle	INPUT
 resources.models.*.lifecycle.prevent_destroy	bool	INPUT
+resources.models.*.model_id	string	REMOTE
 resources.models.*.modified_status	string	INPUT
 resources.models.*.name	string	ALL
 resources.models.*.permission_level	ml.PermissionLevel	REMOTE

acceptance/bundle/resources/permissions/models/current_can_manage/out.plan.direct.json

@@ -14,7 +14,7 @@
       "depends_on": [
         {
           "node": "resources.models.foo",
-          "label": "${resources.models.foo.id}"
+          "label": "${resources.models.foo.model_id}"
         }
       ],
       "action": "create",
@@ -41,7 +41,7 @@
           ]
         },
         "vars": {
-          "object_id": "/registered-models/${resources.models.foo.id}"
+          "object_id": "/registered-models/${resources.models.foo.model_id}"
         }
       }
     }

acceptance/bundle/resources/permissions/models/current_can_manage/out.plan.terraform.json

@@ -0,0 +1,11 @@
+{
+  "cli_version": "[DEV_VERSION]",
+  "plan": {
+    "resources.models.foo": {
+      "action": "create"
+    },
+    "resources.models.foo.permissions": {
+      "action": "create"
+    }
+  }
+}

acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.deploy.direct.json

@@ -1,6 +1,6 @@
 {
   "method": "PUT",
-  "path": "/api/2.0/permissions/registered-models/test-model",
+  "path": "/api/2.0/permissions/registered-models/[FOO_MODEL_ID]",
   "body": {
     "access_control_list": [
       {

acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.deploy.terraform.json

@@ -0,0 +1,24 @@
+{
+  "method": "PUT",
+  "path": "/api/2.0/permissions/registered-models/[FOO_MODEL_ID]",
+  "body": {
+    "access_control_list": [
+      {
+        "permission_level": "CAN_READ",
+        "user_name": "viewer@example.com"
+      },
+      {
+        "permission_level": "CAN_MANAGE",
+        "service_principal_name": "[UUID]"
+      },
+      {
+        "group_name": "data-team",
+        "permission_level": "CAN_MANAGE"
+      },
+      {
+        "permission_level": "CAN_MANAGE",
+        "user_name": "[USERNAME]"
+      }
+    ]
+  }
+}

acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.destroy.terraform.json

@@ -0,0 +1,12 @@
+{
+  "method": "PUT",
+  "path": "/api/2.0/permissions/registered-models/[FOO_MODEL_ID]",
+  "body": {
+    "access_control_list": [
+      {
+        "permission_level": "CAN_MANAGE",
+        "user_name": "[USERNAME]"
+      }
+    ]
+  }
+}

acceptance/bundle/resources/permissions/models/current_can_manage/script

@@ -10,6 +10,12 @@ print_requests() {
 
 rm out.requests.txt
 trace errcode $CLI bundle deploy &> out.deploy.txt
+
+# Register the model's numeric ID for output replacement.
+# The permissions API uses the numeric ID, not the model name.
+MODEL_ID=$($CLI model-registry get-model test-model | jq -r '.registered_model_databricks.id')
+add_repl.py "$MODEL_ID" "FOO_MODEL_ID"
+
 print_requests > out.requests.deploy.$DATABRICKS_BUNDLE_ENGINE.json
 
 trace $CLI bundle destroy --auto-approve

acceptance/bundle/resources/permissions/models/test.toml

@@ -1,2 +1,2 @@
 Env.RESOURCE = "models"  # for ../_script
-EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"]  # terraform mapping issue
+EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"]

acceptance/bundle/resources/permissions/output.txt

@@ -319,8 +319,25 @@ DIFF  jobs/viewers/out.requests.destroy.direct.json
 +    "path": "/api/2.0/permissions/jobs/[NUMID]"
 +  }
 +]
-ERROR models/current_can_manage/out.requests.deploy.direct.json: Missing terraform file models/current_can_manage/out.requests.deploy.terraform.json
-ERROR models/current_can_manage/out.requests.destroy.direct.json: Missing terraform file models/current_can_manage/out.requests.destroy.terraform.json
+MATCH models/current_can_manage/out.requests.deploy.direct.json
+DIFF  models/current_can_manage/out.requests.destroy.direct.json
+--- models/current_can_manage/out.requests.destroy.direct.json
++++ models/current_can_manage/out.requests.destroy.terraform.json
+@@ -1 +1,14 @@
+-[]+[
++  {
++    "body": {
++      "access_control_list": [
++        {
++          "permission_level": "CAN_MANAGE",
++          "user_name": "[USERNAME]"
++        }
++      ]
++    },
++    "method": "PUT",
++    "path": "/api/2.0/permissions/registered-models/[FOO_MODEL_ID]"
++  }
++]
 MATCH pipelines/current_can_manage/out.requests.deploy.direct.json
 EXACT pipelines/current_can_manage/out.requests.destroy.direct.json
 EXACT pipelines/current_is_owner/out.requests.deploy.direct.json

bundle/direct/dresources/model.go

@@ -13,6 +13,15 @@ type ResourceMlflowModel struct {
 	client *databricks.WorkspaceClient
 }
 
+// MlflowModelRemote wraps the API response with the numeric model ID.
+// The state ID for models is the model name (used for CRUD operations), but
+// the permissions API requires the numeric ID. This wrapper exposes the numeric
+// ID as model_id, analogous to RefreshOutput.EndpointId for serving endpoints.
+type MlflowModelRemote struct {
+	ml.ModelDatabricks
+	ModelId string `json:"model_id"`
+}
+
 func (*ResourceMlflowModel) New(client *databricks.WorkspaceClient) *ResourceMlflowModel {
 	return &ResourceMlflowModel{client: client}
 }
@@ -21,53 +30,39 @@ func (*ResourceMlflowModel) PrepareState(input *resources.MlflowModel) *ml.Creat
 	return &input.CreateModelRequest
 }
 
-func (*ResourceMlflowModel) RemapState(model *ml.ModelDatabricks) *ml.CreateModelRequest {
+func (*ResourceMlflowModel) RemapState(output *MlflowModelRemote) *ml.CreateModelRequest {
 	return &ml.CreateModelRequest{
-		Name:            model.Name,
-		Tags:            model.Tags,
-		Description:     model.Description,
-		ForceSendFields: utils.FilterFields[ml.CreateModelRequest](model.ForceSendFields),
+		Name:            output.Name,
+		Tags:            output.Tags,
+		Description:     output.Description,
+		ForceSendFields: utils.FilterFields[ml.CreateModelRequest](output.ForceSendFields),
 	}
 }
 
-func (r *ResourceMlflowModel) DoRead(ctx context.Context, id string) (*ml.ModelDatabricks, error) {
+func (r *ResourceMlflowModel) DoRead(ctx context.Context, id string) (*MlflowModelRemote, error) {
 	response, err := r.client.ModelRegistry.GetModel(ctx, ml.GetModelRequest{
 		Name: id,
 	})
 	if err != nil {
 		return nil, err
 	}
-	return response.RegisteredModelDatabricks, nil
+	return &MlflowModelRemote{
+		ModelDatabricks: *response.RegisteredModelDatabricks,
+		ModelId:         response.RegisteredModelDatabricks.Id,
+	}, nil
 }
 
-func (r *ResourceMlflowModel) DoCreate(ctx context.Context, config *ml.CreateModelRequest) (string, *ml.ModelDatabricks, error) {
+func (r *ResourceMlflowModel) DoCreate(ctx context.Context, config *ml.CreateModelRequest) (string, *MlflowModelRemote, error) {
 	response, err := r.client.ModelRegistry.CreateModel(ctx, *config)
 	if err != nil {
 		return "", nil, err
 	}
-	// Create API call returns [ml.Model] while DoRead returns [ml.ModelDatabricks].
-	// Thus we need to convert the response to the expected type.
-	modelDatabricks := &ml.ModelDatabricks{
-		Name:            response.RegisteredModel.Name,
-		Description:     response.RegisteredModel.Description,
-		Tags:            response.RegisteredModel.Tags,
-		ForceSendFields: utils.FilterFields[ml.ModelDatabricks](response.RegisteredModel.ForceSendFields, "CreationTimestamp", "Id", "LastUpdatedTimestamp", "LatestVersions", "PermissionLevel", "UserId"),
-
-		// Coping the fields only to satisfy the linter. These fields are not
-		// part of the configuration tree so they don't need to be copied.
-		// The linter works as a safeguard to ensure we add new fields to the bundle config tree
-		// to the mapping logic here as well.
-		CreationTimestamp:    0,
-		Id:                   "",
-		LastUpdatedTimestamp: 0,
-		LatestVersions:       nil,
-		PermissionLevel:      "",
-		UserId:               "",
-	}
-	return response.RegisteredModel.Name, modelDatabricks, nil
+	// Return nil for refresh output; the engine will call DoRead to populate the full state
+	// including the numeric model ID needed for permissions.
+	return response.RegisteredModel.Name, nil, nil
 }
 
-func (r *ResourceMlflowModel) DoUpdate(ctx context.Context, id string, config *ml.CreateModelRequest, _ *PlanEntry) (*ml.ModelDatabricks, error) {
+func (r *ResourceMlflowModel) DoUpdate(ctx context.Context, id string, config *ml.CreateModelRequest, entry *PlanEntry) (*MlflowModelRemote, error) {
 	updateRequest := ml.UpdateModelRequest{
 		Name:            id,
 		Description:     config.Description,
@@ -79,26 +74,27 @@ func (r *ResourceMlflowModel) DoUpdate(ctx context.Context, id string, config *m
 		return nil, err
 	}
 
-	// Update API call returns [ml.Model] while DoRead returns [ml.ModelDatabricks].
-	// Thus we need to convert the response to the expected type.
-	modelDatabricks := &ml.ModelDatabricks{
-		Name:            response.RegisteredModel.Name,
-		Description:     response.RegisteredModel.Description,
-		Tags:            response.RegisteredModel.Tags,
-		ForceSendFields: utils.FilterFields[ml.ModelDatabricks](response.RegisteredModel.ForceSendFields, "CreationTimestamp", "Id", "LastUpdatedTimestamp", "LatestVersions", "PermissionLevel", "UserId"),
-
-		// Coping the fields only to satisfy the linter. These fields are not
-		// part of the configuration tree so they don't need to be copied.
-		// The linter works as a safeguard to ensure we add new fields to the bundle config tree
-		// to the mapping logic here as well.
-		CreationTimestamp:    0,
-		Id:                   "",
-		LastUpdatedTimestamp: 0,
-		LatestVersions:       nil,
-		PermissionLevel:      "",
-		UserId:               "",
+	// Carry forward model_id from existing state since UpdateModelResponse doesn't include it.
+	var modelId string
+	if old, ok := entry.RemoteState.(*MlflowModelRemote); ok {
+		modelId = old.ModelId
 	}
-	return modelDatabricks, nil
+
+	return &MlflowModelRemote{
+		ModelDatabricks: ml.ModelDatabricks{
+			CreationTimestamp:    0,
+			Description:          response.RegisteredModel.Description,
+			Id:                   "",
+			LastUpdatedTimestamp: 0,
+			LatestVersions:       nil,
+			Name:                 response.RegisteredModel.Name,
+			PermissionLevel:      "",
+			Tags:                 response.RegisteredModel.Tags,
+			UserId:               "",
+			ForceSendFields:      utils.FilterFields[ml.ModelDatabricks](response.RegisteredModel.ForceSendFields, "CreationTimestamp", "Id", "LastUpdatedTimestamp", "LatestVersions", "PermissionLevel", "UserId"),
+		},
+		ModelId: modelId,
+	}, nil
 }
 
 func (r *ResourceMlflowModel) DoDelete(ctx context.Context, id string) error {

bundle/direct/dresources/permissions.go

@@ -84,6 +84,12 @@ func PreparePermissionsInputConfig(inputConfig any, node string) (*structvar.Str
 		objectIdRef = prefix + "${" + baseNode + ".endpoint_id}"
 	}
 
+	// MLflow models use the model name as the state ID (for CRUD operations),
+	// but the permissions API requires the numeric model ID.
+	if strings.HasPrefix(baseNode, "resources.models.") {
+		objectIdRef = prefix + "${" + baseNode + ".model_id}"
+	}
+
 	// Postgres projects store their hierarchical name ("projects/{project_id}") as the state ID,
 	// but the permissions API expects just the project_id.
 	if strings.HasPrefix(baseNode, "resources.postgres_projects.") {