resource/gitlab_repository_file: Implement syncing when accessing API

Closes #940

Author: timofurrer

docs/resources/repository_file.md

@@ -4,16 +4,13 @@ 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.
-  ~> Limitations: The GitLab Repository Files API https://docs.gitlab.com/ee/api/repository_files.html can only create, update or delete a single file at the time.  The API will also fail with a 400 https://docs.gitlab.com/ee/api/repository_files.html#update-existing-file-in-repository response status code if the underlying repository is changed while the API tries to make changes.  Therefore, it's recommended to make sure that you execute it with -parallelism=1 https://www.terraform.io/docs/cli/commands/apply.html#parallelism-n and that no other entity than the terraform at hand makes changes to the underlying repository while it's executing.
   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.
 
-~> **Limitations**: The [GitLab Repository Files API](https://docs.gitlab.com/ee/api/repository_files.html) can only create, update or delete a single file at the time.  The API will also [fail with a 400](https://docs.gitlab.com/ee/api/repository_files.html#update-existing-file-in-repository) response status code if the underlying repository is changed while the API tries to make changes.  Therefore, it's recommended to make sure that you execute it with [-parallelism=1](https://www.terraform.io/docs/cli/commands/apply.html#parallelism-n) and that no other entity than the terraform at hand makes changes to the underlying repository while it's executing.
-
 **Upstream API**: [GitLab REST API docs](https://docs.gitlab.com/ee/api/repository_files.html)
 
 ## Example Usage

internal/provider/resource_gitlab_repository_file.go

@@ -14,12 +14,23 @@ import (
 
 const encoding = "base64"
 
+// NOTE: this lock is a bit of a hack to prevent parallel calls to the GitLab Repository Files API.
+//       If it is called concurrently, the API will return a 400 error along the lines of:
+//       ```
+//       (400 Bad Request) DELETE https://gitlab.com/api/v4/projects/30716/repository/files/somefile.yaml: 400
+//       {message: 9:Could not update refs/heads/master. Please refresh and try again..}
+//       ```
+//
+//       This lock only solves half of the problem, where the provider is responsible for
+//       the concurrency. The other half is if the API is called outside of terraform at the same time
+//       this resource makes calls to the API.
+//       To mitigate this, simple retries are used.
+var resourceGitlabRepositoryFileApiLock = newLock()
+
 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.
 
-~> **Limitations**: The [GitLab Repository Files API](https://docs.gitlab.com/ee/api/repository_files.html) can only create, update or delete a single file at the time.  The API will also [fail with a 400](https://docs.gitlab.com/ee/api/repository_files.html#update-existing-file-in-repository) response status code if the underlying repository is changed while the API tries to make changes.  Therefore, it's recommended to make sure that you execute it with [-parallelism=1](https://www.terraform.io/docs/cli/commands/apply.html#parallelism-n) and that no other entity than the terraform at hand makes changes to the underlying repository while it's executing.
-
 **Upstream API**: [GitLab REST API docs](https://docs.gitlab.com/ee/api/repository_files.html)`,
 
 		CreateContext: resourceGitlabRepositoryFileCreate,
@@ -69,10 +80,18 @@ var _ = registerResource("gitlab_repository_file", func() *schema.Resource {
 })
 
 func resourceGitlabRepositoryFileCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-	client := meta.(*gitlab.Client)
 	project := d.Get("project").(string)
 	filePath := d.Get("file_path").(string)
 
+	log.Printf("[DEBUG] gitlab_repository_file: waiting for lock to create %s/%s", project, filePath)
+	if err := resourceGitlabRepositoryFileApiLock.lock(ctx); err != nil {
+		return diag.FromErr(err)
+	}
+	defer resourceGitlabRepositoryFileApiLock.unlock()
+	log.Printf("[DEBUG] gitlab_repository_file: got lock to create %s/%s", project, filePath)
+
+	client := meta.(*gitlab.Client)
+
 	options := &gitlab.CreateFileOptions{
 		Branch:        gitlab.String(d.Get("branch").(string)),
 		Encoding:      gitlab.String(encoding),
@@ -126,12 +145,20 @@ func resourceGitlabRepositoryFileRead(ctx context.Context, d *schema.ResourceDat
 }
 
 func resourceGitlabRepositoryFileUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-	client := meta.(*gitlab.Client)
 	project, branch, filePath, err := resourceGitLabRepositoryFileParseId(d.Id())
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
+	log.Printf("[DEBUG] gitlab_repository_file: waiting for lock to update %s/%s", project, filePath)
+	if err := resourceGitlabRepositoryFileApiLock.lock(ctx); err != nil {
+		return diag.FromErr(err)
+	}
+	defer resourceGitlabRepositoryFileApiLock.unlock()
+	log.Printf("[DEBUG] gitlab_repository_file: got lock to update %s/%s", project, filePath)
+
+	client := meta.(*gitlab.Client)
+
 	readOptions := &gitlab.GetFileOptions{
 		Ref: gitlab.String(branch),
 	}
@@ -163,12 +190,20 @@ func resourceGitlabRepositoryFileUpdate(ctx context.Context, d *schema.ResourceD
 }
 
 func resourceGitlabRepositoryFileDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
-	client := meta.(*gitlab.Client)
 	project, branch, filePath, err := resourceGitLabRepositoryFileParseId(d.Id())
 	if err != nil {
 		return diag.FromErr(err)
 	}
 
+	log.Printf("[DEBUG] gitlab_repository_file: waiting for lock to delete %s/%s", project, filePath)
+	if err := resourceGitlabRepositoryFileApiLock.lock(ctx); err != nil {
+		return diag.FromErr(err)
+	}
+	defer resourceGitlabRepositoryFileApiLock.unlock()
+	log.Printf("[DEBUG] gitlab_repository_file: got lock to delete %s/%s", project, filePath)
+
+	client := meta.(*gitlab.Client)
+
 	readOptions := &gitlab.GetFileOptions{
 		Ref: gitlab.String(branch),
 	}

internal/provider/resource_gitlab_repository_file_test.go

@@ -62,6 +62,32 @@ func TestAccGitlabRepositoryFile_createSameFileDifferentRepository(t *testing.T)
 	})
 }
 
+func TestAccGitlabRepositoryFile_concurrentResources(t *testing.T) {
+	testAccCheck(t)
+
+	testProject := testAccCreateProject(t)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		ProviderFactories: providerFactories,
+		CheckDestroy:      testAccCheckGitlabRepositoryFileDestroy,
+		Steps: []resource.TestStep{
+			// NOTE: we don't need to check anything here, just make sure no terraform errors are being raised,
+			//       the other test cases will do the actual testing :)
+			{
+				Config: testAccGitlabRepositoryFileConcurrentResourcesConfig(testProject.ID),
+			},
+			{
+				Config: testAccGitlabRepositoryFileConcurrentResourcesConfigUpdate(testProject.ID),
+			},
+			{
+				Config:  testAccGitlabRepositoryFileConcurrentResourcesConfigUpdate(testProject.ID),
+				Destroy: true,
+			},
+		},
+	})
+}
+
 func TestAccGitlabRepositoryFile_validationOfBase64Content(t *testing.T) {
 	cases := []struct {
 		givenContent           string
@@ -366,3 +392,31 @@ resource "gitlab_repository_file" "bar_file" {
 }
 	`, rInt, rInt)
 }
+
+func testAccGitlabRepositoryFileConcurrentResourcesConfig(projectID int) string {
+	return fmt.Sprintf(`
+resource "gitlab_repository_file" "this" {
+  project = "%d"
+  file_path = "file-${count.index}.txt"
+  branch = "main"
+  content = base64encode("content-${count.index}")
+  commit_message = "Add file ${count.index}"
+
+  count = 50
+}
+	`, projectID)
+}
+
+func testAccGitlabRepositoryFileConcurrentResourcesConfigUpdate(projectID int) string {
+	return fmt.Sprintf(`
+resource "gitlab_repository_file" "this" {
+  project = "%d"
+  file_path = "file-${count.index}.txt"
+  branch = "main"
+  content = base64encode("updated-content-${count.index}")
+  commit_message = "Add file ${count.index}"
+
+  count = 50
+}
+	`, projectID)
+}

internal/provider/util.go

@@ -1,6 +1,7 @@
 package provider
 
 import (
+	"context"
 	"fmt"
 	"net/url"
 	"regexp"
@@ -400,3 +401,27 @@ func setStateMapInResourceData(stateMap map[string]interface{}, d *schema.Resour
 
 	return nil
 }
+
+// lock can be used to lock, but make it `context.Context` aware.
+// e.g. it'll respect cancelling and timeouts.
+type lock chan struct{}
+
+func newLock() lock {
+	return make(lock, 1)
+
+}
+
+func (c lock) lock(ctx context.Context) error {
+	select {
+	case c <- struct{}{}:
+		// lock acquired
+		return nil
+	case <-ctx.Done():
+		// Timeout
+		return ctx.Err()
+	}
+}
+
+func (c lock) unlock() {
+	<-c
+}