Skip to content

feat: add check descriptions [TREE-736]#362

Merged
sorccu merged 18 commits intomainfrom
pmallol/tree-736-check-description
Apr 21, 2026
Merged

feat: add check descriptions [TREE-736]#362
sorccu merged 18 commits intomainfrom
pmallol/tree-736-check-description

Conversation

@pmallol
Copy link
Copy Markdown
Contributor

@pmallol pmallol commented Apr 14, 2026

Affected Components

  • Resources
  • Test
  • Docs
  • Tooling
  • Other

Pre-Requisites

  • Terraform code is formatted with terraform fmt
  • Go code is formatted with go fmt
  • plan & apply command of demo/main.tf file do not produce diffs

Notes for the Reviewer

This adds top-level description support to the legacy checkly_check resource, the newer monitor resources (checkly_url_monitor, checkly_tcp_monitor, checkly_dns_monitor, checkly_icmp_monitor, checkly_heartbeat_monitor), and checkly_playwright_check_suite.

The PR also adds acceptance coverage for description set/remove behavior and regenerates the provider docs.

SDK: checkly/checkly-go-sdk#155

@pmallol pmallol marked this pull request as ready for review April 14, 2026 10:34
@pmallol pmallol requested a review from a team as a code owner April 14, 2026 10:34
@pmallol pmallol requested a review from sorccu April 14, 2026 10:45
@sorccu
Copy link
Copy Markdown
Member

sorccu commented Apr 14, 2026

A couple of things:

  1. I would suggest making a separate commit that bumps the go sdk so that when the change is ready and the go sdk has been released, it's easy to do an interactive rebase that removes the dev version bump commit. This can help with merge conflicts a lot in busier repos.
  2. We have many check resources. Looks like you've currently updated the basic check (old) check resource. This applies to the other check/monitor types too right (e.g. url, tcp, dns).
  3. It's important that there's a test that makes sure the value can be removed once set. We don't have such tests for all properties but it's important to at least try it out. Depending on the backend, it may or may not be possible to unset a value, or you might need to send a specific value to unset the property.
    • If this turns out to be the case, backend changes will likely be required before this can go live.

@pmallol
Copy link
Copy Markdown
Contributor Author

pmallol commented Apr 14, 2026

Thanks @sorccu ! Will get to it

@sorccu
Copy link
Copy Markdown
Member

sorccu commented Apr 14, 2026

Also, include a link to the go sdk PR in the description so that the issues get linked nicely.

@pmallol pmallol force-pushed the pmallol/tree-736-check-description branch 2 times, most recently from fbc75ac to e79f4f6 Compare April 15, 2026 16:20
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sorccu Not sure if it is necessary to update both the original resource as well, like heartbeats, tcp_check, etc

Comment thread checkly/helpers.go
}

func optionalStringPointerFromResourceData(d *schema.ResourceData, key string) *string {
value, ok := d.GetOk(key)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetOkExists is deprecated, using GetOk instead

@sorccu sorccu force-pushed the pmallol/tree-736-check-description branch from 3d8b9f5 to 79af8c9 Compare April 21, 2026 17:26
sorccu and others added 5 commits April 22, 2026 02:42
The Terraform Plugin SDK's ResourceData.Set already dereferences pointer
types and normalizes nil pointers via reflection, so the custom
setOptionalStringResourceData helper was unnecessary.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
d.Set(key, nil) always stores "" in state for TypeString, so the
removal tests can use TestCheckResourceAttr with "" directly.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GetOk already returns false for zero-value strings, so the str == ""
guard in optionalStringPointerFromResourceData is unreachable.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sorccu sorccu merged commit 38010d4 into main Apr 21, 2026
8 of 10 checks passed
@sorccu sorccu deleted the pmallol/tree-736-check-description branch April 21, 2026 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants