Skip to content

Commit 7a8af70

Browse files
committed
oauthclientsecrets: further split the clientSecret logic separately
1 parent 81c8fd9 commit 7a8af70

File tree

4 files changed

+195
-52
lines changed

4 files changed

+195
-52
lines changed

pkg/console/controllers/oauthclients/oauthclients.go

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,15 @@ import (
66
"time"
77

88
corev1 "k8s.io/api/core/v1"
9-
apierrors "k8s.io/apimachinery/pkg/api/errors"
109
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1110
"k8s.io/apimachinery/pkg/util/wait"
1211
corev1informers "k8s.io/client-go/informers/core/v1"
13-
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
1412
corev1listers "k8s.io/client-go/listers/core/v1"
1513
"k8s.io/client-go/tools/cache"
1614
"k8s.io/klog/v2"
1715

1816
configv1 "github.com/openshift/api/config/v1"
1917
operatorv1 "github.com/openshift/api/operator/v1"
20-
configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
2118
configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1"
2219
configv1lister "github.com/openshift/client-go/config/listers/config/v1"
2320
oauthclient "github.com/openshift/client-go/oauth/clientset/versioned"
@@ -29,7 +26,6 @@ import (
2926
routev1listers "github.com/openshift/client-go/route/listers/route/v1"
3027
"github.com/openshift/library-go/pkg/controller/factory"
3128
"github.com/openshift/library-go/pkg/operator/events"
32-
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
3329
"github.com/openshift/library-go/pkg/operator/v1helpers"
3430

3531
"github.com/openshift/console-operator/pkg/api"
@@ -38,17 +34,14 @@ import (
3834
oauthsub "github.com/openshift/console-operator/pkg/console/subresource/oauthclient"
3935
routesub "github.com/openshift/console-operator/pkg/console/subresource/route"
4036
secretsub "github.com/openshift/console-operator/pkg/console/subresource/secret"
41-
"github.com/openshift/console-operator/pkg/crypto"
4237
)
4338

4439
type oauthClientsController struct {
4540
oauthClient oauthv1client.OAuthClientsGetter
4641
operatorClient v1helpers.OperatorClient
47-
secretsClient corev1client.SecretsGetter
4842

4943
oauthClientLister oauthv1lister.OAuthClientLister
5044
oauthClientSwitchedInformer *util.InformerWithSwitch
51-
authentication configv1client.AuthenticationInterface
5245
authnLister configv1lister.AuthenticationLister
5346
consoleOperatorLister operatorv1listers.ConsoleLister
5447
routesLister routev1listers.RouteLister
@@ -61,8 +54,6 @@ type oauthClientsController struct {
6154
func NewOAuthClientsController(
6255
operatorClient v1helpers.OperatorClient,
6356
oauthClient oauthclient.Interface,
64-
secretsClient corev1client.SecretsGetter,
65-
authentication configv1client.AuthenticationInterface,
6657
authnInformer configv1informers.AuthenticationInformer,
6758
consoleOperatorInformer operatorv1informers.ConsoleInformer,
6859
routeInformer routev1informers.RouteInformer,
@@ -74,18 +65,14 @@ func NewOAuthClientsController(
7465
c := oauthClientsController{
7566
oauthClient: oauthClient.OauthV1(),
7667
operatorClient: operatorClient,
77-
secretsClient: secretsClient,
7868

7969
oauthClientLister: oauthClientSwitchedInformer.Lister(),
8070
oauthClientSwitchedInformer: oauthClientSwitchedInformer,
81-
authentication: authentication,
8271
authnLister: authnInformer.Lister(),
8372
consoleOperatorLister: consoleOperatorInformer.Lister(),
8473
routesLister: routeInformer.Lister(),
8574
ingressConfigLister: ingressConfigInformer.Lister(),
8675
targetNSSecretsLister: targetNSsecretsInformer.Lister(),
87-
88-
authStatusHandler: status.NewAuthStatusHandler(authentication, api.OpenShiftConsoleName, api.TargetNamespace, api.OpenShiftConsoleOperator),
8976
}
9077

9178
return factory.New().
@@ -156,10 +143,9 @@ func (c *oauthClientsController) sync(ctx context.Context, controllerContext fac
156143
return statusHandler.FlushAndReturn(fmt.Errorf("timed out waiting for OAuthClients cache sync"))
157144
}
158145

159-
clientSecret, err := c.syncSecret(ctx, operatorConfig, controllerContext.Recorder())
160-
statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "FailedApply", err))
146+
clientSecret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get("console-oauth-config")
161147
if err != nil {
162-
return statusHandler.FlushAndReturn(err)
148+
return err
163149
}
164150

165151
oauthErrReason, err := c.syncOAuthClient(ctx, clientSecret, consoleURL.String())
@@ -195,15 +181,6 @@ func (c *oauthClientsController) handleManaged(ctx context.Context) (bool, error
195181
}
196182
}
197183

198-
func (c *oauthClientsController) syncSecret(ctx context.Context, operatorConfig *operatorv1.Console, recorder events.Recorder) (*corev1.Secret, error) {
199-
secret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get(secretsub.Stub().Name)
200-
if apierrors.IsNotFound(err) || secretsub.GetSecretString(secret) == "" {
201-
secret, _, err = resourceapply.ApplySecret(ctx, c.secretsClient, recorder, secretsub.DefaultSecret(operatorConfig, crypto.Random256BitsString()))
202-
}
203-
// any error should be returned & kill the sync loop
204-
return secret, err
205-
}
206-
207184
// applies changes to the oauthclient
208185
// should not be called until route & secret dependencies are verified
209186
func (c *oauthClientsController) syncOAuthClient(
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
package oauthclientsecret
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
apierrors "k8s.io/apimachinery/pkg/api/errors"
8+
corev1informers "k8s.io/client-go/informers/core/v1"
9+
corev1clients "k8s.io/client-go/kubernetes/typed/core/v1"
10+
corev1listers "k8s.io/client-go/listers/core/v1"
11+
"k8s.io/klog/v2"
12+
13+
configv1 "github.com/openshift/api/config/v1"
14+
operatorv1 "github.com/openshift/api/operator/v1"
15+
configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1"
16+
configv1listers "github.com/openshift/client-go/config/listers/config/v1"
17+
operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1"
18+
operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1"
19+
"github.com/openshift/console-operator/pkg/api"
20+
"github.com/openshift/console-operator/pkg/console/status"
21+
"github.com/openshift/console-operator/pkg/crypto"
22+
"github.com/openshift/library-go/pkg/controller/factory"
23+
"github.com/openshift/library-go/pkg/operator/events"
24+
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
25+
v1helpers "github.com/openshift/library-go/pkg/operator/v1helpers"
26+
27+
secretsub "github.com/openshift/console-operator/pkg/console/subresource/secret"
28+
utilsub "github.com/openshift/console-operator/pkg/console/subresource/util"
29+
)
30+
31+
// oauthClientSecretController behaves differently based on authentication/cluster .spec.type:
32+
//
33+
// - IntegratedOAuth - self-manage the client secret string
34+
// - OIDC - lookup our client in the authentication/cluster .spec.oidcProviders[x].oidcClients
35+
// slice and use the 'clientSecret' from the secret referred to by .clientSecret.name
36+
// - None - do nothing
37+
//
38+
// The secret written is 'openshift-console/console-oauth-config' in .Data['clientSecret']
39+
type oauthClientSecretController struct {
40+
operatorClient v1helpers.OperatorClient
41+
secretsClient corev1clients.SecretsGetter
42+
43+
authConfigLister configv1listers.AuthenticationLister
44+
consoleOperatorLister operatorv1listers.ConsoleLister
45+
configSecretsLister corev1listers.SecretLister
46+
targetNSSecretsLister corev1listers.SecretLister
47+
}
48+
49+
func NewOAuthClientSecretController(
50+
operatorClient v1helpers.OperatorClient,
51+
secretsClient corev1clients.SecretsGetter,
52+
authnInformer configv1informers.AuthenticationInformer,
53+
consoleOperatorInformer operatorv1informers.ConsoleInformer,
54+
configSecretsInformer corev1informers.SecretInformer,
55+
targetNSsecretsInformer corev1informers.SecretInformer,
56+
recorder events.Recorder,
57+
) factory.Controller {
58+
c := &oauthClientSecretController{
59+
operatorClient: operatorClient,
60+
secretsClient: secretsClient,
61+
62+
authConfigLister: authnInformer.Lister(),
63+
consoleOperatorLister: consoleOperatorInformer.Lister(),
64+
configSecretsLister: configSecretsInformer.Lister(),
65+
targetNSSecretsLister: targetNSsecretsInformer.Lister(),
66+
}
67+
68+
return factory.New().
69+
WithSync(c.sync).
70+
WithInformers(
71+
authnInformer.Informer(),
72+
consoleOperatorInformer.Informer(),
73+
configSecretsInformer.Informer(),
74+
).
75+
WithFilteredEventsInformers(
76+
factory.NamesFilter("console-oauth-config"), targetNSsecretsInformer.Informer(),
77+
).
78+
ToController("OAuthClientSecretController", recorder.WithComponentSuffix("oauthclient-secret-controller"))
79+
}
80+
81+
func (c *oauthClientSecretController) sync(ctx context.Context, syncCtx factory.SyncContext) error {
82+
if shouldSync, err := c.handleManaged(ctx); err != nil {
83+
return err
84+
} else if !shouldSync {
85+
return nil
86+
}
87+
88+
statusHandler := status.NewStatusHandler(c.operatorClient)
89+
90+
clientSecret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get("console-oauth-config")
91+
if err != nil && !apierrors.IsNotFound(err) {
92+
return err
93+
}
94+
95+
authConfig, err := c.authConfigLister.Get("cluster")
96+
if err != nil {
97+
return fmt.Errorf("failed to retrieve authentication config: %w", err)
98+
}
99+
100+
var secretString string
101+
switch authConfig.Spec.Type {
102+
case "", configv1.AuthenticationTypeIntegratedOAuth:
103+
// in OpenShift controlled world, we generate the client secret ourselves
104+
if clientSecret != nil {
105+
secretString = secretsub.GetSecretString(clientSecret)
106+
}
107+
if len(secretString) == 0 {
108+
secretString = crypto.Random256BitsString()
109+
}
110+
case configv1.AuthenticationTypeOIDC:
111+
clientConfig := utilsub.GetOIDCClientConfig(authConfig)
112+
if clientConfig == nil {
113+
// no config, flush the condition and return
114+
statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "", nil))
115+
return statusHandler.FlushAndReturn(nil)
116+
}
117+
118+
if len(clientConfig.ClientSecret.Name) == 0 {
119+
statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "MissingClientSecretConfig", fmt.Errorf("missing client secret name reference in config")))
120+
return statusHandler.FlushAndReturn(nil)
121+
}
122+
123+
conficClientSecret, err := c.configSecretsLister.Secrets(api.OpenShiftConfigNamespace).Get(clientConfig.ClientSecret.Name)
124+
if err != nil {
125+
statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "FailedClientSecretGet", err))
126+
return statusHandler.FlushAndReturn(err)
127+
}
128+
129+
secretString = secretsub.GetSecretString(conficClientSecret)
130+
if len(secretString) == 0 {
131+
statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "ClientSecretKeyMissing", fmt.Errorf("missing the 'clientSecret' key in the client secret secret %q", clientConfig.ClientSecret.Name)))
132+
return statusHandler.FlushAndReturn(nil)
133+
}
134+
default:
135+
klog.V(2).Infof("unknown authentication type: %s", authConfig.Spec.Type)
136+
return nil
137+
}
138+
139+
err = c.syncSecret(ctx, secretString, syncCtx.Recorder())
140+
statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "FailedApply", err))
141+
return statusHandler.FlushAndReturn(err)
142+
}
143+
144+
func (c *oauthClientSecretController) syncSecret(ctx context.Context, clientSecret string, recorder events.Recorder) error {
145+
operatorConfig, err := c.consoleOperatorLister.Get(api.ConfigResourceName)
146+
if err != nil {
147+
return err
148+
}
149+
150+
secret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get("console-oauth-config")
151+
if apierrors.IsNotFound(err) || secretsub.GetSecretString(secret) == clientSecret {
152+
_, _, err = resourceapply.ApplySecret(ctx, c.secretsClient, recorder, secretsub.DefaultSecret(operatorConfig, clientSecret))
153+
}
154+
return err
155+
}
156+
157+
// handleStatus returns whether sync should happen and any error encountering
158+
// determining the operator's management state
159+
// TODO: extract this logic to where it can be used for all controllers
160+
func (c *oauthClientSecretController) handleManaged(ctx context.Context) (bool, error) {
161+
operatorSpec, _, _, err := c.operatorClient.GetOperatorState()
162+
if err != nil {
163+
return false, fmt.Errorf("failed to retrieve operator config: %w", err)
164+
}
165+
166+
switch managementState := operatorSpec.ManagementState; managementState {
167+
case operatorv1.Managed:
168+
klog.V(4).Infoln("console is in a managed state.")
169+
return true, nil
170+
case operatorv1.Unmanaged:
171+
klog.V(4).Infoln("console is in an unmanaged state.")
172+
return false, nil
173+
case operatorv1.Removed:
174+
klog.V(4).Infoln("console has been removed.")
175+
return false, nil
176+
default:
177+
return false, fmt.Errorf("console is in an unknown state: %v", managementState)
178+
}
179+
}

