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

Remove old workaround to append clusterUrl in resolveAuthProxyUrl() #5833

Merged
merged 1 commit into from
Jul 19, 2022

Conversation

jim-docker
Copy link
Contributor

@jim-docker jim-docker commented Jul 13, 2022

A similar code fragment was removed in #5550. It's not clear why this code was needed. @jakolehm can you confirm if this is part of the same workaround that is no longer needed? (as stated here: #5550 (review))

fixes #5663
also may fix #5658 and fix #5628

Before this fix this was the kind of pathname going through the auth proxy:

/a412d1bafa4a0722/k8s/clusters/local/api/v1/namespaces/default/pods/nginx-deployment-9456bbbf9-6fc7k/portforward

The clusterUrl, k8s/clusters/local being extraneous and causing a Bad Gateway response.

needs more testing to confirm it doesn't cause regression tried local, spaces, aws clusters, no issues. Others should test...

Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>
@jim-docker jim-docker added the bug Something isn't working label Jul 13, 2022
@jim-docker jim-docker added this to the 5.6.0 milestone Jul 13, 2022
@jim-docker jim-docker requested a review from a team as a code owner July 13, 2022 22:50
@jim-docker jim-docker requested review from lounjukk and Nokel81 and removed request for a team July 13, 2022 22:50
Copy link
Collaborator

@Nokel81 Nokel81 left a comment

Choose a reason for hiding this comment

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

I can confirm that this fixes the terminal issues.

@Nokel81 Nokel81 changed the title removed old workaround to append clusterUrl in resolveAuthProxyUrl() Remove old workaround to append clusterUrl in resolveAuthProxyUrl() Jul 18, 2022
@jansav
Copy link
Contributor

jansav commented Jul 19, 2022

This just highlights the fact that we need to add behaviours here soon. It feels like we don't know how it should work at the moment, which isn't very good thing. :)

@jansav jansav merged commit 0d2aea0 into master Jul 19, 2022
@jansav jansav deleted the fix/cant-port-forward-on-rancher branch July 19, 2022 05:48
@skri547
Copy link

skri547 commented Jul 20, 2022

@jansav should we update the lens for resolving this issue or is that expected to work in current release? We are facing the terminal issues.

@skri547
Copy link

skri547 commented Jul 20, 2022

@Nokel81 @jim-docker @jansav can you please let me know if this fixes #5829 , If yes, can we try on the same version or is that expected to upgrade lens?

@Nokel81
Copy link
Collaborator

Nokel81 commented Jul 20, 2022

@skri547 We think that this should fix that issue. You would need to upgrade Lens, but this fix has currently not been released even in an alpha.

@skri547
Copy link

skri547 commented Jul 20, 2022

@Nokel81 by when or which release it will be available for everyone? Any ETA?

@Nokel81
Copy link
Collaborator

Nokel81 commented Jul 20, 2022

Another alpha should be produced this week. With general availability in the new few weeks.

@skri547
Copy link

skri547 commented Jul 20, 2022

How can i download or upgrade the lens to Alpha to try this in Windows desktop?

@Nokel81
Copy link
Collaborator

Nokel81 commented Jul 20, 2022

When it comes out, you can change the release channel in your preferences and then "check for updates" via the tray menu.

@Nokel81 Nokel81 mentioned this pull request Jul 22, 2022
@jansav jansav mentioned this pull request Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants