-
Notifications
You must be signed in to change notification settings - Fork 0
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: Firewall policy rule with resource manager tag #1
base: master
Are you sure you want to change the base?
Conversation
project = module.peering_project.project_id | ||
} | ||
|
||
resource "google_compute_network_firewall_policy_rule" "allow_iap_ssh" { |
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.
is there a module for this type of firewall rule?
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.
There's a module:
https://github.com/terraform-google-modules/terraform-google-network/tree/master/modules/network-firewall-policy
But it's not working as expected so I open an issue:
terraform-google-modules/terraform-google-network#505
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.
PR 514 will fix the issue.
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.
@Samir-Cit since network firewall policy module is fixed is it possible to call the module instead of resource?
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.
Your PR looks really good :) congrats!
Question: Since we are adding new resources (tags, subnets, etc), don't we need to update any .go tests to reflect/expect these new resources?
// (https://github.com/terraform-google-modules/terraform-google-bootstrap/tree/master/modules/tf_cloudbuild_workspace) | ||
// in the 4-projects shared environment of each business unit. | ||
// the repository name is the same key used for the app_infra_pipeline_service_accounts map and the | ||
// roles will be granted to the service account with the same key. |
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.
How about improve a little this comment block? Looks like the written is not super fluid here.
Example: instead of ...permissions to deploy the resources, a Compute Engine instance...
, how about ...permissions to deploy a Compute Engine instance...
Also, maybe skipping a line between one idea and another could be easier to understand, instead of multiple non-breaking lines of text.
Hello folks.
This PR is regarding the usage of resource manager tags for firewall policy rules and on VMs.
Two new resource manager tags created for the two firewall rules that were refactored, allowing IAP SSH and IAP RDP. The peering network project was used as an example, creating a new sub network on the peered network to be used by the VM.