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

Add support for tag-based environment deployment branch policy #2165

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

sumnerwarren
Copy link

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

  • Tests for the changes have been added (for bug fixes / features)
  • 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
  • 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.

mcevoypeter and others added 15 commits February 20, 2024 08:53
The Type field is only necessary when creating a deployment branch
policy. When updating a deployment branch policy, the Type field is not
needed and is therefore set to nil.

Resources:
- https://docs.github.com/en/rest/deployments/branch-policies?apiVersion=2022-11-28
…repository_environment_deployment_policy resource"

This reverts commit 029960b.
@kfcampbell
Copy link
Member

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
Copy link
Author

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.

@nickfloyd nickfloyd added the Type: Feature New feature or request label Mar 19, 2024
@nickfloyd nickfloyd self-assigned this Mar 19, 2024
@thulasirajkomminar
Copy link

Any update on this?

@the-smooth-operator
Copy link

bump

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT]: Add support for tag pattern to github_repository_environment_deployment_policy resource
7 participants