Skip to content

Commit 4034d23

Browse files
mfojtikmfojtik
committed
configobservation: read service account issuers from operator status
1 parent 14f891a commit 4034d23

File tree

5 files changed

+124
-54
lines changed

5 files changed

+124
-54
lines changed

‎pkg/operator/configobservation/auth/auth_serviceaccountissuer.go

+57-25
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ package auth
22

33
import (
44
"fmt"
5+
operatorv1 "github.com/openshift/api/operator/v1"
6+
"k8s.io/apimachinery/pkg/util/sets"
57
"net/url"
8+
"sort"
69
"strings"
710

811
"k8s.io/klog/v2"
@@ -33,31 +36,28 @@ func ObserveServiceAccountIssuer(
3336
) (map[string]interface{}, []error) {
3437

3538
listers := genericListers.(configobservation.Listers)
36-
ret, errs := observedConfig(existingConfig, listers.AuthConfigLister.Get, listers.InfrastructureLister().Get, recorder)
39+
ret, errs := observedConfig(existingConfig, listers.KubeAPIServerOperatorLister().Get, listers.InfrastructureLister().Get, recorder)
3740
return configobserver.Pruned(ret, serviceAccountIssuerPath, audiencesPath, jwksURIPath), errs
3841
}
3942

4043
// observedConfig returns an unstructured fragment of KubeAPIServerConfig that may
4144
// include an override of the default service account issuer if one was set in the
4245
// Authentication resource.
43-
func observedConfig(
44-
existingConfig map[string]interface{},
45-
getAuthConfig func(string) (*configv1.Authentication, error),
46-
getInfrastructureConfig func(string) (*configv1.Infrastructure, error),
47-
recorder events.Recorder,
48-
) (map[string]interface{}, []error) {
46+
func observedConfig(existingConfig map[string]interface{},
47+
getOperator func(name string) (*operatorv1.KubeAPIServer, error),
48+
getInfrastructureConfig func(string) (*configv1.Infrastructure, error), recorder events.Recorder) (map[string]interface{}, []error) {
4949

5050
errs := []error{}
5151
var issuerChanged bool
52-
var existingIssuer, newIssuer string
52+
var existingActiveIssuer, newActiveIssuer string
5353
// when the issuer will change, indicate that by setting `issuerChanged` to true
5454
// to emit the informative event
5555
defer func() {
5656
if issuerChanged {
5757
recorder.Eventf(
5858
"ObserveServiceAccountIssuer",
5959
"ServiceAccount issuer changed from %v to %v",
60-
existingIssuer, newIssuer,
60+
existingActiveIssuer, newActiveIssuer,
6161
)
6262
}
6363
}()
@@ -68,34 +68,38 @@ func observedConfig(
6868
}
6969

7070
if len(existingIssuers) > 0 {
71-
existingIssuer = existingIssuers[0]
71+
existingActiveIssuer = existingIssuers[0]
7272
}
7373

74-
authConfig, err := getAuthConfig("cluster")
74+
operator, err := getOperator("cluster")
7575
if apierrors.IsNotFound(err) {
76-
klog.Warningf("authentications.config.openshift.io/cluster: not found")
77-
// No issuer if the auth config is missing
78-
authConfig = &configv1.Authentication{}
76+
klog.Warningf("kubeapiserver.operators.openshift.io/cluster: not found")
77+
operator = &operatorv1.KubeAPIServer{}
7978
} else if err != nil {
8079
return existingConfig, append(errs, err)
8180
}
8281

83-
newIssuer = authConfig.Spec.ServiceAccountIssuer
84-
if err := checkIssuer(newIssuer); err != nil {
82+
newActiveIssuer = getActiveServiceAccountIssuer(operator.Status.ServiceAccountIssuers)
83+
if err := checkIssuer(newActiveIssuer); err != nil {
8584
return existingConfig, append(errs, err)
8685
}
8786

88-
if len(newIssuer) != 0 {
89-
issuerChanged = existingIssuer != newIssuer
87+
if len(newActiveIssuer) > 0 {
88+
currentTrustedServiceAccountIssuers := getTrustedServiceAccountIssuers(operator.Status.ServiceAccountIssuers)
89+
issuerChanged = issuersChanged(
90+
existingIssuers,
91+
append([]string{newActiveIssuer}, currentTrustedServiceAccountIssuers...)...,
92+
)
93+
94+
value := []interface{}{newActiveIssuer}
95+
for _, i := range currentTrustedServiceAccountIssuers {
96+
value = append(value, i)
97+
}
9098
// configure the issuer if set by the user and is a valid issuer
9199
return map[string]interface{}{
92100
"apiServerArguments": map[string]interface{}{
93-
"service-account-issuer": []interface{}{
94-
newIssuer,
95-
},
96-
"api-audiences": []interface{}{
97-
newIssuer,
98-
},
101+
"service-account-issuer": value,
102+
"api-audiences": value,
99103
},
100104
}, errs
101105
}
@@ -113,7 +117,7 @@ func observedConfig(
113117
return existingConfig, append(errs, fmt.Errorf("APIServerInternalURL missing from infrastructure/cluster"))
114118
}
115119

116-
issuerChanged = existingIssuer != newIssuer
120+
issuerChanged = existingActiveIssuer != newActiveIssuer
117121
return map[string]interface{}{
118122
"apiServerArguments": map[string]interface{}{
119123
"service-account-jwks-uri": []interface{}{
@@ -123,6 +127,34 @@ func observedConfig(
123127
}, errs
124128
}
125129

130+
// issuersChanged compares the command line flags used for KAS and the operator status service account issuers.
131+
// if these two sets are different, we have a change and we need to update the KAS.
132+
func issuersChanged(kasIssuers []string, trustedOperatorIssuers ...string) bool {
133+
sort.Strings(kasIssuers)
134+
operatorAllIssuers := trustedOperatorIssuers
135+
sort.Strings(operatorAllIssuers)
136+
return !sets.NewString(kasIssuers...).Equal(sets.NewString(operatorAllIssuers...))
137+
}
138+
139+
func getTrustedServiceAccountIssuers(issuers []operatorv1.ServiceAccountIssuerStatus) []string {
140+
result := []string{}
141+
for i := range issuers {
142+
if issuers[i].ExpirationTime != nil {
143+
result = append(result, issuers[i].Name)
144+
}
145+
}
146+
return result
147+
}
148+
149+
func getActiveServiceAccountIssuer(issuers []operatorv1.ServiceAccountIssuerStatus) string {
150+
for i := range issuers {
151+
if issuers[i].ExpirationTime == nil {
152+
return issuers[i].Name
153+
}
154+
}
155+
return ""
156+
}
157+
126158
// checkIssuer validates the issuer in the same way that it will be validated by
127159
// kube-apiserver
128160
func checkIssuer(issuer string) error {

‎pkg/operator/configobservation/auth/auth_serviceaccountissuer_test.go

+48-24
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package auth
33
import (
44
"encoding/json"
55
"fmt"
6+
operatorv1 "github.com/openshift/api/operator/v1"
67
"testing"
8+
"time"
79

810
"github.com/google/go-cmp/cmp"
911
"github.com/stretchr/testify/require"
@@ -23,13 +25,15 @@ func TestObservedConfig(t *testing.T) {
2325
expectedErrInfra := fmt.Errorf("bar")
2426

2527
for _, tc := range []struct {
26-
name string
27-
issuer string
28-
existingIssuer string
29-
authError error
30-
infraError error
31-
expectedIssuer string
32-
expectedChange bool
28+
name string
29+
issuer string
30+
trustedIssuers []string
31+
existingIssuer string
32+
authError error
33+
infraError error
34+
expectedIssuer string
35+
expectedTrustedIssuers []string
36+
expectedChange bool
3337
}{
3438
{
3539
name: "no issuer, no previous issuer",
@@ -57,6 +61,14 @@ func TestObservedConfig(t *testing.T) {
5761
issuer: "https://example.com",
5862
expectedIssuer: "https://example.com",
5963
},
64+
{
65+
name: "issuer set, previous issuer and trusted issuers same",
66+
existingIssuer: "https://example.com",
67+
issuer: "https://example.com",
68+
trustedIssuers: []string{"https://trusted.example.com"},
69+
expectedIssuer: "https://example.com",
70+
expectedTrustedIssuers: []string{"https://trusted.example.com"},
71+
},
6072
{
6173
name: "issuer set, previous issuer different",
6274
existingIssuer: "https://example.com",
@@ -83,9 +95,9 @@ func TestObservedConfig(t *testing.T) {
8395
testRecorder := events.NewInMemoryRecorder("SAIssuerTest")
8496

8597
newConfig, errs := observedConfig(
86-
unstructuredAPIConfigForIssuer(t, tc.existingIssuer),
87-
func(_ string) (*configv1.Authentication, error) {
88-
return authConfigForIssuer(tc.issuer), tc.authError
98+
unstructuredAPIConfigForIssuer(t, tc.existingIssuer, tc.trustedIssuers),
99+
func(_ string) (*operatorv1.KubeAPIServer, error) {
100+
return kasStatusForIssuer(tc.issuer, tc.trustedIssuers...), tc.authError
89101
},
90102
func(_ string) (*configv1.Infrastructure, error) {
91103
return &configv1.Infrastructure{
@@ -101,7 +113,7 @@ func TestObservedConfig(t *testing.T) {
101113
if tc.authError == nil && tc.infraError == nil {
102114
require.Len(t, errs, 0)
103115
}
104-
expectedConfig = apiConfigForIssuer(tc.expectedIssuer)
116+
expectedConfig = apiConfigForIssuer(tc.expectedIssuer, tc.expectedTrustedIssuers)
105117

106118
// Check that errors are passed through
107119
if tc.authError != nil {
@@ -130,22 +142,34 @@ func TestObservedConfig(t *testing.T) {
130142
}
131143
}
132144

133-
func authConfigForIssuer(issuer string) *configv1.Authentication {
134-
return &configv1.Authentication{
135-
Spec: configv1.AuthenticationSpec{
136-
ServiceAccountIssuer: issuer,
145+
func kasStatusForIssuer(active string, trustedIssuers ...string) *operatorv1.KubeAPIServer {
146+
if len(active) == 0 {
147+
return &operatorv1.KubeAPIServer{
148+
Status: operatorv1.KubeAPIServerStatus{},
149+
}
150+
}
151+
status := []operatorv1.ServiceAccountIssuerStatus{
152+
{
153+
Name: active,
154+
},
155+
}
156+
for i := range trustedIssuers {
157+
status = append(status, operatorv1.ServiceAccountIssuerStatus{
158+
Name: trustedIssuers[i],
159+
ExpirationTime: &metav1.Time{Time: time.Now().Add(12 * time.Hour)},
160+
})
161+
}
162+
return &operatorv1.KubeAPIServer{
163+
Status: operatorv1.KubeAPIServerStatus{
164+
ServiceAccountIssuers: status,
137165
},
138166
}
139167
}
140168

141-
func apiConfigForIssuer(issuer string) *kubecontrolplanev1.KubeAPIServerConfig {
169+
func apiConfigForIssuer(issuer string, trustedIssuers []string) *kubecontrolplanev1.KubeAPIServerConfig {
142170
args := map[string]kubecontrolplanev1.Arguments{
143-
"service-account-issuer": {
144-
issuer,
145-
},
146-
"api-audiences": {
147-
issuer,
148-
},
171+
"service-account-issuer": append([]string{issuer}, trustedIssuers...),
172+
"api-audiences": append([]string{issuer}, trustedIssuers...),
149173
}
150174
if len(issuer) == 0 {
151175
delete(args, "service-account-issuer")
@@ -164,8 +188,8 @@ func apiConfigForIssuer(issuer string) *kubecontrolplanev1.KubeAPIServerConfig {
164188
// unstructuredAPIConfigForIssuer round-trips through the golang type
165189
// to ensure the input to the function under test will match what will
166190
// be received at runtime.
167-
func unstructuredAPIConfigForIssuer(t *testing.T, issuer string) map[string]interface{} {
168-
config := apiConfigForIssuer(issuer)
191+
func unstructuredAPIConfigForIssuer(t *testing.T, issuer string, trustedIssuers []string) map[string]interface{} {
192+
config := apiConfigForIssuer(issuer, trustedIssuers)
169193
// Unmarshaling to unstructured requires explicitly setting kind
170194
config.TypeMeta = metav1.TypeMeta{
171195
Kind: "KubeAPIServerConfig",

‎pkg/operator/configobservation/configobservercontroller/observe_config_controller.go

+5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package configobservercontroller
22

33
import (
4+
operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions"
45
"k8s.io/apimachinery/pkg/util/sets"
56
"k8s.io/client-go/tools/cache"
67

@@ -38,6 +39,7 @@ func NewConfigObserver(
3839
operatorClient v1helpers.StaticPodOperatorClient,
3940
kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces,
4041
configInformer configinformers.SharedInformerFactory,
42+
operatorInformer operatorv1informers.SharedInformerFactory,
4143
resourceSyncer resourcesynccontroller.ResourceSyncer,
4244
eventRecorder events.Recorder,
4345
) *ConfigObserver {
@@ -68,6 +70,7 @@ func NewConfigObserver(
6870
configInformer.Config().V1().Nodes().Informer(),
6971
configInformer.Config().V1().Proxies().Informer(),
7072
configInformer.Config().V1().Schedulers().Informer(),
73+
operatorInformer.Operator().V1().KubeAPIServers().Informer(),
7174
}
7275
for _, ns := range interestingNamespaces {
7376
infomers = append(infomers, kubeInformersForNamespaces.InformersFor(ns).Core().V1().ConfigMaps().Informer())
@@ -92,6 +95,8 @@ func NewConfigObserver(
9295
ConfigSecretLister_: kubeInformersForNamespaces.InformersFor(operatorclient.GlobalUserSpecifiedConfigNamespace).Core().V1().Secrets().Lister(),
9396
ConfigmapLister_: kubeInformersForNamespaces.ConfigMapLister(),
9497

98+
KubeAPIServerOperatorLister_: operatorInformer.Operator().V1().KubeAPIServers().Lister(),
99+
95100
ResourceSync: resourceSyncer,
96101
PreRunCachesSynced: append(preRunCacheSynced,
97102
operatorClient.Informer().HasSynced,

‎pkg/operator/configobservation/interfaces.go

+7
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"k8s.io/client-go/tools/cache"
66

77
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
8+
operatorlistersv1 "github.com/openshift/client-go/operator/listers/operator/v1"
89
"github.com/openshift/library-go/pkg/operator/configobserver/cloudprovider"
910
libgoetcd "github.com/openshift/library-go/pkg/operator/configobserver/etcd"
1011
"github.com/openshift/library-go/pkg/operator/resourcesynccontroller"
@@ -24,6 +25,8 @@ type Listers struct {
2425
ProxyLister_ configlistersv1.ProxyLister
2526
SchedulerLister configlistersv1.SchedulerLister
2627

28+
KubeAPIServerOperatorLister_ operatorlistersv1.KubeAPIServerLister
29+
2730
ConfigmapLister_ corelistersv1.ConfigMapLister
2831
SecretLister_ corelistersv1.SecretLister
2932
ConfigSecretLister_ corelistersv1.SecretLister
@@ -36,6 +39,10 @@ func (l Listers) APIServerLister() configlistersv1.APIServerLister {
3639
return l.APIServerLister_
3740
}
3841

42+
func (l Listers) KubeAPIServerOperatorLister() operatorlistersv1.KubeAPIServerLister {
43+
return l.KubeAPIServerOperatorLister_
44+
}
45+
3946
func (l Listers) FeatureGateLister() configlistersv1.FeatureGateLister {
4047
return l.FeatureGateLister_
4148
}

‎pkg/operator/starter.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -120,19 +120,21 @@ func RunOperator(ctx context.Context, controllerContext *controllercmd.Controlle
120120
return err
121121
}
122122

123+
operatorV1Client, err := operatorv1client.NewForConfig(controllerContext.KubeConfig)
124+
if err != nil {
125+
return err
126+
}
127+
operatorInformers := operatorv1informers.NewSharedInformerFactory(operatorV1Client, 10*time.Minute)
128+
123129
configObserver := configobservercontroller.NewConfigObserver(
124130
operatorClient,
125131
kubeInformersForNamespaces,
126132
configInformers,
133+
operatorInformers,
127134
resourceSyncController,
128135
controllerContext.EventRecorder,
129136
)
130137

131-
operatorV1Client, err := operatorv1client.NewForConfig(controllerContext.KubeConfig)
132-
if err != nil {
133-
return err
134-
}
135-
operatorInformers := operatorv1informers.NewSharedInformerFactory(operatorV1Client, 10*time.Minute)
136138
serviceAccountIssuerController := serviceaccountissuercontroller.NewController(operatorV1Client.OperatorV1().KubeAPIServers(), operatorInformers, configInformers, controllerContext.EventRecorder)
137139

138140
eventWatcher := eventwatch.New().

0 commit comments

Comments
 (0)