Skip to content

Commit 1ec8af9

Browse files
jinja2jmsnll
authored andcommitted
[receiver/kubeletstats][internal/kubelet] Fix tls config for service account auth (open-telemetry#27070)
**Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Fix to use the `insecure_skip_verify` config in http client when connecting with kubelet in service account auth mode. **Link to tracking Issue:** [26319](open-telemetry#26319) **Testing:** <Describe what testing was performed and which tests were added.> Unit tests added, e2e test updated **Documentation:** <Describe the documentation added.>
1 parent 7831cd1 commit 1ec8af9

File tree

5 files changed

+99
-15
lines changed

5 files changed

+99
-15
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: breaking
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: receiver/kubeletstats
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fixes a bug where the "insecure_skip_verify" config was not being honored when "auth_type" is "serviceAccount" in kubelet client.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [26319]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
Before the fix, the kubelet client was not verifying kubelet's certificate. The default value of the config is false,
20+
so with the fix the client will start verifying tls cert unless the config is explicitly set to true.
21+
22+
# If your change doesn't affect end users or the exported elements of any package,
23+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
24+
# Optional: The change log or logs in which this entry should be included.
25+
# e.g. '[user]' or '[user, api]'
26+
# Include 'user' if the change is relevant to end users.
27+
# Include 'api' if there is a change to a library API.
28+
# Default: '[user]'
29+
change_logs: [user]

internal/kubelet/client.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,11 @@ func NewClientProvider(endpoint string, cfg *ClientConfig, logger *zap.Logger) (
4242
}, nil
4343
case k8sconfig.AuthTypeServiceAccount:
4444
return &saClientProvider{
45-
endpoint: endpoint,
46-
caCertPath: svcAcctCACertPath,
47-
tokenPath: svcAcctTokenPath,
48-
logger: logger,
45+
endpoint: endpoint,
46+
caCertPath: svcAcctCACertPath,
47+
tokenPath: svcAcctTokenPath,
48+
insecureSkipVerify: cfg.InsecureSkipVerify,
49+
logger: logger,
4950
}, nil
5051
case k8sconfig.AuthTypeNone:
5152
return &readOnlyClientProvider{
@@ -149,10 +150,11 @@ func (p *tlsClientProvider) BuildClient() (Client, error) {
149150
}
150151

151152
type saClientProvider struct {
152-
endpoint string
153-
caCertPath string
154-
tokenPath string
155-
logger *zap.Logger
153+
endpoint string
154+
caCertPath string
155+
tokenPath string
156+
insecureSkipVerify bool
157+
logger *zap.Logger
156158
}
157159

158160
func (p *saClientProvider) BuildClient() (Client, error) {
@@ -167,7 +169,7 @@ func (p *saClientProvider) BuildClient() (Client, error) {
167169
tr := defaultTransport()
168170
tr.TLSClientConfig = &tls.Config{
169171
RootCAs: rootCAs,
170-
InsecureSkipVerify: true,
172+
InsecureSkipVerify: p.insecureSkipVerify,
171173
}
172174
endpoint, err := buildEndpoint(p.endpoint, true, p.logger)
173175
if err != nil {

internal/kubelet/client_test.go

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131

3232
const certPath = "./testdata/testcert.crt"
3333
const keyFile = "./testdata/testkey.key"
34+
const errSignedByUnknownCA = "tls: failed to verify certificate: x509: certificate signed by unknown authority"
3435

3536
func TestClient(t *testing.T) {
3637
tr := &fakeRoundTripper{}
@@ -109,17 +110,18 @@ func TestSvcAcctClient(t *testing.T) {
109110
_, err := rw.Write([]byte(`OK`))
110111
require.NoError(t, err)
111112
}))
112-
cert, err := tls.LoadX509KeyPair("./testdata/testcert.crt", "./testdata/testkey.key")
113+
cert, err := tls.LoadX509KeyPair(certPath, keyFile)
113114
require.NoError(t, err)
114115
server.TLS = &tls.Config{Certificates: []tls.Certificate{cert}}
115116
server.StartTLS()
116117
defer server.Close()
117118

118119
p := &saClientProvider{
119-
endpoint: server.Listener.Addr().String(),
120-
caCertPath: "./testdata/testcert.crt",
121-
tokenPath: "./testdata/token",
122-
logger: zap.NewNop(),
120+
endpoint: server.Listener.Addr().String(),
121+
caCertPath: certPath,
122+
tokenPath: "./testdata/token",
123+
insecureSkipVerify: false,
124+
logger: zap.NewNop(),
123125
}
124126
client, err := p.BuildClient()
125127
require.NoError(t, err)
@@ -128,6 +130,29 @@ func TestSvcAcctClient(t *testing.T) {
128130
require.Equal(t, []byte(`OK`), resp)
129131
}
130132

133+
func TestSAClientBadTLS(t *testing.T) {
134+
server := httptest.NewUnstartedServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
135+
_, _ = rw.Write([]byte(`OK`))
136+
}))
137+
cert, err := tls.LoadX509KeyPair(certPath, keyFile)
138+
require.NoError(t, err)
139+
server.TLS = &tls.Config{Certificates: []tls.Certificate{cert}}
140+
server.StartTLS()
141+
defer server.Close()
142+
143+
p := &saClientProvider{
144+
endpoint: server.Listener.Addr().String(),
145+
caCertPath: "./testdata/mismatch.crt",
146+
tokenPath: "./testdata/token",
147+
insecureSkipVerify: false,
148+
logger: zap.NewNop(),
149+
}
150+
client, err := p.BuildClient()
151+
require.NoError(t, err)
152+
_, err = client.Get("/")
153+
require.ErrorContains(t, err, errSignedByUnknownCA)
154+
}
155+
131156
func TestNewKubeConfigClient(t *testing.T) {
132157
server := httptest.NewUnstartedServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
133158
// Check if call is authenticated using provided kubeconfig
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIFCzCCAvOgAwIBAgIUSzAK6jTGoQ58bOM6sc3OV+9AxAwwDQYJKoZIhvcNAQEL
3+
BQAwFTETMBEGA1UEAwwKdGVzdG9yZy5pbzAeFw0yMzA5MjIwMTM1MjNaFw0zMzA5
4+
MTkwMTM1MjNaMBUxEzARBgNVBAMMCnRlc3RvcmcuaW8wggIiMA0GCSqGSIb3DQEB
5+
AQUAA4ICDwAwggIKAoICAQDd0W5AAcAqomUlVOqDOHEpHv089u9JXqd8MTu0EqDg
6+
bZgddXrB52LTzsn6Kun2bDQtmEYvgamgfj1FkI7o2RmS+NPcXQBuBikbjbdZCMYQ
7+
pQL9zqtdVmwWwhI9fAAcLWL59sPZNp065NVUBr5w2VByQDoMCYUNwmsW+jgvW+2+
8+
nPrluQoINo+1zG0W+PGLkcFnaBen3SzsFK0IQPh2YBBO3K5uIVWrpBn4HA4PE0CZ
9+
aW4fZDX2rfzBNYMbypIbQRebgiG57AeUX7CL8PPGoYugKK+NnPz3P4MbeBGi13xa
10+
xqVR1vX0GrKgr+uzzvIzzBk5Wr77GcpKgTnOwzXaeFTsy48rLmy96Guz70aHlhP+
11+
tnVHvGGypLrgmfifNigrUgG0ZqYRBQgeC8/FlocaAhrVl/RdS/WDprDAvUS/vUkz
12+
OKeGviqD60cPyCHwznorQpf40OKJ54hVmfhXTkf4CvlHbWVf5laJ4hTIZmi4nB7+
13+
SwUUVYDmrVLp6BfO2QaODx2Fh2BUdx1pOpT6ve40DVrNnOOfvJCGJMjIwTeoP7EX
14+
Kru75EevhuNjRNlWKhlygDyI66Vi0MBPNGCNjugWBoUYdBiuKN+SdZbocZViBXnC
15+
GozCiqiZBjjIkb/i2+ntHYPCc3JajxNLtNTIvFB6gGQdhZXDht/CAw7jMLlVbTvm
16+
LQIDAQABo1MwUTAdBgNVHQ4EFgQUmumL/KesQ0Rhf9XRo+DiF62aQdQwHwYDVR0j
17+
BBgwFoAUmumL/KesQ0Rhf9XRo+DiF62aQdQwDwYDVR0TAQH/BAUwAwEB/zANBgkq
18+
hkiG9w0BAQsFAAOCAgEArZ2QqQQiXYWOEQkPpN2Li2qMdzN4LagHckVGCSPGSt03
19+
49gxEjyzJBnDpQo81wq75GnVizMG5t4TuQ0/g9Xg41wEw4gU/rlYbPl1+aF4rDed
20+
lGsjWm7neC28mMscGPVVl8jhHzW3AMnAXAxEjgbnHWzgep47CgjRhxr75G4n2fhz
21+
E0gJdl/wbX355N9XXBM+5kKFzOypkiW3d5mCt4RGIdzxBE0si+51pT0LBTLc2jwz
22+
vyBXp3adDkapoPrr7g16+0QbYHpU9/Odby3MbNqNHoDbaP54CDRhf+YvoMDu5Oqt
23+
YgScuipaStevcOFTX1jUgBS74Tdsjm9YmheWZ81MEXVlTANuYQX2U0Xdjstnao2e
24+
tJzJI5TXi9dRcNE1dKrZ4bA/kdHoIvjw6Wd+Q0PAIov83cNwJDfXr0YrHn9HOvDC
25+
KGdYV1GLrkD51g4roHYJqxNMw9PXfjZv7rveYYajusOlMZh1nP1uEU0vTvZ8HNIA
26+
Wops66yZiahgtMN9xI0Ingg5AIIwin6B8rExtoOi3NLUMc3ZEpgnHK+z406VOlVQ
27+
IHAp6ZL0GqGX86EHlvU5Cf7l9TnzIHSbWZvdNUkkva1V4hvhVVrnxoaNy+ifuCEF
28+
XHXxPDcyLN7LxymgRpximDlupMHhlpB4sUTM3pdb0UKOqSSIbWavW2YKf2Ei8+g=
29+
-----END CERTIFICATE-----

receiver/kubeletstatsreceiver/testdata/e2e/collector/configmap.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ data:
1616
receivers:
1717
kubeletstats:
1818
auth_type: "serviceAccount"
19-
insecure_skip_verify: true
2019
endpoint: "https://${env:K8S_NODE_NAME}:10250"
2120
metric_groups:
2221
- node

0 commit comments

Comments
 (0)