Skip to content

Fix Test and Update AuthZ policy resource to support new API signature#15070

Merged
NickElliot merged 13 commits into
GoogleCloudPlatform:mainfrom
maxi-cit:fix/update-authz-principals-field
Sep 29, 2025
Merged

Fix Test and Update AuthZ policy resource to support new API signature#15070
NickElliot merged 13 commits into
GoogleCloudPlatform:mainfrom
maxi-cit:fix/update-authz-principals-field

Conversation

@maxi-cit

@maxi-cit maxi-cit commented Sep 5, 2025

Copy link
Copy Markdown
Contributor

Fixes: hashicorp/terraform-provider-google#23036
Fixes: hashicorp/terraform-provider-google#23037

Due to the API update for AuthzPolicies, some limits have been revised and the signatures of certain fields have changed. The provider still accepts the old format, but calls will be rejected because of the API change. This PR aims to reconcile the provider with the new signature and deprecate the fields that are no longer available.

The deprecated fields should be removed in another PR for version 8.0 of the provider.

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

networksecurity: added `ip_blocks`  field to `google_network_security_authz_policy` resource
networksecurity: deprecated `ignore_case`, `exact`, `prefix`, `suffix` and `contains` on `google_network_security_authz_policy.http_rules.from.not_sources` and `google_network_security_authz_policy.http_rules.from.sources`. Use `location` instead.

@github-actions github-actions Bot requested a review from NickElliot September 5, 2025 15:11
@github-actions

github-actions Bot commented Sep 5, 2025

Copy link
Copy Markdown

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

Googlers: For automatic test runs see go/terraform-auto-test-runs.

@NickElliot, 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.

@NickElliot NickElliot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

largely lgtm pending test runs, but a bunch of changes to formatting are necessary

also if you could write a changelog note that would be great thanks!

Comment thread mmv1/products/networksecurity/AuthzPolicy.yaml Outdated
Comment thread mmv1/products/networksecurity/AuthzPolicy.yaml Outdated
Comment thread mmv1/products/networksecurity/AuthzPolicy.yaml Outdated
Comment thread mmv1/products/networksecurity/AuthzPolicy.yaml Outdated
Comment thread mmv1/products/networksecurity/AuthzPolicy.yaml Outdated
Comment thread mmv1/products/networksecurity/AuthzPolicy.yaml Outdated
@modular-magician

