Graphql / Uplift branch_protection resource

[patrickmarabeas]

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]

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]

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]

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

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]

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

Yes, further work is required on this PR.

[wuurrd]

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

[patrickmarabeas]

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

[patrickmarabeas]

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]

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.

[jcudit]

🥇 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]

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.

[xorima]

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

[lsowen]

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

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

[usmonster]

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))

?

[jarylc]

Will be rectified in #819

[usmonster]

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