Skip to content

Commit 386c3c6

Browse files
haircommanderhaircommander
committed
observeAuthMode: drop authorization-mode from default config and specify strictly with observer
Signed-off-by: Peter Hunt <[email protected]>
1 parent e4c99d6 commit 386c3c6

File tree

7 files changed

+165
-170
lines changed

7 files changed

+165
-170
lines changed

‎bindata/assets/config/defaultconfig.yaml‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,6 @@ apiServerArguments:
3434
- "true"
3535
anonymous-auth:
3636
- "true"
37-
authorization-mode:
38-
- Scope
39-
- SystemMasters
40-
- RBAC
41-
- Node
4237
audit-log-format:
4338
- json
4439
audit-log-maxbackup:

‎pkg/cmd/render/render.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -358,10 +358,8 @@ func bootstrapDefaultConfig(featureGates featuregates.FeatureGate) ([]byte, erro
358358
}
359359
}
360360

361-
if featureGates.Enabled(features.FeatureGateMinimumKubeletVersion) {
362-
if err := node.SetAPIServerArgumentsToEnforceMinimumKubeletVersion(defaultConfig, true); err != nil {
363-
return nil, err
364-
}
361+
if err := node.AddAuthorizationModes(defaultConfig, featureGates.Enabled(features.FeatureGateMinimumKubeletVersion)); err != nil {
362+
return nil, err
365363
}
366364

367365
defaultConfigRaw, err := json.Marshal(defaultConfig)

‎pkg/operator/configobservation/node/listers_test.go‎

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

‎pkg/operator/configobservation/node/observe_authorization_mode.go‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package node
2+
3+
import (
4+
"github.com/openshift/api/features"
5+
"github.com/openshift/library-go/pkg/operator/configobserver"
6+
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
7+
"github.com/openshift/library-go/pkg/operator/events"
8+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
9+
)
10+
11+
// There are scopes authorizer tests that fail if this order is changed.
12+
// So this should not be sorted
13+
var defaultAuthenticationModes = []string{
14+
"Scope",
15+
"SystemMasters",
16+
"RBAC",
17+
"Node",
18+
}
19+
var (
20+
authModeFlag = "authorization-mode"
21+
apiServerArgs = "apiServerArguments"
22+
authModePath = []string{apiServerArgs, authModeFlag}
23+
)
24+
25+
type authorizationModeObserver struct {
26+
featureGateAccessor featuregates.FeatureGateAccess
27+
authModes []string
28+
}
29+
30+
func NewAuthorizationModeObserver(featureGateAccessor featuregates.FeatureGateAccess) configobserver.ObserveConfigFunc {
31+
return (&authorizationModeObserver{
32+
featureGateAccessor: featureGateAccessor,
33+
}).ObserveAuthorizationMode
34+
}
35+
36+
// ObserveAuthorizationMode watches the featuregate configuration and generates the apiServerArguments.authorization-mode
37+
// It currently hardcodes the default set and adds MinimumKubeletVersion if the feature is set to on.
38+
func (o *authorizationModeObserver) ObserveAuthorizationMode(genericListers configobserver.Listers, _ events.Recorder, existingConfig map[string]interface{}) (ret map[string]interface{}, errs []error) {
39+
defer func() {
40+
// Prune the observed config so that it only contains minimumKubeletVersion field.
41+
ret = configobserver.Pruned(ret, authModePath)
42+
}()
43+
44+
if !o.featureGateAccessor.AreInitialFeatureGatesObserved() {
45+
return existingConfig, nil
46+
}
47+
48+
featureGates, err := o.featureGateAccessor.CurrentFeatureGates()
49+
if err != nil {
50+
return existingConfig, append(errs, err)
51+
}
52+
53+
ret = map[string]interface{}{}
54+
if err := AddAuthorizationModes(ret, featureGates.Enabled(features.FeatureGateMinimumKubeletVersion)); err != nil {
55+
return existingConfig, append(errs, err)
56+
}
57+
return ret, nil
58+
}
59+
60+
// AddAuthorizationModes modifies the passed in config
61+
// to add the "authorization-mode": "MinimumKubeletVersion" if the feature is on. If it's off, it
62+
// removes it instead.
63+
// This function assumes MinimumKubeletVersion auth mode isn't present by default,
64+
// and should likely be removed when it is.
65+
func AddAuthorizationModes(observedConfig map[string]interface{}, isMinimumKubeletVersionEnabled bool) error {
66+
modes := defaultAuthenticationModes
67+
if isMinimumKubeletVersionEnabled {
68+
modes = append(modes, ModeMinimumKubeletVersion)
69+
}
70+
71+
unstructured.RemoveNestedField(observedConfig, authModePath...)
72+
return unstructured.SetNestedStringSlice(observedConfig, modes, authModePath...)
73+
}

‎pkg/operator/configobservation/node/observe_authorization_mode_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
package node
2+
3+
import (
4+
"testing"
5+
6+
"github.com/google/go-cmp/cmp"
7+
)
8+
9+
func TestAddAuthorizationModes(t *testing.T) {
10+
for _, on := range []bool{false, true} {
11+
expectedSet := []any{"Scope", "SystemMasters", "RBAC", "Node"}
12+
if on {
13+
expectedSet = []any{"Scope", "SystemMasters", "RBAC", ModeMinimumKubeletVersion, "Node"}
14+
}
15+
for _, tc := range []struct {
16+
name string
17+
existingConfig map[string]interface{}
18+
expectedConfig map[string]interface{}
19+
}{
20+
{
21+
name: "should not fail if apiServerArguments not present",
22+
existingConfig: map[string]interface{}{
23+
"fakeconfig": "fake",
24+
},
25+
expectedConfig: map[string]interface{}{
26+
"fakeconfig": "fake",
27+
"apiServerArguments": map[string]any{"authorization-mode": expectedSet},
28+
},
29+
},
30+
{
31+
name: "should not fail if authorization-mode not present",
32+
existingConfig: map[string]interface{}{
33+
"apiServerArguments": map[string]any{"fake": []any{"fake"}},
34+
},
35+
expectedConfig: map[string]interface{}{
36+
"apiServerArguments": map[string]any{"fake": []any{"fake"}, "authorization-mode": expectedSet},
37+
},
38+
},
39+
{
40+
name: "should clobber value if not expected",
41+
existingConfig: map[string]interface{}{
42+
"apiServerArguments": map[string]any{"authorization-mode": []any{"fake"}},
43+
},
44+
expectedConfig: map[string]interface{}{
45+
"apiServerArguments": map[string]any{"authorization-mode": expectedSet},
46+
},
47+
},
48+
{
49+
name: "should not fail if MinimumKubeletVersion already present",
50+
existingConfig: map[string]interface{}{
51+
"apiServerArguments": map[string]any{"authorization-mode": []any{"MinimumKubeletVersion"}},
52+
},
53+
expectedConfig: map[string]interface{}{
54+
"apiServerArguments": map[string]any{"authorization-mode": expectedSet},
55+
},
56+
},
57+
{
58+
name: "should not fail if apiServerArguments not present",
59+
existingConfig: map[string]interface{}{
60+
"fakeconfig": "fake",
61+
},
62+
expectedConfig: map[string]interface{}{
63+
"fakeconfig": "fake",
64+
"apiServerArguments": map[string]any{"authorization-mode": expectedSet},
65+
},
66+
},
67+
} {
68+
name := tc.name + " when feature is "
69+
if on {
70+
name += "on"
71+
} else {
72+
name += "off"
73+
}
74+
t.Run(name, func(t *testing.T) {
75+
if err := AddAuthorizationModes(tc.existingConfig, on); err != nil {
76+
t.Fatal(err)
77+
}
78+
79+
if diff := cmp.Diff(tc.expectedConfig, tc.existingConfig); diff != "" {
80+
t.Errorf("unexpected config:\n%s", diff)
81+
}
82+
})
83+
}
84+
}
85+
}

‎pkg/operator/configobservation/node/observe_minimum_kubelet_version.go‎

Lines changed: 2 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
package node
22

33
import (
4-
"sort"
5-
64
configv1 "github.com/openshift/api/config/v1"
75
"github.com/openshift/api/features"
6+
"github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/configobservation"
87
"github.com/openshift/library-go/pkg/operator/configobserver"
98
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
109
"github.com/openshift/library-go/pkg/operator/events"
@@ -16,12 +15,6 @@ import (
1615
var (
1716
ModeMinimumKubeletVersion = "MinimumKubeletVersion"
1817
minimumKubeletVersionConfigPath = "minimumKubeletVersion"
19-
authModeFlag = "authorization-mode"
20-
apiServerArgs = "apiServerArguments"
21-
authModePath = []string{apiServerArgs, authModeFlag}
22-
// The default value for apiServerArguments.authorization-mode.
23-
// Should be synced with bindata/assets/config/defaultconfig.yaml
24-
DefaultAuthorizationModes = []string{"Scope", "SystemMasters", "RBAC", "Node"}
2518
)
2619

2720
type minimumKubeletVersionObserver struct {
@@ -54,7 +47,7 @@ func (o *minimumKubeletVersionObserver) ObserveMinimumKubeletVersion(genericList
5447
return existingConfig, nil
5548
}
5649

57-
nodeLister := genericListers.(NodeLister)
50+
nodeLister := genericListers.(configobservation.Listers)
5851
configNode, err := nodeLister.NodeLister().Get("cluster")
5952
// we got an error so without the node object we are not able to determine minimumKubeletVersion
6053
if err != nil {
@@ -82,51 +75,3 @@ func (o *minimumKubeletVersionObserver) ObserveMinimumKubeletVersion(genericList
8275

8376
return ret, errs
8477
}
85-
86-
type authorizationModeObserver struct {
87-
featureGateAccessor featuregates.FeatureGateAccess
88-
}
89-
90-
func NewAuthorizationModeObserver(featureGateAccessor featuregates.FeatureGateAccess) configobserver.ObserveConfigFunc {
91-
return (&authorizationModeObserver{
92-
featureGateAccessor: featureGateAccessor,
93-
}).ObserveAuthorizationMode
94-
}
95-
96-
// ObserveAuthorizationMode watches the featuregate configuration and generates the apiServerArguments.authorization-mode
97-
// It currently hardcodes the default set and adds MinimumKubeletVersion if the feature is set to on.
98-
func (o *authorizationModeObserver) ObserveAuthorizationMode(genericListers configobserver.Listers, _ events.Recorder, existingConfig map[string]interface{}) (ret map[string]interface{}, errs []error) {
99-
ret = map[string]interface{}{}
100-
if !o.featureGateAccessor.AreInitialFeatureGatesObserved() {
101-
return existingConfig, nil
102-
}
103-
104-
featureGates, err := o.featureGateAccessor.CurrentFeatureGates()
105-
if err != nil {
106-
return existingConfig, append(errs, err)
107-
}
108-
109-
defer func() {
110-
// Prune the observed config so that it only contains minimumKubeletVersion field.
111-
ret = configobserver.Pruned(ret, authModePath)
112-
}()
113-
114-
if err := SetAPIServerArgumentsToEnforceMinimumKubeletVersion(ret, featureGates.Enabled(features.FeatureGateMinimumKubeletVersion)); err != nil {
115-
return existingConfig, append(errs, err)
116-
}
117-
return ret, nil
118-
}
119-
120-
// SetAPIServerArgumentsToEnforceMinimumKubeletVersion modifies the passed in config
121-
// to add the "authorization-mode": "MinimumKubeletVersion" if the feature is on. If it's off, it
122-
// removes it instead.
123-
func SetAPIServerArgumentsToEnforceMinimumKubeletVersion(newConfig map[string]interface{}, on bool) error {
124-
defaultSet := DefaultAuthorizationModes
125-
if on {
126-
defaultSet = append(defaultSet, ModeMinimumKubeletVersion)
127-
}
128-
sort.Sort(sort.StringSlice(defaultSet))
129-
130-
unstructured.RemoveNestedField(newConfig, authModePath...)
131-
return unstructured.SetNestedStringSlice(newConfig, defaultSet, authModePath...)
132-
}

‎pkg/operator/configobservation/node/observe_minimum_kubelet_version_test.go‎

Lines changed: 3 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
configv1 "github.com/openshift/api/config/v1"
99
"github.com/openshift/api/features"
1010
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
11+
"github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/configobservation"
1112
"github.com/openshift/library-go/pkg/operator/configobserver/featuregates"
1213
"github.com/openshift/library-go/pkg/operator/events"
1314
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -75,8 +76,8 @@ func TestObserveKubeletMinimumVersion(t *testing.T) {
7576
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
7677
Spec: configv1.NodeSpec{MinimumKubeletVersion: test.minimumKubeletVersion},
7778
})
78-
listers := testLister{
79-
nodeLister: configlistersv1.NewNodeLister(configNodeIndexer),
79+
listers := configobservation.Listers{
80+
NodeLister_: configlistersv1.NewNodeLister(configNodeIndexer),
8081
}
8182

8283
fg := featuregates.NewHardcodedFeatureGateAccess([]configv1.FeatureGateName{features.FeatureGateMinimumKubeletVersion}, []configv1.FeatureGateName{})
@@ -97,81 +98,3 @@ func TestObserveKubeletMinimumVersion(t *testing.T) {
9798
})
9899
}
99100
}
100-
101-
func TestSetAPIServerArgumentsToEnforceMinimumKubeletVersion(t *testing.T) {
102-
for _, on := range []bool{false, true} {
103-
expectedSet := []any{"Node", "RBAC", "Scope", "SystemMasters"}
104-
if on {
105-
expectedSet = append([]any{ModeMinimumKubeletVersion}, expectedSet...)
106-
}
107-
for _, tc := range []struct {
108-
name string
109-
existingConfig map[string]interface{}
110-
expectedConfig map[string]interface{}
111-
}{
112-
{
113-
name: "should not fail if apiServerArguments not present",
114-
existingConfig: map[string]interface{}{
115-
"fakeconfig": "fake",
116-
},
117-
expectedConfig: map[string]interface{}{
118-
"fakeconfig": "fake",
119-
"apiServerArguments": map[string]any{"authorization-mode": expectedSet},
120-
},
121-
},
122-
{
123-
name: "should not fail if authorization-mode not present",
124-
existingConfig: map[string]interface{}{
125-
"apiServerArguments": map[string]any{"fake": []any{"fake"}},
126-
},
127-
expectedConfig: map[string]interface{}{
128-
"apiServerArguments": map[string]any{"fake": []any{"fake"}, "authorization-mode": expectedSet},
129-
},
130-
},
131-
{
132-
name: "should clobber value if not expected",
133-
existingConfig: map[string]interface{}{
134-
"apiServerArguments": map[string]any{"authorization-mode": []any{"fake"}},
135-
},
136-
expectedConfig: map[string]interface{}{
137-
"apiServerArguments": map[string]any{"authorization-mode": expectedSet},
138-
},
139-
},
140-
{
141-
name: "should not fail if MinimumKubeletVersion already present",
142-
existingConfig: map[string]interface{}{
143-
"apiServerArguments": map[string]any{"authorization-mode": []any{"MinimumKubeletVersion"}},
144-
},
145-
expectedConfig: map[string]interface{}{
146-
"apiServerArguments": map[string]any{"authorization-mode": expectedSet},
147-
},
148-
},
149-
{
150-
name: "should not fail if apiServerArguments not present",
151-
existingConfig: map[string]interface{}{
152-
"fakeconfig": "fake",
153-
},
154-
expectedConfig: map[string]interface{}{
155-
"fakeconfig": "fake",
156-
"apiServerArguments": map[string]any{"authorization-mode": expectedSet},
157-
},
158-
},
159-
} {
160-
name := tc.name + " when feature is "
161-
if on {
162-
name += "on"
163-
} else {
164-
name += "off"
165-
}
166-
t.Run(name, func(t *testing.T) {
167-
if err := SetAPIServerArgumentsToEnforceMinimumKubeletVersion(tc.existingConfig, on); err != nil {
168-
t.Fatal(err)
169-
}
170-
171-
if diff := cmp.Diff(tc.expectedConfig, tc.existingConfig); diff != "" {
172-
t.Errorf("unexpected config:\n%s", diff)
173-
}
174-
})
175-
}
176-
}
177-
}

0 commit comments

Comments
 (0)