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

Development proxy ignores path in K8s API URL #4925

Closed
rbq opened this issue May 19, 2021 · 23 comments · Fixed by #5648
Closed

Development proxy ignores path in K8s API URL #4925

rbq opened this issue May 19, 2021 · 23 comments · Fixed by #5648
Assignees
Labels
language/ansible Issue is related to an Ansible operator project priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Milestone

Comments

@rbq
Copy link

rbq commented May 19, 2021

Bug Report

What did you do?

  1. set ~/.kube/config to a Rancher-managed cluster's API
    (sth. like https://my-domain.tld/k8s/clusters/c-some-id)
  2. kubectl apply […] to deploy CRD
  3. operator-sdk run to run operator

What did you expect to see?

I expected both steps 2 and 3 to target the cluster configured in step 1.

What did you see instead? Under which circumstances?

  1. kubectl apply works as expected
  2. operator-sdk run ignores the path component when setting up the proxy and uses https://my-domain.tld instead

In my particular case this didn't actually surface as an error. Instead the modified API endpoint without a path happened to target the management cluster of my Rancher deployment instead of a development cluster. So it actually ran the operator against a different cluster without any warning at all.

Environment

Operator type:

/language ansible

Kubernetes cluster type:

Rancher, RKE

$ operator-sdk version

operator-sdk version: "v1.7.1-19-g3ce1eeef", […], kubernetes version: "v1.19.4", […]

$ kubectl version

Client Version: version.Info{Major:"1", Minor:"21", […]}
Server Version: version.Info{Major:"1", Minor:"20", […]}

Possible Solution

  1. To prevent people from shooting themselves in the foot I'd suggest that operator-sdk run (and any other commands that might behave similarly) fail with an error message if they encounter an API endpoint with a path component.
  2. It would be nice if Operator SDK could be improved to work with clusters that have an API with a path component, such as the ones behind a Rancher instance or other proxies/multiplexers.

Additional context

@openshift-ci openshift-ci bot added the language/ansible Issue is related to an Ansible operator project label May 19, 2021
@rbq rbq changed the title Development proxy ignores K8s API paths Development proxy ignores path in K8s API URL May 24, 2021
@asmacdo asmacdo added this to the v1.9.0 milestone May 24, 2021
@asmacdo asmacdo added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels May 24, 2021
@kensipe kensipe added the release-blocker This issue blocks the parent release milestone label Jun 9, 2021
@laxmikantbpandhare
Copy link
Member

Fix for this issue is dependent on kubectl PR. Once this PR is merged then only we will be able to move ahead with this fix as it has changes in apimachinery.

@laxmikantbpandhare
Copy link
Member

Created another issue #4980 which will fix this issue and throw an error for now. Once kubectl PR gets merged then we will move ahead with changes.

@rashmigottipati rashmigottipati removed this from the v1.9.0 milestone Jun 11, 2021
@rashmigottipati
Copy link
Member

Removing this from v1.9.0 milestone as #4980 (in milestone v1.9.0) is tracking this issue.

@jberkhahn jberkhahn added this to the v1.10.0 milestone Jun 14, 2021
@jberkhahn jberkhahn added blocked and removed release-blocker This issue blocks the parent release milestone labels Jun 14, 2021
@laxmikantbpandhare
Copy link
Member

As part of the Operator SDK v1.9.0 release, we have moved temporary fix for this issue. It will throw an error if API endpoint has Path in it. @rbq Could you please try with new version of Operator SDK. You can download the release and view the changes here: https://github.com/operator-framework/operator-sdk/releases/tag/v1.9.0

@rbq
Copy link
Author

rbq commented Jun 28, 2021

As part of the Operator SDK v1.9.0 release, we have moved temporary fix for this issue. It will throw an error if API endpoint has Path in it. @rbq Could you please try with new version of Operator SDK. You can download the release and view the changes here: https://github.com/operator-framework/operator-sdk/releases/tag/v1.9.0

Checked, throws an error as expected. :)

@laxmikantbpandhare
Copy link
Member

laxmikantbpandhare commented Jun 28, 2021

@rbq - thank you for your confirmation :)

@jberkhahn jberkhahn modified the milestones: v1.10.0, v1.11.0 Jun 30, 2021
@asmacdo asmacdo modified the milestones: v1.11.0, Backlog Aug 9, 2021
@asmacdo asmacdo removed the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Aug 9, 2021
@asmacdo asmacdo removed the blocked label Sep 22, 2021
@asmacdo asmacdo modified the milestones: Backlog, v1.14.0 Sep 22, 2021
@laxmikantbpandhare
Copy link
Member

