Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

list related resources #140

Merged
merged 1 commit into from
Jul 18, 2023
Merged

Conversation

mikemrm
Copy link
Contributor

@mikemrm mikemrm commented Jul 14, 2023

Expose a method to list resources related to a given resource.

Fetching /relationships/from/:resource_id will return relationships directly assigned to this resource.

# GET /relationships/from/tnntten-abc12-3
{
  "data": [
    {
      "relation": "parent",
      "subject_id": "tnntten-abc12"
    }
  ]
}

Fetching /relationships/to/:resource_id will return relationships for resources which have a relation to this resource.

# GET /relationships/to/tnntten-abc12-3
{
  "data": [
    {
      "resource_id": "loadbal-abc123",
      "relation": "owner"
    },
    {
      "resource_id": "loadbal-abc456",
      "relation": "owner"
    }
  ]
}

@mikemrm mikemrm force-pushed the list-related-resources branch from ccc0df3 to bb7845e Compare July 14, 2023 16:16
@nicolerenee
Copy link
Member

I think I would like to see a way to get the things in a specific relationship.

So GET /resources/tnntten-abc12-3/relationships would return a list of the relationships available, so owner and whatever else. Then /resources/tnntten-abc12-3/relationships/owners would return the owners. Because when there are multiple types of relationships I'm likely going to want to get the specific one.

I'm not 100% sure when we would want to use this API though, so what are you thinking there?

@mikemrm
Copy link
Contributor Author

mikemrm commented Jul 14, 2023

That makes sense. With the limitations of spicedb's query requiring a Resource Type, we would have to loop through all possible resource types and fetch each individually and then combine the outputs. But this is something we're already doing for role deletions as well.

@mikemrm mikemrm marked this pull request as ready for review July 14, 2023 16:35
@mikemrm mikemrm requested review from a team as code owners July 14, 2023 16:35
@jnschaeffer
Copy link
Contributor

@nicolerenee Is your thinking here that we provide a URL path component/query parameter describing the relation itself? For example, if we have a schema that looks like so:

definition foo/tenant {}

definition foo/instance {
  relation owner: foo/tenant
  relation other_tenant: foo/tenant
}

We would then have a URL like /resources/instanc-abc123/relationships/other_tenant?

@nicolerenee
Copy link
Member

@jnschaeffer yeah, that's exactly what I'm thinking. Usually if I'm trying to get the relationships of something I would imagine I'm trying to get a specific one. But I guess the question is what are we wanting this URL for?

@mikemrm mikemrm force-pushed the list-related-resources branch from bb7845e to 2af13a2 Compare July 18, 2023 16:18
Expose endpoints to list related resources From and To a given resource.

Signed-off-by: Mike Mason <[email protected]>
@mikemrm mikemrm force-pushed the list-related-resources branch from 2af13a2 to 84fab14 Compare July 18, 2023 16:32
Copy link
Contributor

@jnschaeffer jnschaeffer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good at a glance - one question though.

Comment on lines +49 to +50
v1.GET("/resources/:id/relationships", r.relationshipListFrom)
v1.GET("/relationships/from/:id", r.relationshipListFrom)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we keeping the existing API endpoint so as to not break compatibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, no real reason other than that.

@mikemrm mikemrm merged commit 00f5ac1 into infratographer:main Jul 18, 2023
@mikemrm mikemrm deleted the list-related-resources branch July 18, 2023 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants