Skip to content

Commit

Permalink
configobservation/auth: add config observer for rolebindingrestrictio…
Browse files Browse the repository at this point in the history
…n plugins
  • Loading branch information
liouk committed Feb 14, 2025
1 parent df07f8b commit 3624731
Show file tree
Hide file tree
Showing 3 changed files with 211 additions and 0 deletions.
62 changes: 62 additions & 0 deletions pkg/operator/configobservation/auth/rolebindingrestrictions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package auth

import (
configv1 "github.com/openshift/api/config/v1"
"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/events"

"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/sets"
)

var (
disableAdmissionPluginsPath = []string{"apiServerArguments", "disable-admission-plugins"}

rbrPlugins = []string{
"authorization.openshift.io/RestrictSubjectBindings",
"authorization.openshift.io/ValidateRoleBindingRestriction",
}
)

// ObserveRoleBindingRestrictionPlugins observes the cluster authentication type and explicitly disables
// the plugins related to the RoleBindingRestriction API, when authentication type is anything other than
// the built-in OAuth stack (i.e. .Spec.Type of `authentications.config.openshift.io/cluster` is neither
// "IntegratedOAuth" nor the empty string).
//
// The observer relies on the plugins to be enabled in the default kube-apiserver config, and therefore
// will not explicitly enable them, but only disable them when necessary.
func ObserveRoleBindingRestrictionPlugins(genericListers configobserver.Listers, recorder events.Recorder, existingConfig map[string]interface{}) (map[string]interface{}, []error) {
observedConfig := map[string]interface{}{}
listers := genericListers.(configobservation.Listers)

auth, err := listers.AuthConfigLister.Get("cluster")
if errors.IsNotFound(err) {
recorder.Eventf("ObserveRoleBindingRestrictions", "authentications.config.openshift.io/cluster: not found")
return observedConfig, nil
} else if err != nil {
return existingConfig, []error{err}
}

if auth.Spec.Type == configv1.AuthenticationTypeIntegratedOAuth || len(auth.Spec.Type) == 0 {
// the plugins will be enabled by default
return existingConfig, nil
}

// the merger used to merge the observed configs will not merge slices, so we have to do it manually
// if there are existing elements in the --disable-admission-plugins slice
alreadyDisabled, _, err := unstructured.NestedStringSlice(existingConfig, disableAdmissionPluginsPath...)
if err != nil {
return existingConfig, []error{err}
}
alreadyDisabledSet := sets.NewString(alreadyDisabled...)
alreadyDisabledSet.Insert(rbrPlugins...)

err = unstructured.SetNestedStringSlice(observedConfig, alreadyDisabledSet.List(), disableAdmissionPluginsPath...)
if err != nil {
return existingConfig, []error{err}
}

return observedConfig, nil
}
148 changes: 148 additions & 0 deletions pkg/operator/configobservation/auth/rolebindingrestrictions_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
package auth

import (
"testing"

configv1 "github.com/openshift/api/config/v1"
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/events"

"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/client-go/tools/cache"
"k8s.io/utils/clock"
"k8s.io/utils/ptr"
)

