-
Notifications
You must be signed in to change notification settings - Fork 3
fix: use organization_id smarter #75
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
Conversation
9d7fe8b to
1ad69d9
Compare
| // if the scanning project id is not provided, use the project specified in the provider | ||
| data "google_project" "selected" { | ||
| count = length(var.scanning_project_id) > 0 ? (length(var.organization_id) > 0 ? 0 : 1) : 1 | ||
| count = length(var.scanning_project_id) > 0 ? 0 : 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will make the logic here less convoluted. Previously there's an implicit dependency:
local.scanning_project_iddepends ondata.google_project.selecteddepends onvar.organization_id
which made things a bit hard to reason about. Hence I'm doing this cleanup, so we'll always try to get the current project regardless whether organization_id is provided. LMK if this condition is actually needed.
|
@badass-aoz, there is feedback in https://lacework.slack.com/archives/C03H3863SQG/p1714591554167629 on the version for the google provider. Can it also be addressed in this PR? |
4462bdf to
4da15ea
Compare
4da15ea to
7ac1ce4
Compare
Summary
While working on #74 I noticed that
organization_idisn't consistently set, so I went through all the example files and ensured it's used in a consistent way. In details:organization_idis not provided, we try to derive it w/ the providerproject_idorganization_idis used, we consistently use the derivedlocal.organization_idinstead of user-providedvar.organization_id.I also updated the min version requirements because minimum TF 1.5 is needed to support
checksblock. This makes it consistent w/ the Azure TF repo.How did you test this change?
terraform applygoes through.checkskick inIssue
https://lacework.atlassian.net/browse/LINK-2695