Improve similarity with kubectl in handling of oidc kubeconfigs #144

mogaika · 2019-07-10T11:40:30Z

allow 'client-secret' to be empty
fix 'verify' parameter of refresh_token call

Fixes: #142

k8s-ci-robot · 2019-07-10T11:40:31Z

Welcome @mogaika!

It looks like this is your first PR to kubernetes-client/python-base 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-client/python-base has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

codecov-io · 2019-07-10T11:47:27Z

Codecov Report

Merging #144 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #144      +/-   ##
==========================================
+ Coverage   93.41%   93.42%   +<.01%     
==========================================
  Files          13       13              
  Lines        1398     1399       +1     
==========================================
+ Hits         1306     1307       +1     
  Misses         92       92

Impacted Files	Coverage Δ
config/kube_config.py	`87.56% <100%> (+0.03%)`	⬆️

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 6b65461...81e84ef. Read the comment docs.

roycaihw · 2019-07-16T17:35:10Z

/cc

yliaog · 2019-07-16T17:43:10Z

i think you mean "Improve similarity with kubectl" instead of "Improve similarity with kubelet"

roycaihw · 2019-07-16T23:05:53Z

config/kube_config.py

-                      provider['config']['client-secret']),
-                verify=config.ssl_ca_cert if config.verify_ssl else None
+                      client_secret),
+                verify=config.ssl_ca_cert if config.verify_ssl else False


As you pointed out in the issue, verify can be either a boolean or a string. When verify is None, the requests Session tries to look for requests environment configuration. I wonder what's the implication of changing verify to False here? More importantly, what's the expected behavior when idp-certificate-authority-data is empty in kubeconfig (and verify_ssl being False as a result)

@yliaog Do you have more context on this as you looked at #69?

i don't have more context. please add a unit test that fails before the PR, and works after the PR.

