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

Better Teleport CLI support #2143

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Better Teleport CLI support #2143

wants to merge 5 commits into from

Conversation

kanersps
Copy link

This PR has two areas of focus, both meant to significantly improve usability for the "tsh" and "tctl" Teleport CLI tools. These two areas of focus are:

  • Improving username@hostname autocomplete for tsh ssh
  • Full support for the tctl binary, which was non existent

This is my first autocompletion spec PR, so please do be gentle. I've been using these completions for a little bit myself and found them incredibly useful. Certainly the updates I did for tsh ssh has been an incredible productivity enhancer.

A small question, and maybe a worry: The custom generator and trigger I use for tsh ssh might decrease performance based on the past iteration, but I do feel that improved UX is worth it. On my M1/M2 macs I do not notice any performance issues.

And P.S. I don't think your Git pre-commit hooks are properly working, as they cause a big error to popup. Let me know if you want to investigate this, but I just used "--no-verify" and ran the checks manually, not a big deal.

@withfig-bot
Copy link
Collaborator

withfig-bot commented Oct 19, 2023

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


1 out of 2 committers have signed the CLA.
✅ (kanersps)[https://github.com/kanersps]
@superbryntendo
You can retrigger this bot by commenting recheck in this Pull Request

@withfig-bot
Copy link
Collaborator

withfig-bot commented Oct 19, 2023

Overview

src/tsh.ts:

Info:

src/tctl.ts:

Info:

Script:
tctl bots ls --format json
postProcess(function):

 function (out) {
    const bots = JSON.parse(out);
    return bots.map((bot: Bot) => {
      return {
        name: bot.metadata.name.slice(4),
        description: bot.metadata.description,
      };
    });
  }

Script:
tctl acl ls --format json
postProcess(function):

 function (out) {
    const acl = JSON.parse(out);
    return acl.map((acl: ACL) => {
      return {
        name: acl.metadata.name,
        description: acl.metadata.description,
      };
    });
  }

Script:
tctl alerts list --format json
postProcess(function):

 function (out) {
    const alerts = JSON.parse(out);
    return alerts.map((alert: Alert) => {
      return {
        name: alert.metadata.name,
        description: alert.metadata.description,
      };
    });
  }

Script:
tctl get namespaces --format json
postProcess(function):

 function (out) {
                    const namespaces = JSON.parse(out);

                    return namespaces.map((namespace: Namespace) => {
                      return {
                        name: namespace.metadata.name,
                        description: namespace.metadata.description,
                      };
                    });
                  }

Script:
tctl get tokens --format json
postProcess(function):

 function (out) {
                const tokens = JSON.parse(out);
                return tokens.map((token: Token) => {
                  return {
                    name: token.metadata.name,
                    description: token.metadata.description,
                  };
                });
              }

Script:
tctl get tokens --format json
postProcess(function):

 function (out) {
                    const clusters = JSON.parse(out);

                    return clusters.map((cluster: Cluster) => {
                      return {
                        name: cluster.kube_cluster_name,
                        description: "Kubernetes cluster connected to Teleport",
                      };
                    });
                  }

Script:
tctl get tokens --format json
postProcess(function):

 function (out) {
                const bots = JSON.parse(out);
                return bots.map((bot: Bot) => {
                  return {
                    name: bot.metadata.name.slice(4),
                    description: "A bot with this name already exists",
                  };
                });
              }

Single Functions:

filterTemplateSuggestions:

 function (paths) {
      return paths.filter(
        (file) =>
          file.name.endsWith("/") ||
          file.name.endsWith(".yaml") ||
          file.name.endsWith(".yml")
      );
    }

URLs:

  • http://127.0.0.1:3000

@withfig-bot
Copy link
Collaborator

Hello @kanersps,
thank you very much for creating a Pull Request!
Here is a small checklist to get this PR merged as quickly as possible:

  • Do all subcommands / options which take arguments include the args property (args: {})?
  • Are all options modular? E.g. -a -u -x instead of -aux
  • Have all other checks passed?

Please add a 👍 as a reaction to this comment to show that you read this.

@kanersps
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

withfig-bot added a commit that referenced this pull request Oct 19, 2023
src/tctl.ts Outdated
Comment on lines 1 to 2
import { type } from "node:os";

Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty sure this was imported automatically by mistake right?

Suggested change
import { type } from "node:os";

Copy link
Author

Choose a reason for hiding this comment

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

Yup, funny how that got in there.

Currently fixing up some of the arguments and style a lil' and I'll push an update.

Copy link
Member

Choose a reason for hiding this comment

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

@kanersps are you still working on this? Would love to get this contribution merged in :)

Copy link
Author

Choose a reason for hiding this comment

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

Hey @mschrage ! I certainly am, I have it completely ready locally but I accidentally messed some Git-things up. Hopefully have this ready by today, but the commits might be a lil' messed up. I advise squashing before merging it.

@kanersps
Copy link
Author

This should be all the features I wanted to implement, I'm not sure if I'm entirely done with messing with the Teleport autocompletions specs, but this will do for this MR.

Let me know if there are any changes required!

@kanersps
Copy link
Author

@mschrage Was there anything else that needs to be changed here, or can we merge? Locally 'publishing' this spec constantly is becoming a lil' annoying (probably a bug too, I presume, every so often I need to re-publish to my local namespace to ensure I have use these specs vs the one in this repo)

gnosticdev pushed a commit to gnosticdev/fig-autocomplete that referenced this pull request Dec 7, 2023
@kanersps
Copy link
Author

kanersps commented Dec 8, 2023

Good morning @superbryntendo . I see you made a quick commit, and I think that will break the functionality. I've specifically used a custom trigger, because I need to know the "username" portion as well to make a proper autocomplete suggestion.

Right now, with your change, the "username" variable does not exist and this will error out.

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 this pull request may close these issues.

5 participants