func TestObserveRoleBindingRestrictions(t *testing.T) {
for _, tt := range []struct {
name string
authType *configv1.AuthenticationType
existingConfig map[string]interface{}

expectEvents bool
expectErrors bool
expectedConfig map[string]interface{}
}{
{
name: "auth resource not found",
authType: nil,
existingConfig: map[string]interface{}{"key": "value"},
expectEvents: true,
expectErrors: false,
expectedConfig: nil,
},
{
name: "auth type IntegratedOAuth without other disabled plugins in config",
authType: ptr.To(configv1.AuthenticationTypeIntegratedOAuth),
existingConfig: nil,
expectEvents: false,
expectErrors: false,
expectedConfig: nil,
},
{
name: "auth type empty without other disabled plugins in config",
authType: ptr.To(configv1.AuthenticationType("")),
existingConfig: nil,
expectEvents: false,
expectErrors: false,
expectedConfig: nil,
},
{
name: "auth type OIDC without other disabled plugins in config",
authType: ptr.To(configv1.AuthenticationTypeOIDC),
existingConfig: nil,
expectEvents: false,
expectErrors: false,
expectedConfig: newTestConfig([]string{rbrPlugins[0], rbrPlugins[1]}),
},
{
name: "auth type None without other disabled plugins in config",
authType: ptr.To(configv1.AuthenticationTypeNone),
existingConfig: nil,
expectEvents: false,
expectErrors: false,
expectedConfig: newTestConfig([]string{rbrPlugins[0], rbrPlugins[1]}),
},
{
name: "auth type IntegratedOAuth with other disabled plugins in config",
authType: ptr.To(configv1.AuthenticationTypeIntegratedOAuth),
existingConfig: newTestConfig([]string{"off1", "off2"}),
expectEvents: false,
expectErrors: false,
expectedConfig: newTestConfig([]string{"off1", "off2"}),
},
{
name: "auth type empty with other disabled plugins in config",
authType: ptr.To(configv1.AuthenticationType("")),
existingConfig: newTestConfig([]string{"off1", "off2"}),
expectEvents: false,
expectErrors: false,
expectedConfig: newTestConfig([]string{"off1", "off2"}),
},
{
name: "auth type OIDC with other disabled plugins in config",
authType: ptr.To(configv1.AuthenticationTypeOIDC),
existingConfig: newTestConfig([]string{"off1", "off2"}),
expectEvents: false,
expectErrors: false,
expectedConfig: newTestConfig([]string{rbrPlugins[0], rbrPlugins[1], "off1", "off2"}),
},
{
name: "auth type None with other disabled plugins in config",
authType: ptr.To(configv1.AuthenticationTypeNone),
existingConfig: newTestConfig([]string{"off1", "off2"}),
expectEvents: false,
expectErrors: false,
expectedConfig: newTestConfig([]string{rbrPlugins[0], rbrPlugins[1], "off1", "off2"}),
},
} {
t.Run(tt.name, func(t *testing.T) {

indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
if tt.authType != nil {
indexer.Add(&configv1.Authentication{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster",
},
Spec: configv1.AuthenticationSpec{
Type: *tt.authType,
},
})
}

eventRecorder := events.NewInMemoryRecorder("externaloidctest", clock.RealClock{})
listers := configobservation.Listers{
AuthConfigLister: configlistersv1.NewAuthenticationLister(indexer),
}

actualConfig, actualErrs := ObserveRoleBindingRestrictionPlugins(listers, eventRecorder, tt.existingConfig)
if tt.expectErrors != (len(actualErrs) > 0) {
t.Errorf("expected errors: %v; got %v", tt.expectErrors, actualErrs)
}

if !equality.Semantic.DeepEqual(tt.expectedConfig, actualConfig) {
t.Errorf("unexpected config diff: %s", diff.ObjectReflectDiff(tt.expectedConfig, actualConfig))
}

if recordedEvents := eventRecorder.Events(); tt.expectEvents != (len(recordedEvents) > 0) {
t.Errorf("expected events: %v; got %v", tt.expectEvents, recordedEvents)
}
})
}
}

func newTestConfig(disabled []string) map[string]interface{} {
cfg := map[string]interface{}{}

if len(disabled) > 0 {
if err := unstructured.SetNestedStringSlice(cfg, disabled, "apiServerArguments", "disable-admission-plugins"); err != nil {
panic(err)
}
}

return cfg
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ func NewConfigObserver(operatorClient v1helpers.StaticPodOperatorClient, kubeInf
auth.ObserveAuthMetadata,
auth.ObserveServiceAccountIssuer,
auth.ObserveWebhookTokenAuthenticator,
auth.ObserveRoleBindingRestrictionPlugins,
auth.NewObservePodSecurityAdmissionEnforcementFunc(featureGateAccessor),
encryption.NewEncryptionConfigObserver(
operatorclient.TargetNamespace,
Expand Down

0 comments on commit 3624731

Please sign in to comment.