Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

support subresource when running kubectl create role #41384

Merged
merged 1 commit into from Apr 24, 2017
Merged

support subresource when running kubectl create role #41384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 40 additions & 13 deletions hack/make-rules/test-cmd-util.sh
View file
Open in desktop
Expand Up @@ -3016,20 +3016,47 @@ runTests() {
kube::test::get_object_assert rolebinding/sarole "{{range.subjects}}{{.name}}:{{end}}" 'sa-name:'
fi

########
# Role #
########
if kube::test::if_supports_resource "${roles}" ; then
kubectl create "${kube_flags[@]}" role pod-admin --verb=* --resource=pods
kube::test::get_object_assert role/pod-admin "{{range.rules}}{{range.verbs}}{{.}}:{{end}}{{end}}" '\*:'
kube::test::get_object_assert role/pod-admin "{{range.rules}}{{range.resources}}{{.}}:{{end}}{{end}}" 'pods:'
kube::test::get_object_assert role/pod-admin "{{range.rules}}{{range.apiGroups}}{{.}}:{{end}}{{end}}" ':'
kubectl create "${kube_flags[@]}" role resource-reader --verb=get,list --resource=pods,deployments.extensions
kube::test::get_object_assert role/resource-reader "{{range.rules}}{{range.verbs}}{{.}}:{{end}}{{end}}" 'get:list:get:list:'
kube::test::get_object_assert role/resource-reader "{{range.rules}}{{range.resources}}{{.}}:{{end}}{{end}}" 'pods:deployments:'
kube::test::get_object_assert role/resource-reader "{{range.rules}}{{range.apiGroups}}{{.}}:{{end}}{{end}}" ':extensions:'
kubectl create "${kube_flags[@]}" role resourcename-reader --verb=get,list --resource=pods --resource-name=foo
kube::test::get_object_assert role/resourcename-reader "{{range.rules}}{{range.verbs}}{{.}}:{{end}}{{end}}" 'get:list:'
kube::test::get_object_assert role/resourcename-reader "{{range.rules}}{{range.resources}}{{.}}:{{end}}{{end}}" 'pods:'
kube::test::get_object_assert role/resourcename-reader "{{range.rules}}{{range.apiGroups}}{{.}}:{{end}}{{end}}" ':'
kube::test::get_object_assert role/resourcename-reader "{{range.rules}}{{range.resourceNames}}{{.}}:{{end}}{{end}}" 'foo:'
# Create Role from command (only resource)
kubectl create "${kube_flags[@]}" role pod-admin --verb=* --resource=pods
kube::test::get_object_assert role/pod-admin "{{range.rules}}{{range.verbs}}{{.}}:{{end}}{{end}}" '\*:'
kube::test::get_object_assert role/pod-admin "{{range.rules}}{{range.resources}}{{.}}:{{end}}{{end}}" 'pods:'
kube::test::get_object_assert role/pod-admin "{{range.rules}}{{range.apiGroups}}{{.}}:{{end}}{{end}}" ':'
output_message=$(! kubectl create "${kube_flags[@]}" role invalid-pod-admin --verb=* --resource=invalid-resource 2>&1)
kube::test::if_has_string "${output_message}" "the server doesn't have a resource type \"invalid-resource\""
# Create Role from command (resource + group)
kubectl create "${kube_flags[@]}" role group-reader --verb=get,list --resource=deployments.extensions
kube::test::get_object_assert role/group-reader "{{range.rules}}{{range.verbs}}{{.}}:{{end}}{{end}}" 'get:list:'
kube::test::get_object_assert role/group-reader "{{range.rules}}{{range.resources}}{{.}}:{{end}}{{end}}" 'deployments:'
kube::test::get_object_assert role/group-reader "{{range.rules}}{{range.apiGroups}}{{.}}:{{end}}{{end}}" 'extensions:'
output_message=$(! kubectl create "${kube_flags[@]}" role invalid-group --verb=get,list --resource=deployments.invalid-group 2>&1)
kube::test::if_has_string "${output_message}" "the server doesn't have a resource type \"deployments\" in group \"invalid-group\""
# Create Role from command (resource / subresource)
kubectl create "${kube_flags[@]}" role subresource-reader --verb=get,list --resource=pods/status
kube::test::get_object_assert role/subresource-reader "{{range.rules}}{{range.verbs}}{{.}}:{{end}}{{end}}" 'get:list:'
kube::test::get_object_assert role/subresource-reader "{{range.rules}}{{range.resources}}{{.}}:{{end}}{{end}}" 'pods/status:'
kube::test::get_object_assert role/subresource-reader "{{range.rules}}{{range.apiGroups}}{{.}}:{{end}}{{end}}" ':'
# Create Role from command (resource + group / subresource)
kubectl create "${kube_flags[@]}" role group-subresource-reader --verb=get,list --resource=replicasets.extensions/scale
kube::test::get_object_assert role/group-subresource-reader "{{range.rules}}{{range.verbs}}{{.}}:{{end}}{{end}}" 'get:list:'
kube::test::get_object_assert role/group-subresource-reader "{{range.rules}}{{range.resources}}{{.}}:{{end}}{{end}}" 'replicasets/scale:'
kube::test::get_object_assert role/group-subresource-reader "{{range.rules}}{{range.apiGroups}}{{.}}:{{end}}{{end}}" 'extensions:'
output_message=$(! kubectl create "${kube_flags[@]}" role invalid-group --verb=get,list --resource=rs.invalid-group/scale 2>&1)
kube::test::if_has_string "${output_message}" "the server doesn't have a resource type \"rs\" in group \"invalid-group\""
# Create Role from command (resource + resourcename)
kubectl create "${kube_flags[@]}" role resourcename-reader --verb=get,list --resource=pods --resource-name=foo
kube::test::get_object_assert role/resourcename-reader "{{range.rules}}{{range.verbs}}{{.}}:{{end}}{{end}}" 'get:list:'
kube::test::get_object_assert role/resourcename-reader "{{range.rules}}{{range.resources}}{{.}}:{{end}}{{end}}" 'pods:'
kube::test::get_object_assert role/resourcename-reader "{{range.rules}}{{range.apiGroups}}{{.}}:{{end}}{{end}}" ':'
kube::test::get_object_assert role/resourcename-reader "{{range.rules}}{{range.resourceNames}}{{.}}:{{end}}{{end}}" 'foo:'
# Create Role from command (multi-resources)
kubectl create "${kube_flags[@]}" role resource-reader --verb=get,list --resource=pods/status,deployments.extensions
kube::test::get_object_assert role/resource-reader "{{range.rules}}{{range.verbs}}{{.}}:{{end}}{{end}}" 'get:list:get:list:'
kube::test::get_object_assert role/resource-reader "{{range.rules}}{{range.resources}}{{.}}:{{end}}{{end}}" 'pods/status:deployments:'
kube::test::get_object_assert role/resource-reader "{{range.rules}}{{range.apiGroups}}{{.}}:{{end}}{{end}}" ':extensions:'
fi

#########################
Expand Down
8 changes: 7 additions & 1 deletion pkg/kubectl/cmd/create_clusterrole.go
View file
Open in desktop
Expand Up @@ -36,7 +36,13 @@ var (
kubectl create clusterrole pod-reader --verb=get,list,watch --resource=pods

# Create a ClusterRole named "pod-reader" with ResourceName specified
kubectl create clusterrole pod-reader --verb=get,list,watch --resource=pods --resource-name=readablepod`))
kubectl create clusterrole pod-reader --verb=get,list,watch --resource=pods --resource-name=readablepod

# Create a ClusterRole named "foo" with API Group specified
kubectl create clusterrole foo --verb=get,list,watch --resource=rs.extensions

# Create a ClusterRole named "foo" with SubResource specified
kubectl create clusterrole foo --verb=get,list,watch --resource=pods,pods/status`))
)

