Skip to content

Commit

Permalink
observeAuthMode: drop authorization-mode from default config and spec…
Browse files Browse the repository at this point in the history
…ify strictly with observer

Signed-off-by: Peter Hunt <[email protected]>
  • Loading branch information
haircommander committed Dec 9, 2024
1 parent 0c32bfb commit 3b1fb7d
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 148 deletions.
5 changes: 0 additions & 5 deletions bindata/assets/config/defaultconfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ apiServerArguments:
- "true"
anonymous-auth:
- "true"
authorization-mode:
- Scope
- SystemMasters
- RBAC
- Node
audit-log-format:
- json
audit-log-maxbackup:
Expand Down
6 changes: 2 additions & 4 deletions pkg/cmd/render/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
69 changes: 69 additions & 0 deletions pkg/operator/configobservation/node/observe_authorization_mode.go
Original file line number Diff line number Diff line change
@@ -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...)
}
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
}
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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...)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{})
Expand All @@ -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)
}
})
}
}
}

0 comments on commit 3b1fb7d

Please sign in to comment.