diff --git a/internal/provider/device/model.go b/internal/provider/device/model.go index 4f6d9a185de799dfaae293938c9846fba767ec8c..d261d141a37f1fe022ed58276071bdf681b02618 100644 --- a/internal/provider/device/model.go +++ b/internal/provider/device/model.go @@ -116,7 +116,20 @@ func fromAPI(ctx context.Context, device *v20250101.Device, state utils.Attribut diags.Append(d...) model.Metadata = metadata } else { - model.Metadata = types.MapNull(types.StringType) + // see comment in utils/optional.go + metadataFromState := types.Map{} + d := state.GetAttribute(ctx, path.Root("metadata"), &metadataFromState) + diags.Append(d...) + switch { + case metadataFromState.IsUnknown(): + model.Metadata = types.MapNull(types.StringType) + case metadataFromState.IsNull(): + model.Metadata = metadataFromState + case len(metadataFromState.Elements()) == 0: + model.Metadata = metadataFromState + default: + model.Metadata = types.MapNull(types.StringType) + } } if device.ApprovedAt != nil { @@ -173,7 +186,9 @@ func toAPI(ctx context.Context, m *Model) (*v20250101.DeviceRequest, diag.Diagno Value: v.ValueString(), }) } - d.Metadata = &metadata + if len(metadata) > 0 { + d.Metadata = &metadata + } } if !m.Tags.IsNull() && !m.Tags.IsUnknown() { var tags []string diff --git a/internal/provider/device/resource.go b/internal/provider/device/resource.go index 93a3f3a84901beed2476ca35f28c605f26a3c662..de59ae4341488ba6c1ca8fccf434718b6d0f8c36 100644 --- a/internal/provider/device/resource.go +++ b/internal/provider/device/resource.go @@ -6,12 +6,15 @@ import ( "fmt" "net/http" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-framework/types/basetypes" v20250101 "github.com/smallstep/terraform-provider-smallstep/internal/apiclient/v20250101" "github.com/smallstep/terraform-provider-smallstep/internal/provider/utils" ) @@ -26,6 +29,59 @@ type Resource struct { client *v20250101.Client } +func (r *Resource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { + if req.Plan.Raw.IsNull() { + // Delete + return + } + // Attributes that may either be set by API or synced from an MDM are marked as + // optional and computed. When one of these attributes is removed from the + // config the plan keeps the old value instead of setting it to null. We have + // to modify the plan and change the attribute to null. + optionalComputedStrings := []path.Path{ + path.Root("display_id"), + path.Root("display_name"), + path.Root("serial"), + path.Root("os"), + path.Root("ownership"), + } + for _, p := range optionalComputedStrings { + config := types.String{} + diags := req.Config.GetAttribute(ctx, p, &config) + resp.Diagnostics.Append(diags...) + if !config.IsNull() { + continue + } + + resp.Plan.SetAttribute(ctx, p, config) + } + + email := types.String{} + diags := req.Config.GetAttribute(ctx, path.Root("user").AtName("email"), &email) + resp.Diagnostics.Append(diags...) + if email.IsNull() { + user := basetypes.NewObjectNull(userAttrTypes) + diags = resp.Plan.SetAttribute(ctx, path.Root("user"), user) + resp.Diagnostics.Append(diags...) + } + + tags := types.Set{} + diags = req.Config.GetAttribute(ctx, path.Root("tags"), &tags) + resp.Diagnostics.Append(diags...) + if tags.IsNull() { + diags = resp.Plan.SetAttribute(ctx, path.Root("tags"), tags) + resp.Diagnostics.Append(diags...) + } + + metadata := types.Map{} + diags = req.Config.GetAttribute(ctx, path.Root("metadata"), &metadata) + resp.Diagnostics.Append(diags...) + if metadata.IsNull() || len(metadata.Elements()) == 0 { + diags = resp.Plan.SetAttribute(ctx, path.Root("metadata"), metadata) + resp.Diagnostics.Append(diags...) + } +} + func (r *Resource) Schema(ctx context.Context, req resource.SchemaRequest, resp *resource.SchemaResponse) { device, props, err := utils.Describe("device") if err != nil { @@ -61,7 +117,7 @@ func (r *Resource) Schema(ctx context.Context, req resource.SchemaRequest, resp Required: true, }, "serial": schema.StringAttribute{ - MarkdownDescription: props["permanent_identifier"], + MarkdownDescription: props["serial"], Optional: true, Computed: true, }, @@ -110,6 +166,9 @@ func (r *Resource) Schema(ctx context.Context, req resource.SchemaRequest, resp "email": schema.StringAttribute{ MarkdownDescription: userProps["email"], Required: true, + Validators: []validator.String{ + stringvalidator.LengthBetween(1, 256), + }, }, }, }, @@ -337,6 +396,10 @@ func (r *Resource) Update(ctx context.Context, req resource.UpdateRequest, resp patch.UserEmail = &resource.User.Email } + if len(remove) > 0 { + patch.Remove = &remove + } + httpResp, err := r.client.PatchDevice(ctx, deviceID, &v20250101.PatchDeviceParams{}, patch) if err != nil { resp.Diagnostics.AddError( diff --git a/internal/provider/device/resource_test.go b/internal/provider/device/resource_test.go index 5eb16e697e1ba55726686834dafede475e08bc19..d913b0d2ea814f64baf6f3ff25c2bdabf7b18d3a 100644 --- a/internal/provider/device/resource_test.go +++ b/internal/provider/device/resource_test.go @@ -28,24 +28,63 @@ var providerFactories = map[string]func() (tfprotov6.ProviderServer, error){ "smallstep": providerserver.NewProtocol6WithError(provider), } -func TestAccAuthorityResource(t *testing.T) { - t.Parallel() +const minConfig = ` +resource "smallstep_device" "laptop1" { + permanent_identifier = %q +}` - permanentID := uuid.NewString() +const maxConfig = ` +resource "smallstep_device" "laptop1" { + permanent_identifier = %q + display_id = "9 9" + display_name = "Employee Laptop" + serial = "678" + tags = ["ubuntu"] + metadata = { + k1 = "v1" + } + os = "Windows" + ownership = "company" + user = { + email = "user@example.com" + } +}` - emptyConfig := fmt.Sprintf(` +const emptyConfig = ` resource "smallstep_device" "laptop1" { permanent_identifier = %q -}`, permanentID) + display_id = "" + display_name = "" + serial = "" + tags = [] + metadata = {} +} +` +func TestAccDeviceResource(t *testing.T) { + permanentID := uuid.NewString() + + // min -> max helper.Test(t, helper.TestCase{ ProtoV6ProviderFactories: providerFactories, Steps: []helper.TestStep{ { - Config: emptyConfig, + Config: fmt.Sprintf(minConfig, permanentID), Check: helper.ComposeAggregateTestCheckFunc( helper.TestMatchResourceAttr("smallstep_device.laptop1", "id", regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)), helper.TestCheckResourceAttr("smallstep_device.laptop1", "permanent_identifier", permanentID), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "connected", "false"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "high_assurance", "false"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "display_id"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "display_name"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "serial"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "user"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "os"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "ownership"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "enrolled_at"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "last_seen"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "metadata"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "tags"), ), }, { @@ -53,32 +92,66 @@ resource "smallstep_device" "laptop1" { ImportState: true, ImportStateVerify: true, }, + { + Config: fmt.Sprintf(maxConfig, permanentID), + Check: helper.ComposeAggregateTestCheckFunc( + helper.TestMatchResourceAttr("smallstep_device.laptop1", "id", regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "permanent_identifier", permanentID), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "display_id", "9 9"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "display_name", "Employee Laptop"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "serial", "678"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "user.email", "user@example.com"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "os", "Windows"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "ownership", "company"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "tags.0", "ubuntu"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "metadata.k1", "v1"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "connected", "false"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "high_assurance", "false"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "enrolled_at"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "last_seen"), + ), + }, + }, + }) + + // min -> empty + permanentID1 := uuid.NewString() + helper.Test(t, helper.TestCase{ + ProtoV6ProviderFactories: providerFactories, + Steps: []helper.TestStep{ + { + Config: fmt.Sprintf(minConfig, permanentID1), + }, + { + Config: fmt.Sprintf(emptyConfig, permanentID1), + Check: helper.ComposeAggregateTestCheckFunc( + helper.TestMatchResourceAttr("smallstep_device.laptop1", "id", regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "permanent_identifier", permanentID1), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "display_id", ""), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "display_name", ""), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "serial", ""), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "user"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "os"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "ownership"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "connected", "false"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "high_assurance", "false"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "enrolled_at"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "last_seen"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "metadata.%", "0"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "tags.%", "0"), + ), + }, }, }) permanentID2 := uuid.NewString() - fullConfig := fmt.Sprintf(` -resource "smallstep_device" "laptop1" { - permanent_identifier = %q - display_id = "9 9" - display_name = "Employee Laptop" - serial = "678" - tags = ["ubuntu"] - metadata = { - k1 = "v1" - } - os = "Windows" - ownership = "company" - user = { - email = "user@example.com" - } -}`, permanentID2) + // max -> min helper.Test(t, helper.TestCase{ ProtoV6ProviderFactories: providerFactories, Steps: []helper.TestStep{ { - Config: fullConfig, + Config: fmt.Sprintf(maxConfig, permanentID2), Check: helper.ComposeAggregateTestCheckFunc( helper.TestMatchResourceAttr("smallstep_device.laptop1", "id", regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)), helper.TestCheckResourceAttr("smallstep_device.laptop1", "permanent_identifier", permanentID2), @@ -101,39 +174,34 @@ resource "smallstep_device" "laptop1" { ImportState: true, ImportStateVerify: true, }, + { + Config: fmt.Sprintf(minConfig, permanentID2), + Check: helper.ComposeAggregateTestCheckFunc( + helper.TestMatchResourceAttr("smallstep_device.laptop1", "id", regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "permanent_identifier", permanentID2), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "connected", "false"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "high_assurance", "false"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "display_id"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "display_name"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "serial"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "os"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "ownership"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "user.email"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "metadata"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "tags"), + ), + }, }, }) permanentID3 := uuid.NewString() - config1 := fmt.Sprintf(` -resource "smallstep_device" "laptop1" { - permanent_identifier = %q -}`, permanentID3) - config2 := fmt.Sprintf(` -resource "smallstep_device" "laptop1" { - permanent_identifier = %q - display_id = "9 9" - display_name = "Employee Laptop" - serial = "678" - tags = ["ubuntu"] - metadata = { - k1 = "v1" - } - os = "Windows" - ownership = "company" - user = { - email = "user@example.com" - } -}`, permanentID3) + // max -> empty helper.Test(t, helper.TestCase{ ProtoV6ProviderFactories: providerFactories, Steps: []helper.TestStep{ { - Config: config1, - }, - { - Config: config2, + Config: fmt.Sprintf(maxConfig, permanentID3), Check: helper.ComposeAggregateTestCheckFunc( helper.TestMatchResourceAttr("smallstep_device.laptop1", "id", regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)), helper.TestCheckResourceAttr("smallstep_device.laptop1", "permanent_identifier", permanentID3), @@ -156,6 +224,109 @@ resource "smallstep_device" "laptop1" { ImportState: true, ImportStateVerify: true, }, + { + Config: fmt.Sprintf(emptyConfig, permanentID3), + Check: helper.ComposeAggregateTestCheckFunc( + helper.TestMatchResourceAttr("smallstep_device.laptop1", "id", regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "permanent_identifier", permanentID3), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "display_id", ""), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "display_name", ""), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "serial", ""), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "user"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "os"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "ownership"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "connected", "false"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "high_assurance", "false"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "enrolled_at"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "last_seen"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "metadata.%", "0"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "tags.%", "0"), + ), + }, + }, + }) + + permanentID4 := uuid.NewString() + + // empty -> min + helper.Test(t, helper.TestCase{ + ProtoV6ProviderFactories: providerFactories, + Steps: []helper.TestStep{ + { + Config: fmt.Sprintf(emptyConfig, permanentID4), + Check: helper.ComposeAggregateTestCheckFunc( + helper.TestMatchResourceAttr("smallstep_device.laptop1", "id", regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "permanent_identifier", permanentID4), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "display_id", ""), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "display_name", ""), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "serial", ""), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "user"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "os"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "ownership"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "connected", "false"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "high_assurance", "false"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "enrolled_at"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "last_seen"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "metadata.%", "0"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "tags.%", "0"), + ), + }, + { + ResourceName: "smallstep_device.laptop1", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"display_id", "display_name", "serial"}, + }, + { + Config: fmt.Sprintf(minConfig, permanentID4), + Check: helper.ComposeAggregateTestCheckFunc( + helper.TestMatchResourceAttr("smallstep_device.laptop1", "id", regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "permanent_identifier", permanentID4), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "display_id"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "display_name"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "serial"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "user"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "os"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "ownership"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "connected", "false"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "high_assurance", "false"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "enrolled_at"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "last_seen"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "metadata"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "tags"), + ), + }, + }, + }) + + // empty -> max + permanentID5 := uuid.NewString() + + helper.Test(t, helper.TestCase{ + ProtoV6ProviderFactories: providerFactories, + Steps: []helper.TestStep{ + { + Config: fmt.Sprintf(emptyConfig, permanentID5), + }, + { + Config: fmt.Sprintf(maxConfig, permanentID5), + Check: helper.ComposeAggregateTestCheckFunc( + helper.TestMatchResourceAttr("smallstep_device.laptop1", "id", regexp.MustCompile(`^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`)), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "permanent_identifier", permanentID5), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "display_id", "9 9"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "display_name", "Employee Laptop"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "serial", "678"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "user.email", "user@example.com"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "os", "Windows"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "ownership", "company"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "tags.0", "ubuntu"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "metadata.k1", "v1"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "connected", "false"), + helper.TestCheckResourceAttr("smallstep_device.laptop1", "high_assurance", "false"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "enrolled_at"), + helper.TestCheckNoResourceAttr("smallstep_device.laptop1", "last_seen"), + ), + }, }, }) }