diff --git a/pkg/cmd/list.go b/pkg/cmd/list.go index 2b1ec68..fd0fe06 100644 --- a/pkg/cmd/list.go +++ b/pkg/cmd/list.go @@ -69,15 +69,15 @@ NONRESOURCEURL is a partial URL that starts with "/".` // Action represents an action a subject can be given permission to. type Action struct { - verb string - resource string - resourceName string - subResource string + Verb string + Resource string + ResourceName string + SubResource string - nonResourceURL string + NonResourceURL string - namespace string - allNamespaces bool + Namespace string + AllNamespaces bool } type resolvedAction struct { @@ -173,8 +173,8 @@ func NewWhoCanCommand(streams clioptions.IOStreams) (*cobra.Command, error) { }, } - cmd.Flags().String("subresource", "", "SubResource such as pod/log or deployment/scale") - cmd.Flags().BoolP("all-namespaces", "A", false, "If true, check for users that can do the specified action in any of the available namespaces") + cmd.Flags().String(subResourceFlag, "", "SubResource such as pod/log or deployment/scale") + cmd.Flags().BoolP(allNamespacesFlag, "A", false, "If true, check for users that can do the specified action in any of the available namespaces") flag.CommandLine.VisitAll(func(gf *flag.Flag) { cmd.Flags().AddGoFlag(gf) @@ -192,61 +192,61 @@ func ActionFrom(clientConfig clientcmd.ClientConfig, flags *pflag.FlagSet, args return } - action.verb = args[0] + action.Verb = args[0] if strings.HasPrefix(args[1], "/") { - action.nonResourceURL = args[1] - glog.V(3).Infof("Resolved nonResourceURL `%s`", action.nonResourceURL) + action.NonResourceURL = args[1] + glog.V(3).Infof("Resolved nonResourceURL `%s`", action.NonResourceURL) } else { resourceTokens := strings.SplitN(args[1], "/", 2) - action.resource = resourceTokens[0] + action.Resource = resourceTokens[0] if len(resourceTokens) > 1 { - action.resourceName = resourceTokens[1] - glog.V(3).Infof("Resolved resourceName `%s`", action.resourceName) + action.ResourceName = resourceTokens[1] + glog.V(3).Infof("Resolved resourceName `%s`", action.ResourceName) } } - action.subResource, err = flags.GetString(subResourceFlag) + action.SubResource, err = flags.GetString(subResourceFlag) if err != nil { return } - action.allNamespaces, err = flags.GetBool(allNamespacesFlag) + action.AllNamespaces, err = flags.GetBool(allNamespacesFlag) if err != nil { return } - if action.allNamespaces { - action.namespace = core.NamespaceAll - glog.V(3).Infof("Resolved namespace `%s` from --all-namespaces flag", action.namespace) + if action.AllNamespaces { + action.Namespace = core.NamespaceAll + glog.V(3).Infof("Resolved namespace `%s` from --all-namespaces flag", action.Namespace) return } - action.namespace, err = flags.GetString(namespaceFlag) + action.Namespace, err = flags.GetString(namespaceFlag) if err != nil { return } - if action.namespace != "" { - glog.V(3).Infof("Resolved namespace `%s` from --namespace flag", action.namespace) + if action.Namespace != "" { + glog.V(3).Infof("Resolved namespace `%s` from --namespace flag", action.Namespace) return } // Neither --all-namespaces nor --namespace flag was specified - action.namespace, _, err = clientConfig.Namespace() + action.Namespace, _, err = clientConfig.Namespace() if err != nil { err = fmt.Errorf("getting namespace from current context: %v", err) } - glog.V(3).Infof("Resolved namespace `%s` from current context", action.namespace) + glog.V(3).Infof("Resolved namespace `%s` from current context", action.Namespace) return } // Validate makes sure that the specified action is valid. func (w *WhoCan) validate(action Action) error { - if action.nonResourceURL != "" && action.subResource != "" { + if action.NonResourceURL != "" && action.SubResource != "" { return fmt.Errorf("--subresource cannot be used with NONRESOURCEURL") } - err := w.namespaceValidator.Validate(action.namespace) + err := w.namespaceValidator.Validate(action.Namespace) if err != nil { return fmt.Errorf("validating namespace: %v", err) } @@ -265,8 +265,8 @@ func (w *WhoCan) Check(action Action) (roleBindings []rbac.RoleBinding, clusterR resolvedAction := resolvedAction{Action: action} - if action.resource != "" { - resolvedAction.gr, err = w.resourceResolver.Resolve(action.verb, action.resource, action.subResource) + if action.Resource != "" { + resolvedAction.gr, err = w.resourceResolver.Resolve(action.Verb, action.Resource, action.SubResource) if err != nil { err = fmt.Errorf("resolving resource: %v", err) return @@ -316,7 +316,7 @@ func (w *WhoCan) CheckAPIAccess(action Action) ([]string, error) { var warnings []string // Determine which checks need to be executed. - if action.namespace == "" { + if action.Namespace == "" { checks = append(checks, check{"list", "namespaces", ""}) nsList, err := w.clientNamespace.List(meta.ListOptions{}) @@ -328,8 +328,8 @@ func (w *WhoCan) CheckAPIAccess(action Action) ([]string, error) { checks = append(checks, check{"list", "rolebindings", ns.Name}) } } else { - checks = append(checks, check{"list", "roles", action.namespace}) - checks = append(checks, check{"list", "rolebindings", action.namespace}) + checks = append(checks, check{"list", "roles", action.Namespace}) + checks = append(checks, check{"list", "rolebindings", action.Namespace}) } // Actually run the checks and collect warnings. @@ -356,7 +356,7 @@ func (w *WhoCan) CheckAPIAccess(action Action) ([]string, error) { // GetRolesFor returns a set of names of Roles matching the specified Action. func (w *WhoCan) getRolesFor(action resolvedAction) (roles, error) { - rl, err := w.clientRBAC.Roles(action.namespace).List(meta.ListOptions{}) + rl, err := w.clientRBAC.Roles(action.Namespace).List(meta.ListOptions{}) if err != nil { return nil, err } @@ -396,11 +396,11 @@ func (w *WhoCan) getClusterRolesFor(action resolvedAction) (clusterRoles, error) // GetRoleBindings returns the RoleBindings that refer to the given set of Role names or ClusterRole names. func (w *WhoCan) getRoleBindings(action resolvedAction, roleNames roles, clusterRoleNames clusterRoles) (roleBindings []rbac.RoleBinding, err error) { // TODO I'm wondering if GetRoleBindings should be invoked at all when the --all-namespaces flag is specified? - if action.namespace == core.NamespaceAll { + if action.Namespace == core.NamespaceAll { return } - list, err := w.clientRBAC.RoleBindings(action.namespace).List(meta.ListOptions{}) + list, err := w.clientRBAC.RoleBindings(action.Namespace).List(meta.ListOptions{}) if err != nil { return } @@ -452,12 +452,10 @@ func PrintChecks(out io.Writer, action Action, roleBindings []rbac.RoleBinding, wr := new(tabwriter.Writer) wr.Init(out, 0, 8, 2, ' ', 0) - actionStr := action.PrettyPrint() - - if action.resource != "" { + if action.Resource != "" { // NonResourceURL permissions can only be granted through ClusterRoles. Hence no point in printing RoleBindings section. if len(roleBindings) == 0 { - _, _ = fmt.Fprintf(out, "No subjects found with permissions to %s assigned through RoleBindings\n", actionStr) + _, _ = fmt.Fprintf(out, "No subjects found with permissions to %s assigned through RoleBindings\n", action) } else { _, _ = fmt.Fprintln(wr, "ROLEBINDING\tNAMESPACE\tSUBJECT\tTYPE\tSA-NAMESPACE") for _, rb := range roleBindings { @@ -471,7 +469,7 @@ func PrintChecks(out io.Writer, action Action, roleBindings []rbac.RoleBinding, } if len(clusterRoleBindings) == 0 { - _, _ = fmt.Fprintf(out, "No subjects found with permissions to %s assigned through ClusterRoleBindings\n", actionStr) + _, _ = fmt.Fprintf(out, "No subjects found with permissions to %s assigned through ClusterRoleBindings\n", action) } else { _, _ = fmt.Fprintln(wr, "CLUSTERROLEBINDING\tSUBJECT\tTYPE\tSA-NAMESPACE") for _, rb := range clusterRoleBindings { @@ -483,13 +481,13 @@ func PrintChecks(out io.Writer, action Action, roleBindings []rbac.RoleBinding, _ = wr.Flush() } -func (w Action) PrettyPrint() string { - if w.nonResourceURL != "" { - return fmt.Sprintf("%s %s", w.verb, w.nonResourceURL) +func (w Action) String() string { + if w.NonResourceURL != "" { + return fmt.Sprintf("%s %s", w.Verb, w.NonResourceURL) } - name := w.resourceName + name := w.ResourceName if name != "" { name = "/" + name } - return fmt.Sprintf("%s %s%s", w.verb, w.resource, name) + return fmt.Sprintf("%s %s%s", w.Verb, w.Resource, name) } diff --git a/pkg/cmd/list_test.go b/pkg/cmd/list_test.go index 46518ba..62c66a4 100644 --- a/pkg/cmd/list_test.go +++ b/pkg/cmd/list_test.go @@ -100,10 +100,10 @@ func TestActionFrom(t *testing.T) { args: []string{"list", "pods"}, flags: flags{namespace: "", allNamespaces: false}, expectedAction: Action{ - namespace: "foo", - verb: "list", - resource: "pods", - resourceName: "", + Namespace: "foo", + Verb: "list", + Resource: "pods", + ResourceName: "", }, }, { @@ -112,10 +112,10 @@ func TestActionFrom(t *testing.T) { flags: flags{namespace: "", allNamespaces: false}, args: []string{"list", "pods"}, expectedAction: Action{ - namespace: "", - verb: "list", - resource: "pods", - resourceName: "", + Namespace: "", + Verb: "list", + Resource: "pods", + ResourceName: "", }, expectedError: errors.New("getting namespace from current context: cannot open context"), }, @@ -124,11 +124,11 @@ func TestActionFrom(t *testing.T) { flags: flags{namespace: "", allNamespaces: true}, args: []string{"get", "service/mongodb"}, expectedAction: Action{ - allNamespaces: true, - namespace: core.NamespaceAll, - verb: "get", - resource: "service", - resourceName: "mongodb", + AllNamespaces: true, + Namespace: core.NamespaceAll, + Verb: "get", + Resource: "service", + ResourceName: "mongodb", }, }, { @@ -136,9 +136,9 @@ func TestActionFrom(t *testing.T) { flags: flags{namespace: "bar", allNamespaces: false}, args: []string{"delete", "pv"}, expectedAction: Action{ - namespace: "bar", - verb: "delete", - resource: "pv", + Namespace: "bar", + Verb: "delete", + Resource: "pv", }, }, { @@ -146,9 +146,9 @@ func TestActionFrom(t *testing.T) { flags: flags{namespace: "foo"}, args: []string{"get", "/logs"}, expectedAction: Action{ - namespace: "foo", - verb: "get", - nonResourceURL: "/logs", + Namespace: "foo", + Verb: "get", + NonResourceURL: "/logs", }, }, { @@ -236,9 +236,9 @@ func TestValidate(t *testing.T) { } action := Action{ - nonResourceURL: tt.nonResourceURL, - subResource: tt.subResource, - namespace: tt.namespace, + NonResourceURL: tt.nonResourceURL, + SubResource: tt.subResource, + Namespace: tt.namespace, } // when @@ -345,7 +345,7 @@ func TestWhoCan_CheckAPIAccess(t *testing.T) { policyRuleMatcher: policyRuleMatcher, } action := Action{ - namespace: tt.namespace, + Namespace: tt.namespace, } // when @@ -366,7 +366,7 @@ func TestWhoCan_GetRolesFor(t *testing.T) { policyRuleMatcher := new(policyRuleMatcherMock) client := fake.NewSimpleClientset() - action := resolvedAction{Action: Action{verb: "list", resource: "services"}} + action := resolvedAction{Action: Action{Verb: "list", Resource: "services"}} viewServicesRole := rbac.Role{ ObjectMeta: meta.ObjectMeta{ @@ -425,7 +425,7 @@ func TestWhoCan_GetClusterRolesFor(t *testing.T) { policyRuleMatcher := new(policyRuleMatcherMock) client := fake.NewSimpleClientset() - action := resolvedAction{Action: Action{verb: "get", resource: "/logs"}} + action := resolvedAction{Action: Action{Verb: "get", Resource: "/logs"}} getLogsRole := rbac.ClusterRole{ ObjectMeta: meta.ObjectMeta{ @@ -521,7 +521,7 @@ func TestWhoCan_GetRoleBindings(t *testing.T) { wc := WhoCan{ clientRBAC: client.RbacV1(), } - action := resolvedAction{Action: Action{namespace: namespace}} + action := resolvedAction{Action: Action{Namespace: namespace}} // when bindings, err := wc.getRoleBindings(action, roleNames, clusterRoleNames) @@ -690,10 +690,10 @@ Bob-and-Eve-can-view-pods Eve User // given var buf bytes.Buffer action := Action{ - verb: tt.verb, - resource: tt.resource, - nonResourceURL: tt.nonResourceURL, - resourceName: tt.resourceName, + Verb: tt.verb, + Resource: tt.resource, + NonResourceURL: tt.nonResourceURL, + ResourceName: tt.resourceName, } // when diff --git a/pkg/cmd/policy_rule_matcher.go b/pkg/cmd/policy_rule_matcher.go index 0cb282f..9b3a7ea 100644 --- a/pkg/cmd/policy_rule_matcher.go +++ b/pkg/cmd/policy_rule_matcher.go @@ -50,20 +50,20 @@ func (m *matcher) MatchesClusterRole(role rbac.ClusterRole, action resolvedActio // matches returns `true` if the given PolicyRule matches the specified Action, `false` otherwise. func (m *matcher) matches(rule rbac.PolicyRule, action resolvedAction) bool { - if action.nonResourceURL != "" { - return m.matchesVerb(rule, action.verb) && - m.matchesNonResourceURL(rule, action.nonResourceURL) + if action.NonResourceURL != "" { + return m.matchesVerb(rule, action.Verb) && + m.matchesNonResourceURL(rule, action.NonResourceURL) } resource := action.gr.Resource - if action.subResource != "" { - resource += "/" + action.subResource + if action.SubResource != "" { + resource += "/" + action.SubResource } - return m.matchesVerb(rule, action.verb) && + return m.matchesVerb(rule, action.Verb) && m.matchesResource(rule, resource) && m.matchesAPIGroup(rule, action.gr.Group) && - m.matchesResourceName(rule, action.resourceName) + m.matchesResourceName(rule, action.ResourceName) } func (m *matcher) matchesAPIGroup(rule rbac.PolicyRule, actionGroup string) bool { diff --git a/pkg/cmd/policy_rule_matcher_test.go b/pkg/cmd/policy_rule_matcher_test.go index f8a6a85..2ba139d 100644 --- a/pkg/cmd/policy_rule_matcher_test.go +++ b/pkg/cmd/policy_rule_matcher_test.go @@ -28,7 +28,7 @@ func TestMatcher_MatchesRole(t *testing.T) { } action := resolvedAction{ Action: Action{ - verb: "list", + Verb: "list", }, gr: schema.GroupResource{ Group: "extensions", @@ -60,8 +60,8 @@ func TestMatcher_MatchesClusterRole(t *testing.T) { } action := resolvedAction{ Action: Action{ - verb: "update", - subResource: "scale", + Verb: "update", + SubResource: "scale", }, gr: schema.GroupResource{ Group: "extensions", @@ -87,7 +87,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "A", action: resolvedAction{ - Action: Action{verb: "get"}, + Action: Action{Verb: "get"}, gr: servicesGR, }, rule: rbac.PolicyRule{ @@ -100,7 +100,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "B", action: resolvedAction{ - Action: Action{verb: "get"}, + Action: Action{Verb: "get"}, gr: servicesGR, }, rule: rbac.PolicyRule{ @@ -113,7 +113,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "C", action: resolvedAction{ - Action: Action{verb: "get"}, + Action: Action{Verb: "get"}, gr: servicesGR, }, rule: rbac.PolicyRule{ @@ -126,7 +126,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "D", action: resolvedAction{ - Action: Action{verb: "get", resourceName: "mongodb"}, + Action: Action{Verb: "get", ResourceName: "mongodb"}, gr: servicesGR, }, rule: rbac.PolicyRule{ @@ -139,7 +139,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "E", action: resolvedAction{ - Action: Action{verb: "get", resourceName: "mongodb"}, + Action: Action{Verb: "get", ResourceName: "mongodb"}, gr: servicesGR, }, rule: rbac.PolicyRule{ @@ -153,7 +153,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "F", action: resolvedAction{ - Action: Action{verb: "get", resourceName: "mongodb"}, + Action: Action{Verb: "get", ResourceName: "mongodb"}, gr: servicesGR, }, rule: rbac.PolicyRule{ @@ -167,7 +167,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "G", action: resolvedAction{ - Action: Action{verb: "get"}, + Action: Action{Verb: "get"}, gr: servicesGR, }, rule: rbac.PolicyRule{ @@ -181,7 +181,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "H", action: resolvedAction{ - Action: Action{verb: "get"}, + Action: Action{Verb: "get"}, gr: schema.GroupResource{Resource: "pods"}, }, rule: rbac.PolicyRule{ @@ -194,7 +194,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "I", action: resolvedAction{ - Action: Action{verb: "get"}, + Action: Action{Verb: "get"}, gr: schema.GroupResource{Resource: "persistentvolumes"}, }, rule: rbac.PolicyRule{ @@ -206,7 +206,7 @@ func TestMatcher_matches(t *testing.T) { }, { scenario: "J", - action: resolvedAction{Action: Action{verb: "get", nonResourceURL: "/logs"}}, + action: resolvedAction{Action: Action{Verb: "get", NonResourceURL: "/logs"}}, rule: rbac.PolicyRule{ Verbs: []string{"get"}, NonResourceURLs: []string{"/logs"}, @@ -215,7 +215,7 @@ func TestMatcher_matches(t *testing.T) { }, { scenario: "K", - action: resolvedAction{Action: Action{verb: "get", nonResourceURL: "/logs"}}, + action: resolvedAction{Action: Action{Verb: "get", NonResourceURL: "/logs"}}, rule: rbac.PolicyRule{ Verbs: []string{"post"}, NonResourceURLs: []string{"/logs"}, @@ -224,7 +224,7 @@ func TestMatcher_matches(t *testing.T) { }, { scenario: "L", - action: resolvedAction{Action: Action{verb: "get", nonResourceURL: "/logs"}}, + action: resolvedAction{Action: Action{Verb: "get", NonResourceURL: "/logs"}}, rule: rbac.PolicyRule{ Verbs: []string{"get"}, NonResourceURLs: []string{"/api"}, @@ -234,7 +234,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "Should return true when PolicyRule's APIGroup matches resolved resource's group", action: resolvedAction{ - Action: Action{verb: "get"}, + Action: Action{Verb: "get"}, gr: schema.GroupResource{Resource: "deployments", Group: "extensions"}, }, rule: rbac.PolicyRule{ @@ -247,7 +247,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "Should return true when PolicyRule's APIGroup matches all ('*') resource groups", action: resolvedAction{ - Action: Action{verb: "get"}, + Action: Action{Verb: "get"}, gr: schema.GroupResource{Resource: "pods", Group: "metrics.k8s.io"}, }, rule: rbac.PolicyRule{ @@ -260,7 +260,7 @@ func TestMatcher_matches(t *testing.T) { { scenario: "Should return false when PolicyRule's APIGroup doesn't match resolved resource's Group", action: resolvedAction{ - Action: Action{verb: "get"}, + Action: Action{Verb: "get"}, gr: schema.GroupResource{Resource: "pods", Group: "metrics.k8s.io"}, }, rule: rbac.PolicyRule{ diff --git a/pkg/cmd/resource_resolver_test.go b/pkg/cmd/resource_resolver_test.go index 56d31fa..1a18077 100644 --- a/pkg/cmd/resource_resolver_test.go +++ b/pkg/cmd/resource_resolver_test.go @@ -69,7 +69,7 @@ func TestResourceResolver_Resolve(t *testing.T) { }{ { scenario: "A", - action: Action{verb: "list", resource: "pods"}, + action: Action{Verb: "list", Resource: "pods"}, mappingResult: &mappingResult{ argGVR: schema.GroupVersionResource{Resource: "pods"}, returnGVR: podsGVR, @@ -78,7 +78,7 @@ func TestResourceResolver_Resolve(t *testing.T) { }, { scenario: "B", - action: Action{verb: "list", resource: "po"}, + action: Action{Verb: "list", Resource: "po"}, mappingResult: &mappingResult{ argGVR: schema.GroupVersionResource{Resource: "po"}, returnGVR: podsGVR, @@ -87,7 +87,7 @@ func TestResourceResolver_Resolve(t *testing.T) { }, { scenario: "C", - action: Action{verb: "eat", resource: "pods"}, + action: Action{Verb: "eat", Resource: "pods"}, mappingResult: &mappingResult{ argGVR: schema.GroupVersionResource{Resource: "pods"}, returnGVR: podsGVR, @@ -96,7 +96,7 @@ func TestResourceResolver_Resolve(t *testing.T) { }, { scenario: "D", - action: Action{verb: "list", resource: "deployments.extensions"}, + action: Action{Verb: "list", Resource: "deployments.extensions"}, mappingResult: &mappingResult{ argGVR: schema.GroupVersionResource{Group: "extensions", Version: "", Resource: "deployments"}, returnGVR: deploymentsGVR, @@ -105,7 +105,7 @@ func TestResourceResolver_Resolve(t *testing.T) { }, { scenario: "E", - action: Action{verb: "get", resource: "pods", subResource: "log"}, + action: Action{Verb: "get", Resource: "pods", SubResource: "log"}, mappingResult: &mappingResult{ argGVR: schema.GroupVersionResource{Resource: "pods"}, returnGVR: podsGVR, @@ -114,7 +114,7 @@ func TestResourceResolver_Resolve(t *testing.T) { }, { scenario: "F", - action: Action{verb: "get", resource: "pods", subResource: "logz"}, + action: Action{Verb: "get", Resource: "pods", SubResource: "logz"}, mappingResult: &mappingResult{ argGVR: schema.GroupVersionResource{Resource: "pods"}, returnGVR: podsGVR, @@ -123,7 +123,7 @@ func TestResourceResolver_Resolve(t *testing.T) { }, { scenario: "G", - action: Action{verb: "list", resource: "bees"}, + action: Action{Verb: "list", Resource: "bees"}, mappingResult: &mappingResult{ argGVR: schema.GroupVersionResource{Resource: "bees"}, returnError: errors.New("mapping failed"), @@ -132,7 +132,7 @@ func TestResourceResolver_Resolve(t *testing.T) { }, { scenario: "H", - action: Action{verb: rbac.VerbAll, resource: "pods"}, + action: Action{Verb: rbac.VerbAll, Resource: "pods"}, mappingResult: &mappingResult{ argGVR: schema.GroupVersionResource{Resource: "pods"}, returnGVR: podsGVR, @@ -141,7 +141,7 @@ func TestResourceResolver_Resolve(t *testing.T) { }, { scenario: "I", - action: Action{verb: "list", resource: rbac.ResourceAll}, + action: Action{Verb: "list", Resource: rbac.ResourceAll}, expected: expected{gr: schema.GroupResource{Resource: rbac.ResourceAll}}, }, } @@ -157,7 +157,7 @@ func TestResourceResolver_Resolve(t *testing.T) { resolver := NewResourceResolver(client.Discovery(), mapper) - resource, err := resolver.Resolve(tt.action.verb, tt.action.resource, tt.action.subResource) + resource, err := resolver.Resolve(tt.action.Verb, tt.action.Resource, tt.action.SubResource) assert.Equal(t, tt.expected.err, err) assert.Equal(t, tt.expected.gr, resource)