Include repositories Info associated to the Teams

[jbenaventem]

When I get a github_team I need that the repositories that it has associated are informed. I think that It's very usual that the behaviour of the organization can be replaced with the use of the teams for some enterprise accounts, it's for that reason that I need a get the repositories information associated with the Teams.

• github/data_source_github_organization_teams.go
• github/data_source_github_team.go
• github/util_v4_teams.go

   .....
    "repositories": {
         Type:     schema.TypeList,
         Computed: true,
         Elem:     &schema.Schema{Type: schema.TypeString
    },
    ....

[jcudit]

The implementation looks solid, but the tests need a bit of fixing up. On the right track overall though!

[jcudit]

Suggested change

	config := fmt.Sprintf(`
	resource "github_repository" "repo-test" {
	name = "tf-acc-repo-%s"
	auto_init = true
	}
	resource "github_team" "team-test" {
	name = "tf-acc-test-team01"
	}
	resource "github_team_repository" "team-repo-test" {
	repository = "${github_repository.repo-test.id}"
	team_id = "${github_team.team-test.id}"
	}
	data "github_team" "example" {
	slug = "team-test-01"
	}
	output "team_repository_name" {
	value = data.github_team.example.repositories.0
	}
	output "team_repository_numbers" {
	value = data.github_team.example.repositories.#
	}
	`, randomID)
	config := fmt.Sprintf(`
	resource "github_repository" "repo-test" {
	name = "tf-acc-repo-%s"
	auto_init = true
	}
	resource "github_team" "team-test" {
	name = "tf-acc-test-team-%[1]s"
	}
	resource "github_team_repository" "team-repo-test" {
	repository = "${github_repository.repo-test.id}"
	team_id = "${github_team.team-test.id}"
	}
	data "github_team" "example" {
	slug = github_team.team-test.slug
	}
	`, randomID)

[jcudit]

Ran into some difficulty with getting tests to pass here. This suggests we remove the output blocks as they do not seem to be used and were leading to errors like:

testing.go:654: Step 0 error: Invalid attribute name: On /var/folders/t1/p9cyhv6s2wg4g8s_x6zkpz_m0000gn/T/tf-test790708991/main.tf line 26: An attribute name is required after a dot.

[jbenaventem]

It´s a good point.
I´ll try to improve the test! Thks

[jcudit]

Suggested change

	func TestAccGithubTeamRepositorty(t *testing.T) {
	func TestAccGithubTeamRepository(t *testing.T) {

[jbenaventem]

I'll change it to TestAccGithubTeamRepositories to avoid duplication with test resource_github_team_repository_test.go

[jcudit]

Suggested change

	resource.TestCheckResourceAttrSet("data.github_team.example", "name"),
	resource.TestCheckResourceAttr("data.github_team.example", "repositories.#", "1"),

A more helpful attribute to check would be the new repositories.# field that this PR adds. When running the test with TF_LOG=DEBUG, I'm seeing expected state:

data.github_team.example:
  ID = 4838313
  provider = provider.github
  description = 
  name = tf-acc-test-team-zniiu
  node_id = MDQ6VGVhbTQ4MzgzMTM=
  permission = pull
  privacy = secret
  repositories.# = 1
  repositories.0 = tf-acc-repo-zniiu
  slug = tf-acc-test-team-zniiu

However, the test fails 😕 . We should find a way to check this attribute as a next step, but I've run out of time for today.

[jbenaventem]

The final Change is to use "depends_on". Is a good point for You?

....
                data "github_team" "example" {
			depends_on = ["github_repository.test", "github_team.test", "github_team_repository.test"]
			slug = github_team.test.slug
		  }
		`, randomID)

		check := resource.ComposeAggregateTestCheckFunc(
			resource.TestCheckResourceAttr("data.github_team.example", "repositories.#", "1"),
		)

		testCase := func(t *testing.T, mode string) {
			resource.Test(t, resource.TestCase{
				PreCheck:  func() { skipUnlessMode(t, mode) },
				Providers: testAccProviders,
				Steps: []resource.TestStep{
					{
						Config:             config,
						Check:              check,
						ExpectNonEmptyPlan: true,
					},
				},
			})
		}

[jcudit]

Looks good! Suggested a docs change but the test / implementation looks ready to ship 🚀

[jcudit]

Can we get this added to website/docs/d/organization_teams.html.markdown as well?

[jbenaventem]

Done!!

[jcudit]

🙇🏾 for the contribution!

[robmoss2k]

This change has made this provider almost unusable for me. I use this data source to look up the team ID from the slug to grant them access to newly created repositories. As the three teams in question have tens of thousands of repositories, terraform now sits there spamming the API until I hit the rate limit. Would it be possible to add a boolean option to this data source stating that you don't want it to read in all the repositories?

[jbenaventem]

I think that it´s possible to include that boolean to retrieve the repositories by the team by request.
Its possible that you can give me more context to see as I can help?. If a Team that it has tens of thousands of repositories its possible that this same error you get when try get an Organization too because I suppose that an organization has ten of thousands of repositories too.

Apologies for the problems but I think that it´s important for the provider that it can get this information

[jbenaventem]

If you agree with me, we can break the data source into two distinct data sources, one back that it´ll be as the older version and the other that we can retrieve the repositories by team.

[robmoss2k]

Sorry - I've been away for a few days. I can confirm that I get the same problem with a github_organization data source. Breaking the data source into two distinct data sources, perhaps github_team and github_team_repositories, would be ideal.

[jcudit]

Happy to release this change once it lands. I agree that a flag to enable the new functionality is a good fit for everyone.

[jbenaventem]

I´ll try to contribute it to the next release.
Thx