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

v3.4.14 Broken redirect to archived workflows #13056

Closed
2 of 4 tasks
apiwoni opened this issue May 16, 2024 · 8 comments
Closed
2 of 4 tasks

v3.4.14 Broken redirect to archived workflows #13056

apiwoni opened this issue May 16, 2024 · 8 comments
Assignees
Labels
area/ui solution/suggested A solution to the bug has been suggested. Someone needs to implement it. type/bug type/regression Regression from previous behavior (a specific type of bug)

Comments

@apiwoni
Copy link

apiwoni commented May 16, 2024

Pre-requisites

  • I have double-checked my configuration
  • I have tested with the :latest image tag (i.e. quay.io/argoproj/workflow-controller:latest) and can confirm the issue still exists on :latest. If not, I have explained why, in detail, in my description below.
  • I have searched existing issues and could not find a match for this bug
  • I'd like to contribute the fix myself (see contributing guide)

What happened/what did you expect to happen?

The following URL https://{{serverUrl}}/workflows/{namespace}/{workflowName} no longer redirects to https://{{serverUrl}}/archived-workflows/{namespace}/{metadata.uid} starting with v3.4.14 when fix(ui): missing uiUrl in ArchivedWorkflowsList #12172 has been committed.

Pasting https://{{serverUrl}}/archived-workflows/{namespace}/{metadata.uid} directly in the browser works as expected.

Argo UI shows the following:

Error: Failed to execute 'pushState' on 'History': A history state object with URL 'https://archived-workflows/{{namepsace}}/87e895f1-50df-443b-971d-218beae9d25f' cannot be created in a document with origin 'https://xxx' and URL 'https://xxx/archived-workflows?namespace={namespace}&name=workflowName&deep=true'.
    at https://xxx/main.fa82dae05c4e68e1ec09.js:446:1047769
    at Object.confirmTransitionTo (https://xxx/main.fa82dae05c4e68e1ec09.js:446:1045618)
    at Object.push (https://xxx/main.fa82dae05c4e68e1ec09.js:446:1047680)
    at t.componentDidUpdate (https://xxx/main.fa82dae05c4e68e1ec09.js:433:843312)
    at is (https://xxx/main.fa82dae05c4e68e1ec09.js:398:83471)
    at hc (https://xxx/main.fa82dae05c4e68e1ec09.js:398:101246)
    at t.unstable_runWithPriority (https://xxx/main.fa82dae05c4e68e1ec09.js:406:3844)
    at Ho (https://xxx/main.fa82dae05c4e68e1ec09.js:398:45024)
    at pc (https://xxx/main.fa82dae05c4e68e1ec09.js:398:97718)
    at Qs (https://xxx/main.fa82dae05c4e68e1ec09.js:398:93872)

componentDidUpdate has added uiUrl in v3.4.14 per https://github.com/argoproj/argo-workflows/pull/12172/files

Version

v3.4.16

Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.

N/A

Logs from the workflow controller

N/A

Logs from in your workflow's wait container

N/A
@agilgur5 agilgur5 added the type/regression Regression from previous behavior (a specific type of bug) label May 16, 2024
@agilgur5 agilgur5 added this to the v3.4.x patches milestone May 16, 2024
@agilgur5 agilgur5 changed the title Broken redirect to archived workflows v3.4.14 Broken redirect to archived workflows Jun 5, 2024
@diegofd
Copy link

diegofd commented Jun 25, 2024

We're still experiencing the same issue on 3.4.17.

Navigating to https://SERVER/workflows/NAMESPACE/WORKFLOW_NAME on an already archived workflow doesn't redirect to it, but fails with above Error: Failed to execute 'pushState' on 'History' error.

Navigating directly to the archived workflow on https://SERVER/archived-workflows/NAMESPACE/UUID works.

@apiwoni
Copy link
Author

apiwoni commented Jun 26, 2024

@diegofd This has not been fixed yet

@diegofd
Copy link

diegofd commented Jun 26, 2024

@apiwoni I know, sorry if my wording told a different thing :)
I just wanted to highlight that we're also hitting this issue.

@ShannonHickey
Copy link

ShannonHickey commented Aug 22, 2024

We're encountering this issue too (or similar):

Navigating to https://SERVER/workflows/NAMESPACE/WORKFLOW_NAME instead lands me on:
https://SERVER/archived-workflows?namespace=NAMESPACE&name=WORKFLOW_NAME&deep=true

This page doesn't work for most of us (but seems to for admins). For me it shows a permission denied error since it's trying to list all workflows for the empty namespace.

Manually changing the link to this works:
https://SERVER/archived-workflows/NAMESPACE?name=WORKFLOW_NAME&deep=true

I wonder if this wasn't caught since it's not affecting admins.

@tooptoop4
Copy link
Contributor

any idea @terrytangyuan ?

@agilgur5
Copy link

agilgur5 commented Sep 30, 2024

Navigating to https://SERVER/workflows/NAMESPACE/WORKFLOW_NAME instead lands me on:
https://SERVER/archived-workflows?namespace=NAMESPACE&name=WORKFLOW_NAME&deep=true

I took another look based on this and I'm thinking that the closing paren on uiUrl in #12172 can be placed closer. Should be a simple fix if so.
But there are many other parts of the codebase that do a full wrap too, including the preceding PR #9790 and below in the same file, so I'm still not exactly sure why this one in particular fails.

EDIT: it's not the closing paren, it's actually the leading slash that's the problem per below comment.

The uiUrl function also just adds a prefix via string concatenation, so strange that it has a diff to the string 🤔

I wonder if this wasn't caught since it's not affecting admins.

Possibly. That and that 3.4 isn't used during dev at all. Tests won't run on PRs to branches either.
Nice analysis!

@agilgur5 agilgur5 self-assigned this Sep 30, 2024
@agilgur5
Copy link

agilgur5 commented Sep 30, 2024

including the preceding PR #9790

Error: Failed to execute 'pushState' on 'History': A history state object with URL 'https://archived-workflows/{{namepsace}}/87e895f1-50df-443b-971d-218beae9d25f' cannot be created in a document with origin 'https://xxx' and URL 'https://xxx/archived-workflows?namespace={namespace}&name=workflowName&deep=true'.

I see the diff now, the first slash needed to be removed when uiUrl was added, my bad

@agilgur5 agilgur5 added the solution/suggested A solution to the bug has been suggested. Someone needs to implement it. label Sep 30, 2024
tooptoop4 added a commit to tooptoop4/argo-workflows that referenced this issue Oct 6, 2024
agilgur5 pushed a commit that referenced this issue Oct 7, 2024
…irect. Fixes #13056 (#13713)

Signed-off-by: tooptoop4 <33283496+tooptoop4@users.noreply.github.com>
@agilgur5
Copy link

agilgur5 commented Oct 7, 2024

Fixed in #13712 and should be released in the next 3.4 patch, 3.4.18

@agilgur5 agilgur5 closed this as completed Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui solution/suggested A solution to the bug has been suggested. Someone needs to implement it. type/bug type/regression Regression from previous behavior (a specific type of bug)
Projects
None yet
Development

No branches or pull requests

6 participants