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

Support "internal" repository types #304

Closed
mindfulmonk opened this issue Dec 1, 2019 · 18 comments · Fixed by #441 or #454
Closed

Support "internal" repository types #304

mindfulmonk opened this issue Dec 1, 2019 · 18 comments · Fixed by #441 or #454
Labels
Good first issue Good for newcomers size/S Status: Up for grabs Issues that are ready to be worked on by anyone Type: Feature New feature or request
Milestone

Comments

@mindfulmonk
Copy link

Hi,

Currently the provider only supports private = true, but there is now a new type called internal.
I think this should now be switched to a new variable called visibility with three possible values public,private and internal

References:
https://www.terraform.io/docs/providers/github/r/repository.html#private
https://help.github.com/en/github/creating-cloning-and-archiving-repositories/creating-an-internal-repository

@alexbde
Copy link

alexbde commented Jan 3, 2020

Any update on this? We are really looking forward to this.

Details on API: https://developer.github.com/v3/repos/#parameters-4

@jcudit jcudit added Type: Feature New feature or request Good first issue Good for newcomers size/S labels Jan 26, 2020
@vikkyomkar
Copy link
Contributor

@jcudit I would like to work on this issue

@jcudit
Copy link
Contributor

jcudit commented Jan 28, 2020

@vikkyomkar that is great to hear. I can help with breaking down the work if needed as well as validating acceptance tests pass once a PR is up for review.

Please mention me if there is any support needed while you work through this. Thanks again for volunteering!

@benj-fletch
Copy link
Contributor

benj-fletch commented Feb 12, 2020

Hi @vikkyomkar / @jcudit - is this actively being worked on still? I would also like to work on this issue if you are not actively working on it? Thanks

@sheldonhull
Copy link

I recently adapted my team for using github to manage our repos, and found it also missing the internal setting. Only adding comment instead of a plus + here to make sure it's bumped in attention. Following

@jcudit
Copy link
Contributor

jcudit commented Mar 27, 2020

Thanks for the bump. I am in favour of opening this one up to another collaborator if @vikkyomkar has no objections within a week.

@jcudit jcudit added the Status: Up for grabs Issues that are ready to be worked on by anyone label Mar 27, 2020
@jcudit jcudit added this to the v2.6.0 milestone Mar 27, 2020
@sheldonhull
Copy link

Github Documentation indicates the valid types are:

Can be one of all, public, private, forks, sources, member, internal. Default: all. If your organization is associated with an enterprise account using GitHub Enterprise Cloud or GitHub Enterprise Server 2.20+, type can also be internal.

I'd love to contribute if this is straight forward (newer to Go). To your knowledge, is it simply added a new valid value type for internal, or are there more complex requirements that change based on an internal flag (such as need to lookup an organization, even though it's the provided as the "Owner" for the repo)?

If it's simply adding a new value then I'd be willing to dig in and take a first swing at this, just wanted to ask so I don't spin my wheels on something that has more complex requirements I'm not aware of.

@jcudit jcudit modified the milestones: v2.6.0, v2.7.0 Apr 2, 2020
@jcudit
Copy link
Contributor

jcudit commented Apr 2, 2020

@sheldonhull I agree with @mindfulmonk that we should add a new visibility parameter, leaving the internal parameter intact. This aligns our interface with that of the underlying go-github client library and ultimately the GitHub API.

We would then have to document and write acceptance test cases to ensure the new parameter behaves as expected alongside the existing internal parameter:

The visibility parameter overrides the private parameter when you use both parameter


Let us know if you're interested in claiming this one. If not, other contributors should feel free to claim it by posting to the thread.

@patrickmarabeas
Copy link
Contributor

Got this feature merged into (google/go-github#1471) last week and it's been released in v30.1.0 - so don't forget to update the go-github dependency.

@jcudit jcudit modified the milestones: v2.7.0, v2.8.0 Apr 3, 2020
@sheldonhull
Copy link

How do effectively track this to know when it's released in terraform-provider-github? Is it when the issue is closed?

@patrickmarabeas
Copy link
Contributor

@sheldonhull releases are done on request. New versions are tagged on master - so anything merged before that point.

@gesellix
Copy link
Contributor

Related: PR #424 makes the dependency update for go-github.

@patrickmarabeas
Copy link
Contributor

Looks like no one is claiming this @jcudit - happy to do so

patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 1, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 1, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

Resolves integrations#304
@patrickmarabeas
Copy link
Contributor

Added the functionality in: #441. This should be rebased once #424 is merged.

@anGie44 anGie44 linked a pull request May 1, 2020 that will close this issue
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 2, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 2, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 6, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 6, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 8, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 8, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 8, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
@anGie44 anGie44 reopened this May 10, 2020
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 11, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 11, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 12, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

The endpoint is a little sketchy:
* an error if trying to update with a visibility value that is the same
* auth issues are encountered if trying to create with a visibility
  value. The parameter needs to be ignored during create, and updated
  subsequently.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 12, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 13, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue May 13, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

The endpoint is a little sketchy:
* an error if trying to update with a visibility value that is the same
* auth issues are encountered if trying to create with a visibility
  value. The parameter needs to be ignored during create, and updated
  subsequently.

Resolves integrations#304
@anGie44 anGie44 modified the milestones: v2.8.0, v2.8.1 May 15, 2020
@grispin
Copy link

grispin commented May 19, 2020

I saw @patrickmarabeas changes were removed from v2.8.0 last week. When can will a release be done with internal support included?

@gesellix
Copy link
Contributor

@grispin see https://github.com/terraform-providers/terraform-provider-github/milestone/8 with PR #454.

patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue Jun 5, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue Jun 5, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

The endpoint is a little sketchy:
* an error if trying to update with a visibility value that is the same
* auth issues are encountered if trying to create with a visibility
  value. The parameter needs to be ignored during create, and updated
  subsequently.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue Jun 25, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
patrickmarabeas added a commit to patrickmarabeas/terraform-provider-github that referenced this issue Jun 25, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

The endpoint is a little sketchy:
* an error if trying to update with a visibility value that is the same
* auth issues are encountered if trying to create with a visibility
  value. The parameter needs to be ignored during create, and updated
  subsequently.

Resolves integrations#304
jcudit pushed a commit that referenced this issue Jun 25, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves #304
jcudit pushed a commit that referenced this issue Jun 25, 2020
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

The endpoint is a little sketchy:
* an error if trying to update with a visibility value that is the same
* auth issues are encountered if trying to create with a visibility
  value. The parameter needs to be ignored during create, and updated
  subsequently.

Resolves #304
@sheldonhull
Copy link

When reviewing the commit 3a52bc6 itself, I see it tagged with 2.9.0,2.9.1,2.9.2

What should I use to know? Can anyone give me a quick idea of how to verify this commit is actually in a release?

@match-caseydaniell
Copy link

When reviewing the commit 3a52bc6 itself, I see it tagged with 2.9.0,2.9.1,2.9.2

What should I use to know? Can anyone give me a quick idea of how to verify this commit is actually in a release?

I am setting my version locally to 2.9.0, it seems like a regression occurred in the 2.9.x series and broke the internal repo visibility and a few other settings. I haven't revised with the latest release in a while.

kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this issue Jul 26, 2022
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

Resolves integrations#304
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this issue Jul 26, 2022
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this issue Jul 26, 2022
Adds the additional visibility parameter allowing for enterprise
accounts to have set the repository to be only internally visible.

Resolves integrations#304
kfcampbell pushed a commit to kfcampbell/terraform-provider-github that referenced this issue Jul 26, 2022
Adds the additional visibility parameter allowing for enterprise
accounts to set the repository to be only internally visible.

The endpoint is a little sketchy:
* an error if trying to update with a visibility value that is the same
* auth issues are encountered if trying to create with a visibility
  value. The parameter needs to be ignored during create, and updated
  subsequently.

Resolves integrations#304
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Good for newcomers size/S Status: Up for grabs Issues that are ready to be worked on by anyone Type: Feature New feature or request
Projects
None yet