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 notification groups with tf-framework #221

Merged
merged 3 commits into from
Jul 4, 2024
Merged

Reimplement notification groups with tf-framework #221

merged 3 commits into from
Jul 4, 2024

Conversation

tosuke
Copy link
Contributor

@tosuke tosuke commented Jun 26, 2024 •
edited
Loading

Output from acceptance testing:

$ MACKEREL_EXPERIMENTAL_TFFRAMEWORK=1 make testacc TESTS='"TestAcc(DataSource)?MackerelNotificationGroup"'
TF_ACC=1 go test -v ./mackerel/... -run "TestAcc(DataSource)?MackerelNotificationGroup" -timeout 120m
2024/07/03 20:27:09 [INFO] mackerel: use terraform-plugin-framework based implementation
=== RUN   TestAccDataSourceMackerelNotificationGroup
=== PAUSE TestAccDataSourceMackerelNotificationGroup
=== RUN   TestAccMackerelNotificationGroup
=== PAUSE TestAccMackerelNotificationGroup
=== CONT  TestAccDataSourceMackerelNotificationGroup
=== CONT  TestAccMackerelNotificationGroup
--- PASS: TestAccDataSourceMackerelNotificationGroup (6.15s)
--- PASS: TestAccMackerelNotificationGroup (10.66s)
PASS
ok      github.com/mackerelio-labs/terraform-provider-mackerel/mackerel 12.024s

@tosuke tosuke force-pushed the framework-notification-group branch from f26e8cd to 2875806 Compare July 3, 2024 11:23
@tosuke tosuke force-pushed the framework-notification-group branch from 2875806 to f8eef7b Compare July 3, 2024 11:26
tosuke
tosuke commented Jul 3, 2024
View reviewed changes
internal/provider/data_source_mackerel_notification_group.go
Comment on lines +30 to +97
resp.Schema = schema.Schema{
Description: "This data source allows access to details of a specific notitication group setting.",

Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Description: "The ID of the notitication group",

Required: true,
},
"name": schema.StringAttribute{
Description: "The name of the notification group",

Computed: true,
},
"notification_level": schema.StringAttribute{
MarkdownDescription: "The level of notitication (`all` or `critical`)",

Computed: true,
},
"child_notification_group_ids": schema.SetAttribute{
Description: "A set of notification group IDs",

ElementType: types.StringType,
Computed: true,
},
"child_channel_ids": schema.SetAttribute{
Description: "A set of notification channel IDs",

ElementType: types.StringType,
Computed: true,
},
},
// TODO: migrate to nested attributes (terraform plugin protocol v6 is required)
Blocks: map[string]schema.Block{
"monitor": schema.SetNestedBlock{
Description: "A set of notification target monitor rules",

NestedObject: schema.NestedBlockObject{
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Description: "The monitor rule ID",

Computed: true,
},
"skip_default": schema.BoolAttribute{
Description: "If true, send notifications to this notification group only",

Computed: true,
},
},
},
},
"service": schema.SetNestedBlock{
Description: "A set of notification target services",

NestedObject: schema.NestedBlockObject{
Attributes: map[string]schema.Attribute{
"name": schema.StringAttribute{
Description: "the name of the service",

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_notification_group.go

Lines 15 to 48 in 3c50804

Schema: map[string]*schema.Schema{
"id": {
Type: schema.TypeString,
Required: true,
},
"name": {
Type: schema.TypeString,
Computed: true,
},
"notification_level": {
Type: schema.TypeString,
Computed: true,
},
"child_notification_group_ids": {
Type: schema.TypeSet,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"child_channel_ids": {
Type: schema.TypeSet,
Computed: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"monitor": {
Type: schema.TypeSet,
Computed: true,
Elem: monitorResource,
},
"service": {
Type: schema.TypeSet,
Computed: true,
Elem: serviceResource,
},
},

tosuke
tosuke commented Jul 3, 2024
View reviewed changes
internal/provider/resource_mackerel_notification_group.go Outdated
Comment on lines 37 to 117
resp.Schema = schema.Schema{
Description: "This resouce allows creating and management of notification groups",

Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Description: "The ID of the notitication group",

Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(),
},
},
"name": schema.StringAttribute{
Description: "The name of the notification group",

Required: true,
},
"notification_level": schema.StringAttribute{
MarkdownDescription: "The level of notitication (`all` or `critical`)",

Optional: true,
Computed: true,
Validators: []validator.String{
mackerel.NotificationLevelValidator(),
},
Default: stringdefault.StaticString("all"),
},
"child_notification_group_ids": schema.SetAttribute{
Description: "A set of notification group IDs",

ElementType: types.StringType,
Optional: true,
},
"child_channel_ids": schema.SetAttribute{
Description: "A set of notification channel IDs",

ElementType: types.StringType,
Optional: true,
},
},
// TODO: migrate to nested attributes
Blocks: map[string]schema.Block{
"monitor": schema.SetNestedBlock{
Description: "Configuration block(s) with monitor rules",

NestedObject: schema.NestedBlockObject{
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Description: "The monitor rule ID",

Required: true,
},
"skip_default": schema.BoolAttribute{
Description: "If true, send notifications to this notification group only.",

Optional: true,
Computed: true,
Default: booldefault.StaticBool(false),
},
},
},
},
"service": schema.SetNestedBlock{
Description: "Configuration block(s) with services",

NestedObject: schema.NestedBlockObject{
Attributes: map[string]schema.Attribute{
"name": schema.StringAttribute{
Description: "The name of the service",

Required: true,
Validators: []validator.String{
mackerel.ServiceNameValidator(),
},
},
},
},
},
},
}
}
Copy link
Contributor Author

