add cluster roles to diagnostics

pkg/cmd/admin/policy/reconcile_clusterroles.go

@@ -21,7 +21,7 @@ import (
 // ReconcileClusterRolesRecommendedName is the recommended command name
 const ReconcileClusterRolesRecommendedName = "reconcile-cluster-roles"
 
-type reconcileClusterOptions struct {
+type ReconcileClusterRolesOptions struct {
 	Confirmed bool
 	Union     bool
 
@@ -53,7 +53,7 @@ You can see which cluster role have recommended changed by choosing an output ty
 
 // NewCmdReconcileClusterRoles implements the OpenShift cli reconcile-cluster-roles command
 func NewCmdReconcileClusterRoles(name, fullName string, f *clientcmd.Factory, out io.Writer) *cobra.Command {
-	o := &reconcileClusterOptions{Out: out}
+	o := &ReconcileClusterRolesOptions{Out: out}
 
 	cmd := &cobra.Command{
 		Use:     name,
@@ -84,7 +84,7 @@ func NewCmdReconcileClusterRoles(name, fullName string, f *clientcmd.Factory, ou
 	return cmd
 }
 
-func (o *reconcileClusterOptions) Complete(cmd *cobra.Command, f *clientcmd.Factory, args []string) error {
+func (o *ReconcileClusterRolesOptions) Complete(cmd *cobra.Command, f *clientcmd.Factory, args []string) error {
 	if len(args) != 0 {
 		return kcmdutil.UsageError(cmd, "no arguments are allowed")
 	}
@@ -100,7 +100,7 @@ func (o *reconcileClusterOptions) Complete(cmd *cobra.Command, f *clientcmd.Fact
 	return nil
 }
 
-func (o *reconcileClusterOptions) Validate() error {
+func (o *ReconcileClusterRolesOptions) Validate() error {
 	if o.RoleClient == nil {
 		return errors.New("a role client is required")
 	}
@@ -111,7 +111,7 @@ func (o *reconcileClusterOptions) Validate() error {
 }
 
 // RunReconcileClusterRoles contains all the necessary functionality for the OpenShift cli reconcile-cluster-roles command
-func (o *reconcileClusterOptions) RunReconcileClusterRoles(cmd *cobra.Command, f *clientcmd.Factory) error {
+func (o *ReconcileClusterRolesOptions) RunReconcileClusterRoles(cmd *cobra.Command, f *clientcmd.Factory) error {
 	changedClusterRoles, err := o.ChangedClusterRoles()
 	if err != nil {
 		return err
@@ -141,7 +141,7 @@ func (o *reconcileClusterOptions) RunReconcileClusterRoles(cmd *cobra.Command, f
 
 // ChangedClusterRoles returns the roles that must be created and/or updated to
 // match the recommended bootstrap policy
-func (o *reconcileClusterOptions) ChangedClusterRoles() ([]*authorizationapi.ClusterRole, error) {
+func (o *ReconcileClusterRolesOptions) ChangedClusterRoles() ([]*authorizationapi.ClusterRole, error) {
 	changedRoles := []*authorizationapi.ClusterRole{}
 
 	bootstrapClusterRoles := bootstrappolicy.GetBootstrapClusterRoles()
@@ -170,7 +170,7 @@ func (o *reconcileClusterOptions) ChangedClusterRoles() ([]*authorizationapi.Clu
 }
 
 // ReplaceChangedRoles will reconcile all the changed roles back to the recommended bootstrap policy
-func (o *reconcileClusterOptions) ReplaceChangedRoles(changedRoles []*authorizationapi.ClusterRole) error {
+func (o *ReconcileClusterRolesOptions) ReplaceChangedRoles(changedRoles []*authorizationapi.ClusterRole) error {
 	for i := range changedRoles {
 		role, err := o.RoleClient.Get(changedRoles[i].Name)
 		if err != nil && !kapierrors.IsNotFound(err) {

pkg/cmd/experimental/diagnostics/cluster.go

@@ -21,7 +21,7 @@ import (
 var (
 	// availableClusterDiagnostics contains the names of cluster diagnostics that can be executed
 	// during a single run of diagnostics. Add more diagnostics to the list as they are defined.
-	availableClusterDiagnostics = util.NewStringSet(clustdiags.NodeDefinitionsName, clustdiags.ClusterRegistryName, clustdiags.ClusterRouterName)
+	availableClusterDiagnostics = util.NewStringSet(clustdiags.NodeDefinitionsName, clustdiags.ClusterRegistryName, clustdiags.ClusterRouterName, clustdiags.ClusterRolesName)
 )
 
 // buildClusterDiagnostics builds cluster Diagnostic objects if a cluster-admin client can be extracted from the rawConfig passed in.
@@ -52,6 +52,8 @@ func (o DiagnosticsOptions) buildClusterDiagnostics(rawConfig *clientcmdapi.Conf
 			diagnostics = append(diagnostics, &clustdiags.ClusterRegistry{kclusterClient, clusterClient})
 		case clustdiags.ClusterRouterName:
 			diagnostics = append(diagnostics, &clustdiags.ClusterRouter{kclusterClient, clusterClient})
+		case clustdiags.ClusterRolesName:
+			diagnostics = append(diagnostics, &clustdiags.ClusterRoles{clusterClient, clusterClient})
 
 		default:
 			return nil, false, fmt.Errorf("unknown diagnostic: %v", diagnosticName)

pkg/diagnostics/cluster/node_definitions.go

@@ -67,7 +67,7 @@ func (d *NodeDefinitions) CanRun() (bool, error) {
 	if d.KubeClient == nil || d.OsClient == nil {
 		return false, errors.New("must have kube and os client")
 	}
-	can, err := adminCan(d.OsClient, authorizationapi.AuthorizationAttributes{
+	can, err := userCan(d.OsClient, authorizationapi.AuthorizationAttributes{
 		Verb:     "list",
 		Resource: "nodes",
 	})

pkg/diagnostics/cluster/registry.go

@@ -140,7 +140,7 @@ func (d *ClusterRegistry) CanRun() (bool, error) {
 	if d.OsClient == nil || d.KubeClient == nil {
 		return false, fmt.Errorf("must have kube and os clients")
 	}
-	return adminCan(d.OsClient, authorizationapi.AuthorizationAttributes{
+	return userCan(d.OsClient, authorizationapi.AuthorizationAttributes{
 		Namespace:    kapi.NamespaceDefault,
 		Verb:         "get",
 		Resource:     "services",

pkg/diagnostics/cluster/roles.go

@@ -0,0 +1,96 @@
+package cluster
+
+import (
+	"fmt"
+	"io/ioutil"
+
+	kerrs "k8s.io/kubernetes/pkg/api/errors"
+
+	authorizationapi "github.com/openshift/origin/pkg/authorization/api"
+	"github.com/openshift/origin/pkg/authorization/rulevalidation"
+	osclient "github.com/openshift/origin/pkg/client"
+	policycmd "github.com/openshift/origin/pkg/cmd/admin/policy"
+	"github.com/openshift/origin/pkg/diagnostics/types"
+)
+
+// ClusterRoles is a Diagnostic to check that the default cluster roles match expectations
+type ClusterRoles struct {
+	ClusterRolesClient osclient.ClusterRolesInterface
+	SARClient          osclient.SubjectAccessReviews
+}
+
+const (
+	ClusterRolesName = "ClusterRoles"
+)
+
+func (d *ClusterRoles) Name() string {
+	return ClusterRolesName
+}
+
+func (d *ClusterRoles) Description() string {
+	return "Check that the ClusterRoles are up-to-date"
+}
+
+func (d *ClusterRoles) CanRun() (bool, error) {
+	if d.ClusterRolesClient == nil {
+		return false, fmt.Errorf("must have client.ClusterRolesInterface")
+	}
+	if d.SARClient == nil {
+		return false, fmt.Errorf("must have client.SubjectAccessReviews")
+	}
+
+	return userCan(d.SARClient, authorizationapi.AuthorizationAttributes{
+		Verb:     "list",
+		Resource: "clusterroles",
+	})
+}
+
+func (d *ClusterRoles) Check() types.DiagnosticResult {
+	r := types.NewDiagnosticResult(ClusterRolesName)
+
+	reconcileOptions := &policycmd.ReconcileClusterRolesOptions{
+		Confirmed:  false,
+		Union:      false,
+		Out:        ioutil.Discard,
+		RoleClient: d.ClusterRolesClient.ClusterRoles(),
+	}
+
+	changedClusterRoles, err := reconcileOptions.ChangedClusterRoles()
+	if err != nil {
+		r.Error("CRD1000", err, fmt.Sprintf("Error inspecting ClusterRoles: %v", err))
+	}
+
+	// success
+	if len(changedClusterRoles) == 0 {
+		return r
+	}
+
+	for _, changedClusterRole := range changedClusterRoles {
+		actualClusterRole, err := d.ClusterRolesClient.ClusterRoles().Get(changedClusterRole.Name)
+		if kerrs.IsNotFound(err) {
+			r.Error("CRD1002", nil, fmt.Sprintf("clusterrole/%s is missing.\n\nUse the `oadm policy reconcile-cluster-roles` command to create the role.", changedClusterRole.Name))
+			continue
+		}
+		if err != nil {
+			r.Error("CRD1001", err, fmt.Sprintf("Unable to get clusterrole/%s: %v", changedClusterRole.Name, err))
+		}
+
+		_, missingRules := rulevalidation.Covers(actualClusterRole.Rules, changedClusterRole.Rules)
+		if len(missingRules) == 0 {
+			r.Warn("CRD1003", nil, fmt.Sprintf("clusterrole/%s has changed, but the existing role has more permissions than the new role.\n\nUse the `oadm policy reconcile-cluster-roles` command to update the role to reduce permissions.", changedClusterRole.Name))
+			_, extraRules := rulevalidation.Covers(changedClusterRole.Rules, actualClusterRole.Rules)
+			for _, extraRule := range extraRules {
+				r.Info("CRD1008", fmt.Sprintf("clusterrole/%s has extra permission %v.", changedClusterRole.Name, extraRule))
+			}
+			continue
+		}
+
+		r.Error("CRD1005", nil, fmt.Sprintf("clusterrole/%s has changed and the existing role does not have enough permissions.\n\nUse the `oadm policy reconcile-cluster-roles` command to update the role.", changedClusterRole.Name))
+		for _, missingRule := range missingRules {
+			r.Info("CRD1007", fmt.Sprintf("clusterrole/%s is missing permission %v.", changedClusterRole.Name, missingRule))
+		}
+		r.Debug("CRD1006", fmt.Sprintf("clusterrole/%s is now %v.", changedClusterRole.Name, changedClusterRole))
+	}
+
+	return r
+}

pkg/diagnostics/cluster/router.go

@@ -95,7 +95,7 @@ func (d *ClusterRouter) CanRun() (bool, error) {
 	if d.KubeClient == nil || d.OsClient == nil {
 		return false, errors.New("must have kube and os client")
 	}
-	can, err := adminCan(d.OsClient, authorizationapi.AuthorizationAttributes{
+	can, err := userCan(d.OsClient, authorizationapi.AuthorizationAttributes{
 		Namespace:    kapi.NamespaceDefault,
 		Verb:         "get",
 		Resource:     "dc",

pkg/diagnostics/cluster/util.go

@@ -5,11 +5,15 @@ import (
 	osclient "github.com/openshift/origin/pkg/client"
 )
 
-func adminCan(client *osclient.Client, action authorizationapi.AuthorizationAttributes) (bool, error) {
-	if resp, err := client.SubjectAccessReviews().Create(&authorizationapi.SubjectAccessReview{Action: action}); err != nil {
+func userCan(sarClient osclient.SubjectAccessReviews, action authorizationapi.AuthorizationAttributes) (bool, error) {
+	resp, err := sarClient.SubjectAccessReviews().Create(&authorizationapi.SubjectAccessReview{Action: action})
+	if err != nil {
 		return false, err
-	} else if resp.Allowed {
+	}
+
+	if resp.Allowed {
 		return true, nil
 	}
+
 	return false, nil
 }