App check debug token

[rainshen49]

Add support for App Check debug tokens. Part of hashicorp/terraform-provider-google#17095

Note to reviewers: I used a trick to populate a derived field debug_token_id in name's flattening phase. I've tried other hooks such as custom decoders, post_create, etc, but they are injected in awkward places i.e. after the reference of debug_token_id to populate the id, which leads to more overriding code. This one minimizes the amount of custom code and sits just right. I'm open to suggestions if there are better ways. I see an existing similar usage in

magic-modules/mmv1/templates/terraform/custom_flatten/bigtable_app_profile_routing.erb

Line 25 in 9c04e74

if err := d.Set("multi_cluster_routing_cluster_ids", v.(map[string]interface{})["clusterIds"]); err != nil {

Release Note Template for Downstream PRs (will be copied)

`google_firebase_app_check_debug_token`

[modular-magician]

Hello! I am a robot. It looks like you are a: Community Contributor Googler Core Contributor. Tests will run automatically.

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

Terraform GA: Diff ( 6 files changed, 857 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 857 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 79 insertions(+))

[modular-magician]

Tests analytics

Total tests: 6
Passed tests 4
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages

• firebaseappcheck

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

TestAccFirebaseAppCheckDebugToken_firebaseAppCheckDebugTokenBasicExample|TestAccFirebaseAppCheckDebugToken_firebaseAppCheckDebugTokenUpdate

Get to know how VCR tests work

[modular-magician]

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

Rerun these tests in REPLAYING mode to catch issues

$\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

[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.

Terraform GA: Diff ( 6 files changed, 857 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 857 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 79 insertions(+))

[rainshen49]

@weixifan I distilled the requests to help with review. You can verify this manually in the generated code https://github.com/modular-magician/terraform-provider-google/compare/auto-pr-9930-old..auto-pr-9930#diff-7e164839ef21d8a5bf2f8b31c61bcce433157e556f95a7508a024c91f9ad3bb1

Create

POST firebaseappcheck.googleapis.com/projects/{{project}}/apps/{{app_id}}/debugTokens
{
  displayName: "...",
  token: "..."
}

Update (only displayName is updatable)

PATCH firebaseappcheck.googleapis.com/projects/{{project}}/apps/{{app_id}}/debugTokens/{{debug_token_id}}?updateMask=displayName
{
    displayName: "...",
}

Delete

DELETE firebaseappcheck.googleapis.com/projects/{{project}}/apps/{{app_id}}/debugTokens/{{debug_token_id}}
{} empty body

[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.

Terraform GA: Diff ( 5 files changed, 799 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 5 files changed, 799 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 79 insertions(+))

[modular-magician]

Tests analytics

Total tests: 5
Passed tests 5
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

• firebaseappcheck

$\textcolor{green}{\textsf{All tests passed in REPLAYING mode.}}$
View the build log

[ScottSuarez]

Hey @rainshen49. Looks like you're still working. Let me know when you want me to do a pass :)

[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.

Terraform GA: Diff ( 5 files changed, 798 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 5 files changed, 798 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 79 insertions(+))

[modular-magician]

Tests analytics

Total tests: 5
Passed tests 5
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

• firebaseappcheck

$\textcolor{green}{\textsf{All tests passed in REPLAYING mode.}}$
View the build log

[rainshen49]

Hi @ScottSuarez this is ready for a pass.

[ScottSuarez]

Can you add a handwritten update test (assuming update is supported). Also quick feedback on the name, debug_token_id.

We can sync over a call on this too if that would help.

[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.

Terraform GA: Diff ( 6 files changed, 881 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 881 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 79 insertions(+))

[modular-magician]

Tests analytics

Total tests: 6
Passed tests 5
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages

• firebaseappcheck

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

TestAccFirebaseAppCheckDebugToken_firebaseAppCheckDebugTokenUpdate

Get to know how VCR tests work

[modular-magician]

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

Rerun these tests in REPLAYING mode to catch issues

$\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

[ScottSuarez]

Hey @slevenick , could you take over here? I'll be out

[slevenick]

I think this should be url_param_only instead of output. That also implies ignore_read

[slevenick]

It should also be required.

[rainshen49]

debug_token_id is generated by the server upon creation. The user is not expected to supply it. I guess this means it should be output only? This field is still needed to maintain the structure of the self_link for import & reading.

[slevenick]

Ah I thought the user could set it.

In this case I think we should just use name as the output-only field, remove debug_token_id, set self_link and the resource ID to {{name}} and remove the custom flattener

[rainshen49]

How can I still support importing with all 3 formats?

[
    "projects/{{project}}/apps/{{app_id}}/debugTokens/{{debug_token_id}}",
    "{{project}}/{{app_id}}/{{debug_token_id}}",
    "{{app_id}}/{{debug_token_id}}",
]

[rainshen49]

Just a {{name}} import format has caused confusion before. See hashicorp/terraform-provider-google#7907

Thus, I'm learning towards supporting it if able.

[slevenick]

Ok, how about just using {{name}} to represent the identifier that appears after debugTokens/

You can remove all instances of debug_token_id, replace with name, and use the standard 'templates/terraform/custom_flatten/name_from_self_link.erb' custom flatten

[rainshen49]

I've also seen other resources do this. While this is probably the path with least custom code, I'm concerned about possible user confusion around ID and name. The underlying API neatly follows AIP-122. However, for a resource i.e.

resource "google_firebase_app_check_debug_token" "example" {
  name = "server_generated_debug_token_id"
}

The AIP-122 Resource name is projects/{{project}}/apps/{{app_id}}/debugTokens/server_generated_debug_token_id, but google_firebase_app_check_debug_token.example.name gives “server_generated_debug_token_id”. In order to retrieve the AIP-122 resource name, users need to use google_pubsub_schema.example.id. As you can see, Terraform's ID and name are swapped when compared with AIP-122's resource ID and resource name.

I'm fine with this solution. Is there any prior discussion in the team for this phenomenon? I expect more and more modern APIs to follow AIP-122 so the issue only becomes bigger. An official guidance would be appreciated here.

[slevenick]

As you indicated in the previous message, often users are confused by the long-form name 😉

It's fairly common in these Terraform providers for the name field to mean just the short-form ending of the self link, and the .id field is the long-form. So in this case I think it's sticking to precedent to continue using name as such. It might actually be worth calling it something different to identify that it's a server-set name rather than user-settable (you could go back to the original debug_token_id). In this case you could use api_name: name and the shared custom_flatten, I just have a problem with using a custom flatten to read from a different field.

The AIPs are not commonly known by users, many APIs (like this one!) don't follow them all too well, and they aren't meant to be guidelines on how client surfaces represent APIs

[rainshen49]

YES, this is what I'm looking for. I didn't know about the api_name field, but it seems perfect for this use case. I was able to get it working with the additional identity field. I'm very happy with how this turns out.

[slevenick]

/gcbrun

[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.

Terraform GA: Diff ( 6 files changed, 872 insertions(+), 2 deletions(-))
Terraform Beta: Diff ( 6 files changed, 872 insertions(+), 2 deletions(-))
TF Conversion: Diff ( 1 file changed, 79 insertions(+))

[modular-magician]

Tests analytics

Total tests: 6
Passed tests: 6
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages

• firebaseappcheck

$\textcolor{green}{\textsf{All tests passed!}}$
View the build log

stale