Add support for tag-based environment deployment branch policy

sumnerwarren commented 6 months ago

Resolves #1974. Supersedes #2050.

Before the change?

The github_repository_environment_deployment_policy only supported branch-based policies even though the GitHub API has support for both branch-based and tag-based policies.

After the change?

A tag_pattern attribute has been added to the github_repository_environment_deployment_policy resource.

Pull request checklist

[x] Tests for the changes have been added (for bug fixes / features)
[x] Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

[ ] Yes
[x] No

Because the update API does not support pattern type (only name), I attempted to make switching between branch_pattern and tag_pattern require a new resource. Not entirely sure I did that or tested it well, so would appreciate particular attention on that aspect of this PR.

kfcampbell commented 5 months ago

Tests are all passing for me. The chosen branch_pattern vs. tag_pattern choice seems reasonable to me. Can you explain more about

I attempted to make switching between branch_pattern and tag_pattern require a new resource

It doesn't seem to me like a new resource in Terraform is created here; the same github_repository_deployment_branch_policy is used.

One other reasonable option might be to create a new resource, something like github_repository_deployment_tag_policy rather than use the branch policy one for tags.

sumnerwarren commented 5 months ago

Can you explain more about

The update endpoint for deployment branch policies does not support changing from a branch pattern to a tag pattern or vice versa; it only supports pattern changes within the pattern type that was chosen at creation time. So if terraform config changes like this:

 resource "github_repository_environment_deployment_policy" "test" {
    repository     = github_repository.test.name
    environment    = github_repository_environment.test.environment
-   branch_pattern = "release/*"
+   tag_pattern    = "release/*"
 }

then this plugin marks the resource with the ForceNew attribute so it will be recreated.

These two tests: (1, 2) were designed to ensure that a new resource is created by comparing the unique ids of the created and updated policies. This is in contrast to these two tests (1, 2) which confirm the policy id does not change when only the pattern changes.

A simpler alternative is to always, overeagerly recreate environment deployment policies when their configuration changes.

One other reasonable option might be to create a new resource, something like github_repository_deployment_tag_policy rather than use the branch policy one for tags.

GitHub only uses one term (deployment branch policy) for both branch-based and tag-based policies and I think there's some advantage in matching what they're doing. That said, I don't think the naming used for these resources is perfect. Right now we have both: github_repository_deployment_branch_policy and github_repository_environment_deployment_policy. This PR mostly updates the latter. I'm not sure what the history is around having two resource types that seem to represent the same thing in GitHub; maybe the former was not removed simply for backward compatibility? I would support combining and renaming these to better match what GitHub calls them, but that seems like a bigger conversation.