search

argoproj / argo-workflows

Workflow Engine for Kubernetes
https://argo-workflows.readthedocs.io/
Apache License 2.0
15k stars 3.2k forks source link

V3: In namespaced mode, Argo UI's /workflows tries to serve all workflows across the cluster #5577

Closed Radolumbo closed 3 years ago

Radolumbo commented 3 years ago

Summary

What happened/what you expected to happen? In Argo v2, in namespaced server mode, going to /workflows in the Argo UI served only the workflows for the corresponding namespace of the server you're connected to. In v3, it appears to serve the workflows across the cluster, even though you're connecting to the server of a specific namespace. This forces you to go to /workflows/namespace to view your workflows if you are not permissioned to view workflows in the cluster. Before, it wasn't even possible to view cluster-wide workflows in namespaced mode, which I think is the more expected behavior.

Is this behavior intentional? Did something change about how routes are served?

Diagnostics

What version of Argo Workflows are you running? v3.0.0

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

alexec commented 3 years ago

Can I please request...

Storage {managedNamespace: "argo", loglevel:webpack-dev-server: "INFO", ListOptions/options: "{"selectedPhases":[],"selectedLabels":[],"paginationLimit":500}", current_namespace: "", length: 4}
Radolumbo commented 3 years ago

E.G.

Screenshot 2021-04-02 125956 
Storage {managedNamespace: "s-workflow-platform", ListOptions/options: "{"selectedPhases":[],"selectedLabels":[],"paginationLimit":500}", current_namespace: "", length: 3}
ListOptions/options: "{"selectedPhases":[],"selectedLabels":[],"paginationLimit":500}"
current_namespace: ""
length: 3
managedNamespace: "s-workflow-platform"
__proto__: Storage
Radolumbo commented 3 years ago

When I browse to the base URL (aka just /), it auto redirects me to /workflows/s-workflow-platform

But I can manually delete the namespace name in the URL (thus browsing to /workflows), whereupon it will serve (or attempt to, if I'm not permissioned) all workflows in the cluster. My expectation would be that there is no way to view cluster-wide workflows from a namespaced server, which is how it behaved in V2.

hadim commented 3 years ago

I see the same error.

alexec commented 3 years ago

This needs to be fixed here:

https://github.com/argoproj/argo-workflows/blob/master/ui/src/app/workflows/components/workflows-list/workflows-list.tsx#L198

Basically we need to check to see if managed namespace is set.

Would anyone like to submit a PR?

Radolumbo commented 3 years ago

Are you suggesting that we just filter down to the managed namespace on all fetchWorkflow calls?

To go in hand with this, we should probably also change the redirect from base URL to just go to /workflows/ when there is a managed namespace, too, right? That would https://github.com/argoproj/argo-workflows/blob/master/ui/src/app/app-router.tsx#L167 here, I guess?

alexec commented 3 years ago

Either that - or there's a bug in app-router?

Radolumbo commented 3 years ago

I just edited my like 30 before your post so not sure if you saw it -- but I linked where I think this might be happening https://github.com/argoproj/argo-workflows/blob/master/ui/src/app/app-router.tsx#L167 .. if any namespace is set, which will always be true I think since we have a managed namespace in this case .. it directs there.

That said, after playing with the code a bit, I think there's more to this. This is my first time really digging into this code, so might take me a bit to fully understand, but just changing the app-router redirect definitely has some other behavior (trying to redirect just to / takes me to the login page for some reason). And just changing the filter that you originally linked to will still give an attempt to access the cluster (so it will fail on permissions if you don't have them).

Anyway, I'm still just messing with it and learning the codebase, I'll try to come up with something, but I would not discourage others from attempting a PR if they want.

alexec commented 3 years ago

I wonder if the order of those elements is important and the redirect should be top of the list?

alexec commented 3 years ago

Bump! @Radolumbo are you still investigating?

Radolumbo commented 3 years ago

I definitely still intend on coming back to this! I've been on vacation for the last week so haven't had a chance. If someone else feels like they want to make the fix they can, else I'll be working on it next week!

alexec commented 3 years ago

Related:

fix(ui): Fix editor. Fixes #5613 Fixes #5617 (#5620)

alexec commented 3 years ago

I think this may have been fixed on master.

Radolumbo commented 3 years ago

I'll test it out in a bit, and see!

Radolumbo commented 3 years ago

I'm still seeing the same behavior.

alexec commented 3 years ago

I could not reproduce this. Can anyone volunteer to debug via Zoom?

Radolumbo commented 3 years ago

I could do so. I just got back to looking at this, and I think I understand what's happening, though I haven't decided exactly the best way to approach it, yet.

Radolumbo commented 3 years ago

I should at least have a draft PR soon for one way we can approach this. I think it's a little hacky, but it's similar to what was being done in V2, and should at least start a discussion.

Radolumbo commented 3 years ago

Please see https://github.com/argoproj/argo-workflows/pull/5729