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.

Sign up for GitHub

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

Jump to bottom

Added "disabled" support to firewall endpoint association resource #10246

Merged

rileykarson merged 8 commits into GoogleCloudPlatform:main from matheusaleixo-cit:add-disabled-to-firewall_endpoint_association

Apr 3, 2024

Contributor

matheusaleixo-cit commented Mar 20, 2024 •

edited by rileykarson

Loading

Adds support for optional 'disabled' field to 'google_network_security_firewall_endpoint_association'

Fixes: hashicorp/terraform-provider-google#17610
Fixes: hashicorp/terraform-provider-google#17699

Also fixes to the test 'testAccNetworkSecurityFirewallEndpointAssociation_basic' that started failing due to recent changes to the API, such as the addition of the 'billingProjectId' field to the Firewall Endpoint. (Similar to hashicorp/terraform-provider-google#17396)

Release Note Template for Downstream PRs (will be copied)

networksecurity: Added 'disabled' field to 'google_network_security_firewall_endpoint_association' resource;

networksecurity: fixed an issue where `google_network_security_firewall_endpoint_association` resources could not be created due to a bad parameter

matheusaleixo-cit added 2 commits

March 20, 2024 16:57


          - Added new field "disabled" to Firewall Endpoint Association;

1857c0a

- Fixed create_url sending incorrect parameter (firewallEndpointId ~> firewallEndpointAssociationId);

Tests:
- Added new test for "disabled" field;
- Added "billingProjectId" field to the "google_network_security_firewall_endpoint" resource";
- Changed "parent" attribute in "google_network_security_firewall_endpoint_association";
- Added "disabled" argument to update function;
- Cleaned Sprintf formatting;


          Merge branch 'main' into add-disabled-to-firewall_endpoint_association

4a4e053

google-cla bot commented Mar 20, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

matheusaleixo-cit marked this pull request as ready for review

March 21, 2024 12:08

github-actions bot commented Mar 21, 2024

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@rileykarson, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

github-actions bot requested a review from rileykarson

March 21, 2024 12:09

rileykarson reviewed

View reviewed changes

mmv1/products/networksecurity/FirewallEndpointAssociation.yaml Show resolved Hide resolved

mmv1/products/networksecurity/FirewallEndpointAssociation.yaml Show resolved Hide resolved

...services/networksecurity/resource_network_security_firewall_endpoint_association_test.go.erb Outdated

               	orgId := envvar.GetTestOrgFromEnv(t)
               	randomSuffix := acctest.RandString(t, 10)
+                billingProjectId := envvar.GetTestProjectFromEnv()

Member

rileykarson Mar 21, 2024

nit: spacing (spaces -> tabs)

...services/networksecurity/resource_network_security_firewall_endpoint_association_test.go.erb Outdated

+              		Steps: []resource.TestStep{
+              			{
+              				Config: testAccNetworkSecurityFirewallEndpointAssociation_basic(randomSuffix, orgId, billingProjectId, projectNumberId),
+                      Check: resource.ComposeTestCheckFunc(

Member

rileykarson Mar 21, 2024

nit: spacing (spaces -> tabs)

...services/networksecurity/resource_network_security_firewall_endpoint_association_test.go.erb Outdated

               	return fmt.Sprintf(`
               resource "google_compute_network" "foobar" {
-              	provider                = google-beta
-                  name                    = "tf-test-my-vpc%s"
+              	provider                  = google-beta

Member

rileykarson Mar 21, 2024

nit: spacing (tabs -> spaces)

...services/networksecurity/resource_network_security_firewall_endpoint_association_test.go.erb Outdated

-              	provider                = google-beta
-                  name                    = "tf-test-my-vpc%s"
+              	provider                  = google-beta
+                  name                    = "tf-test-my-vpc%[2]s"

Member

rileykarson Mar 21, 2024

Can you switch to context / Nprintfinstead of Sprintf? This will be much easier to read using that formatting instead.

...services/networksecurity/resource_network_security_firewall_endpoint_association_test.go.erb Outdated Show resolved Hide resolved


          - Converted tests to now use Nprintf;

9ea5dfc

- Code spacing;

github-actions bot requested a review from rileykarson

March 22, 2024 12:29

matheusaleixo-cit added 3 commits

March 22, 2024 14:44


          - Code spacing;

a1393d1


          - Code spacing;

66d0dbf


          - Reverted changes to "network" field in thest config;

392af99

- Removed explicit dependency now unnecessary due to these changes;

slevenick reviewed

View reviewed changes

...services/networksecurity/resource_network_security_firewall_endpoint_association_test.go.erb Outdated

		@@ -20,28 +21,91 @@ func TestAccNetworkSecurityFirewallEndpointAssociations_basic(t *testing.T) {
		acctest.SkipIfVcr(t)

Contributor

slevenick Mar 28, 2024

This can be removed I believe. I looked into this resource in #10308 and though I didn't get the test to successfully pass I believe VCR should work for this test.

Member

rileykarson Mar 28, 2024

@matheusaleixo-cit let's apply this change- it'll make our presubmits run the test.

slevenick mentioned this pull request

google_network_security_firewall_endpoint_association throwing `Error 400: Invalid JSON payload received. Unknown name "firewallEndpointId" hashicorp/terraform-provider-google#17699

Closed

Contributor

slevenick commented Mar 28, 2024

Added to the PR description that this also fixes hashicorp/terraform-provider-google#17699


          - Removed Vcr skip from basic and disabled tests;

2d9b69c

github-actions bot requested a review from slevenick

April 1, 2024 15:18


          Merge branch 'main' into add-disabled-to-firewall_endpoint_association

67ad957

Contributor

slevenick commented Apr 1, 2024

/gcbrun

modular-magician added awaiting-approval service/network-security-distributed-firewall and removed awaiting-approval labels

Collaborator

modular-magician commented Apr 2, 2024

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 1 file changed, 6 insertions(+))
google-beta provider: Diff ( 3 files changed, 160 insertions(+), 48 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 10 insertions(+))

Collaborator

modular-magician commented Apr 2, 2024

Tests analytics

Total tests: 37
Passed tests: 34
Skipped tests: 1
Affected tests: 2

Click here to see the affected service packages

networksecurity

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests

TestAccNetworkSecurityFirewallEndpointAssociations_basic|TestAccNetworkSecurityFirewallEndpointAssociations_disabled

Get to know how VCR tests work

Collaborator

modular-magician commented Apr 2, 2024

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$
TestAccNetworkSecurityFirewallEndpointAssociations_basic[Debug log]
TestAccNetworkSecurityFirewallEndpointAssociations_disabled[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log or the debug log for each test

Contributor Author

matheusaleixo-cit commented Apr 3, 2024

Hello @rileykarson @slevenick! Are there any other pending issues preventing the merge of this PR?

rileykarson approved these changes

View reviewed changes

Member

rileykarson left a comment

LGTM, thanks! No further action needed.

mmv1/products/networksecurity/FirewallEndpointAssociation.yaml Show resolved Hide resolved

rileykarson merged commit 4b61bc2 into GoogleCloudPlatform:main

14 checks passed

This was referenced Apr 3, 2024

Added "disabled" support to firewall endpoint association resource hashicorp/terraform-provider-google-beta#7184

Merged

Added "disabled" support to firewall endpoint association resource hashicorp/terraform-provider-google#17762

Merged

modular-magician mentioned this pull request

Added "disabled" support to firewall endpoint association resource GoogleCloudPlatform/terraform-google-conversion#2127

Merged

matheusaleixo-cit deleted the add-disabled-to-firewall_endpoint_association branch

April 4, 2024 20:23

cmfeng pushed a commit to cmfeng/cmfeng-magic-modules that referenced this pull request


          Added "disabled" support to firewall endpoint association resource (G…

caf964b

…oogleCloudPlatform#10246)

hao-nan-li pushed a commit to hao-nan-li/magic-modules that referenced this pull request


          Added "disabled" support to firewall endpoint association resource (G…

af9a4b1

…oogleCloudPlatform#10246)

balanaguharsha pushed a commit to balanaguharsha/magic-modules that referenced this pull request


          Added "disabled" support to firewall endpoint association resource (G…

91f32eb

…oogleCloudPlatform#10246)

balanaguharsha pushed a commit to balanaguharsha/magic-modules that referenced this pull request


          Added "disabled" support to firewall endpoint association resource (G…

314ac36

…oogleCloudPlatform#10246)

balanaguharsha pushed a commit to balanaguharsha/magic-modules that referenced this pull request


          Added "disabled" support to firewall endpoint association resource (G…

de048f1

…oogleCloudPlatform#10246)

BBBmau pushed a commit to BBBmau/magic-modules that referenced this pull request


          Added "disabled" support to firewall endpoint association resource (G…

336afd4

…oogleCloudPlatform#10246)

pawelJas pushed a commit to pawelJas/magic-modules that referenced this pull request


          Added "disabled" support to firewall endpoint association resource (G…

b47ab04

…oogleCloudPlatform#10246)

pengq-google pushed a commit to pengq-google/magic-modules that referenced this pull request


          Added "disabled" support to firewall endpoint association resource (G…

a727a40

…oogleCloudPlatform#10246)

Cheriit pushed a commit to Cheriit/magic-modules that referenced this pull request


          Added "disabled" support to firewall endpoint association resource (G…

01d1b62

…oogleCloudPlatform#10246)

pcostell pushed a commit to pcostell/magic-modules that referenced this pull request


          Added "disabled" support to firewall endpoint association resource (G…

5b27734

…oogleCloudPlatform#10246)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

service/network-security-distributed-firewall