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

UI: links for label containing dot not working #11741

Closed
2 of 3 tasks
tooptoop4 opened this issue Sep 4, 2023 · 8 comments · Fixed by #13752
Closed
2 of 3 tasks

UI: links for label containing dot not working #11741

tooptoop4 opened this issue Sep 4, 2023 · 8 comments · Fixed by #13752
Labels
area/ui P3 Low priority solution/suggested A solution to the bug has been suggested. Someone needs to implement it. solution/workaround There's a workaround, might not be great, but exists type/bug

Comments

@tooptoop4
Copy link
Contributor

Pre-requisites

  • I have double-checked my configuration
  • I can confirm the issues exists when I tested with :latest
  • I'd like to contribute the fix myself (see contributing guide)

What happened/what you expected to happen?

using links: in wfcontroller configmap:

for a label key not containing slash it works, ie this works:
label=s3-folder%3D${workflow.metadata.labels.s3-folder}

for a label key containing dot slash it returns null in the generated link url:
label=workflows.argoproj.io%2Fworkflow-template%3D${workflow.metadata.labels.workflows.argoproj.io/workflow-template}
below also returns null
label=workflows.argoproj.io%2Fworkflow-template%3D${workflow.metadata.labels.workflows.argoproj.io%2Fworkflow-template}

Version

3.4.8

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

kubectl logs -n argo deploy/workflow-controller | grep ${workflow}

Logs from in your workflow's wait container

kubectl logs -n argo -c wait -l workflows.argoproj.io/workflow=${workflow},workflow.argoproj.io/phase!=Succeeded
@agilgur5
Copy link
Contributor

agilgur5 commented Sep 4, 2023

If this is a UI-only error, this function may need updating to handle this.

Is it slashes specifically that fail, or the fact that the label has dots in it?
This line in the example configmap works, but that's a static link, not a templated link

@tooptoop4
Copy link
Contributor Author

my guess is that it doesn't know how to reference label key with dot because it might be searching for a key of 'workflows'

@tooptoop4 tooptoop4 changed the title links for label containing slash not working links for label containing dot / slash not working Sep 4, 2023
@agilgur5
Copy link
Contributor

agilgur5 commented Sep 4, 2023

Yes, that's what I was referring to.

On more detailed investigation, it's definitely the dot. It's used as a delimiter. The slash shouldn't matter since it uses bracket notation.

This would need a more complicated regex to solve. Either the user input would have to be in bracket notation or could specifically check for labels and annotations and stop delimiting if those are reached. Bracket notation would be more consistent with expr on the backend and JS bracket notation, so I'd probably opt for that. Although stopping delimiting would be simpler 🤔
We'd need a new example for the docs of this as well. And tests for this case would be good to add too.

@tooptoop4
Copy link
Contributor Author

i tried bracket notation but still no luck

@agilgur5
Copy link
Contributor

agilgur5 commented Sep 4, 2023

Correct, it is not currently supported. That was just one option / proposal I wrote if a fix were to be implemented.

@agilgur5 agilgur5 added the P3 Low priority label Sep 7, 2023
@agilgur5 agilgur5 added solution/suggested A solution to the bug has been suggested. Someone needs to implement it. solution/workaround There's a workaround, might not be great, but exists labels Sep 26, 2023
@agilgur5
Copy link
Contributor

Oh right, figured I'd mention that the workaround would be to not use a dot in the label.

@tooptoop4
Copy link
Contributor Author

maybe #13752 will help

@agilgur5 agilgur5 changed the title links for label containing dot / slash not working links for label containing dot not working Oct 14, 2024
@agilgur5 agilgur5 changed the title links for label containing dot not working UI: links for label containing dot not working Oct 14, 2024
@agilgur5 agilgur5 changed the title UI: links for label containing dot not working UI: links for label containing dot not working Oct 14, 2024
agilgur5 pushed a commit that referenced this issue Oct 14, 2024
Signed-off-by: Alan Clucas <alan@clucas.org>
isubasinghe pushed a commit that referenced this issue Oct 30, 2024
Signed-off-by: Alan Clucas <alan@clucas.org>
isubasinghe pushed a commit that referenced this issue Oct 30, 2024
Signed-off-by: Alan Clucas <alan@clucas.org>
@BojanZelic
Copy link

it seems like with the merged PR there would still be an issue getting labels/annotations from podMetadata

ex:

${workflow.spec.podMetadata.annotations.workflows.argoproj.io/something}

wouldn't work because the logic is only limited to workflow.metadata.labels or workflow.metadata.annotations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ui P3 Low priority solution/suggested A solution to the bug has been suggested. Someone needs to implement it. solution/workaround There's a workaround, might not be great, but exists type/bug
Projects
Development

Successfully merging a pull request may close this issue.

3 participants