Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reimplement dashboards with terraform-plugin-framework #239

Merged
merged 9 commits into from
Sep 12, 2024
Merged

Reimplement dashboards with terraform-plugin-framework #239

merged 9 commits into from
Sep 12, 2024

Conversation

tosuke
Copy link
Contributor

@tosuke tosuke commented Sep 5, 2024 •
edited
Loading

Output from acceptance testing:

$ MACKEREL_EXPERIMENTAL_TFFRAMEWORK=1 make testacc TESTS='"Acc(DataSource)?MackerelDashboard"'
TF_ACC=1 go test -v ./mackerel/... -run "Acc(DataSource)?MackerelDashboard" -timeout 120m                                                                                                          2024/09/06 12:50:53 [INFO] mackerel: use terraform-plugin-framework based implementation
=== RUN   TestAccDataSourceMackerelDashboardGraph
=== PAUSE TestAccDataSourceMackerelDashboardGraph
=== RUN   TestAccDataSourceMackerelDashboardValue
=== PAUSE TestAccDataSourceMackerelDashboardValue
=== RUN   TestAccDataSourceMackerelDashboardMarkdown
=== PAUSE TestAccDataSourceMackerelDashboardMarkdown
=== RUN   TestAccDataSourceMackerelDashboardAlertStatus
=== PAUSE TestAccDataSourceMackerelDashboardAlertStatus
=== RUN   TestAccMackerelDashboardGraph
=== PAUSE TestAccMackerelDashboardGraph
=== RUN   TestAccMackerelDashboardValue
=== PAUSE TestAccMackerelDashboardValue
=== RUN   TestAccMackerelDashboardMarkdown
=== PAUSE TestAccMackerelDashboardMarkdown
=== RUN   TestAccMackerelDashboardAlertStatus
=== PAUSE TestAccMackerelDashboardAlertStatus
=== CONT  TestAccDataSourceMackerelDashboardGraph
=== CONT  TestAccMackerelDashboardMarkdown
=== CONT  TestAccDataSourceMackerelDashboardAlertStatus
=== CONT  TestAccDataSourceMackerelDashboardMarkdown
=== CONT  TestAccMackerelDashboardAlertStatus
=== CONT  TestAccMackerelDashboardValue
=== CONT  TestAccMackerelDashboardGraph
=== CONT  TestAccDataSourceMackerelDashboardValue
--- PASS: TestAccDataSourceMackerelDashboardMarkdown (13.84s)
--- PASS: TestAccDataSourceMackerelDashboardAlertStatus (13.85s)
--- PASS: TestAccDataSourceMackerelDashboardGraph (14.78s)
--- PASS: TestAccDataSourceMackerelDashboardValue (18.29s)
--- PASS: TestAccMackerelDashboardMarkdown (19.34s)
--- PASS: TestAccMackerelDashboardValue (19.96s)
--- PASS: TestAccMackerelDashboardAlertStatus (20.02s)
--- PASS: TestAccMackerelDashboardGraph (20.50s)
PASS
ok      github.com/mackerelio-labs/terraform-provider-mackerel/mackerel 21.649s

tosuke
tosuke commented Sep 6, 2024
View reviewed changes
internal/provider/resource_mackerel_dashboard.go
Comment on lines +134 to +178
s := schema.Schema{
Description: "This resource allows creating and management of dashboards.",
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Description: schemaDashboardIDDesc,
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"title": schema.StringAttribute{
Description: schemaDashboardTitleDesc,
Required: true,
},
"memo": schema.StringAttribute{
Description: schemaDashboardMemoDesc,
Optional: true,
Computed: true,
Default: stringdefault.StaticString(""),
},
"url_path": schema.StringAttribute{
Description: schemaDashboardURLPathDesc,
Optional: true,
Computed: true,
Default: stringdefault.StaticString(""),
},
"created_at": schema.Int64Attribute{
Description: schemaDashboardCreatedAtDesc,
Computed: true,
PlanModifiers: []planmodifier.Int64{
int64planmodifier.UseStateForUnknown(),
},
},
"updated_at": schema.Int64Attribute{
Description: schemaDashboardUpdatedAtDesc,
Computed: true,
},
},
Blocks: map[string]schema.Block{
"graph": schemaDashboardResource_graph,
"value": schemaDashboardResource_value,
"markdown": schemaDashboardResource_markdown,
"alert_status": schemaDashboardResource_alertStatus,
},
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Original schema:

