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

Merged

Conversation

sumnerwarren
Copy link
Contributor

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

@bradam12
Copy link
Contributor

bradam12 commented Jun 1, 2024

Code looks good to me. Would love to see this added soon as I'm setting envs up in GHES.

@kfcampbell @nickfloyd

@av-andrii-ievtukhov
Copy link

Do you know if there's any progress on this? ))

@aatzer
Copy link

aatzer commented Aug 27, 2024

Is there any progress on this?

@avivek
Copy link

avivek commented Sep 16, 2024

This is something that we are waiting for.
Would be great to get this in.

@CarlosEspejo
Copy link

In the mean time I am using this work around that leverages the GitHub CLI.
In my module I created bin/create_tag_policy.sh script with

#!/bin/bash

# Variables
ORG=[replace-me-with-your-org]
REPO=$1
ENVIRONMENT=$2
TAG_PATTERN=$3

# Create or update the environment with the tag pattern
gh api \
  -X POST \
  -H "Accept: application/vnd.github.v3+json" \
  "/repos/$ORG/$REPO/environments/$ENVIRONMENT/deployment-branch-policies" \
  -f "name=$TAG_PATTERN" \
  -f "type=tag"

echo "Environment '$ENVIRONMENT' in repository '$REPO' has been configured with tag pattern '$TAG_PATTERN'."

Next I call this script using the local provisioner

resource "github_repository_environment" "production" {
  count               = var.enable_production ? 1 : 0
  environment         = var.production_env_name
  repository          = var.repository_name
  prevent_self_review = true
  can_admins_bypass   = false

  reviewers {
    teams = [
      data.github_team.devops.id,
    ]
  }

  deployment_branch_policy {
    protected_branches     = false
    custom_branch_policies = true
  }

  ## Work around until Tag based rules are merged into the GitHub provider
  ## https://github.com/integrations/terraform-provider-github/pull/2165
  provisioner "local-exec" {
    command = "./bin/create_tag_policy.sh ${var.repository_name} ${var.production_env_name} ${local.production_tag_pattern}"
  }
}

Once it executes you will end up with:

Screenshot 2024-09-22 at 3 18 30 AM

Copy link
Member

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

There's an obnoxious issue with the repository resource's vulnerability alerts that was breaking the tests. Adding the line ignore_vulnerability_alerts_during_read = true to the repository's test schema fixes it. Now the old and newly-added tests are all passing. Thank you for the contributions!

@kfcampbell kfcampbell merged commit 84c6bd2 into integrations:main Nov 21, 2024
1 check passed
MXfive pushed a commit to MXfive/terraform-provider-github that referenced this pull request Nov 27, 2024
…rations#2165)

* Add Type field to DeploymentBranchPolicyRequest struct

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

* Change name of branch_pattern argument to pattern for github_repository_environment_deployment_policy resource

* Add type argument to github_repository_environment_deployment_policy resource

* Add tag-based test for github_repository_environment_deployment_policy

* Revert "Add tag-based test for github_repository_environment_deployment_policy"

This reverts commit 88b1369.

* Revert "Add type argument to github_repository_environment_deployment_policy resource"

This reverts commit a534219.

* Revert "Change name of branch_pattern argument to pattern for github_repository_environment_deployment_policy resource"

This reverts commit 029960b.

* Revert "Add Type field to DeploymentBranchPolicyRequest struct"

This reverts commit af308a1.

* Add tag_pattern attribute to github_repository_environment_deployment_policy resource

* Correct typo

See integrations#2050 (comment).

* Remove type parameter from deployment policy update

* Add pattern assertions to existing tests

* Fix pattern read to address existing tag policy test

* Fix update to read the configured pattern

* Force new resource when pattern type changes

* Fix tests by ignoring vulnerability_alerts

---------

Co-authored-by: Peter McEvoy <git@mcevoypeter.com>
Co-authored-by: Keegan Campbell <me@kfcampbell.com>
Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com>
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