Add PinnedResources to ListUnifiedResources Request#32077
Conversation
| ids := make([]string, 0) | ||
| clusterName, err := a.authServer.GetClusterName() | ||
| if err != nil { | ||
| return nil, trace.Wrap(err, "getting cluster name") | ||
| } | ||
|
|
||
| prefs, err := a.authServer.GetUserPreferences(ctx, a.context.User.GetName()) | ||
| if err != nil { | ||
| return nil, trace.Wrap(err, "getting user preferences") | ||
| } | ||
| clusters := prefs.PinnedResources.GetPinnedResources() | ||
| clusterIds, ok := clusters[clusterName.GetClusterName()] | ||
| if ok { | ||
| ids = clusterIds.ResourceIds | ||
| } |
There was a problem hiding this comment.
this whole song and dance of getting userprefs and clustername and all that seems really messy but i couldn't find a simpler way to do it. Open to suggestions
|
This is sort of temporary because we will be updating how we get these IDs when we move towards the indexes/map of resources in the upcoming performance updates. But for now, this is my idea for the current implementation. |
|
Looking into this crazy amount of unit test errors. |
| // PinnedResources indicates that the request will pull only the pinned resources | ||
| // of the requesting user | ||
| bool PinnedResources = 11 [(gogoproto.jsontag) = "pinned_resources,omitempty"]; |
There was a problem hiding this comment.
Weird that the linter says this field isn't present
There was a problem hiding this comment.
How about PinnedResourcesOnly for a name?
There was a problem hiding this comment.
Thank you! I was trying to think of a name like "include pinned resources" or "use pinned resources" but every name sounded inclusive instead of exclusive so I gave up. I like Only. Will update.
| var name, kind string | ||
| // set the kind to the appropriate "contained" type, rather than | ||
| // the container type. This better represents what resource is | ||
| // being sent to the ui | ||
| switch r := resource.(type) { | ||
| case types.AppServer: | ||
| app := r.GetApp() | ||
| if app != nil { | ||
| name = app.GetName() | ||
| kind = types.KindApp | ||
| } | ||
| case types.SAMLIdPServiceProvider: | ||
| name = r.GetName() | ||
| kind = types.KindApp | ||
| case types.KubeServer: | ||
| cluster := r.GetCluster() | ||
| if cluster != nil { | ||
| name = r.GetCluster().GetName() | ||
| kind = types.KindKubernetesCluster | ||
| } | ||
| case types.DatabaseServer: | ||
| db := r.GetDatabase() | ||
| if db != nil { | ||
| name = db.GetName() | ||
| kind = types.KindDatabase | ||
| } | ||
| default: | ||
| name = resource.GetName() | ||
| kind = resource.GetKind() | ||
| } | ||
| return backend.Key(prefix, name, kind) |
There was a problem hiding this comment.
This is currently broke and I'm trying to fix it. When a resource is added, it's "contained resource" exists, but if something gets updated (or deleted), like AppServer, then the event is sent without its contained resource which means I can't generate the resource key again. The frontend only receives the contained resource and uses it as the ID for pinning.
So, backend updates don't know about the 'contained' resource, and the frontend doesn't know about the 'container' resource. I have to figure out a way to connect them again.
You can ignore this PR until I fix it. Thanks! (Or check out other parts)
There was a problem hiding this comment.
Once #32709 is in, I can refactor to use the resourceSortKey.ByName to fetch the correct resource by the pinned resource ID.
There was a problem hiding this comment.
Refactored using resourceSortKey and added the matcher.
|
This is ready to be reviewed again |
| if trace.IsAccessDenied(err) { | ||
| return false | ||
| } | ||
| return false |
There was a problem hiding this comment.
is the if trace.isAccessDenied necessary? we return false regardless
public-teleport-github-review-bot
Bot
removed request for
lxea and
probakowski
avatus
force-pushed
the
avatus/pinned_user_prefs
branch
from
d2d0b55 to
dceb604
Compare
avatus
force-pushed
the
avatus/get_unified_resources_by_id
branch
from
887e3ab to
3ee5435
Compare
avatus
force-pushed
the
avatus/get_unified_resources_by_id
branch
from
d43d08c to
06af5ca
Compare
4 participants
Relies on #32009
Part of #32075
This is the backend portion to fetch unified resources per the user's pinned resources.
If the
pinnedResourcesparam is present in the request, we will populate the unified resources only from the IDs present in their pinned resources preferences rather than all the resources. Afterwards, all the same filters apply such as rbac, search/query, etc.