From a7a341d18fad52888cf333a74591ec3f1505e000 Mon Sep 17 00:00:00 2001 From: deads2k Date: Wed, 16 Sep 2015 13:06:27 -0400 Subject: [PATCH] add cluster roles to diagnostics --- .../admin/policy/reconcile_clusterroles.go | 14 +-- pkg/cmd/experimental/diagnostics/cluster.go | 4 +- pkg/diagnostics/cluster/node_definitions.go | 2 +- pkg/diagnostics/cluster/registry.go | 2 +- pkg/diagnostics/cluster/roles.go | 96 +++++++++++++++++++ pkg/diagnostics/cluster/router.go | 2 +- pkg/diagnostics/cluster/util.go | 10 +- 7 files changed, 116 insertions(+), 14 deletions(-) create mode 100644 pkg/diagnostics/cluster/roles.go diff --git a/pkg/cmd/admin/policy/reconcile_clusterroles.go b/pkg/cmd/admin/policy/reconcile_clusterroles.go index 8126ec71a8b1..310080d7c825 100644 --- a/pkg/cmd/admin/policy/reconcile_clusterroles.go +++ b/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) { diff --git a/pkg/cmd/experimental/diagnostics/cluster.go b/pkg/cmd/experimental/diagnostics/cluster.go index 8679aafd5645..221dd74d70e2 100644 --- a/pkg/cmd/experimental/diagnostics/cluster.go +++ b/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) diff --git a/pkg/diagnostics/cluster/node_definitions.go b/pkg/diagnostics/cluster/node_definitions.go index 87f6c39966fb..4ea37f00eada 100644 --- a/pkg/diagnostics/cluster/node_definitions.go +++ b/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", }) diff --git a/pkg/diagnostics/cluster/registry.go b/pkg/diagnostics/cluster/registry.go index 0c9983bb0a1d..60fb15ec9d0f 100644 --- a/pkg/diagnostics/cluster/registry.go +++ b/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", diff --git a/pkg/diagnostics/cluster/roles.go b/pkg/diagnostics/cluster/roles.go new file mode 100644 index 000000000000..8d1d4074567b --- /dev/null +++ b/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 +} diff --git a/pkg/diagnostics/cluster/router.go b/pkg/diagnostics/cluster/router.go index fd3beb3b4ef4..7f604feeb117 100644 --- a/pkg/diagnostics/cluster/router.go +++ b/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", diff --git a/pkg/diagnostics/cluster/util.go b/pkg/diagnostics/cluster/util.go index fdc942ffa5d1..c33f96fd9617 100644 --- a/pkg/diagnostics/cluster/util.go +++ b/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 }