From b8a88b3cc629998da721bd04a75f6b0173c03124 Mon Sep 17 00:00:00 2001 From: tagur87 <43474056+tagur87@users.noreply.github.com> Date: Tue, 1 Aug 2023 12:15:03 -0400 Subject: [PATCH 1/3] feat: Add funcs for custom fields of any type Create functions for overahauling custom fields to support multiple underlying types. Create function for the schema with using TypeString instead of TypeMap. This allows for complex json types using jsonencode/jsondecode. Create handleCustomFieldUpdate function and tests for it. This function takes in the results of a `d.GetChange` funtion `old` & `new`, which allows us to compare old and new values, and update the `custom_fields` field in the api. Add unit test to validate functionality. handleCustomFieldRead and tests were added to handle the reading of custom fields and setting the field to "" if all the custom fields are nil. Adds tests to validate the functionality. --- netbox/custom_fields.go | 83 ++++++++++++++++++++++++ netbox/custom_fields_test.go | 118 +++++++++++++++++++++++++++++++++++ 2 files changed, 201 insertions(+) create mode 100644 netbox/custom_fields_test.go diff --git a/netbox/custom_fields.go b/netbox/custom_fields.go index ad35c37d..3afbde56 100644 --- a/netbox/custom_fields.go +++ b/netbox/custom_fields.go @@ -1,7 +1,11 @@ package netbox import ( + "encoding/json" + "fmt" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) const customFieldsKey = "custom_fields" @@ -23,3 +27,82 @@ func getCustomFields(cf interface{}) map[string]interface{} { } return cfm } + +// customFieldsSchemaFunc is a function that returns the schema for all custom +// fields. +func customFieldsSchemaFunc() *schema.Schema { + return &schema.Schema{ + Type: schema.TypeString, + Optional: true, + Description: "A JSON string that defines the custom fields as defined under the `custom_fields` key in the object's api." + + "This is best managed with the `jsonencode()` & `jsondecode()` functions.", + ValidateFunc: validation.StringIsJSON, + } +} + +// handleCustomFieldUpdate is a function that takes in the old and new values returned +// from a terraform d.GetChange() function and returns a map with the values to be sent +// in the custom_fields field on an update to the netbox api. +// This function handles setting the custom_field fields to null when needed. It does +// this by comparing the custom fields that were previously in terraform state, and +// compares them with then new state. It then sets any fields that were previously +// set to nil, if the new state does not include them. +func handleCustomFieldUpdate(old, new interface{}) (map[string]interface{}, error) { + ret := make(map[string]interface{}) + var newData map[string]interface{} + + if new.(string) != "" { + err := json.Unmarshal([]byte(new.(string)), &newData) + if err != nil { + return nil, fmt.Errorf("err1: %w", err) + } + for k, v := range newData { + ret[k] = v + } + } + var oldData map[string]interface{} + if old.(string) != "" { + err := json.Unmarshal([]byte(old.(string)), &oldData) + if err != nil { + return nil, fmt.Errorf("err2: %w", err) + } + for k := range oldData { + if val, ok := ret[k]; !ok { + ret[k] = nil + } else { + ret[k] = val + } + } + } + return ret, nil +} + +// handleCustomFieldRead is a function that take an input of the interface +// from the CustomField struct field, and returns a string with the value +// to set for terraform and an error. +// This function checks the number of keys in the map, and if the number of +// nil fields equal the number of fields, it returns an empty string. Since +// this means the custom fields are not managed. Otherwise, it will return +// the result of unmarshalling the field to a json string. +// This allows the custom_field field to be set to empty and have an empty plan +// even though the api still has the custom fields with null values. +func handleCustomFieldRead(cf interface{}) (string, error) { + cfMap, ok := cf.(map[string]interface{}) + if !ok { + return "", fmt.Errorf("cannot cast %v to map[string]interface{}", cf) + } + numNull := 0 + for k := range cfMap { + if cfMap[k] == nil { + numNull += 1 + } + } + if len(cfMap) == 0 || numNull == len(cfMap) { + return "", nil + } + b, err := json.Marshal(cf) + if err != nil { + return "", err + } + return string(b), nil +} diff --git a/netbox/custom_fields_test.go b/netbox/custom_fields_test.go new file mode 100644 index 00000000..7214d7f1 --- /dev/null +++ b/netbox/custom_fields_test.go @@ -0,0 +1,118 @@ +package netbox + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_handleCustomFieldUpdate(t *testing.T) { + type args struct { + old interface{} + new interface{} + } + tests := []struct { + name string + args args + want map[string]interface{} + wantErr bool + }{ + { + name: "create new custom field", + args: args{ + old: "", + new: "{\"a\": \"b\", \"c\": true, \"d\": {\"e\": \"f\"}}", + }, + want: map[string]interface{}{ + "a": "b", + "c": true, + "d": map[string]interface{}{ + "e": "f", + }, + }, + wantErr: false, + }, + { + name: "update custom fields", + args: args{ + old: "{\"a\": \"b\", \"c\": true, \"d\": {\"e\": \"f\"}}", + new: "{\"a\": \"q\", \"c\": false}", + }, + want: map[string]interface{}{ + "a": "q", + "c": false, + "d": nil, + }, + wantErr: false, + }, + { + name: "remove custom fields", + args: args{ + old: "{\"a\": \"b\", \"c\": true, \"d\": {\"e\": \"f\"}}", + new: "", + }, + want: map[string]interface{}{ + "a": nil, + "c": nil, + "d": nil, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := handleCustomFieldUpdate(tt.args.old, tt.args.new) + if (err != nil) != tt.wantErr { + t.Errorf("handleCustomFieldUpdate() error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.Equal(t, tt.want, got) + }) + } +} + +func Test_handleCustomFieldRead(t *testing.T) { + tests := []struct { + name string + cf interface{} + want string + wantErr bool + }{ + { + name: "all fields are nil", + cf: map[string]interface{}{ + "a": nil, + "c": nil, + "d": nil, + }, + want: "", + wantErr: false, + }, + { + name: "one field is valid", + cf: map[string]interface{}{ + "a": nil, + "c": true, + "d": nil, + }, + want: "{\"a\":null,\"c\":true,\"d\":null}", + wantErr: false, + }, + { + name: "cannot marshal nil", + cf: nil, + want: "", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := handleCustomFieldRead(tt.cf) + if (err != nil) != tt.wantErr { + t.Errorf("handleCustomFieldRead() error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.Equal(t, tt.want, got) + }) + } +} From eedbd4700c4ebf003127c140ed857232736fe3ae Mon Sep 17 00:00:00 2001 From: tagur87 <43474056+tagur87@users.noreply.github.com> Date: Tue, 1 Aug 2023 12:19:31 -0400 Subject: [PATCH 2/3] feat: allow any custom field type in netbox_devices data source Update the data source `netbox_devices` to use the string based schema for the `custom_fields` attribute. This allows for any underlying type to be accessed by using the `jsondecode()` terraform function. For example, to access custom fields, the following code can be used. ```terraform data "netbox_devices" "test" { filter { name = "name" value = "device1" } } output "device_field" { value = jsondecode(data.netbox_devices.test[0].custom_fields).field1 } ``` --- docs/data-sources/devices.md | 1 + netbox/data_source_netbox_devices.go | 10 +++++++--- netbox/data_source_netbox_devices_test.go | 18 +++++++++++++----- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/docs/data-sources/devices.md b/docs/data-sources/devices.md index 9528c396..9da5d1c7 100644 --- a/docs/data-sources/devices.md +++ b/docs/data-sources/devices.md @@ -45,6 +45,7 @@ Read-Only: - `comments` (String) - `custom_fields` (Map of String) - `description` (String) +- `custom_fields` (String) - `device_id` (Number) - `device_type_id` (Number) - `location_id` (Number) diff --git a/netbox/data_source_netbox_devices.go b/netbox/data_source_netbox_devices.go index 13d3463f..5e1ab822 100644 --- a/netbox/data_source_netbox_devices.go +++ b/netbox/data_source_netbox_devices.go @@ -64,8 +64,10 @@ func dataSourceNetboxDevices() *schema.Resource { Computed: true, }, "custom_fields": { - Type: schema.TypeMap, + Type: schema.TypeString, Computed: true, + Description: "A JSON string that defines the custom fields as defined under the `custom_fields` key in the object's api." + + "The data can be accessed by using the `jsondecode()` function around the `custom_fields` attribute.", }, "description": { Type: schema.TypeString, @@ -268,9 +270,11 @@ func dataSourceNetboxDevicesRead(d *schema.ResourceData, m interface{}) error { if device.Status != nil { mapping["status"] = *device.Status.Value } - if device.CustomFields != nil { - mapping["custom_fields"] = device.CustomFields + cf, err := handleCustomFieldRead(device.CustomFields) + if err != nil { + return err } + mapping["custom_fields"] = cf if device.Rack != nil { mapping["rack_id"] = device.Rack.ID } diff --git a/netbox/data_source_netbox_devices_test.go b/netbox/data_source_netbox_devices_test.go index 3f63bc84..a218345e 100644 --- a/netbox/data_source_netbox_devices_test.go +++ b/netbox/data_source_netbox_devices_test.go @@ -245,7 +245,6 @@ func TestAccNetboxDevicesDataSource_CustomFields(t *testing.T) { data "netbox_devices" "test" { depends_on = [ netbox_device.test, - netbox_custom_field.test, ] filter { @@ -254,12 +253,18 @@ data "netbox_devices" "test" { } } -resource "netbox_custom_field" "test" { - name = "%[1]s" +resource "netbox_custom_field" "text" { + name = "%[1]s_text" type = "text" content_types = ["dcim.device"] } +resource "netbox_custom_field" "boolean" { + name = "%[1]s_boolean" + type = "boolean" + content_types = ["dcim.device"] +} + resource "netbox_device" "test" { name = "%[2]s" comments = "thisisacomment" @@ -274,7 +279,10 @@ resource "netbox_device" "test" { location_id = netbox_location.test.id status = "staged" serial = "ABCDEF" - custom_fields = {"${netbox_custom_field.test.name}" = "81"} + custom_fields = jsonencode({ + "${netbox_custom_field.text.name}" = "81" + "${netbox_custom_field.boolean.name}" = true + }) } `, testField, testName), Check: resource.ComposeTestCheckFunc( @@ -290,7 +298,7 @@ resource "netbox_device" "test" { resource.TestCheckResourceAttrPair("data.netbox_devices.test", "devices.0.location_id", "netbox_location.test", "id"), resource.TestCheckResourceAttr("data.netbox_devices.test", "devices.0.serial", "ABCDEF"), resource.TestCheckResourceAttr("data.netbox_devices.test", "devices.0.status", "staged"), - resource.TestCheckResourceAttr("data.netbox_devices.test", "devices.0.custom_fields."+testField, "81"), + resource.TestCheckResourceAttr("data.netbox_devices.test", "devices.0.custom_fields", "{\""+testField+"_boolean\":true,\""+testField+"_text\":\"81\"}"), ), }, }, From df26a3937a09a15167d8304e8344bedcc93b78a4 Mon Sep 17 00:00:00 2001 From: tagur87 <43474056+tagur87@users.noreply.github.com> Date: Tue, 1 Aug 2023 12:25:39 -0400 Subject: [PATCH 3/3] feat: allow any custom field type in netbox_device Updates the `netbox_device` resource to allow for any custom field type to be set. This is done by using a string field with the `jsonencode()` and `jsondecode()` terraform functions. This allows for any complex custom fields or data types that are needed, since the underlying types don't matter and are unmarshalled using map[string]interface{}. To set custom fields using this new method, use the following syntax. ```terraform resource "netbox_device" "test" { name = "device1" tenant_id = netbox_tenant.test.id role_id = netbox_device_role.test.id device_type_id = netbox_device_type.test.id site_id = netbox_site.test.id custom_fields = jsonencode({ "text_field" = "81" "bool_field" = true }) } ``` NOTE: This may be a breaking change for some users. --- docs/resources/device.md | 1 + netbox/resource_netbox_device.go | 25 ++++++---- netbox/resource_netbox_device_test.go | 67 +++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 8 deletions(-) diff --git a/docs/resources/device.md b/docs/resources/device.md index e7be169f..56e274b7 100644 --- a/docs/resources/device.md +++ b/docs/resources/device.md @@ -64,6 +64,7 @@ resource "netbox_device" "test" { - `custom_fields` (Map of String) - `description` (String) - `local_context_data` (String) This is best managed through the use of `jsonencode` and a map of settings. +- `custom_fields` (String) A JSON string that defines the custom fields as defined under the `custom_fields` key in the object's api.This is best managed with the `jsonencode()` & `jsondecode()` functions. - `location_id` (Number) - `platform_id` (Number) - `rack_face` (String) Valid values are `front` and `rear`. Required when `rack_position` is set. diff --git a/netbox/resource_netbox_device.go b/netbox/resource_netbox_device.go index e82b2b3a..42f27f21 100644 --- a/netbox/resource_netbox_device.go +++ b/netbox/resource_netbox_device.go @@ -112,7 +112,7 @@ func resourceNetboxDevice() *schema.Resource { Optional: true, Description: "This is best managed through the use of `jsonencode` and a map of settings.", }, - customFieldsKey: customFieldsSchema, + customFieldsKey: customFieldsSchemaFunc(), }, Importer: &schema.ResourceImporter{ StateContext: schema.ImportStatePassthroughContext, @@ -203,9 +203,14 @@ func resourceNetboxDeviceCreate(ctx context.Context, d *schema.ResourceData, m i } } - ct, ok := d.GetOk(customFieldsKey) + cf, ok := d.GetOk(customFieldsKey) if ok { - data.CustomFields = ct + var cfMap map[string]interface{} + err := json.Unmarshal([]byte(cf.(string)), &cfMap) + if err != nil { + return diag.Errorf("error in resourceNetboxDeviceCreate[CustomFields]: %v", err) + } + data.CustomFields = cfMap } data.Tags, _ = getNestedTagListFromResourceDataSet(api, d.Get(tagsKey)) @@ -300,10 +305,11 @@ func resourceNetboxDeviceRead(ctx context.Context, d *schema.ResourceData, m int d.Set("site_id", nil) } - cf := getCustomFields(res.GetPayload().CustomFields) - if cf != nil { - d.Set(customFieldsKey, cf) + cf, err := handleCustomFieldRead(device.CustomFields) + if err != nil { + return diag.FromErr(err) } + d.Set(customFieldsKey, cf) d.Set("asset_tag", device.AssetTag) @@ -420,8 +426,11 @@ func resourceNetboxDeviceUpdate(ctx context.Context, d *schema.ResourceData, m i } } - cf, ok := d.GetOk(customFieldsKey) - if ok { + if d.HasChange(customFieldsKey) { + cf, err := handleCustomFieldUpdate(d.GetChange(customFieldsKey)) + if err != nil { + return diag.Errorf("error in resourceNetboxDeviceUpdate[CustomFields]: %v", err) + } data.CustomFields = cf } diff --git a/netbox/resource_netbox_device_test.go b/netbox/resource_netbox_device_test.go index ba1f9357..1e002fa1 100644 --- a/netbox/resource_netbox_device_test.go +++ b/netbox/resource_netbox_device_test.go @@ -246,6 +246,73 @@ resource "netbox_device" "test" { }) } +func TestAccNetboxDevice_CustomFields(t *testing.T) { + testSlug := "device_basic" + testName := testAccGetTestName(testSlug) + testField := strings.ReplaceAll(testAccGetTestName(testSlug), "-", "_") + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckDeviceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccNetboxDeviceFullDependencies(testName) + fmt.Sprintf(` +resource "netbox_custom_field" "text" { + name = "%[1]s_text" + type = "text" + content_types = ["dcim.device"] +} +resource "netbox_custom_field" "boolean" { + name = "%[1]s_boolean" + type = "boolean" + content_types = ["dcim.device"] +} +resource "netbox_device" "test" { + name = "%[2]s" + tenant_id = netbox_tenant.test.id + role_id = netbox_device_role.test.id + device_type_id = netbox_device_type.test.id + site_id = netbox_site.test.id + custom_fields = jsonencode({ + "${netbox_custom_field.text.name}" = "81" + "${netbox_custom_field.boolean.name}" = true + }) +}`, testField, testName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("netbox_device.test", "name", testName), + resource.TestCheckResourceAttrPair("netbox_device.test", "tenant_id", "netbox_tenant.test", "id"), + resource.TestCheckResourceAttrPair("netbox_device.test", "role_id", "netbox_device_role.test", "id"), + resource.TestCheckResourceAttrPair("netbox_device.test", "site_id", "netbox_site.test", "id"), + resource.TestCheckResourceAttr("netbox_device.test", "custom_fields", "{\""+testField+"_boolean\":true,\""+testField+"_text\":\"81\"}"), + ), + }, + { + Config: testAccNetboxDeviceFullDependencies(testName) + fmt.Sprintf(` +resource "netbox_custom_field" "text" { + name = "%[1]s_text" + type = "text" + content_types = ["dcim.device"] +} +resource "netbox_custom_field" "boolean" { + name = "%[1]s_boolean" + type = "boolean" + content_types = ["dcim.device"] +} +resource "netbox_device" "test" { + name = "%[2]s" + tenant_id = netbox_tenant.test.id + role_id = netbox_device_role.test.id + device_type_id = netbox_device_type.test.id + site_id = netbox_site.test.id +}`, testField, testName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("netbox_device.test", "custom_fields", ""), + ), + }, + }, + }) +} + func testAccCheckDeviceDestroy(s *terraform.State) error { // retrieve the connection established in Provider configuration conn := testAccProvider.Meta().(*client.NetBoxAPI)