separate plugin configuration function and add tests

Signed-off-by: Bryce Palmer <[email protected]>

Author: everettraven

openshift-kube-apiserver/admission/authorization/restrictusers/restrictusers.go

@@ -30,20 +30,21 @@ import (
 const RestrictSubjectBindingsPluginName = "authorization.openshift.io/RestrictSubjectBindings"
 
 func Register(plugins *admission.Plugins) {
-	plugins.Register(RestrictSubjectBindingsPluginName,
-		func(config io.Reader) (admission.Interface, error) {
-			cfg, err := readConfig(config)
-			if err != nil {
-				return nil, err
-			}
+	plugins.Register(RestrictSubjectBindingsPluginName, pluginForConfig)
+}
 
-			if cfg != nil && cfg.OpenShiftOAuthDesiredState == v1alpha1.OpenShiftOAuthStateNotDesired {
-				klog.Infof("Admission plugin %q configured to expect the OpenShift oauth-apiserver as not being available. This is effectively the same as disabling the plugin, so it will be disabled.", RestrictSubjectBindingsPluginName)
-				return nil, nil
-			}
+func pluginForConfig(config io.Reader) (admission.Interface, error) {
+	cfg, err := readConfig(config)
+	if err != nil {
+		return nil, err
+	}
+
+	if cfg != nil && cfg.OpenShiftOAuthDesiredState == v1alpha1.OpenShiftOAuthStateNotDesired {
+		klog.Infof("Admission plugin %q configured to expect the OpenShift oauth-apiserver as not being available. This is effectively the same as disabling the plugin, so it will be disabled.", RestrictSubjectBindingsPluginName)
+		return nil, nil
+	}
 
-			return NewRestrictUsersAdmission()
-		})
+	return NewRestrictUsersAdmission()
 }
 
 func readConfig(reader io.Reader) (*v1alpha1.RestrictSubjectBindingsAdmissionConfig, error) {

openshift-kube-apiserver/admission/authorization/restrictusers/restrictusers_test.go

@@ -3,10 +3,12 @@ package restrictusers
 import (
 	"context"
 	"fmt"
+	"io"
 	"strings"
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -402,3 +404,74 @@ func TestAdmission(t *testing.T) {
 		}
 	}
 }
+
+func TestPluginForConfig(t *testing.T) {
+	testcases := []struct {
+		name           string
+		config         string
+		expectedErr    string
+		expectedPlugin admission.Interface
+	}{
+		{
+			name: "no config, no err, expect plugin",
+			expectedPlugin: func() admission.Interface {
+				plugin, _ := NewRestrictUsersAdmission()
+				return plugin
+			}(),
+		},
+		{
+			name: "config sets openshiftOAuthDesiredState to NotDesired, no err, nil plugin",
+			config: `apiVersion: authorization.openshift.io/v1alpha1
+kind: RestrictSubjectBindingsAdmissionConfig
+openshiftOAuthDesiredState: NotDesired
+            `,
+			expectedPlugin: nil,
+		},
+		{
+			name: "config sets openshiftOAuthDesiredState to Desired, no err, expect plugin",
+			config: `apiVersion: authorization.openshift.io/v1alpha1
+kind: RestrictSubjectBindingsAdmissionConfig
+openshiftOAuthDesiredState: Desired
+            `,
+			expectedPlugin: func() admission.Interface {
+				plugin, _ := NewRestrictUsersAdmission()
+				return plugin
+			}(),
+		},
+		{
+			name: "config sets openshiftOAuthDesiredState to invalid value, err, nil plugin",
+			config: `apiVersion: authorization.openshift.io/v1alpha1
+kind: RestrictSubjectBindingsAdmissionConfig
+openshiftOAuthDesiredState: FooBar
+            `,
+			expectedPlugin: nil,
+			expectedErr:    "config is invalid, openshiftOAuthDesiredState must be one of Desired,NotDesired",
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			var reader io.Reader
+			if len(tc.config) > 0 {
+				reader = strings.NewReader(tc.config)
+			}
+
+			plugin, err := pluginForConfig(reader)
+			switch {
+			case len(tc.expectedErr) == 0 && err == nil:
+			case len(tc.expectedErr) == 0 && err != nil:
+				t.Errorf("%s: unexpected error: %v", tc.name, err)
+			case len(tc.expectedErr) != 0 && err == nil:
+				t.Errorf("%s: missing error: %v", tc.name, tc.expectedErr)
+			case len(tc.expectedErr) != 0 && err != nil &&
+				!strings.Contains(err.Error(), tc.expectedErr):
+				t.Errorf("%s: missing error: expected %v, got %v",
+					tc.name, tc.expectedErr, err)
+			}
+
+			if !equality.Semantic.DeepEqual(tc.expectedPlugin, plugin) {
+				t.Errorf("plugin does not match. expected %v, got %v", tc.expectedPlugin, plugin)
+			}
+		})
+	}
+}