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

Accept PAT's as cli args in addition to env vars #4

Closed
Tracked by #105
dylan-smith opened this issue Oct 14, 2021 · 5 comments · Fixed by #289
Closed
Tracked by #105

Accept PAT's as cli args in addition to env vars #4

dylan-smith opened this issue Oct 14, 2021 · 5 comments · Fixed by #289
Assignees

Comments

@dylan-smith
Copy link
Collaborator

dylan-smith commented Oct 14, 2021

I did this as env vars to start because it's usually a more convenient way to pass around secrets, especially if you may be screen sharing while using the CLI. And you definately don't want secrets to be included in the generated script from the generate-script command.

For convenience though, an option to specify them directly as CLI args would be a nice addition.

@ArinGhazarian
Copy link
Collaborator

For generate-script when PATs are provided, should we also include the PATs in the generated script? I mean we have two options:

  1. Use the provided PATs only to get the required data but don't include them in the generated script (i.e. don't include --ado-pat option at the end of create-team script, etc.). This way the script still expects the PATs to be set as env variables.
  2. Include the PATs (i.e. --github-pat or --ado-pat) in the generated script at the end of each command. This way the script will entirely use the provided PATs and no env variable needs to be set.

@roferg
Copy link
Contributor

roferg commented Mar 24, 2022

Why can't we just pull the PATs from the env variables?

@ArinGhazarian
Copy link
Collaborator

ArinGhazarian commented Mar 24, 2022

Why can't we just pull the PATs from the env variables?

Of course we can, but this issue is to support passing them as command args. So I was just wondering what the desired behavior in for generate-script would be. Since we have the two options I mentioned in my question, we can go with both. Personally I think that for generate-script we should just use the provided PATs to fetch the required data to be able to generate the script and we shouldn't include the PATs in the script (at the end of each command).

@dylan-smith
Copy link
Collaborator Author

dylan-smith commented Mar 28, 2022

so if I think about why I wanted to do this work, it's to make things more easy/convenient for people that don't want to mess around with how to set env variables, and just want to pass everything on the command-line.

I would say if they pass them on the command line for generate script, then the generated script should work without needing them to manually set env vars. I see 3 options that I think would both be fine IMO:

  1. Have the migrate.ps1 script include a couple lines at the start that set the env vars, and don't pass them in to each individual command
  2. Pass the PAT's into each individual command in the script.
  3. Don't allow secrets as args in generate-script (only in the other commands).

In 1/2 cases the generated script will contain their secrets, I would suggest having generate-script spit out a warning making them aware of that fact.

I would probably lean towards whichever of these is the easiest to implement, which has to be 3.

@ArinGhazarian
Copy link
Collaborator

Yup 3 is the easiest to implement given the fact that if a CLI command has more than 16 args then we cannot use the Func delegate format for Invoke and we need to switch to a container class for args (see this for more info). But now I am wondering if it would be any helpful to pass the env variables as command args for other commands!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants