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

tctl resources #1137

Closed
kontsevoy opened this issue Jul 16, 2017 · 8 comments · Fixed by #1266
Closed

tctl resources #1137

kontsevoy opened this issue Jul 16, 2017 · 8 comments · Fixed by #1266
Assignees
Milestone

Comments

@kontsevoy
Copy link
Contributor

kontsevoy commented Jul 16, 2017

Problem

Teleport 2.2 has allowed for configuration and resources to merge. 2.3 needs to have a clear separation between them.

A configuration is a singleton: it always exists and there's always one copy. Not zero, not 1+.

A resource should have a true CRUD semantics: i.e. get, list, create, delete, update. We also have strange resources like SAMLResponse. What is that? Why do they exist? This is just a temporary data structure inside Teleport. Things like "reverse tunnels" or "certificate authorities" are the same: they are not user-visible resources, they are internal data structures.

Proposal

Since we're moving configuration to a separate issue, the following objects need to have first-class CLI resource support:

  1. Trusted Clusters
  2. UserRoles (enterprise only)
  3. Auth connectors like OIDC or SAML (enterprise only)

Teleport also supports low-level undocumented resources which we use for debugging or from Telekube, like "cert authorities" or "reverse tunnels". They should remain in code but we are not documenting them and keeping the right of changing their format as we wish.

YAML format

We already have YAML format defined for the 3 resources above.
Should we change anything there?

CLI syntax

Create & Update

$ tctl create <flags> resources.yaml
Flags:
    -f, --force    Overwrites a resource if it already exists. (update)

Note: the YAML file may contain a list of resources of different types (--- syntax).

Delete

$ tctl rm <flags> <args>
Flags:
    -f, --force    Do not return errors if a resource is not found

Args can take two forms:
   - `resource.yaml` a path to an existing file.
   - `type name1 name2 name3` resource type followed by a resource names

Examples:
  $ tctl rm role admin
  $ tctl rm admin.yaml

Note: the YAML file may contain a list of resources of different types (--- syntax).

List & Get

$ tctl get <args>
Args be one of:
    - When missing, show help
    - Resource type: `tctl get roles`
    - Resource type + name: `tctl get role admin`
    - `all` a special "resource type" which means to dump all resources

Examples:
  $ tctl get roles
  $ tctl get all
  $ tctl get role admin  
  $ tctl get role admin ops

Notes:

  • tctl get all > file.yaml should produce YAML file which then can be fed into tctl create -f file.yaml.
  • we do not support things like tctl get roles users (multiple resource types as arguments).

Deprecate old commands

tctl also supports commands like users and tokens. They are also resources, of course. Lets add a deprecation warning to them, saying that they will be removed in future versions.

We don't have to implement users and tokens resources right now to shrink the scope of this release.

Undocumented resources

tctl currently supports all kinds of resources (low level) which we use for Telekube and other internal purposes. It's also a good test bed to add new resources and trying them out before making them official. The following needs to happen here:

  • tctl create should refuse to accept resources not supported by Teleport distribution (i.e. OSS version should not accept Enterprise resources)
  • tctl create should print a clear DANGER message when undocumented resource is being created.

Other notes

  • Lets not support --format flag for now and always default to YAML
@kontsevoy kontsevoy added this to the 2.3 milestone Jul 16, 2017
@kontsevoy kontsevoy self-assigned this Jul 16, 2017
@klizhentas
Copy link
Contributor

klizhentas commented Jul 16, 2017

@kontsevoy there is a missing apply or upsert command that is necessary to make scripts easier - without those the scripts would look like this:

if tctl get resource then;
   tctl update -f resources.yaml
else
   tctl create -f resources.yaml
fi

This makes ansible scripts hard, with apply or upsert ansible scripts will be much simpler to write.

tctl apply -f resources.yaml

@kontsevoy
Copy link
Contributor Author

@klizhentas no, you just use tctl create -f without any conditions. That's what -f (force) is for. You're probably thinking it means "file" like in Kubernetes but it means "force".

@klizhentas
Copy link
Contributor

@kontsevoy question on rm command

How would one distinguish that resource.myresource is a filename on disk or resource name

  • resource.yaml a path to an existing file.
  • type name1 name2 name3 resource type followed by a resource names

@kontsevoy
Copy link
Contributor Author

@klizhentas re: rm command params:

We have two choices. The command can be dumb and "pedantic", thus requiring an additional "tell me how to interpret my own args" flag (something like --file) or it could apply a bit of intelligence, placing fewer restrictions to a user.

I always vote for CLI commands that aren't dumb. In this case, we can use the following information to avoid adding another flag. For the first argument to be a type instead of a file, all of the following must be true:

  1. It needs to belong to a known list of reserved words (allowed resource types).
  2. There needs to be more than one arg (i.e. followed by the resource names)

Additionally, for an argument to be interpreted as a file, the following must be true:

  1. It needs to exist
  2. It must have an extension (and only .yaml or .yml are supported at this time).

The requirement to have an extension IMO is a sensible one because it leaves us an easy way to add additional resource formats in the future (JSON, text)

To summarize, the flag processing would go like:

  1. Try to interpret arguments as type + names (using steps above)
  2. If failed, try to interpret arguments as files.
  3. If failed, show an error.

@klizhentas
Copy link
Contributor

Also, our current resources are specified by kind and not type, so CLI should be consistent with this naming scheme

@klizhentas
Copy link
Contributor

klizhentas commented Jul 16, 2017

For get we need a format flag, on how to output the result - e.g. --format=json or --format=yaml for export purposes

@klizhentas
Copy link
Contributor

Also optional --with-secrets that is set to off to allow connectors export

@kontsevoy
Copy link
Contributor Author

kontsevoy commented Jul 17, 2017

Re: format. the ticket explicitly states that we're not officially supporting anything other than YAML to save time.

Not following what --with-secrets means, maybe you'll explain in person

This was referenced Jul 26, 2017
@russjones russjones mentioned this issue Jul 26, 2017
30 tasks
@kontsevoy kontsevoy mentioned this issue Sep 4, 2017
kontsevoy added a commit that referenced this issue Sep 4, 2017
This commit refs #1137

- tctl get user/joe now works (as reported in #1247)
- tctl create/rm roles changes
- added synonyms for various resources
- made YAML the default output for tctl get
- added better help + examples for tctl get
- edited error messages
- minor refactoring
- added the system of "command plugins" which allows enterprise version
  of tctl to introduce different behavior to OSS commands
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants