Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add visibility parameter to data source and resource #454

Merged
merged 2 commits into from
Jun 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions github/data_source_github_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ func dataSourceGithubRepository() *schema.Resource {
Type: schema.TypeBool,
Computed: true,
},
"visibility": {
Type: schema.TypeString,
Computed: true,
},
"has_issues": {
Type: schema.TypeBool,
Computed: true,
Expand Down Expand Up @@ -138,6 +142,7 @@ func dataSourceGithubRepositoryRead(d *schema.ResourceData, meta interface{}) er
d.Set("description", repo.GetDescription())
d.Set("homepage_url", repo.GetHomepage())
d.Set("private", repo.GetPrivate())
d.Set("visibility", repo.GetVisibility())
d.Set("has_issues", repo.GetHasIssues())
d.Set("has_wiki", repo.GetHasWiki())
d.Set("allow_merge_commit", repo.GetAllowMergeCommit())
Expand Down
1 change: 1 addition & 0 deletions github/data_source_github_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ func testRepoCheck() resource.TestCheckFunc {
resource.TestCheckResourceAttr("data.github_repository.test", "id", "test-repo"),
resource.TestCheckResourceAttr("data.github_repository.test", "name", "test-repo"),
resource.TestCheckResourceAttr("data.github_repository.test", "private", "false"),
resource.TestCheckResourceAttr("data.github_repository.test", "visibility", "public"),
resource.TestCheckResourceAttr("data.github_repository.test", "description", "Test description, used in GitHub Terraform provider acceptance test."),
resource.TestCheckResourceAttr("data.github_repository.test", "homepage_url", "http://www.example.com"),
resource.TestCheckResourceAttr("data.github_repository.test", "has_issues", "true"),
Expand Down
27 changes: 25 additions & 2 deletions github/resource_github_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,17 @@ func resourceGithubRepository() *schema.Resource {
Optional: true,
},
"private": {
Type: schema.TypeBool,
Optional: true,
Type: schema.TypeBool,
Computed: true, // is affected by "visibility"
Optional: true,
ConflictsWith: []string{"visibility"},
Deprecated: "use visibility instead",
},
"visibility": {
Type: schema.TypeString,
Optional: true,
Computed: true, // is affected by "private"
ValidateFunc: validation.StringInSlice([]string{"public", "private", "internal"}, false),
},
"has_issues": {
Type: schema.TypeBool,
Expand Down Expand Up @@ -178,6 +187,7 @@ func resourceGithubRepositoryObject(d *schema.ResourceData) *github.Repository {
Description: github.String(d.Get("description").(string)),
Homepage: github.String(d.Get("homepage_url").(string)),
Private: github.Bool(d.Get("private").(bool)),
Visibility: github.String(d.Get("visibility").(string)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from running the acceptance tests, it seems passing an empty string here causes a strange 403 error about not having admin rights..in this case, it may make more sense to pull out this attribute and only set it if the string's available

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aiming to perform a follow-up pass to address this. Thanks for raising this.

HasDownloads: github.Bool(d.Get("has_downloads").(bool)),
HasIssues: github.Bool(d.Get("has_issues").(bool)),
HasProjects: github.Bool(d.Get("has_projects").(bool)),
Expand All @@ -204,6 +214,12 @@ func resourceGithubRepositoryCreate(d *schema.ResourceData, meta interface{}) er

repoReq := resourceGithubRepositoryObject(d)
owner := meta.(*Owner).name

// Auth issues (403 You need admin access to the organization before adding a repository to it.)
// are encountered when the resources is created with the visibility parameter. As
// resourceGithubRepositoryUpdate is called immediately after, this is subsequently corrected.
repoReq.Visibility = nil

repoName := repoReq.GetName()
ctx := context.Background()

Expand Down Expand Up @@ -300,6 +316,7 @@ func resourceGithubRepositoryRead(d *schema.ResourceData, meta interface{}) erro
d.Set("description", repo.GetDescription())
d.Set("homepage_url", repo.GetHomepage())
d.Set("private", repo.GetPrivate())
d.Set("visibility", repo.GetVisibility())
Copy link
Contributor

@anGie44 anGie44 May 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in cases where visibility isn't provided, looks like we'll be left with a non-empty plan as the API returns one of the 3 visibility type values. we can account for this if we make the param also Computed .. I was considering not setting this if the param is not available but I believe it's generally discouraged to use d.GetOk() or d.Get() to influence d.Set()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This too is queued to be addressed by a subsequent pass.

d.Set("has_issues", repo.GetHasIssues())
d.Set("has_projects", repo.GetHasProjects())
d.Set("has_wiki", repo.GetHasWiki())
Expand Down Expand Up @@ -338,6 +355,12 @@ func resourceGithubRepositoryUpdate(d *schema.ResourceData, meta interface{}) er
client := meta.(*Owner).v3client

repoReq := resourceGithubRepositoryObject(d)

// The endpoint will throw an error if trying to PATCH with a visibility value that is the same
if !d.HasChange("visibility") {
repoReq.Visibility = nil
}

// Can only set `default_branch` on an already created repository with the target branches ref already in-place
if v, ok := d.GetOk("default_branch"); ok {
branch := v.(string)
Expand Down
29 changes: 22 additions & 7 deletions github/resource_github_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ func TestAccGithubRepository_basic(t *testing.T) {
HasProjects: false,
DefaultBranch: "master",
Archived: false,
Visibility: "public",
}),
),
},
Expand All @@ -100,6 +101,7 @@ func TestAccGithubRepository_basic(t *testing.T) {
DefaultBranch: "master",
HasProjects: false,
Archived: false,
Visibility: "public",
}),
),
},
Expand Down Expand Up @@ -144,6 +146,7 @@ func TestAccGithubRepository_archive(t *testing.T) {
HasDownloads: true,
DefaultBranch: "master",
Archived: true,
Visibility: "public",
}),
),
},
Expand Down Expand Up @@ -188,6 +191,7 @@ func TestAccGithubRepository_archiveUpdate(t *testing.T) {
HasDownloads: true,
DefaultBranch: "master",
Archived: false,
Visibility: "public",
}),
),
},
Expand All @@ -207,6 +211,7 @@ func TestAccGithubRepository_archiveUpdate(t *testing.T) {
HasDownloads: true,
DefaultBranch: "master",
Archived: true,
Visibility: "public",
}),
),
},
Expand Down Expand Up @@ -273,6 +278,7 @@ func TestAccGithubRepository_defaultBranch(t *testing.T) {
HasDownloads: true,
DefaultBranch: "master",
Archived: false,
Visibility: "public",
}),
),
},
Expand All @@ -298,6 +304,7 @@ func TestAccGithubRepository_defaultBranch(t *testing.T) {
HasDownloads: true,
DefaultBranch: "foo",
Archived: false,
Visibility: "public",
}),
),
},
Expand Down Expand Up @@ -345,6 +352,7 @@ func TestAccGithubRepository_templates(t *testing.T) {
LicenseTemplate: "ms-pl",
GitignoreTemplate: "C++",
Archived: false,
Visibility: "public",
}),
),
},
Expand Down Expand Up @@ -397,6 +405,7 @@ func TestAccGithubRepository_topics(t *testing.T) {

// non-zero defaults
DefaultBranch: "master",
Visibility: "public",
AllowMergeCommit: true,
AllowSquashMerge: true,
AllowRebaseMerge: true,
Expand All @@ -415,6 +424,7 @@ func TestAccGithubRepository_topics(t *testing.T) {

// non-zero defaults
DefaultBranch: "master",
Visibility: "public",
AllowMergeCommit: true,
AllowSquashMerge: true,
AllowRebaseMerge: true,
Expand All @@ -433,6 +443,7 @@ func TestAccGithubRepository_topics(t *testing.T) {

// non-zero defaults
DefaultBranch: "master",
Visibility: "public",
AllowMergeCommit: true,
AllowSquashMerge: true,
AllowRebaseMerge: true,
Expand Down Expand Up @@ -520,6 +531,7 @@ type testAccGithubRepositoryExpectedAttributes struct {
Description string
Homepage string
Private bool
Visibility string
HasDownloads bool
HasIssues bool
HasProjects bool
Expand Down Expand Up @@ -552,6 +564,9 @@ func testAccCheckGithubRepositoryAttributes(repo *github.Repository, want *testA
if private := repo.GetPrivate(); private != want.Private {
return fmt.Errorf("got private %#v; want %#v", private, want.Private)
}
if visibility := repo.GetVisibility(); visibility != want.Visibility {
return fmt.Errorf("got visibility %#v; want %#v", visibility, want.Visibility)
}
if hasIssues := repo.GetHasIssues(); hasIssues != want.HasIssues {
return fmt.Errorf("got has issues %#v; want %#v", hasIssues, want.HasIssues)
}
Expand Down Expand Up @@ -714,7 +729,7 @@ resource "github_repository" "foo" {

# So that acceptance tests can be run in a github organization
# with no billing
private = false
visibility = "public"

has_issues = true
has_wiki = true
Expand Down Expand Up @@ -746,7 +761,7 @@ resource "github_repository" "foo" {

# So that acceptance tests can be run in a github organization
# with no billing
private = false
visibility = "public"

has_issues = false
has_wiki = false
Expand All @@ -768,7 +783,7 @@ resource "github_repository" "foo" {

# So that acceptance tests can be run in a github organization
# with no billing
private = false
visibility = "public"

has_issues = true
has_wiki = true
Expand All @@ -790,7 +805,7 @@ resource "github_repository" "foo" {

# So that acceptance tests can be run in a github organization
# with no billing
private = false
visibility = "public"

has_issues = true
has_wiki = true
Expand All @@ -813,7 +828,7 @@ resource "github_repository" "foo" {

# So that acceptance tests can be run in a github organization
# with no billing
private = false
visibility = "public"

has_issues = true
has_wiki = true
Expand All @@ -836,7 +851,7 @@ resource "github_repository" "foo" {

# So that acceptance tests can be run in a github organization
# with no billing
private = false
visibility = "public"

has_issues = true
has_wiki = true
Expand Down Expand Up @@ -867,7 +882,7 @@ resource "github_repository" "foo" {

# So that acceptance tests can be run in a github organization
# with no billing
private = false
visibility = "public"

has_issues = true
has_wiki = true
Expand Down
2 changes: 2 additions & 0 deletions website/docs/d/repository.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ The following arguments are supported:

* `private` - Whether the repository is private.

* `visibility` - Whether the repository is public, private or internal.

* `has_issues` - Whether the repository has GitHub Issues enabled.

* `has_projects` - Whether the repository has the GitHub Projects enabled.
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/repository.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ The following arguments are supported:

* `private` - (Optional) Set to `true` to create a private repository.
Repositories are created as public (e.g. open source) by default.

* `visibility` - (Optional) 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`. The `visibility` parameter overrides the `private` parameter.

* `has_issues` - (Optional) Set to `true` to enable the GitHub Issues features
on the repository.
Expand Down