feat: add support for status pages [sc-23757] #309
Conversation
WalkthroughThis pull request implements new functionality in the Checkly provider by adding two new Terraform resources for managing status pages and associated services. It introduces new Go files that define schemas and CRUD operations along with helper functions. Acceptance tests have been added to validate required fields and behavior for these resources. New documentation files and example configurations are provided to guide users in configuring these resources. Additionally, the Checkly Go SDK dependency is updated to a newer version. Changes
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Terraform Format and Style 🖌
|
Fixes default_theme's default value so that plans are clean.
sorccu
requested review from
maxigimenez,
pilimartinez,
Antoine-C and
shiini2
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (14)
examples/resources/checkly_status_page_service/resource.tf (1)
1-7: Implementation examples are minimal but functionalThe example demonstrates basic usage by creating two status page services. This is sufficient for showing how to use the resource with its required attributes.
However, consider enhancing the example to show:
- How this resource relates to the
checkly_status_pageresource- Any optional attributes that might be useful for users
docs/resources/status_page_service.md (1)
11-12: Add additional context about the resourceThis section could benefit from a description of what a status page service is, how it's used, and how it relates to other resources like status pages and checks.
Consider adding content like:
Status page services allow you to group related checks under a common service name that will be displayed on your status pages. Each service aggregates the status of its associated checks to show an overall health status.examples/resources/checkly_status_page/resource.tf (1)
9-25: Good example, but consider showing more optional attributes.The status page resource example clearly demonstrates the core structure with required attributes and the card/service_attachment relationship. However, it could be more informative to showcase additional optional attributes like
custom_domain,logo, orfaviconthat are mentioned in the documentation.resource "checkly_status_page" "example" { name = "Example Application" url = "my-example-status-page" default_theme = "DARK" + logo = "https://example.org/logo.png" + favicon = "https://example.org/favicon.png" + redirect_to = "https://example.org" card { name = "Services"checkly/resource_status_page_service_test.go (1)
20-37: Consider adding update/delete test scenarios.The happy path test effectively verifies that a properly configured resource works as expected. However, to ensure complete coverage, consider adding test cases for resource updates and deletions. This would validate the full CRUD lifecycle.
func TestAccStatusPageServiceHappyPath(t *testing.T) { accTestCase(t, []resource.TestStep{ { Config: ` resource "checkly_status_page_service" "test" { name = "foo" } `, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr( "checkly_status_page_service.test", "name", "foo", ), ), }, + { + Config: ` + resource "checkly_status_page_service" "test" { + name = "bar" + } + `, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr( + "checkly_status_page_service.test", + "name", + "bar", + ), + ), + }, }) }docs/resources/status_page.md (2)
1-7: Add a meaningful description in the frontmatter.The description field in the frontmatter is empty. Consider adding a concise description that explains the purpose of status pages within the Checkly ecosystem.
description: |- - + Manages a Checkly status page that displays the health of your monitored services.
56-56: Enhance the documentation of allowed values.For the
default_themeattribute, the possible values (AUTO,DARK, andLIGHT) are mentioned, but it would be helpful to explain what each theme represents and how theAUTOoption behaves.- `default_theme` (String) Possible values are `AUTO`, `DARK`, and `LIGHT`. (Default `AUTO`). + `default_theme` (String) Sets the color theme of the status page. Possible values are `AUTO` (adapts to user's system preferences), `DARK` (dark mode regardless of system settings), and `LIGHT` (light mode regardless of system settings). Defaults to `AUTO`.checkly/resource_status_page_service.go (3)
13-30: Resource schema definition looks solid.You have properly defined the schema with a required
namefield. Consider whether additional validations or constraints (e.g., minimum length or a regex pattern) are necessary to prevent invalid values for thenamefield.
59-73: Fragile 404 detection logic.Using
strings.Contains(err.Error(), "404")to detect a 404 might be brittle if the error message changes. Consider using a typed or status code-based check from thecheckly-go-sdk(if available), or verifying the error type to reliably detect 404 conditions.
90-98: Delete function is standard.If
DeleteStatusPageServicereturns a 404 for already-deleted items, you can optionally handle that scenario similarly to theReadfunction to avoid errors for missing resources.checkly/resource_status_page.go (5)
13-32: Schema definition for name and url is well-structured.Both fields are required, which is logical for a status page. Consider adding a light URL validation function for
"url"to guard against invalid inputs.
33-52: Custom domain validation is mostly sufficient.Though blocking domains ending with
example.comand related TLDs may be a placeholder, consider exploring a more robust domain pattern check if needed.
88-120: Card schema requires at least one card.If allowing zero cards is a possibility, consider making the field optional or setting
MinItemsto zero. Otherwise, this is a valid approach if at least one card is mandatory in all status pages.
220-234: Similar 404 detection concern.Like the service resource, this code checks
strings.Contains(err.Error(), "404"). Consider using a more robust typed error check if the SDK or standard library supports it.
251-259: Delete function may benefit from consistent 404 logic.You might optionally catch a 404 error and allow Terraform to treat it as a successful delete.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
checkly/provider.go(1 hunks)checkly/resource_status_page.go(1 hunks)checkly/resource_status_page_service.go(1 hunks)checkly/resource_status_page_service_test.go(1 hunks)checkly/resource_status_page_test.go(1 hunks)docs/resources/status_page.md(1 hunks)docs/resources/status_page_service.md(1 hunks)examples/resources/checkly_status_page/resource.tf(1 hunks)examples/resources/checkly_status_page_service/resource.tf(1 hunks)go.mod(1 hunks)
🔇 Additional comments (26)
go.mod (1)
7-7:✅ Verification successful
Dependency update to support status page functionality
The update to checkly-go-sdk v1.11.0 is necessary to support the new status page resources being added in this PR. Since the PR description mentions this work is dependent on checkly-go-sdk PR #131, we should ensure that PR has been merged and the v1.11.0 release is published before merging this PR.
🏁 Script executed:
#!/bin/bash # Verify that the v1.11.0 tag exists in the checkly-go-sdk repository curl -s https://api.github.com/repos/checkly/checkly-go-sdk/tags | grep -A 2 "v1.11.0" # Check if PR #131 has been merged curl -s https://api.github.com/repos/checkly/checkly-go-sdk/pulls/131 | grep -E '"state": "(open|closed)"'Length of output: 512
Status page dependency verified: checkly-go-sdk v1.11.0 confirmed
- Verified that the checkly-go-sdk v1.11.0 tag exists in the repository.
- Confirmed that PR Improve CI and local tooling #131 has been merged.
This dependency update is valid and supports the new status page functionality as described in the PR.
docs/resources/status_page_service.md (1)
25-34:✅ Verification successful
Schema documentation is minimal
The schema only documents the required
nameattribute and the read-onlyid. If there are any optional attributes or relations to other resources, they should be documented here as well.
🏁 Script executed:
#!/bin/bash # Check for additional attributes in the resource implementation grep -r "Schema:" -A 50 checkly/resource_status_page_service.go | grep -A 5 "\".*\": {"Length of output: 213
Schema documentation accuracy verified
After reviewing the resource implementation in
checkly/resource_status_page_service.go, only the requirednameattribute (with its corresponding properties) is defined—the read-onlyidis documented separately in the generated schema. There are no optional attributes or relationships present that require additional documentation. The current documentation indocs/resources/status_page_service.mdaccurately reflects the implemented schema.checkly/provider.go (1)
47-48:✅ Verification successful
Successfully added new resources to the provider
The new status page resources have been properly added to the ResourcesMap, making them available for use in Terraform configurations.
We should verify that the referenced resource functions (
resourceStatusPage()andresourceStatusPageService()) are properly implemented in their respective files.
🏁 Script executed:
#!/bin/bash # Verify that the resource functions exist grep -r "func resourceStatusPage()" --include="*.go" . grep -r "func resourceStatusPageService()" --include="*.go" . # Check for the schemas and CRUD functions in the implementation grep -r "Create:" --include="resource_status_page*.go" . grep -r "Read:" --include="resource_status_page*.go" . grep -r "Update:" --include="resource_status_page*.go" . grep -r "Delete:" --include="resource_status_page*.go" .Length of output: 2258
Resource Implementation Verified: New Status Page Resources are Correctly Added
The verification confirms that the resource functions exist and their respective implementations correctly include all necessary CRUD operations.
checkly/resource_status_page.go: Contains theresourceStatusPage()implementation with valid definitions for Create, Read, Update, and Delete.checkly/resource_status_page_service.go: Contains theresourceStatusPageService()implementation along with all CRUD functions.The new status page resources are properly integrated and available for use in Terraform configurations.
examples/resources/checkly_status_page/resource.tf (1)
1-7: Clear and concise service definitions.The service resource definitions are simple and straightforward, demonstrating the minimal required configuration. This helps users understand the basic usage pattern.
checkly/resource_status_page_service_test.go (2)
1-9: LGTM: Standard imports setup.The package declaration and imports follow Go standard practices.
10-18: Required field validation test is properly implemented.The test correctly verifies that the resource requires a "name" attribute by expecting an appropriate error message when it's missing.
docs/resources/status_page.md (1)
15-41: Complete and helpful usage example.The example in the documentation provides clear guidance on how to create and configure status page resources, demonstrating the relationship between status pages and services.
checkly/resource_status_page_test.go (4)
10-27: Well-structured test for required fields.This test properly validates that all required fields (
name,url, and at least onecardblock) are enforced by the resource.
29-50: Effective test for nested required fields.The test correctly validates that the
service_idattribute is required in service attachments, ensuring proper configuration of nested blocks.
52-191: Comprehensive happy path testing with all attributes.This test thoroughly validates both minimal and complete configurations of the status page resource, including all optional attributes. The step-based approach effectively tests updates as well.
193-232: Domain validation test is well-implemented, but documentation should be improved.The test for unsupported custom domains is comprehensive and uses a clean approach to test multiple domains. However, these domain restrictions are not clearly documented in the schema section of the documentation.
Please update the documentation to clearly indicate which domains are not supported. You can add this information to the
custom_domainattribute description in the documentation.- `custom_domain` (String) A custom user domain, e.g. "status.example.com". See the docs on updating your DNS and SSL usage. + `custom_domain` (String) A custom user domain, e.g. "status.mycompany.com". Common domains like example.com, example.net, example.org, and their subdomains are not supported. See the docs on updating your DNS and SSL usage.checkly/resource_status_page_service.go (5)
1-12: No issues found at file initialization.The package declaration and imports follow standard Go conventions for a Terraform provider.
32-45: Create function aligns with Terraform patterns.The creation logic includes a translation from schema to the SDK type and ends with a
Readcall to refresh the state. This is a standard and correct Terraform pattern.
47-52: Mapping resource data is straightforward.The function accurately extracts the single
namefield. No further action needed.
54-57: Resource data population is correct.Properly assigns the
namefield back to Terraform’s state, and no issues stand out.
75-88: Update function looks good.Similar to the create function, you correctly translate resource data and call the
Readfunction afterward. This approach is consistent with Terraform’s best practices.checkly/resource_status_page.go (10)
1-12: Initial declarations are fine.No concerns regarding package naming or imports.
53-87: Optional fields and default theme are correctly handled.Setting
"AUTO"as the default theme with validation for"DARK"and"LIGHT"is properly implemented.
122-135: Create function follows standard Terraform patterns.Mapping data, making the API call, and finalizing with
Readis the expected Terraform workflow.
137-149: Resource data translation is clear.All fields are mapped directly from Terraform to the Checkly SDK. If the API enforces a specific format for URLs, consider adding a validation function or verifying through a typed error.
151-166: Status page cards mapping is straightforward.Each card includes a list of service attachments. No issues identified.
168-179: List conversion function is correct.Populating the Terraform schema from the native SDK structure is well-formatted and consistent with a typical Terraform approach.
181-194: Service attachments successfully translated.Mapping the
service_idfrom Terraform to the SDK is simple and effective.
196-206: Reverse conversion of service attachments is fine.Again, consistent approach for bridging the Terraform schema with the Checkly SDK.
208-218: Resource data population is properly handled.All relevant fields, including theme and cards, are updated in the state after reading from the API.
236-249: Update function approach is correct.The resource is re-read into state post-update, in line with Terraform norms.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
checkly/resource_status_page_service.go (3)
49-54: Consider adding validation for the name field.The translation function looks good, but consider adding validation for the name field if there are specific constraints on what constitutes a valid name for a status page service.
func statusPageServiceFromResourceData(d *schema.ResourceData) (checkly.StatusPageService, error) { + name := d.Get("name").(string) + if name == "" { + return checkly.StatusPageService{}, fmt.Errorf("name cannot be empty") + } return checkly.StatusPageService{ ID: d.Id(), - Name: d.Get("name").(string), + Name: name, }, nil }
61-75: Improve error handling for resource not found.The current approach of checking for "404" in the error message is brittle and may break if error message formats change. Consider using a more robust method if the SDK provides one.
func resourceStatusPageServiceRead(d *schema.ResourceData, client interface{}) error { ctx, cancel := context.WithTimeout(context.Background(), apiCallTimeout()) defer cancel() service, err := client.(checkly.Client).GetStatusPageService(ctx, d.Id()) if err != nil { - if strings.Contains(err.Error(), "404") { + // Check if the SDK provides a more robust way to check for not found errors + // For example: if errors.Is(err, checkly.ErrNotFound) { + if strings.Contains(err.Error(), "404") { //if resource is deleted remotely, then mark it as //successfully gone by unsetting it's ID d.SetId("") return nil } return fmt.Errorf("resourceStatusPageServiceRead: API error: %w", err) } return resourceDataFromStatusPageService(service, d) }
77-90: Setting ID in update may be redundant.The update function sets the ID in the Terraform state, which might be redundant since it should already be set from creation or import. Consider removing this line unless there's a specific reason to keep it.
func resourceStatusPageServiceUpdate(d *schema.ResourceData, client interface{}) error { service, err := statusPageServiceFromResourceData(d) if err != nil { return fmt.Errorf("resourceStatusPageServiceUpdate: translation error: %w", err) } ctx, cancel := context.WithTimeout(context.Background(), apiCallTimeout()) defer cancel() _, err = client.(checkly.Client).UpdateStatusPageService(ctx, service.ID, service) if err != nil { return fmt.Errorf("resourceStatusPageServiceUpdate: API error: %w", err) } - d.SetId(service.ID) return resourceStatusPageServiceRead(d, client) }checkly/resource_status_page.go (1)
39-53: Consider expanding custom domain validation logic.Currently, there is only a check for specific “example.*” suffixes. You might consider a more robust validation approach (e.g., via regex or domain syntax checks) if you want to ensure valid user-provided domains and prevent potential misconfigurations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
checkly/resource_status_page.go(1 hunks)checkly/resource_status_page_service.go(1 hunks)docs/resources/status_page.md(1 hunks)docs/resources/status_page_service.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/resources/status_page_service.md
- docs/resources/status_page.md
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Matrix test (1.1.*)
- GitHub Check: Matrix test (0.15.*)
- GitHub Check: Analyze (go)
🔇 Additional comments (8)
checkly/resource_status_page_service.go (3)
13-32: Good resource schema implementation with clear description.This resource schema for status page services is well-structured with appropriate CRUD functions and a clear description. The schema correctly defines the required 'name' field.
34-47: Resource creation function looks good.The create function follows best practices with proper error wrapping, timeout management, and calling Read at the end to ensure state consistency.
92-100: Delete function implementation is clean and concise.The delete function correctly handles the API call with proper timeout and error handling.
checkly/resource_status_page.go (5)
70-89: Validation fordefault_themeis clear and concise.Your approach to restricting the
default_themevalues toAUTO,DARK, orLIGHTis well-structured and user-friendly. This ensures that only supported themes are passed to the API. Good job!
90-119: Validate the requirement of at least one card.Requiring at least one card (
MinItems: 1) is a strong constraint. Verify that this aligns with real usage scenarios. If users need the option to create an empty status page with no cards, consider removing or relaxing this requirement.
124-137: Creation logic looks solid.The resource creation flow appears compliant with Terraform patterns, featuring an immediate read to synchronize state. Good use of timeouts and error handling ensures resilience against API delays.
139-151: Data translation utility is straightforward.Your
statusPageFromResourceDatafunction cleanly transforms Terraform input into the SDK struct. This approach eases debugging and keeps your resource methods concise.
253-261: Deletion logic properly clears the Terraform resource ID.Setting the resource ID to empty upon successful deletion is aligned with Terraform conventions, ensuring Terraform no longer tracks the resource.
This PR adds new resources for status pages and status page services.
Needs checkly/checkly-go-sdk#131.
Affected Components
Pre-Requisites
terraform fmtgo fmtplan&applycommand ofdemo/main.tffile do not produce diffsNotes for the Reviewer
New Dependency Submission