Skip to content

feat(github_team_repository): allow for custom repository roles#1038

Merged
kfcampbell merged 4 commits intomainfrom
unknown repository
May 27, 2022
Merged

feat(github_team_repository): allow for custom repository roles#1038
kfcampbell merged 4 commits intomainfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Jan 17, 2022

Allow custom repository roles to be passed to the github_team_repository resource by removing the string validation on the permission argument.

Update the default permission to push, which is the default for the newer API resource; previously the API pulled the default from the (deprecated) permission parameter of the team resource. This may cause an unintended change for some provider users, so I'll leave this up for the maintainers to decide whether to include or not.

I also opted to leave the tests unchanged as "pull", "triage", "push", "maintain", "admin" are still valid test cases.

Closes: #988

@kfcampbell kfcampbell added this to the v5.0.0 milestone Feb 2, 2022
Copy link
Copy Markdown
Contributor

@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 like the idea of our default matching the API. Perhaps this can land in the next major version update.

Type: schema.TypeString,
Optional: true,
Default: "pull",
ValidateFunc: validateValueFunc([]string{"pull", "triage", "push", "maintain", "admin"}),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the reason for removing the validation to allow all custom role names?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah, that's it. GitHub itself would handle whether the role exists going forward

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a way to pull all possible roles from GitHub and then validate against that?

@kfcampbell kfcampbell self-requested a review February 2, 2022 02:50
Co-authored-by: Keegan Campbell <me@kfcampbell.com>
@ghost
Copy link
Copy Markdown
Author

ghost commented Mar 8, 2022

@kfcampbell is there a date for v5.0.0? We're waiting to use the functionality this PR brings 😅

Does removing the change to the default permission allow it to be moved to the next minor release instead?

@kfcampbell
Copy link
Copy Markdown
Contributor

@grant-adarga Sorry for the delay. There's currently no estimated time on the date for v5.0.0.

Does removing the change to the default permission allow it to be moved to the next minor release instead?

I think this would be reasonable! If you change that, we can merge as-is, get into an upcoming minor release, and then open up a new PR we can put in the v5.0.0 milestone.

Copy link
Copy Markdown
Contributor

@joshua-hancox joshua-hancox left a comment

Choose a reason for hiding this comment

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

I'd love to use this asap, can we merge with the old defaults?

@kfcampbell kfcampbell modified the milestones: v5.0.0, v4.25.0, v4.26.0 Apr 28, 2022
@stefansedich
Copy link
Copy Markdown

Any indication on when this one will land? we desperately need this functionality so we can assign custom roles to users.

@kfcampbell kfcampbell modified the milestones: v4.25.0, v4.26.0 May 19, 2022
Grant and others added 2 commits May 27, 2022 18:48
Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>
Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>
@ghost
Copy link
Copy Markdown
Author

ghost commented May 27, 2022

@kfcampbell this should be ready to go now I've incorporated @joshuahancox suggested changes

Copy link
Copy Markdown
Contributor

@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.

Thanks for changing the default back! I've described here more on a possible longer-term route for this.

@kfcampbell kfcampbell merged commit 366aac0 into integrations:main May 27, 2022
@ghost ghost deleted the feat/allow-custom-repository-roles branch June 17, 2022 14:42
kfcampbell added a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 26, 2022
…grations#1038)

* feat(github_team_repository): allow for custom roles to be passed to team repository permissions

* Update website/docs/r/team_repository.html.markdown

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

* Update github/resource_github_team_repository.go

Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>

* Update website/docs/r/team_repository.html.markdown

Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>
kfcampbell added a commit to kfcampbell/terraform-provider-github that referenced this pull request Jul 27, 2022
…grations#1038)

* feat(github_team_repository): allow for custom roles to be passed to team repository permissions

* Update website/docs/r/team_repository.html.markdown

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

* Update github/resource_github_team_repository.go

Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>

* Update website/docs/r/team_repository.html.markdown

Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>
kazaker pushed a commit to auto1-oss/terraform-provider-github that referenced this pull request Dec 28, 2022
…grations#1038)

* feat(github_team_repository): allow for custom roles to be passed to team repository permissions

* Update website/docs/r/team_repository.html.markdown

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

* Update github/resource_github_team_repository.go

Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>

* Update website/docs/r/team_repository.html.markdown

Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
Co-authored-by: joshuahancox <67631498+joshuahancox@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow custom role names for permission argument

4 participants