Repository: Mastercard/terraform-provider-restapi

Issue:
Avoid sending empty JSON body when using destroy_method = "PUT"
I am trying to implement some Terraform-like behavior in a resource exposed by the Infoblox API but not available via the first-class provider. In this case, I want to change an attribute of an object to mark it as reserved, then null those properties when the resource is no longer needed. This is because the object I'm working with persistently exists and can't/shouldn't be deleted.

So I followed the example in [terraform-provider-restapi](https://github.com/Mastercard/terraform-provider-restapi)/[examples](https://github.com/Mastercard/terraform-provider-restapi/tree/master/examples)/update_field_with_put_request.tf. I use an external script with a restapi datasource to get the next available object from my API's pool. Then I use the restapi resource to redirect the create action from POST to PUT. The create and change methods work great:

```
locals {
  tenant_private_extattrs = {
    extattrs = {
      "Tenant ID" = {
        value = "sh_${var.tenant_name}"
      }
    }
  }
}
resource "restapi_object" "tenant_private_vlan" {
  path           = "/${data.external.next_private_vlan.result._ref}?_return_as_object=1"
  object_id      = data.external.next_private_vlan.result._ref
  create_method  = "PUT"
  destroy_method = "PUT"
  data           = jsonencode(local.tenant_private_extattrs)
}
```
When it comes time to destroy though, the upstream API complains because the restapi provider tries to send an empty json body and gives the following error message:

```
│ Error: unexpected response code '400': { "Error": "AdmConProtoError: PUT requires indata, none given",
│   "code": "Client.Ibap.Proto",
│   "text": "PUT requires indata, none given"
│ }
│
```
Ideally I'd like to be able to send a json body where each `key = ''` in the `data` parameter on destroy, but I realize that's not especially RESTy. I'm prepared to accept an answer of out-of-scope! My backup plan is to instead use a terraform lifecycle command to just prevent destroy for the resource and do my parameter nulling via a local destroy-time provisioner. 

Discussion:
@DRuggeri: Hi, @broestls - thanks for opening an issue. Sadly, you're correct - the provider is written in a way that expects certain REST semantics to be used. I'm curious, though... for your use case, do you know if the Infoblox server would accept the original record as the body of the DELETE request? Not saying that this would be a definite addition to the provider, but it would be more reasonable to add a flag or behavior that sends the `data` field with DELETE method.@DRuggeri: Doing some spring cleaning on the repo and I think this is a stale issue - feel free to reopen!

Solution patch:
diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml
index 77a4a08..e62853b 100644
--- a/.github/workflows/release.yaml
+++ b/.github/workflows/release.yaml
@@ -14,32 +14,34 @@ on:
   push:
     tags:
       - 'v*'
+permissions:
+  contents: write
 jobs:
   goreleaser:
     runs-on: ubuntu-latest
     steps:
       -
         name: Checkout
-        uses: actions/checkout@v2.4.0
+        uses: actions/checkout@v3
       -
         name: Unshallow
         run: git fetch --prune --unshallow
       -
         name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v3
         with:
-          go-version: 1.17
+          go-version-file: 'go.mod'
+          cache: true
       -
         name: Import GPG key
+        uses: crazy-max/ghaction-import-gpg@v5
         id: import_gpg
-        uses: hashicorp/ghaction-import-gpg@v2.1.0
-        env:
-          # These secrets will need to be configured for the repository:
-          GPG_PRIVATE_KEY: ${{ secrets.GPG_PRIVATE_KEY }}
-          PASSPHRASE: ${{ secrets.PASSPHRASE }}
+        with:
+          gpg_private_key: ${{ secrets.GPG_PRIVATE_KEY }}
+          passphrase: ${{ secrets.PASSPHRASE }}
       -
         name: Run GoReleaser
-        uses: goreleaser/goreleaser-action@v2.8.1
+        uses: goreleaser/goreleaser-action@v3.0.0
         with:
           version: latest
           args: release --parallelism 2 --rm-dist --timeout 1h --release-notes .release_info.md
diff --git a/README.md b/README.md
index bb9a582..7596842 100644
--- a/README.md
+++ b/README.md
@@ -3,6 +3,16 @@
 [![Go Report Card](https://goreportcard.com/badge/github.com/burbon/terraform-provider-restapi)](https://goreportcard.com/report/github.com/burbon/terraform-provider-restapi)
 # Terraform provider for generic REST APIs
 
+## Maintenance Note
+This provider is largely feature-complete and in maintenance mode.
+* It's not dead - it's just slow moving and updates must be done very carefully
+* We encourage community participation with open issues for usage and remain welcoming of pull requests
+* Code updates happen sporadically throughout the year, driven primarily by security fixes and PRs
+* Because of the many API variations and flexibility of this provider, detailed per-API troubleshooting cannot be guaranteed
+
+&nbsp;
+
+## About This Provider
 This terraform provider allows you to interact with APIs that may not yet have a first-class provider available by implementing a "dumb" REST API client.
 
 This provider is essentially created to be a terraform-wrapped `cURL` client. Because of this, you need to know quite a bit about the API you are interacting with as opposed to full-featured terraform providers written with a specific API in mind.
diff --git a/docs/resources/object.md b/docs/resources/object.md
index 7ee727c..1d1fdc4 100644
--- a/docs/resources/object.md
+++ b/docs/resources/object.md
@@ -24,6 +24,7 @@ description: |-
 - **create_path** (String, Optional) Defaults to `path`. The API path that represents where to CREATE (POST) objects of this type on the API server. The string `{id}` will be replaced with the terraform ID of the object if the data contains the `id_attribute`.
 - **debug** (Boolean, Optional) Whether to emit verbose debug output while working with the API object on the server.
 - **destroy_method** (String, Optional) Defaults to `destroy_method` set on the provider. Allows per-resource override of `destroy_method` (see `destroy_method` provider config documentation)
+- **destroy_data** (String, Optional) Valid JSON data that this provider will send to the API server on DESTROY (DELETE) operations. If not set, defaults to the value of `data`.
 - **destroy_path** (String, Optional) Defaults to `path/{id}`. The API path that represents where to DESTROY (DELETE) objects of this type on the API server. The string `{id}` will be replaced with the terraform ID of the object.
 - **force_new** (List of String, Optional) Any changes to these values will result in recreating the resource instead of updating.
 - **id** (String, Optional) The ID of this resource.
@@ -34,6 +35,7 @@ description: |-
 - **read_path** (String, Optional) Defaults to `path/{id}`. The API path that represents where to READ (GET) objects of this type on the API server. The string `{id}` will be replaced with the terraform ID of the object.
 - **read_search** (Map of String, Optional) Custom search for `read_path`. This map will take `search_key`, `search_value`, `results_key` and `query_string` (see datasource config documentation)
 - **update_method** (String, Optional) Defaults to `update_method` set on the provider. Allows per-resource override of `update_method` (see `update_method` provider config documentation)
+- **update_data** (String, Optional) Valid JSON data that this provider will send to the API server on UPDATE (PUT) operations. If not set, defaults to the value of `data`.
 - **update_path** (String, Optional) Defaults to `path/{id}`. The API path that represents where to UPDATE (PUT) objects of this type on the API server. The string `{id}` will be replaced with the terraform ID of the object.
 
 ### Read-only
diff --git a/restapi/api_client.go b/restapi/api_client.go
index e4bfa66..b8d31f5 100644
--- a/restapi/api_client.go
+++ b/restapi/api_client.go
@@ -30,7 +30,9 @@ type apiClientOpt struct {
 	createMethod        string
 	readMethod          string
 	updateMethod        string
+	updateData          string
 	destroyMethod       string
+	destroyData         string
 	copyKeys            []string
 	writeReturnsObject  bool
 	createReturnsObject bool
@@ -61,7 +63,9 @@ type APIClient struct {
 	createMethod        string
 	readMethod          string
 	updateMethod        string
+	updateData          string
 	destroyMethod       string
+	destroyData         string
 	copyKeys            []string
 	writeReturnsObject  bool
 	createReturnsObject bool
@@ -158,7 +162,9 @@ func NewAPIClient(opt *apiClientOpt) (*APIClient, error) {
 		createMethod:        opt.createMethod,
 		readMethod:          opt.readMethod,
 		updateMethod:        opt.updateMethod,
+		updateData:          opt.updateData,
 		destroyMethod:       opt.destroyMethod,
+		destroyData:         opt.destroyData,
 		copyKeys:            opt.copyKeys,
 		writeReturnsObject:  opt.writeReturnsObject,
 		createReturnsObject: opt.createReturnsObject,
diff --git a/restapi/api_object.go b/restapi/api_object.go
index 60e7738..771e821 100644
--- a/restapi/api_object.go
+++ b/restapi/api_object.go
@@ -19,7 +19,9 @@ type apiObjectOpts struct {
 	createMethod  string
 	readMethod    string
 	updateMethod  string
+	updateData    string
 	destroyMethod string
+	destroyData   string
 	deletePath    string
 	searchPath    string
 	queryString   string
@@ -50,11 +52,13 @@ type APIObject struct {
 
 	/* Set internally */
 	data        map[string]interface{} /* Data as managed by the user */
+	updateData  map[string]interface{} /* Update data as managed by the user */
+	destroyData map[string]interface{} /* Destroy data as managed by the user */
 	apiData     map[string]interface{} /* Data as available from the API */
 	apiResponse string
 }
 
-//NewAPIObject makes an APIobject to manage a RESTful object in an API
+// NewAPIObject makes an APIobject to manage a RESTful object in an API
 func NewAPIObject(iClient *APIClient, opts *apiObjectOpts) (*APIObject, error) {
 	if opts.debug {
 		log.Printf("api_object.go: Constructing debug api_object\n")
@@ -78,10 +82,15 @@ func NewAPIObject(iClient *APIClient, opts *apiObjectOpts) (*APIObject, error) {
 	if opts.updateMethod == "" {
 		opts.updateMethod = iClient.updateMethod
 	}
+	if opts.updateData == "" {
+		opts.updateData = iClient.updateData
+	}
 	if opts.destroyMethod == "" {
 		opts.destroyMethod = iClient.destroyMethod
 	}
-
+	if opts.destroyData == "" {
+		opts.destroyData = iClient.destroyData
+	}
 	if opts.postPath == "" {
 		opts.postPath = opts.path
 	}
@@ -115,6 +124,8 @@ func NewAPIObject(iClient *APIClient, opts *apiObjectOpts) (*APIObject, error) {
 		id:            opts.id,
 		idAttribute:   opts.idAttribute,
 		data:          make(map[string]interface{}),
+		updateData:    make(map[string]interface{}),
+		destroyData:   make(map[string]interface{}),
 		apiData:       make(map[string]interface{}),
 	}
 
@@ -146,6 +157,28 @@ func NewAPIObject(iClient *APIClient, opts *apiObjectOpts) (*APIObject, error) {
 		}
 	}
 
+	if opts.updateData != "" {
+		if opts.debug {
+			log.Printf("api_object.go: Parsing update data: '%s'", opts.updateData)
+		}
+
+		err := json.Unmarshal([]byte(opts.updateData), &obj.updateData)
+		if err != nil {
+			return &obj, fmt.Errorf("api_object.go: error parsing update data provided: %v", err.Error())
+		}
+	}
+
+	if opts.destroyData != "" {
+		if opts.debug {
+			log.Printf("api_object.go: Parsing destroy data: '%s'", opts.destroyData)
+		}
+
+		err := json.Unmarshal([]byte(opts.destroyData), &obj.destroyData)
+		if err != nil {
+			return &obj, fmt.Errorf("api_object.go: error parsing destroy data provided: %v", err.Error())
+		}
+	}
+
 	if opts.debug {
 		log.Printf("api_object.go: Constructed object: %s", obj.toString())
 	}
@@ -169,13 +202,18 @@ func (obj *APIObject) toString() string {
 	buffer.WriteString(fmt.Sprintf("debug: %t\n", obj.debug))
 	buffer.WriteString(fmt.Sprintf("read_search: %s\n", spew.Sdump(obj.readSearch)))
 	buffer.WriteString(fmt.Sprintf("data: %s\n", spew.Sdump(obj.data)))
+	buffer.WriteString(fmt.Sprintf("update_data: %s\n", spew.Sdump(obj.updateData)))
+	buffer.WriteString(fmt.Sprintf("destroy_data: %s\n", spew.Sdump(obj.destroyData)))
 	buffer.WriteString(fmt.Sprintf("api_data: %s\n", spew.Sdump(obj.apiData)))
 	return buffer.String()
 }
 
-/* Centralized function to ensure that our data as managed by
-   the api_object is updated with data that has come back from
-   the API */
+/*
+Centralized function to ensure that our data as managed by
+
+	the api_object is updated with data that has come back from
+	the API
+*/
 func (obj *APIObject) updateState(state string) error {
 	if obj.debug {
 		log.Printf("api_object.go: Updating API object state to '%s'\n", state)
@@ -327,6 +365,14 @@ func (obj *APIObject) updateObject() error {
 
 	b, _ := json.Marshal(obj.data)
 
+	updateData, _ := json.Marshal(obj.updateData)
+	if string(updateData) != "{}" {
+		if obj.debug {
+			log.Printf("api_object.go: Using update data '%s'", string(updateData))
+		}
+		b = updateData
+	}
+
 	putPath := obj.putPath
 	if obj.queryString != "" {
 		if obj.debug {
@@ -368,7 +414,16 @@ func (obj *APIObject) deleteObject() error {
 		deletePath = fmt.Sprintf("%s?%s", obj.deletePath, obj.queryString)
 	}
 
-	_, err := obj.apiClient.sendRequest(obj.destroyMethod, strings.Replace(deletePath, "{id}", obj.id, -1), "")
+	b := []byte{}
+	destroyData, _ := json.Marshal(obj.destroyData)
+	if string(destroyData) != "{}" {
+		if obj.debug {
+			log.Printf("api_object.go: Using destroy data '%s'", string(destroyData))
+		}
+		b = destroyData
+	}
+
+	_, err := obj.apiClient.sendRequest(obj.destroyMethod, strings.Replace(deletePath, "{id}", obj.id, -1), string(b))
 	if err != nil {
 		return err
 	}
diff --git a/restapi/resource_api_object.go b/restapi/resource_api_object.go
index 2358e30..252b9bc 100644
--- a/restapi/resource_api_object.go
+++ b/restapi/resource_api_object.go
@@ -84,7 +84,7 @@ func resourceRestAPI() *schema.Resource {
 			},
 			"data": {
 				Type:        schema.TypeString,
-				Description: "Valid JSON data that this provider will manage with the API server.",
+				Description: "Valid JSON object that this provider will manage with the API server.",
 				Required:    true,
 				Sensitive:   isDataSensitive,
 				ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) {
@@ -142,6 +142,40 @@ func resourceRestAPI() *schema.Resource {
 				ForceNew:    true,
 				Description: "Any changes to these values will result in recreating the resource instead of updating.",
 			},
+			"update_data": {
+				Type:        schema.TypeString,
+				Optional:    true,
+				Description: "Valid JSON data to pass during to update requests.",
+				Sensitive:   isDataSensitive,
+				ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) {
+					v := val.(string)
+					if v != "" {
+						data := make(map[string]interface{})
+						err := json.Unmarshal([]byte(v), &data)
+						if err != nil {
+							errs = append(errs, fmt.Errorf("update_data attribute is invalid JSON: %v", err))
+						}
+					}
+					return warns, errs
+				},
+			},
+			"destroy_data": {
+				Type:        schema.TypeString,
+				Optional:    true,
+				Description: "Valid JSON data to pass during to destroy requests.",
+				Sensitive:   isDataSensitive,
+				ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) {
+					v := val.(string)
+					if v != "" {
+						data := make(map[string]interface{})
+						err := json.Unmarshal([]byte(v), &data)
+						if err != nil {
+							errs = append(errs, fmt.Errorf("destroy_data attribute is invalid JSON: %v", err))
+						}
+					}
+					return warns, errs
+				},
+			},
 		}, /* End schema */
 
 	}
@@ -365,9 +399,15 @@ func buildAPIObjectOpts(d *schema.ResourceData) (*apiObjectOpts, error) {
 	if v, ok := d.GetOk("update_method"); ok {
 		opts.updateMethod = v.(string)
 	}
+	if v, ok := d.GetOk("update_data"); ok {
+		opts.updateData = v.(string)
+	}
 	if v, ok := d.GetOk("destroy_method"); ok {
 		opts.destroyMethod = v.(string)
 	}
+	if v, ok := d.GetOk("destroy_data"); ok {
+		opts.destroyData = v.(string)
+	}
 	if v, ok := d.GetOk("destroy_path"); ok {
 		opts.deletePath = v.(string)
 	}



Test patch:
diff --git a/restapi/api_object_test.go b/restapi/api_object_test.go
index 4bb5a53..ede3b0b 100644
--- a/restapi/api_object_test.go
+++ b/restapi/api_object_test.go
@@ -212,6 +212,21 @@ func TestAPIObject(t *testing.T) {
 		}
 	})
 
+	/* Update once more with update_data */
+	t.Run("update_object_with_update_data", func(t *testing.T) {
+		if testDebug {
+			log.Printf("api_object_test.go: Testing update_object() with update_data")
+		}
+		testingObjects["minimal"].updateData["Thing"] = "knife"
+		testingObjects["minimal"].updateObject()
+		if err != nil {
+			t.Fatalf("api_object_test.go: Failed in update_object() test: %s", err)
+		} else if testingObjects["minimal"].apiData["Thing"] != "knife" {
+			t.Fatalf("api_object_test.go: Failed to update 'Thing' field of 'minimal' object. Expected it to be '%s' but it is '%s'\nFull obj: %+v\n",
+				"knife", testingObjects["minimal"].apiData["Thing"], testingObjects["minimal"])
+		}
+	})
+
 	/* Delete one and make sure a 404 follows */
 	t.Run("delete_object", func(t *testing.T) {
 		if testDebug {
@@ -233,9 +248,9 @@ func TestAPIObject(t *testing.T) {
 		err = testingObjects["pet"].createObject()
 		if err != nil {
 			t.Fatalf("api_object_test.go: Failed in create_object() test: %s", err)
-		} else if testingObjects["minimal"].apiData["Thing"] != "spoon" {
+		} else if testingObjects["minimal"].apiData["Thing"] != "knife" {
 			t.Fatalf("api_object_test.go: Failed to update 'Thing' field of 'minimal' object. Expected it to be '%s' but it is '%s'\nFull obj: %+v\n",
-				"spoon", testingObjects["minimal"].apiData["Thing"], testingObjects["minimal"])
+				"knife", testingObjects["minimal"].apiData["Thing"], testingObjects["minimal"])
 		}
 
 		/* verify it's there */
@@ -276,6 +291,19 @@ func TestAPIObject(t *testing.T) {
 		}
 	})
 
+	/* Delete it again with destroy_data and make sure a 404 follows */
+	t.Run("delete_object_with_destroy_data", func(t *testing.T) {
+		if testDebug {
+			log.Printf("api_object_test.go: Testing delete_object() with destroy_data")
+		}
+		testingObjects["pet"].destroyData["destroy"] = "true"
+		testingObjects["pet"].deleteObject()
+		err = testingObjects["pet"].readObject()
+		if err != nil {
+			t.Fatalf("api_object_test.go: 'pet' object deleted, but an error was returned when reading the object (expected the provider to cope with this!\n")
+		}
+	})
+
 	if testDebug {
 		log.Println("api_object_test.go: Stopping HTTP server")
 	}