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

fix: find correct permission #1230

Closed
wants to merge 66 commits into from
Closed

fix: find correct permission #1230

wants to merge 66 commits into from

Conversation

joshua-hancox
Copy link
Contributor

@joshua-hancox joshua-hancox commented Jul 22, 2022

attempt to fix: #1192 (closed)

Closes: #1183

bpaquet and others added 6 commits July 15, 2022 14:55
This will allow to retrieve only the protected branches instead of the first 30.

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
This reverts commit ce4c046.
@SharpEdgeMarshall
Copy link
Contributor

SharpEdgeMarshall commented Jul 29, 2022

With this PR you are removing the support for custom role permissions, am I right?
We were waiting for the support of custom roles and then the PR was reverted, there's a way to make it work and keep the retrocompatibility?

kfcampbell and others added 13 commits August 10, 2022 16:31
* Switch to Go1.19

* Fix linting locally

* Update linter version
* Initial commit of EMU data source

* Set groups in state

* Some renames

* Correct google/go-github client version to v45 in new data source

* Rename data source file to something more appropriate

* Set a sensible ID

* Add documentation for new data source

* Fix website formatting
Co-authored-by: Keegan Campbell <me@kfcampbell.com>
* Add membership_type to data.github_team

* Improve doc

* Add extra integration test check

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
Signed-off-by: Tobias Gesellchen <tobias@gesellix.de>

Signed-off-by: Tobias Gesellchen <tobias@gesellix.de>
* feat(github_ip_ranges): support for web and api CIDR blocks from meta API.

* fix: Api --> API as per sdk changes. Re-adding accidentally removed Pages.

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
Co-authored-by: Keegan Campbell <me@kfcampbell.com>
* Update to latest version of github.com/golangci/golangci-lint

* Grab latest version of gopkg.in/square/go-jose.v2

* Update away from vulnerable version of github.com/hashicorp/go-getter

* Use compatible version of afero without chown method
* 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 (#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 (#1223)

* Revert PR #1162 (#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>
* [Feature] New datasource github_repository_teams

List the teams which have access to a repository

* Update sdk version

* Add .erb file

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
@SharpEdgeMarshall
Copy link
Contributor

@joshua-hancox @kfcampbell any news? this is blocking us from adopting this provider

@Luis-3M
Copy link

Luis-3M commented Sep 12, 2022

We're also interested in having this fix available - quiet confusing to see TF plans that always report changes when using custom roles.

Optional: true,
ForceNew: true,
Default: "push",
ValidateFunc: validateValueFunc([]string{"pull", "triage", "push", "maintain", "admin"}),
Copy link
Member

Choose a reason for hiding this comment

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

The addition of this ValidateFunc means that custom roles can no longer be used, correct?

@nickfloyd
Copy link
Contributor

Hey @joshua-hancox I'd be glad to resolve those merge conflicts for you to move this along if you'd like. I'll need push or maintainer access to joshua-hancox:main, though so that we can get your fork synced with this repo. Let us know if you'd like us to handle that or if you'd be up for doing it!

@joshua-hancox
Copy link
Contributor Author

@kfcampbell I don't believe that was part of my change here, sorry it's been ages since I looked at this.

@nickfloyd I doubt I have any time to look at this any time soon, I have added you as collaborator please feel free to carry on.

@SharpEdgeMarshall
Copy link
Contributor

SharpEdgeMarshall commented Nov 2, 2022

From my understanding this PR #1192 was good but didn't consider backwards compatibility.
May I propose to release a new major with those changes or add some kind of mapping to preserve old naming compatibility?
Sooner or later I think this provider should move to the new API version

kfcampbell and others added 6 commits November 4, 2022 14:11
* Add release assets

* Add assets to release documentation

* Fix typo in documentation

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
* Include All Branches in template repositories

When creating a repository from a template, you can pass in a boolean to
include all the branches in the template repo.

This is documented here: https://docs.github.com/en/repositories/creating-and-managing-repositories/creating-a-repository-from-a-template

In addition, the provider already supports that here: https://github.com/google/go-github/blob/3e8a7f0fbe0e4402f9ad5a80e0ab0a050786e0f8/github/repos.go#L441

Adding a variable under the template structure and then pass it in to
the provider function works.

Added tests to make sure it works. All tests are passing

* Remove failing test. Nothing to see here, folks

* Add clarifying doc note

Co-authored-by: KensoDev <avi@kensodev.com>
* Fix assets_url typo

* Add deprecation message

* Follow instructions more closely

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

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

I'll get this merged and released in the near future.

Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

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

Ahh oops I should've looked at the checks before approving 🤦

@joshua-hancox do you have any interest in fixing up the merge conflicts? If not, I can take a look at it at some point.

* new resource: github_actions_repository_permissions

This commit adds the following new resources for modifying Actions
permissions for a given repository:

* `github_actions_repository_permissions`

Resource is heavily modelled on the `github_actions_organization_permissions`
resource.

* Fixup the failing tests.

* Only specify `AllowedActions` if actions are enabled on repo
* Ensure that the `repository` param gets set when importing.
* Fix the logic when deleting the repository permissions.

* Fixup merge

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
@joshua-hancox
Copy link
Contributor Author

joshua-hancox commented Nov 10, 2022

Sorry @kfcampbell I don't think I will get time to look at this any time soon, if you'd like to please feel free to carry on, or I think @nickfloyd also planned to take a look at it.

@kfcampbell kfcampbell added the Status: Up for grabs Issues that are ready to be worked on by anyone label Nov 10, 2022
@kfcampbell
Copy link
Member

Sounds good! I've added the "Status: Up for grabs" label so the community can jump on the submission if anybody can get to it sooner rather than later.

Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

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

I don't feel comfortable merging this PR given the diff/changes here. The commit history includes commits that I've force-pushed away, and I'm scared of reintroducing this incident merging this PR as-is.

I'm going to close this for the time being, and would be very receptive to reviewing PRs that cherry-pick relevant commits or reimplement the functionality.

@kfcampbell
Copy link
Member

kfcampbell commented Nov 15, 2022

Oops, I accidentally "closed with comment" with a single g in the comment field. Now the body of this comment can't be blank.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Up for grabs Issues that are ready to be worked on by anyone Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

resource_github_team_repository cannot handle custom role after creation