Fix google_service_account converter#5
Conversation
… file at `mmv1/third_party/tgc/services/resourcemanager/service_account.go` and adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. |
| return obj, nil | ||
| } | ||
|
|
||
| func expandServiceAccountName(d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) { |
There was a problem hiding this comment.
There are few functions already defined. Can you match the previous function signature so diff looks smaller, if possible?
There was a problem hiding this comment.
Done. I've updated the function signatures to match the previous style.
… file at `mmv1/third_party/tgc/services/resourcemanager/service_account.go` and adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table. The function signatures have been updated to match the previous style for a cleaner diff.
| } | ||
|
|
||
| func expandServiceAccountDisabled(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) { | ||
| func expandServiceAccountDisplayName(v interface{}, d tpgresource.TerraformResourceData, config *transport_tpg.Config) (interface{}, error) { |
There was a problem hiding this comment.
Can you reorder function so diff is minimal
There was a problem hiding this comment.
Done. I've reordered the functions to minimize the diff.
… file at `mmv1/third_party/tgc/services/resourcemanager/service_account.go` and adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table. The function signatures and order have been updated to match the previous style for a cleaner diff.
… file at `mmv1/third_party/tgc/services/resourcemanager/service_account.go` and adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table. The function signatures and order have been updated to match the previous style for a cleaner diff.
… file at `mmv1/third_party/tgc/services/resourcemanager/service_account.go` and adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table. The function signatures and order have been updated to match the previous style for a cleaner diff.
| if email, ok := d.GetOk("email"); ok && email != "" { | ||
| return email, nil | ||
| } |
There was a problem hiding this comment.
I think this is already handled at caller side so it can be removed from here
There was a problem hiding this comment.
Done. I've removed the redundant logic from the expandServiceAccountEmail function.
… file at `mmv1/third_party/tgc/services/resourcemanager/service_account.go` and adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table. The function signatures and order have been updated to match the previous style for a cleaner diff.
| return v, nil | ||
| return tpgresource.GetProject(d, config) |
There was a problem hiding this comment.
Done. I've reverted the change to the expandServiceAccountProject function.
… file at `mmv1/third_party/tgc/services/resourcemanager/service_account.go` and adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table. The function signatures and order have been updated to match the previous style for a cleaner diff.
|
|
||
| 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") |
There was a problem hiding this comment.
Original asset name was right. Can you revert this change
There was a problem hiding this comment.
Done. I've reverted the change to the asset name.
There was a problem hiding this comment.
Done. I've reverted the change to the asset name.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Done. I've reverted the asset name to its original value, while keeping the changes to the resource.data.name.
| @@ -0,0 +1,122 @@ | |||
| [ | |||
There was a problem hiding this comment.
There is an existing test case see if you can reuse that instead
There was a problem hiding this comment.
Done. I've removed the new test case and updated the existing one.
… file at `mmv1/third_party/tgc/services/resourcemanager/service_account.go` and adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table. The function signatures and order have been updated to match the previous style for a cleaner diff.
… file at `mmv1/third_party/tgc/services/resourcemanager/service_account.go` and adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table. The function signatures and order have been updated to match the previous style for a cleaner diff.
… file at `mmv1/third_party/tgc/services/resourcemanager/service_account.go` and adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table. The function signatures and order have been updated to match the previous style for a cleaner diff.
1 participant
This change fixes the
google_service_accountconverter by updating the existing file atmmv1/third_party/tgc/services/resourcemanager/service_account.goand adding comprehensive test cases. The converter logic is now aligned with the provided examples and mapping table.PR created automatically by Jules for task 5488053681581532142