My logic was based on the fact that kubectl ignores cert verification problems (like I had in #142). And since request method description notes that 'verify' parameter can be either bool or string, I thought that original code planned to use False instead of None, because its strange to explicitly point that 'verify' parameter must be None since it's None by default. I remove the parameter change from this commit since it's another unclear subject to discuss (if kubectl ignores verification problems is problem of kubectl or no, and does python client mimic this strange behaviour)

scottilee · 2019-09-26T04:38:42Z

@mogaika could you provide an update to the comment above?

roycaihw · 2019-09-27T22:10:25Z

config/kube_config.py

@@ -361,13 +361,14 @@ def _refresh_oidc(self, provider):
            return

        response = json.loads(response.data)
+        client_secret = provider['config'].safe_get('client-secret') or ''


Should client_secret be empty string or None? Could you show what kubectl sends?

I've stumbled on the same issue, and while poking around, None is what works, so just delete the 'or' clause

- allow 'client-secret' to be empty

mogaika · 2019-10-18T14:42:47Z

I currently have no option to test real traffic but looking on kubectl client go code I confirm that @roycaihw and @pshchelo right

fejta-bot · 2020-01-16T15:23:39Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

k8s-ci-robot · 2020-01-16T15:23:43Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

fejta-bot · 2020-02-15T16:04:34Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

pshchelo · 2020-02-18T17:34:54Z

/remove-lifecycle rotten

roycaihw · 2020-02-18T23:39:44Z

/lgtm
/approve

allowing client-secret to be empty looks good. Please fix the commit missing GitHub user issue

k8s-ci-robot · 2020-02-18T23:39:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mogaika, roycaihw

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [roycaihw]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

fejta-bot · 2020-05-19T00:32:23Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot · 2020-06-18T01:15:11Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

fejta-bot · 2020-07-18T01:56:15Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

k8s-ci-robot · 2020-07-18T01:56:18Z

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

z0r0 · 2020-08-06T21:56:11Z

Can we re-open this one? I'm facing the same issue here.

yliaog · 2020-08-06T22:05:15Z

/reopen

k8s-ci-robot · 2020-08-06T22:05:17Z

@yliaog: Reopened this PR.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

fejta-bot · 2020-09-05T22:21:38Z

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

k8s-ci-robot · 2020-09-05T22:21:40Z

@fejta-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot requested review from mbohlool and yliaog July 10, 2019 11:40

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 10, 2019

mogaika changed the title ~~Improve similarity with kubelet in handling of oidc kubeconfigs (#142)~~ Improve similarity with kubelet in handling of oidc kubeconfigs Jul 10, 2019

k8s-ci-robot requested a review from roycaihw July 16, 2019 17:35

roycaihw reviewed Jul 16, 2019

View reviewed changes

roycaihw changed the title ~~Improve similarity with kubelet in handling of oidc kubeconfigs~~ Improve similarity with kubectl in handling of oidc kubeconfigs Jul 16, 2019

mogaika force-pushed the issue_142 branch 2 times, most recently from 4b9da57 to 8f41346 Compare September 26, 2019 20:45

roycaihw reviewed Sep 27, 2019

View reviewed changes

Improve similarity with kubelet in handling of oidc kubeconfigs

81e84ef

- allow 'client-secret' to be empty

mogaika force-pushed the issue_142 branch from 8f41346 to 81e84ef Compare October 18, 2019 14:40

k8s-ci-robot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 16, 2020

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jan 16, 2020

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Feb 15, 2020

k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Feb 18, 2020

k8s-ci-robot assigned roycaihw Feb 18, 2020

k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 18, 2020

roycaihw mentioned this pull request Mar 2, 2020

python client doesn't work with oidc with public endpoint kubernetes-client/python#1055

Closed

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 19, 2020

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 18, 2020

k8s-ci-robot closed this Jul 18, 2020

k8s-ci-robot reopened this Aug 6, 2020

k8s-ci-robot closed this Sep 5, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve similarity with kubectl in handling of oidc kubeconfigs #144

Improve similarity with kubectl in handling of oidc kubeconfigs #144

mogaika commented Jul 10, 2019 •

edited

Loading

k8s-ci-robot commented Jul 10, 2019

codecov-io commented Jul 10, 2019 •

edited

Loading

roycaihw commented Jul 16, 2019

yliaog commented Jul 16, 2019

roycaihw Jul 16, 2019 •

edited

Loading

roycaihw Jul 16, 2019

yliaog Jul 17, 2019

mogaika Sep 26, 2019 •

edited

Loading

scottilee commented Sep 26, 2019

roycaihw Sep 27, 2019

pshchelo Oct 9, 2019 •

edited

Loading

mogaika commented Oct 18, 2019

fejta-bot commented Jan 16, 2020

k8s-ci-robot commented Jan 16, 2020

fejta-bot commented Feb 15, 2020

pshchelo commented Feb 18, 2020

roycaihw commented Feb 18, 2020

k8s-ci-robot commented Feb 18, 2020

fejta-bot commented May 19, 2020

fejta-bot commented Jun 18, 2020

fejta-bot commented Jul 18, 2020

k8s-ci-robot commented Jul 18, 2020

z0r0 commented Aug 6, 2020

yliaog commented Aug 6, 2020

k8s-ci-robot commented Aug 6, 2020

fejta-bot commented Sep 5, 2020

k8s-ci-robot commented Sep 5, 2020

Improve similarity with kubectl in handling of oidc kubeconfigs #144

Improve similarity with kubectl in handling of oidc kubeconfigs #144

Conversation

mogaika commented Jul 10, 2019 • edited Loading

k8s-ci-robot commented Jul 10, 2019

codecov-io commented Jul 10, 2019 • edited Loading

Codecov Report

roycaihw commented Jul 16, 2019

yliaog commented Jul 16, 2019

roycaihw Jul 16, 2019 • edited Loading

Choose a reason for hiding this comment

roycaihw Jul 16, 2019

Choose a reason for hiding this comment

yliaog Jul 17, 2019

Choose a reason for hiding this comment

mogaika Sep 26, 2019 • edited Loading

Choose a reason for hiding this comment

scottilee commented Sep 26, 2019

roycaihw Sep 27, 2019

Choose a reason for hiding this comment

pshchelo Oct 9, 2019 • edited Loading

Choose a reason for hiding this comment

mogaika commented Oct 18, 2019

fejta-bot commented Jan 16, 2020

k8s-ci-robot commented Jan 16, 2020

fejta-bot commented Feb 15, 2020

pshchelo commented Feb 18, 2020

roycaihw commented Feb 18, 2020

k8s-ci-robot commented Feb 18, 2020

fejta-bot commented May 19, 2020

fejta-bot commented Jun 18, 2020

fejta-bot commented Jul 18, 2020

k8s-ci-robot commented Jul 18, 2020

z0r0 commented Aug 6, 2020

yliaog commented Aug 6, 2020

k8s-ci-robot commented Aug 6, 2020

fejta-bot commented Sep 5, 2020

k8s-ci-robot commented Sep 5, 2020

mogaika commented Jul 10, 2019 •

edited

Loading

codecov-io commented Jul 10, 2019 •

edited

Loading

roycaihw Jul 16, 2019 •

edited

Loading

mogaika Sep 26, 2019 •

edited

Loading

pshchelo Oct 9, 2019 •

edited

Loading