tailscale_federated_identity, tailscale_posture_integration: import empty string fields cleanly - #791
Open
keeleysam wants to merge 1 commit into
Conversation
optional+computed string fields as "" instead of null The `description` attribute on tailscale_federated_identity and the `cloud_id`, `client_id`, and `tenant_id` attributes on tailscale_posture_integration are Optional+Computed with a schema Default of "". Their Read paths ran the API value through CoalesceStringEmptyOrNull, which returns a null StringValue when the value is empty. On import the prior model value is null, so an empty field reads back as null while the schema default plans "", leaving a permanent null-vs-"" mismatch that Terraform renders as a spurious in-place update. For federated identities that update also flips the computed updated_at to "(known after apply)", so importing an existing identity that never had a description shows a noisy diff. Set these fields with types.StringValue directly, so an empty value is represented as "", consistent with the schema Default, and imports cleanly. Add a unit test that imports a federated identity with no description and asserts the imported state matches. The existing tailscale_posture_integration acceptance test already imports a falcon integration with no tenant_id and verifies state, so it covers the posture case. tailscale_oauth_client's description is SDKv2, which does not exhibit this behaviour, so it is left unchanged. Signed-off-by: Samuel Keeley <samuel@keeley.net>
mpminardi
reviewed
Jul 16, 2026
| // "" rather than null. Otherwise an empty description reads back as null on | ||
| // import, leaving a permanent null-vs-"" mismatch that plans as a spurious | ||
| // in-place update (which also flips the computed updated_at to known-after-apply). | ||
| data.Description = types.StringValue(key.Description) |
Member
There was a problem hiding this comment.
I think we can drop the leading comment here and leave the comment in the regression test since this won't be a special case after this fix!
mpminardi
reviewed
Jul 16, 2026
| // empty values as "" rather than null. Otherwise an empty value reads back as | ||
| // null on import, leaving a permanent null-vs-"" mismatch (a spurious in-place | ||
| // update). | ||
| state.CloudID = types.StringValue(integration.CloudID) |
Member
There was a problem hiding this comment.
Same RE: the leading comment here!
Member
|
Thank you for the PR @keeleysam ! And apologies for this having slipped through.
I think we can drop the helper at this point! We've standardized instead on providing a default value and the helper is now vestigial. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Importing an existing
tailscale_federated_identitythat has no description shows a spurious in-place update on the first plan.descriptionisOptional+Computedwith a schemaDefaultof"", but its Read path runs the API value throughCoalesceStringEmptyOrNull, which returns a nullStringValuewhen the value is empty. On import the prior model value is null, so an empty description reads back as null while the default plans"". That leaves a permanent null-vs-"" mismatch, which Terraform renders as an update. Because it plans an update, the computedupdated_atalso goes(known after apply), so importing a batch of identities that never had descriptions looks like they are all changing when nothing actually is.tailscale_posture_integrationhas the same shape and the same bug oncloud_id,client_id, andtenant_id(allOptional+ComputedwithDefault"", all read throughCoalesceStringEmptyOrNull).The fix sets these with
types.StringValueso an empty value stays"", matching the schema default, and imports cleanly.StringValueNullIfEmptyis unchanged and still used for the fields that are genuinely null-when-empty and have no default (logstreams3_authentication_type, webhookprovider_type).Which issue this PR fixes (use
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged):Fixes #790
Special notes for your reviewer:
tailscale_federated_identitywith no description and asserts the imported state matches (it fails without the fix onImportStateVerify, passes with it). The posture case is already covered byTestAccTailscalePostureIntegration, which imports a Falcon integration with notenant_idand runsImportStateVerify.tailscale_oauth_client'sdescriptionis on the SDKv2 code path, which does not exhibit this, so it is untouched.CoalesceStringEmptyOrNullhas no remaining callers. I left it in place since it is the correct helper for its documented pattern (a null-when-empty field with no default); its twoDefault: ""call sites were the wrong use. Happy to drop it if you would rather not carry an unused helper.description(and apply) before importing, so the field is non-empty on both sides and imports clean.go test ./tailscale/,go build,go vet, andgofmtare all clean.