Skip to content

Commit cc54807

Browse files
Merge pull request #773 from benluddy/oauth-apiserver-feature-gate-configobserver
CNTRLPLANE-5: Wire a feature-gates config observer for oauth-apiserver.
2 parents 220cc9b + dffdb12 commit cc54807

File tree

23 files changed

+126
-103
lines changed

23 files changed

+126
-103
lines changed

pkg/operator/configobservation/configobservercontroller/config_observer_controller.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
package configobservation
22

33
import (
4+
"k8s.io/apimachinery/pkg/util/sets"
45
"k8s.io/client-go/tools/cache"
56

67
configinformers "github.com/openshift/client-go/config/informers/externalversions"
78
"github.com/openshift/library-go/pkg/controller/factory"
89
"github.com/openshift/library-go/pkg/operator/configobserver"
910
"github.com/openshift/library-go/pkg/operator/configobserver/apiserver"
1011
libgoetcd "github.com/openshift/library-go/pkg/operator/configobserver/etcd"
12+
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
1113
encryptobserver "github.com/openshift/library-go/pkg/operator/encryption/observer"
1214
"github.com/openshift/library-go/pkg/operator/events"
1315
"github.com/openshift/library-go/pkg/operator/resourcesynccontroller"
1416
"github.com/openshift/library-go/pkg/operator/v1helpers"
1517

18+
configv1 "github.com/openshift/api/config/v1"
1619
"github.com/openshift/cluster-authentication-operator/pkg/operator/configobservation"
1720
observeauthentication "github.com/openshift/cluster-authentication-operator/pkg/operator/configobservation/authentication"
1821
observeoauth "github.com/openshift/cluster-authentication-operator/pkg/operator/configobservation/oauth"
@@ -28,6 +31,7 @@ func NewConfigObserverController(
2831
configInformer configinformers.SharedInformerFactory,
2932
resourceSyncer resourcesynccontroller.ResourceSyncer,
3033
eventRecorder events.Recorder,
34+
featureGateAccessor featuregates.FeatureGateAccess,
3135
) factory.Controller {
3236

3337
preRunCacheSynced := []cache.InformerSynced{
@@ -70,6 +74,16 @@ func NewConfigObserverController(
7074
observeoauth.ObserveAccessTokenInactivityTimeout,
7175
libgoetcd.ObserveStorageURLsToArguments,
7276
encryptobserver.NewEncryptionConfigObserver("openshift-oauth-apiserver", "/var/run/secrets/encryption-config/encryption-config"),
77+
featuregates.NewObserveFeatureFlagsFunc(
78+
sets.New(
79+
configv1.FeatureGateName("CBORServingAndStorage"),
80+
configv1.FeatureGateName("ClientsAllowCBOR"),
81+
configv1.FeatureGateName("ClientsPreferCBOR"),
82+
),
83+
nil,
84+
[]string{"apiServerArguments", "feature-gates"},
85+
featureGateAccessor,
86+
),
7387
} {
7488
observers = append(observers,
7589
configobserver.WithPrefix(o, OAuthAPIServerConfigPrefix))

pkg/operator/replacement_starter.go

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -343,9 +343,9 @@ func CreateOperatorStarter(ctx context.Context, authOperatorInput *authenticatio
343343
return ret, nil
344344
}
345345

346-
type featureGateAccessorFunc func(ctx context.Context, authOperatorInput *authenticationOperatorInput, informerFactories authenticationOperatorInformerFactories) (featuregates.FeatureGate, error)
346+
type featureGateAccessorFunc func(ctx context.Context, authOperatorInput *authenticationOperatorInput, informerFactories authenticationOperatorInformerFactories) (featuregates.FeatureGateAccess, error)
347347

348-
func defaultFeatureGateAccessor(ctx context.Context, authOperatorInput *authenticationOperatorInput, informerFactories authenticationOperatorInformerFactories) (featuregates.FeatureGate, error) {
348+
func defaultFeatureGateAccessor(ctx context.Context, authOperatorInput *authenticationOperatorInput, informerFactories authenticationOperatorInformerFactories) (featuregates.FeatureGateAccess, error) {
349349
// By default, this will exit(0) if the featuregates change
350350
featureGateAccessor := featuregates.NewFeatureGateAccess(
351351
status.VersionForOperatorFromEnv(), "0.0.1-snapshot",
@@ -356,19 +356,12 @@ func defaultFeatureGateAccessor(ctx context.Context, authOperatorInput *authenti
356356
go featureGateAccessor.Run(ctx)
357357
go informerFactories.operatorConfigInformer.Start(ctx.Done())
358358

359-
var featureGates featuregates.FeatureGate
360-
select {
361-
case <-featureGateAccessor.InitialFeatureGatesObserved():
362-
featureGates, _ = featureGateAccessor.CurrentFeatureGates()
363-
case <-time.After(1 * time.Minute):
364-
return nil, fmt.Errorf("timed out waiting for FeatureGate detection")
365-
}
366-
return featureGates, nil
359+
return featureGateAccessor, nil
367360
}
368361

369362
// staticFeatureGateAccessor is primarly used during testing to statically enable or disable features.
370363
func staticFeatureGateAccessor(enabled, disabled []ocpconfigv1.FeatureGateName) featureGateAccessorFunc {
371-
return func(_ context.Context, _ *authenticationOperatorInput, _ authenticationOperatorInformerFactories) (featuregates.FeatureGate, error) {
372-
return featuregates.NewFeatureGate(enabled, disabled), nil
364+
return func(_ context.Context, _ *authenticationOperatorInput, _ authenticationOperatorInformerFactories) (featuregates.FeatureGateAccess, error) {
365+
return featuregates.NewHardcodedFeatureGateAccess(enabled, disabled), nil
373366
}
374367
}

pkg/operator/starter.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
workloadcontroller "github.com/openshift/library-go/pkg/operator/apiserver/controller/workload"
4242
apiservercontrollerset "github.com/openshift/library-go/pkg/operator/apiserver/controllerset"
4343
"github.com/openshift/library-go/pkg/operator/certrotation"
44+
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
4445
"github.com/openshift/library-go/pkg/operator/csr"
4546
"github.com/openshift/library-go/pkg/operator/encryption"
4647
"github.com/openshift/library-go/pkg/operator/encryption/controllers/migrators"
@@ -639,12 +640,18 @@ func prepareOauthAPIServerOperator(
639640
return nil, nil, err
640641
}
641642

643+
featureGateAccessor, err := authOperatorInput.featureGateAccessor(ctx, authOperatorInput, informerFactories)
644+
if err != nil {
645+
return nil, nil, err
646+
}
647+
642648
configObserver := oauthapiconfigobservercontroller.NewConfigObserverController(
643649
authOperatorInput.authenticationOperatorClient,
644650
informerFactories.kubeInformersForNamespaces,
645651
informerFactories.operatorConfigInformer,
646652
resourceSyncController,
647653
authOperatorInput.eventRecorder,
654+
featureGateAccessor,
648655
)
649656

650657
webhookAuthController := webhookauthenticator.NewWebhookAuthenticatorController(
@@ -734,11 +741,19 @@ func prepareExternalOIDC(
734741
informerFactories authenticationOperatorInformerFactories,
735742
) ([]libraryapplyconfiguration.NamedRunOnce, []libraryapplyconfiguration.RunFunc, error) {
736743

737-
featureGates, err := authOperatorInput.featureGateAccessor(ctx, authOperatorInput, informerFactories)
744+
featureGateAccessor, err := authOperatorInput.featureGateAccessor(ctx, authOperatorInput, informerFactories)
738745
if err != nil {
739746
return nil, nil, err
740747
}
741748

749+
var featureGates featuregates.FeatureGate
750+
select {
751+
case <-featureGateAccessor.InitialFeatureGatesObserved():
752+
featureGates, _ = featureGateAccessor.CurrentFeatureGates()
753+
case <-time.After(1 * time.Minute):
754+
return nil, nil, fmt.Errorf("timed out waiting for FeatureGate detection")
755+
}
756+
742757
if !(featureGates.Enabled(features.FeatureGateExternalOIDC) || featureGates.Enabled(features.FeatureGateExternalOIDCWithAdditionalClaimMappings)) {
743758
return nil, nil, nil
744759
}

test-data/apply-configuration/overall/minimal-cluster/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/0f09-body-cluster.yaml

Lines changed: 0 additions & 12 deletions
This file was deleted.

test-data/apply-configuration/overall/minimal-cluster/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/0f09-metadata-cluster.yaml

Lines changed: 0 additions & 9 deletions
This file was deleted.

test-data/apply-configuration/overall/minimal-cluster/expected-output/Management/ApplyStatus/cluster-scoped-resources/operator.openshift.io/authentications/0f09-options-cluster.yaml

Lines changed: 0 additions & 2 deletions
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
apiVersion: v1
2+
count: 1
3+
eventTime: null
4+
firstTimestamp: "2024-10-14T22:38:20Z"
5+
involvedObject:
6+
kind: Deployment
7+
name: authentication-operator
8+
namespace: openshift-authentication-operator
9+
kind: Event
10+
lastTimestamp: "2024-10-14T22:38:20Z"
11+
message: 'Writing updated section ("oauthServer") of observed config: "\u00a0\u00a0map[string]any(\n-\u00a0\tnil,\n+\u00a0\t{\n+\u00a0\t\t\"corsAllowedOrigins\":
12+
[]any{string(`//127\\.0\\.0\\.1(:|$)`), string(\"//localhost(:|$)\")},\n+\u00a0\t\t\"oauthConfig\":
13+
map[string]any{\n+\u00a0\t\t\t\"loginURL\": string(\"https://api.ostest.test.metalkube.org:6443\"),\n+\u00a0\t\t\t\"tokenConfig\":
14+
map[string]any{\n+\u00a0\t\t\t\t\"accessTokenMaxAgeSeconds\": float64(86400),\n+\u00a0\t\t\t\t\"authorizeTokenMaxAgeSeconds\":
15+
float64(300),\n+\u00a0\t\t\t},\n+\u00a0\t\t},\n+\u00a0\t\t\"serverArguments\": map[string]any{\n+\u00a0\t\t\t\"audit-log-format\": []any{string(\"json\")},\n+\u00a0\t\t\t\"audit-log-maxbackup\":
16+
[]any{string(\"10\")},\n+\u00a0\t\t\t\"audit-log-maxsize\": []any{string(\"100\")},\n+\u00a0\t\t\t\"audit-log-path\": []any{string(\"/var/log/oauth-server/audit.log\")},\n+\u00a0\t\t\t\"audit-policy-file\": []any{string(\"/var/run/configmaps/audit/audit.\"...)},\n+\u00a0\t\t},\n+\u00a0\t\t\"servingInfo\":
17+
map[string]any{\n+\u00a0\t\t\t\"cipherSuites\": []any{\n+\u00a0\t\t\t\tstring(\"TLS_AES_128_GCM_SHA256\"),
18+
string(\"TLS_AES_256_GCM_SHA384\"),\n+\u00a0\t\t\t\tstring(\"TLS_CHACHA20_POLY1305_SHA256\"),\n+\u00a0\t\t\t\tstring(\"TLS_ECDHE_ECDSA_WITH_AES_128_GCM\"...),
19+
...,\n+\u00a0\t\t\t},\n+\u00a0\t\t\t\"minTLSVersion\": string(\"VersionTLS12\"),\n+\u00a0\t\t},\n+\u00a0\t},\n\u00a0\u00a0)\n"'
20+
metadata:
21+
creationTimestamp: null
22+
name: authentication-operator.17fe72c59b829800.b2cdb588
23+
namespace: openshift-authentication-operator
24+
reason: ObservedConfigChanged
25+
reportingComponent: ""
26+
reportingInstance: ""
27+
source:
28+
component: cluster-authentication-operator-run-once-sync-context
29+
type: Normal
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
action: Create
22
controllerInstanceName: ""
33
generateName: ""
4-
mame: authentication-operator.17fe72c59b829800.57eb8535
4+
mame: authentication-operator.17fe72c59b829800.b2cdb588
55
namespace: openshift-authentication-operator
66
resourceType:
77
Group: ""

test-data/apply-configuration/overall/minimal-cluster/expected-output/Management/Create/namespaces/openshift-authentication-operator/core/events/6471-body-authentication-operator.17fe72c59b829800.57eb8535.yaml

Lines changed: 0 additions & 35 deletions
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
apiVersion: v1
2+
count: 1
3+
eventTime: null
4+
firstTimestamp: "2024-10-14T22:38:20Z"
5+
involvedObject:
6+
kind: Deployment
7+
name: authentication-operator
8+
namespace: openshift-authentication-operator
9+
kind: Event
10+
lastTimestamp: "2024-10-14T22:38:20Z"
11+
message: 'Updated apiServerArguments.feature-gates to '
12+
metadata:
13+
creationTimestamp: null
14+
name: authentication-operator.17fe72c59b829800.7cfd43de
15+
namespace: openshift-authentication-operator
16+
reason: ObserveFeatureFlagsUpdated
17+
reportingComponent: ""
18+
reportingInstance: ""
19+
source:
20+
component: cluster-authentication-operator-run-once-sync-context
21+
type: Normal

0 commit comments

Comments
 (0)