Add Managed Kafka Acl resource and tests.

[amfisher-404]

Description: Added a new Managed Kafka Acl terraform resource to the Google Terraform provider. Includes a basic create and update test.

Issue: https://b.corp.google.com/issues/374342184

`google_managed_kafka_acl`

[github-actions]

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.

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

[modular-magician]

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 ( 6 files changed, 1055 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 1055 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 139 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 137 insertions(+))

Missing test report

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

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

resource "google_managed_kafka_acl" "primary" {
  etag = # value needed
}

[modular-magician]

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 ( 6 files changed, 1056 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 1055 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 139 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 137 insertions(+))

Missing test report

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

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

resource "google_managed_kafka_acl" "primary" {
  etag = # value needed
}

[modular-magician]

Tests analytics

Total tests: 9
Passed tests: 7
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages

• managedkafka

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

• TestAccManagedKafkaAcl_managedkafkaAclBasicExample
• TestAccManagedKafkaAcl_update

Get to know how VCR tests work

[modular-magician]

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 ( 6 files changed, 1055 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 1055 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 139 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 137 insertions(+))

Missing test report

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

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

resource "google_managed_kafka_acl" "primary" {
  etag = # value needed
}

[modular-magician]

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 ( 6 files changed, 1055 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 1055 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 139 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 137 insertions(+))

Missing test report

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

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

resource "google_managed_kafka_acl" "primary" {
  etag = # value needed
}

[modular-magician]

Tests analytics

Total tests: 9
Passed tests: 7
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages

• managedkafka

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

• TestAccManagedKafkaAcl_managedkafkaAclBasicExample
• TestAccManagedKafkaAcl_update

Get to know how VCR tests work

[modular-magician]

Tests analytics

Total tests: 9
Passed tests: 7
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages

• managedkafka

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

• TestAccManagedKafkaAcl_managedkafkaAclBasicExample
• TestAccManagedKafkaAcl_update

Get to know how VCR tests work

[modular-magician]

Tests analytics

Total tests: 9
Passed tests: 7
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages

• managedkafka

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

• TestAccManagedKafkaAcl_managedkafkaAclBasicExample
• TestAccManagedKafkaAcl_update

Get to know how VCR tests work

[modular-magician]

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 ( 6 files changed, 1056 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 1056 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 139 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 137 insertions(+))

[modular-magician]

Tests analytics

Total tests: 9
Passed tests: 7
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages

• managedkafka

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

• TestAccManagedKafkaAcl_managedkafkaAclBasicExample
• TestAccManagedKafkaAcl_update

