Add securitySettings to the backendService

[sanjaypujare]

Fixes: hashicorp/terraform-provider-google#9545

Need the new securitySettings field in backendService in Compute.

If this PR is for Terraform, I acknowledge that I have: (not applicable for draft PR)

Release Note Template for Downstream PRs (will be copied)

compute: added support for `security_settings` to `google_compute_backend_service`

[modular-magician]

Oops! It looks like you're using an unknown release-note type in your changelog entries:

• REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

[modular-magician]

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review.

Thanks for your contribution! A human will be with you soon.

@slevenick, please review this PR or find an appropriate assignee.

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 124 insertions(+))
Terraform Beta: Diff ( 2 files changed, 124 insertions(+))
Ansible: Diff ( 2 files changed, 88 insertions(+))
TF Conversion: Diff ( 1 file changed, 40 insertions(+))
Inspec: Diff ( 5 files changed, 49 insertions(+))

[modular-magician]

I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccAssuredWorkloadsWorkload_basic|TestAccAssuredWorkloadsWorkload_full|TestAccBigqueryDataTransferConfig|TestAccBigqueryReservationReservation_bigqueryReservationBasicExample|TestAccBillingBudget_budgetFilterProjectsOrdering|TestAccBillingBudget_billingBudgetUpdate|TestAccCloudBuildTrigger_pubsub_config|TestAccCloudBuildTrigger_webhook_config|TestAccComposerEnvironment_withMaintenanceWindow|TestAccComputeBackendService_withBackend|TestAccComputeBackendService_withBackendAndMaxUtilization|TestAccComputeBackendService_withBackendAndIAP|TestAccComputeBackendService_withMaxConnections|TestAccComputeBackendService_withMaxConnectionsPerInstance|TestAccComputeBackendService_withMaxRatePerEndpoint|TestAccComputeBackendService_withMaxConnectionsPerEndpoint|TestAccComputeGlobalForwardingRule_globalForwardingRuleInternalExample|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccComputeHaVpnGateway_computeHaVpnGatewayEncryptedInterconnectExample|TestAccComputeInstanceGroup_rename|TestAccComputeRouter_computeRouterEncryptedInterconnectExample|TestAccContainerCluster_backend|TestAccContainerNodePool_withGPU|TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplateBasicExample|TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplateUpdate|TestAccDialogflowCXFlow_dialogflowcxFlowFullExample|TestAccDialogflowCXVersion_dialogflowcxVersionFullExample|TestAccDialogflowCXFlow_update|TestAccDialogflowCXVersion_update|TestAccGKEHubMembership_gkehubMembershipBasicExample|TestAccGKEHubMembership_gkehubMembershipIssuerExample|TestAccNetworkServicesEdgeCacheKeyset_networkServicesEdgeCacheKeysetBasicExample|TestAccNetworkServicesEdgeCacheOrigin_networkServicesEdgeCacheOriginBasicExample|TestAccNetworkServicesEdgeCacheOrigin_networkServicesEdgeCacheOriginAdvancedExample|TestAccNetworkServicesEdgeCacheOrigin_updateAndImport|TestAccNetworkServicesEdgeCacheService_networkServicesEdgeCacheServiceBasicExample|TestAccNetworkServicesEdgeCacheService_networkServicesEdgeCacheServiceAdvancedExample|TestAccNetworkServicesEdgeCacheService_updateAndImport|TestAccPrivatecaCertificate_privatecaCertificateCsrExample|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthorityBasicExample|TestAccPrivatecaCertificate_privatecaCertificateConfigExample|TestAccPrivatecaCertificate_privatecaCertificateNoAuthorityExample|TestAccSecurityCenterNotificationConfig_sccNotificationConfigBasicExample You can view the result here: https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=198580

[modular-magician]

Tests failed during RECORDING mode: TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplateBasicExample|TestAccGKEHubMembership_gkehubMembershipBasicExample|TestAccGKEHubMembership_gkehubMembershipIssuerExample|TestAccPrivatecaCertificate_privatecaCertificateCsrExample|TestAccNetworkServicesEdgeCacheService_networkServicesEdgeCacheServiceAdvancedExample Please fix these to complete your PR

