Refactor out branches attribute to new data source #1117

k24dizzle · 2022-04-18T23:20:39Z

Refactor out branches attribute from resource_github_repository and data_source_github_repository
Introduce a new data_source called data_source_github_repository_branches that returns branches instead.

Rationale:
I believe branches shouldn't belong to the attributes of the github_repository resource/data source. These fields should be relevant to the repository itself, not other auxiliary resources (like teams, collaborators, branches, etc.)

Similar to github_collaborators, github_repository_pull_requests, and other data sources, it would be better to create a new data source to contain this information instead.

If a developer wants to interact with the branches of a repo, they can use data_source_github_repository_branches instead. This helps prevent some of the problems described in the issues below.

Testing done in this repo:
https://github.com/kzhaotest/branches

Fixes:
#1037
#1010

While still supporting the functionality of:
#748

k24dizzle · 2022-04-18T23:25:56Z

github/data_source_github_repository_branches.go

+	orgName := meta.(*Owner).name
+	repoName := d.Get("repository").(string)
+
+	branches, _, err := client.Repositories.ListBranches(context.TODO(), orgName, repoName, nil)


note, this data source will only display the first 30 branches of a repository, this is the default setting of the Github API endpoint

This mimics the previous behavior introduced in #892: https://github.com/kzhaotest/branches_test1 (see in this repo although there are 50+ branches, only 30+ are shown in the README (tf located here)).

We can introduce pagination in a future PR, but for now just replicate the previous behavior

k24dizzle · 2022-05-03T18:28:52Z

Hi @kfcampbell, I'm curious if there's any thoughts on this/best ways to roll this out

…ata_source_github_repository to data_source_github_repository_branches

k24dizzle · 2022-05-31T18:29:17Z

@kfcampbell friendly bump

kfcampbell · 2022-06-10T23:48:33Z

@k24dizzle apologies for the slow response time. I'll shoot to get this into the next release!

github/data_source_github_repository_branches.go

Co-authored-by: mareksapota-lyft <msapota@lyft.com>

…tions#1162) This will allow to retrieve only the protected branches instead of the first 30. Co-authored-by: Keegan Campbell <me@kfcampbell.com>

eriksw · 2022-07-20T17:51:46Z

@kfcampbell Any update on getting this into a release? It looks like the 4.27.0 milestone was missed by the release.

DoctorPolski · 2022-08-05T10:20:52Z

@kfcampbell it would be great to get this out ASAP if you can squeeze it in.

Thanks.

kfcampbell · 2022-08-27T00:05:07Z

@k24dizzle Apologies for the delay. Integration tests on this aren't passing for me:

--- FAIL: TestAccGithubBranchesDataSource (83.24s)
    --- FAIL: TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository (80.94s)
        --- SKIP: TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_anonymous_account (0.00s)
        --- SKIP: TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_individual_account (0.00s)
        --- FAIL: TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_organization_account (79.90s)
FAIL

The error seems to be a result of this snippet:

testing.go:654: Step 0 error: After applying this step, the plan was not empty:

    DIFF:
    
    UPDATE: data.github_repository_branches.test
      branches:   "" => "<computed>"
      id:         "" => "<computed>"
      repository: "" => "tf-acc-test-branches-2wdmv"
    UPDATE: github_repository.test
      allow_auto_merge:       "false" => "false"
      allow_merge_commit:     "true" => "true"
      allow_rebase_merge:     "true" => "true"
      allow_squash_merge:     "true" => "true"
      archived:               "false" => "false"
      auto_init:              "true" => "true"
      default_branch:         "main" => "main"
      delete_branch_on_merge: "false" => "false"
      description:            "" => ""
      etag:                   "W/\"b693979949e92b7a9a6fe1a72dfb591c806dbf206830fb62131e9b10b6419376\"" => "W/\"b693979949e92b7a9a6fe1a72dfb591c806dbf206830fb62131e9b10b6419376\""
      full_name:              "kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv" => "kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv"
      git_clone_url:          "git://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv.git" => "git://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv.git"
      has_downloads:          "false" => "false"
      has_issues:             "false" => "false"
      has_projects:           "false" => "false"
      has_wiki:               "false" => "false"
      homepage_url:           "" => ""
      html_url:               "https://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv" => "https://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv"
      http_clone_url:         "https://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv.git" => "https://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv.git"
      id:                     "tf-acc-test-branches-2wdmv" => "tf-acc-test-branches-2wdmv"
      is_template:            "false" => "false"
      name:                   "tf-acc-test-branches-2wdmv" => "tf-acc-test-branches-2wdmv"
      node_id:                "R_kgDOH46FUQ" => "R_kgDOH46FUQ"
      pages.#:                "0" => "0"
      private:                "false" => "false"
      repo_id:                "529433937" => "529433937"
      ssh_clone_url:          "git@github.com:kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv.git" => "git@github.com:kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv.git"
      svn_url:                "https://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv" => "https://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv"
      template.#:             "0" => "0"
      visibility:             "public" => "public"
      vulnerability_alerts:   "true" => ""
    
    
    
    STATE:
    
    data.github_repository_branches.test:
      ID = kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv
      provider = provider.github
      branches.# = 1
      branches.0.name = main
      branches.0.protected = false
      repository = tf-acc-test-branches-2wdmv
    
      Dependencies:
        github_repository.test
    github_repository.test:
      ID = tf-acc-test-branches-2wdmv
      provider = provider.github
      allow_auto_merge = false
      allow_merge_commit = true
      allow_rebase_merge = true
      allow_squash_merge = true
      archived = false
      auto_init = true
      default_branch = main
      delete_branch_on_merge = false
      description = 
      etag = W/"b693979949e92b7a9a6fe1a72dfb591c806dbf206830fb62131e9b10b6419376"
      full_name = kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv
      git_clone_url = git://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv.git
      has_downloads = false
      has_issues = false
      has_projects = false
      has_wiki = false
      homepage_url = 
      html_url = https://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv
      http_clone_url = https://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv.git
      is_template = false
      name = tf-acc-test-branches-2wdmv
      node_id = R_kgDOH46FUQ
      private = false
      repo_id = 529433937
      ssh_clone_url = git@github.com:kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv.git
      svn_url = https://github.com/kfcampbell-terraform-provider/tf-acc-test-branches-2wdmv
      visibility = public
      vulnerability_alerts = true

Are the tests passing for you locally?

mareksapota-lyft · 2022-09-01T22:17:56Z

github/data_source_github_repository_branches_test.go

+	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
+)
+
+func TestAccGithubBranchesDataSource(t *testing.T) {


Suggested change

func TestAccGithubBranchesDataSource(t *testing.T) {

func TestAccGithubRepositoryBranchesDataSource(t *testing.T) {

mareksapota-lyft · 2022-09-01T23:11:50Z

@kfcampbell The TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_organization_account test is passing for me.

$ export GITHUB_ORGANIZATION=test-org-mareksapota-lyft
$ export GITHUB_OWNER=mareksapota-lyft
$ go clean -testcache
$ TF_ACC=1 go test -v -count=1 ./... -run '^TestAccGithubBranchesDataSource'
?       github.com/integrations/terraform-provider-github/v4    [no test files]
=== RUN   TestAccGithubBranchesDataSource
=== RUN   TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository
=== RUN   TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_anonymous_account
    data_source_github_repository_branches_test.go:46: anonymous account not supported for this operation
=== RUN   TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_individual_account
=== RUN   TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_organization_account
--- PASS: TestAccGithubBranchesDataSource (13.99s)
    --- PASS: TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository (13.99s)
        --- SKIP: TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_anonymous_account (0.00s)
        --- PASS: TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_individual_account (7.22s)
        --- PASS: TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_organization_account (6.77s)
PASS
ok      github.com/integrations/terraform-provider-github/v4/github     14.358s

mareksapota-lyft · 2022-09-01T23:15:40Z

@kfcampbell The TestAccGithubBranchesDataSource/manages_branches_of_a_new_repository/with_an_organization_account test is passing for me.

On both f857f8f and 2b683f6.

k24dizzle · 2022-09-01T23:19:08Z

@kfcampbell went ahead and updated the branch with the upstream ^

the tests work for me locally as well:

~/go/src/github.com/terraform-provider-github [branches] % go test -v ./... -run ^TestAccGithubRepositoryBranchesDataSource         
?   	github.com/integrations/terraform-provider-github/v4	[no test files]
=== RUN   TestAccGithubRepositoryBranchesDataSource
=== RUN   TestAccGithubRepositoryBranchesDataSource/manages_branches_of_a_new_repository
=== RUN   TestAccGithubRepositoryBranchesDataSource/manages_branches_of_a_new_repository/with_an_anonymous_account
    data_source_github_repository_branches_test.go:46: anonymous account not supported for this operation
=== RUN   TestAccGithubRepositoryBranchesDataSource/manages_branches_of_a_new_repository/with_an_individual_account
    provider_utils.go:56: GITHUB_TOKEN and GITHUB_OWNER environment variables should be set
    provider_utils.go:66: Skipping TestAccGithubRepositoryBranchesDataSource/manages_branches_of_a_new_repository/with_an_individual_account which requires individual mode
=== RUN   TestAccGithubRepositoryBranchesDataSource/manages_branches_of_a_new_repository/with_an_organization_account
--- PASS: TestAccGithubRepositoryBranchesDataSource (8.38s)
    --- PASS: TestAccGithubRepositoryBranchesDataSource/manages_branches_of_a_new_repository (8.38s)
        --- SKIP: TestAccGithubRepositoryBranchesDataSource/manages_branches_of_a_new_repository/with_an_anonymous_account (0.00s)
        --- SKIP: TestAccGithubRepositoryBranchesDataSource/manages_branches_of_a_new_repository/with_an_individual_account (0.00s)
        --- PASS: TestAccGithubRepositoryBranchesDataSource/manages_branches_of_a_new_repository/with_an_organization_account (8.38s)
PASS
ok  	github.com/integrations/terraform-provider-github/v4/github	8.641s

If I'm interpreting the failure correctly, it seems like the provider thinks the repository resource needs to be updated.
(My data source is irrelevant to the failure as it should not be a part of the plan b/c it is not a resource)

vulnerability_alerts:   "true" => ""

This seems like the only field the plan wants to change in the repo resource.
This PR seems relevant, but still not sure why it works for us but not for you...
#768

mareksapota-lyft · 2022-09-01T23:28:33Z

If dependabot is enabled on the organization by default tests will fail. Tests pass with this setting off:

https://github.com/organizations/ORGNAME/settings/security_analysis

…itory_branches Remove leftover code and doc after integrations#1117 Reintroduce the feature from integrations#1162 with integration tests

…itory_branches (#1296) * Allow to get only protected on non protected branch with github_repository_branches Remove leftover code and doc after #1117 Reintroduce the feature from #1162 with integration tests * Add ConflicsWith * Implement pagination Co-authored-by: Keegan Campbell <me@kfcampbell.com>

* Refactor out branches attribute from resource_github_repository and data_source_github_repository to data_source_github_repository_branches * Add link to docs * Update github/data_source_github_repository_branches.go Co-authored-by: mareksapota-lyft <msapota@lyft.com> * Add only_protected_branches in data source github_repository (integrations#1162) This will allow to retrieve only the protected branches instead of the first 30. Co-authored-by: Keegan Campbell <me@kfcampbell.com> * Remove mistakenly committed binary (integrations#1223) * Revert PR integrations#1162 (integrations#1224) * marek's suggestion + use v47 Co-authored-by: mareksapota-lyft <msapota@lyft.com> Co-authored-by: Bertrand Paquet <bertrand.paquet@gmail.com> Co-authored-by: Keegan Campbell <me@kfcampbell.com>

…itory_branches (integrations#1296) * Allow to get only protected on non protected branch with github_repository_branches Remove leftover code and doc after integrations#1117 Reintroduce the feature from integrations#1162 with integration tests * Add ConflicsWith * Implement pagination Co-authored-by: Keegan Campbell <me@kfcampbell.com>

…-github#1117)

…-github#1117) (#479) Co-authored-by: rorybryett <rory.bryett@dwp.gov.uk>

* Refactor out branches attribute from resource_github_repository and data_source_github_repository to data_source_github_repository_branches * Add link to docs * Update github/data_source_github_repository_branches.go Co-authored-by: mareksapota-lyft <msapota@lyft.com> * Add only_protected_branches in data source github_repository (integrations#1162) This will allow to retrieve only the protected branches instead of the first 30. Co-authored-by: Keegan Campbell <me@kfcampbell.com> * Remove mistakenly committed binary (integrations#1223) * Revert PR integrations#1162 (integrations#1224) * marek's suggestion + use v47 Co-authored-by: mareksapota-lyft <msapota@lyft.com> Co-authored-by: Bertrand Paquet <bertrand.paquet@gmail.com> Co-authored-by: Keegan Campbell <me@kfcampbell.com>

…itory_branches (integrations#1296) * Allow to get only protected on non protected branch with github_repository_branches Remove leftover code and doc after integrations#1117 Reintroduce the feature from integrations#1162 with integration tests * Add ConflicsWith * Implement pagination Co-authored-by: Keegan Campbell <me@kfcampbell.com>

k24dizzle commented Apr 18, 2022

View reviewed changes

This was referenced Apr 18, 2022

provider wants to endlessly change the branches attribute of github_repository resource when repo is archived #1037

Closed

Add ability to opt out of listing branches in github_repository #1010

Open

k24dizzle added 2 commits May 31, 2022 11:29

Refactor out branches attribute from resource_github_repository and d…

9f1c430

…ata_source_github_repository to data_source_github_repository_branches

Add link to docs

468cfc3

k24dizzle force-pushed the branches branch from 3ac24f7 to 468cfc3 Compare May 31, 2022 18:29

kfcampbell added this to the v4.27.0 milestone Jun 10, 2022

mareksapota-lyft reviewed Jun 21, 2022

View reviewed changes

github/data_source_github_repository_branches.go Outdated Show resolved Hide resolved

k24dizzle and others added 4 commits June 22, 2022 11:46

Update github/data_source_github_repository_branches.go

00aa131

Co-authored-by: mareksapota-lyft <msapota@lyft.com>

Add only_protected_branches in data source github_repository (integra…

1bd34d6

…tions#1162) This will allow to retrieve only the protected branches instead of the first 30. Co-authored-by: Keegan Campbell <me@kfcampbell.com>

Remove mistakenly committed binary (integrations#1223)

5f1a989

Revert PR integrations#1162 (integrations#1224)

ce4c046

Merge branch 'main' into branches

026fe7f

kfcampbell force-pushed the main branch from ce4c046 to 754d73f Compare July 27, 2022 23:48

Merge branch 'main' into branches

5534c8a

Merge branch 'main' into branches

f857f8f

mareksapota-lyft reviewed Sep 1, 2022

View reviewed changes

k24dizzle added 2 commits September 1, 2022 16:01

Merge branch 'integrations:main' into branches

4bb8eec

marek's suggestion + use v47

2b683f6

Merge branch 'main' into branches

d3aaa76

kfcampbell merged commit dbda9b9 into integrations:main Sep 6, 2022

kfcampbell mentioned this pull request Sep 14, 2022

Bump version number to v5 #1288

Merged

mengesb mentioned this pull request Sep 15, 2022

Bump integrations/github from 4.29.0 to 5.0.0 mengesb/terraform-github-repository#31

Merged

bpaquet mentioned this pull request Sep 20, 2022

Allow to get only protected on non protected branch with github_repository_branches #1296

Merged

maroux mentioned this pull request Oct 26, 2022

Upgrading to v5 is not seamless #1343

Closed

jvandijk mentioned this pull request Nov 1, 2022

feat: add v4 branch protections mineiros-io/terraform-github-repository#128

Merged

rbryett pushed a commit to dwp/dataworks-github-config that referenced this pull request Feb 21, 2023

DTWRKS-854 ignore branch changes (see integrations/terraform-provider…

97cf99a

…-github#1117)

rbryett added a commit to dwp/dataworks-github-config that referenced this pull request Feb 21, 2023

DTWRKS-854 ignore branch changes (see integrations/terraform-provider…

8e94c1c

…-github#1117) (#479) Co-authored-by: rorybryett <rory.bryett@dwp.gov.uk>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor out branches attribute to new data source #1117

Refactor out branches attribute to new data source #1117

k24dizzle commented Apr 18, 2022 •

edited

Loading

k24dizzle Apr 18, 2022

k24dizzle commented May 3, 2022

k24dizzle commented May 31, 2022

kfcampbell commented Jun 10, 2022

eriksw commented Jul 20, 2022

DoctorPolski commented Aug 5, 2022

kfcampbell commented Aug 27, 2022

mareksapota-lyft Sep 1, 2022

k24dizzle Sep 1, 2022

mareksapota-lyft commented Sep 1, 2022

mareksapota-lyft commented Sep 1, 2022

k24dizzle commented Sep 1, 2022

mareksapota-lyft commented Sep 1, 2022

	func TestAccGithubBranchesDataSource(t *testing.T) {
	func TestAccGithubRepositoryBranchesDataSource(t *testing.T) {

Refactor out branches attribute to new data source #1117

Refactor out branches attribute to new data source #1117

Conversation

k24dizzle commented Apr 18, 2022 • edited Loading

k24dizzle Apr 18, 2022

Choose a reason for hiding this comment

k24dizzle commented May 3, 2022

k24dizzle commented May 31, 2022

kfcampbell commented Jun 10, 2022

eriksw commented Jul 20, 2022

DoctorPolski commented Aug 5, 2022

kfcampbell commented Aug 27, 2022

mareksapota-lyft Sep 1, 2022

Choose a reason for hiding this comment

k24dizzle Sep 1, 2022

Choose a reason for hiding this comment

mareksapota-lyft commented Sep 1, 2022

mareksapota-lyft commented Sep 1, 2022

k24dizzle commented Sep 1, 2022

mareksapota-lyft commented Sep 1, 2022

k24dizzle commented Apr 18, 2022 •

edited

Loading