fix: add UseStateForUnknown plan modifiers to prevent phantom diffs

[adamtaylor13]

Summary

Adds UseStateForUnknown() plan modifiers to computed attributes that were causing phantom diffs on every terraform plan.

The Problem

Before this fix, every terraform plan would show changes like this, even when nothing actually changed:

# render_web_service.my_app will be updated in-place
~ resource "render_web_service" "my_app" {
    ~ env_vars = {
        ~ "DATABASE_URL" = {
            ~ value = (sensitive value)
          }
      }
    ~ notification_override = {
        ~ notifications_to_send         = "default" -> (known after apply)
        ~ preview_notifications_enabled = "default" -> (known after apply)
      }
    + num_instances                 = (known after apply)
    + max_shutdown_delay_seconds    = (known after apply)
    ~ previews = {
        ~ generation = "off" -> (known after apply)
      }
    + root_directory                = (known after apply)
    ~ slug                          = "my-app" -> (known after apply)
  }

The root cause: connection_info on postgres resources lacked plan modifiers, so Terraform marked it as unknown on every plan. Any resource interpolating connection_info.internal_connection_string into an env var would inherit this "unknown" status.

After This Fix

No changes. Your infrastructure matches the configuration.

Changes

postgres/resource/schema.go - Add plan modifiers to connection_info:

"connection_info": schema.SingleNestedAttribute{
    Computed:  true,
    Sensitive: true,
    PlanModifiers: []planmodifier.Object{
        objectplanmodifier.UseStateForUnknown(),
    },
    Attributes: map[string]schema.Attribute{
        "internal_connection_string": schema.StringAttribute{
            Computed:  true,
            Sensitive: true,
            PlanModifiers: []planmodifier.String{
                stringplanmodifier.UseStateForUnknown(),
            },
        },
        // ... same for other nested attributes
    },
},

types/resource/services.go - Add plan modifiers to service attributes:

var NumInstances = schema.Int64Attribute{
    Optional: true,
    Computed: true,
    PlanModifiers: []planmodifier.Int64{
        int64planmodifier.UseStateForUnknown(),
    },
}

var Slug = schema.StringAttribute{
    Computed: true,
    PlanModifiers: []planmodifier.String{
        stringplanmodifier.UseStateForUnknown(),
    },
}

types/resource/logstreamoverride.go and notificationoverride.go - Convert from datasource/schema to resource/schema to support plan modifiers:

// Before: used datasource/schema (no plan modifier support)
import "github.com/hashicorp/terraform-plugin-framework/datasource/schema"

// After: uses resource/schema with plan modifiers
import "github.com/hashicorp/terraform-plugin-framework/resource/schema"

var LogStreamOverride = schema.SingleNestedAttribute{
    Optional: true,
    Computed: true,
    PlanModifiers: []planmodifier.Object{
        objectplanmodifier.UseStateForUnknown(),
    },
    // ...
}

Testing

• Built and installed provider locally with go install
• Configured ~/.terraformrc with dev_overrides
• Ran terraform plan against existing infrastructure with postgres + web services + background workers
• Confirmed phantom diffs are resolved

Related Issues

• Fixes num_instances and digest cause plan noise #60 (num_instances and digest plan noise)
• Related to database_name doesn't reflect real name created in Render #52 (database_name drift)
• max_shutdown_delay_seconds causes plan noise #59 (max_shutdown_delay_seconds plan noise) is not resolved here. As far as I can tell, both schema-level fixes (UseStateForUnknown and Default) break disk-backed services. The Render API rejects this field for services with an attached disk (disks disable zero-downtime deploys, so graceful shutdown doesn't apply). A proper fix needs request-builder logic to skip the field for disk-backed services plus a plan-time validator. I have tracked this as a follow-up PR.

[adamtaylor13]

This is my first stab at contributing to OSS mostly through AI. I know this is a hotly debated subject so I want to be very upfront—Opus 4.7 was driving, but I tested these changes on my own infrastructure after building the changes locally. I did confirm it worked as I expected it to.

EDIT: The consquence of the above statement is that I don't fully understand the implications of some of these changes, such as swapping the log stream from datasource/schema to resource/schema. So please be aware—I verified the behavior appears correct to me, but I can't actually tell if it's correct based on my limited understanding of how terraform providers work under the hood.

[adamtaylor13]

Note to self, if I want to pull up the previous convo w/ Claude:

claude --resume d2577a1f-a07e-4807-8657-0124dca9eee3

[adamtaylor13]

Do I need to fix the tests in CI to proceed here?

[adamtaylor13]

@jakemalachowski I looked at the latest failing test and from what I can tell it's not related to the actual code, it appears to be related to the terraform CLI missing..? Do I need to do anything here?

[tuckerchapin]

Do I need to fix the tests in CI to proceed here?

@adamtaylor13 Apologies for that, the CI tests are fixed now (#85). If you incorporate the latest you should be good to go.

[tuckerchapin]

Thanks for opening this and your patience in getting it reviewed. It looks like when this was forked, our CI was borked. That's now fixed. If you could pull in that fix, take a look at the feedback, and confirm that the cassette tests are passing/updated accordingly, that would be great!

• there are a few places where I think we'd prefer a Default over relying on unknowns from state
• there's also logstreamoverride and notificationoverride where the UseStateForUknown will likely break the removal semantics, so we should be intentional about doing so and I don't think doing so at the top/parent level is wise

[tuckerchapin]

If you want to manage the service's instance count outside Terraform, leave num_instances unset.

Because of the intent in this description, I don't think this is the right move here. Or at least worth calling out. With UseStateForUnknown I believe users would have to tf state rm, rather than just "...leave num_instances_unset".

If this is the aim, then we should update the description accordingly.

[adamtaylor13]

@tuckerchapin My understanding is that the biggest edge-case here is if someone has already set this value, and then wishes to remove it later, can you confirm? In that case, would be it appropriate to simply update the description to explain that "handing control back to Render" involves removing the local state entry for this field? I shipped an update to the description in 1198b46. Let me know if that addresses the concern here.

EDIT: Oops... I think that commit included some other changes. Standby...

EDIT 2: Commit reference should be correct now.