[sanjaypujare]

Tests failed during RECORDING mode: TestAccDataLossPreventionDeidentifyTemplate_dlpDeidentifyTemplateBasicExample|TestAccGKEHubMembership_gkehubMembershipBasicExample|TestAccGKEHubMembership_gkehubMembershipIssuerExample|TestAccPrivatecaCertificate_privatecaCertificateCsrExample|TestAccNetworkServicesEdgeCacheService_networkServicesEdgeCacheServiceAdvancedExample Please fix these to complete your PR

Where can I see the test failure output?

Also regarding this:

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:
Terraform GA: Diff ( 2 files changed, 124 insertions(+))
Terraform Beta: Diff ( 2 files changed, 124 insertions(+))

Should I open a PR for these other repos once this PR is merged?

CC @rileykarson

[rileykarson]

None of those failures are related, you can ignore them. We're testing most of GCP and over nearly 2000 tests hitting multiple APIs, flakes happen most of the time. And nope, no PRs required- the @modular-magician account makes them automatically after merging.

Can you add or modify a test to exercise this field? We'll want to set value(s) and test updating it- either turning it on, off, or both are great candidates for updates to test. There should be examples of writing them internally in go/terraform-contribution-guide.

[sanjaypujare]

Can you add or modify a test to exercise this field? We'll want to set value(s) and test updating it- either turning it on, off, or both are great candidates for updates to test. There should be examples of writing them internally in go/terraform-contribution-guide.

I looked at the guide and I figured out that a test will need to be added to github.com/hashicorp/terraform-provider-google-beta/google-beta/resource_compute_backend_service_generated_test.go and/or github.com/hashicorp/terraform-provider-google/google/resource_compute_backend_service_generated_test.go . Can you please confirm? If that's true then I have a couple of questions:

• that's a different repo and those tests won't pass until this PR is merged so we have a circular dependency
• can I just create a PR in those other repos so you can look at the new tests and use that to approve this PR?

Just want to know how it's going to work. Or if I got the location wrong please let me know where the tests should reside.

[rileykarson]

You create test config templates in https://github.com/GoogleCloudPlatform/magic-modules/tree/master/mmv1/templates/terraform/examples and register them in the terraform.yaml file containing the resource. They'll get generated by Magic Modules as well.

[google-cla]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[google-cla]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[sanjaypujare]

You create test config templates in https://github.com/GoogleCloudPlatform/magic-modules/tree/master/mmv1/templates/terraform/examples and register them in the terraform.yaml file containing the resource. They'll get generated by Magic Modules as well.

@rileykarson I think there is a problem: so the new field securitySettings is a nested-object containing a clientTlsPolicy resource ref (string). But I cannot create this resource using the magic module artifacts. I can test the subjectAltNames field but probably not clientTlsPolicy unless there is a way I am not aware of. Pls let me know

[rileykarson]

If subjectAltNames can be set while clientTlsPolicy isn't set, let's do that. I'll test manually that clientTlsPolicy works as intended before merging.

For clientTlsPolicy are we able to make it the ResourceRef type while specifying an invalid resource value? I forget if that's possible or not, but Terraform shouldn't actually use the value and will apply some useful rules we will need to specify manually otherwise.

[sanjaypujare]

If subjectAltNames can be set while clientTlsPolicy isn't set, let's do that. I'll test manually that clientTlsPolicy works as intended before merging.

I just talked to the dev who implemented this stuff and he says subjectAltNames with empty clientTlsPolicy isn't allowed and will be rejected by the backend.

For clientTlsPolicy are we able to make it the ResourceRef type while specifying an invalid resource value? I forget if that's possible or not, but Terraform shouldn't actually use the value and will apply some useful rules we will need to specify manually otherwise.

I see 2 reasons for not making it a ResourceRef:

• ClientTlsPolicy is not in magic modules yet so it cannot be checked by the framework?
• if you just send a name (not a full URI) I believe the backend looks up the name and replaces that name with the URI before storing the value. So for input it accepts a non-ResourceRef I believe

