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 role metadata with tf-framework #228

Merged
merged 4 commits into from
Aug 9, 2024
Merged

Reimplement role metadata with tf-framework #228

merged 4 commits into from
Aug 9, 2024
+638 −1

Conversation

tosuke
Copy link
Contributor

@tosuke tosuke commented Jul 10, 2024 •
edited
Loading

Output from acceptance testing:

$ MACKEREL_EXPERIMENTAL_TFFRAMEWORK=1 make testacc TESTS='"TestAcc(DataSource)?MackerelRoleMetadata"'
TF_ACC=1 go test -v ./mackerel/... -run "TestAcc(DataSource)?MackerelRoleMetadata" -timeout 120m
2024/08/07 17:16:18 [INFO] mackerel: use terraform-plugin-framework based implementation
=== RUN   TestAccDataSourceMackerelRoleMetadata
=== PAUSE TestAccDataSourceMackerelRoleMetadata
=== RUN   TestAccMackerelRoleMetadata
=== PAUSE TestAccMackerelRoleMetadata
=== CONT  TestAccDataSourceMackerelRoleMetadata
=== CONT  TestAccMackerelRoleMetadata
--- PASS: TestAccDataSourceMackerelRoleMetadata (6.20s)
--- PASS: TestAccMackerelRoleMetadata (11.06s)
PASS
ok      github.com/mackerelio-labs/terraform-provider-mackerel/mackerel 12.943s

@tosuke tosuke force-pushed the framework-role-metadata branch 2 times, most recently from dad098d to a7ca184 Compare August 7, 2024 08:37
tosuke
tosuke commented Aug 7, 2024
View reviewed changes
internal/provider/resource_mackerel_role_metadata.go
Comment on lines +38 to +77
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Computed: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.UseStateForUnknown(), // immutable
},
},
"service": schema.StringAttribute{
Description: "The name of the service.",
Required: true,
Validators: []validator.String{
mackerel.ServiceNameValidator(),
},
PlanModifiers: []planmodifier.String{
stringplanmodifier.RequiresReplace(), // force new
},
},
"role": schema.StringAttribute{
Description: "The name of the role.",
Required: true,
Validators: []validator.String{
mackerel.RoleNameValidator(),
},
PlanModifiers: []planmodifier.String{
stringplanmodifier.RequiresReplace(), // force new
},
},
"namespace": schema.StringAttribute{
Description: "The identifier for the metadata.",
Required: true,
PlanModifiers: []planmodifier.String{
stringplanmodifier.RequiresReplace(), // force new
},
},
"metadata_json": schema.StringAttribute{
Description: "The arbitrary JSON data for the role.",
Required: true,
CustomType: jsontypes.NormalizedType{},
},
},
Copy link
Contributor Author

@tosuke tosuke Aug 7, 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_role_metadata.go

Lines 24 to 43 in 9a863e6

Schema: map[string]*schema.Schema{
"service": {
Type: schema.TypeString,
Required: true,
},
"role": {
Type: schema.TypeString,
Required: true,
},
"namespace": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
},
"metadata_json": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.StringIsJSON,
},
},

Copy link
Contributor Author

Choose a reason for hiding this comment

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

[memo] The service and role on the original schema does not have ForceNew: true (equivalence of planmodifier.RequiresReplace()). It seems like a bug (FYI: #231).

tosuke
tosuke commented Aug 7, 2024
View reviewed changes
internal/provider/data_source_mackerel_role_metadata.go
Comment on lines +31 to +57
resp.Schema = schema.Schema{
Description: "This data source accesses to details of a specific Role Metadata.",
Attributes: map[string]schema.Attribute{
"id": schema.StringAttribute{
Computed: true,
},
"service": schema.StringAttribute{
Description: "The name of the service.",
Required: true,
Validators: []validator.String{mackerel.ServiceNameValidator()},
},
"role": schema.StringAttribute{
Description: "The name of the role.",
Required: true,
Validators: []validator.String{mackerel.RoleNameValidator()},
},
"namespace": schema.StringAttribute{
Description: "The identifier for the metadata.",
Required: true,
},
"metadata_json": schema.StringAttribute{
Description: "The arbitrary JSON data for the role.",
Computed: true,
CustomType: jsontypes.NormalizedType{},
},
},
}
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_role_metadata.go

Lines 16 to 33 in 9a863e6

Schema: map[string]*schema.Schema{
"service": {
Type: schema.TypeString,
Required: true,
},
"role": {
Type: schema.TypeString,
Required: true,
},
"namespace": {
Type: schema.TypeString,
Required: true,
},
"metadata_json": {
Type: schema.TypeString,
Computed: true,
},
},

@tosuke tosuke marked this pull request as ready for review August 7, 2024 10:44
azukiazusa1
azukiazusa1 approved these changes Aug 8, 2024
View reviewed changes
Copy link
Contributor

@azukiazusa1 azukiazusa1 left a comment

Choose a reason for hiding this comment

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

seems good 👍

@azukiazusa1 azukiazusa1 merged commit e56828c into mackerelio-labs:main Aug 9, 2024
1 check passed
@tosuke tosuke deleted the framework-role-metadata branch August 15, 2024 06:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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