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

Client doesn't honor tls-server-name setting in kubeconfig #267

Closed
multani opened this issue Aug 16, 2023 · 4 comments · Fixed by #270
Closed

Client doesn't honor tls-server-name setting in kubeconfig #267

multani opened this issue Aug 16, 2023 · 4 comments · Fixed by #270

Comments

@multani
Copy link
Contributor

multani commented Aug 16, 2023

If a Kubernetes configuration file proposes the tls-server-name option, the asyncio client should configure the underlying HTTP library with this particular option.

See the upstream client issue about this: kubernetes-client/python#1933
@multani
Copy link
Contributor Author

multani commented Aug 16, 2023

I tried to look in the underlying aiohttp library to see how that could be configured, but I couldn't find the right option there :/

@multani
Copy link
Contributor Author

multani commented Aug 19, 2023

It's currently blocked by aiohttp's lack of custom SNI support: aio-libs/aiohttp#7114

multani added a commit to multani/aiohttp that referenced this issue Aug 19, 2023
@multani
Support passing a custom server name parameter on HTTPS connection
0cd16ea 
This add the missing support to set the `server_hostname` setting when
creating TCP connection, when the underlying connection is authenticated
using TLS.

See the documentation for the 2 stdlib functions:

* https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.create_connection
* https://docs.python.org/3/library/asyncio-eventloop.html#opening-network-connections

The implemention is similar to what was done in urllib3 in urllib3/urllib3#1397

This would be needed to support features in clients using aiohttp, such as tomplus/kubernetes_asyncio#267

Closes: aio-libs#7114
@multani multani mentioned this issue Aug 19, 2023
Merged
5 tasks
multani added a commit to multani/aiohttp that referenced this issue Aug 19, 2023
@multani
Support passing a custom server name parameter on HTTPS connection
a6bd740 
This add the missing support to set the `server_hostname` setting when
creating TCP connection, when the underlying connection is authenticated
using TLS.

See the documentation for the 2 stdlib functions:

* https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.create_connection
* https://docs.python.org/3/library/asyncio-eventloop.html#opening-network-connections

The implemention is similar to what was done in urllib3 in urllib3/urllib3#1397

This would be needed to support features in clients using aiohttp, such as tomplus/kubernetes_asyncio#267

Closes: aio-libs#7114
@multani
Copy link
Contributor Author

multani commented Aug 19, 2023

I created a pull request on aiohttp to add the missing feature: aio-libs/aiohttp#7541

@multani multani mentioned this issue Aug 19, 2023
Merged
@tomplus
Copy link
Owner

tomplus commented Aug 19, 2023

@multani Thanks for you investigation!

multani added a commit to multani/aiohttp that referenced this issue Aug 20, 2023
@multani
Support passing a custom server name parameter on HTTPS connection
13c4465 
This add the missing support to set the `server_hostname` setting when
creating TCP connection, when the underlying connection is authenticated
using TLS.

See the documentation for the 2 stdlib functions:

* https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.create_connection
* https://docs.python.org/3/library/asyncio-eventloop.html#opening-network-connections

The implemention is similar to what was done in urllib3 in urllib3/urllib3#1397

This would be needed to support features in clients using aiohttp, such as tomplus/kubernetes_asyncio#267

Closes: aio-libs#7114
Dreamsorcerer added a commit to aio-libs/aiohttp that referenced this issue Aug 20, 2023
@multani @pre-commit-ci @Dreamsorcerer
Support passing a custom server name parameter on HTTPS connection (#…
ac29dea 
…7541)

## What do these changes do?

This adds the missing support to set the `server_hostname` setting when
creating TCP connection, when the underlying connection is authenticated
using TLS.

See the documentation for the 2 stdlib functions:

*
https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.create_connection
*
https://docs.python.org/3/library/asyncio-eventloop.html#opening-network-connections


This would be needed to support features in clients using aiohttp, such
as tomplus/kubernetes_asyncio#267

## Are there changes in behavior for the user?

The default behavior should not change, but this would allow on a
per-connection basis to specify a custom server name to check the
certificate name against.

## Related issue number

Closes: #7114

