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

fix(cdk): Pass all global flags via env variables #993

Merged
merged 4 commits into from
Nov 2, 2022

Conversation

afiune
Copy link
Contributor

@afiune afiune commented Oct 31, 2022

Summary

We were not passing a number of global flags via environment variables, this change adds them all.

Signed-off-by: Salim Afiune Maya [email protected]

How did you test this change?

Ran CDK commands

Issue

https://lacework.atlassian.net/browse/ALLY-1243

@afiune afiune force-pushed the afiune/ALLY-1243/afiune/cdk-global-flags branch from 1a4bd71 to 4634212 Compare October 31, 2022 16:09
cli/cmd/cli_state.go Outdated Show resolved Hide resolved
@hazedav
Copy link
Collaborator

hazedav commented Nov 1, 2022

@afiune , I noticed that the removal of global flags has broken -h for help as well as the help subcommand

root@buildkitsandbox:/lars/integration# lacework remediate -h
Lacework Remediate is a tool that helps you isolate and remediate
resources in response to Lacework platform alerts.

Usage:
  remediate [command]

Available Commands:
  alert       Remediate an alert
  version     Print the Lacework CDK version

Flags:
      --debug   turn on debug logging

Use "remediate [command] --help" for more information about a command.
ERROR unable to run component: exit status 127
root@buildkitsandbox:/lars/integration# lacework remediate help
Lacework Remediate is a tool that helps you isolate and remediate
resources in response to Lacework platform alerts.

Usage:
  remediate [command]

Available Commands:
  alert       Remediate an alert
  version     Print the Lacework CDK version

Flags:
      --debug   turn on debug logging

Use "remediate [command] --help" for more information about a command.
ERROR unable to run component: exit status 127

I had integration tests for this which passed on 0.44.0...Thoughts?

@afiune
Copy link
Contributor Author

afiune commented Nov 1, 2022

@hazedav amazing catch!!! We should pass that flag to the component!!! 🤦🏽 Fixing now

@hazedav
Copy link
Collaborator

hazedav commented Nov 1, 2022

@afiune , I tested your change w/ my component and it's looking salad:

bash-5.1# lacework remediate -h
Lacework Remediate is a tool that helps you isolate and remediate
resources in response to Lacework platform alerts.

Usage:
  remediate [command]

Available Commands:
  alert       Remediate an alert
  help        Help about any command
  version     Print the Lacework CDK version

Flags:
      --debug   turn on debug logging
  -h, --help    help for remediate

Use "remediate [command] --help" for more information about a command.
bash-5.1# lacework remediate help
Lacework Remediate is a tool that helps you isolate and remediate
resources in response to Lacework platform alerts.

Usage:
  remediate [command]

Available Commands:
  alert       Remediate an alert
  help        Help about any command
  version     Print the Lacework CDK version

Flags:
      --debug   turn on debug logging
  -h, --help    help for remediate

Use "remediate [command] --help" for more information about a command.

@hazedav
Copy link
Collaborator

hazedav commented Nov 1, 2022

@afiune afiune force-pushed the afiune/ALLY-1243/afiune/cdk-global-flags branch from 6a81b70 to 0581a7a Compare November 1, 2022 22:53
We were not passing a number of global flags via environment variables,
this change adds them all.

Signed-off-by: Salim Afiune Maya <[email protected]>
Signed-off-by: Salim Afiune Maya <[email protected]>
@afiune afiune force-pushed the afiune/ALLY-1243/afiune/cdk-global-flags branch from 0581a7a to a636fca Compare November 2, 2022 04:54
@afiune afiune merged commit 1dc9fad into main Nov 2, 2022
@afiune afiune deleted the afiune/ALLY-1243/afiune/cdk-global-flags branch November 2, 2022 05:17
@afiune afiune mentioned this pull request Nov 3, 2022
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.

2 participants