Skip to content

Commit 9b08c04

Browse files
diofeherdiofeher
authored
fix: change fork to TypeString and add conditional ForceNew (#2959)
* fix: change "fork" to TypeString * lint + computed on source_owner and source_repo Signed-off-by: Diogenes Fernandes <diofeher@gmail.com> * adding automated test Signed-off-by: Diogenes Fernandes <diofeher@gmail.com> * source_repo and source_owner logic added Signed-off-by: Diogenes Fernandes <diofeher@gmail.com> * lint Signed-off-by: Diogenes Fernandes <diofeher@gmail.com> --------- Signed-off-by: Diogenes Fernandes <diofeher@gmail.com>
1 parent 6e45f0f commit 9b08c04

File tree

2 files changed

+148
-18
lines changed

2 files changed

+148
-18
lines changed

‎github/resource_github_repository.go‎

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"strings"
1111

1212
"github.com/google/go-github/v67/github"
13+
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
1314
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1415
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
1516
)
@@ -63,22 +64,24 @@ func resourceGithubRepository() *schema.Resource {
6364
ValidateDiagFunc: toDiagFunc(validation.StringInSlice([]string{"public", "private", "internal"}, false), "visibility"),
6465
Description: "Can be 'public' or 'private'. If your organization is associated with an enterprise account using GitHub Enterprise Cloud or GitHub Enterprise Server 2.20+, visibility can also be 'internal'.",
6566
},
67+
// terraform-sdk-provider doesn't properly support tristate booleans: https://github.com/hashicorp/terraform-plugin-sdk/issues/817
68+
// Using TypeString as the best alternative for now.
6669
"fork": {
67-
Type: schema.TypeBool,
70+
Type: schema.TypeString,
6871
Optional: true,
69-
ForceNew: true,
72+
Computed: true,
7073
Description: "Set to 'true' to fork an existing repository.",
7174
},
7275
"source_owner": {
7376
Type: schema.TypeString,
7477
Optional: true,
75-
ForceNew: true,
78+
Computed: true,
7679
Description: "The owner of the source repository to fork from.",
7780
},
7881
"source_repo": {
7982
Type: schema.TypeString,
8083
Optional: true,
81-
ForceNew: true,
84+
Computed: true,
8285
Description: "The name of the source repository to fork from.",
8386
},
8487
"security_and_analysis": {
@@ -479,10 +482,32 @@ func resourceGithubRepository() *schema.Resource {
479482
Description: " Set to 'true' to always suggest updating pull request branches.",
480483
},
481484
},
482-
CustomizeDiff: customDiffFunction,
485+
CustomizeDiff: customdiff.All(
486+
customDiffFunction,
487+
customdiff.ForceNewIfChange("fork", valueChangedButNotEmpty),
488+
customdiff.ForceNewIfChange("source_repo", valueChangedButNotEmpty),
489+
customdiff.ForceNewIfChange("source_owner", valueChangedButNotEmpty),
490+
),
483491
}
484492
}
485493

494+
// valueChangedButNotEmpty is a customdiff function that triggers recreation of the resource
495+
// if the field's value changes from a non-empty state to a different non-empty value.
496+
func valueChangedButNotEmpty(ctx context.Context, oldVal, newVal, meta any) bool {
497+
oldValStr := oldVal.(string)
498+
newValStr := newVal.(string)
499+
return oldValStr != "" && oldValStr != newValStr
500+
}
501+
502+
func customDiffFunction(_ context.Context, diff *schema.ResourceDiff, v any) error {
503+
if diff.HasChange("name") {
504+
if err := diff.SetNewComputed("full_name"); err != nil {
505+
return err
506+
}
507+
}
508+
return nil
509+
}
510+
486511
func calculateVisibility(d *schema.ResourceData) string {
487512
if value, ok := d.GetOk("visibility"); ok {
488513
return value.(string)
@@ -658,12 +683,11 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta any) error {
658683

659684
d.SetId(*repo.Name)
660685
}
661-
} else if d.Get("fork").(bool) {
686+
} else if d.Get("fork").(string) == "true" {
662687
// Handle repository forking
663688
sourceOwner := d.Get("source_owner").(string)
664689
sourceRepo := d.Get("source_repo").(string)
665690
requestedName := d.Get("name").(string)
666-
owner := meta.(*Owner).name
667691
log.Printf("[INFO] Creating fork of %s/%s in %s", sourceOwner, sourceRepo, owner)
668692

669693
if sourceOwner == "" || sourceRepo == "" {
@@ -838,15 +862,15 @@ func resourceGithubRepositoryRead(d *schema.ResourceData, meta any) error {
838862

839863
// Set fork information if this is a fork
840864
if repo.GetFork() {
841-
_ = d.Set("fork", true)
865+
_ = d.Set("fork", "true")
842866

843867
// If the repository has parent information, set the source details
844868
if repo.Parent != nil {
845869
_ = d.Set("source_owner", repo.Parent.GetOwner().GetLogin())
846870
_ = d.Set("source_repo", repo.Parent.GetName())
847871
}
848872
} else {
849-
_ = d.Set("fork", false)
873+
_ = d.Set("fork", "false")
850874
_ = d.Set("source_owner", "")
851875
_ = d.Set("source_repo", "")
852876
}
@@ -1218,15 +1242,6 @@ func resourceGithubParseFullName(resourceDataLike interface {
12181242
return parts[0], parts[1], true
12191243
}
12201244

1221-
func customDiffFunction(_ context.Context, diff *schema.ResourceDiff, v any) error {
1222-
if diff.HasChange("name") {
1223-
if err := diff.SetNewComputed("full_name"); err != nil {
1224-
return err
1225-
}
1226-
}
1227-
return nil
1228-
}
1229-
12301245
func updateVulnerabilityAlerts(d *schema.ResourceData, client *github.Client, ctx context.Context, owner, repoName string) error {
12311246
updateVulnerabilityAlerts := client.Repositories.DisableVulnerabilityAlerts
12321247
if vulnerabilityAlerts, ok := d.GetOk("vulnerability_alerts"); ok && vulnerabilityAlerts.(bool) {

‎github/resource_github_repository_test.go‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@ package github
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"log"
78
"regexp"
89
"strings"
910
"testing"
1011

12+
"github.com/google/go-github/v67/github"
1113
"github.com/hashicorp/go-cty/cty"
1214
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
1315
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
@@ -1933,4 +1935,117 @@ func TestAccGithubRepository_fork(t *testing.T) {
19331935
t.Skip("anonymous account not supported for this operation")
19341936
})
19351937
})
1938+
1939+
t.Run("can migrate a forked repository from a previous framework version", func(t *testing.T) {
1940+
rName := fmt.Sprintf("terraform-provider-github-%s", randomID)
1941+
olderConfig := fmt.Sprintf(`
1942+
import {
1943+
to = github_repository.forked
1944+
id = "%[1]s"
1945+
}
1946+
resource "github_repository" "forked" {
1947+
name = "%[1]s"
1948+
description = "Terraform acceptance test - forked repository %[1]s"
1949+
}
1950+
`, rName)
1951+
newerConfig := fmt.Sprintf(`
1952+
resource "github_repository" "forked" {
1953+
name = "%[1]s"
1954+
description = "Terraform acceptance test - forked repository %[1]s"
1955+
fork = true
1956+
source_owner = "integrations"
1957+
source_repo = "terraform-provider-github"
1958+
}
1959+
`, rName)
1960+
1961+
testCase := func(t *testing.T, mode string) {
1962+
providers := []*schema.Provider{testAccProvider}
1963+
resource.Test(t, resource.TestCase{
1964+
PreCheck: func() { skipUnlessMode(t, mode) },
1965+
Steps: []resource.TestStep{
1966+
{
1967+
ExternalProviders: map[string]resource.ExternalProvider{
1968+
"github": {
1969+
VersionConstraint: "~> 6.7.0",
1970+
Source: "integrations/github",
1971+
},
1972+
},
1973+
PreConfig: func() {
1974+
err := createForkedRepository(rName)
1975+
if err != nil {
1976+
t.Fatalf("failed to create fork of %s: %v", rName, err)
1977+
}
1978+
},
1979+
Config: olderConfig,
1980+
Check: resource.ComposeTestCheckFunc(
1981+
resource.TestCheckNoResourceAttr(
1982+
"github_repository.forked", "fork",
1983+
),
1984+
resource.TestCheckNoResourceAttr(
1985+
"github_repository.forked", "source_owner",
1986+
),
1987+
resource.TestCheckNoResourceAttr(
1988+
"github_repository.forked", "source_repo",
1989+
),
1990+
),
1991+
},
1992+
{
1993+
ProviderFactories: testAccProviderFactories(&providers),
1994+
Config: newerConfig,
1995+
Check: resource.ComposeTestCheckFunc(
1996+
resource.TestCheckResourceAttr(
1997+
"github_repository.forked", "fork",
1998+
"true",
1999+
),
2000+
resource.TestCheckResourceAttr(
2001+
"github_repository.forked", "source_owner",
2002+
"integrations",
2003+
),
2004+
resource.TestCheckResourceAttr(
2005+
"github_repository.forked", "source_repo",
2006+
"terraform-provider-github",
2007+
),
2008+
),
2009+
},
2010+
},
2011+
})
2012+
}
2013+
2014+
t.Run("with an individual account", func(t *testing.T) {
2015+
testCase(t, individual)
2016+
})
2017+
2018+
t.Run("with an organization account", func(t *testing.T) {
2019+
testCase(t, organization)
2020+
})
2021+
2022+
t.Run("with an anonymous account", func(t *testing.T) {
2023+
t.Skip("anonymous account not supported for this operation")
2024+
})
2025+
})
2026+
}
2027+
2028+
func createForkedRepository(repositoryName string) error {
2029+
config := Config{BaseURL: "https://api.github.com/", Owner: testOrganizationFunc(), Token: testToken}
2030+
meta, err := config.Meta()
2031+
if err != nil {
2032+
return fmt.Errorf("failed to create client: %w", err)
2033+
}
2034+
client := meta.(*Owner).v3client
2035+
orgName := meta.(*Owner).name
2036+
ctx := context.TODO()
2037+
2038+
_, _, err = client.Repositories.CreateFork(ctx, "integrations", "snappydoo", &github.RepositoryCreateForkOptions{
2039+
Organization: orgName,
2040+
Name: repositoryName,
2041+
})
2042+
2043+
acceptedError := &github.AcceptedError{}
2044+
if err != nil {
2045+
if errors.As(err, &acceptedError) {
2046+
return nil
2047+
}
2048+
return fmt.Errorf("failed to create fork: %w", err)
2049+
}
2050+
return nil
19362051
}

0 commit comments

Comments
 (0)