-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add support for network_firewall_policy_association and region_network_firewall_policy_association #6796
Add support for network_firewall_policy_association and region_network_firewall_policy_association #6796
Conversation
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 7 files changed, 1049 insertions(+), 31 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccComputeRegionNetworkFirewallPolicyAssociation_RegionalHandWritten|TestAccComputeNetworkFirewallPolicyAssociation_GlobalHandWritten|TestAccFirebaserulesRelease_BasicRelease |
Tests passed during RECORDING mode: All tests passed |
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.
One comment about the id/import format, let's use a synthetic one over the format of the GET URL. That's consistent with other similar resources (we call this pattern "fine-grained resources"), particularly the FirewallPolicy suite of 'em.
Also left some newline suggestions, but that's just nitpicking. Feel free to make 'em in GitHub or locally. They're non-blocking.
tpgtools/overrides/compute/beta/network_firewall_policy_association.yaml
Outdated
Show resolved
Hide resolved
tpgtools/overrides/compute/samples/networkfirewallpolicyassociation/global.tf.tmpl
Show resolved
Hide resolved
tpgtools/overrides/compute/samples/networkfirewallpolicyassociation/global_update.tf.tmpl
Show resolved
Hide resolved
tpgtools/overrides/compute/samples/networkfirewallpolicyassociation/global_update.tf.tmpl
Show resolved
Hide resolved
tpgtools/overrides/compute/samples/networkfirewallpolicyassociation/regional.tf.tmpl
Show resolved
Hide resolved
tpgtools/overrides/compute/samples/networkfirewallpolicyassociation/regional_update.tf.tmpl
Show resolved
Hide resolved
tpgtools/overrides/compute/samples/networkfirewallpolicyassociation/regional_update.tf.tmpl
Show resolved
Hide resolved
fix whitespaces Co-authored-by: Riley Karson <rileykarson@google.com>
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 7 files changed, 1057 insertions(+), 31 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccDataLossPreventionJobTrigger_dlpJobTriggerBasicExample|TestAccComputeNetworkFirewallPolicyAssociation_GlobalHandWritten|TestAccComputeRegionNetworkFirewallPolicyAssociation_RegionalHandWritten|TestAccComputeForwardingRule_update |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
tpgtools/overrides/compute/beta/network_firewall_policy_association.yaml
Outdated
Show resolved
Hide resolved
tpgtools/overrides/compute/network_firewall_policy_association.yaml
Outdated
Show resolved
Hide resolved
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 7 files changed, 1057 insertions(+), 31 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccDataLossPreventionJobTrigger_dlpJobTriggerBasicExample|TestAccComputeForwardingRule_update |
all set now @rileykarson ? |
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.
LGTM, thanks!
…k_firewall_policy_association (GoogleCloudPlatform#6796) Co-authored-by: Riley Karson <rileykarson@google.com> Co-authored-by: Ghaleb Al-habian <galhabian@google.com>
…k_firewall_policy_association (GoogleCloudPlatform#6796) Co-authored-by: Riley Karson <rileykarson@google.com> Co-authored-by: Ghaleb Al-habian <galhabian@google.com>
…k_firewall_policy_association (GoogleCloudPlatform#6796) Co-authored-by: Riley Karson <rileykarson@google.com> Co-authored-by: Ghaleb Al-habian <galhabian@google.com>
This PR adds support for the network_firewall_policy_association resource, and its regional counterpart (region_network_firewall_policy_association). These resources have been integrated into the DCL already and requires DCL v1.26 .
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)