diff --git a/bindata/assets/config/defaultconfig.yaml b/bindata/assets/config/defaultconfig.yaml index 5fe580be4f..af053fce7b 100644 --- a/bindata/assets/config/defaultconfig.yaml +++ b/bindata/assets/config/defaultconfig.yaml @@ -34,11 +34,6 @@ apiServerArguments: - "true" anonymous-auth: - "true" - authorization-mode: - - Scope - - SystemMasters - - RBAC - - Node audit-log-format: - json audit-log-maxbackup: diff --git a/pkg/cmd/render/render.go b/pkg/cmd/render/render.go index f348373915..54ce85dead 100644 --- a/pkg/cmd/render/render.go +++ b/pkg/cmd/render/render.go @@ -358,10 +358,8 @@ func bootstrapDefaultConfig(featureGates featuregates.FeatureGate) ([]byte, erro } } - if featureGates.Enabled(features.FeatureGateMinimumKubeletVersion) { - if err := node.SetAPIServerArgumentsToEnforceMinimumKubeletVersion(defaultConfig, true); err != nil { - return nil, err - } + if err := node.AddAuthorizationModes(defaultConfig, featureGates.Enabled(features.FeatureGateMinimumKubeletVersion)); err != nil { + return nil, err } defaultConfigRaw, err := json.Marshal(defaultConfig) diff --git a/pkg/operator/configobservation/node/observe_authorization_mode.go b/pkg/operator/configobservation/node/observe_authorization_mode.go new file mode 100644 index 0000000000..15c99543bd --- /dev/null +++ b/pkg/operator/configobservation/node/observe_authorization_mode.go @@ -0,0 +1,69 @@ +package node + +import ( + "sort" + + "github.com/openshift/api/features" + "github.com/openshift/library-go/pkg/operator/configobserver" + "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" + "github.com/openshift/library-go/pkg/operator/events" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +var defaultAuthenticationModes = []string{ + "Node", + "RBAC", + "Scope", + "SystemMasters", +} + +type authorizationModeObserver struct { + featureGateAccessor featuregates.FeatureGateAccess + authModes []string +} + +func NewAuthorizationModeObserver(featureGateAccessor featuregates.FeatureGateAccess) configobserver.ObserveConfigFunc { + return (&authorizationModeObserver{ + featureGateAccessor: featureGateAccessor, + }).ObserveAuthorizationMode +} + +// ObserveAuthorizationMode watches the featuregate configuration and generates the apiServerArguments.authorization-mode +// It currently hardcodes the default set and adds MinimumKubeletVersion if the feature is set to on. +func (o *authorizationModeObserver) ObserveAuthorizationMode(genericListers configobserver.Listers, _ events.Recorder, existingConfig map[string]interface{}) (ret map[string]interface{}, errs []error) { + defer func() { + // Prune the observed config so that it only contains minimumKubeletVersion field. + ret = configobserver.Pruned(ret, authModePath) + }() + + ret = map[string]interface{}{} + if !o.featureGateAccessor.AreInitialFeatureGatesObserved() { + return existingConfig, nil + } + + featureGates, err := o.featureGateAccessor.CurrentFeatureGates() + if err != nil { + return existingConfig, append(errs, err) + } + + if err := AddAuthorizationModes(ret, featureGates.Enabled(features.FeatureGateMinimumKubeletVersion)); err != nil { + return existingConfig, append(errs, err) + } + return ret, nil +} + +// AddAuthorizationModes modifies the passed in config +// to add the "authorization-mode": "MinimumKubeletVersion" if the feature is on. If it's off, it +// removes it instead. +// This function assumes MinimumKubeletVersion auth mode isn't present by default, +// and should likely be removed when it is. +func AddAuthorizationModes(observedConfig map[string]interface{}, isMinimumKubeletVersionEnabled bool) error { + modes := defaultAuthenticationModes + if isMinimumKubeletVersionEnabled { + modes = append(modes, ModeMinimumKubeletVersion) + } + sort.Sort(sort.StringSlice(modes)) + + unstructured.RemoveNestedField(observedConfig, authModePath...) + return unstructured.SetNestedStringSlice(observedConfig, modes, authModePath...) +} diff --git a/pkg/operator/configobservation/node/observe_authorization_mode_test.go b/pkg/operator/configobservation/node/observe_authorization_mode_test.go new file mode 100644 index 0000000000..a7c7723a52 --- /dev/null +++ b/pkg/operator/configobservation/node/observe_authorization_mode_test.go @@ -0,0 +1,85 @@ +package node + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestAddAuthorizationModes(t *testing.T) { + for _, on := range []bool{false, true} { + expectedSet := []any{"Node", "RBAC", "Scope", "SystemMasters"} + if on { + expectedSet = append([]any{ModeMinimumKubeletVersion}, expectedSet...) + } + for _, tc := range []struct { + name string + existingConfig map[string]interface{} + expectedConfig map[string]interface{} + }{ + { + name: "should not fail if apiServerArguments not present", + existingConfig: map[string]interface{}{ + "fakeconfig": "fake", + }, + expectedConfig: map[string]interface{}{ + "fakeconfig": "fake", + "apiServerArguments": map[string]any{"authorization-mode": expectedSet}, + }, + }, + { + name: "should not fail if authorization-mode not present", + existingConfig: map[string]interface{}{ + "apiServerArguments": map[string]any{"fake": []any{"fake"}}, + }, + expectedConfig: map[string]interface{}{ + "apiServerArguments": map[string]any{"fake": []any{"fake"}, "authorization-mode": expectedSet}, + }, + }, + { + name: "should clobber value if not expected", + existingConfig: map[string]interface{}{ + "apiServerArguments": map[string]any{"authorization-mode": []any{"fake"}}, + }, + expectedConfig: map[string]interface{}{ + "apiServerArguments": map[string]any{"authorization-mode": expectedSet}, + }, + }, + { + name: "should not fail if MinimumKubeletVersion already present", + existingConfig: map[string]interface{}{ + "apiServerArguments": map[string]any{"authorization-mode": []any{"MinimumKubeletVersion"}}, + }, + expectedConfig: map[string]interface{}{ + "apiServerArguments": map[string]any{"authorization-mode": expectedSet}, + }, + }, + { + name: "should not fail if apiServerArguments not present", + existingConfig: map[string]interface{}{ + "fakeconfig": "fake", + }, + expectedConfig: map[string]interface{}{ + "fakeconfig": "fake", + "apiServerArguments": map[string]any{"authorization-mode": expectedSet}, + }, + }, + } { + name := tc.name + " when feature is " + if on { + name += "on" + } else { + name += "off" + } + t.Run(name, func(t *testing.T) { + if err := AddAuthorizationModes(tc.existingConfig, on); err != nil { + t.Fatal(err) + } + + if diff := cmp.Diff(tc.expectedConfig, tc.existingConfig); diff != "" { + t.Errorf("unexpected config:\n%s", diff) + } + }) + } + } +} diff --git a/pkg/operator/configobservation/node/observe_minimum_kubelet_version.go b/pkg/operator/configobservation/node/observe_minimum_kubelet_version.go index b00a1867f4..d9d4caecd9 100644 --- a/pkg/operator/configobservation/node/observe_minimum_kubelet_version.go +++ b/pkg/operator/configobservation/node/observe_minimum_kubelet_version.go @@ -1,10 +1,9 @@ package node import ( - "sort" - configv1 "github.com/openshift/api/config/v1" "github.com/openshift/api/features" + "github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/configobservation" "github.com/openshift/library-go/pkg/operator/configobserver" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/events" @@ -19,9 +18,6 @@ var ( authModeFlag = "authorization-mode" apiServerArgs = "apiServerArguments" authModePath = []string{apiServerArgs, authModeFlag} - // The default value for apiServerArguments.authorization-mode. - // Should be synced with bindata/assets/config/defaultconfig.yaml - DefaultAuthorizationModes = []string{"Scope", "SystemMasters", "RBAC", "Node"} ) type minimumKubeletVersionObserver struct { @@ -36,6 +32,11 @@ func NewMinimumKubeletVersionObserver(featureGateAccessor featuregates.FeatureGa // ObserveKubeletMinimumVersion watches the node configuration and generates the minimumKubeletVersion func (o *minimumKubeletVersionObserver) ObserveMinimumKubeletVersion(genericListers configobserver.Listers, _ events.Recorder, existingConfig map[string]interface{}) (ret map[string]interface{}, errs []error) { + defer func() { + // Prune the observed config so that it only contains minimumKubeletVersion field. + ret = configobserver.Pruned(ret, []string{minimumKubeletVersionConfigPath}) + }() + ret = map[string]interface{}{} if !o.featureGateAccessor.AreInitialFeatureGatesObserved() { return existingConfig, nil @@ -50,12 +51,7 @@ func (o *minimumKubeletVersionObserver) ObserveMinimumKubeletVersion(genericList return existingConfig, nil } - defer func() { - // Prune the observed config so that it only contains minimumKubeletVersion field. - ret = configobserver.Pruned(ret, []string{minimumKubeletVersionConfigPath}) - }() - - nodeLister := genericListers.(NodeLister) + nodeLister := genericListers.(configobservation.Listers) configNode, err := nodeLister.NodeLister().Get("cluster") // we got an error so without the node object we are not able to determine minimumKubeletVersion if err != nil { @@ -82,51 +78,3 @@ func (o *minimumKubeletVersionObserver) ObserveMinimumKubeletVersion(genericList return ret, errs } - -type authorizationModeObserver struct { - featureGateAccessor featuregates.FeatureGateAccess -} - -func NewAuthorizationModeObserver(featureGateAccessor featuregates.FeatureGateAccess) configobserver.ObserveConfigFunc { - return (&authorizationModeObserver{ - featureGateAccessor: featureGateAccessor, - }).ObserveAuthorizationMode -} - -// ObserveAuthorizationMode watches the featuregate configuration and generates the apiServerArguments.authorization-mode -// It currently hardcodes the default set and adds MinimumKubeletVersion if the feature is set to on. -func (o *authorizationModeObserver) ObserveAuthorizationMode(genericListers configobserver.Listers, _ events.Recorder, existingConfig map[string]interface{}) (ret map[string]interface{}, errs []error) { - ret = map[string]interface{}{} - if !o.featureGateAccessor.AreInitialFeatureGatesObserved() { - return existingConfig, nil - } - - featureGates, err := o.featureGateAccessor.CurrentFeatureGates() - if err != nil { - return existingConfig, append(errs, err) - } - - defer func() { - // Prune the observed config so that it only contains minimumKubeletVersion field. - ret = configobserver.Pruned(ret, authModePath) - }() - - if err := SetAPIServerArgumentsToEnforceMinimumKubeletVersion(ret, featureGates.Enabled(features.FeatureGateMinimumKubeletVersion)); err != nil { - return existingConfig, append(errs, err) - } - return ret, nil -} - -// SetAPIServerArgumentsToEnforceMinimumKubeletVersion modifies the passed in config -// to add the "authorization-mode": "MinimumKubeletVersion" if the feature is on. If it's off, it -// removes it instead. -func SetAPIServerArgumentsToEnforceMinimumKubeletVersion(newConfig map[string]interface{}, on bool) error { - defaultSet := DefaultAuthorizationModes - if on { - defaultSet = append(defaultSet, ModeMinimumKubeletVersion) - } - sort.Sort(sort.StringSlice(defaultSet)) - - unstructured.RemoveNestedField(newConfig, authModePath...) - return unstructured.SetNestedStringSlice(newConfig, defaultSet, authModePath...) -} diff --git a/pkg/operator/configobservation/node/observe_minimum_kubelet_version_test.go b/pkg/operator/configobservation/node/observe_minimum_kubelet_version_test.go index e76beaa193..e79df035cd 100644 --- a/pkg/operator/configobservation/node/observe_minimum_kubelet_version_test.go +++ b/pkg/operator/configobservation/node/observe_minimum_kubelet_version_test.go @@ -8,6 +8,7 @@ import ( configv1 "github.com/openshift/api/config/v1" "github.com/openshift/api/features" configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" + "github.com/openshift/cluster-kube-apiserver-operator/pkg/operator/configobservation" "github.com/openshift/library-go/pkg/operator/configobserver/featuregates" "github.com/openshift/library-go/pkg/operator/events" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -75,8 +76,8 @@ func TestObserveKubeletMinimumVersion(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, Spec: configv1.NodeSpec{MinimumKubeletVersion: test.minimumKubeletVersion}, }) - listers := testLister{ - nodeLister: configlistersv1.NewNodeLister(configNodeIndexer), + listers := configobservation.Listers{ + NodeLister_: configlistersv1.NewNodeLister(configNodeIndexer), } fg := featuregates.NewHardcodedFeatureGateAccess([]configv1.FeatureGateName{features.FeatureGateMinimumKubeletVersion}, []configv1.FeatureGateName{}) @@ -97,81 +98,3 @@ func TestObserveKubeletMinimumVersion(t *testing.T) { }) } } - -func TestSetAPIServerArgumentsToEnforceMinimumKubeletVersion(t *testing.T) { - for _, on := range []bool{false, true} { - expectedSet := []any{"Node", "RBAC", "Scope", "SystemMasters"} - if on { - expectedSet = append([]any{ModeMinimumKubeletVersion}, expectedSet...) - } - for _, tc := range []struct { - name string - existingConfig map[string]interface{} - expectedConfig map[string]interface{} - }{ - { - name: "should not fail if apiServerArguments not present", - existingConfig: map[string]interface{}{ - "fakeconfig": "fake", - }, - expectedConfig: map[string]interface{}{ - "fakeconfig": "fake", - "apiServerArguments": map[string]any{"authorization-mode": expectedSet}, - }, - }, - { - name: "should not fail if authorization-mode not present", - existingConfig: map[string]interface{}{ - "apiServerArguments": map[string]any{"fake": []any{"fake"}}, - }, - expectedConfig: map[string]interface{}{ - "apiServerArguments": map[string]any{"fake": []any{"fake"}, "authorization-mode": expectedSet}, - }, - }, - { - name: "should clobber value if not expected", - existingConfig: map[string]interface{}{ - "apiServerArguments": map[string]any{"authorization-mode": []any{"fake"}}, - }, - expectedConfig: map[string]interface{}{ - "apiServerArguments": map[string]any{"authorization-mode": expectedSet}, - }, - }, - { - name: "should not fail if MinimumKubeletVersion already present", - existingConfig: map[string]interface{}{ - "apiServerArguments": map[string]any{"authorization-mode": []any{"MinimumKubeletVersion"}}, - }, - expectedConfig: map[string]interface{}{ - "apiServerArguments": map[string]any{"authorization-mode": expectedSet}, - }, - }, - { - name: "should not fail if apiServerArguments not present", - existingConfig: map[string]interface{}{ - "fakeconfig": "fake", - }, - expectedConfig: map[string]interface{}{ - "fakeconfig": "fake", - "apiServerArguments": map[string]any{"authorization-mode": expectedSet}, - }, - }, - } { - name := tc.name + " when feature is " - if on { - name += "on" - } else { - name += "off" - } - t.Run(name, func(t *testing.T) { - if err := SetAPIServerArgumentsToEnforceMinimumKubeletVersion(tc.existingConfig, on); err != nil { - t.Fatal(err) - } - - if diff := cmp.Diff(tc.expectedConfig, tc.existingConfig); diff != "" { - t.Errorf("unexpected config:\n%s", diff) - } - }) - } - } -}