[rileykarson]

Terraform doesn't actually check the resource or imports value of a ResourceRef, that behaviour was only used by some of the other tools MM is used for. Adding the ResourceRef type will allow Terraform to mediate between shortname formats / versioned URLs and the versioned URLs returned by the API, so point 2. is more signal we should use one.

If the field is a raw string, equivalent fields will be see as different. For example: if the user sends my-network and the API sends back https://www.googleapis.com/compute/v1/projects/my-project/global/networks/my-network a string will see a diff, but a resourceref won't.

[sanjaypujare]

Terraform doesn't actually check the resource or imports value of a ResourceRef, that behaviour was only used by some of the other tools MM is used for. Adding the ResourceRef type will allow Terraform to mediate between shortname formats / versioned URLs and the versioned URLs returned by the API, so point 2. is more signal we should use one.

If the field is a raw string, equivalent fields will be see as different. For example: if the user sends my-network and the API sends back https://www.googleapis.com/compute/v1/projects/my-project/global/networks/my-network a string will see a diff, but a resourceref won't.

Good point. I will make it a ResourceRef then in my next commit.

[sanjaypujare]

Good point. I will make it a ResourceRef then in my next commit.

What does it take to make ClientTlsPolicy a ResourceRef? I had the following:

         - !ruby/object:Api::Type::ResourceRef
            name: 'clientTlsPolicy'
            resource: 'ClientTlsPolicy'
            imports: 'selfLink'

But I get errors while building:

bundler: failed to load command: compiler (compiler)
RuntimeError: Missing 'ClientTlsPolicy'
  /usr/local/google/home/sanjaypujare/Projects/magic-modules/mmv1/api/type.rb:559:in `check_resource_ref_exists'
  /usr/local/google/home/sanjaypujare/Projects/magic-modules/mmv1/api/type.rb:528:in `validate'
  /usr/local/google/home/sanjaypujare/Projects/magic-modules/mmv1/google/yaml_validator.rb:121:in `check_property_value'
  /usr/local/google/home/sanjaypujare/Projects/magic-modules/mmv1/google/yaml_validator.rb:75:in `block in check'

[rileykarson]

I would just point to the 'Region' resource and comment that that's incorrect and it should be 'ClientTLSPolicy'- I'll verify that the right code gets generated. I'm very likely to remove the unused portions from MM as a whole in the next couple weeks anyways.

[google-cla]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[sanjaypujare]

I am not able to see the 2 failures because of perms (for Project cloudbuild.builds.get ?)

You do not have sufficient permissions to view this page
There was an error while loading /cloud-build?folder=&organizationId=&project=673497134629.

[sanjaypujare]

@rileykarson pls do let me know what else is needed for this PR

[rileykarson]

Because of changes as part of the fixit our CI environment has been spotty, I am waiting for that to get fixed.

[sanjaypujare]

Because of changes as part of the fixit our CI environment has been spotty, I am waiting for that to get fixed.

Thanks for letting me know.

[rileykarson]

/gcbrun

[rileykarson]

Can you rebase? Our CI has mostly calmed down after the fixit, and the fix for the generate-diffs failure is in. The last outstanding thing is that the Magician may insist it failed to cancel builds incorrectly is all.

[sanjaypujare]

Can you rebase? Our CI has mostly calmed down after the fixit, and the fix for the generate-diffs failure is in. The last outstanding thing is that the Magician may insist it failed to cancel builds incorrectly is all.

Just rebased. Hopefully this is ready to go now. Pls let me know if there are any issues.

[modular-magician]

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 22 insertions(+))
Terraform Beta: Diff ( 3 files changed, 134 insertions(+), 1 deletion(-))

[modular-magician]

Error trying to cancel build ()

[rileykarson]

Do you have a valid gcloud yaml kicking around? I'm trying to provision a ClientTLSPolicy one-off to test this, and:

• API explorer is broken on https://cloud.google.com/traffic-director/docs/reference/network-security/rest/v1beta1/projects.locations.clientTlsPolicies/create
• gcloud doesn't support create, just the import/export function with no examples of the format: https://cloud.google.com/sdk/gcloud/reference/beta/network-security/client-tls-policies
• The resource isn't supported in the Console
• Config Connector takes a lot of setup / connection munging

I checked multiple user guides and found no examples, either.

To be clear, testing this manually is a requirement for me to merge it.

[sanjaypujare]

I checked multiple user guides and found no examples, either.

To be clear, testing this manually is a requirement for me to merge it.

Sure, actually there is a UG https://cloud.google.com/traffic-director/docs/security-proxyless-setup#configuring_mtls_on_the_client_side which has steps for:

• creating a client-tls-policy (steps 1 & 2)
• creating a security-settings snippet (step 3)
• attaching the security-settings to a backend-service by exporting, concatenating the snippet, and re-importing (step 4)

That should answer your questions? If not, let me know

[rileykarson]

Alright, tested manually. Details included below, for my own sake.

See my comment inline - the description of the field suggests you may have missed that Terraform mirrors the API structure, and this change only affects the Global BackendService resource. Please ack whether this was intended or not.

---

Base config:

resource "google_compute_backend_service" "default" {
  provider              = google-beta
  name                  = "backend-service"
  health_checks         = [google_compute_health_check.health_check.id]
  protocol              = "GRPC"
  load_balancing_scheme = "INTERNAL_SELF_MANAGED"

  security_settings {
    client_tls_policy = "my-client-tls-policy2"
    subject_alt_names = [
      "spiffe://my-project.svc.id.goog/ns/default/sa/example-grpc-server", "spiffe://my-project.svc.id.goog/ns/default/sa/example-grpc-server2"
    ]
    #subject_alt_names = []
  }
}

resource "google_compute_health_check" "health_check" {
  provider = google-beta

  name = "health-check"
  grpc_health_check {
    port = 80
  }
}

gcloud config to set up the policy out of band was:

name: "client_mtls_policy"
clientCertificate:
  certificateProviderInstance:
    pluginInstance: google_cloud_private_spiffe
serverValidationCa:
- certificateProviderInstance:
    pluginInstance: google_cloud_private_spiffe

w/ command

gcloud beta network-security client-tls-policies import my-client-tls-policy2 --source=my-client-tls-policy.yaml --location=global

I tested the following:

• Provisioning the config above
• Adding and removing the feature entirely from config
• Changing the ClientTLSPolicy to one that did not exist
  • Note that this had unusual behaviour, as the request was accepted and the following returned after waiting on an operation: Error: Error waiting for Updating BackendService: Invalid value for field 'global_network_security_resources.client_tls_policy_resource.client_tls_policy.name': ''. Cannot delete ClientTlsPolicy resource which is in use by BackendService resource BACKEND_SERVICE/1234567890.backend-service
• Changing the ClientTLSPolicy to one that did exist
• Adding 1 entry to alt names
• Removing 1 entry from alt names
• Adding 2 entries to alt names
• Removing 2 entries from alt names

[rileykarson]

Note: you have only added this to global backend service- is that intentional?

[sanjaypujare]

Yes, because we need support for global BES urgently (as requested by the user/customer). We can add support for regional BES later once there is need for it (and we confirm the feature works with those)

[sanjaypujare]

Alright, tested manually. Details included below, for my own sake.

Thanks.

...only affects the Global BackendService resource. Please ack whether this was intended or not.

yes.

...

• Note that this had unusual behaviour, as the request was accepted and the following returned after waiting on an operation: Error: Error waiting for Updating BackendService: Invalid value for field 'global_network_security_resources.client_tls_policy_resource.client_tls_policy.name': ''. Cannot delete ClientTlsPolicy resource which is in use by BackendService resource BACKEND_SERVICE/1234567890.backend-service

yes I have seen this error as well and it seems to be a bug in the gcloud API since it doesn't make sense. I will file an internal bug.

....

• Removing 2 entries from alt names

Looks good. Thanks a lot!

[sanjaypujare]

Thanks a lot !