(for reference, similar implementation in urllib3:
urllib3/urllib3#1397)

## Checklist

- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [x] Documentation reflects the changes
- [x] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt`
  * The format is <Name> <Surname>.
  * Please keep alphabetical order, the file is sorted by names.
- [x] Add a new news fragment into the `CHANGES` folder
  * name it `<issue_id>.<type>` for example (588.bugfix)
* if you don't have an `issue_id` change it to the pr id after creating
the pr
  * ensure type is one of the following:
    * `.feature`: Signifying a new feature.
    * `.bugfix`: Signifying a bug fix.
    * `.doc`: Signifying a documentation improvement.
    * `.removal`: Signifying a deprecation or removal of public API.
* `.misc`: A ticket has been closed, but it is not of interest to users.
* Make sure to use full sentences with correct case and punctuation, for
example: "Fix issue with non-ascii contents in doctest text files."

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
multani added a commit to multani/aiohttp that referenced this issue Aug 20, 2023
@multani
Support passing a custom server name parameter on HTTPS connection (a…
7d7aef1 
…io-libs#7541)

This adds the missing support to set the `server_hostname` setting when
creating TCP connection, when the underlying connection is authenticated
using TLS.

See the documentation for the 2 stdlib functions:

*
https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.create_connection
*
https://docs.python.org/3/library/asyncio-eventloop.html#opening-network-connections

This would be needed to support features in clients using aiohttp, such
as tomplus/kubernetes_asyncio#267

The default behavior should not change, but this would allow on a
per-connection basis to specify a custom server name to check the
certificate name against.

Closes: aio-libs#7114

(for reference, similar implementation in urllib3:
urllib3/urllib3#1397)

- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [x] Documentation reflects the changes
- [x] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt`
  * The format is &lt;Name&gt; &lt;Surname&gt;.
  * Please keep alphabetical order, the file is sorted by names.
- [x] Add a new news fragment into the `CHANGES` folder
  * name it `<issue_id>.<type>` for example (588.bugfix)
* if you don't have an `issue_id` change it to the pr id after creating
the pr
  * ensure type is one of the following:
    * `.feature`: Signifying a new feature.
    * `.bugfix`: Signifying a bug fix.
    * `.doc`: Signifying a documentation improvement.
    * `.removal`: Signifying a deprecation or removal of public API.
* `.misc`: A ticket has been closed, but it is not of interest to users.
* Make sure to use full sentences with correct case and punctuation, for
example: "Fix issue with non-ascii contents in doctest text files."

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
(cherry picked from commit ac29dea)
@multani multani mentioned this issue Aug 20, 2023
Merged
5 tasks
Dreamsorcerer pushed a commit to aio-libs/aiohttp that referenced this issue Aug 20, 2023
@multani
Support passing a custom server name parameter on HTTPS connection (#…
f5b2f3d 
…7543)

This adds the missing support to set the `server_hostname` setting when
creating TCP connection, when the underlying connection is authenticated
using TLS.

See the documentation for the 2 stdlib functions:

*
https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.create_connection
*
https://docs.python.org/3/library/asyncio-eventloop.html#opening-network-connections

This would be needed to support features in clients using aiohttp, such
as tomplus/kubernetes_asyncio#267

The default behavior should not change, but this would allow on a
per-connection basis to specify a custom server name to check the
certificate name against.

Closes: #7114

Backport of #7541 to 3.9

- [x] I think the code is well written
- [x] Unit tests for the changes exist
- [x] Documentation reflects the changes
- [x] If you provide code modification, please add yourself to
`CONTRIBUTORS.txt`
  * The format is &lt;Name&gt; &lt;Surname&gt;.
  * Please keep alphabetical order, the file is sorted by names.
- [x] Add a new news fragment into the `CHANGES` folder
  * name it `<issue_id>.<type>` for example (588.bugfix)
* if you don't have an `issue_id` change it to the pr id after creating
the pr
* ensure type is one of the following: * `.feature`: Signifying a new
feature. * `.bugfix`: Signifying a bug fix. * `.doc`: Signifying a
documentation improvement. * `.removal`: Signifying a deprecation or
removal of public API.
* `.misc`: A ticket has been closed, but it is not of interest to users.
* Make sure to use full sentences with correct case and punctuation, for
example: "Fix issue with non-ascii contents in doctest text files."

---------

Co-authored-by: pre-commit-ci[bot]
<66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Sam Bull <aa6bs0@sambull.org>
(cherry picked from commit ac29dea)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support loading a custom TLS server name from kubeconfig multani/kubernetes_asyncio
2 participants
@multani @tomplus