@tosuke tosuke Jul 3, 2024 •
edited
Loading

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_notification_group.go

Lines 44 to 75 in 3c50804

Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
},
"notification_level": {
Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{"all", "critical"}, false),
Default: "all",
},
"child_notification_group_ids": {
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"child_channel_ids": {
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"monitor": {
Type: schema.TypeSet,
Optional: true,
Elem: monitorResource,
},
"service": {
Type: schema.TypeSet,
Optional: true,
Elem: serviceResource,
},
},

tosuke
tosuke commented Jul 3, 2024
View reviewed changes
internal/provider/resource_mackerel_notification_group.go
Comment on lines +103 to +112
Attributes: map[string]schema.Attribute{
"name": schema.StringAttribute{
Description: "The name of the service",

Required: true,
Validators: []validator.String{
mackerel.ServiceNameValidator(),
},
},
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

terraform-provider-mackerel/mackerel/resource_mackerel_notification_group.go

Lines 12 to 19 in 3c50804

var serviceResource = &schema.Resource{
Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
},
},
}

tosuke
tosuke commented Jul 3, 2024
View reviewed changes
internal/provider/resource_mackerel_notification_group.go
Comment on lines +83 to +96
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Description: "The monitor rule ID",

Required: true,
},
"skip_default": schema.BoolAttribute{
Description: "If true, send notifications to this notification group only.",

Optional: true,
Computed: true,
Default: booldefault.StaticBool(false),
},
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

terraform-provider-mackerel/mackerel/resource_mackerel_notification_group.go

Lines 21 to 33 in 3c50804

var monitorResource = &schema.Resource{
Schema: map[string]*schema.Schema{
"id": {
Type: schema.TypeString,
Required: true,
},
"skip_default": {
Type: schema.TypeBool,
Optional: true,
Default: false,
},
},
}

tosuke
tosuke commented Jul 3, 2024
View reviewed changes
internal/provider/resource_mackerel_notification_group.go
},
},
// TODO: migrate to nested attributes
Blocks: map[string]schema.Block{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Blocks are a restricted version of nested object attributes (e.g. cannot be used as expressions).

Copy link
Member

Choose a reason for hiding this comment

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

I read the document. https://developer.hashicorp.com/terraform/plugin/framework/migrating/attributes-blocks/blocks

tosuke
tosuke commented Jul 3, 2024
View reviewed changes
internal/provider/resource_mackerel_notification_group.go
Comment on lines +92 to +94
Optional: true,
Computed: true,
Default: booldefault.StaticBool(false),
Copy link
Contributor Author

@tosuke tosuke Jul 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Attributes with a default value are both inputs and outputs. If the user provides a value, they are act as input. If not, the provider computes and provides a default value. So they need not to be only optional but also computed.

@tosuke tosuke force-pushed the framework-notification-group branch from f8eef7b to 4cfea49 Compare July 3, 2024 11:52
@tosuke tosuke marked this pull request as ready for review July 3, 2024 11:52
Arthur1
Arthur1 approved these changes Jul 4, 2024
View reviewed changes
Copy link
Member

@Arthur1 Arthur1 left a comment

Choose a reason for hiding this comment

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

👍

internal/provider/resource_mackerel_notification_group.go
},
},
// TODO: migrate to nested attributes
Blocks: map[string]schema.Block{
Copy link
Member

Choose a reason for hiding this comment

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

I read the document. https://developer.hashicorp.com/terraform/plugin/framework/migrating/attributes-blocks/blocks

@Arthur1 Arthur1 merged commit 780e445 into mackerelio-labs:main Jul 4, 2024
1 check passed
@tosuke tosuke deleted the framework-notification-group branch July 4, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Arthur1 Arthur1 Arthur1 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 @Arthur1