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

fix(ui): strip inner quotes from argoToken #5677

Merged
merged 1 commit into from
Apr 15, 2021

Conversation

mruoss
Copy link

@mruoss mruoss commented Apr 14, 2021

closes #4991

Checklist:

@codecov
Copy link

codecov bot commented Apr 14, 2021

Codecov Report

Merging #5677 (42bc4a8) into master (71dfc79) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 42bc4a8 differs from pull request most recent head 8dce2ff. Consider uploading reports for the commit 8dce2ff to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5677      +/-   ##
==========================================
- Coverage   47.12%   47.10%   -0.02%     
==========================================
  Files         242      242              
  Lines       15135    15135              
==========================================
- Hits         7133     7130       -3     
- Misses       7095     7098       +3     
  Partials      907      907              
Impacted Files Coverage Δ
cmd/argo/commands/get.go 56.31% <0.00%> (-0.65%) ⬇️
workflow/controller/operator.go 70.84% <0.00%> (-0.27%) ⬇️
cmd/argoexec/commands/emissary.go 50.00% <0.00%> (+1.56%) ⬆️
workflow/metrics/server.go 16.66% <0.00%> (+4.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71dfc79...8dce2ff. Read the comment docs.

@mruoss mruoss changed the title fix: strip inner quotes from argoToken fix(ui): strip inner quotes from argoToken Apr 14, 2021
@alexec
Copy link
Contributor

alexec commented Apr 14, 2021

Could I please ask for a screenshotot?

@mruoss
Copy link
Author

mruoss commented Apr 14, 2021

A screenshot? Of what? The token is redacted in the UI!

@alexec
Copy link
Contributor

alexec commented Apr 14, 2021

Of the page. You may redact any secrets.

@mruoss
Copy link
Author

mruoss commented Apr 15, 2021

What I was trying to tell you is that visually nothing changes with my commit. I testes my code in my browser's console while being on a live instance. I was relying on tests but realized now that there is not really a test suite for the ui. Guess Screenshots are your test suite?

So if you want me to set up the local env and send you a screenshot, it might take a while.

closes argoproj#4991

Signed-off-by: Michael Ruoss <michael.ruoss@ufirstgroup.com>
@mruoss
Copy link
Author

mruoss commented Apr 15, 2021

The UI:
user-info

The textarea that is copied to clipboard (I had logged in with a fake "mytoken")
textarea

@alexec
Copy link
Contributor

alexec commented Apr 15, 2021

Guess Screenshots are your test suite?

Yes. It ensures that that people have run the code. You'd be amazed by the number of people who submit PRs without actually having even run their code.

@alexec alexec merged commit 1d367dd into argoproj:master Apr 15, 2021
@alexec alexec added this to the v3.1 milestone Apr 15, 2021
@mruoss mruoss deleted the fix-argo-token-cli-help branch April 16, 2021 08:53
@simster7 simster7 mentioned this pull request Apr 19, 2021
50 tasks
simster7 pushed a commit that referenced this pull request Apr 19, 2021
Signed-off-by: Michael Ruoss <michael.ruoss@ufirstgroup.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

token not valid for running mode after SSO authentication
2 participants