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 ability to upload Policies in a PolicySet resource #333

Merged
merged 22 commits into from
Jul 18, 2021

Conversation

omarismail
Copy link
Contributor

@omarismail omarismail commented Jul 13, 2021

Description

This PR augments the tfe_policy_set resource to allow uploading of local policies. This is done primarily through the use of PolicySetVersions.

Example usage

This functionality only works when there is no VCS repo attached to a PolicySet. The usage is:

data "tfe_slug" "policy" {
  // path to local policies
  source_path = "policies/my-policy-set"
}

resource "tfe_policy_set" "test" {
  name          = "my-policy-set"
  description   = "A brand new policy set"
  organization  = "<org-name>"

  // reference to the tfe_slug data source
  slug = data.tfe_slug.policy
}

Behind the scenes, this creates a new PolicySetVersion and then Uploads the contents of the source_path in the tfe_slug data source.

Testing plan

  1. Pull down this branch and make build the provider. Make sure to enable development override for the provider.
  2. Create a new directory and terraform init
  3. Copy the tfe/test-fixtures/policy-set-version directory to your local working directory under policies/my-policy-set
  4. Add this to your config
data "tfe_slug" "policy" {
  source_path = "policies/my-policy-set"
}

resource "tfe_policy_set" "test" {
  name          = "my-policy-set"
  description   = "A brand new policy set"
  organization  = tfe_organization.test.name
  slug = data.tfe_slug.policy
}

output "policy" {
  value = tfe_policy_set.test
}

output "data_policy" {
  value = data.tfe_slug.policy
}
  1. Run terraform apply
  2. Validate in the UI in this organization that the policy set is created, and verify the terraform output that the status is ready. You can also curl to get the PolicySetVersion and confirm the status.
  3. Then edit/add a new file in the policies/my-policy-set directory and run terraform apply and notice an PolicySet update taking place (terraform outputs 1 changed).

Documentation

Images of the new documentation is put as a PR comment near the file.

External links

Closes #279

Output from acceptance tests

terraform-provider-tfe % TFE_TOKEN=<token> TFE_HOSTNAME=tfe-zone-4c292d5b.ngrok.io TF_ACC=1 TF_LOG_PROVIDER=DEBUG  gotestsum --format testname -- ./tfe/... -v -run TestAccTFEPolicySet_version
PASS tfe.TestAccTFEPolicySet_version (7.01s)
PASS tfe.TestAccTFEPolicySet_version_updateCreate (12.06s)
PASS tfe.TestAccTFEPolicySet_version_no_vcs_repo (1.20s)
PASS tfe

DONE 3 tests in 26.078s

@omarismail
Copy link
Contributor Author

Will squash and clean the commit message before merging.

@@ -49,6 +49,27 @@ resource "tfe_policy_set" "test" {
}
```

Local policies (non-VCS based usage, relies on Policy Set Version upload).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@@ -0,0 +1,35 @@
---
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@omarismail omarismail marked this pull request as ready for review July 13, 2021 15:27
@omarismail omarismail requested review from a team July 13, 2021 15:27
Copy link
Member

@chrisarcand chrisarcand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome! I really like how this turned out.

tfe/data_source_slug.go Outdated Show resolved Hide resolved
tfe/resource_tfe_policy_set.go Outdated Show resolved Hide resolved
if err != nil {
t.Fatalf("Error removing file %v", err)
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true of any test in this suite, no? If so, I'd say this is unnecessary but also harmless to leave!

I think the way to 'solve' these potential leftovers is via the use of sweepers, but that's not within our scope here of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow. Do you mean that having the extra removeFile function call below as a precaution isn't that useful because this would happen to any test fatal?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, yeah. It's a fix for this particular test but is something inherent to any of them. Harmless to leave, just figured I'd say I wouldn't expect it as that's a bigger issue with the whole suite 😩

tfe/resource_tfe_policy_set_test.go Outdated Show resolved Hide resolved
tfe/resource_tfe_policy_set_test.go Outdated Show resolved Hide resolved
website/docs/d/slug.html.markdown Outdated Show resolved Hide resolved
website/docs/d/slug.html.markdown Show resolved Hide resolved
website/docs/d/slug.html.markdown Outdated Show resolved Hide resolved
website/docs/r/policy_set.html.markdown Outdated Show resolved Hide resolved
website/docs/r/policy_set.html.markdown Outdated Show resolved Hide resolved
@omarismail omarismail requested review from a team July 13, 2021 23:02
@omarismail omarismail merged commit 7b46f69 into main Jul 18, 2021
@omarismail omarismail deleted the omarismail/policy-upload branch July 18, 2021 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sentinel Policy Set w/ no VCS repo
2 participants