Skip to content

Commit 9a70a1e

Browse files
jkhelilcursoragent
jkhelil
and
cursoragent
committed
refactor(tls): extract SetupAPIServerTLSWatch into occommon for reuse by webhook
Move setupAPIServerTLSWatch, tlsProfileChanged, and customProfilesEqual out of the TektonConfig controller into pkg/reconciler/openshift/common as exported functions (SetupAPIServerTLSWatch, APIServerTLSProfileChanged). Both the operator controller and the webhook binary can now call the shared helper independently. Also moves the SKIP_APISERVER_TLS_WATCH constant to occommon.SkipAPIServerTLSWatch to avoid duplication between controller.go and the upcoming webhook main.go. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 7c96af4 commit 9a70a1e

4 files changed

Lines changed: 231 additions & 123 deletions

File tree

‎pkg/reconciler/openshift/common/apiserver_watch.go‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
/*
2+
Copyright 2026 The Tekton 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 common
18+
19+
import (
20+
"context"
21+
"fmt"
22+
"time"
23+
24+
configv1 "github.com/openshift/api/config/v1"
25+
openshiftconfigclient "github.com/openshift/client-go/config/clientset/versioned"
26+
configinformers "github.com/openshift/client-go/config/informers/externalversions"
27+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
28+
"k8s.io/client-go/rest"
29+
"k8s.io/client-go/tools/cache"
30+
"knative.dev/pkg/logging"
31+
)
32+
33+
// SkipAPIServerTLSWatch is the env var name used as an escape hatch to suppress
34+
// a fatal error when the APIServer resource is unreachable (e.g. in tests).
35+
// Both the operator controller and the webhook check this variable.
36+
const SkipAPIServerTLSWatch = "SKIP_APISERVER_TLS_WATCH"
37+
38+
// SetupAPIServerTLSWatch creates an OpenShift APIServer informer, registers
39+
// onTLSChange to be called whenever the TLS security profile changes, waits for
40+
// the informer cache to sync, and then sets the shared APIServer lister so that
41+
// GetTLSProfileFromAPIServer works in the calling process.
42+
//
43+
// This function is intentionally generic so it can be used by any process:
44+
// - The operator controller passes impl.EnqueueKey(...) as onTLSChange.
45+
// - The webhook binary passes os.Exit(1) as onTLSChange (restarts to pick up new TLS config).
46+
func SetupAPIServerTLSWatch(ctx context.Context, cfg *rest.Config, onTLSChange func()) error {
47+
logger := logging.FromContext(ctx)
48+
49+
configClient, err := openshiftconfigclient.NewForConfig(cfg)
50+
if err != nil {
51+
return fmt.Errorf("creating OpenShift config client: %w", err)
52+
}
53+
54+
// Verify we can access the APIServer resource before starting the informer.
55+
if _, err := configClient.ConfigV1().APIServers().Get(ctx, "cluster", metav1.GetOptions{}); err != nil {
56+
return fmt.Errorf("accessing APIServer resource: %w", err)
57+
}
58+
59+
// 30-minute resync is sufficient; the watch mechanism handles real-time updates.
60+
factory := configinformers.NewSharedInformerFactory(configClient, 30*time.Minute)
61+
apiServerInformer := factory.Config().V1().APIServers()
62+
63+
if _, err := apiServerInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
64+
UpdateFunc: func(oldObj, newObj interface{}) {
65+
oldAPIServer, ok := oldObj.(*configv1.APIServer)
66+
if !ok {
67+
return
68+
}
69+
newAPIServer, ok := newObj.(*configv1.APIServer)
70+
if !ok {
71+
return
72+
}
73+
if !APIServerTLSProfileChanged(oldAPIServer, newAPIServer) {
74+
return
75+
}
76+
logger.Info("APIServer TLS security profile changed")
77+
onTLSChange()
78+
},
79+
}); err != nil {
80+
return fmt.Errorf("adding APIServer event handler: %w", err)
81+
}
82+
83+
factory.Start(ctx.Done())
84+
85+
if !cache.WaitForCacheSync(ctx.Done(), apiServerInformer.Informer().HasSynced) {
86+
return fmt.Errorf("failed to sync APIServer informer cache")
87+
}
88+
89+
// Populate the shared lister so GetTLSProfileFromAPIServer works in this process.
90+
SetSharedAPIServerLister(apiServerInformer.Lister(), configClient)
91+
return nil
92+
}
93+
94+
// APIServerTLSProfileChanged reports whether the TLS security profile has changed
95+
// between two APIServer resources.
96+
func APIServerTLSProfileChanged(old, new *configv1.APIServer) bool {
97+
oldProfile := old.Spec.TLSSecurityProfile
98+
newProfile := new.Spec.TLSSecurityProfile
99+
100+
if oldProfile == nil && newProfile == nil {
101+
return false
102+
}
103+
if (oldProfile == nil) != (newProfile == nil) {
104+
return true
105+
}
106+
if oldProfile.Type != newProfile.Type {
107+
return true
108+
}
109+
if oldProfile.Type == configv1.TLSProfileCustomType {
110+
return !customTLSProfilesEqual(oldProfile.Custom, newProfile.Custom)
111+
}
112+
return false
113+
}
114+
115+
// customTLSProfilesEqual compares two custom TLS profile specs for equality.
116+
func customTLSProfilesEqual(old, new *configv1.CustomTLSProfile) bool {
117+
if old == nil && new == nil {
118+
return true
119+
}
120+
if (old == nil) != (new == nil) {
121+
return false
122+
}
123+
if old.MinTLSVersion != new.MinTLSVersion {
124+
return false
125+
}
126+
if len(old.Ciphers) != len(new.Ciphers) {
127+
return false
128+
}
129+
for i := range old.Ciphers {
130+
if old.Ciphers[i] != new.Ciphers[i] {
131+
return false
132+
}
133+
}
134+
return true
135+
}

‎pkg/reconciler/openshift/common/apiserver_watch_test.go‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
Copyright 2026 The Tekton 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 common
18+
19+
import (
20+
"testing"
21+
22+
configv1 "github.com/openshift/api/config/v1"
23+
)
24+
25+
func TestAPIServerTLSProfileChanged_BothNil(t *testing.T) {
26+
old := &configv1.APIServer{}
27+
new := &configv1.APIServer{}
28+
if APIServerTLSProfileChanged(old, new) {
29+
t.Error("expected no change when both TLS profiles are nil")
30+
}
31+
}
32+
33+
func TestAPIServerTLSProfileChanged_TypeChange(t *testing.T) {
34+
old := &configv1.APIServer{Spec: configv1.APIServerSpec{
35+
TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileIntermediateType},
36+
}}
37+
new := &configv1.APIServer{Spec: configv1.APIServerSpec{
38+
TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType},
39+
}}
40+
if !APIServerTLSProfileChanged(old, new) {
41+
t.Error("expected change when profile type differs")
42+
}
43+
}
44+
45+
func TestAPIServerTLSProfileChanged_NilToNonNil(t *testing.T) {
46+
old := &configv1.APIServer{}
47+
new := &configv1.APIServer{Spec: configv1.APIServerSpec{
48+
TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileModernType},
49+
}}
50+
if !APIServerTLSProfileChanged(old, new) {
51+
t.Error("expected change when old is nil and new is non-nil")
52+
}
53+
}
54+
55+
func TestAPIServerTLSProfileChanged_SameType(t *testing.T) {
56+
old := &configv1.APIServer{Spec: configv1.APIServerSpec{
57+
TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileIntermediateType},
58+
}}
59+
new := &configv1.APIServer{Spec: configv1.APIServerSpec{
60+
TLSSecurityProfile: &configv1.TLSSecurityProfile{Type: configv1.TLSProfileIntermediateType},
61+
}}
62+
if APIServerTLSProfileChanged(old, new) {
63+
t.Error("expected no change for same named profile type")
64+
}
65+
}
66+
67+
func TestAPIServerTLSProfileChanged_CustomCipherChange(t *testing.T) {
68+
customOld := &configv1.TLSSecurityProfile{
69+
Type: configv1.TLSProfileCustomType,
70+
Custom: &configv1.CustomTLSProfile{TLSProfileSpec: configv1.TLSProfileSpec{
71+
MinTLSVersion: configv1.VersionTLS12,
72+
Ciphers: []string{"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"},
73+
}},
74+
}
75+
customNew := &configv1.TLSSecurityProfile{
76+
Type: configv1.TLSProfileCustomType,
77+
Custom: &configv1.CustomTLSProfile{TLSProfileSpec: configv1.TLSProfileSpec{
78+
MinTLSVersion: configv1.VersionTLS12,
79+
Ciphers: []string{"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"},
80+
}},
81+
}
82+
old := &configv1.APIServer{Spec: configv1.APIServerSpec{TLSSecurityProfile: customOld}}
83+
new := &configv1.APIServer{Spec: configv1.APIServerSpec{TLSSecurityProfile: customNew}}
84+
if !APIServerTLSProfileChanged(old, new) {
85+
t.Error("expected change when custom cipher differs")
86+
}
87+
}

‎pkg/reconciler/openshift/tektonconfig/controller.go‎

Lines changed: 7 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,13 @@ package tektonconfig
1818

1919
import (
2020
"context"
21-
"fmt"
2221
"os"
23-
"time"
2422

25-
configv1 "github.com/openshift/api/config/v1"
26-
openshiftconfigclient "github.com/openshift/client-go/config/clientset/versioned"
27-
configinformers "github.com/openshift/client-go/config/informers/externalversions"
2823
"github.com/tektoncd/operator/pkg/apis/operator/v1alpha1"
2924
openshiftpipelinesascodeinformer "github.com/tektoncd/operator/pkg/client/injection/informers/operator/v1alpha1/openshiftpipelinesascode"
3025
tektonAddoninformer "github.com/tektoncd/operator/pkg/client/injection/informers/operator/v1alpha1/tektonaddon"
3126
occommon "github.com/tektoncd/operator/pkg/reconciler/openshift/common"
3227
"github.com/tektoncd/operator/pkg/reconciler/shared/tektonconfig"
33-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3428
"k8s.io/apimachinery/pkg/types"
3529
"k8s.io/client-go/tools/cache"
3630
"knative.dev/pkg/configmap"
@@ -59,11 +53,10 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl
5953

6054
// Setup APIServer TLS profile watcher
6155
// When the cluster's TLS security profile changes, enqueue TektonConfig for reconciliation
62-
const skipAPIServerWatch = "SKIP_APISERVER_TLS_WATCH"
6356
if err := setupAPIServerTLSWatch(ctx, ctrl); err != nil {
6457
// On OpenShift clusters the APIServer resource should always exist.
6558
// This env var is an escape hatch for edge cases and must be explicitly enabled.
66-
if os.Getenv(skipAPIServerWatch) == "true" {
59+
if os.Getenv(occommon.SkipAPIServerTLSWatch) == "true" {
6760
logger.Warnf("APIServer TLS profile watch not enabled: %v", err)
6861
} else {
6962
logger.Panicf("Couldn't setup APIServer TLS profile watch: %v", err)
@@ -73,121 +66,13 @@ func NewController(ctx context.Context, cmw configmap.Watcher) *controller.Impl
7366
return ctrl
7467
}
7568

76-
// setupAPIServerTLSWatch sets up a watch on the OpenShift APIServer resource
77-
// to monitor TLS security profile changes. When changes are detected, it enqueues
69+
// setupAPIServerTLSWatch sets up a watch on the OpenShift APIServer resource to
70+
// monitor TLS security profile changes. When changes are detected, it enqueues
7871
// TektonConfig for reconciliation so TLS config can be propagated to components.
7972
func setupAPIServerTLSWatch(ctx context.Context, impl *controller.Impl) error {
8073
logger := logging.FromContext(ctx)
81-
restConfig := injection.GetConfig(ctx)
82-
83-
// Create OpenShift config client
84-
configClient, err := openshiftconfigclient.NewForConfig(restConfig)
85-
if err != nil {
86-
return err
87-
}
88-
89-
// Check if we can access the APIServer resource
90-
_, err = configClient.ConfigV1().APIServers().Get(ctx, "cluster", metav1.GetOptions{})
91-
if err != nil {
92-
return err
93-
}
94-
95-
// Create a shared informer factory for OpenShift config resources.
96-
// 30 minute resync is sufficient since the APIServer resource rarely changes
97-
// and the watch mechanism handles real-time updates.
98-
configInformerFactory := configinformers.NewSharedInformerFactory(configClient, 30*time.Minute)
99-
100-
// Get the APIServer informer
101-
apiServerInformer := configInformerFactory.Config().V1().APIServers()
102-
103-
// Add event handler to watch for APIServer changes
104-
if _, err := apiServerInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
105-
UpdateFunc: func(oldObj, newObj interface{}) {
106-
oldAPIServer, ok := oldObj.(*configv1.APIServer)
107-
if !ok {
108-
return
109-
}
110-
newAPIServer, ok := newObj.(*configv1.APIServer)
111-
if !ok {
112-
return
113-
}
114-
115-
// Check if TLS security profile actually changed
116-
if !tlsProfileChanged(oldAPIServer, newAPIServer) {
117-
return
118-
}
119-
120-
logger.Info("APIServer TLS security profile changed, triggering TektonConfig reconciliation")
121-
impl.EnqueueKey(types.NamespacedName{Name: v1alpha1.ConfigResourceName})
122-
},
123-
}); err != nil {
124-
return err
125-
}
126-
127-
// Start the informer factory
128-
configInformerFactory.Start(ctx.Done())
129-
130-
// Wait for caches to sync
131-
if !cache.WaitForCacheSync(ctx.Done(), apiServerInformer.Informer().HasSynced) {
132-
return fmt.Errorf("failed to sync APIServer informer cache")
133-
}
134-
135-
// Share the lister with other components so they don't need to create their own informers
136-
occommon.SetSharedAPIServerLister(apiServerInformer.Lister(), configClient)
137-
138-
return nil
139-
}
140-
141-
// tlsProfileChanged checks if the TLS security profile has changed between two APIServer resources
142-
func tlsProfileChanged(old, new *configv1.APIServer) bool {
143-
oldProfile := old.Spec.TLSSecurityProfile
144-
newProfile := new.Spec.TLSSecurityProfile
145-
146-
// Both nil - no change
147-
if oldProfile == nil && newProfile == nil {
148-
return false
149-
}
150-
151-
// One nil, one not - changed
152-
if (oldProfile == nil) != (newProfile == nil) {
153-
return true
154-
}
155-
156-
// Different types - changed
157-
if oldProfile.Type != newProfile.Type {
158-
return true
159-
}
160-
161-
// For custom profiles, check the actual settings
162-
if oldProfile.Type == configv1.TLSProfileCustomType {
163-
return !customProfilesEqual(oldProfile.Custom, newProfile.Custom)
164-
}
165-
166-
// For predefined profiles (Old, Intermediate, Modern), type change is sufficient
167-
return false
168-
}
169-
170-
// customProfilesEqual checks if two custom TLS profiles are equal
171-
func customProfilesEqual(old, new *configv1.CustomTLSProfile) bool {
172-
if old == nil && new == nil {
173-
return true
174-
}
175-
if (old == nil) != (new == nil) {
176-
return false
177-
}
178-
179-
if old.MinTLSVersion != new.MinTLSVersion {
180-
return false
181-
}
182-
183-
if len(old.Ciphers) != len(new.Ciphers) {
184-
return false
185-
}
186-
for i := range old.Ciphers {
187-
if old.Ciphers[i] != new.Ciphers[i] {
188-
return false
189-
}
190-
}
191-
192-
return true
74+
return occommon.SetupAPIServerTLSWatch(ctx, injection.GetConfig(ctx), func() {
75+
logger.Info("APIServer TLS security profile changed, triggering TektonConfig reconciliation")
76+
impl.EnqueueKey(types.NamespacedName{Name: v1alpha1.ConfigResourceName})
77+
})
19378
}

0 commit comments

Comments
 (0)