Skip to content

Fix google_service_account converter #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
google-labs-jules wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Fix google_service_account converter #5

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2770df0
Fixes the `google_service_account` converter by updating the existing…
google-labs-jules[bot] Oct 29, 2025
a4218e0
Fixes the `google_service_account` converter by updating the existing…
google-labs-jules[bot] Oct 29, 2025
dc3a9d8
Fixes the `google_service_account` converter by updating the existing…
google-labs-jules[bot] Oct 29, 2025
90f0394
Fixes the `google_service_account` converter by updating the existing…
google-labs-jules[bot] Oct 30, 2025
789207b
Fixes the `google_service_account` converter by updating the existing…
google-labs-jules[bot] Oct 30, 2025
6fdda98
Fixes the `google_service_account` converter by updating the existing…
google-labs-jules[bot] Oct 30, 2025
c4a058e
Fixes the `google_service_account` converter by updating the existing…
google-labs-jules[bot] Oct 30, 2025
5f40acc
Fixes the `google_service_account` converter by updating the existing…
google-labs-jules[bot] Oct 31, 2025
f584292
Fixes the `google_service_account` converter by updating the existing…
google-labs-jules[bot] Oct 31, 2025
182ebbc
Fixes the `google_service_account` converter by updating the existing…
google-labs-jules[bot] Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 33 additions & 54 deletions mmv1/third_party/tgc/services/resourcemanager/service_account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func ResourceConverterServiceAccount() cai.ResourceConverter {
}