pkg/console/controllers/oidcsetup/oidcsetup.go

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,9 @@ import (
1010
apiexensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1111
apiexensionsv1informers "k8s.io/apiextensions-apiserver/pkg/client/informers/externalversions/apiextensions/v1"
1212
apiexensionsv1listers "k8s.io/apiextensions-apiserver/pkg/client/listers/apiextensions/v1"
13-
apierrors "k8s.io/apimachinery/pkg/api/errors"
1413
"k8s.io/apimachinery/pkg/util/wait"
1514
appsv1informers "k8s.io/client-go/informers/apps/v1"
1615
corev1informers "k8s.io/client-go/informers/core/v1"
17-
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
1816
appsv1listers "k8s.io/client-go/listers/apps/v1"
1917
corev1listers "k8s.io/client-go/listers/core/v1"
2018
"k8s.io/klog/v2"
@@ -29,20 +27,16 @@ import (
2927
"github.com/openshift/console-operator/pkg/console/status"
3028
"github.com/openshift/library-go/pkg/controller/factory"
3129
"github.com/openshift/library-go/pkg/operator/events"
32-
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
3330
"github.com/openshift/library-go/pkg/operator/v1helpers"
3431

3532
deploymentsub "github.com/openshift/console-operator/pkg/console/subresource/deployment"
36-
secretsub "github.com/openshift/console-operator/pkg/console/subresource/secret"
3733
utilsub "github.com/openshift/console-operator/pkg/console/subresource/util"
3834
)
3935

4036
type oidcSetupController struct {
4137
operatorClient v1helpers.OperatorClient
42-
secretsClient corev1client.SecretsGetter
4338

4439
authnLister configv1listers.AuthenticationLister
45-
configNSSecretsLister corev1listers.SecretLister
4640
crdLister apiexensionsv1listers.CustomResourceDefinitionLister
4741
consoleOperatorLister operatorv1listers.ConsoleLister
4842
targetNSSecretsLister corev1listers.SecretLister
@@ -54,24 +48,20 @@ type oidcSetupController struct {
5448

5549
func NewOIDCSetupController(
5650
operatorClient v1helpers.OperatorClient,
57-
secretsClient corev1client.SecretsGetter,
5851
authnInformer configv1informers.AuthenticationInformer,
5952
authenticationClient configv1client.AuthenticationInterface,
6053
consoleOperatorInformer operatorv1informers.ConsoleInformer,
6154
crdInformer apiexensionsv1informers.CustomResourceDefinitionInformer,
62-
configNSSecretsInformer corev1informers.SecretInformer,
6355
targetNSsecretsInformer corev1informers.SecretInformer,
6456
targetNSConfigMapInformer corev1informers.ConfigMapInformer,
6557
targetNSDeploymentsInformer appsv1informers.DeploymentInformer,
6658
recorder events.Recorder,
6759
) factory.Controller {
6860
c := &oidcSetupController{
6961
operatorClient: operatorClient,
70-
secretsClient: secretsClient,
7162

7263
authnLister: authnInformer.Lister(),
7364
consoleOperatorLister: consoleOperatorInformer.Lister(),
74-
configNSSecretsLister: configNSSecretsInformer.Lister(),
7565
crdLister: crdInformer.Lister(),
7666
targetNSSecretsLister: targetNSsecretsInformer.Lister(),
7767
targetNSDeploymentsLister: targetNSDeploymentsInformer.Lister(),
@@ -90,7 +80,6 @@ func NewOIDCSetupController(
9080
authnInformer.Informer(),
9181
consoleOperatorInformer.Informer(),
9282
targetNSsecretsInformer.Informer(),
93-
configNSSecretsInformer.Informer(),
9483
targetNSDeploymentsInformer.Informer(),
9584
targetNSConfigMapInformer.Informer(),
9685
).
@@ -166,23 +155,13 @@ func (c *oidcSetupController) syncAuthTypeOIDC(
166155
return nil
167156
}
168157

169-
clientSecret, err := c.configNSSecretsLister.Secrets(api.OpenShiftConfigNamespace).Get(clientConfig.ClientSecret.Name)
158+
clientSecret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get("console-oauth-config")
170159
if err != nil {
171160
c.authStatusHandler.Degraded("OIDCClientSecretGet", err.Error())
172161
return err
173162
}
174163

175-
secret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get(secretsub.Stub().Name)
176-
expectedClientSecret := secretsub.GetSecretString(clientSecret)
177-
if apierrors.IsNotFound(err) || secretsub.GetSecretString(secret) != expectedClientSecret {
178-
secret, _, err = resourceapply.ApplySecret(ctx, c.secretsClient, controllerContext.Recorder(), secretsub.DefaultSecret(operatorConfig, expectedClientSecret))
179-
if err != nil {
180-
statusHandler.AddConditions(status.HandleProgressingOrDegraded("OIDCClientSecretSync", "FailedApply", err))
181-
return err
182-
}
183-
}
184-
185-
if valid, msg, err := c.checkClientConfigStatus(authnConfig, secret); err != nil {
164+
if valid, msg, err := c.checkClientConfigStatus(authnConfig, clientSecret); err != nil {
186165
c.authStatusHandler.Degraded("DeploymentOIDCConfig", err.Error())
187166
return err
188167

0 commit comments

Comments
 (0)