resource/gitlab_repository_file: Implement explicit retries for refre…

…sh errors Closes #940
gitlabhq · Mar 21, 2022 · e189068 · e189068
1 parent e8bd2c1
commit e189068
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 21 deletions.
diff --git a/docs/resources/repository_file.md b/docs/resources/repository_file.md
@@ -4,13 +4,25 @@ 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
 ---
 
 # gitlab_repository_file (Resource)
 
 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.
 
+<a id="nestedblock--timeouts"></a>
+### 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
@@ -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")
+}