Add deployment policy resource #1530

ilmax · 2023-02-02T18:57:34Z

Add the initial code to manage the resource
Add sample configuration used to test it
Add test

Resolves #922

Behavior

Before the change?

Not possible to set environment deployment branch policies

After the change?

You can to set environment deployment branch policies

Other information

N/A

Additional info

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)
Added the appropriate label for the given change

Does this introduce a breaking change?

Please see our docs on breaking changes to help!

Yes (Please add the Type: Breaking change label)
No

If Yes, what's the impact:

N/A

Pull request type

Please add the corresponding label for change this PR introduces:

Bugfix: Type: Bug
Feature/model/API additions: Type: Feature
Updates to docs or samples: Type: Documentation
Dependencies/code cleanup: Type: Maintenance

ilmax · 2023-02-02T19:04:05Z

@kfcampbell I put this one up for early (tested locally via the new folder in examples and works as expected) review because, as mentioned in #922 I'm not a go expert 😅.
I'd like to know what's the approach for documentation, is it added manually? If memory serves me (and it may not) in the azurerm provider it's auto-generated so I was wondering if here it's the same.

Tests will follow soon.

kfcampbell · 2023-02-18T00:34:49Z

@limax the approach for documentation is manual unfortunately. The idea is to create a new .html.markdown file and link it in the github.erb file; you should be able to follow the pattern from other resources. Please let me know if you're having issues!

nickfloyd · 2023-02-21T22:20:03Z

Hey @ilmax are you still interested in getting your changes wrapped up here? We'd be glad to review them when ready! Also, let us know if you need more information on how to build out the docs for your change. Thanks again for making our community better by being involved! ❤️

ilmax · 2023-02-21T23:49:01Z

Hey @nickfloyd sorry for the delay but didn't had time recently to move forward in this one, is still on my radar and I plan to finish it within next week. Sorry for the delay!

ilmax · 2023-02-22T18:06:36Z

@kfcampbell @nickfloyd I think this may be ready for review

kfcampbell

I had a couple questions I'd like to hear about before getting this in. The tests and examples look good, thanks!

kfcampbell · 2023-02-28T19:19:49Z

github/resource_github_repository_environment_deployment_policy.go

+		Read:   resourceGithubRepositoryEnvironmentDeploymentPolicyRead,
+		Update: resourceGithubRepositoryEnvironmentDeploymentPolicyUpdate,
+		Delete: resourceGithubRepositoryEnvironmentDeploymentPolicyDelete,
+		Schema: map[string]*schema.Schema{


It looks like Import isn't implemented here. Is that an intentional omission?

Preamble: Not an expert in implementing a terraform provider.

The GitHub api that exposes a get deployment branch policy requires an ID, this ID is not exposed anywhere in the UI so the user has no way (unless it's queries the GitHub api) to know such ID.

One option I was thinking about was to read all the deployment branch policies and then filter by branch pattern but I thought such an implementation to be overkill, especially because this resource only contains a branch name.

What are your thoughts?

I think it's still worth implementing with an ID even if it's not clearly visible in the GitHub UI; there's still value in it.

I just implemented support for import

kfcampbell · 2023-02-28T19:20:46Z

github/resource_github_repository_environment_deployment_policy_test.go

+	"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
+)
+
+func TestAccGithubRepositoryEnvironmentDeploymentPolicy(t *testing.T) {


Confirming that integration tests are passing for me locally; thank you for the attention to testing.

examples/repository_deployment_policy/README.md

nickfloyd · 2023-03-08T16:02:19Z

Hey, @ilmax Let us know your thoughts on the questions from @kfcampbell and we'll get this merged in. Thanks again for the work here ❤️

ilmax · 2023-03-09T08:08:14Z

Hey @nickfloyd sorry for the late reply, replied to the questions

jjgriff93

Just some formatting suggestions :) would love to see this in the provider soon so I can get rid of my local-exec cli step!

examples/repository_deployment_policy/main.tf

ghost · 2023-03-17T16:32:24Z

Just some formatting suggestions :) would love to see this in the provider soon so I can get rid of my local-exec cli step!

Hi, would you mind sharing your local-exec code?

- Add the initial code to manage the resource - Add sample configuration used to test it TODO - Documentation - Tests

ilmax · 2023-03-17T18:49:45Z

@kfcampbell @nickfloyd I implemented the import, removed the extra sample and rebased the PR, mind you taking another look?

Thanks

jjgriff93 · 2023-03-20T14:32:05Z

Just some formatting suggestions :) would love to see this in the provider soon so I can get rid of my local-exec cli step!

Hi, would you mind sharing your local-exec code?

Sure @ivan-santos-eq, here you go:

resource "github_repository_environment" "all" {
  for_each    = local.branches_and_envs
  repository  = local.repository_name
  environment = each.value

  deployment_branch_policy {
    custom_branch_policies = true
    protected_branches     = false
  }

  # TODO: remove when https://github.com/integrations/terraform-provider-github/pull/1530 is merged
  provisioner "local-exec" {
    command = <<EOF
curl https://api.github.com/repos/${var.github_owner}/${local.repository_name}/environments/${each.value}/deployment-branch-policies \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  -H "Authorization: Bearer ${var.github_access_token}" \
  -d '{"name":"${each.key}"}'
EOF
  }

  depends_on = [
    github_repository.app
  ]
}

examples/dev.tfrc

ilmax · 2023-04-04T15:02:12Z

Hello @kfcampbell @nickfloyd did you had a chance to review this one?

kfcampbell · 2023-04-10T21:16:33Z

