franckverrot / terraform-provider-stripe

A Terraform Provider for Stripe
https://registry.terraform.io/providers/franckverrot/stripe/latest
Mozilla Public License 2.0
242 stars 51 forks source link

Add plan_id to the plan resource #18

Closed bradrydzewski closed 5 years ago

bradrydzewski commented 5 years ago

Stripe gives you the ability to set a custom plan_id. This can be useful if you want your development and production environments to mirror eachother. This way I can ensure the code I tested in development will not fail due to incorrect plan ids in production (since they are the same).

In terraform 12.x the id field is reserved. I looked at other providers to see how they handle this, and they seem to use the convention where they prefix the attribute with the resource name. I therefore used plan_id instead of id.

I have tested these changes in live and production mode in Stripe and they work as expected :)

andylayton commented 5 years ago

Hey @franckverrot I think the way this was implemented, plan_id is optional and will only take effect if it's provided.

The only difference I see in the output is that for those resources where it was provided, an additional parameter is also generated ('terraform show' below)

resource "stripe_plan" "month" {
    active         = true
    amount         = 4800
    billing_scheme = "per_unit"
    currency       = "usd"
    id             = "my_custom_plan_id"
    interval       = "month"
    interval_count = 1
    nickname       = "snip"
    plan_id        = "my_custom_plan_id"
    product        = "prod_Fd7ab1lqYgDOGM"
    usage_type     = "licensed"
}

versus

resource "stripe_plan" "quarterly" {
    active         = true
    amount         = 14400
    billing_scheme = "per_unit"
    currency       = "usd"
    id             = "plan_FabcVyiq9TKUc1"
    interval       = "month"
    interval_count = 3
    nickname       = "snip"
    product        = "prod_Fd7ab1lqYgDOGM"
    usage_type     = "licensed"
}

Other than that difference, though, it looks like the plan_id parameter is indeed optional.

franckverrot commented 5 years ago

I think the way this was implemented, plan_id is optional and will only take effect if it's provided.

If I’m not providing a plan_id when creating the new plan, and re-run plan/apply, Terraform will notice differences and mention an update in-place because the API will return a plan_id for the state to store, but the definition doesn’t mention it. The only fix I found was to set Computed: true, on plan_id, and Terraform doesn’t complain anymore.

I think it’s fine to make it both computed and optional, I tried locally and it seems to be working. WDYT?

bradrydzewski commented 5 years ago

as an aside, I do not think the plan id can ever be updated. So if terraform detects the plan has been updated it should be considered an error.

franckverrot commented 5 years ago

Merged here e16bdb8, thanks again!