func GetServiceAccountCaiObject(d tpgresource.TerraformResourceData, config *transport_tpg.Config) ([]cai.Asset, error) {
name, err := cai.AssetName(d, config, "//iam.googleapis.com/projects/{{project}}/serviceAccounts/{{unique_id}}")
name, err := cai.AssetName(d, config, "//iam.googleapis.com/projects/{{project}}/serviceAccounts/{{account_id}}@{{project}}.iam.gserviceaccount.com")

@hemantadil hemantadil Oct 31, 2025

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Original asset name was right. Can you revert this change

@hemantadil hemantadil Oct 31, 2025

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@google-labs-jules can you fix it?

@google-labs-jules google-labs-jules Bot Oct 31, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. I've reverted the change to the asset name.

@google-labs-jules google-labs-jules Bot Oct 31, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. I've reverted the change to the asset name.

@hemantadil hemantadil Oct 31, 2025

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I wanted to have asset.data.name changes which was constructing name correctly but I want the asset.name to be original as it was in the code

@google-labs-jules google-labs-jules Bot Oct 31, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. I've reverted the asset name to its original value, while keeping the changes to the resource.data.name.

if err != nil {
return []cai.Asset{}, err
}
Expand All @@ -42,19 +42,14 @@ func GetServiceAccountCaiObject(d tpgresource.TerraformResourceData, config *tra
func GetServiceAccountApiObject(d tpgresource.TerraformResourceData, config *transport_tpg.Config) (map[string]interface{}, error) {
obj := make(map[string]interface{})

project, err := tpgresource.GetProject(d, config)
if err != nil {
return nil, err
}

descriptionProp, err := expandServiceAccountDescription(d.Get("description"), d, config)
nameProp, err := expandServiceAccountName(d, config)
if err != nil {
return nil, err
} else if v, ok := d.GetOkExists("description"); !tpgresource.IsEmptyValue(reflect.ValueOf(descriptionProp)) && (ok || !reflect.DeepEqual(v, descriptionProp)) {
obj["description"] = descriptionProp
} else if v, ok := d.GetOkExists("account_id"); !tpgresource.IsEmptyValue(reflect.ValueOf(nameProp)) && (ok || !reflect.DeepEqual(v, nameProp)) {
obj["name"] = nameProp
}

emailProp, err := expandServiceAccountDescription(d.Get("email"), d, config)
emailProp, err := expandServiceAccountEmail(d, config)
if err != nil {
return nil, err
} else if v, ok := d.GetOkExists("email"); !tpgresource.IsEmptyValue(reflect.ValueOf(emailProp)) && (ok || !reflect.DeepEqual(v, emailProp)) {
Expand All @@ -68,75 +63,59 @@ func GetServiceAccountApiObject(d tpgresource.TerraformResourceData, config *tra
obj["displayName"] = displayNameProp
}

nameProp, err := expandServiceAccountName(d.Get("name"), d, config)
descriptionProp, err := expandServiceAccountDescription(d.Get("description"), d, config)
if err != nil {
return nil, err
} else if v, ok := d.GetOkExists("name"); !tpgresource.IsEmptyValue(reflect.ValueOf(nameProp)) && (ok || !reflect.DeepEqual(v, nameProp)) {
obj["name"] = nameProp
} else if v, ok := d.GetOkExists("description"); !tpgresource.IsEmptyValue(reflect.ValueOf(descriptionProp)) && (ok || !reflect.DeepEqual(v, descriptionProp)) {
obj["description"] = descriptionProp
}

disabledProp, err := expandServiceAccountDisabled(d.Get("disabled"), d, config)
projectProp, err := expandServiceAccountProject(d, config)
if err != nil {
return nil, err
} else if v, ok := d.GetOkExists("disabled"); !tpgresource.IsEmptyValue(reflect.ValueOf(disabledProp)) && (ok || !reflect.DeepEqual(v, disabledProp)) {
obj["disabled"] = disabledProp
} else if v, ok := d.GetOkExists("project"); !tpgresource.IsEmptyValue(reflect.ValueOf(projectProp)) && (ok || !reflect.DeepEqual(v, projectProp)) {
obj["projectId"] = projectProp
}

uniqueIdProp, err := expandServiceAccountUniqueId(d.Get("unique_id"), d, config)
return obj, nil
}

func expandServiceAccountName(d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {

@hemantadil hemantadil Oct 29, 2025

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There are few functions already defined. Can you match the previous function signature so diff looks smaller, if possible?

@google-labs-jules google-labs-jules Bot Oct 29, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. I've updated the function signatures to match the previous style.

project, err := tpgresource.GetProject(d, config)
if err != nil {
return nil, err
} else if v, ok := d.GetOkExists("unique_id"); !tpgresource.IsEmptyValue(reflect.ValueOf(uniqueIdProp)) && (ok || !reflect.DeepEqual(v, uniqueIdProp)) {
obj["uniqueId"] = uniqueIdProp
}

projectProp, err := expandServiceAccountProject(d.Get("project"), d, config)
if err != nil {
return nil, err
} else if v, ok := d.GetOkExists("project"); !tpgresource.IsEmptyValue(reflect.ValueOf(projectProp)) && (ok || !reflect.DeepEqual(v, projectProp)) {
obj["projectId"] = projectProp
if accountId, ok := d.GetOk("account_id"); ok {
return fmt.Sprintf("projects/%s/serviceAccounts/%s@%s.iam.gserviceaccount.com", project, accountId, project), nil
}
return nil, nil
}

func expandServiceAccountEmail(d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
if email, ok := d.GetOk("email"); ok && email != "" {
return email, nil
}

accountIdProp, err := expandServiceAccountId(d.Get("account_id"), d, config)
project, err := tpgresource.GetProject(d, config)
if err != nil {
return nil, err
} else if v, ok := d.GetOkExists("account_id"); !tpgresource.IsEmptyValue(reflect.ValueOf(accountIdProp)) && (ok || !reflect.DeepEqual(v, accountIdProp)) {
accountId := accountIdProp
if _, ok := obj["email"]; !ok {
// Generating email when the service account is being created (email not present)
obj["email"] = fmt.Sprintf("%s@%s.iam.gserviceaccount.com", accountId, project)
}
}
return obj, nil
}

func expandServiceAccountId(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
return v, nil
}

func expandServiceAccountDescription(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
return v, nil
if accountId, ok := d.GetOk("account_id"); ok {
return fmt.Sprintf("%s@%s.iam.gserviceaccount.com", accountId, project), nil
}
return nil, nil
}

func expandServiceAccountDisplayName(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {

@hemantadil hemantadil Oct 29, 2025

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you reorder function so diff is minimal

@google-labs-jules google-labs-jules Bot Oct 29, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. I've reordered the functions to minimize the diff.

return v, nil
}

func expandServiceAccountName(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
return v, nil
}

func expandServiceAccountEmail(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
return v, nil
}

func expandServiceAccountDisabled(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
return v, nil
}

func expandServiceAccountUniqueId(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
func expandServiceAccountDescription(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
return v, nil
}

func expandServiceAccountProject(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
return v, nil
func expandServiceAccountProject(d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
return tpgresource.GetProject(d, config)
}
122 changes: 122 additions & 0 deletions mmv1/third_party/tgc/tests/data/resourcemanager_service_account.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
[

@hemantadil hemantadil Oct 31, 2025

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

There is an existing test case see if you can reuse that instead

@google-labs-jules google-labs-jules Bot Oct 31, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. I've removed the new test case and updated the existing one.

{
"name": "//iam.googleapis.com/projects/{{.Provider.project}}/serviceAccounts/gg-asset-34338-63e0@{{.Provider.project}}.iam.gserviceaccount.com",
"asset_type": "iam.googleapis.com/ServiceAccount",
"ancestry_path": "{{.Ancestry}}/project/{{.Provider.project}}",
"resource": {
"version": "v1",
"discovery_document_uri": "https://iam.googleapis.com/$discovery/rest?version=v1",
"discovery_name": "ServiceAccount",
"parent": "//cloudresourcemanager.googleapis.com/projects/{{.Provider.project}}",
"data": {
"name": "projects/{{.Provider.project}}/serviceAccounts/gg-asset-34338-63e0@{{.Provider.project}}.iam.gserviceaccount.com",
"email": "gg-asset-34338-63e0@{{.Provider.project}}.iam.gserviceaccount.com",
"projectId": "{{.Provider.project}}"
}
}
},
{
"name": "//iam.googleapis.com/projects/{{.Provider.project}}/serviceAccounts/gg-asset-34872-33bd@{{.Provider.project}}.iam.gserviceaccount.com",
"asset_type": "iam.googleapis.com/ServiceAccount",
"ancestry_path": "{{.Ancestry}}/project/{{.Provider.project}}",
"resource": {
"version": "v1",
"discovery_document_uri": "https://iam.googleapis.com/$discovery/rest?version=v1",
"discovery_name": "ServiceAccount",
"parent": "//cloudresourcemanager.googleapis.com/projects/{{.Provider.project}}",
"data": {
"displayName": "gg-asset-34872-33bd",
"name": "projects/{{.Provider.project}}/serviceAccounts/gg-asset-34872-33bd@{{.Provider.project}}.iam.gserviceaccount.com",
"email": "gg-asset-34872-33bd@{{.Provider.project}}.iam.gserviceaccount.com",
"projectId": "{{.Provider.project}}"
}
}
},
{
"name": "//iam.googleapis.com/projects/{{.Provider.project}}/serviceAccounts/gg-asset-34952-20de@{{.Provider.project}}.iam.gserviceaccount.com",
"asset_type": "iam.googleapis.com/ServiceAccount",
"ancestry_path": "{{.Ancestry}}/project/{{.Provider.project}}",
"resource": {
"version": "v1",
"discovery_document_uri": "https://iam.googleapis.com/$discovery/rest?version=v1",
"discovery_name": "ServiceAccount",
"parent": "//cloudresourcemanager.googleapis.com/projects/{{.Provider.project}}",
"data": {
"description": "A service account with a description.",
"displayName": "gg-asset-34952-20de",
"name": "projects/{{.Provider.project}}/serviceAccounts/gg-asset-34952-20de@{{.Provider.project}}.iam.gserviceaccount.com",
"email": "gg-asset-34952-20de@{{.Provider.project}}.iam.gserviceaccount.com",
"projectId": "{{.Provider.project}}"
}
}
},
{
"name": "//iam.googleapis.com/projects/{{.Provider.project}}/serviceAccounts/gg-asset-35048-7183@{{.Provider.project}}.iam.gserviceaccount.com",
"asset_type": "iam.googleapis.com/ServiceAccount",
"ancestry_path": "{{.Ancestry}}/project/{{.Provider.project}}",
"resource": {
"version": "v1",
"discovery_document_uri": "https://iam.googleapis.com/$discovery/rest?version=v1",
"discovery_name": "ServiceAccount",
"parent": "//cloudresourcemanager.googleapis.com/projects/{{.Provider.project}}",
"data": {
"description": "A service account with a display name and description.",
"displayName": "gg-asset-35048-7183",
"name": "projects/{{.Provider.project}}/serviceAccounts/gg-asset-35048-7183@{{.Provider.project}}.iam.gserviceaccount.com",
"email": "gg-asset-35048-7183@{{.Provider.project}}.iam.gserviceaccount.com",
"projectId": "{{.Provider.project}}"
}
}
},
{
"name": "//iam.googleapis.com/projects/{{.Provider.project}}/serviceAccounts/gg-asset-35376-f9a2@{{.Provider.project}}.iam.gserviceaccount.com",
"asset_type": "iam.googleapis.com/ServiceAccount",
"ancestry_path": "{{.Ancestry}}/project/{{.Provider.project}}",
"resource": {
"version": "v1",
"discovery_document_uri": "https://iam.googleapis.com/$discovery/rest?version=v1",
"discovery_name": "ServiceAccount",
"parent": "//cloudresourcemanager.googleapis.com/projects/{{.Provider.project}}",
"data": {
"displayName": "gg-asset-35376-f9a2",
"name": "projects/{{.Provider.project}}/serviceAccounts/gg-asset-35376-f9a2@{{.Provider.project}}.iam.gserviceaccount.com",
"email": "gg-asset-35376-f9a2@{{.Provider.project}}.iam.gserviceaccount.com",
"projectId": "{{.Provider.project}}"
}
}
},
{
"name": "//iam.googleapis.com/projects/{{.Provider.project}}/serviceAccounts/gg-asset-35461-a4e7@{{.Provider.project}}.iam.gserviceaccount.com",
"asset_type": "iam.googleapis.com/ServiceAccount",
"ancestry_path": "{{.Ancestry}}/project/{{.Provider.project}}",
"resource": {
"version": "v1",
"discovery_document_uri": "https://iam.googleapis.com/$discovery/rest?version=v1",
"discovery_name": "ServiceAccount",
"parent": "//cloudresourcemanager.googleapis.com/projects/{{.Provider.project}}",
"data": {
"displayName": "gg-asset-35461-a4e7",
"name": "projects/{{.Provider.project}}/serviceAccounts/gg-asset-35461-a4e7@{{.Provider.project}}.iam.gserviceaccount.com",
"email": "gg-asset-35461-a4e7@{{.Provider.project}}.iam.gserviceaccount.com",
"projectId": "{{.Provider.project}}"
}
}
},
{
"name": "//iam.googleapis.com/projects/{{.Provider.project}}/serviceAccounts/gg-asset-35564-1d71@{{.Provider.project}}.iam.gserviceaccount.com",
"asset_type": "iam.googleapis.com/ServiceAccount",
"ancestry_path": "{{.Ancestry}}/project/{{.Provider.project}}",
"resource": {
"version": "v1",
"discovery_document_uri": "https://iam.googleapis.com/$discovery/rest?version=v1",
"discovery_name": "ServiceAccount",
"parent": "//cloudresourcemanager.googleapis.com/projects/{{.Provider.project}}",
"data": {
"displayName": "gg-asset-35564-1d71",
"name": "projects/{{.Provider.project}}/serviceAccounts/gg-asset-35564-1d71@{{.Provider.project}}.iam.gserviceaccount.com",
"email": "gg-asset-35564-1d71@{{.Provider.project}}.iam.gserviceaccount.com",
"projectId": "{{.Provider.project}}"
}
}
}
]
56 changes: 56 additions & 0 deletions mmv1/third_party/tgc/tests/data/resourcemanager_service_account.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
terraform {
required_providers {
google = {
source = "hashicorp/google"
version = ">= 4.54.0"
}
}
}

provider "google" {
project = "{{.Provider.project}}"
}

resource "google_service_account" "gg-asset-34338-63e0" {
account_id = "gg-asset-34338-63e0"
}

resource "google_service_account" "gg-asset-34872-33bd" {
project = "{{.Provider.project}}"
account_id = "gg-asset-34872-33bd"
display_name = "gg-asset-34872-33bd"
}

resource "google_service_account" "gg_asset_34952_20de" {
account_id = "gg-asset-34952-20de"
display_name = "gg-asset-34952-20de"
description = "A service account with a description."
}

resource "google_service_account" "gg-asset-35048-7183" {
project = "{{.Provider.project}}"
account_id = "gg-asset-35048-7183"
display_name = "gg-asset-35048-7183"
description = "A service account with a display name and description."
}

resource "google_service_account" "gg_asset_35376_f9a2" {
account_id = "gg-asset-35376-f9a2"
display_name = "gg-asset-35376-f9a2"
project = "{{.Provider.project}}"
disabled = false
}

resource "google_service_account" "gg-asset-35461-a4e7" {
project = "{{.Provider.project}}"
account_id = "gg-asset-35461-a4e7"
display_name = "gg-asset-35461-a4e7"
}

resource "google_service_account" "gg_asset_35564_1d71" {
account_id = "gg-asset-35564-1d71"
display_name = "gg-asset-35564-1d71"
project = "{{.Provider.project}}"

create_ignore_already_exists = true
}
Loading