From 0a39b5a9ce99e2157cb73eb745ba7c980e13b6b8 Mon Sep 17 00:00:00 2001 From: Wilson de Carvalho <796900+wcmjunior@users.noreply.github.com> Date: Wed, 3 Apr 2024 22:50:59 -0700 Subject: [PATCH] Fix bugs that caused conf_auth and conf_analysis to not recover from state out-of-sync --- .../repository/confanalysis/resource.go | 94 ++++++++++++++----- .../internal/repository/confauth/resource.go | 88 ++++++++--------- 2 files changed, 114 insertions(+), 68 deletions(-) diff --git a/cyral/internal/repository/confanalysis/resource.go b/cyral/internal/repository/confanalysis/resource.go index 8059cf16..fd437a60 100644 --- a/cyral/internal/repository/confanalysis/resource.go +++ b/cyral/internal/repository/confanalysis/resource.go @@ -9,9 +9,15 @@ import ( "github.com/cyralinc/terraform-provider-cyral/cyral/core" "github.com/cyralinc/terraform-provider-cyral/cyral/core/types/operationtype" "github.com/cyralinc/terraform-provider-cyral/cyral/core/types/resourcetype" + "github.com/hashicorp/terraform-plugin-log/tflog" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) +// TODO This resource is more complex than it should be due to the fact that a call to +// repo creation automatically creates the conf/auth and also the conf/analysis configurations. +// Our API should be refactored so these operations should happen separately. + var urlFactory = func(d *schema.ResourceData, c *client.Client) string { return fmt.Sprintf("https://%s/v1/repos/%s/conf/analysis", c.ControlPlane, @@ -28,6 +34,8 @@ var resourceContextHandler = core.DefaultContextHandler{ GetPutDeleteURLFactory: urlFactory, } +var requestErrorHandler = &core.IgnoreNotFoundByMessage{MessageMatches: "Cannot find config data for repo"} + func resourceSchema() *schema.Resource { return &schema.Resource{ Description: "Manages Repository Analysis Configuration. This resource allows configuring " + @@ -35,27 +43,22 @@ func resourceSchema() *schema.Resource { "[Alerts](https://cyral.com/docs/data-repos/config/#alerts) and " + "[Policy Enforcement](https://cyral.com/docs/data-repos/config/#policy-enforcement) " + "settings for Data Repositories.", - CreateContext: core.CreateResource( - core.ResourceOperationConfig{ - ResourceName: resourceName, - Type: operationtype.Create, - HttpMethod: http.MethodPut, - URLFactory: urlFactory, - SchemaReaderFactory: func() core.SchemaReader { return &UserConfig{} }, - SchemaWriterFactory: func(_ *schema.ResourceData) core.SchemaWriter { return &RepositoryConfAnalysisData{} }, - }, - core.ResourceOperationConfig{ - ResourceName: resourceName, - Type: operationtype.Read, - HttpMethod: http.MethodGet, - URLFactory: urlFactory, - SchemaWriterFactory: func(_ *schema.ResourceData) core.SchemaWriter { return &RepositoryConfAnalysisData{} }, - RequestErrorHandler: &core.IgnoreHttpNotFound{}, - }, - ), - ReadContext: resourceContextHandler.ReadContext(), - UpdateContext: resourceContextHandler.UpdateContext(), - DeleteContext: resourceContextHandler.DeleteContext(), + CreateContext: resourceRepositoryConfAnalysisCreate, + ReadContext: resourceContextHandler.ReadContextCustomErrorHandling(&core.IgnoreNotFoundByMessage{ + ResName: resourceName, + MessageMatches: "Cannot find config data for repo", + OperationType: operationtype.Read, + }), + UpdateContext: resourceContextHandler.UpdateContextCustomErrorHandling(&core.IgnoreNotFoundByMessage{ + ResName: resourceName, + MessageMatches: "Cannot find config data for repo", + OperationType: operationtype.Update, + }, nil), + DeleteContext: resourceContextHandler.DeleteContextCustomErrorHandling(&core.IgnoreNotFoundByMessage{ + ResName: resourceName, + MessageMatches: "Cannot find config data for repo", + OperationType: operationtype.Delete, + }), SchemaVersion: 1, StateUpgraders: []schema.StateUpgrader{ @@ -192,3 +195,52 @@ func UpgradeRepositoryConfAnalysisV0( rawState["id"] = rawState["repository_id"] return rawState, nil } + +func resourceRepositoryConfAnalysisCreate( + ctx context.Context, + d *schema.ResourceData, + m interface{}, +) diag.Diagnostics { + tflog.Debug(ctx, "Init resourceRepositoryConfAnalysisCreate") + c := m.(*client.Client) + httpMethod := http.MethodPost + if confAnalysisAlreadyExists(ctx, c, d) { + httpMethod = http.MethodPut + } + tflog.Debug(ctx, "End resourceRepositoryConfAnalysisCreate") + return core.CreateResource( + core.ResourceOperationConfig{ + ResourceName: resourceName, + Type: operationtype.Create, + HttpMethod: httpMethod, + URLFactory: urlFactory, + SchemaReaderFactory: func() core.SchemaReader { return &UserConfig{} }, + SchemaWriterFactory: func(_ *schema.ResourceData) core.SchemaWriter { return &RepositoryConfAnalysisData{} }, + }, + core.ResourceOperationConfig{ + ResourceName: resourceName, + ResourceType: resourcetype.Resource, + Type: operationtype.Read, + HttpMethod: http.MethodGet, + URLFactory: urlFactory, + SchemaWriterFactory: func(_ *schema.ResourceData) core.SchemaWriter { return &RepositoryConfAnalysisData{} }, + RequestErrorHandler: &core.IgnoreNotFoundByMessage{ + ResName: resourceName, + MessageMatches: "Cannot find config data for repo", + OperationType: operationtype.Read, + }, + }, + )(ctx, d, m) +} + +func confAnalysisAlreadyExists(ctx context.Context, c *client.Client, d *schema.ResourceData) bool { + _, err := c.DoRequest(ctx, urlFactory(d, c), http.MethodGet, nil) + // See TODO on the top of this file + if err != nil { + + tflog.Debug(ctx, fmt.Sprintf("Unable to read Conf Analysis resource for repository %s: %v", + d.Get("repository_id").(string), err)) + return false + } + return true +} diff --git a/cyral/internal/repository/confauth/resource.go b/cyral/internal/repository/confauth/resource.go index 43b646b4..17a51d74 100644 --- a/cyral/internal/repository/confauth/resource.go +++ b/cyral/internal/repository/confauth/resource.go @@ -17,8 +17,9 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) -// TODO Fix the API status codes and simplify this code. We could easily use the default handlers -// instead of this complex set of operations. +// TODO This resource is more complex than it should be due to the fact that a call to +// repo creation automatically creates the conf/auth and also the conf/analysis configurations. +// Our API should be refactored so these operations should happen separately. var urlFactory = func(d *schema.ResourceData, c *client.Client) string { return fmt.Sprintf("https://%s/v1/repos/%s/conf/auth", @@ -27,18 +28,13 @@ var urlFactory = func(d *schema.ResourceData, c *client.Client) string { ) } -var readConfig = core.ResourceOperationConfig{ - ResourceName: resourceName, - ResourceType: resourcetype.Resource, - Type: operationtype.Read, - HttpMethod: http.MethodGet, - URLFactory: urlFactory, - SchemaWriterFactory: func(_ *schema.ResourceData) core.SchemaWriter { return &ReadRepositoryConfAuthResponse{} }, - RequestErrorHandler: &core.IgnoreNotFoundByMessage{ - ResName: resourceName, - MessageMatches: "Failed to read repo", - OperationType: operationtype.Read, - }, +var resourceContextHandler = core.DefaultContextHandler{ + ResourceName: resourceName, + ResourceType: resourcetype.Resource, + SchemaReaderFactory: func() core.SchemaReader { return &RepositoryConfAuthData{} }, + SchemaWriterFactoryGetMethod: func(_ *schema.ResourceData) core.SchemaWriter { return &ReadRepositoryConfAuthResponse{} }, + PostURLFactory: urlFactory, + GetPutDeleteURLFactory: urlFactory, } func resourceSchema() *schema.Resource { @@ -48,32 +44,21 @@ func resourceSchema() *schema.Resource { "and [Advanced settings](https://cyral.com/docs/manage-repositories/repo-advanced-settings) " + "(Logs, Alerts, Analysis and Enforcement) configurations for Data Repositories.", CreateContext: resourceRepositoryConfAuthCreate, - ReadContext: core.ReadResource(readConfig), - UpdateContext: core.UpdateResource( - core.ResourceOperationConfig{ - ResourceName: resourceName, - ResourceType: resourcetype.Resource, - Type: operationtype.Update, - HttpMethod: http.MethodPut, - URLFactory: urlFactory, - SchemaReaderFactory: func() core.SchemaReader { return &RepositoryConfAuthData{} }, - }, - readConfig, - ), - DeleteContext: core.DeleteResource( - core.ResourceOperationConfig{ - ResourceName: resourceName, - ResourceType: resourcetype.Resource, - Type: operationtype.Delete, - HttpMethod: http.MethodDelete, - URLFactory: urlFactory, - RequestErrorHandler: &core.IgnoreNotFoundByMessage{ - ResName: resourceName, - MessageMatches: "Failed to read repo", - OperationType: operationtype.Read, - }, - }, - ), + ReadContext: resourceContextHandler.ReadContextCustomErrorHandling(&core.IgnoreNotFoundByMessage{ + ResName: resourceName, + MessageMatches: "Failed to read repo", + OperationType: operationtype.Read, + }), + UpdateContext: resourceContextHandler.UpdateContextCustomErrorHandling(&core.IgnoreNotFoundByMessage{ + ResName: resourceName, + MessageMatches: "Failed to read repo", + OperationType: operationtype.Update, + }, nil), + DeleteContext: resourceContextHandler.DeleteContextCustomErrorHandling(&core.IgnoreNotFoundByMessage{ + ResName: resourceName, + MessageMatches: "Failed to read repo", + OperationType: operationtype.Delete, + }), SchemaVersion: 1, StateUpgraders: []schema.StateUpgrader{ @@ -192,17 +177,26 @@ func resourceRepositoryConfAuthCreate( URLFactory: urlFactory, SchemaReaderFactory: func() core.SchemaReader { return &RepositoryConfAuthData{} }, SchemaWriterFactory: func(_ *schema.ResourceData) core.SchemaWriter { return &CreateRepositoryConfAuthResponse{} }, - }, readConfig)(ctx, d, m) + }, + core.ResourceOperationConfig{ + ResourceName: resourceName, + ResourceType: resourcetype.Resource, + Type: operationtype.Read, + HttpMethod: http.MethodGet, + URLFactory: urlFactory, + SchemaWriterFactory: func(_ *schema.ResourceData) core.SchemaWriter { return &ReadRepositoryConfAuthResponse{} }, + RequestErrorHandler: &core.IgnoreNotFoundByMessage{ + ResName: resourceName, + MessageMatches: "Failed to read repo", + OperationType: operationtype.Read, + }, + }, + )(ctx, d, m) } func confAuthAlreadyExists(ctx context.Context, c *client.Client, d *schema.ResourceData) bool { _, err := c.DoRequest(ctx, urlFactory(d, c), http.MethodGet, nil) - // TODO: Fix this API. - - // The GET /v1/repos/{repoID}/conf/auth API currently returns 500 status code for every type - // of error, so its not possible to distinguish if the error is due to a 404 Not Found or not. - // Once the status code returned by this API is fixed we should return false only if it returns - // a 404 Not Found, otherwise, if a different error occurs, this function should return an error. + // See TODO on the top of this file if err != nil { tflog.Debug(ctx, fmt.Sprintf("Unable to read Conf Auth resource for repository %s: %v",