Copy link
Copy Markdown
Collaborator

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 ( 4 files changed, 974 insertions(+), 175 deletions(-))
google-beta provider: Diff ( 4 files changed, 974 insertions(+), 175 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 258 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_network_security_authz_policy (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_network_security_authz_policy" "primary" {
  http_rules {
    from {
      not_sources {
        ip_blocks {
          length = # value needed
          prefix = # value needed
        }
        principals {
          contains    = # value needed
          ignore_case = # value needed
          prefix      = # value needed
          suffix      = # value needed
        }
      }
      sources {
        ip_blocks {
          length = # value needed
          prefix = # value needed
        }
        principals {
          contains    = # value needed
          exact       = # value needed
          ignore_case = # value needed
          prefix      = # value needed
          suffix      = # value needed
        }
      }
    }
  }
}

@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 67
Passed tests: 61
Skipped tests: 4
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
  • TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyAdvancedExample
  • TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyHttpRules

Get to know how VCR tests work

@modular-magician

Copy link
Copy Markdown
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyAdvancedExample [Error message] [Debug log]
TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyHttpRules [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

@github-actions github-actions Bot requested a review from NickElliot September 18, 2025 21:32
@maxi-cit

maxi-cit commented Sep 18, 2025

Copy link
Copy Markdown
Contributor Author

Hello @NickElliot, I added the missing IpBlocks to test. However I am not sure on how to proceed with the test check for untested fields. The old fields were removed from the API signature but I understand I will have to follow the deprecation steps to get them removed in provider ver 8.0

@modular-magician

Copy link
Copy Markdown
Collaborator

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 ( 4 files changed, 995 insertions(+), 176 deletions(-))
google-beta provider: Diff ( 4 files changed, 995 insertions(+), 176 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 258 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_network_security_authz_policy (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_network_security_authz_policy" "primary" {
  http_rules {
    from {
      not_sources {
        principals {
          contains    = # value needed
          ignore_case = # value needed
          prefix      = # value needed
          suffix      = # value needed
        }
      }
      sources {
        principals {
          contains    = # value needed
          exact       = # value needed
          ignore_case = # value needed
          prefix      = # value needed
          suffix      = # value needed
        }
      }
    }
  }
}

@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 67
Passed tests: 61
Skipped tests: 4
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
  • TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyAdvancedExample
  • TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyHttpRules

Get to know how VCR tests work

@modular-magician

Copy link
Copy Markdown
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyAdvancedExample [Error message] [Debug log]
TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyHttpRules [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

@github-actions

Copy link
Copy Markdown

@NickElliot This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

@NickElliot NickElliot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the tests are failing due to errors about prefix, i.e.

          on terraform_plugin_test.tf line 135, in resource "google_network_security_authz_policy" "default":
         135:           prefix = "10.1.6.0"
        
        Inappropriate value for attribute "prefix": a number is required.

can you fix your additions to the tests? thanks!

…incipals-field

sync merge from upstream/main
@maxi-cit

Copy link
Copy Markdown
Contributor Author

Hi @NickElliot, this is not consistent with API docs.

https://cloud.google.com/load-balancing/docs/reference/network-security/rest/v1beta1/projects.locations.authzPolicies#IpBlock

I am checking this with the appropiate team.

@github-actions github-actions Bot requested a review from NickElliot September 25, 2025 19:09
@maxi-cit

Copy link
Copy Markdown
Contributor Author

Hello @NickElliot, could you run tests please? I already talked with team regarding some of the updates and I updated the PR appropiately.

@modular-magician

Copy link
Copy Markdown
Collaborator

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 ( 4 files changed, 924 insertions(+), 206 deletions(-))
google-beta provider: Diff ( 4 files changed, 924 insertions(+), 206 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 270 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_network_security_authz_policy (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_network_security_authz_policy" "primary" {
  http_rules {
    from {
      not_sources {
        principals {
          contains    = # value needed
          ignore_case = # value needed
          prefix      = # value needed
          principal {
            contains = # value needed
            prefix   = # value needed
            suffix   = # value needed
          }
          suffix = # value needed
        }
      }
      sources {
        principals {
          contains    = # value needed
          exact       = # value needed
          ignore_case = # value needed
          prefix      = # value needed
          principal {
            contains = # value needed
            prefix   = # value needed
            suffix   = # value needed
          }
          suffix = # value needed
        }
      }
    }
  }
}

@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 68
Passed tests: 62
Skipped tests: 4
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
  • TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyAdvancedExample
  • TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyHttpRules

Get to know how VCR tests work

@modular-magician

Copy link
Copy Markdown
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyHttpRules [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🔴 Tests failed during RECORDING mode:
TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyAdvancedExample [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

NickElliot
NickElliot previously approved these changes Sep 26, 2025

@NickElliot NickElliot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@github-actions github-actions Bot requested a review from NickElliot September 26, 2025 22:32
@modular-magician

Copy link
Copy Markdown
Collaborator

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 ( 5 files changed, 942 insertions(+), 208 deletions(-))
google-beta provider: Diff ( 5 files changed, 942 insertions(+), 208 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 270 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_network_security_authz_policy (2 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_network_security_authz_policy" "primary" {
  http_rules {
    from {
      not_sources {
        principals {
          contains    = # value needed
          exact       = # value needed
          ignore_case = # value needed
          prefix      = # value needed
          principal {
            contains = # value needed
            prefix   = # value needed
            suffix   = # value needed
          }
          suffix = # value needed
        }
      }
      sources {
        principals {
          contains    = # value needed
          exact       = # value needed
          ignore_case = # value needed
          prefix      = # value needed
          principal {
            contains = # value needed
            prefix   = # value needed
            suffix   = # value needed
          }
          suffix = # value needed
        }
      }
    }
  }
}

@modular-magician

Copy link
Copy Markdown
Collaborator

Tests analytics

Total tests: 68
Passed tests: 63
Skipped tests: 4
Affected tests: 1

Click here to see the affected service packages
  • networksecurity

Action taken

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

Get to know how VCR tests work

@modular-magician

Copy link
Copy Markdown
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccNetworkSecurityAuthzPolicy_networkServicesAuthzPolicyAdvancedExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

@NickElliot NickElliot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@NickElliot NickElliot added this pull request to the merge queue Sep 29, 2025
Merged via the queue into GoogleCloudPlatform:main with commit 15e896b Sep 29, 2025
28 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants