Update viz objects validation #245
Conversation
util/validation_mode.go
Outdated
| FULL ValidationMode = "FULL" | ||
| TERRAFORM ValidationMode = "TERRAFORM" |
There was a problem hiding this comment.
Are you able to expand on what the differences are here?
There was a problem hiding this comment.
It is possible to define a resource that references the ID of another resource. For example, a dashboard might reference chart ID, like:
resource "signalfx_time_chart" "test_chart" {
name = "test chart"
# ...
}
resource "signalfx_dashboard" "test_dashboard" {
name = "test dashboard"
chart {
chart_id = signalfx_time_chart.test_chart.id
}
# ...
}However, during the plan phase, the referenced chart has not yet been created, and Terraform replaces the chart_id with a placeholder string. This string cannot be parsed by the backend, as it expects a valid ID.
To address this, the TERRAFORM mode allows a dashboard request to omit chart_id, enabling validation of the other dashboard fields without causing errors.
util/validation_mode.go
Outdated
| func ValidateValidationMode(validationMode ValidationMode) error { | ||
| switch validationMode { | ||
| case FULL, TERRAFORM: | ||
| return nil | ||
| default: | ||
| return errors.New("invalid validation mode: " + string(validationMode)) | ||
| } | ||
| } |
There was a problem hiding this comment.
I would suggest making this a method on ValidationMode instead of a seperate function. Comparing the two options, it makes it simpiler to understand when reviewing ValidationMode.Validate() when compared to util.ValidateValidationMode(Full).
util/validation_mode.go
Outdated
There was a problem hiding this comment.
util is the wrong package for this, I am not strictly sure what would be the best option here but for now I'd probably move this to be in the top level directly next to dashboards since they are currently coupled together in their implementation details.
There was a problem hiding this comment.
Also, please don't forget to add test cases and validate that the validate method works :)
dashboardgroup_test.go
Outdated
| defer teardown() | ||
|
|
||
| params := url.Values{"validationMode": []string{"TERRAFORM"}} | ||
| mux.HandleFunc("/v2/dashboardgroup/validate", verifyRequest(t, "POST", true, http.StatusNoContent, params, "")) |
There was a problem hiding this comment.
Use http.MethodPost instead of a magic string
There was a problem hiding this comment.
All methods in dashbaord_test.go and dashboardgroup_test.go used strings, so I changed them also for consistency.
dashboard_test.go
Outdated
|
|
||
| err := client.ValidateDashboardWithMode(context.Background(), &dashboard.CreateUpdateDashboardRequest{ | ||
| Name: "string", | ||
| }, "IVALID MODE") |
Update validation for dashboards and dashboard groups in order to pass validation mode.