Skip to content

Commit

Permalink
fix(ssh-key): data inconsistency with empty label objects (#922)
Browse files Browse the repository at this point in the history
When we implemented the Label helper we added some extra logic that
handled the difference between the `label` attribute default (null) and
the Hetzner Cloud API default (`{}`). This worked well in our test
cases, but breaks if you pass an empty object to the attribute:

```
When applying changes to hcloud_ssh_key.this, provider
"provider[\"registry.terraform.io/hetznercloud/hcloud\"]" produced an
unexpected new value: .labels: was cty.MapValEmpty(cty.String), but now
null.
```

We have now fixed this by setting the default of the labels field to an
empty object to match the return value of the API. With this, we no
longer need the workaround to handle null labels in Terraform config.

Fixes #921
  • Loading branch information
apricote committed May 2, 2024
1 parent 932c47b commit 7e1bf2c
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 9 deletions.
1 change: 1 addition & 0 deletions internal/loadbalancer/resource_target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/terraform"

"github.com/hetznercloud/hcloud-go/hcloud"
"github.com/hetznercloud/terraform-provider-hcloud/internal/loadbalancer"
"github.com/hetznercloud/terraform-provider-hcloud/internal/network"
Expand Down
1 change: 1 addition & 0 deletions internal/sshkey/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/hetznercloud/terraform-provider-hcloud/internal/teste2e"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"

"github.com/hetznercloud/hcloud-go/hcloud"
"github.com/hetznercloud/terraform-provider-hcloud/internal/testsupport"
"github.com/hetznercloud/terraform-provider-hcloud/internal/testtemplate"
Expand Down
15 changes: 7 additions & 8 deletions internal/util/resourceutil/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package resourceutil
import (
"context"

"github.com/hashicorp/terraform-plugin-framework/attr"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/mapdefault"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"

Expand Down Expand Up @@ -42,21 +44,18 @@ func LabelsSchema() schema.MapAttribute {
return schema.MapAttribute{
MarkdownDescription: "User-defined [labels](https://docs.hetzner.cloud/#labels) (key-value pairs) for the resource.",
Optional: true,
Computed: true, // Required to use Default
ElementType: types.StringType,
// In the resource schemas, labels can be null, but the API always returns an empty object for labels.
// To avoid a data consistency issue, we set the default value to the empty object.
Default: mapdefault.StaticValue(types.MapValueMust(types.StringType, map[string]attr.Value{})),
Validators: []validator.Map{
labelsValidator{},
},
}
}

// LabelsMapValueFrom prepare the labels from the API to be assigned into the resource model.
//
// In the resource schemas, labels can be null, but the API always returns an empty object for labels.
// This causes a conflict in the Terraform Data Consistency check. This method handles empty label
// objects by instead returning a null map.
func LabelsMapValueFrom(ctx context.Context, in map[string]string) (types.Map, diag.Diagnostics) {
if len(in) > 0 {
return types.MapValueFrom(ctx, types.StringType, in)
}
return types.MapNull(types.StringType), nil
return types.MapValueFrom(ctx, types.StringType, in)
}
2 changes: 1 addition & 1 deletion internal/util/resourceutil/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func TestLabelsMapValueFrom(t *testing.T) {
{
name: "Empty Map",
in: map[string]string{},
want: types.MapNull(types.StringType),
want: types.MapValueMust(types.StringType, map[string]attr.Value{}),
diags: nil,
},
}
Expand Down

0 comments on commit 7e1bf2c

Please sign in to comment.