terraform-provider-mackerel/mackerel/resource_mackerel_dashboard.go

Lines 143 to 342 in 2918d8d

Schema: map[string]*schema.Schema{
"title": {
Type: schema.TypeString,
Required: true,
},
"memo": {
Type: schema.TypeString,
Optional: true,
},
"url_path": {
Type: schema.TypeString,
Optional: true,
},
"graph": {
Type: schema.TypeList,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"title": {
Type: schema.TypeString,
Required: true,
},
"host": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"host_id": {
Type: schema.TypeString,
Required: true,
},
"name": {
Type: schema.TypeString,
Required: true,
},
},
},
},
"role": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"role_fullname": {
Type: schema.TypeString,
Required: true,
},
"name": {
Type: schema.TypeString,
Required: true,
},
"is_stacked": {
Type: schema.TypeBool,
Optional: true,
Default: false,
},
},
},
},
"service": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"service_name": {
Type: schema.TypeString,
Required: true,
},
"name": {
Type: schema.TypeString,
Required: true,
},
},
},
},
"expression": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"expression": {
Type: schema.TypeString,
Required: true,
},
},
},
},
"query": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"query": {
Type: schema.TypeString,
Required: true,
},
"legend": {
Type: schema.TypeString,
Optional: true,
},
},
},
},
"range": {
Type: schema.TypeList,
Optional: true,
MaxItems: 1,
Elem: dashboardRangeResource,
},
"layout": {
Type: schema.TypeList,
Required: true,
MaxItems: 1,
Elem: dashboardLayoutResource,
},
},
},
},
"value": {
Type: schema.TypeList,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"title": {
Type: schema.TypeString,
Required: true,
},
"metric": {
Type: schema.TypeList,
Required: true,
MaxItems: 1,
Elem: dashboardMetricResource,
},
"fraction_size": {
Type: schema.TypeInt,
Optional: true,
},
"suffix": {
Type: schema.TypeString,
Required: true,
},
"layout": {
Type: schema.TypeList,
Required: true,
MaxItems: 1,
Elem: dashboardLayoutResource,
},
},
},
},
"markdown": {
Type: schema.TypeList,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"title": {
Type: schema.TypeString,
Required: true,
},
"markdown": {
Type: schema.TypeString,
Required: true,
},
"layout": {
Type: schema.TypeList,
Required: true,
MaxItems: 1,
Elem: dashboardLayoutResource,
},
},
},
},
"alert_status": {
Type: schema.TypeList,
Optional: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"title": {
Type: schema.TypeString,
Required: true,
},
"role_fullname": {
Type: schema.TypeString,
Required: true,
},
"layout": {
Type: schema.TypeList,
Required: true,
MaxItems: 1,
Elem: dashboardLayoutResource,
},
},
},
},
},