Get to know how VCR tests work

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccManagedKafkaAcl_managedkafkaAclBasicExample [Debug log]
TestAccManagedKafkaAcl_update [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

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccManagedKafkaAcl_managedkafkaAclBasicExample [Debug log]
TestAccManagedKafkaAcl_update [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

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccManagedKafkaAcl_managedkafkaAclBasicExample [Debug log]
TestAccManagedKafkaAcl_update [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

[modular-magician]

🟢 Tests passed during RECORDING mode:
TestAccManagedKafkaAcl_managedkafkaAclBasicExample [Debug log]
TestAccManagedKafkaAcl_update [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

[slevenick]

required: true?

[amfisher-404]

To clarify: For an Array of NestedObject where the array must not be empty, should the required annotation go on the aclEntries array field, or within the item_type NestedObject part? (I'm thinking the former.)

[slevenick]

The former.

I would guess most of the nested fields should also have required on them as well. I'd guess if someone specifies a specific acl_entry it must have a principal, so principal should be required. Similar for any other fields that must be specified for an individual entry.

[amfisher-404]

Thanks!

I would guess most of the nested fields should also have required on them as well.

Agreed. I added required for the fields without default values in prior commit ddb99a2.

[slevenick]

I think this should be output_only unless there's a reason to specify etag within Terraform

[amfisher-404]

The issue is, the etag does need to be provided to the API in the ACL for future updates to succeed.

Test failure with etag as output_only:

resource_managed_kafka_acl_test.go:19: Step 3/4 error: Error running apply: exit status 1
        
        Error: Error updating Acl "projects/ci-test-project-188019/locations/us-central1/clusters/tf-test-my-cluster8wqckoe84a/acls/topic/tf-test-my-acl8wqckoe84a": googleapi: Error 400: The request was invalid: etag must be specified on UpdateAcl. Use GetAcl to get the current etag
        Details:
        [
          {
            "@type": "type.googleapis.com/google.rpc.BadRequest",
            "fieldViolations": [
              {
                "field": "etag"
              }
            ]
          },
          {
            "@type": "type.googleapis.com/google.rpc.RequestInfo",
            "requestId": "4b3e5414714372b5"
          }
        ]
        
          with google_managed_kafka_acl.example,
          on terraform_plugin_test.tf line 18, in resource "google_managed_kafka_acl" "example":
          18: resource "google_managed_kafka_acl" "example" {

This is why I had gone with default_from_api, so the API-provided value is persisted in future requests. However, IMO it doesn't make sense for tests to exercise the user providing this field. Is there some better alternative you'd propose?

[slevenick]

Does the API not accept requests without the etag set? In general etag should be an optional request parameter: https://google.aip.dev/154

If it's required on purpose you may be able to mark the etag field as output: true and change the type to type: Fingerprint. I believe that adds the field to every request

[amfisher-404]

We've implemented the etag on the resource level (not the request level) as specified in AIP-154. The etag is only required on Update requests, to meet the need for consistency/concurrency control: https://screenshot.googleplex.com/8NobwFxUKjof6y3. As such, there are no REQUIRED field annotations on etag; the API will return INVALID_ARGUMENT if the etag in UpdateAcl is missing.

Thanks for the suggestion. Marking etag as output:true with type:Fingerprint achieves what we're after:

• Users can't provide the field themselves: https://paste.googleplex.com/6450133052686336
• Its value is persisted from the API and sent on subsequent UpdateAcl requests: https://paste.googleplex.com/6546873382600704

[modular-magician]

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

🟢 No issues found for passed tests after REPLAYING rerun.

---

🔴 Tests failed during RECORDING mode:
TestAccManagedKafkaAcl_update [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

[modular-magician]

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 ( 6 files changed, 1033 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 1033 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 129 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 137 insertions(+))

[modular-magician]

Tests analytics

Total tests: 9
Passed tests: 8
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

• managedkafka

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

• TestAccManagedKafkaAcl_update

Get to know how VCR tests work

[modular-magician]

🔴 Tests failed during RECORDING mode:
TestAccManagedKafkaAcl_update [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

[modular-magician]

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 ( 6 files changed, 1055 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 1055 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 139 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 137 insertions(+))

Missing test report

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

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

resource "google_managed_kafka_acl" "primary" {
  etag = # value needed
}

[modular-magician]

Tests analytics

Total tests: 9
Passed tests: 9
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

• managedkafka

🟢 All tests passed!

View the build log

[slevenick]

Does the API not accept requests without the etag set? In general etag should be an optional request parameter: https://google.aip.dev/154

If it's required on purpose you may be able to mark the etag field as output: true and change the type to type: Fingerprint. I believe that adds the field to every request

[modular-magician]

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 ( 6 files changed, 1067 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 1067 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 139 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 137 insertions(+))

[modular-magician]

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 ( 6 files changed, 1067 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 6 files changed, 1067 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 139 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 137 insertions(+))

[modular-magician]

Tests analytics

Total tests: 9
Passed tests: 9
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

• managedkafka

🟢 All tests passed!

View the build log

[modular-magician]

Tests analytics

Total tests: 9
Passed tests: 9
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

• managedkafka

🟢 All tests passed!

View the build log

[slevenick]

Thanks!