Add support for google_compute_target_tcp_proxy #528
Conversation
| ForceNew: true, | ||
| }, | ||
|
|
||
| "id": &schema.Schema{ |
There was a problem hiding this comment.
Could you rename it to proxy_id? We are moving away from "id" field. See #399 for more details.
| return err | ||
| } | ||
|
|
||
| d.SetId(proxy.Name) |
There was a problem hiding this comment.
Should probably set this to proxy.Id.
Can two separate proxies have the same name?
There was a problem hiding this comment.
At least through the UI you can not have more than one with the same name. I think I'd be in favour of leaving it with name which is also in line with the others target resources
| } | ||
|
|
||
| if v, ok := d.GetOk("description"); ok { | ||
| proxy.Description = v.(string) |
There was a problem hiding this comment.
No need for the if statement around it. Move it inside &compute.TargetTcpProxy{...} above. d.Get("description") will return an empty string if not set which will yield the same behavior.
|
|
||
| proxy := &compute.TargetTcpProxy{ | ||
| Name: d.Get("name").(string), | ||
| Service: d.Get("backend_service").(string), |
There was a problem hiding this comment.
Have you forgot to set the ProxyHeader field?
| Providers: testAccProviders, | ||
| CheckDestroy: testAccCheckComputeTargetTcpProxyDestroy, | ||
| Steps: []resource.TestStep{ | ||
| resource.TestStep{ |
There was a problem hiding this comment.
How is this testing the update feature? You need at least two TestStep. Make sure to test the update with the proxyHeader field (only updateable field) otherwise it will do a Delete/Create operation which won't test the update method.
| In addition to the arguments listed above, the following computed attributes are | ||
| exported: | ||
|
|
||
| * `id` - A unique ID assigned by GCE. |
There was a problem hiding this comment.
Rename to proxy_id like suggested in a comment above.
|
Hey @rosbo thanks for your comments! proxy_id and tests should be fine now |
|
Thanks Alberto for adding this new resource. Merging in! TF_ACC=1 go test ./google -v -run TestAccComputeTargetTcpProxy_ -timeout 120m
=== RUN TestAccComputeTargetTcpProxy_basic
--- PASS: TestAccComputeTargetTcpProxy_basic (67.52s)
=== RUN TestAccComputeTargetTcpProxy_update
--- PASS: TestAccComputeTargetTcpProxy_update (79.31s)
PASS |
rosbo
changed the title
) <!-- This change is generated by MagicModules. --> /cc @drebes
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
Fixes #363
Introduces support google_compute_target_tcp_proxy