tosuke
tosuke commented Sep 6, 2024
View reviewed changes
internal/provider/data_source_mackerel_dashboard.go
Comment on lines +73 to +99
return schema.Schema{
Description: "This data source allows access to details of a specific dashboard.",
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Description: schemaDashboardIDDesc,
Required: true,
},
"title": schema.StringAttribute{
Description: schemaDashboardTitleDesc,
Computed: true,
},
"memo": schema.StringAttribute{
Description: schemaDashboardMemoDesc,
Computed: true,
},
"url_path": schema.StringAttribute{
Description: schemaDashboardURLPathDesc,
Computed: true,
},
"created_at": schema.Int64Attribute{
Description: schemaDashboardCreatedAtDesc,
Computed: true,
},
"updated_at": schema.Int64Attribute{
Description: schemaDashboardUpdatedAtDesc,
Computed: true,
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Original schema:

terraform-provider-mackerel/mackerel/data_source_mackerel_dashboard.go

Lines 137 to 336 in 2918d8d

Schema: map[string]*schema.Schema{
"id": {
Type: schema.TypeString,
Required: true,
},
"title": {
Type: schema.TypeString,
Computed: true,
},
"memo": {
Type: schema.TypeString,
Computed: true,
},
"url_path": {
Type: schema.TypeString,
Computed: true,
},
"created_at": {
Type: schema.TypeInt,
Computed: true,
},
"updated_at": {
Type: schema.TypeInt,
Computed: true,
},
"graph": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"title": {
Type: schema.TypeString,
Computed: true,
},
"host": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"host_id": {
Type: schema.TypeString,
Computed: true,
},
"name": {
Type: schema.TypeString,
Computed: true,
},
},
},
},
"role": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"role_fullname": {
Type: schema.TypeString,
Computed: true,
},
"name": {
Type: schema.TypeString,
Computed: true,
},
"is_stacked": {
Type: schema.TypeBool,
Computed: true,
},
},
},
},
"service": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"service_name": {
Type: schema.TypeString,
Computed: true,
},
"name": {
Type: schema.TypeString,
Computed: true,
},
},
},
},
"expression": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"expression": {
Type: schema.TypeString,
Computed: true,
},
},
},
},
"query": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"query": {
Type: schema.TypeString,
Computed: true,
},
"legend": {
Type: schema.TypeString,
Computed: true,
},
},
},
},
"range": {
Type: schema.TypeList,
Computed: true,
Elem: dashboardRangeDataResource,
},
"layout": {
Type: schema.TypeList,
Computed: true,
Elem: dashboardLayoutDataResource,
},
},
},
},
"value": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"title": {
Type: schema.TypeString,
Computed: true,
},
"metric": {
Type: schema.TypeList,
Computed: true,
Elem: dashboardMetricDataResource,
},
"fraction_size": {
Type: schema.TypeInt,
Computed: true,
},
"suffix": {
Type: schema.TypeString,
Computed: true,
},
"layout": {
Type: schema.TypeList,
Computed: true,
Elem: dashboardLayoutDataResource,
},
},
},
},
"markdown": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"title": {
Type: schema.TypeString,
Computed: true,
},
"markdown": {
Type: schema.TypeString,
Computed: true,
},
"layout": {
Type: schema.TypeList,
Computed: true,
Elem: dashboardLayoutDataResource,
},
},
},
},
"alert_status": {
Type: schema.TypeList,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"title": {
Type: schema.TypeString,
Computed: true,
},
"role_fullname": {
Type: schema.TypeString,
Computed: true,
},
"layout": {
Type: schema.TypeList,
Computed: true,
Elem: dashboardLayoutDataResource,
},
},
},
},
},

tosuke
tosuke commented Sep 6, 2024
View reviewed changes
go.mod
github.com/hashicorp/terraform-plugin-framework v1.8.0
github.com/hashicorp/terraform-plugin-framework v1.11.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Embedded struct in a schema is supported from v1.11.0 (FYI: https://github.com/hashicorp/terraform-plugin-framework/releases/tag/v1.11.0).

@tosuke tosuke marked this pull request as ready for review September 6, 2024 04:07
yohfee
yohfee reviewed Sep 10, 2024
View reviewed changes
Copy link
Contributor

@yohfee yohfee left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It seems almost ok, but just in case.

Since this is a migration, is it okay to leave the things that are not supported from the original as they are?
For example, there are valueRange, formatRules, referenceLines and so on in help page.

internal/provider/resource_mackerel_dashboard.go Outdated Show resolved Hide resolved
@tosuke
Copy link
Contributor Author

tosuke commented Sep 12, 2024

@yohfee These features are not yet supported because they are outside the scope of this PR. When importing a dashboard that uses these features, some of the settings will be lost, so an error will be displayed to inform the user(2145cb5).

yohfee
yohfee approved these changes Sep 12, 2024
View reviewed changes
Copy link
Contributor

@yohfee yohfee left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM 👍

@tosuke tosuke merged commit 5f8da91 into mackerelio-labs:main Sep 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yohfee yohfee yohfee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tosuke @yohfee