Skip to content

Commit 8e89808

Browse files
refactor(kubeclient): consolidate duplicate code (kubernetes-sigs#6076)
* refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Co-authored-by: vflaux <38909103+vflaux@users.noreply.github.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> * refactore(kubeclient): consolidate duplicate code to ensure consistent client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> --------- Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com> Co-authored-by: vflaux <38909103+vflaux@users.noreply.github.com>
1 parent ae370da commit 8e89808

11 files changed

Lines changed: 281 additions & 160 deletions

File tree

controller/execute.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ func Execute() {
131131
os.Exit(0)
132132
}
133133

134-
ctrl, err := buildController(ctx, cfg, endpointsSource, prvdr, domainFilter)
134+
ctrl, err := buildController(ctx, cfg, sCfg, endpointsSource, prvdr, domainFilter)
135135
if err != nil {
136136
log.Fatal(err)
137137
}
@@ -362,6 +362,7 @@ func buildProvider(
362362
func buildController(
363363
ctx context.Context,
364364
cfg *externaldns.Config,
365+
sCfg *source.Config,
365366
src source.Source,
366367
p provider.Provider,
367368
filter *endpoint.DomainFilter,
@@ -375,14 +376,17 @@ func buildController(
375376
return nil, err
376377
}
377378
eventsCfg := events.NewConfig(
378-
events.WithKubeConfig(cfg.KubeConfig, cfg.APIServerURL, cfg.RequestTimeout),
379379
events.WithEmitEvents(cfg.EmitEvents),
380380
events.WithDryRun(cfg.DryRun))
381381
var eventEmitter events.EventEmitter
382382
if eventsCfg.IsEnabled() {
383-
eventCtrl, err := events.NewEventController(eventsCfg)
383+
kubeClient, err := sCfg.ClientGenerator().KubeClient()
384384
if err != nil {
385-
log.Fatal(err)
385+
return nil, err
386+
}
387+
eventCtrl, err := events.NewEventController(kubeClient.EventsV1(), eventsCfg)
388+
if err != nil {
389+
return nil, err
386390
}
387391
eventCtrl.Run(ctx)
388392
eventEmitter = eventCtrl

controller/execute_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,9 +446,10 @@ func TestControllerRunCancelContextStopsLoop(t *testing.T) {
446446
Registry: "txt",
447447
TXTOwnerID: "test-owner",
448448
}
449+
sCfg := source.NewSourceConfig(cfg)
449450
ctx, cancel := context.WithCancel(context.Background())
450451
defer cancel()
451-
src, err := buildSource(ctx, source.NewSourceConfig(cfg))
452+
src, err := buildSource(ctx, sCfg)
452453
require.NoError(t, err)
453454
domainFilter := endpoint.NewDomainFilterWithOptions(
454455
endpoint.WithDomainFilter(cfg.DomainFilter),
@@ -458,7 +459,7 @@ func TestControllerRunCancelContextStopsLoop(t *testing.T) {
458459
)
459460
p, err := buildProvider(ctx, cfg, domainFilter)
460461
require.NoError(t, err)
461-
ctrl, err := buildController(ctx, cfg, src, p, domainFilter)
462+
ctrl, err := buildController(ctx, cfg, sCfg, src, p, domainFilter)
462463
require.NoError(t, err)
463464

464465
done := make(chan struct{})

pkg/OWNERS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# See the OWNERS docs at https://go.k8s.io/owners
2+
3+
labels:
4+
- pkg

pkg/client/OWNERS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# See the OWNERS docs at https://go.k8s.io/owners
2+
3+
labels:
4+
- client

pkg/client/config.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
Copyright 2026 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Package kubeclient provides shared utilities for creating Kubernetes REST configurations
18+
// and clients with standardized metrics instrumentation.
19+
package kubeclient
20+
21+
import (
22+
"os"
23+
"time"
24+
25+
log "github.com/sirupsen/logrus"
26+
"k8s.io/client-go/kubernetes"
27+
"k8s.io/client-go/rest"
28+
"k8s.io/client-go/tools/clientcmd"
29+
30+
extdnshttp "sigs.k8s.io/external-dns/pkg/http"
31+
)
32+
33+
// GetRestConfig returns the REST client configuration for Kubernetes API access.
34+
// Supports both in-cluster and external cluster configurations.
35+
//
36+
// Configuration Priority:
37+
// 1. KubeConfig file if specified
38+
// 2. Recommended home file (~/.kube/config)
39+
// 3. In-cluster config
40+
// TODO: consider clientcmd.NewDefaultClientConfigLoadingRules() with clientcmd.NewNonInteractiveDeferredLoadingClientConfig
41+
func GetRestConfig(kubeConfig, apiServerURL string) (*rest.Config, error) {
42+
if kubeConfig == "" {
43+
if _, err := os.Stat(clientcmd.RecommendedHomeFile); err == nil {
44+
kubeConfig = clientcmd.RecommendedHomeFile
45+
}
46+
}
47+
log.Debugf("apiServerURL: %s", apiServerURL)
48+
log.Debugf("kubeConfig: %s", kubeConfig)
49+
50+
// evaluate whether to use kubeConfig-file or serviceaccount-token
51+
var (
52+
config *rest.Config
53+
err error
54+
)
55+
if kubeConfig == "" {
56+
log.Debug("Using inCluster-config based on serviceaccount-token")
57+
config, err = rest.InClusterConfig()
58+
} else {
59+
log.Debug("Using kubeConfig")
60+
config, err = clientcmd.BuildConfigFromFlags(apiServerURL, kubeConfig)
61+
}
62+
if err != nil {
63+
return nil, err
64+
}
65+
66+
return config, nil
67+
}
68+
69+
// InstrumentedRESTConfig creates a REST config with request instrumentation for monitoring.
70+
// Adds HTTP transport wrapper for Prometheus metrics collection and request timeout configuration.
71+
//
72+
// Metrics: Wraps the transport with pkg/http.NewInstrumentedTransport to collect
73+
// HTTP request duration metrics for all Kubernetes API calls.
74+
//
75+
// Timeout: Applies the specified request timeout to prevent hanging requests.
76+
func InstrumentedRESTConfig(kubeConfig, apiServerURL string, requestTimeout time.Duration) (*rest.Config, error) {
77+
config, err := GetRestConfig(kubeConfig, apiServerURL)
78+
if err != nil {
79+
return nil, err
80+
}
81+
82+
config.WrapTransport = extdnshttp.NewInstrumentedTransport
83+
config.Timeout = requestTimeout
84+
85+
return config, nil
86+
}
87+
88+
// NewKubeClient returns a new Kubernetes client object. It takes a Config and
89+
// uses APIServerURL and KubeConfig attributes to connect to the cluster. If
90+
// KubeConfig isn't provided it defaults to using the recommended default.
91+
func NewKubeClient(kubeConfig, apiServerURL string, requestTimeout time.Duration) (*kubernetes.Clientset, error) {
92+
log.Infof("Instantiating new Kubernetes client")
93+
config, err := InstrumentedRESTConfig(kubeConfig, apiServerURL, requestTimeout)
94+
if err != nil {
95+
return nil, err
96+
}
97+
client, err := kubernetes.NewForConfig(config)
98+
if err != nil {
99+
return nil, err
100+
}
101+
log.Infof("Created Kubernetes client %s", config.Host)
102+
return client, nil
103+
}

pkg/client/config_test.go

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/*
2+
Copyright 2026 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package kubeclient
18+
19+
import (
20+
"fmt"
21+
"net/http"
22+
"net/http/httptest"
23+
"os"
24+
"path/filepath"
25+
"testing"
26+
"time"
27+
28+
"github.com/stretchr/testify/assert"
29+
"github.com/stretchr/testify/require"
30+
"k8s.io/client-go/tools/clientcmd"
31+
)
32+
33+
func TestGetRestConfig_WithKubeConfig(t *testing.T) {
34+
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
35+
defer svr.Close()
36+
37+
mockKubeCfgDir := filepath.Join(t.TempDir(), ".kube")
38+
mockKubeCfgPath := filepath.Join(mockKubeCfgDir, "config")
39+
err := os.MkdirAll(mockKubeCfgDir, 0755)
40+
require.NoError(t, err)
41+
42+
kubeCfgTemplate := `apiVersion: v1
43+
kind: Config
44+
clusters:
45+
- cluster:
46+
server: %s
47+
name: test-cluster
48+
contexts:
49+
- context:
50+
cluster: test-cluster
51+
user: test-user
52+
name: test-context
53+
current-context: test-context
54+
users:
55+
- name: test-user
56+
user:
57+
token: fake-token
58+
`
59+
err = os.WriteFile(mockKubeCfgPath, fmt.Appendf(nil, kubeCfgTemplate, svr.URL), 0644)
60+
require.NoError(t, err)
61+
62+
config, err := GetRestConfig(mockKubeCfgPath, "")
63+
require.NoError(t, err)
64+
require.NotNil(t, config)
65+
assert.Equal(t, svr.URL, config.Host)
66+
}
67+
68+
func TestInstrumentedRESTConfig_AddsMetrics(t *testing.T) {
69+
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
70+
defer svr.Close()
71+
72+
mockKubeCfgDir := filepath.Join(t.TempDir(), ".kube")
73+
mockKubeCfgPath := filepath.Join(mockKubeCfgDir, "config")
74+
err := os.MkdirAll(mockKubeCfgDir, 0755)
75+
require.NoError(t, err)
76+
77+
kubeCfgTemplate := `apiVersion: v1
78+
kind: Config
79+
clusters:
80+
- cluster:
81+
server: %s
82+
name: test-cluster
83+
contexts:
84+
- context:
85+
cluster: test-cluster
86+
user: test-user
87+
name: test-context
88+
current-context: test-context
89+
users:
90+
- name: test-user
91+
user:
92+
token: fake-token
93+
`
94+
err = os.WriteFile(mockKubeCfgPath, fmt.Appendf(nil, kubeCfgTemplate, svr.URL), 0644)
95+
require.NoError(t, err)
96+
97+
timeout := 30 * time.Second
98+
config, err := InstrumentedRESTConfig(mockKubeCfgPath, "", timeout)
99+
require.NoError(t, err)
100+
require.NotNil(t, config)
101+
102+
assert.Equal(t, timeout, config.Timeout)
103+
assert.NotNil(t, config.WrapTransport, "WrapTransport should be set for metrics")
104+
}
105+
106+
func TestGetRestConfig_RecommendedHomeFile(t *testing.T) {
107+
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
108+
defer svr.Close()
109+
110+
mockKubeCfgDir := filepath.Join(t.TempDir(), ".kube")
111+
mockKubeCfgPath := filepath.Join(mockKubeCfgDir, "config")
112+
err := os.MkdirAll(mockKubeCfgDir, 0755)
113+
require.NoError(t, err)
114+
115+
kubeCfgTemplate := `apiVersion: v1
116+
kind: Config
117+
clusters:
118+
- cluster:
119+
server: %s
120+
name: test-cluster
121+
contexts:
122+
- context:
123+
cluster: test-cluster
124+
user: test-user
125+
name: test-context
126+
current-context: test-context
127+
`
128+
err = os.WriteFile(mockKubeCfgPath, fmt.Appendf(nil, kubeCfgTemplate, svr.URL), 0644)
129+
require.NoError(t, err)
130+
131+
prevRecommendedHomeFile := clientcmd.RecommendedHomeFile
132+
t.Cleanup(func() {
133+
clientcmd.RecommendedHomeFile = prevRecommendedHomeFile
134+
})
135+
clientcmd.RecommendedHomeFile = mockKubeCfgPath
136+
137+
config, err := GetRestConfig("", "")
138+
require.NoError(t, err)
139+
require.NotNil(t, config)
140+
assert.Equal(t, svr.URL, config.Host)
141+
}

pkg/events/controller.go

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ import (
2828
"k8s.io/apimachinery/pkg/util/sets"
2929
"k8s.io/apimachinery/pkg/util/wait"
3030
v1 "k8s.io/client-go/kubernetes/typed/events/v1"
31-
"k8s.io/client-go/rest"
32-
"k8s.io/client-go/tools/clientcmd"
3331
"k8s.io/client-go/util/workqueue"
3432
)
3533

@@ -53,23 +51,11 @@ type Controller struct {
5351
hostname string
5452
}
5553

56-
func NewEventController(cfg *Config) (*Controller, error) {
54+
func NewEventController(client v1.EventsV1Interface, cfg *Config) (*Controller, error) {
5755
queue := workqueue.NewTypedRateLimitingQueueWithConfig[any](
5856
workqueue.DefaultTypedControllerRateLimiter[any](),
5957
workqueue.TypedRateLimitingQueueConfig[any]{Name: controllerName},
6058
)
61-
// TODO: to externalize this as similar to source.GetRestConfig
62-
// TODO: instrument with metrics
63-
rConfig, err := GetRestConfig(cfg.kubeConfig, cfg.apiServerURL)
64-
if err != nil {
65-
return nil, err
66-
}
67-
rConfig.Timeout = cfg.timeout
68-
69-
client, err := v1.NewForConfig(rConfig)
70-
if err != nil {
71-
return nil, err
72-
}
7359
hostname, _ := os.Hostname()
7460
return &Controller{
7561
client: client,
@@ -155,28 +141,3 @@ func (ec *Controller) emit(event *eventsv1.Event) {
155141
event.ReportingController = controllerName
156142
ec.queue.Add(event)
157143
}
158-
159-
// GetRestConfig TODO: copy of source.GetRestConfig, consider moving to a common package
160-
func GetRestConfig(kubeConfig, apiServerURL string) (*rest.Config, error) {
161-
if kubeConfig == "" {
162-
if _, err := os.Stat(clientcmd.RecommendedHomeFile); err == nil {
163-
kubeConfig = clientcmd.RecommendedHomeFile
164-
}
165-
}
166-
167-
var (
168-
config *rest.Config
169-
err error
170-
)
171-
if kubeConfig == "" {
172-
log.Debug("Using inCluster-config based on serviceaccount-token")
173-
config, err = rest.InClusterConfig()
174-
} else {
175-
log.Debug("Using kubeConfig")
176-
config, err = clientcmd.BuildConfigFromFlags(apiServerURL, kubeConfig)
177-
}
178-
if err != nil {
179-
return nil, err
180-
}
181-
return config, nil
182-
}

0 commit comments

Comments
 (0)