@ilmax 😭 😭 😭 the tests are now failing for me with the following error:

        Error: Provider produced inconsistent result after apply
        
        When applying changes to github_repository_environment_deployment_policy.test,
        provider "github" produced an unexpected new value for was present, but now
        absent.

I would imagine that there's supposed to be a value in between "for was" that describes a specific property, though I'm not sure how we ended up in this situation. Any thoughts?

ilmax · 2023-04-11T06:50:45Z

I'll try to look into this soon

edheliel · 2023-06-30T11:16:11Z

Is this still ongoing or stale?

pragmaticivan · 2023-07-11T13:40:30Z

Hi, do you need help with getting it over the finish line?

edheliel · 2023-07-19T14:46:44Z

@kfcampbell @jjgriff93 @nickfloyd had some time and investigated. The issue was due to some double encoding of the environment name which cause s the name to be invalid and the API to not be able to find the newly created resource.

I opened a PR containing the original author's work and my fix on top of it. Feel free to review and let me know if something else is needed.

PR: #1799

kfcampbell · 2023-07-24T17:56:15Z

Closing as #1799 has been merged. Thank you @ilmax and @edheliel!

nickfloyd added Type: Feature New feature or request Priority: Normal labels Feb 21, 2023

ilmax marked this pull request as ready for review February 22, 2023 18:06

ilmax mentioned this pull request Feb 27, 2023

Add support for github actions variables #1569

Merged

5 tasks

kfcampbell reviewed Feb 28, 2023

View reviewed changes

jjgriff93 reviewed Mar 17, 2023

View reviewed changes

examples/repository_deployment_policy/main.tf Outdated Show resolved Hide resolved

examples/repository_deployment_policy/main.tf Outdated Show resolved Hide resolved

examples/repository_deployment_policy/main.tf Outdated Show resolved Hide resolved

ilmax added 8 commits March 17, 2023 19:46

Add deployment policy resource

e9d592c

- Add the initial code to manage the resource - Add sample configuration used to test it TODO - Documentation - Tests

Add schema description

e845f87

Fix creation of resource ID

8c72108

Add tests

21c5dee

Add documentation

a3b21aa

Add terraform import support

bf38b3b

Undo example add

fc1a22b

Fix formatting

35321df

ilmax force-pushed the feat/environment-protection-rules branch from 5e7b5a6 to 35321df Compare March 17, 2023 18:47

jjgriff93 mentioned this pull request Mar 23, 2023

Deploy app in CI UCLH-Foundry/FlowEHR#196

Merged

4 tasks

ghost reviewed Mar 27, 2023

View reviewed changes

examples/dev.tfrc Outdated Show resolved Hide resolved

PR feedback

d487284

Merge branch 'main' into feat/environment-protection-rules

2acf03b

Merge branch 'main' into feat/environment-protection-rules

e09de38

edheliel mentioned this pull request Jul 19, 2023

fix: Github Environments Policy feature causing the provider to produce inconsistent result #1799

Merged

5 tasks

nickfloyd removed the Priority: Normal label Jul 24, 2023

kfcampbell closed this Jul 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add deployment policy resource #1530

Add deployment policy resource #1530

ilmax commented Feb 2, 2023 •

edited

ilmax commented Feb 2, 2023

kfcampbell commented Feb 18, 2023

nickfloyd commented Feb 21, 2023

ilmax commented Feb 21, 2023

ilmax commented Feb 22, 2023

kfcampbell left a comment

kfcampbell Feb 28, 2023

ilmax Mar 9, 2023

kfcampbell Mar 10, 2023

ilmax Mar 17, 2023

kfcampbell Feb 28, 2023

nickfloyd commented Mar 8, 2023

ilmax commented Mar 9, 2023

jjgriff93 left a comment

ghost commented Mar 17, 2023

ilmax commented Mar 17, 2023

jjgriff93 commented Mar 20, 2023

ilmax commented Apr 4, 2023

kfcampbell commented Apr 10, 2023

ilmax commented Apr 11, 2023

edheliel commented Jun 30, 2023

pragmaticivan commented Jul 11, 2023

edheliel commented Jul 19, 2023 •

edited

kfcampbell commented Jul 24, 2023

Add deployment policy resource #1530

Add deployment policy resource #1530

Conversation

ilmax commented Feb 2, 2023 • edited

Behavior

Before the change?

After the change?

Other information

Additional info

Pull request checklist

Does this introduce a breaking change?

Pull request type

ilmax commented Feb 2, 2023

kfcampbell commented Feb 18, 2023

nickfloyd commented Feb 21, 2023

ilmax commented Feb 21, 2023

ilmax commented Feb 22, 2023

kfcampbell left a comment

Choose a reason for hiding this comment

kfcampbell Feb 28, 2023

Choose a reason for hiding this comment

ilmax Mar 9, 2023

Choose a reason for hiding this comment

kfcampbell Mar 10, 2023

Choose a reason for hiding this comment

ilmax Mar 17, 2023

Choose a reason for hiding this comment

kfcampbell Feb 28, 2023

Choose a reason for hiding this comment

nickfloyd commented Mar 8, 2023

ilmax commented Mar 9, 2023

jjgriff93 left a comment

Choose a reason for hiding this comment

ghost commented Mar 17, 2023

ilmax commented Mar 17, 2023

jjgriff93 commented Mar 20, 2023

ilmax commented Apr 4, 2023

kfcampbell commented Apr 10, 2023

ilmax commented Apr 11, 2023

edheliel commented Jun 30, 2023

pragmaticivan commented Jul 11, 2023

edheliel commented Jul 19, 2023 • edited

kfcampbell commented Jul 24, 2023

ilmax commented Feb 2, 2023 •

edited

edheliel commented Jul 19, 2023 •

edited