Till now, we did the temporary fix for this issue as this issue was dependent on kubectl PR. kubectl PR got merged and we are moving ahead with the permanent fix for this issue.

@laxmikantbpandhare
Copy link
Member

@rbq - I wanted to re-create this issue on my local machine. Wanted to know about the steps you did. I am working RKE cluster for the first time.

@rbq
Copy link
Author

rbq commented Oct 6, 2021

@laxmikantbpandhare I don't know if you can actually use your local machine to run both Rancher and a cluster node, but the steps should be:

  1. run Rancher as a single container
  2. provision a cluster via Rancher
  3. download the kubeconfig file

The kubeconfig file will instruct clients to access the provisioned cluster through Rancher using a URL that contains a path.

@laxmikantbpandhare
Copy link
Member

@laxmikantbpandhare I don't know if you can actually use your local machine to run both Rancher and a cluster node, but the steps should be:

  1. run Rancher as a single container
  2. provision a cluster via Rancher
  3. download the kubeconfig file

The kubeconfig file will instruct clients to access the provisioned cluster through Rancher using a URL that contains a path.

Great, I will try this. thank you.

@laxmikantbpandhare
Copy link
Member

laxmikantbpandhare commented Oct 13, 2021

The upstream PR got merged in Mid August and Kubectl had 1.22 release. But, in 1.22 release, Kubectl did not released the change required for this fix. I can see it is now added in 1.23-alpha release and it is likely to get release in 1.23. Changelog for the upcoming release of Kubectl - https://github.com/kubernetes/kubernetes/blob/master/CHANGELOG/CHANGELOG-1.23.md - PR - 97350

@asmacdo asmacdo modified the milestones: v1.14.0, Backlog Oct 20, 2021
@asmacdo asmacdo added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Nov 10, 2021
@jmrodri
Copy link
Member

jmrodri commented Feb 2, 2022

This was waiting on k8s 1.23 release per the above comment. We merged k8s 1.23 in PR #5505

@jmrodri jmrodri closed this as completed Feb 2, 2022
@jmrodri jmrodri modified the milestones: Backlog, v1.17.0 Feb 2, 2022
@rbq
Copy link
Author

rbq commented Apr 9, 2022

I'm still getting the message "[...] contains a path component, which the proxy server is currently unable to handle properly. Work on this issue is being tracked here: [...]" in 1.19.0.

@theishshah theishshah modified the milestones: v1.17.0, v1.20.0 Apr 11, 2022
@theishshah theishshah reopened this Apr 11, 2022
@jmrodri jmrodri removed their assignment Apr 11, 2022
@jmrodri
Copy link
Member

jmrodri commented Apr 11, 2022

/cc @jmrodri

@laxmikantbpandhare
Copy link
Member

thank you for bringing this @rbq - We are working on it and will fix this soon.

@laxmikantbpandhare laxmikantbpandhare linked a pull request Apr 11, 2022 that will close this issue
2 tasks
@laxmikantbpandhare
Copy link
Member

Raised above PR that will remove the content and will not throw an error if the proxy server contains a path in it.

@laxmikantbpandhare
Copy link
Member

@rbq - We removed the change that was throwing an error when there is a path in the proxy server. It will be available in the 1.20 operator-sdk release.

@rbq
Copy link
Author

rbq commented Apr 13, 2022

@laxmikantbpandhare That's great to hear, thanks a lot!

@rbq
Copy link
Author

rbq commented Apr 27, 2022

@laxmikantbpandhare Are releases cut based on schedule (the milestone says due today) or features?

@laxmikantbpandhare
Copy link
Member

@rbq - We planned to do it by EOD tomorrow. I will update you here.

@laxmikantbpandhare
Copy link
Member

laxmikantbpandhare commented Apr 28, 2022

@rbq - We have released Operator SDK v1.20.0. Could you please verify and let us know your observations.

@rbq
Copy link
Author

rbq commented May 3, 2022

@laxmikantbpandhare Yay, it's been merged into Homebrew formulas and does seem to work!

@laxmikantbpandhare
Copy link
Member

@rbq - that's great, thank you for the confirmation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language/ansible Issue is related to an Ansible operator project priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants