diff --git a/docs/resources/actions_environment_secret.md b/docs/resources/actions_environment_secret.md index a8766dec48..67cd90719a 100644 --- a/docs/resources/actions_environment_secret.md +++ b/docs/resources/actions_environment_secret.md @@ -90,7 +90,7 @@ The following arguments are supported: ## Import -This resource can be imported using an ID made of the repository name, environment name (URL escaped), and secret name all separated by a `:`. +This resource can be imported using an ID made of the repository name, environment name (any `:` in the environment name need to be escaped as `??`), and secret name all separated by a `:`. ~> **Note**: When importing secrets, the `value`, `value_encrypted`, `encrypted_value`, or `plaintext_value` fields will not be populated in the state. You may need to ignore changes for these as a workaround if you're not planning on updating the secret through Terraform. diff --git a/docs/resources/repository_file.md b/docs/resources/repository_file.md index 700b3c00fc..aa6ca09cf4 100644 --- a/docs/resources/repository_file.md +++ b/docs/resources/repository_file.md @@ -35,13 +35,19 @@ resource "github_repository_file" "foo" { ### Auto Created Branch ```terraform -resource "github_repository" "foo" { - name = "example" +resource "github_repository" "bar" { + name = "example2" auto_init = true } -resource "github_repository_file" "foo" { - repository = github_repository.foo.name +resource "github_branch" "bar" { + branch = "does/not/exist" + repository = github_repository.bar.name + +} + +resource "github_repository_file" "bar" { + repository = github_repository.bar.name branch = "does/not/exist" file = ".gitignore" content = "**/*.tfstate" @@ -49,7 +55,6 @@ resource "github_repository_file" "foo" { commit_author = "Terraform User" commit_email = "terraform@example.com" overwrite_on_create = true - autocreate_branch = true } ``` @@ -93,7 +98,7 @@ The following additional attributes are exported: ## Import -Repository files can be imported using a combination of the `repo`, `file` and `branch` or empty branch for the default branch, e.g. +Repository files can be imported using a combination of the `repo`, `file path` (any `:` in the file path need to be escaped as `??`) and `branch` or empty branch for the default branch, e.g. ```sh terraform import github_repository_file.gitignore example:.gitignore:feature-branch diff --git a/examples/dev.tfrc b/examples/dev.tfrc index 82a9812a63..e89c638dda 100644 --- a/examples/dev.tfrc +++ b/examples/dev.tfrc @@ -1,7 +1,9 @@ provider_installation { dev_overrides { - "integrations/github" = "~/go/bin/" + "integrations/github" = "$HOME/go/bin/" } - direct {} -} \ No newline at end of file + direct { + exclude = ["registry.terraform.io/hashicorp/github"] + } +} diff --git a/examples/resources/repository_file/example_2.tf b/examples/resources/repository_file/example_2.tf index 0cd472fba5..6e23752c8d 100644 --- a/examples/resources/repository_file/example_2.tf +++ b/examples/resources/repository_file/example_2.tf @@ -1,11 +1,17 @@ -resource "github_repository" "foo" { - name = "example" +resource "github_repository" "bar" { + name = "example2" auto_init = true } -resource "github_repository_file" "foo" { - repository = github_repository.foo.name +resource "github_branch" "bar" { + branch = "does/not/exist" + repository = github_repository.bar.name + +} + +resource "github_repository_file" "bar" { + repository = github_repository.bar.name branch = "does/not/exist" file = ".gitignore" content = "**/*.tfstate" @@ -13,6 +19,4 @@ resource "github_repository_file" "foo" { commit_author = "Terraform User" commit_email = "terraform@example.com" overwrite_on_create = true - autocreate_branch = true } - diff --git a/github/resource_github_repository_file.go b/github/resource_github_repository_file.go index 870636c4ec..8c4a45f9b0 100644 --- a/github/resource_github_repository_file.go +++ b/github/resource_github_repository_file.go @@ -242,7 +242,7 @@ func resourceGithubRepositoryFileCreate(ctx context.Context, d *schema.ResourceD return diag.FromErr(err) } - newResourceID, err := buildID(repo, file, branch) + newResourceID, err := buildID(repo, escapeIDPart(file), branch) if err != nil { return diag.FromErr(err) } @@ -392,7 +392,7 @@ func resourceGithubRepositoryFileUpdate(ctx context.Context, d *schema.ResourceD } if d.HasChanges("repository", "file", "branch") { - newResourceID, err := buildID(repo, file, branch) + newResourceID, err := buildID(repo, escapeIDPart(file), branch) if err != nil { return diag.FromErr(err) } @@ -433,11 +433,13 @@ func autoBranchDiffSuppressFunc(k, _, _ string, d *schema.ResourceData) bool { } func resourceGithubRepositoryFileImport(ctx context.Context, d *schema.ResourceData, meta any) ([]*schema.ResourceData, error) { - repo, filePath, branch, err := parseID3(d.Id()) + repo, filePathPart, branch, err := parseID3(d.Id()) if err != nil { return nil, fmt.Errorf("invalid ID specified. Supplied ID must be written as :: (when branch is default) or ::. %w", err) } + filePath := unescapeIDPart(filePathPart) + client := meta.(*Owner).v3client owner := meta.(*Owner).name @@ -468,7 +470,7 @@ func resourceGithubRepositoryFileImport(ctx context.Context, d *schema.ResourceD return nil, err } - newResourceID, err := buildID(repo, filePath, branch) + newResourceID, err := buildID(repo, escapeIDPart(filePath), branch) if err != nil { return nil, err } diff --git a/github/resource_github_repository_file_migration.go b/github/resource_github_repository_file_migration.go index 1c6739e58d..219d1f0eca 100644 --- a/github/resource_github_repository_file_migration.go +++ b/github/resource_github_repository_file_migration.go @@ -85,7 +85,7 @@ func resourceGithubRepositoryFileV0() *schema.Resource { } func resourceGithubRepositoryFileStateUpgradeV0(ctx context.Context, rawState map[string]any, m any) (map[string]any, error) { - tflog.Debug(ctx, "GitHub Repository File State before migration", map[string]any{ + tflog.Debug(ctx, "GitHub Repository File State before v0 migration", map[string]any{ "rawState": rawState, }) @@ -93,30 +93,54 @@ func resourceGithubRepositoryFileStateUpgradeV0(ctx context.Context, rawState ma client := meta.v3client owner := meta.name - repoName, ok := rawState["repository"].(string) - if !ok { - return nil, fmt.Errorf("repository not found or is not a string") + repoName := "" + if v, ok := rawState["repository"]; ok { + if s, ok := v.(string); ok && s != "" { + repoName = s + } + } + if repoName == "" { + return nil, fmt.Errorf("state upgrade v0: repository is not a string or not set") } repo, _, err := client.Repositories.Get(ctx, owner, repoName) if err != nil { - return nil, fmt.Errorf("failed to retrieve repository '%s': %w", repoName, err) + return nil, fmt.Errorf("state upgrade v0: failed to retrieve repository '%s': %w", repoName, err) } rawState["repository_id"] = int(repo.GetID()) - // If branch is missing or empty, fetch the default branch from the repository - if branch, ok := rawState["branch"].(string); !ok || branch == "" { - rawState["branch"] = repo.GetDefaultBranch() + branch := "" + if v, ok := rawState["branch"]; ok { + if s, ok := v.(string); ok && s != "" { + branch = s + } + } + + // If branch is empty, fetch the default branch from the repository + if branch == "" { + branch = repo.GetDefaultBranch() + rawState["branch"] = branch + } + + filePath := "" + if v, ok := rawState["file"]; ok { + if s, ok := v.(string); ok && s != "" { + filePath = s + } + } + + if filePath == "" { + return nil, fmt.Errorf("state upgrade v0: file path is not a string") } - newResourceID, err := buildID(rawState["repository"].(string), rawState["file"].(string), rawState["branch"].(string)) + newResourceID, err := buildID(repoName, escapeIDPart(filePath), branch) if err != nil { - return nil, fmt.Errorf("failed to build ID: %w", err) + return nil, fmt.Errorf("state upgrade v0: failed to build ID: %w", err) } rawState["id"] = newResourceID - tflog.Debug(ctx, "GitHub Repository File State after migration", map[string]any{ + tflog.Debug(ctx, "GitHub Repository File State after v0 migration", map[string]any{ "rawState": rawState, }) return rawState, nil diff --git a/github/resource_github_repository_file_migration_test.go b/github/resource_github_repository_file_migration_test.go index f9bd8f866b..a9081639cc 100644 --- a/github/resource_github_repository_file_migration_test.go +++ b/github/resource_github_repository_file_migration_test.go @@ -2,154 +2,180 @@ package github // TODO: Enable this test once we have a way to mock the GitHub API -// import ( -// "context" -// "encoding/json" -// "fmt" -// "net/http" -// "net/url" -// "testing" +import ( + "encoding/json" + "fmt" + "net/http" + "testing" -// "github.com/google/go-cmp/cmp" -// "github.com/google/go-github/v88/github" -// ) + "github.com/google/go-cmp/cmp" + "github.com/google/go-github/v88/github" +) -// func buildMockResponsesForRepositoryFileMigrationV0toV1(mockOwner, mockRepo string, wantRepoID int) []*mockResponse { -// responseBodyJson, err := json.Marshal(github.Repository{ -// ID: github.Ptr(int64(wantRepoID)), -// DefaultBranch: github.Ptr("main"), -// Name: github.Ptr(mockRepo), -// }) -// if err != nil { -// panic(fmt.Sprintf("error marshalling repository response: %s", err)) -// } -// return []*mockResponse{{ -// ExpectedUri: fmt.Sprintf("/repos/%s/%s", mockOwner, mockRepo), -// ExpectedHeaders: map[string]string{ -// "Accept": "application/vnd.github.scarlet-witch-preview+json, application/vnd.github.mercy-preview+json, application/vnd.github.baptiste-preview+json, application/vnd.github.nebula-preview+json", -// }, -// ResponseBody: string(responseBodyJson), -// StatusCode: http.StatusOK, -// }} -// } +func buildMockResponsesForRepositoryFileMigrationV0toV1(mockOwner, mockRepo string, wantRepoID int) []*mockResponse { + responseBodyJson, err := json.Marshal(github.Repository{ + ID: new(int64(wantRepoID)), + DefaultBranch: new("main"), + Name: new(mockRepo), + }) + if err != nil { + panic(fmt.Sprintf("error marshalling repository response: %s", err)) + } + return []*mockResponse{{ + ExpectedUri: fmt.Sprintf("/repos/%s/%s", mockOwner, mockRepo), + ExpectedHeaders: map[string]string{ + "Accept": "application/vnd.github.scarlet-witch-preview+json, application/vnd.github.mercy-preview+json, application/vnd.github.baptiste-preview+json, application/vnd.github.nebula-preview+json", + }, + ResponseBody: string(responseBodyJson), + StatusCode: http.StatusOK, + }} +} -// func Test_resourceGithubRepositoryFileStateUpgradeV0toV1(t *testing.T) { -// t.Parallel() +func Test_resourceGithubRepositoryFileStateUpgradeV0toV1(t *testing.T) { + t.Parallel() -// for _, d := range []struct { -// testName string -// rawState map[string]any -// want map[string]any -// shouldError bool -// }{ -// { -// testName: "preserves_existing_branch", -// rawState: map[string]any{ -// "id": "test-repo/path/to/file.txt", -// "repository": "test-repo", -// "file": "path/to/file.txt", -// "content": "file content", -// "branch": "main", -// "commit_sha": "abc123", -// "sha": "def456", -// "overwrite_on_create": false, -// }, -// want: map[string]any{ -// "id": "test-repo:path/to/file.txt:main", -// "repository": "test-repo", -// "repository_id": 1234567890, -// "file": "path/to/file.txt", -// "content": "file content", -// "branch": "main", -// "commit_sha": "abc123", -// "sha": "def456", -// "overwrite_on_create": false, -// }, -// shouldError: false, -// }, -// { -// testName: "preserves_custom_branch", -// rawState: map[string]any{ -// "id": "test-repo/README.md", -// "repository": "test-repo", -// "file": "README.md", -// "content": "# README", -// "branch": "develop", -// }, -// want: map[string]any{ -// "id": "test-repo:README.md:develop", -// "repository": "test-repo", -// "repository_id": 1234567890, -// "file": "README.md", -// "content": "# README", -// "branch": "develop", -// }, -// shouldError: false, -// }, -// { -// testName: "migrates_with_missing_branch", -// rawState: map[string]any{ -// "id": "test-repo/path/to/file.txt", -// "repository": "test-repo", -// "file": "path/to/file.txt", -// "content": "file content", -// }, -// want: map[string]any{ -// "id": "test-repo:path/to/file.txt:main", -// "repository": "test-repo", -// "repository_id": 1234567890, -// "file": "path/to/file.txt", -// "content": "file content", -// "branch": "main", // fetched from API -// }, -// shouldError: false, -// }, -// { -// testName: "migrates_with_empty_branch", -// rawState: map[string]any{ -// "id": "test-repo/path/to/file.txt", -// "repository": "test-repo", -// "file": "path/to/file.txt", -// "content": "file content", -// "branch": "", -// }, -// want: map[string]any{ -// "id": "test-repo:path/to/file.txt:main", -// "repository": "test-repo", -// "repository_id": 1234567890, -// "file": "path/to/file.txt", -// "content": "file content", -// "branch": "main", // fetched from API -// }, -// shouldError: false, -// }, -// } { -// t.Run(d.testName, func(t *testing.T) { -// t.Parallel() + for _, d := range []struct { + testName string + rawState map[string]any + want map[string]any + shouldError bool + }{ + { + testName: "preserves_existing_branch", + rawState: map[string]any{ + "id": "test-repo/path/to/file.txt", + "repository": "test-repo", + "file": "path/to/file.txt", + "content": "file content", + "branch": "main", + "commit_sha": "abc123", + "sha": "def456", + "overwrite_on_create": false, + }, + want: map[string]any{ + "id": "test-repo:path/to/file.txt:main", + "repository": "test-repo", + "repository_id": 1234567890, + "file": "path/to/file.txt", + "content": "file content", + "branch": "main", + "commit_sha": "abc123", + "sha": "def456", + "overwrite_on_create": false, + }, + shouldError: false, + }, + { + testName: "preserves_custom_branch", + rawState: map[string]any{ + "id": "test-repo/README.md", + "repository": "test-repo", + "file": "README.md", + "content": "# README", + "branch": "develop", + }, + want: map[string]any{ + "id": "test-repo:README.md:develop", + "repository": "test-repo", + "repository_id": 1234567890, + "file": "README.md", + "content": "# README", + "branch": "develop", + }, + shouldError: false, + }, + { + testName: "migrates_with_missing_branch", + rawState: map[string]any{ + "id": "test-repo/path/to/file.txt", + "repository": "test-repo", + "file": "path/to/file.txt", + "content": "file content", + }, + want: map[string]any{ + "id": "test-repo:path/to/file.txt:main", + "repository": "test-repo", + "repository_id": 1234567890, + "file": "path/to/file.txt", + "content": "file content", + "branch": "main", // fetched from API + }, + shouldError: false, + }, + { + testName: "migrates_with_empty_branch", + rawState: map[string]any{ + "id": "test-repo/path/to/file.txt", + "repository": "test-repo", + "file": "path/to/file.txt", + "content": "file content", + "branch": "", + }, + want: map[string]any{ + "id": "test-repo:path/to/file.txt:main", + "repository": "test-repo", + "repository_id": 1234567890, + "file": "path/to/file.txt", + "content": "file content", + "branch": "main", // fetched from API + }, + shouldError: false, + }, + { + testName: "migrates_with_colon_in_file_path", + rawState: map[string]any{ + "id": "test-repo/path/to:file.txt", + "repository": "test-repo", + "file": "path/to:file.txt", + "content": "file content", + "branch": "main", + }, + want: map[string]any{ + "id": "test-repo:path/to??file.txt:main", + "repository": "test-repo", + "repository_id": 1234567890, + "file": "path/to:file.txt", + "content": "file content", + "branch": "main", + }, + shouldError: false, + }, + } { + t.Run(d.testName, func(t *testing.T) { + t.Parallel() -// meta := &Owner{ -// name: "test-org", -// } + meta := &Owner{ + name: "test-org", + } -// ts := githubApiMock(buildMockResponsesForRepositoryFileMigrationV0toV1(meta.name, d.want["repository"].(string), d.want["repository_id"].(int))) -// defer ts.Close() + wantRepositoryName := "" + if v, ok := d.want["repository"]; ok { + if s, ok := v.(string); ok { + wantRepositoryName = s + } + } -// httpCl := http.DefaultClient -// httpCl.Transport = http.DefaultTransport + wantRepositoryID := -1 + if v, ok := d.want["repository_id"]; ok { + if i, ok := v.(int); ok { + wantRepositoryID = i + } + } -// client := github.NewClient(httpCl) -// u, _ := url.Parse(ts.URL + "/") -// client.BaseURL = u -// meta.v3client = client + ts := githubApiMock(buildMockResponsesForRepositoryFileMigrationV0toV1(meta.name, wantRepositoryName, wantRepositoryID)) + defer ts.Close() -// got, err := resourceGithubRepositoryFileStateUpgradeV0(t.Context(), d.rawState, meta) -// if (err != nil) != d.shouldError { -// t.Fatalf("unexpected error state: got error %v, shouldError %v", err, d.shouldError) -// } + client := mustGitHubClient(t, ts.URL) + meta.v3client = client -// if diff := cmp.Diff(got, d.want); diff != "" && !d.shouldError { -// t.Fatalf("got %+v, want %+v, diff %s", got, d.want, diff) -// } -// }) -// } -// } + got, err := resourceGithubRepositoryFileStateUpgradeV0(t.Context(), d.rawState, meta) + if (err != nil) != d.shouldError { + t.Fatalf("unexpected error state: got error %v, shouldError %v", err, d.shouldError) + } + + if diff := cmp.Diff(got, d.want); diff != "" && !d.shouldError { + t.Fatalf("got %+v, want %+v, diff %s", got, d.want, diff) + } + }) + } +} diff --git a/github/resource_github_repository_file_test.go b/github/resource_github_repository_file_test.go index 57343b6dd8..ed778fcab2 100644 --- a/github/resource_github_repository_file_test.go +++ b/github/resource_github_repository_file_test.go @@ -8,6 +8,9 @@ import ( "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/knownvalue" + "github.com/hashicorp/terraform-plugin-testing/statecheck" + "github.com/hashicorp/terraform-plugin-testing/tfjsonpath" ) func TestAccGithubRepositoryFile(t *testing.T) { @@ -455,4 +458,47 @@ func TestAccGithubRepositoryFile(t *testing.T) { }, }) }) + + t.Run("verify_that_id_can_contain_colon_in_file_path", func(t *testing.T) { + randomID := acctest.RandString(5) + repoName := fmt.Sprintf("%srepo-file-%s", testResourcePrefix, randomID) + filePathWithColon := "repro/example:one.yaml" + config := fmt.Sprintf(` + + resource "github_repository" "test" { + name = "%s" + auto_init = true + vulnerability_alerts = true + } + + resource "github_repository_file" "test" { + repository = github_repository.test.name + branch = "main" + file = "%s" + content = "bar" + commit_message = "Managed by Terraform" + commit_author = "Terraform User" + commit_email = "terraform@example.com" + } + `, repoName, filePathWithColon) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { skipUnauthenticated(t) }, + ProviderFactories: providerFactories, + Steps: []resource.TestStep{ + { + Config: config, + ConfigStateChecks: []statecheck.StateCheck{ + statecheck.ExpectKnownValue("github_repository_file.test", tfjsonpath.New("id"), knownvalue.StringFunc(func(v string) error { + if strings.Contains(v, escapeIDPart(filePathWithColon)) { + return nil + } + return fmt.Errorf("expected id to contain escaped file path, got: %s", v) + })), + statecheck.ExpectKnownValue("github_repository_file.test", tfjsonpath.New("file"), knownvalue.StringExact(filePathWithColon)), + }, + }, + }, + }) + }) } diff --git a/templates/resources/actions_environment_secret.md.tmpl b/templates/resources/actions_environment_secret.md.tmpl index eda57c9fad..e6eee7316f 100644 --- a/templates/resources/actions_environment_secret.md.tmpl +++ b/templates/resources/actions_environment_secret.md.tmpl @@ -48,7 +48,7 @@ The following arguments are supported: ## Import -This resource can be imported using an ID made of the repository name, environment name (URL escaped), and secret name all separated by a `:`. +This resource can be imported using an ID made of the repository name, environment name (any `:` in the environment name need to be escaped as `??`), and secret name all separated by a `:`. ~> **Note**: When importing secrets, the `value`, `value_encrypted`, `encrypted_value`, or `plaintext_value` fields will not be populated in the state. You may need to ignore changes for these as a workaround if you're not planning on updating the secret through Terraform. diff --git a/templates/resources/repository_file.md.tmpl b/templates/resources/repository_file.md.tmpl index 25b9d6c198..54ce081ec0 100644 --- a/templates/resources/repository_file.md.tmpl +++ b/templates/resources/repository_file.md.tmpl @@ -60,7 +60,7 @@ The following additional attributes are exported: ## Import -Repository files can be imported using a combination of the `repo`, `file` and `branch` or empty branch for the default branch, e.g. +Repository files can be imported using a combination of the `repo`, `file path` (any `:` in the file path need to be escaped as `??`) and `branch` or empty branch for the default branch, e.g. ```sh terraform import github_repository_file.gitignore example:.gitignore:feature-branch