Standardize error handling and refactor old resources (#521)

* Refactor cyral_repository * Refactor cyral_repository_access_gateway * Refactor cyral_repository_access_rules * Refactor cyral_repository_binding * Refactor cyral_sidecar * Refactor cyral_sidecar_credentials * Refactor cyral_repository_conf_analysis * Ignore local tf test files * Improve debugging messages and fix error handling * Improve API error handling * Refactor cyral_repository_conf_auth * Add constant for datamap resource name * Fix broken links for docs * Refactor cyral_repository_network_access_policy * Refactor cyral_sidecar_listener * Improve names in DefaultContextHandler * Refactor cyral_datalabel * Add support for custom error handling in context functions * Fix bugs that caused conf_auth and conf_analysis to not recover from state out-of-sync * Remove dead code * Renamed PostURLFactory back to BaseURLFactory * Address review comments
cyralinc · Apr 5, 2024 · bfc82a5 · bfc82a5
1 parent 86711a2
commit bfc82a5
Show file tree

Hide file tree

Showing 104 changed files with 3,304 additions and 3,108 deletions.
diff --git a/.gitignore b/.gitignore
@@ -2,6 +2,7 @@
 .terraform
 .terraform.lock.hcl
 terraform.tfstate*
+*.tf
 
 # Provider binary
 terraform-provider-cyral*

diff --git a/cyral/client/client.go b/cyral/client/client.go
@@ -71,23 +71,26 @@ func New(clientID, clientSecret, controlPlane string, tlsSkipVerify bool) (*Clie
 // DoRequest calls the httpMethod informed and delivers the resourceData as a payload,
 // filling the response parameter (if not nil) with the response body.
 func (c *Client) DoRequest(ctx context.Context, url, httpMethod string, resourceData interface{}) ([]byte, error) {
-	tflog.Debug(ctx, "Init DoRequest")
-	tflog.Debug(ctx, fmt.Sprintf("Resource info: %#v", resourceData))
-	tflog.Debug(ctx, fmt.Sprintf("%s URL: %s", httpMethod, url))
+	tflog.Debug(ctx, "=> Init DoRequest")
+	tflog.Debug(ctx, fmt.Sprintf("==> Resource info: %#v", resourceData))
+	tflog.Debug(ctx, fmt.Sprintf("==> %s URL: %s", httpMethod, url))
 	var req *http.Request
 	var err error
 	if resourceData != nil {
 		payloadBytes, err := json.Marshal(resourceData)
 		if err != nil {
+			tflog.Debug(ctx, "=> End DoRequest - Error")
 			return nil, fmt.Errorf("failed to encode payload: %v", err)
 		}
 		payload := string(payloadBytes)
 		tflog.Debug(ctx, fmt.Sprintf("%s payload: %s", httpMethod, payload))
 		if req, err = http.NewRequest(httpMethod, url, strings.NewReader(payload)); err != nil {
+			tflog.Debug(ctx, "=> End DoRequest - Error")
 			return nil, fmt.Errorf("unable to create request; err: %v", err)
 		}
 	} else {
 		if req, err = http.NewRequest(httpMethod, url, nil); err != nil {
+			tflog.Debug(ctx, "=> End DoRequest - Error")
 			return nil, fmt.Errorf("unable to create request; err: %v", err)
 		}
 	}
@@ -96,31 +99,35 @@ func (c *Client) DoRequest(ctx context.Context, url, httpMethod string, resource
 	token := &oauth2.Token{}
 	if c.TokenSource != nil {
 		if token, err = c.TokenSource.Token(); err != nil {
+			tflog.Debug(ctx, "=> End DoRequest - Error")
 			return nil, fmt.Errorf("unable to retrieve authorization token. error: %v", err)
 		} else {
-			tflog.Debug(ctx, fmt.Sprintf("Token Type: %s", token.Type()))
-			tflog.Debug(ctx, fmt.Sprintf("Access Token: %s", redactContent(token.AccessToken)))
-			tflog.Debug(ctx, fmt.Sprintf("Token Expiry: %s", token.Expiry))
+			tflog.Debug(ctx, fmt.Sprintf("==> Token Type: %s", token.Type()))
+			tflog.Debug(ctx, fmt.Sprintf("==> Access Token: %s", redactContent(token.AccessToken)))
+			tflog.Debug(ctx, fmt.Sprintf("==> Token Expiry: %s", token.Expiry))
 			req.Header.Add("Authorization", fmt.Sprintf("%s %s", token.Type(), token.AccessToken))
 		}
 	}
 
-	tflog.Debug(ctx, fmt.Sprintf("Executing %s", httpMethod))
+	tflog.Debug(ctx, fmt.Sprintf("==> Executing %s", httpMethod))
 	res, err := c.client.Do(req)
 	if err != nil {
+		tflog.Debug(ctx, "=> End DoRequest - Error")
 		return nil, fmt.Errorf("unable to execute request. Check the control plane address; err: %v", err)
 	}
 
 	defer res.Body.Close()
 	if res.StatusCode == http.StatusConflict ||
 		(httpMethod == http.MethodPost && strings.Contains(strings.ToLower(res.Status), "already exists")) {
+		tflog.Debug(ctx, "=> End DoRequest - Error")
 		return nil, NewHttpError(
 			fmt.Sprintf("resource possibly exists in the control plane. Response status: %s", res.Status),
 			res.StatusCode)
 	}
 
 	body, err := ioutil.ReadAll(res.Body)
 	if err != nil {
+		tflog.Debug(ctx, "=> End DoRequest - Error")
 		return nil, NewHttpError(
 			fmt.Sprintf("unable to read data from request body; err: %v", err),
 			res.StatusCode)
@@ -129,18 +136,19 @@ func (c *Client) DoRequest(ctx context.Context, url, httpMethod string, resource
 	// Redact token before logging the request
 	req.Header.Set("Authorization", fmt.Sprintf("%s %s", token.Type(), redactContent(token.AccessToken)))
 
-	tflog.Debug(ctx, fmt.Sprintf("Request: %#v", req))
-	tflog.Debug(ctx, fmt.Sprintf("Response status code: %d", res.StatusCode))
-	tflog.Debug(ctx, fmt.Sprintf("Response body: %s", string(body)))
+	tflog.Debug(ctx, fmt.Sprintf("==> Request: %#v", req))
+	tflog.Debug(ctx, fmt.Sprintf("==> Response status code: %d", res.StatusCode))
+	tflog.Debug(ctx, fmt.Sprintf("==> Response body: %s", string(body)))
 
 	if !(res.StatusCode >= 200 && res.StatusCode < 300) {
+		tflog.Debug(ctx, "=> End DoRequest - Error")
 		return nil, NewHttpError(
 			fmt.Sprintf("error executing %s request; status code: %d; body: %q",
 				httpMethod, res.StatusCode, body),
 			res.StatusCode)
 	}
 
-	tflog.Debug(ctx, "End DoRequest")
+	tflog.Debug(ctx, "=> End DoRequest - Success")
 
 	return body, nil
 }

diff --git a/cyral/core/README.md b/cyral/core/README.md
@@ -45,7 +45,7 @@ type NewFeature struct {
 	Description string `json:"description,omitempty"`
 }
 
-func (r *NewFeature) WriteToSchema(d *schema.ResourceData) error {
+func (r NewFeature) WriteToSchema(d *schema.ResourceData) error {
 	if err := d.Set("description", r.Description); err != nil {
 		return fmt.Errorf("error setting 'description' field: %w", err)
 	}
@@ -65,9 +65,7 @@ func (r *NewFeature) ReadFromSchema(d *schema.ResourceData) error {
 
 ### datasource.go
 
-Even though the `GET` url for this new feature is `https://<CP>/v1/NewFeature/<ID>`,
-the `BaseURLFactory` provided does not provide the `ID` as it will be automatically
-added by the default read handler returned in `contextHandler.ReadContext()`.
+Use the `GetPutDeleteURLFactory` to provide the URL factory to read the data source from the API.
 
 ```go
 // datasource.go
@@ -76,10 +74,9 @@ package newfeature
 var dsContextHandler = core.DefaultContextHandler{
 	ResourceName:        dataSourceName,
 	ResourceType:        resourcetype.DataSource,
-	SchemaReaderFactory: func() core.SchemaReader { return &NewFeature{} },
-	SchemaWriterFactory: func(_ *schema.ResourceData) core.SchemaWriter { return &NewFeature{} },
-	BaseURLFactory: func(d *schema.ResourceData, c *client.Client) string {
-		return fmt.Sprintf("https://%s/v1/NewFeature", c.ControlPlane)
+	SchemaWriterFactoryGetMethod: func(_ *schema.ResourceData) core.SchemaWriter { return &NewFeature{} },
+	GetPutDeleteURLFactory: func(d *schema.ResourceData, c *client.Client) string {
+		return fmt.Sprintf("https://%s/v1/NewFeature/%s", c.ControlPlane, d.Get("my_id_field").(string))
 	},
 }
 
@@ -113,7 +110,7 @@ var resourceContextHandler = core.DefaultContextHandler{
 	ResourceName:        resourceName,
 	ResourceType:        resourcetype.Resource,
 	SchemaReaderFactory: func() core.SchemaReader { return &NewFeature{} },
-	SchemaWriterFactory: func(_ *schema.ResourceData) core.SchemaWriter { return &NewFeature{} },
+	SchemaWriterFactoryGetMethod: func(_ *schema.ResourceData) core.SchemaWriter { return &NewFeature{} },
 	BaseURLFactory: func(d *schema.ResourceData, c *client.Client) string {
 		return fmt.Sprintf("https://%s/v1/NewFeature", c.ControlPlane)
 	},

diff --git a/cyral/core/default_context_handler.go b/cyral/core/default_context_handler.go
@@ -12,13 +12,20 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
-// Implementation of a default context handler that can be used by all resources
-// which API follows these principles:
-//  1. The resource is backed by an ID coming from the API.
-//  2. The creation is a POST that returns a JSON with an `id` field, meaning
-//     it can be used with the `IDBasedResponse` struct.
-//  3. The endpoint to perform GET, PUT and DELETE calls are composed by the
-//     POST endpoint plus the ID specification like the following:
+// Implementation of a default context handler that can be used by all resources.
+//
+//  1. `SchemaWriterFactoryGetMethod“ must be provided.
+//  2. In case `SchemaWriterFactoryPostMethod“ is not provided,
+//     it will assume that a call to POST returns a JSON with
+//     an `id` field, meaning it will use the
+//     `IDBasedResponse` struct in such cases.
+//  3. `BaseURLFactory` must be provided for resources. It will be used to
+//     create the POST endpoint and others in case `GetPutDeleteURLFactory`
+//     is not provided.
+//  4. `GetPutDeleteURLFactory` must be provided for data sources.
+//  5. If `GetPutDeleteURLFactory` is NOT provided (data sources or resources),
+//     the endpoint to perform GET, PUT and DELETE calls are composed by the
+//     `BaseURLFactory` endpoint plus the ID specification as follows:
 //     - POST:    https://<CP>/<apiVersion>/<featureName>
 //     - GET:     https://<CP>/<apiVersion>/<featureName>/<id>
 //     - PUT:     https://<CP>/<apiVersion>/<featureName>/<id>
@@ -27,76 +34,104 @@ type DefaultContextHandler struct {
 	ResourceName        string
 	ResourceType        rt.ResourceType
 	SchemaReaderFactory SchemaReaderFactoryFunc
-	SchemaWriterFactory SchemaWriterFactoryFunc
-	BaseURLFactory      URLFactoryFunc
+	// SchemaWriterFactoryGetMethod defines how the schema will be
+	// written in GET operations.
+	SchemaWriterFactoryGetMethod SchemaWriterFactoryFunc
+	// SchemaWriterFactoryPostMethod defines how the schema will be
+	// written in POST operations.
+	SchemaWriterFactoryPostMethod SchemaWriterFactoryFunc
+	// BaseURLFactory provides the URL used for POSTs and that
+	// will also be used to compose the ID URL for GET, PUT and
+	// DELETE in case `GetPutDeleteURLFactory` is not provided.
+	BaseURLFactory         URLFactoryFunc
+	GetPutDeleteURLFactory URLFactoryFunc
 }
 
-func defaultSchemaWriterFactory(d *schema.ResourceData) SchemaWriter {
+func DefaultSchemaWriterFactory(d *schema.ResourceData) SchemaWriter {
 	return &IDBasedResponse{}
 }
 
-func defaultOperationHandler(
-	resourceName string,
-	resourceType rt.ResourceType,
+func (dch DefaultContextHandler) defaultOperationHandler(
 	operationType ot.OperationType,
-	baseURLFactory URLFactoryFunc,
 	httpMethod string,
 	schemaReaderFactory SchemaReaderFactoryFunc,
 	schemaWriterFactory SchemaWriterFactoryFunc,
+	requestErrorHandler RequestErrorHandler,
 ) ResourceOperationConfig {
 	// POST = https://<CP>/<apiVersion>/<feature>
 	// GET, PUT and DELETE = https://<CP>/<apiVersion>/<feature>/<id>
 	endpoint := func(d *schema.ResourceData, c *client.Client) string {
-		url := baseURLFactory(d, c)
-		if d.Id() != "" {
-			url = fmt.Sprintf("%s/%s", baseURLFactory(d, c), d.Id())
+		var url string
+		if httpMethod == http.MethodPost {
+			url = dch.BaseURLFactory(d, c)
+		} else if dch.GetPutDeleteURLFactory != nil {
+			url = dch.GetPutDeleteURLFactory(d, c)
+		} else {
+			url = fmt.Sprintf("%s/%s", dch.BaseURLFactory(d, c), d.Id())
 		}
 		tflog.Debug(context.Background(), fmt.Sprintf("Returning base URL for %s '%s' operation '%s' and httpMethod %s: %s",
-			resourceType, resourceName, operationType, httpMethod, url))
+			dch.ResourceType, dch.ResourceName, operationType, httpMethod, url))
 		return url
 	}
 
-	var errorHandler RequestErrorHandler
-	if httpMethod == http.MethodGet {
-		errorHandler = &ReadIgnoreHttpNotFound{ResName: resourceName}
-	} else if httpMethod == http.MethodDelete {
-		errorHandler = &DeleteIgnoreHttpNotFound{ResName: resourceName}
-	}
 	result := ResourceOperationConfig{
-		ResourceName:        resourceName,
+		ResourceName:        dch.ResourceName,
 		Type:                operationType,
-		ResourceType:        resourceType,
+		ResourceType:        dch.ResourceType,
 		HttpMethod:          httpMethod,
 		URLFactory:          endpoint,
 		SchemaReaderFactory: schemaReaderFactory,
 		SchemaWriterFactory: schemaWriterFactory,
-		RequestErrorHandler: errorHandler,
+		RequestErrorHandler: requestErrorHandler,
 	}
 
 	return result
 }
 
 func (dch DefaultContextHandler) CreateContext() schema.CreateContextFunc {
+	return dch.CreateContextCustomErrorHandling(&IgnoreHttpNotFound{ResName: dch.ResourceName}, nil)
+}
+
+func (dch DefaultContextHandler) CreateContextCustomErrorHandling(getErrorHandler RequestErrorHandler,
+	postErrorHandler RequestErrorHandler) schema.CreateContextFunc {
+	// By default, assumes that if no SchemaWriterFactoryPostMethod is provided,
+	// the POST api will return an ID
+	schemaWriterPost := DefaultSchemaWriterFactory
+	if dch.SchemaWriterFactoryPostMethod != nil {
+		schemaWriterPost = dch.SchemaWriterFactoryPostMethod
+	}
 	return CreateResource(
-		defaultOperationHandler(dch.ResourceName, dch.ResourceType, ot.Create, dch.BaseURLFactory, http.MethodPost, dch.SchemaReaderFactory, nil),
-		defaultOperationHandler(dch.ResourceName, dch.ResourceType, ot.Create, dch.BaseURLFactory, http.MethodGet, nil, dch.SchemaWriterFactory),
+		dch.defaultOperationHandler(ot.Create, http.MethodPost, dch.SchemaReaderFactory, schemaWriterPost, postErrorHandler),
+		dch.defaultOperationHandler(ot.Create, http.MethodGet, nil, dch.SchemaWriterFactoryGetMethod, getErrorHandler),
 	)
 }
 
 func (dch DefaultContextHandler) ReadContext() schema.ReadContextFunc {
-	return ReadResource(dch.ReadResourceOperationConfig())
+	return dch.ReadContextCustomErrorHandling(&IgnoreHttpNotFound{ResName: dch.ResourceName})
 }
-func (dch DefaultContextHandler) ReadResourceOperationConfig() ResourceOperationConfig {
-	return defaultOperationHandler(dch.ResourceName, dch.ResourceType, ot.Read, dch.BaseURLFactory, http.MethodGet, nil, dch.SchemaWriterFactory)
+
+func (dch DefaultContextHandler) ReadContextCustomErrorHandling(getErrorHandler RequestErrorHandler) schema.ReadContextFunc {
+	return ReadResource(
+		dch.defaultOperationHandler(ot.Read, http.MethodGet, nil, dch.SchemaWriterFactoryGetMethod, getErrorHandler),
+	)
 }
 
 func (dch DefaultContextHandler) UpdateContext() schema.UpdateContextFunc {
+	return dch.UpdateContextCustomErrorHandling(&IgnoreHttpNotFound{ResName: dch.ResourceName}, nil)
+}
+
+func (dch DefaultContextHandler) UpdateContextCustomErrorHandling(getErrorHandler RequestErrorHandler,
+	putErrorHandler RequestErrorHandler) schema.UpdateContextFunc {
 	return UpdateResource(
-		defaultOperationHandler(dch.ResourceName, dch.ResourceType, ot.Update, dch.BaseURLFactory, http.MethodPut, dch.SchemaReaderFactory, nil),
-		defaultOperationHandler(dch.ResourceName, dch.ResourceType, ot.Update, dch.BaseURLFactory, http.MethodGet, nil, dch.SchemaWriterFactory))
+		dch.defaultOperationHandler(ot.Update, http.MethodPut, dch.SchemaReaderFactory, nil, putErrorHandler),
+		dch.defaultOperationHandler(ot.Update, http.MethodGet, nil, dch.SchemaWriterFactoryGetMethod, getErrorHandler),
+	)
 }
 
 func (dch DefaultContextHandler) DeleteContext() schema.DeleteContextFunc {
-	return DeleteResource(defaultOperationHandler(
-		dch.ResourceName, dch.ResourceType, ot.Delete, dch.BaseURLFactory, http.MethodDelete, nil, nil))
+	return dch.DeleteContextCustomErrorHandling(&IgnoreHttpNotFound{ResName: dch.ResourceName})
+}
+
+func (dch DefaultContextHandler) DeleteContextCustomErrorHandling(deleteErrorHandler RequestErrorHandler) schema.DeleteContextFunc {
+	return DeleteResource(dch.defaultOperationHandler(ot.Delete, http.MethodDelete, nil, nil, deleteErrorHandler))
 }
diff --git a/cyral/core/error_handlers.go b/cyral/core/error_handlers.go
@@ -4,46 +4,69 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"regexp"
 
 	"github.com/hashicorp/terraform-plugin-log/tflog"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 
 	"github.com/cyralinc/terraform-provider-cyral/cyral/client"
+	"github.com/cyralinc/terraform-provider-cyral/cyral/core/types/operationtype"
 )
 
-type DeleteIgnoreHttpNotFound struct {
-	ResName string
+type IgnoreNotFoundByMessage struct {
+	ResName        string
+	MessageMatches string
+	OperationType  operationtype.OperationType
 }
 
-func (h *DeleteIgnoreHttpNotFound) HandleError(
+func (h *IgnoreNotFoundByMessage) HandleError(
 	ctx context.Context,
-	_ *schema.ResourceData,
+	r *schema.ResourceData,
 	_ *client.Client,
 	err error,
 ) error {
-	httpError, ok := err.(*client.HttpError)
-	if !ok || httpError.StatusCode != http.StatusNotFound {
-		return err
+	tflog.Debug(ctx, "==> Init HandleError core.IgnoreNotFoundByMessage")
+
+	matched, regexpError := regexp.MatchString(
+		h.MessageMatches,
+		err.Error(),
+	)
+
+	if regexpError != nil {
+		return fmt.Errorf("regex failed to compile trying to match '%s' in '%w'. Error: %w",
+			h.MessageMatches, err, regexpError)
 	}
-	tflog.Debug(ctx, fmt.Sprintf("%s not found. Skipping deletion.", h.ResName))
-	return nil
+
+	if matched {
+		tflog.Debug(ctx, fmt.Sprintf("===> %s not found. Skipping %s operation. Error: %v",
+			h.ResName, h.OperationType, err))
+		r.SetId("")
+		tflog.Debug(ctx, "==> End HandleError core.IgnoreNotFoundByMessage - Success")
+		return nil
+	}
+
+	tflog.Debug(ctx, "==> End HandleError core.IgnoreNotFoundByMessage - No match found, thus returning the original error")
+	return err
 }
 
-type ReadIgnoreHttpNotFound struct {
+type IgnoreHttpNotFound struct {
 	ResName string
 }
 
-func (h *ReadIgnoreHttpNotFound) HandleError(
+func (h *IgnoreHttpNotFound) HandleError(
 	ctx context.Context,
 	r *schema.ResourceData,
 	_ *client.Client,
 	err error,
 ) error {
+	tflog.Debug(ctx, "==> Init HandleError core.IgnoreHttpNotFound")
 	httpError, ok := err.(*client.HttpError)
 	if !ok || httpError.StatusCode != http.StatusNotFound {
+		tflog.Debug(ctx, "==> End HandleError core.IgnoreHttpNotFound - Did not find a 404, thus returning the original error")
 		return err
 	}
 	r.SetId("")
-	tflog.Debug(ctx, fmt.Sprintf("%s not found. Marking resource for recreation.", h.ResName))
+	tflog.Debug(ctx, fmt.Sprintf(
+		"==> End HandleError core.IgnoreHttpNotFound - %s not found. Marking resource for recreation or deletion.", h.ResName))
 	return nil
 }