chore: log permissions external command#5027
chore: log permissions external command#5027chadinwork wants to merge 12 commits intorunatlantis:mainfrom
Conversation
chadinwork
requested review from
X-Guardian,
chenrui333 and
lukemassa
and removed request for
a team
dosubot
bot
added
the
refactoring
chadinwork
force-pushed
the
log-permissions-external-command
branch
from
03b61b1 to
3219b58
Compare
Signed-off-by: Chadin Anuwattanaporn <[email protected]>
Signed-off-by: Chadin Anuwattanaporn <[email protected]>
chadinwork
force-pushed
the
log-permissions-external-command
branch
from
3219b58 to
45d47e0
Compare
|
Thanks for this @chadinwork. Can you use |
Signed-off-by: Chadin Anuwattanaporn <[email protected]>
Signed-off-by: Chadin Anuwattanaporn <[email protected]>
|
Sure @X-Guardian, I was following https://github.com/runatlantis/atlantis/blob/main/CONTRIBUTING.md which asked for Updated! |
Good spot. I'll get that updated. |
X-Guardian
left a comment
X-Guardian
left a comment
There was a problem hiding this comment.
Can you also add some sample log output from these changes to the PR description, so that we can see what they look like.
Co-authored-by: Simon Heather <[email protected]> Signed-off-by: C <[email protected]>
|
@X-Guardian I've applied suggested changes, and added examples. Let me know if anything else is needed! FYI: I'm out of town from tomorrow onwards for one week, so in case I miss your replies before I go, I'll come back and attend to them after I return. |
| return checker.checkOutputResults(out) | ||
| outputResults := checker.checkOutputResults(out) | ||
| if !outputResults { | ||
| ctx.Log.Info("command '%s' returns '%s'", cmd, out) |
There was a problem hiding this comment.
I'm not convinced that these should be Info messages. I think Debug would be better, so they only display when an Atlantis command is run with the -verbose flag . Same below.
Your examples in the description aren't useful, as they are made up. Please can you provide real world usage messages.
|
This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month. |
|
@chadinwork do you have time to address the comments? Thanks. |
|
This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month. |
|
This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month. |
3 participants
what
command '/etc/scripts/authorization.sh apply repo/here ' returns 'the custom script echo goes here'such ascommand '/etc/scripts/authorization.sh apply repo/here ' returns 'user \"chadin\" must be a member of \"alice, bob\" to apply changes.')Command '/etc/scripts/authorization.sh apply repo/here ' error 'exit status 1: running \"sh -c /etc/scripts/authorization.sh apply repo/here \": \nuser \"chadin\" must be a member of \"xx, yy\" to apply changes.\n')User 'chadin' in team '[]' does not have permissions to execute the 'apply' command)why
tests
make testandmake test-allin Docker. All tests passedGive me a while to run manual tests to confirm the logs are actually emitted (need just a bit more time to set up test Terraform repo)Tested and verified working!references