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.

Sign up for GitHub

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

Jump to bottom

chore: Refactor terminal handler to use auth-middleware #12052

Merged
merged 11 commits into from
Jan 24, 2023
Merged

chore: Refactor terminal handler to use auth-middleware #12052

merged 11 commits into from
Jan 24, 2023

Conversation

leoluz
Copy link
Collaborator

@leoluz leoluz commented Jan 19, 2023 •
edited
Loading

Signed-off-by: Leonardo Luz Almeida leonardo_almeida@intuit.com

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).

@leoluz
chore: Refactor terminal handler to use auth-middleware
d5e3322 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
@leoluz leoluz requested a review from crenshaw-dev January 19, 2023 19:55
@leoluz
remove context key for now
cab3023 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
@codecov
Copy link

codecov bot commented Jan 19, 2023 •
edited
Loading

Codecov Report

Attention: Patch coverage is 58.53659% with 17 lines in your changes missing coverage. Please review.

Project coverage is 47.44%. Comparing base (584428e) to head (c5c337b).
Report is 2294 commits behind head on master.

Files with missing lines Patch % Lines
server/application/terminal.go 0.00% 13 Missing ⚠️
util/session/sessionmanager.go 83.33% 4 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #12052      +/-   ##
==========================================
+ Coverage   47.38%   47.44%   +0.05%     
==========================================
  Files         245      245              
  Lines       41670    41690      +20     
==========================================
+ Hits        19745    19778      +33     
+ Misses      19937    19925      -12     
+ Partials     1988     1987       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

leoluz added 2 commits January 23, 2023 10:22
@leoluz
implement unit-tests
a805f7c 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
@leoluz
remove claim valid check for now
37fc54b 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
leoluz added 2 commits January 23, 2023 11:36
@leoluz
remove unnecessary test
6b5a2de 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
@leoluz
fix lint
3036514 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
leoluz added 3 commits January 23, 2023 14:58
@leoluz
don't too much details in http response
27016dc 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
@leoluz
Fix error
b803679 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
@leoluz
Fix lint
1fad505 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
@leoluz leoluz requested a review from jannfis January 23, 2023 20:06
@leoluz
trigger build
b392651 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
jannfis
jannfis reviewed Jan 23, 2023
View reviewed changes
server/server.go Outdated Show resolved Hide resolved
@leoluz
builder pattern in terminal feature-flag middleware
c5c337b 
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
crenshaw-dev
crenshaw-dev approved these changes Jan 24, 2023
View reviewed changes
Copy link
Member

@crenshaw-dev crenshaw-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and local test checks out. Thanks!

@crenshaw-dev crenshaw-dev merged commit 5a6f969 into master Jan 24, 2023
@crenshaw-dev crenshaw-dev deleted the term-auth branch January 24, 2023 15:46
@crenshaw-dev crenshaw-dev added cherry-pick/2.3 Candidate for cherry picking into the 2.3 release branch cherry-pick/2.4 Candidate for cherry picking into the 2.4 release branch cherry-pick/2.5 cherry-pick/2.6 labels Jan 24, 2023
@crenshaw-dev
Copy link
Member

As discussed in #argo-maintainers, we'll cherry-pick this back to help keep consistency in the auth code.

This is a special case for a relatively small change. We won't be making a habit of it. :-)

crenshaw-dev pushed a commit that referenced this pull request Jan 27, 2023
@leoluz @crenshaw-dev
chore: Refactor terminal handler to use auth-middleware (#12052)
671f3f4 
* chore: Refactor terminal handler to use auth-middleware

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove context key for now

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* implement unit-tests

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove claim valid check for now

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove unnecessary test

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* fix lint

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* don't too much details in http response

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* Fix error

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* Fix lint

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* trigger build

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* builder pattern in terminal feature-flag middleware

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
crenshaw-dev pushed a commit that referenced this pull request Jan 27, 2023
@leoluz @crenshaw-dev
chore: Refactor terminal handler to use auth-middleware (#12052)
35546fd 
* chore: Refactor terminal handler to use auth-middleware

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove context key for now

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* implement unit-tests

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove claim valid check for now

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove unnecessary test

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* fix lint

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* don't too much details in http response

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* Fix error

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* Fix lint

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* trigger build

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* builder pattern in terminal feature-flag middleware

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
@crenshaw-dev crenshaw-dev removed the cherry-pick/2.3 Candidate for cherry picking into the 2.3 release branch label Jan 27, 2023
@crenshaw-dev
Copy link
Member

No cherry-pick to 2.3, because web terminal didn't exist in 2.3. :-)

crenshaw-dev pushed a commit that referenced this pull request Jan 27, 2023
@leoluz @crenshaw-dev
chore: Refactor terminal handler to use auth-middleware (#12052)
8f4678e 
* chore: Refactor terminal handler to use auth-middleware

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove context key for now

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* implement unit-tests

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove claim valid check for now

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove unnecessary test

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* fix lint

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* don't too much details in http response

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* Fix error

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* Fix lint

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* trigger build

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* builder pattern in terminal feature-flag middleware

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
@crenshaw-dev
Copy link
Member

Cherry-picked onto release-2.6 for 2.6.0-rc6, release-2.5 for 2.5.9, and release-2.4 for 2.4.21.

emirot pushed a commit to emirot/argo-cd that referenced this pull request Jan 27, 2023
@leoluz @emirot
chore: Refactor terminal handler to use auth-middleware (argoproj#12052)
12b1fb1 
* chore: Refactor terminal handler to use auth-middleware

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove context key for now

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* implement unit-tests

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove claim valid check for now

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove unnecessary test

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* fix lint

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* don't too much details in http response

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* Fix error

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* Fix lint

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* trigger build

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* builder pattern in terminal feature-flag middleware

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: emirot <emirot.nolan@gmail.com>
schakrad pushed a commit to schakrad/argo-cd that referenced this pull request Mar 14, 2023
@leoluz @schakrad
chore: Refactor terminal handler to use auth-middleware (argoproj#12052)
166c0fc 
* chore: Refactor terminal handler to use auth-middleware

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove context key for now

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* implement unit-tests

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove claim valid check for now

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* remove unnecessary test

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* fix lint

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* don't too much details in http response

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* Fix error

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* Fix lint

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* trigger build

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

* builder pattern in terminal feature-flag middleware

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: schakrad <chakradari.sindhu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crenshaw-dev crenshaw-dev crenshaw-dev approved these changes

@jannfis jannfis Awaiting requested review from jannfis

Assignees
No one assigned
Labels
cherry-pick/2.4 Candidate for cherry picking into the 2.4 release branch cherry-pick/2.5 cherry-pick/2.6
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@leoluz @crenshaw-dev @jannfis