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

feat: added datastream support in shared_vpc_access module #788

Conversation

imrannayer
Copy link
Contributor

Added datastream permission requirements in shared_vpc_access module #787
Fix missing profile_id in test for vpc-sc-project-local test #780

@imrannayer imrannayer requested a review from a team as a code owner February 28, 2023 15:33
@imrannayer
Copy link
Contributor Author

@bharathkkb can you plz review and execute the pipeline?
Thanks

@g-awmalik
Copy link
Contributor

/gcbrun

@g-awmalik
Copy link
Contributor

policy_id seems to be required variable but is not getting set. Can you take a look at that?

@g-awmalik
Copy link
Contributor

/gcbrun

2 similar comments
@g-awmalik
Copy link
Contributor

/gcbrun

@g-awmalik
Copy link
Contributor

/gcbrun

@@ -175,3 +175,4 @@ options:
- 'TF_VAR_gsuite_admin_email=project-factory-test-admin@test.infra.cft.tips'
- 'TF_VAR_gsuite_domain=test.infra.cft.tips'
- 'TF_VAR_domain=test.infra.cft.tips.'
- 'TF_VAR_policy_id=$_POLICY_ID'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of adding this new variable and trigger key/value pair we could do something like
export TF_VAR_policy_id=$(gcloud access-context-manager policies list --organization="${TF_VAR_org_id:?}" --format="value(name)") as part of the args to the prepare step where this is need. That would eliminate the need for the new output and variable as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have following lines in cloudbuild but for some reason it does not work.

- id: converge vpc-sc-project-local
  waitFor:
      - create-all
  name: 'gcr.io/cloud-foundation-cicd/$_DOCKER_IMAGE_DEVELOPER_TOOLS:$_DOCKER_TAG_VERSION_DEVELOPER_TOOLS'
  args: ['/bin/bash', '-c', 'source /usr/local/bin/task_helper_functions.sh && export TF_VAR_policy_id=$(gcloud access-context-manager policies list --organization="${TF_VAR_org_id:?}" --format="value(name)") && kitchen_do converge vpc-sc-project-local']

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Ok let's leave the issue open so we can investigate what's happening. I'll merge this for now.

@@ -58,3 +58,7 @@ output "group_name" {
output "service_account_email" {
value = google_service_account.int_test.email
}

output "policy_id" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use the gcloud command to evaluate the policy id during the prepare step, we could probably get rid of this.

@g-awmalik g-awmalik merged commit a03c5e8 into terraform-google-modules:master Mar 1, 2023
@imrannayer imrannayer self-assigned this Apr 24, 2024
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.

None yet

2 participants