Should policy response body have resource ID and type on it?

[david-crespo]

I'm starting to work with the GET and PUT org and project policy endpoints in the console and found it surprising that the policy response body has no reference to the resource the policy is for. I assume this was to keep the Policy type simple by not having it know about the resource:

omicron/nexus/src/external_api/shared.rs

Lines 32 to 44 in 4c933a1

	/// Client view of a [`Policy`], which describes how this resource may be
	/// accessed
	///
	/// Note that the Policy only describes access granted explicitly for this
	/// resource. The policies of parent resources can also cause a user to have
	/// access to this resource.
	#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)]
	#[schemars(rename = "{AllowedRoles}Policy")]
	pub struct Policy<AllowedRoles: serde::de::DeserializeOwned> {
	/// Roles directly assigned on this resource
	#[serde(deserialize_with = "role_assignments_deserialize")]
	pub role_assignments: Vec<RoleAssignment<AllowedRoles>>,
	}

The simplest thing we could do is add a resource_id field, though it would be a bit weird if it didn't also indicate the resource type. So that could either be resource_id: "abc" and resource_type: "project" or we could combine that into one thing: project_id. On one hand project_id is neater. On the other hand, if a client has a policy and wants to know what kind of resource it is for, policy.resource_type === "project" makes a lot more sense than "is project_id present?" or some horrible /^(.+)_id$/ situation. On the other other hand, I don't know why a client would have a policy on hand but not know what kind of resource it came from. You just made the request, buddy.

So I guess the change I'm suggesting is this (sorry for the TS syntax, it's the most concise way to put it):

 type IdentityType = 'silo_user'

 type ProjectRoles = 'admin' | 'collaborator' | 'viewer'

 type ProjectRolesRoleAssignment = {
   identity_id: string
   identity_type: IdentityType
   role_name: ProjectRoles
 }
 
 type ProjectRolesPolicy = {
+  project_id: string
   role_assignments: ProjectRolesRoleAssignment[]
 }

 type OrganizationRolesPolicy = {
+  organization_id: string
   role_assignments: OrganizationRolesRoleAssignment[]
 }

// etc

cc @davepacheco @plotnick

[davepacheco]

I think I left it out because it seemed weird to include the type and id when you just made the request on a specific resource. I don't object to leaving it in though. I like resource_id and resource_type (and it seems a lot easier to implement).

[david-crespo]

It's true, we don't have resource_type: 'project' on the response when you fetch a project.

I would say this is pretty low priority for now. Originally what prompted this is that I wanted to store policy response JSON in my mock API DB basically as-is, which requires resource type and ID because you need to distinguish between org policies and project policies. But then I realized if I have to mess with the object anyway, I might as well imitate the API's actual data model, i.e., each row is a (resource type, resource id, actor type, actor id) tuple and then the endpoint aggregates those into a policy object (source here). So, bottom line is I don't need the above. In fact, I think I'll close the issue because I don't see a practical problem this solves.