From e18906888d08fab5c439dab561b0b3b7cd0fedc7 Mon Sep 17 00:00:00 2001 From: Timo Furrer Date: Wed, 16 Mar 2022 16:55:55 +0100 Subject: [PATCH] resource/gitlab_repository_file: Implement explicit retries for refresh errors Closes #940 --- docs/resources/repository_file.md | 22 +++++ .../resource_gitlab_repository_file.go | 96 +++++++++++++++---- 2 files changed, 97 insertions(+), 21 deletions(-) diff --git a/docs/resources/repository_file.md b/docs/resources/repository_file.md index 3f89a1148..746ade8e7 100644 --- a/docs/resources/repository_file.md +++ b/docs/resources/repository_file.md @@ -4,6 +4,11 @@ page_title: "gitlab_repository_file Resource - terraform-provider-gitlab" subcategory: "" description: |- The gitlab_repository_file resource allows to manage the lifecycle of a file within a repository. + -> Timeouts Default timeout for Create, Update and Delete is one minute and can be configured in the timeouts block. + -> Implementation Detail GitLab is unable to handle concurrent calls to the GitLab repository files API for the same project. + Therefore, this resource queues every call to the repository files API no matter of the project, which may slow down the terraform + execution time for some configurations. In addition, retries are performed in case a refresh is required because another application + changed the repository at the same time. Upstream API: GitLab REST API docs https://docs.gitlab.com/ee/api/repository_files.html --- @@ -11,6 +16,13 @@ description: |- The `gitlab_repository_file` resource allows to manage the lifecycle of a file within a repository. +-> **Timeouts** Default timeout for *Create*, *Update* and *Delete* is one minute and can be configured in the `timeouts` block. + +-> **Implementation Detail** GitLab is unable to handle concurrent calls to the GitLab repository files API for the same project. + Therefore, this resource queues every call to the repository files API no matter of the project, which may slow down the terraform + execution time for some configurations. In addition, retries are performed in case a refresh is required because another application + changed the repository at the same time. + **Upstream API**: [GitLab REST API docs](https://docs.gitlab.com/ee/api/repository_files.html) ## Example Usage @@ -54,6 +66,7 @@ resource "gitlab_repository_file" "this" { - `author_name` (String) Name of the commit author. - `id` (String) The ID of this resource. - `start_branch` (String) Name of the branch to start the new commit from. +- `timeouts` (Block, Optional) (see [below for nested schema](#nestedblock--timeouts)) ### Read-Only @@ -66,6 +79,15 @@ resource "gitlab_repository_file" "this" { - `ref` (String) The name of branch, tag or commit. - `size` (Number) The file size. + +### Nested Schema for `timeouts` + +Optional: + +- `create` (String) +- `delete` (String) +- `update` (String) + ## Import Import is supported using the following syntax: diff --git a/internal/provider/resource_gitlab_repository_file.go b/internal/provider/resource_gitlab_repository_file.go index f97f2b8ef..45612a750 100644 --- a/internal/provider/resource_gitlab_repository_file.go +++ b/internal/provider/resource_gitlab_repository_file.go @@ -3,11 +3,15 @@ package provider import ( "context" "encoding/base64" + "errors" "fmt" "log" + "net/http" "strings" + "time" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" gitlab "github.com/xanzy/go-gitlab" ) @@ -31,6 +35,13 @@ var _ = registerResource("gitlab_repository_file", func() *schema.Resource { return &schema.Resource{ Description: `The ` + "`gitlab_repository_file`" + ` resource allows to manage the lifecycle of a file within a repository. +-> **Timeouts** Default timeout for *Create*, *Update* and *Delete* is one minute and can be configured in the ` + "`timeouts`" + ` block. + +-> **Implementation Detail** GitLab is unable to handle concurrent calls to the GitLab repository files API for the same project. + Therefore, this resource queues every call to the repository files API no matter of the project, which may slow down the terraform + execution time for some configurations. In addition, retries are performed in case a refresh is required because another application + changed the repository at the same time. + **Upstream API**: [GitLab REST API docs](https://docs.gitlab.com/ee/api/repository_files.html)`, CreateContext: resourceGitlabRepositoryFileCreate, @@ -40,6 +51,11 @@ var _ = registerResource("gitlab_repository_file", func() *schema.Resource { Importer: &schema.ResourceImporter{ StateContext: schema.ImportStatePassthroughContext, }, + Timeouts: &schema.ResourceTimeout{ + Create: schema.DefaultTimeout(1 * time.Minute), + Update: schema.DefaultTimeout(1 * time.Minute), + Delete: schema.DefaultTimeout(1 * time.Minute), + }, // the schema matches https://docs.gitlab.com/ee/api/repository_files.html#create-new-file-in-repository // However, we don't support the `encoding` parameter as it seems to be broken. @@ -104,12 +120,22 @@ func resourceGitlabRepositoryFileCreate(ctx context.Context, d *schema.ResourceD options.StartBranch = gitlab.String(startBranch.(string)) } - repositoryFile, _, err := client.RepositoryFiles.CreateFile(project, filePath, options, gitlab.WithContext(ctx)) + err := resource.RetryContext(ctx, d.Timeout(schema.TimeoutCreate), func() *resource.RetryError { + repositoryFile, _, err := client.RepositoryFiles.CreateFile(project, filePath, options, gitlab.WithContext(ctx)) + if err != nil { + if isRefreshError(err) { + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + + d.SetId(resourceGitLabRepositoryFileBuildId(project, repositoryFile.Branch, repositoryFile.FilePath)) + return nil + }) if err != nil { return diag.FromErr(err) } - d.SetId(resourceGitLabRepositoryFileBuildId(project, repositoryFile.Branch, repositoryFile.FilePath)) return resourceGitlabRepositoryFileRead(ctx, d, meta) } @@ -163,25 +189,36 @@ func resourceGitlabRepositoryFileUpdate(ctx context.Context, d *schema.ResourceD Ref: gitlab.String(branch), } - existingRepositoryFile, _, err := client.RepositoryFiles.GetFile(project, filePath, readOptions, gitlab.WithContext(ctx)) - if err != nil { - return diag.FromErr(err) - } - - options := &gitlab.UpdateFileOptions{ + updateOptions := &gitlab.UpdateFileOptions{ Branch: gitlab.String(branch), Encoding: gitlab.String(encoding), AuthorEmail: gitlab.String(d.Get("author_email").(string)), AuthorName: gitlab.String(d.Get("author_name").(string)), Content: gitlab.String(d.Get("content").(string)), CommitMessage: gitlab.String(d.Get("commit_message").(string)), - LastCommitID: gitlab.String(existingRepositoryFile.LastCommitID), } if startBranch, ok := d.GetOk("start_branch"); ok { - options.StartBranch = gitlab.String(startBranch.(string)) + updateOptions.StartBranch = gitlab.String(startBranch.(string)) } - _, _, err = client.RepositoryFiles.UpdateFile(project, filePath, options, gitlab.WithContext(ctx)) + err = resource.RetryContext(ctx, d.Timeout(schema.TimeoutUpdate), func() *resource.RetryError { + // NOTE: we also re-read the file to obtain an eventually changed `LastCommitID` for which we needed the refresh + existingRepositoryFile, _, err := client.RepositoryFiles.GetFile(project, filePath, readOptions, gitlab.WithContext(ctx)) + if err != nil { + return resource.NonRetryableError(err) + } + + updateOptions.LastCommitID = gitlab.String(existingRepositoryFile.LastCommitID) + _, _, err = client.RepositoryFiles.UpdateFile(project, filePath, updateOptions, gitlab.WithContext(ctx)) + if err != nil { + if isRefreshError(err) { + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + + return nil + }) if err != nil { return diag.FromErr(err) } @@ -207,23 +244,33 @@ func resourceGitlabRepositoryFileDelete(ctx context.Context, d *schema.ResourceD readOptions := &gitlab.GetFileOptions{ Ref: gitlab.String(branch), } - - existingRepositoryFile, _, err := client.RepositoryFiles.GetFile(project, filePath, readOptions, gitlab.WithContext(ctx)) - if err != nil { - return diag.FromErr(err) - } - - options := &gitlab.DeleteFileOptions{ + deleteOptions := &gitlab.DeleteFileOptions{ Branch: gitlab.String(d.Get("branch").(string)), AuthorEmail: gitlab.String(d.Get("author_email").(string)), AuthorName: gitlab.String(d.Get("author_name").(string)), CommitMessage: gitlab.String(fmt.Sprintf("[DELETE]: %s", d.Get("commit_message").(string))), - LastCommitID: gitlab.String(existingRepositoryFile.LastCommitID), } - resp, err := client.RepositoryFiles.DeleteFile(project, filePath, options) + err = resource.RetryContext(ctx, d.Timeout(schema.TimeoutDelete), func() *resource.RetryError { + // NOTE: we also re-read the file to obtain an eventually changed `LastCommitID` for which we needed the refresh + + existingRepositoryFile, _, err := client.RepositoryFiles.GetFile(project, filePath, readOptions, gitlab.WithContext(ctx)) + if err != nil { + return resource.NonRetryableError(err) + } + + deleteOptions.LastCommitID = gitlab.String(existingRepositoryFile.LastCommitID) + resp, err := client.RepositoryFiles.DeleteFile(project, filePath, deleteOptions) + if err != nil { + if isRefreshError(err) { + return resource.RetryableError(err) + } + return resource.NonRetryableError(fmt.Errorf("%s failed to delete repository file: (%s) %v", d.Id(), resp.Status, err)) + } + return nil + }) if err != nil { - return diag.Errorf("%s failed to delete repository file: (%s) %v", d.Id(), resp.Status, err) + return diag.FromErr(err) } return nil @@ -249,3 +296,10 @@ func resourceGitLabRepositoryFileParseId(id string) (string, string, string, err func resourceGitLabRepositoryFileBuildId(project string, branch string, filePath string) string { return fmt.Sprintf("%s:%s:%s", project, branch, filePath) } + +func isRefreshError(err error) bool { + var httpErr *gitlab.ErrorResponse + return errors.As(err, &httpErr) && + httpErr.Response.StatusCode == http.StatusBadRequest && + strings.Contains(httpErr.Message, "Please refresh and try again") +}