integrations / terraform-provider-github

Terraform GitHub provider
https://www.terraform.io/docs/providers/github/
MIT License
888 stars 729 forks source link

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

Closed mcevoypeter closed 1 month ago

mcevoypeter commented 9 months ago

Resolves #1974


Before the change?

After the change?

Pull request checklist

Does this introduce a breaking change?

Please see our docs on breaking changes to help!


mcevoypeter commented 9 months ago

I switched from a pattern + type approach to a branch_pattern + tag_pattern approach. The former was a breaking change, whereas the latter is not and is also simpler for users of the provider to use correctly.

mcevoypeter commented 9 months ago

@kfcampbell, is there anything I can do to help move this along?

brandocomando commented 9 months ago

Does repository need to be updated as well? github_repository_environment Currently it has deployment_branch_policy with custom_branch_policies.

Does it also need custom_tag_policies boolean?

mcevoypeter commented 9 months ago

Does it [the github_repository_environment resource ]also need custom_tag_policies boolean?

I don't think so. The GitHub docs only lists two fields within the deployment_branch_policy object: protected_branches and custom_branch_policies.

kfcampbell commented 8 months ago

@mcevoypeter Sorry about the delay; it's been slow with holidays. Thank you for the contribution!

When running the newly-added tests, I get the following error that fails them:

    testing.go:705: Step 0 error: After applying this step, the plan was not empty:

        DIFF:

        UPDATE: github_repository_environment_deployment_policy.test
          branch_pattern: "v*" => ""
          environment:    "environment/test" => "environment/test"
          id:             "tf-acc-test-7vk43:environment%2Ftest:13663474" => "tf-acc-test-7vk43:environment%2Ftest:13663474"
          repository:     "tf-acc-test-7vk43" => "tf-acc-test-7vk43"
          tag_pattern:    "v*" => "v*"

        STATE:

        (left out for now)

Can you reproduce this? Do they succeed for you?

boredland commented 7 months ago

Hi there! We're currently a bit blocked by this missing. Anything we could help with moving this forward?

kfcampbell commented 7 months ago

@boredland you're certainly welcome to take this code as it is, resolve the issues/conflicts, and submit as another PR! I'd be happy to review it should you decide to do so.

mcevoypeter commented 7 months ago

@boredland, go for it. I've been swamped since the beginning of January and can't get to this for the forseeable future. Let me know if there's anything I can do to help with hand-off.

boredland commented 7 months ago

Didn't find any time yet :(

sumnerwarren commented 7 months ago

@kfcampbell I took a stab at reworking this in #2165.

nickfloyd commented 1 month ago

Thanks for the efforts here; closing in favor of https://github.com/integrations/terraform-provider-github/pull/2165.