Skip to content
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
27 changes: 19 additions & 8 deletions mmv1/third_party/tgc/services/resourcemanager/service_account.go
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")

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

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?

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.

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.

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

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 @@ -41,7 +41,6 @@ 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
Expand All @@ -54,7 +53,7 @@ func GetServiceAccountApiObject(d tpgresource.TerraformResourceData, config *tra
obj["description"] = descriptionProp
}

emailProp, err := expandServiceAccountDescription(d.Get("email"), d, config)
emailProp, err := expandServiceAccountEmail(d.Get("email"), 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,10 +67,10 @@ func GetServiceAccountApiObject(d tpgresource.TerraformResourceData, config *tra
obj["displayName"] = displayNameProp
}

nameProp, err := expandServiceAccountName(d.Get("name"), d, config)
nameProp, err := expandServiceAccountName(d.Get("account_id"), 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)) {
} else if v, ok := d.GetOkExists("account_id"); !tpgresource.IsEmptyValue(reflect.ValueOf(nameProp)) && (ok || !reflect.DeepEqual(v, nameProp)) {
obj["name"] = nameProp
}

Expand Down Expand Up @@ -106,6 +105,7 @@ func GetServiceAccountApiObject(d tpgresource.TerraformResourceData, config *tra
obj["email"] = fmt.Sprintf("%s@%s.iam.gserviceaccount.com", accountId, project)
}
}

return obj, nil
}

Expand All @@ -122,11 +122,22 @@ func expandServiceAccountDisplayName(v interface{}, d tpgresource.TerraformResou
}

func expandServiceAccountName(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
return v, nil
project, err := tpgresource.GetProject(d, config)
if err != nil {
return nil, err
}

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(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
return v, nil
if email, ok := d.GetOk("email"); ok && email != "" {
return email, nil
}

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 think this is already handled at caller side so it can be removed from here

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 redundant logic from the expandServiceAccountEmail function.

return nil, nil
}

func expandServiceAccountDisabled(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) {
Expand All @@ -138,5 +149,5 @@ func expandServiceAccountUniqueId(v interface{}, d tpgresource.TerraformResource
}

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

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.

Do we need this change?

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 expandServiceAccountProject function.

}
122 changes: 122 additions & 0 deletions mmv1/third_party/tgc/tests/data/resourcemanager_service_account.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
[

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

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