-
Notifications
You must be signed in to change notification settings - Fork 21
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
Allow passing in GITHUB_TOKEN
as an env var instead of a flag
#71
Comments
Hi @nitrocode great question! We probably need to communicate this in the docs better but |
It does make sense, but if I'd prefer it to either read infracost comment github --repo $BASE_REPO_OWNER/$BASE_REPO_NAME \
--pull-request $PULL_NUM \
--path /tmp/$BASE_REPO_OWNER-$BASE_REPO_NAME-$PULL_NUM/'*'-infracost.json \
--behavior new |
@nitrocode good point! We've learned from debugging lots of infracost user issues that explicit interfaces work best for them. So when users pass-in env vars it's clear (and self-documenting) which tokens are being used, and what env var in the user's env they map to. For example, some users already have |
Couldn't both be supported? The app could check for an environment variable to read and fill the value. If the flag is used, since it's explicit, it could take precedence. I suppose it's not a big deal unless you're using |
One other issue with passing in tokens from the cli is that if you run I'd argue that both ways are explicit. One is more secure than the other. Please consider reopening this. |
@nitrocode thanks - good point! BTW, last month we made the Infracost GH App free as it has several benefits, would you be interested in trying that? That removes the need for tokens altogether. |
Ah that does seem like a very nice replacement for the |
Great - thanks for trying it @nitrocode, feedback is welcome :) |
In the official integration docs, it shows a custom workflow with the following.
I'd much rather omit the
--github-token
and haveinfracost comment github
subcommand read the token from an environment variable. Is this possible today? If so, can we update the docs accordingly? If not, could this gh issue serve as a feature request to enable that?The text was updated successfully, but these errors were encountered: