Graphql / Uplift branch_protection resource by patrickmarabeas · Pull Request #337 · integrations/terraform-provider-github

patrickmarabeas · 2020-01-31T00:11:27Z

Uplifts the branch_protection resource to use v4 API.

API surface has changed slightly.

Migration function only worries about:

migration of id from repository name to the current branch protection's global object ID
branch -> pattern

notes:

This branch will be rebased as necessary
Shared consts and functions will be abstracted int util_* files as required, I've simply included them within the one file for simplicity for the time being
This constitutes a preliminary migration to the v4 API
Tests will be written / updated once the API surface is nailed down

patrickmarabeas · 2020-02-10T03:35:53Z

Unfortunately it seems as though v3 has continued to provide some newer features - see #305. I'll do some testing to see whether conditionally updating branch protection with the v3 API if the branch exists is a feasible solution.

twexler · 2020-04-02T13:22:45Z

Looks like this one kind of died, any chance we could revive it? Could really use the ability to specify patterns for branch protections!

Willing to help if needed.

patrickmarabeas · 2020-04-02T14:48:46Z

@twexler Still alive. There's currently discussion on how to roll v4 resources into this provider.

If you are wanting something now - have a look at https://github.com/patrickmarabeas/terraform-provider-github-v4 I'm using this as a test bed for v4 resources. Documentation is certainly lacking, but happy to add to it if questions are raised.

twexler · 2020-04-02T19:26:32Z

Not super urgent, just a nice thing to have! I saw the discussion in #83 after I wrote that message.

I'd be happy to help with docs or anything else that is needed on the v4 transition!

jcudit · 2020-04-24T14:06:39Z

@patrickmarabeas @twexler -

I'm hoping to cut a new release with the incremental GraphQL changes accumulated so far.

What are thoughts around deferring this uplift to another release? I am unclear on the confidence of what is outlined in this PR, but happy to resolve conflicts and merge if this approach is still favoured.

patrickmarabeas · 2020-04-24T15:13:48Z

Yes, further work is required on this PR.

wuurrd · 2020-05-14T15:30:21Z

👍 this PR is extremely useful to fill in the gaps of missing v3 API stuff

patrickmarabeas · 2020-05-16T11:19:28Z

Apologies, haven't had the time recently to focus on getting this squared away.

patrickmarabeas · 2020-06-16T05:27:29Z

Uplifting the tests at the moment, but it would be good to have a review on the changes to the parameters and what not.

There is no available v3 API endpoints to add the missing functionality (required_linear_history, allow_force_pushes, allow_deletions ), that doesn't also require you to provide ALL the fields (https://developer.github.com/v3/repos/branches/#update-branch-protection) which simply doesn't align with v4 (actor_ids vs apps, teams, users for instance).

CC @jcudit @anGie44

patrickmarabeas · 2020-08-31T03:34:26Z

I've pulled the uplifted resource into its own files to avoid merge hell, and commented out the legacy files. This can be corrected when it's ready to be merged.

* Uplift branch_protection resource to use v4 API * Add state migration for branch_protection v0 to v1 * Update test for branch_protection resource * Update README for branch_protection resource

jcudit · 2020-09-06T12:15:10Z

🥇 for patience on seeing this one through. I'll merge this shortly and plan to follow on with a few commits to:

update the tests to run with the latest test suite
rename the branch_protection_v4 resources back to branch_protection

patrickmarabeas · 2020-09-09T00:23:04Z

Awesome, great to have this in!

Just an FYI for the future, you can squash fixup! commits by way of: git rebase --autosquash --interactive master.

/cc https://github.com/terraform-providers/terraform-provider-github/pull/337

xorima · 2020-10-13T17:26:15Z

Why is this change not in the release notes as breaking change or updated in the docs?

lsowen · 2020-10-15T20:31:01Z

@patrickmarabeas in v3, you could specify apps for push restrictions. It doesn't look like it is possible to get the node_id for an app for use with v4?

Krenair · 2020-10-16T15:36:13Z

Yeah, this being listed as an enhancement for 3.1.0 instead of a breaking change seems like a mistake?

usmonster · 2021-05-24T19:27:28Z

github/util_v4_branch_protection.go

+			pushActors = append(pushActors, a.Actor.Team.ID.(string))
+		}
+		if a.Actor.User != (Actor{}) {
+			pushActors = append(pushActors, a.Actor.Team.ID.(string))


Is this a bug? Should it instead be:

Suggested change

pushActors = append(pushActors, a.Actor.Team.ID.(string))

pushActors = append(pushActors, a.Actor.User.ID.(string))

?

Will be rectified in #819

I've just seen your comment—there's a more complete fix (since the copy-paste error propagated) in #824.

This resolves issue caused by moving to use the v4 API for this resouce, as part of provider version: 3.1.0 Issue: integrations/terraform-provider-github#908 Provider bump: integrations/terraform-provider-github#337 Signed-off-by: Stefan Wessels Beljaars <swesselsbeljaars@schubergphilis.com>

ghost added the size/XXL label Jan 31, 2020

This was referenced Feb 10, 2020

github_branch_protection requires branch to exist #164

Closed

Branch protection for a branch that does not yet exist or regex pattern #167

Closed

Switch to API v4 (GraphQL) #83

Closed

patrickmarabeas force-pushed the graphql/r-protection branch from 3d00be0 to db941c0 Compare February 10, 2020 05:27

jcudit added this to the v2.7.0 milestone Apr 3, 2020

jcudit modified the milestones: v2.7.0, v2.8.0 Apr 24, 2020

patrickmarabeas force-pushed the graphql/r-protection branch from db941c0 to d0f95ec Compare May 2, 2020 11:24

anGie44 modified the milestones: v2.8.0, v2.8.1 May 15, 2020

jcudit modified the milestones: v2.9.0, v2.10.0 Jun 3, 2020

patrickmarabeas force-pushed the graphql/r-protection branch 2 times, most recently from 8430e4b to 2e39980 Compare June 16, 2020 05:21

ghost added the Type: Documentation Improvements or additions to documentation label Jun 16, 2020

jcudit modified the milestones: v2.10.0, v3.1.0 Jul 10, 2020

patrickmarabeas force-pushed the graphql/r-protection branch from 2e39980 to 5ccc094 Compare August 17, 2020 03:56

patrickmarabeas force-pushed the graphql/r-protection branch from 5ccc094 to 237d79f Compare August 31, 2020 03:32

patrickmarabeas added 2 commits August 31, 2020 14:00

Uplift branch_protection resource to use v4 API

8091d19

* Uplift branch_protection resource to use v4 API * Add state migration for branch_protection v0 to v1 * Update test for branch_protection resource * Update README for branch_protection resource

Comment out legacy branch_protection

2d1e571

patrickmarabeas force-pushed the graphql/r-protection branch from 8146cda to 2d1e571 Compare August 31, 2020 04:00

patrickmarabeas added 3 commits August 31, 2020 14:09

fixup! Uplift branch_protection resource to use v4 API

e1752ba

fixup! Uplift branch_protection resource to use v4 API

abfa982

fixup! Uplift branch_protection resource to use v4 API

7eaea4c

jcudit approved these changes Sep 6, 2020

View reviewed changes

jcudit merged commit 3bc761a into integrations:master Sep 6, 2020

jcudit referenced this pull request in terraformtesting/terraform-provider-github Sep 13, 2020

Re-Instate Branch Protection To Original Name

63b10ff

/cc https://github.com/terraform-providers/terraform-provider-github/pull/337

jcudit referenced this pull request in terraformtesting/terraform-provider-github Sep 13, 2020

Re-Instate Branch Protection To Original Name

267e5a0

/cc https://github.com/terraform-providers/terraform-provider-github/pull/337

tru2dagame mentioned this pull request Sep 16, 2020

How to force the github provider to use v4 graphql api #551

Closed

SanderKnape mentioned this pull request Oct 13, 2020

v3.1.0 severily slows down the github_branch_protection resource #565

Closed

xorima mentioned this pull request Oct 13, 2020

3.1.0 has breaking changes which are not identified in changelog #566

Closed

woodb mentioned this pull request Oct 15, 2020

Fix documentation for dismissal_restrictions #569

Merged

jcudit mentioned this pull request Nov 1, 2020

Revert github_branch_protection GraphQL Implementation #582

Closed

usmonster reviewed May 24, 2021

View reviewed changes

stefanwb mentioned this pull request Sep 24, 2021

Use repository name as the repository_id for branch protection schubergphilis/terraform-github-mcaf-repository#36

Merged

	pushActors = append(pushActors, a.Actor.Team.ID.(string))
	pushActors = append(pushActors, a.Actor.User.ID.(string))

Conversation

patrickmarabeas commented Jan 31, 2020

Uh oh!

patrickmarabeas commented Feb 10, 2020

Uh oh!

twexler commented Apr 2, 2020

Uh oh!

patrickmarabeas commented Apr 2, 2020

Uh oh!

twexler commented Apr 2, 2020

Uh oh!

jcudit commented Apr 24, 2020

Uh oh!

patrickmarabeas commented Apr 24, 2020

Uh oh!

wuurrd commented May 14, 2020

Uh oh!

patrickmarabeas commented May 16, 2020

Uh oh!

patrickmarabeas commented Jun 16, 2020

Uh oh!

patrickmarabeas commented Aug 31, 2020

Uh oh!

jcudit commented Sep 6, 2020

Uh oh!

patrickmarabeas commented Sep 9, 2020

Uh oh!

xorima commented Oct 13, 2020

Uh oh!

lsowen commented Oct 15, 2020

Uh oh!

Krenair commented Oct 16, 2020

Uh oh!

usmonster May 24, 2021

Choose a reason for hiding this comment

Uh oh!

jarylc Jun 15, 2021

Choose a reason for hiding this comment

Uh oh!

usmonster Jun 17, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants