Skip to content

Commit b6b184b

Browse files
Merge pull request #814 from stlaz/oauthclients_controller_fix
CONSOLE-3804: Reapply separation of the oauthclients controller
2 parents 506cf3d + a7a9876 commit b6b184b

File tree

7 files changed

+325
-153
lines changed

7 files changed

+325
-153
lines changed

manifests/03-rbac-role-cluster.yaml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,13 @@ rules:
2323
verbs:
2424
- get
2525
- list
26-
- update
2726
- watch
27+
- apiGroups:
28+
- oauth.openshift.io
29+
resources:
30+
- oauthclients
31+
verbs:
32+
- update
2833
resourceNames:
2934
- console
3035
- apiGroups:

pkg/console/controllers/downloadsdeployment/controller.go

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ import (
1414

1515
configv1 "github.com/openshift/api/config/v1"
1616
operatorv1 "github.com/openshift/api/operator/v1"
17-
configclientv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
1817
configinformer "github.com/openshift/client-go/config/informers/externalversions"
19-
operatorclientv1 "github.com/openshift/client-go/operator/clientset/versioned/typed/operator/v1"
18+
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
2019
operatorinformerv1 "github.com/openshift/client-go/operator/informers/externalversions/operator/v1"
20+
operatorlistersv1 "github.com/openshift/client-go/operator/listers/operator/v1"
2121
"github.com/openshift/console-operator/pkg/api"
2222
"github.com/openshift/console-operator/pkg/console/status"
2323
"github.com/openshift/library-go/pkg/controller/factory"
@@ -33,18 +33,15 @@ import (
3333
type DownloadsDeploymentSyncController struct {
3434
operatorClient v1helpers.OperatorClient
3535
// configs
36-
operatorConfigClient operatorclientv1.ConsoleInterface
37-
infrastructureConfigClient configclientv1.InfrastructureInterface
36+
consoleOperatorLister operatorlistersv1.ConsoleLister
37+
infrastructureLister configlistersv1.InfrastructureLister
3838
// core kube
3939
deploymentClient appsclientv1.DeploymentsGetter
4040
}
4141

4242
func NewDownloadsDeploymentSyncController(
43-
// top level config
44-
configClient configclientv1.ConfigV1Interface,
4543
// clients
4644
operatorClient v1helpers.OperatorClient,
47-
operatorConfigClient operatorclientv1.ConsoleInterface,
4845
// informer
4946
configInformer configinformer.SharedInformerFactory,
5047
operatorConfigInformer operatorinformerv1.ConsoleInformer,
@@ -58,9 +55,9 @@ func NewDownloadsDeploymentSyncController(
5855

5956
ctrl := &DownloadsDeploymentSyncController{
6057
// configs
61-
operatorClient: operatorClient,
62-
operatorConfigClient: operatorConfigClient,
63-
infrastructureConfigClient: configClient.Infrastructures(),
58+
operatorClient: operatorClient,
59+
consoleOperatorLister: operatorConfigInformer.Lister(),
60+
infrastructureLister: configInformer.Config().V1().Infrastructures().Lister(),
6461
// client
6562
deploymentClient: deploymentClient,
6663
}
@@ -81,13 +78,13 @@ func NewDownloadsDeploymentSyncController(
8178
}
8279

8380
func (c *DownloadsDeploymentSyncController) Sync(ctx context.Context, controllerContext factory.SyncContext) error {
84-
operatorConfig, err := c.operatorConfigClient.Get(ctx, api.ConfigResourceName, metav1.GetOptions{})
81+
operatorConfig, err := c.consoleOperatorLister.Get(api.ConfigResourceName)
8582
if err != nil {
8683
return err
8784
}
88-
updatedOperatorConfig := operatorConfig.DeepCopy()
85+
operatorConfigCopy := operatorConfig.DeepCopy()
8986

90-
switch updatedOperatorConfig.Spec.ManagementState {
87+
switch operatorConfigCopy.Spec.ManagementState {
9188
case operatorv1.Managed:
9289
klog.V(4).Infoln("console is in a managed state: syncing downloads deployment")
9390
case operatorv1.Unmanaged:
@@ -97,17 +94,17 @@ func (c *DownloadsDeploymentSyncController) Sync(ctx context.Context, controller
9794
klog.V(4).Infoln("console is in an removed state: removing synced downloads deployment")
9895
return c.removeDownloadsDeployment(ctx)
9996
default:
100-
return fmt.Errorf("unknown state: %v", updatedOperatorConfig.Spec.ManagementState)
97+
return fmt.Errorf("unknown state: %v", operatorConfigCopy.Spec.ManagementState)
10198
}
10299
statusHandler := status.NewStatusHandler(c.operatorClient)
103100

104-
infrastructureConfig, err := c.infrastructureConfigClient.Get(ctx, api.ConfigResourceName, metav1.GetOptions{})
101+
infrastructureConfig, err := c.infrastructureLister.Get(api.ConfigResourceName)
105102
statusHandler.AddCondition(status.HandleDegraded("DownloadsDeploymentSync", "FailedInfrastructureConfigGet", err))
106103
if err != nil {
107104
return statusHandler.FlushAndReturn(err)
108105
}
109106

110-
actualDownloadsDownloadsDeployment, _, downloadsDeploymentErr := c.SyncDownloadsDeployment(ctx, updatedOperatorConfig, infrastructureConfig, controllerContext)
107+
actualDownloadsDownloadsDeployment, _, downloadsDeploymentErr := c.SyncDownloadsDeployment(ctx, operatorConfigCopy, infrastructureConfig, controllerContext)
111108
statusHandler.AddConditions(status.HandleProgressingOrDegraded("DownloadsDeploymentSync", "FailedApply", downloadsDeploymentErr))
112109
if downloadsDeploymentErr != nil {
113110
return statusHandler.FlushAndReturn(downloadsDeploymentErr)
@@ -117,16 +114,15 @@ func (c *DownloadsDeploymentSyncController) Sync(ctx context.Context, controller
117114
return statusHandler.FlushAndReturn(nil)
118115
}
119116

120-
func (c *DownloadsDeploymentSyncController) SyncDownloadsDeployment(ctx context.Context, operatorConfig *operatorv1.Console, infrastructureConfig *configv1.Infrastructure, controllerContext factory.SyncContext) (*appsv1.Deployment, bool, error) {
117+
func (c *DownloadsDeploymentSyncController) SyncDownloadsDeployment(ctx context.Context, operatorConfigCopy *operatorv1.Console, infrastructureConfig *configv1.Infrastructure, controllerContext factory.SyncContext) (*appsv1.Deployment, bool, error) {
121118

122-
updatedOperatorConfig := operatorConfig.DeepCopy()
123-
requiredDownloadsDeployment := deploymentsub.DefaultDownloadsDeployment(updatedOperatorConfig, infrastructureConfig)
119+
requiredDownloadsDeployment := deploymentsub.DefaultDownloadsDeployment(operatorConfigCopy, infrastructureConfig)
124120

125121
return resourceapply.ApplyDeployment(ctx,
126122
c.deploymentClient,
127123
controllerContext.Recorder(),
128124
requiredDownloadsDeployment,
129-
resourcemerge.ExpectedDeploymentGeneration(requiredDownloadsDeployment, updatedOperatorConfig.Status.Generations),
125+
resourcemerge.ExpectedDeploymentGeneration(requiredDownloadsDeployment, operatorConfigCopy.Status.Generations),
130126
)
131127
}
132128

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
package oauthclients
2+
3+
import (
4+
"context"
5+
"fmt"
6+
7+
corev1 "k8s.io/api/core/v1"
8+
apierrors "k8s.io/apimachinery/pkg/api/errors"
9+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10+
corev1informers "k8s.io/client-go/informers/core/v1"
11+
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
12+
corev1listers "k8s.io/client-go/listers/core/v1"
13+
"k8s.io/klog/v2"
14+
15+
operatorv1 "github.com/openshift/api/operator/v1"
16+
configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1"
17+
configv1lister "github.com/openshift/client-go/config/listers/config/v1"
18+
oauthv1client "github.com/openshift/client-go/oauth/clientset/versioned/typed/oauth/v1"
19+
oauthv1informers "github.com/openshift/client-go/oauth/informers/externalversions/oauth/v1"
20+
oauthv1lister "github.com/openshift/client-go/oauth/listers/oauth/v1"
21+
operatorv1informers "github.com/openshift/client-go/operator/informers/externalversions/operator/v1"
22+
operatorv1listers "github.com/openshift/client-go/operator/listers/operator/v1"
23+
routev1informers "github.com/openshift/client-go/route/informers/externalversions/route/v1"
24+
routev1listers "github.com/openshift/client-go/route/listers/route/v1"
25+
"github.com/openshift/library-go/pkg/controller/factory"
26+
"github.com/openshift/library-go/pkg/operator/events"
27+
"github.com/openshift/library-go/pkg/operator/resource/resourceapply"
28+
"github.com/openshift/library-go/pkg/operator/v1helpers"
29+
30+
"github.com/openshift/console-operator/pkg/api"
31+
"github.com/openshift/console-operator/pkg/console/status"
32+
oauthsub "github.com/openshift/console-operator/pkg/console/subresource/oauthclient"
33+
routesub "github.com/openshift/console-operator/pkg/console/subresource/route"
34+
secretsub "github.com/openshift/console-operator/pkg/console/subresource/secret"
35+
"github.com/openshift/console-operator/pkg/crypto"
36+
)
37+
38+
type oauthClientsController struct {
39+
oauthClient oauthv1client.OAuthClientsGetter
40+
operatorClient v1helpers.OperatorClient
41+
secretsClient corev1client.SecretsGetter
42+
43+
oauthClientLister oauthv1lister.OAuthClientLister
44+
consoleOperatorLister operatorv1listers.ConsoleLister
45+
routesLister routev1listers.RouteLister
46+
ingressConfigLister configv1lister.IngressLister
47+
targetNSSecretsLister corev1listers.SecretLister
48+
49+
statusHandler status.StatusHandler
50+
}
51+
52+
func NewOAuthClientsController(
53+
operatorClient v1helpers.OperatorClient,
54+
oauthClient oauthv1client.OAuthClientsGetter,
55+
secretsClient corev1client.SecretsGetter,
56+
oauthClientInformer oauthv1informers.OAuthClientInformer,
57+
consoleOperatorInformer operatorv1informers.ConsoleInformer,
58+
routeInformer routev1informers.RouteInformer,
59+
ingressConfigInformer configv1informers.IngressInformer,
60+
targetNSsecretsInformer corev1informers.SecretInformer,
61+
recorder events.Recorder,
62+
) factory.Controller {
63+
c := oauthClientsController{
64+
oauthClient: oauthClient,
65+
operatorClient: operatorClient,
66+
secretsClient: secretsClient,
67+
68+
oauthClientLister: oauthClientInformer.Lister(),
69+
consoleOperatorLister: consoleOperatorInformer.Lister(),
70+
routesLister: routeInformer.Lister(),
71+
ingressConfigLister: ingressConfigInformer.Lister(),
72+
targetNSSecretsLister: targetNSsecretsInformer.Lister(),
73+
74+
statusHandler: status.NewStatusHandler(operatorClient),
75+
}
76+
77+
return factory.New().
78+
WithSync(c.sync).
79+
WithInformers(
80+
consoleOperatorInformer.Informer(),
81+
routeInformer.Informer(),
82+
ingressConfigInformer.Informer(),
83+
targetNSsecretsInformer.Informer(),
84+
).
85+
WithFilteredEventsInformers(
86+
factory.NamesFilter(api.OAuthClientName),
87+
oauthClientInformer.Informer(),
88+
).
89+
WithSyncDegradedOnError(operatorClient).
90+
ToController("OAuthClientsController", recorder.WithComponentSuffix("oauth-clients-controller"))
91+
}
92+
93+
func (c *oauthClientsController) sync(ctx context.Context, controllerContext factory.SyncContext) error {
94+
if shouldSync, err := c.handleStatus(ctx); err != nil {
95+
return err
96+
} else if !shouldSync {
97+
return nil
98+
}
99+
100+
operatorConfig, err := c.consoleOperatorLister.Get("cluster")
101+
if err != nil {
102+
return err
103+
}
104+
105+
ingressConfig, err := c.ingressConfigLister.Get("cluster")
106+
if err != nil {
107+
return err
108+
}
109+
110+
routeName := api.OpenShiftConsoleRouteName
111+
routeConfig := routesub.NewRouteConfig(operatorConfig, ingressConfig, routeName)
112+
if routeConfig.IsCustomHostnameSet() {
113+
routeName = api.OpenshiftConsoleCustomRouteName
114+
}
115+
116+
_, consoleURL, _, routeErr := routesub.GetActiveRouteInfo(c.routesLister, routeName)
117+
if routeErr != nil {
118+
return routeErr
119+
}
120+
121+
clientSecret, secErr := c.syncSecret(ctx, operatorConfig, controllerContext.Recorder())
122+
c.statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSecretSync", "FailedApply", secErr))
123+
if secErr != nil {
124+
return c.statusHandler.FlushAndReturn(secErr)
125+
}
126+
127+
oauthErrReason, oauthErr := c.syncOAuthClient(ctx, clientSecret, consoleURL.String())
128+
c.statusHandler.AddConditions(status.HandleProgressingOrDegraded("OAuthClientSync", oauthErrReason, oauthErr))
129+
130+
return c.statusHandler.FlushAndReturn(oauthErr)
131+
}
132+
133+
// handleStatus returns whether sync should happen and any error encountering
134+
// determining the operator's management state
135+
// TODO: extract this logic to where it can be used for all controllers
136+
func (c *oauthClientsController) handleStatus(ctx context.Context) (bool, error) {
137+
operatorSpec, _, _, err := c.operatorClient.GetOperatorState()
138+
if err != nil {
139+
return false, fmt.Errorf("failed to retrieve operator config: %w", err)
140+
}
141+
142+
switch managementState := operatorSpec.ManagementState; managementState {
143+
case operatorv1.Managed:
144+
klog.V(4).Infoln("console is in a managed state.")
145+
return true, nil
146+
case operatorv1.Unmanaged:
147+
klog.V(4).Infoln("console is in an unmanaged state.")
148+
return false, nil
149+
case operatorv1.Removed:
150+
klog.V(4).Infoln("console has been removed.")
151+
return false, c.deregisterClient(ctx)
152+
default:
153+
return false, fmt.Errorf("console is in an unknown state: %v", managementState)
154+
}
155+
}
156+
157+
func (c *oauthClientsController) syncSecret(ctx context.Context, operatorConfig *operatorv1.Console, recorder events.Recorder) (*corev1.Secret, error) {
158+
secret, err := c.targetNSSecretsLister.Secrets(api.TargetNamespace).Get(secretsub.Stub().Name)
159+
if apierrors.IsNotFound(err) || secretsub.GetSecretString(secret) == "" {
160+
secret, _, err = resourceapply.ApplySecret(ctx, c.secretsClient, recorder, secretsub.DefaultSecret(operatorConfig, crypto.Random256BitsString()))
161+
}
162+
// any error should be returned & kill the sync loop
163+
return secret, err
164+
}
165+
166+
// applies changes to the oauthclient
167+
// should not be called until route & secret dependencies are verified
168+
func (c *oauthClientsController) syncOAuthClient(
169+
ctx context.Context,
170+
sec *corev1.Secret,
171+
consoleURL string,
172+
) (reason string, err error) {
173+
oauthClient, err := c.oauthClientLister.Get(oauthsub.Stub().Name)
174+
if err != nil {
175+
// at this point we must die & wait for someone to fix the lack of an outhclient. there is nothing we can do.
176+
return "FailedGet", fmt.Errorf("oauth client for console does not exist and cannot be created (%w)", err)
177+
}
178+
clientCopy := oauthClient.DeepCopy()
179+
oauthsub.RegisterConsoleToOAuthClient(clientCopy, consoleURL, secretsub.GetSecretString(sec))
180+
_, _, oauthErr := oauthsub.CustomApplyOAuth(c.oauthClient, clientCopy, ctx)
181+
if oauthErr != nil {
182+
return "FailedRegister", oauthErr
183+
}
184+
return "", nil
185+
}
186+
187+
func (c *oauthClientsController) deregisterClient(ctx context.Context) error {
188+
// existingOAuthClient is not a delete, it is a deregister/neutralize
189+
existingOAuthClient, err := c.oauthClientLister.Get(oauthsub.Stub().Name)
190+
if err != nil {
191+
return err
192+
}
193+
194+
if len(existingOAuthClient.RedirectURIs) == 0 {
195+
return nil
196+
}
197+
198+
updated := oauthsub.DeRegisterConsoleFromOAuthClient(existingOAuthClient.DeepCopy())
199+
_, err = c.oauthClient.OAuthClients().Update(ctx, updated, metav1.UpdateOptions{})
200+
return err
201+
202+
}

0 commit comments

Comments
 (0)