fix: Add tls-server-name support for Teleport proxy #496
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch adds support for the `tls-server-name` field from kubeconfig, which is required when connecting to Kubernetes clusters through Teleport proxy or similar services that use TLS SNI for routing. The fix follows the same pattern already used for `proxy-url` support, loading the tls-server-name from cluster config and passing it to the kubernetes client configuration. Fixes #495
WalkthroughAdds support for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@robusta_krr/core/integrations/kubernetes/config_patch.py`:
- Around line 32-42: The patch adds a tls_server_name attribute to the
Configuration class (Configuration.__init__, proxy and tls_server_name) but
kubernetes-client v26.1.0 does not honor that field; either update the
kubernetes-client dependency to 28.1.0+ so the library will use tls_server_name
for TLS/SNI, or remove reliance on that attribute and implement a compatible
workaround (for example configure SNI via the HTTP client adapter or SSLContext
used by the client) in the Configuration wrapper so the behavior works on
v26.1.0; update packaging/requirements to pin >=28.1.0 if choosing the upgrade
path or implement and document the alternative adapter/SSLContext approach if
staying on older clients.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
robusta_krr/core/integrations/kubernetes/config_patch.py
🔇 Additional comments (2)
robusta_krr/core/integrations/kubernetes/config_patch.py (2)
19-22: LGTM!Clean implementation that follows the existing pattern for
proxy-url. The comment with the issue reference provides good context for future maintainers.
27-29: LGTM!Good refactoring to DRY up the attribute propagation. Using
self.__dict__correctly ensures only explicitly-set attributes are propagated.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
32
to
+42
| class Configuration(configuration.Configuration): | ||
| def __init__( | ||
| self, | ||
| proxy: Optional[str] = None, | ||
| tls_server_name: Optional[str] = None, | ||
| **kwargs, | ||
| ): | ||
| super().__init__(**kwargs) | ||
|
|
||
| self.proxy = proxy | ||
| self.tls_server_name = tls_server_name |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if kubernetes-client 26.1.0's ApiClient or REST layer uses tls_server_name
# First, find where tls_server_name might be used in the kubernetes client
pip install kubernetes==26.1.0 -q 2>&1 | grep -v "already satisfied"
python3 -c "
import inspect
from kubernetes.client import ApiClient, Configuration
# Check if Configuration has tls_server_name property/attribute defined
print('=== Configuration class attributes ===')
config = Configuration()
print('Has tls_server_name attr:', hasattr(config, 'tls_server_name'))
# Check ApiClient's rest_client setup for tls_server_name usage
print('\n=== Searching for tls_server_name in ApiClient ===')
source = inspect.getsource(ApiClient)
if 'tls_server_name' in source:
print('Found tls_server_name in ApiClient')
else:
print('tls_server_name NOT found in ApiClient')
# Check the REST client
from kubernetes.client import rest
print('\n=== Searching for tls_server_name in REST client ===')
source = inspect.getsource(rest)
if 'tls_server_name' in source:
print('Found tls_server_name in REST client')
else:
print('tls_server_name NOT found in REST client')
# Try to check urllib3 pool manager configuration
print('\n=== Checking REST client SSL context setup ===')
import re
# Look for ssl_context or ca_certs setup in rest module
if 'ssl_context' in source:
print('Found ssl_context references in REST client')
# Find the lines that mention ssl_context
for i, line in enumerate(source.split('\n')):
if 'ssl_context' in line and 'tls_server_name' in source[max(0, source.find(line)-500):source.find(line)+500]:
print(f' Line {i}: {line.strip()}')
"Repository: robusta-dev/krr
Length of output: 868
🏁 Script executed:
pip install kubernetes==28.0.0 -q 2>&1 | grep -v "already satisfied" || true
python3 -c "
from kubernetes.client import Configuration
import inspect
# Check if v28.0.0 has tls_server_name
config = Configuration()
print('v28.0.0 Configuration has tls_server_name:', hasattr(config, 'tls_server_name'))
# Also check the REST client in v28
from kubernetes.client import rest
source = inspect.getsource(rest)
print('v28.0.0 REST client uses tls_server_name:', 'tls_server_name' in source)
"Repository: robusta-dev/krr
Length of output: 1293
🏁 Script executed:
#!/bin/bash
# Check latest kubernetes versions
pip install 'kubernetes==28.1.0' -q 2>&1 | grep -v "WARNING" | grep -v "already satisfied"
python3 << 'EOF'
from kubernetes.client import Configuration
config = Configuration()
print('kubernetes 28.1.0 Configuration has tls_server_name:', hasattr(config, 'tls_server_name'))
EOF
# Also check the actual latest stable version
pip install 'kubernetes>=30,<31' -q 2>&1 | grep -v "WARNING" | tail -1
python3 << 'EOF'
from kubernetes.client import Configuration
config = Configuration()
print('kubernetes 30.x Configuration has tls_server_name:', hasattr(config, 'tls_server_name'))
EOF
# Try to find release notes or check source
python3 << 'EOF'
import subprocess
result = subprocess.run(['pip', 'show', 'kubernetes'], capture_output=True, text=True)
print(result.stdout)
EOFRepository: robusta-dev/krr
Length of output: 833
🏁 Script executed:
python3 << 'EOF'
pip_output = """Requires: certifi, google-auth, oauthlib, python-dateutil, pyyaml, requests, requests-oauthlib, six, urllib3, websocket-client"""
# The key finding: 26.1.0 doesn't have tls_server_name, but 28.1.0 does
# Let's verify 27.2.0 (between the two) to narrow down where it was added
import subprocess
subprocess.run(['pip', 'install', '-q', 'kubernetes==27.2.0'], capture_output=True)
from kubernetes.client import Configuration
config = Configuration()
print('kubernetes 27.2.0 Configuration has tls_server_name:', hasattr(config, 'tls_server_name'))
EOFRepository: robusta-dev/krr
Length of output: 117
This patch will not work with kubernetes-client 26.1.0—upgrade to 28.1.0+ required.
The tls_server_name attribute is not supported in kubernetes-client v26.1.0. While the code correctly stores the attribute on the Configuration object, the kubernetes-client library won't read or use it for TLS/SNI configuration. Support for tls_server_name was introduced in kubernetes-client 28.1.0+. Either upgrade the kubernetes-client dependency to 28.1.0 or later, or implement a different approach compatible with v26.1.0.
🤖 Prompt for AI Agents
In `@robusta_krr/core/integrations/kubernetes/config_patch.py` around lines 32 -
42, The patch adds a tls_server_name attribute to the Configuration class
(Configuration.__init__, proxy and tls_server_name) but kubernetes-client
v26.1.0 does not honor that field; either update the kubernetes-client
dependency to 28.1.0+ so the library will use tls_server_name for TLS/SNI, or
remove reliance on that attribute and implement a compatible workaround (for
example configure SNI via the HTTP client adapter or SSLContext used by the
client) in the Configuration wrapper so the behavior works on v26.1.0; update
packaging/requirements to pin >=28.1.0 if choosing the upgrade path or implement
and document the alternative adapter/SSLContext approach if staying on older
clients.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for the
tls-server-namefield from kubeconfig, fixing SSL certificate verification failures when connecting to Kubernetes clusters through Teleport proxy.Problem
When using Teleport to access Kubernetes clusters, the kubeconfig contains a
tls-server-namefield that specifies the SNI (Server Name Indication) to use during TLS negotiation. Without this, the SSL certificate verification fails because the CA is valid for the SNI hostname, not the server URL hostname.Error users see:
Solution
This fix follows the exact same pattern already used for
proxy-urlsupport:tls-server-namefrom cluster config in_load_cluster_info()_set_config()tls_server_nameparameter to theConfigurationclassTesting
Users with Teleport-proxied Kubernetes clusters can test by:
tls-server-nameset (Teleport generates this automatically)krr simple -p <prometheus-url>Fixes #495