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.

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

Add support for a policy resource to Teleport #17220

Merged
merged 14 commits into from
Oct 26, 2022

Conversation

xacrimon
Copy link
Contributor

@xacrimon xacrimon commented Oct 10, 2022

This PR adds support for a predicate based policy resource to Teleport. It primarily contains expressions that will be evaluated by a predicate (the internal expression language) based evaluator. This PR pairs with gravitational/predicate-lang#17. See that PR for the practical usage for the resource & commands introduced by this PR.

@klizhentas Should we keep predicate related changes in a separate branch or merge to master?

EDIT: I've changed the target branch to dev-predicate.

@xacrimon xacrimon requested a review from klizhentas October 10, 2022 14:53
@xacrimon xacrimon force-pushed the joel/dont-delete/predicate branch from bc849c3 to a96f953 Compare October 12, 2022 14:13
@xacrimon xacrimon marked this pull request as ready for review October 12, 2022 14:44
@github-actions github-actions bot added the tctl tctl - Teleport admin tool label Oct 12, 2022
@fspmarshall
Copy link
Contributor

Is there an RFD or design doc for this somewhere? Hard to give a meaningful review without more context. I'm hesitant to add something like this to the public API without a clear idea of how the feature as a whole will work.

@fspmarshall
Copy link
Contributor

Should we keep predicate related changes in a separate branch or merge to master?

IMO if there isn't an RFD yet, this should have its own separate dev branch for now.

@xacrimon xacrimon changed the base branch from master to dev-predicate October 13, 2022 15:18
@xacrimon
Copy link
Contributor Author

xacrimon commented Oct 13, 2022

@fspmarshall Seems apt, I've changed the base to a dev-predicate branch for the time being. We don't really have an RFD for this yet as it's an an exploration phase. The rundown is:

  • Tooling from https://github.com/gravitational/predicate-lang will automatically convert policies defined in a Python DSL into this resource, the various fields being expressions understood by Teleports existing parser/evaluator mechanism for where expressions
  • Checks will be introduced later to also query an RBAC engine that works by evaluating these expressions, working in addition to the existing RBAC system

This PR provides a more practical example of the admin-side of things.

@xacrimon xacrimon force-pushed the joel/dont-delete/predicate branch from bf4f74b to 17f68db Compare October 13, 2022 15:27
@github-actions
Copy link

@xacrimon - this PR is large and will require admin approval to merge. Consider breaking it up into a series smaller changes.

@xacrimon xacrimon changed the base branch from dev-predicate to master October 13, 2022 15:31
@xacrimon xacrimon changed the base branch from master to dev-predicate October 13, 2022 15:31
@xacrimon xacrimon requested review from nklaassen and removed request for ravicious October 13, 2022 22:03
api/proto/teleport/legacy/types/types.proto Outdated Show resolved Hide resolved
api/proto/teleport/legacy/types/types.proto Outdated Show resolved Hide resolved
api/proto/teleport/legacy/types/types.proto Outdated Show resolved Hide resolved
api/types/constants.go Outdated Show resolved Hide resolved
Copy link
Contributor

@nklaassen nklaassen left a comment

Choose a reason for hiding this comment

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

Code looks fine, just a couple comments on comments and naming. I don't want to block this from merging to a dev branch but I do thing policy is probably too generic of a name

@russjones
Copy link
Contributor

@xacrimon Investigate if we need to add this resource into the cache system. I seem to remember having to add resources into some giant switch statement whenever I had to do this.

@github-actions github-actions bot removed the request for review from klizhentas October 25, 2022 16:53
@xacrimon xacrimon merged commit 9d483e5 into dev-predicate Oct 26, 2022
xacrimon added a commit that referenced this pull request Nov 10, 2022
* add workings for policy resource

* add grpc defs for create/get policy

* policy resource plumbing

* fix some bugs & add tctl get command

* add listing code

* use a custom api instead of listresources

* rename ListPolicies to GetPolicies

* update allow/deny/options to be a map of scopes instead

* keep option a nonmap

* trim policy def

* touchups

* add rbac checks

* revert e ref

* rename
@xacrimon xacrimon deleted the joel/dont-delete/predicate branch January 23, 2023 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tctl tctl - Teleport admin tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants