Skip to content

clean up remaining "fetch" interfaces in datastore #845

Closed

Description

#798 and follow-on work converted the vast majority of Nexus to use the new internal lookup API, but there are a handful of stragglers in the DataStore that will be more work to convert.

The two big blockers are:

  • Tables with primary keys other than "id" and ([parent_id], name). The macro that implements new internal lookup API #798 only works for resources backed by tables with an "id" and "name" column per RFD 192. Some resources are missing a "name" (we call these "assets" in some places), some are missing an "id", and some have composite primary keys. More below. I'd propose that we make the macro more configurable to support these cases.
  • A couple of the remaining resource types are (or should be) implemented with authz::FleetChild, which is constructed differently than both the "generic" and "typed" authz resources that are supported by the macro. It'd be pretty easy to extend the macro to support this, but I think that's wasted effort. I think we should do generic authz types could be more type-safe #848 instead.

A fair question is whether it's worth fixing these just to get these stragglers into the main lookup API. I think it is worth that effort because the main problem is just the different primary keys, and I think that's a really useful pattern that I want to make sure remains first-class. We don't want to start adding columns that don't make sense or indexes that don't make sense just to make things fit a pattern that doesn't make sense for them.

More details on the remaining "fetch" stuff in DataStore:

  • sled_fetch: these have no "name" column.
  • user_builtin_fetch: for authz, these use the generic authz::FleetChild. Like ProjectChild, these are constructed using parent.child_generic(), but they take a different number of arguments. As I mentioned above, we could generalize the macro's AuthzKind argument to support this but it feels like going further down the wrong path.
  • silo_fetch: generic FleetChild problem.
  • role_builtin_fetch: these have no id column and their primary key is the composite (resource_type, role_name). They will also have the generic authz::FleetChild problem.
  • update_available_artifact_fetch: these have no id column and their primary key is the composite (name, kind, version). Their authz check today is based on being able to read the Fleet. It'd be easy to add a new authz::FleetChild for them... at which point they'll have the same problem as the above.
  • silo_user_fetch: silo_user has no name column.
  • silo_user_fetch and session_fetch: see authentication operations should use special Nexus context #846.

This is essentially blocked on #847 and #848.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions