terraform suport for google_folder_service_identity

[yerniyazN]

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

`google_folder_service_identity`

[github-actions]

Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: see go/terraform-auto-test-runs to set up automatic test runs.

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

google provider: Diff ( 4 files changed, 211 insertions(+))
google-beta provider: Diff ( 4 files changed, 211 insertions(+))

Missing service labels

The following new resources do not have corresponding service labels:

• google_folder_service_identity

If you believe this detection to be incorrect please raise the concern with your reviewer. Googlers: This error is safe to ignore once you've completed go/fix-missing-service-labels.
An override-missing-service-label label can be added to allow merging.

[modular-magician]

Tests analytics

Total tests: 166
Passed tests: 134
Skipped tests: 30
Affected tests: 2

Click here to see the affected service packages

• resourcemanager

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

• TestAccEphemeralServiceAccountKey_basic
• TestAccFolderServiceIdentity_basic

Get to know how VCR tests work

[modular-magician]

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

🟢 No issues found for passed tests after REPLAYING rerun.

---

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

[yerniyazN]

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

🟢 No issues found for passed tests after REPLAYING rerun.

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

Failing test are not related to my changes

[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 ( 5 files changed, 282 insertions(+))
google-beta provider: Diff ( 5 files changed, 282 insertions(+))

Missing service labels

The following new resources do not have corresponding service labels:

• google_folder_service_identity

If you believe this detection to be incorrect please raise the concern with your reviewer. Googlers: This error is safe to ignore once you've completed go/fix-missing-service-labels.
An override-missing-service-label label can be added to allow merging.

[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 ( 5 files changed, 282 insertions(+))
google-beta provider: Diff ( 5 files changed, 282 insertions(+))

Missing service labels

The following new resources do not have corresponding service labels:

• google_folder_service_identity

If you believe this detection to be incorrect please raise the concern with your reviewer. Googlers: This error is safe to ignore once you've completed go/fix-missing-service-labels.
An override-missing-service-label label can be added to allow merging.

[modular-magician]

Tests analytics

Total tests: 166
Passed tests: 135
Skipped tests: 30
Affected tests: 1

Click here to see the affected service packages

• resourcemanager

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

• TestAccEphemeralServiceAccountKey_basic

Get to know how VCR tests work

[modular-magician]

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

Tests analytics

Total tests: 166
Passed tests: 135
Skipped tests: 30
Affected tests: 1

Click here to see the affected service packages

• resourcemanager

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

• TestAccEphemeralServiceAccountKey_basic

Get to know how VCR tests work

[modular-magician]

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

[ScottSuarez]

I'm a bit perplexed by this resource. It seems like an imperative operation that will do a one time thing. From my read of the resource code. It isn't guaranteed to return the SA in question even which brings up it's usefulness in a terraform config.

• what happens if the service identity was already generated? Will this error out or no-op? Will it return the value for the SA? If not, how the customer expected to use this resource within their config and with other terraform resources?

[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 ( 5 files changed, 282 insertions(+))
google-beta provider: Diff ( 5 files changed, 282 insertions(+))

Missing service labels

The following new resources do not have corresponding service labels:

• google_folder_service_identity

If you believe this detection to be incorrect please raise the concern with your reviewer. Googlers: This error is safe to ignore once you've completed go/fix-missing-service-labels.
An override-missing-service-label label can be added to allow merging.

[modular-magician]

Tests analytics

Total tests: 166
Passed tests: 135
Skipped tests: 30
Affected tests: 1

Click here to see the affected service packages

• resourcemanager

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

• TestAccEphemeralServiceAccountKey_basic

Get to know how VCR tests work

[modular-magician]

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

[yerniyazN]

I'm a bit perplexed by this resource. It seems like an imperative operation that will do a one time thing. From my read of the resource code. It isn't guaranteed to return the SA in question even which brings up it's usefulness in a terraform config.

• what happens if the service identity was already generated? Will this error out or no-op? Will it return the value for the SA? If not, how the customer expected to use this resource within their config and with other terraform resources?

If service identity already created, it returns existing service identity. There is already a project-level analog resource named google_project_service_identity which does the same.

Yes, you are right. It is one time thing. But we might need it to configure the newly created folders.

Here use case for it: I want to add terraform support for the folder level resource called PolicyOrchestratorForFolder. To create PolicyOrchestratorForFolder, I should have folder-level service account with granted permissions. Currently terraform supports only project-level service account creation. So, there is no way to add VCR tests for PolicyOrchestratorForFolder without those service accounts

[ScottSuarez]

sorry for the delay here. Hoping to get a formal review sometime today. I need to interact with the api directly to understand the IO

[ScottSuarez]

So I believe this can be implemented through the generator and would prefer we do it there if possible.

exclude_delete can be true for this resource. The resource would also be marked as immutable.

for the read_url/self_link it would be the same as the create url.

/v1beta1/folders/--/services/osconfig.googleapis.com:generateServiceIdentity

Since if called multiple times this endpoint effectively serves as read only.

Could you give it a try and let me know if you have issues? I'm not going to excessively block on this if you strongly desire hand written. As a general philosophy we only add handwritten resources when the resource pattern is calls for it.

As is, all your current code does work

[ScottSuarez]

Also could you follow go/fix-missing-service-labels fix the failing check there?

[yerniyazN]

Also could you follow go/fix-missing-service-labels fix the failing check there?

@ScottSuarez
Created cl/742278755 to add a new owned_resource into existing enrollment data

[ScottSuarez]

I'm tentatively approving. If you are in progress of writing mmv1 yaml we can wait. But I won't block, as mentioned above. Ultimately your team is managing this resource so I am okay merging.

[yerniyazN]

I'm tentatively approving. If you are in progress of writing mmv1 yaml we can wait. But I won't block, as mentioned above. Ultimately your team is managing this resource so I am okay merging.

@ScottSuarez Thanks!

You're absolutely right that our general preference is to implement resources via MMv1 when possible. But, with MMv1, I ran into a few challenges:

1. The API doesn’t support full CRUD. The only supported operation is generateServiceIdentity, which behaves more like a “get-or-create” call. If we try to work arround, and use create_url as read_url, it might behave unexpected when we try to get non-existing resource.

2. Project-level equivalent is also handwritten. google_project_service_identity is implemented as a handwritten resource and uses the same underlying API pattern (just scoped to projects instead of folders). Given the strong similarity, I mirrored its implementation.

I need this resource to unblock VCR testing and support for PolicyOrchestratorForFolder, which depends on the existence of the service identity. Since Terraform doesn't currently support creation of folder-level service identities, this resource is essential to move that forward.

I understand the philosophy of preferring MMv1 when feasible. And for now, I’d prefer to move forward with this version so we can unblock the PolicyOrchestratorForFolder work. Let me know if you have any concerns, otherwise I’d like to proceed with merging.

[ScottSuarez]

merged ! :)