Skip to content

Add a new role.allow.request field called kubernetes_resources#47173

Merged
kimlisa merged 2 commits intomasterfrom
lisa/add-request-mode-role-option
Nov 1, 2024
Merged

Add a new role.allow.request field called kubernetes_resources#47173
kimlisa merged 2 commits intomasterfrom
lisa/add-request-mode-role-option

Conversation

@kimlisa
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa commented Oct 3, 2024

part of #46742
rfd: #46691

Defines a new role.allow.request field called kubernetes_resources.

For now it holds a field kubernetes_resources that follows same format as existing allow.kubernetes_resources, except the only field we support in the options field is Kind (defining other fields will reject the role upserting actions).

The Kind allows admins to define what kube subresources a user can request during request creation and disallow requesting request for kube_cluster. It allows the wildcard to mean allow request to any kube subresources.

If role.allow.request.kubernetes_resources is not defined, or length 0, it means a user can request for kube_cluster or any of its subresources.

example, if requester role says:

kind: role
metadata:
  name: requester
spec:
  allow:
    request:
      search_as_roles:
      - kube-access
  options:
    request_mode:
      kubernetes_resources:
      - kind: namespace

requesting kind kube_cluster is denied:

tsh request create --resource /kimlisa22.cloud.gravitational.io/kube_cluster/coffee-kube-cluster 

Creating request...
ERROR: your Teleport role's "request.kubernetes_resources" field did not allow requesting to some or all of the requested Kubernetes resources. allowed kinds for each requestable roles: access-kube-pumpkin: [pod], access: [pod], access-kube-coffee: [namespace]
Try searching for specific kinds with:
> tsh request search --kube-cluster=KUBE_CLUSTER_NAME --kind=KIND

requesting kind pod is denied:

tsh request create --resource /kimlisa22.cloud.gravitational.io/pod/coffee-kube-cluster/kube-system/coredns-7db6d8ff4d-mhjlv

Creating request...
ERROR: your Teleport role's "request.kubernetes_resources" field did not allow requesting to some or all of the requested Kubernetes resources. allowed kinds for each requestable roles: access-kube-coffee: [namespace]
Try searching for specific kinds with:
> tsh request search --kube-cluster=KUBE_CLUSTER_NAME --kind=KIND

requesting kind namespace is allowed:

Creating request...
Request ID:     0192bc03-0421-765b-9f5d-db01d9f7f647                                                    
Username:       lisa+1@goteleport.com                                                                   
Roles:          access-kube-coffee                                                                      
Resources:      ["/kimlisa22.cloud.gravitational.io/namespace/coffee-kube-cluster/coffee-kube-cluster"] 
Reason:         [none]                                                                                  
Reviewers:      [none] (suggested)                                                                      
Access Expires: 2024-10-24 01:51:54                                                                     
Status:         PENDING                                                                                 

hint: use 'tsh login --request-id=<request-id>' to login with an approved request

Waiting for request approval...

wildcard example output:

tsh request create --resource /kimlisa22.cloud.gravitational.io/kube_cluster/coffee-kube-cluster 

Creating request...
ERROR: your Teleport role's "request.kubernetes_resources" field did not allow requesting to some or all of the requested Kubernetes resources. allowed kinds for each requestable roles: access-kube-coffee: [pod secret configmap namespace service serviceaccount kube_node persistentvolume persistentvolumeclaim deployment replicaset statefulset daemonset clusterrole kube_role clusterrolebinding rolebinding cronjob job certificatesigningrequest ingress]
Try searching for specific kinds with:
> tsh request search --kube-cluster=KUBE_CLUSTER_NAME --kind=KIND

changelog: Define a new role.allow.request field called kubernetes_resources that allows admins to define what kinds of Kubernetes resources a requester can make.

@kimlisa kimlisa requested a review from tigrato October 3, 2024 22:43
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 3, 2024

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@kimlisa kimlisa changed the title Lisa/add request mode role option Add a new role.options field called request_mode.kubernetes_resources Oct 3, 2024
}