type CreateClusterRoleOptions struct {
Expand Down
48 changes: 36 additions & 12 deletions pkg/kubectl/cmd/create_role.go
View file
Open in desktop
Expand Up @@ -43,16 +43,28 @@ var (
kubectl create role pod-reader --verb=get --verb=list --verb=watch --resource=pods

# Create a Role named "pod-reader" with ResourceName specified
kubectl create role pod-reader --verb=get --verb=list --verb=watch --resource=pods --resource-name=readablepod`))
kubectl create role pod-reader --verb=get --verb=list --verb=watch --resource=pods --resource-name=readablepod

# Create a Role named "foo" with API Group specified
kubectl create role foo --verb=get,list,watch --resource=rs.extensions

# Create a Role named "foo" with SubResource specified
kubectl create role foo --verb=get,list,watch --resource=pods,pods/status`))

// Valid resource verb list for validation.
validResourceVerbs = []string{"*", "get", "delete", "list", "create", "update", "patch", "watch", "proxy", "redirect", "deletecollection", "use", "bind", "impersonate"}
)

type ResourceOptions struct {
Group string
Resource string
SubResource string
}

type CreateRoleOptions struct {
Name string
Verbs []string
Resources []schema.GroupVersionResource
Resources []ResourceOptions
ResourceNames []string

DryRun bool
Expand All @@ -70,7 +82,7 @@ func NewCmdCreateRole(f cmdutil.Factory, cmdOut io.Writer) *cobra.Command {
Out: cmdOut,
}
cmd := &cobra.Command{
Use: "role NAME --verb=verb --resource=resource.group [--resource-name=resourcename] [--dry-run]",
Use: "role NAME --verb=verb --resource=resource.group/subresource [--resource-name=resourcename] [--dry-run]",
Short: roleLong,
Long: roleLong,
Example: roleExample,
Expand Down Expand Up @@ -116,13 +128,20 @@ func (c *CreateRoleOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args
// e.g. --resource=pods,deployments.extensions
resources := cmdutil.GetFlagStringSlice(cmd, "resource")
for _, r := range resources {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logically, this should split first on the / to get a subresource (if any), then split the first segment on . to get the resource and group

sections := strings.Split(r, ".")
sections := strings.SplitN(r, "/", 2)

resource := &ResourceOptions{}
if len(sections) == 2 {
resource.SubResource = sections[1]
}

if len(sections) == 1 {
c.Resources = append(c.Resources, schema.GroupVersionResource{Resource: r})
} else {
c.Resources = append(c.Resources, schema.GroupVersionResource{Resource: sections[0], Group: strings.Join(sections[1:], ".")})
parts := strings.SplitN(sections[0], ".", 2)
if len(parts) == 2 {
resource.Group = parts[1]
}
resource.Resource = parts[0]

c.Resources = append(c.Resources, *resource)
}

// Remove duplicate resource names.
Expand Down Expand Up @@ -180,8 +199,10 @@ func (c *CreateRoleOptions) Validate() error {
}

for _, r := range c.Resources {
_, err := c.Mapper.ResourceFor(r)
if err != nil {
if len(r.Resource) == 0 {
return fmt.Errorf("resource must be specified if apiGroup/subresource specified")
}
if _, err := c.Mapper.ResourceFor(schema.GroupVersionResource{Resource: r.Resource, Group: r.Group}); err != nil {
return err
}
}
Expand Down Expand Up @@ -228,7 +249,7 @@ func arrayContains(s []string, e string) bool {
return false
}

func generateResourcePolicyRules(mapper meta.RESTMapper, verbs []string, resources []schema.GroupVersionResource, resourceNames []string) ([]rbac.PolicyRule, error) {
func generateResourcePolicyRules(mapper meta.RESTMapper, verbs []string, resources []ResourceOptions, resourceNames []string) ([]rbac.PolicyRule, error) {
// groupResourceMapping is a apigroup-resource map. The key of this map is api group, while the value
// is a string array of resources under this api group.
// E.g. groupResourceMapping = {"extensions": ["replicasets", "deployments"], "batch":["jobs"]}
Expand All @@ -239,10 +260,13 @@ func generateResourcePolicyRules(mapper meta.RESTMapper, verbs []string, resourc
// 2. Prevents pointing to non-existent resources.
// 3. Transfers resource short name to long name. E.g. rs.extensions is transferred to replicasets.extensions
for _, r := range resources {
resource, err := mapper.ResourceFor(r)
resource, err := mapper.ResourceFor(schema.GroupVersionResource{Resource: r.Resource, Group: r.Group})
if err != nil {
return []rbac.PolicyRule{}, err
}
if len(r.SubResource) > 0 {
resource.Resource = resource.Resource + "/" + r.SubResource
}
if !arrayContains(groupResourceMapping[resource.Group], resource.Resource) {
groupResourceMapping[resource.Group] = append(groupResourceMapping[resource.Group], resource.Resource)
}
Expand Down
91 changes: 81 additions & 10 deletions pkg/kubectl/cmd/create_role_test.go
View file
Open in desktop
Expand Up @@ -24,7 +24,6 @@ import (

"k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/client-go/rest/fake"
"k8s.io/kubernetes/pkg/apis/rbac"
cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing"
Expand Down Expand Up @@ -80,6 +79,40 @@ func TestCreateRole(t *testing.T) {
},
},
},
"test-subresources": {
verbs: "get,watch,list",
resources: "replicasets/scale",
expectedRole: &rbac.Role{
ObjectMeta: v1.ObjectMeta{
Name: roleName,
},
Rules: []rbac.PolicyRule{
{
Verbs: []string{"get", "watch", "list"},
Resources: []string{"replicasets/scale"},
APIGroups: []string{"extensions"},
ResourceNames: []string{},
},
},
},
},
"test-subresources-with-apigroup": {
verbs: "get,watch,list",
resources: "replicasets.extensions/scale",
expectedRole: &rbac.Role{
ObjectMeta: v1.ObjectMeta{
Name: roleName,
},
Rules: []rbac.PolicyRule{
{
Verbs: []string{"get", "watch", "list"},
Resources: []string{"replicasets/scale"},
APIGroups: []string{"extensions"},
ResourceNames: []string{},
},
},
},
},
"test-valid-case-with-multiple-apigroups": {
verbs: "get,watch,list",
resources: "pods,deployments.extensions",
Expand Down Expand Up @@ -148,11 +181,35 @@ func TestValidate(t *testing.T) {
},
expectErr: true,
},
"test-missing-resource-existing-apigroup": {
roleOptions: &CreateRoleOptions{
Name: "my-role",
Verbs: []string{"get"},
Resources: []ResourceOptions{
{
Group: "extensions",
},
},
},
expectErr: true,
},
"test-missing-resource-existing-subresource": {
roleOptions: &CreateRoleOptions{
Name: "my-role",
Verbs: []string{"get"},
Resources: []ResourceOptions{
{
SubResource: "scale",
},
},
},
expectErr: true,
},
"test-invalid-verb": {
roleOptions: &CreateRoleOptions{
Name: "my-role",
Verbs: []string{"invalid-verb"},
Resources: []schema.GroupVersionResource{
Resources: []ResourceOptions{
{
Resource: "pods",
},
Expand All @@ -164,7 +221,7 @@ func TestValidate(t *testing.T) {
roleOptions: &CreateRoleOptions{
Name: "my-role",
Verbs: []string{"post"},
Resources: []schema.GroupVersionResource{
Resources: []ResourceOptions{
{
Resource: "pods",
},
Expand All @@ -176,7 +233,7 @@ func TestValidate(t *testing.T) {
roleOptions: &CreateRoleOptions{
Name: "my-role",
Verbs: []string{"get"},
Resources: []schema.GroupVersionResource{
Resources: []ResourceOptions{
{
Resource: "invalid-resource",
},
Expand All @@ -188,7 +245,7 @@ func TestValidate(t *testing.T) {
roleOptions: &CreateRoleOptions{
Name: "my-role",
Verbs: []string{"get"},
Resources: []schema.GroupVersionResource{
Resources: []ResourceOptions{
{
Resource: "pods",
},
Expand All @@ -205,7 +262,7 @@ func TestValidate(t *testing.T) {
roleOptions: &CreateRoleOptions{
Name: "role-binder",
Verbs: []string{"get", "list", "bind"},
Resources: []schema.GroupVersionResource{
Resources: []ResourceOptions{
{
Resource: "roles",
Group: "rbac.authorization.k8s.io",
Expand All @@ -215,6 +272,20 @@ func TestValidate(t *testing.T) {
},
expectErr: false,
},
"test-valid-case-with-subresource": {
roleOptions: &CreateRoleOptions{
Name: "my-role",
Verbs: []string{"get", "list"},
Resources: []ResourceOptions{
{
Resource: "replicasets",
SubResource: "scale",
},
},
ResourceNames: []string{"bar"},
},
expectErr: false,
},
}

for name, test := range tests {
Expand Down Expand Up @@ -271,7 +342,7 @@ func TestComplete(t *testing.T) {
"watch",
"list",
},
Resources: []schema.GroupVersionResource{
Resources: []ResourceOptions{
{
Resource: "pods",
Group: "",
Expand Down Expand Up @@ -299,7 +370,7 @@ func TestComplete(t *testing.T) {
expected: &CreateRoleOptions{
Name: roleName,
Verbs: []string{"*"},
Resources: []schema.GroupVersionResource{
Resources: []ResourceOptions{
{
Resource: "pods",
Group: "",
Expand All @@ -323,7 +394,7 @@ func TestComplete(t *testing.T) {
expected: &CreateRoleOptions{
Name: roleName,
Verbs: []string{"*"},
Resources: []schema.GroupVersionResource{
Resources: []ResourceOptions{
{
Resource: "pods",
Group: "",
Expand All @@ -347,7 +418,7 @@ func TestComplete(t *testing.T) {
expected: &CreateRoleOptions{
Name: roleName,
Verbs: []string{"*"},
Resources: []schema.GroupVersionResource{
Resources: []ResourceOptions{
{
Resource: "pods",
Group: "",
Expand Down