frontend: Details: Make multi cluster aware

[illume]

This makes Details views work with multiple cluster.

The ?cluster is passed via a URL query parameter. Because in the normal cluster part of the URL is the group of clusters, and the details view needs to know which of the clusters the resource is in.

Here you can see details views being clicked on and the work (even though multiple clusters are active).

multi-cluster-small.mp4

• frontend: DetailsGrid: Fix for using a specific cluster
• frontend: router: Fix createRouteURL to take query params
• frontend: KubeObject: Add cluster query param to getDetailsLink
• frontend: k8s: Add useClusterFromURLVar to get cluster
• frontend: Details: Add support for cluster via URL var

[joaquimrocha]

Please look into this branch, since I think I had also added a way to llink to resources details without having to create a new URL query param:
https://github.com/headlamp-k8s/headlamp/tree/aggregated-view-original-rebased

[joaquimrocha]

I think (haven't double checked) that in https://github.com/headlamp-k8s/headlamp/tree/aggregated-view-original-rebased , I had computed the route directly from the resource in ResourceLink, thus using the known URLs without a cluster param.

[illume]

frontend: Automatically add the cluster param to ResourceLink 0e2f0f3
IIUC, this replaces that 1st PR from the list you shared earlier

That's included already. It adds to the cluster param AFAIK. Which is /c//
Thanks for the tip though. Let me try running that branch and see if I can find it that way.

Yes, and that's okay because when you go into a resource details view, you want to see only one cluster.
When you press back, it goes back to whatever the URL was, including the c/SOMEGROUP

The problem with only relying on this is that:

• the URL has only one cluster in it, which means sharing/using links loses the cluster group
• if a person clicks on another link in the detail view(or in the navigation) then they lose the cluster group

This PR makes these things work.

[joaquimrocha]

I see, and it's a good idea to keep the group context. But for that, I think it makes more sense to do it the other way around: keep the URL for the details view matching the normal details URL (i.e. /c/SINGLE_CLUSTER/resource-kind/my-resource) and use the query param for a group association: /c/SINGLE_CLUSTER/resource-kind/my-resource?cluster-group=MyGroup. This way, we accomplish what you want (keep a reference of the group) we don't have 2 different URLs for the same view, and we can eventually lose the query param and the view still works.
Does this make sense?

[illume]

All other links would then need to special case where /c/<cluster> means something different?

I don't think your proposal makes sense. Because why would we change the URL to '/c/<clusterGroup>' to just a single cluster just for this?

Edit:

With that proposal clicking on a detail view would update all the links in the navigation. With this way none of the other components need to rerender.

Unless I’m misunderstanding something?

This way, we accomplish what you want (keep a reference of the group) we don't have 2 different URLs for the same view, and we can eventually lose the query param and the view still works.
Does this make sense?

But they are two different views. There are other things being displayed than just the resource details. Every thing else related to the clusters is different.

[sniok]

I think we should be very explicit about what this means, because we already have a cluster 'variable' in the url /c/cluster/. To disambiguate I think this should be named something like useClusterFromQueryParam

of course if the approach of storing single cluster in query params stays, I only just saw discussion above

[illume]

What do you mean, never mind?

[illume]

I was thinking “query” is maybe overloaded with react query stuff. But maybe it’s ok.

useClusterFromSearchParam is also not quite right making it sound like it’s from a search.

useClusterFromURLQuery?
useURLQueryCluster?

[sniok]

well there's a separate discussion about the url format, so this comment may not be relevant, let me rephrase it better

[sniok]

useURLQueryCluster sounds good
hm or maybe an approach of naming it for what it is instead of where it's stored, something like useDetailsSingleCluster?

[illume]

Yeah maybe useDetailsSingleCluster is the least misdirecting/confusing.

[illume]

well there's a separate discussion about the url format, so this comment may not be relevant, let me rephrase it better

Can you please let us know which proposal you’d prefer and why?

[sniok]

posted it in a separate comment

[sniok]

URL is supposed to be a unique resource identifier
if we think in terms of purely displaying details of a resource then /c/SINGLE_CLUSTER/resource-kind/my-resource makes sense
but the app also has selected clusters in the sidebar for example, so url has to represent that too

with the previous multi cluster changes we already changed the meaning of the /c/cluster/ param, it now represents all the selected clusters and not the cluster of a given resource.

so now we need to store information about a given resource cluster:

we could repurpose the /c/cluster/ param for it, but now it means different thing depending where we are in the app (selected clusters or cluster of a resource when showing details) and IMHO it's a bit confusing

we could put it in the query params /c/clusetA+clusterB/pods/my-pod?cluster=clusterA but query parameters are not supposed to be used for unique identification of a resource

a logical solution would be to introduce a new param in the url, like
/clusterB+clusterA/details/clusterA/pods/my-pod where /{selectedclusters}/details/{resourceCluster}/{resourceKind}/{resourceName}
(details can be ommited, variable order can be changed, it doesn't matter)

but this kind of solution seems like a big breaking change. so I think doing ?cluster=clusterA is the simplest solution here?

[illume]

a logical solution would be to introduce a new param in the url, like
/clusterB+clusterA/details/clusterA/pods/my-pod where /{selectedclusters}/details/{resourceCluster}/{resourceKind}/{resourceName}
(details can be ommited, variable order can be changed, it doesn't matter)

but this kind of solution seems like a big breaking change. so I think doing ?cluster=clusterA is the simplest solution here?

Adding {resourceCluster} into the URL could work too... I feel like it might even be the more correct choice?

How is it a breaking change though?

Edit:
Maybe adding it after the clusters?: /{selectedclusters}/{resourceCluster}/details/{resourceKind}/{resourceName}.

• ?cluster pros simplest, and don't need to change routes
• /{selectedclusters}/{resourceCluster}/details/{resourceKind}/{resourceName} is maybe more pure and correct. The con is having to change some routes as well... and also change the links in the navigation.

Considering that.. I'm leaning towards ?cluster... but if we can't come to a consensus, and someone asks I can try to implement one of the other ways. eg. /{selectedclusters}/{resourceCluster}/details/{resourceKind}/{resourceName} and see how it turns out?

[sniok]

How is it a breaking change though?

Detail links from current version wouldn't work in new version (if that is something we're concerned about)

[illume]

Detail links from current version wouldn't work in new version (if that is something we're concerned about)

I guess we can keep the previous route? So it will keep working as well.

• new /{selectedclusters}/{resourceCluster}/details/{resourceKind}/{resourceName}
• prev /{selectedclusters}/details/{resourceKind}/{resourceName}

One other con against ?cluster is that if you remove it the URL is broken.

• if you have /{selectedclusters}/details/{resourceKind}/{resourceName}?cluster={resourceCluster}
• then remove ?cluster the URL is broken: /{selectedclusters}/details/{resourceKind}/{resourceName}

So yeah... for these reasons I think it's worth trying out:

• new /{selectedclusters}/{resourceCluster}/details/{resourceKind}/{resourceName}
• prev /{selectedclusters}/details/{resourceKind}/{resourceName}

What do you think? If you both agree I'll try it. cc @joaquimrocha