message AccessRequestMode {
repeated KubernetesResource KubernetesResources = 1 [
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we use a dedicated type for this setting?
Reusing the KubernetesResource gives confusion because you can set a lot of data that isn't allowed

@kimlisa kimlisa force-pushed the lisa/add-request-mode-role-option branch 3 times, most recently from ae41067 to b03a2f3 Compare October 4, 2024 21:58
@kimlisa kimlisa requested a review from tigrato October 4, 2024 21:59
@kimlisa kimlisa force-pushed the lisa/add-request-mode-role-option branch 2 times, most recently from 945322f to 70ecfbb Compare October 7, 2024 03:18
@kimlisa
Copy link
Copy Markdown
Contributor Author

kimlisa commented Oct 8, 2024

friendly ping @tigrato @nklaassen

Copy link
Copy Markdown
Contributor

@tigrato tigrato left a comment

Choose a reason for hiding this comment

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

Can you please add unmarshal from/to yaml of roles with this section defined?

Comment thread api/proto/teleport/legacy/types/types.proto Outdated
Comment thread api/proto/teleport/legacy/types/types.proto Outdated
Comment thread api/proto/teleport/legacy/types/types.proto Outdated
Comment thread lib/services/access_request.go Outdated
@kimlisa kimlisa force-pushed the lisa/add-request-mode-role-option branch 2 times, most recently from 4892231 to f987020 Compare October 16, 2024 07:26
@kimlisa
Copy link
Copy Markdown
Contributor Author

kimlisa commented Oct 16, 2024

i made a few adjustments based on review:

the request mode found on the same role as the search as roles will be enforced:

  • querying for kube resources with search as roles will prune roles that doesn't match request mode with request type
  • when creating request, request modes will be enforced during:
    • pruning search as roles with only root resource request
    • pruning search as roles with leaf and root resources (pruning doesn't happen, but we will still enforce request mode without any special matchers)
    • requesting custom roles, users can manually change/request roles so this will skip pruning check altogether, but we will still enforce request mode checking without any special matchers

@kimlisa kimlisa force-pushed the lisa/add-request-mode-role-option branch from f987020 to 7988cc6 Compare October 16, 2024 07:52
@kimlisa kimlisa requested a review from tigrato October 16, 2024 07:55
@kimlisa kimlisa force-pushed the lisa/add-request-mode-role-option branch 2 times, most recently from 449ca3d to 415537c Compare October 16, 2024 21:47
@kimlisa kimlisa marked this pull request as draft October 17, 2024 06:04
@kimlisa kimlisa force-pushed the lisa/add-request-mode-role-option branch from 415537c to 7f0453e Compare October 17, 2024 08:30
@kimlisa kimlisa marked this pull request as ready for review October 17, 2024 08:31
Copy link
Copy Markdown
Contributor

@tigrato tigrato left a comment

Choose a reason for hiding this comment

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

In addition to these changes, you’ll also need to update the following code:

if req.UseSearchAsRoles {
extraRoles = append(extraRoles, userContext.Checker.GetAllowedSearchAsRoles()...)
}

These changes are necessary to prevent users who are blocked from requesting access to specific types from being able to verify the existence of those assets in k8s.

GetSearchAsRoles must return the search as roles allowed to be used for the particular kubernetes subresource and deny if none match.

It's not a blocker but it must be released before v17

Comment thread lib/services/access_request.go Outdated
allowedKinds, deniedKinds := getKubeResourceKinds(m.kubernetesResource.allow[requestedRoleName]), getKubeResourceKinds(m.kubernetesResource.deny)

// Any resource is allowed.
if allowedKinds == nil && deniedKinds == nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we check for len(xxx)==0?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure i guess checking len is slightly safer b/c i'm thinking you're concerned this can happen

var array []string, by itself is nil
array := []string{} is not?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah. If someone changes the approach from using a non-initialized array to an initialized array, the code won't catch it.

Comment thread lib/services/access_request.go Outdated
}

// All supported kube kinds are allowed when there was nothing configured.
if allowedKinds == nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above

// the access request can be reviewed. Defaults to 1 week.
requestTTL = 7 * day

InvalidKubernetesKindAccessRequest = `your Teleport role's "request.kubernetes_resources" field`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

godoc

@public-teleport-github-review-bot public-teleport-github-review-bot Bot removed the request for review from fheinecke October 30, 2024 10:19
@kimlisa kimlisa force-pushed the lisa/add-request-mode-role-option branch 2 times, most recently from b243c2f to b98a970 Compare October 31, 2024 08:14
@kimlisa kimlisa force-pushed the lisa/add-request-mode-role-option branch from b98a970 to 51998b5 Compare October 31, 2024 15:50
@kimlisa kimlisa enabled auto-merge October 31, 2024 23:47
@kimlisa kimlisa force-pushed the lisa/add-request-mode-role-option branch from 51998b5 to d69b2f5 Compare October 31, 2024 23:52
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 1, 2024

🤖 Vercel preview here: https://docs-6onolp5ve-goteleport.vercel.app/docs/ver/preview

@kimlisa kimlisa force-pushed the lisa/add-request-mode-role-option branch from d69b2f5 to f8dc68e Compare November 1, 2024 00:07
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 1, 2024

🤖 Vercel preview here: https://docs-dnfaf44ij-goteleport.vercel.app/docs/ver/preview

@public-teleport-github-review-bot
Copy link
Copy Markdown

@kimlisa - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 1, 2024

🤖 Vercel preview here: https://docs-5jy8ahdem-goteleport.vercel.app/docs/ver/preview

@kimlisa kimlisa added this pull request to the merge queue Nov 1, 2024
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 1, 2024
@kimlisa kimlisa added this pull request to the merge queue Nov 1, 2024
Merged via the queue into master with commit a132be0 Nov 1, 2024
@kimlisa kimlisa deleted the lisa/add-request-mode-role-option branch November 1, 2024 04:05
@public-teleport-github-review-bot
Copy link
Copy Markdown

@kimlisa See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Failed

kimlisa added a commit that referenced this pull request Nov 1, 2024
…47173)

* Add a new role.allow.request field called kubernetes_resources

* Fix lint: update terraform docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v17 kubernetes-access size/lg size/md tsh tsh - Teleport's command line tool for logging into nodes running Teleport. ui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants