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

Adding support for the resource_count attribute on tfe_workspace #712

Merged
merged 2 commits into from
Dec 8, 2022

Conversation

annawinkler
Copy link
Contributor

@annawinkler annawinkler commented Dec 7, 2022

Description

This PR exposes the resource_count attribute on the tfe_workspace resource. This is to make it easier for users writing a Sentinel policy to prevent workspaces from being deleted prior to deleting all of the resources that belong to the given workspace. This attribute is already exposed as part of the data source call.

Below is a screenshot of the documentation update on the tfe_workspace resource:

image

Testing plan

  1. Create a new workspace using the configuration below
resource "tfe_organization" "test-organization" {
  name  = "my-org-name"
  email = "[email protected]"
}

resource "tfe_workspace" "test" {
  name         = "my-workspace-name"
  organization = tfe_organization.test-organization.name
  tag_names    = ["test", "app"]
}

output "workspace_resource_count" {
  value = tfe_workspace.test.resource_count
}

This should yield a result like this:

$ terraform refresh
...
Outputs:

workspace_resource_count = 0
  1. Inside the new workspace (ie my-workspace-name), use the following configuration (or similar) to add resources under the control of the workspace:
resource "tfe_team" "test" {
  name         = "my-team-name"
  organization = "my-org-name"
}
  1. Back on the original workspace, re-run a terraform refresh so we can now see that the resource_count has changed from 0 to 1:
$ terraform refresh
...
Outputs:

workspace_resource_count = 1

External links

Output from acceptance tests

$ TF_ACC=1 TF_LOG_SDK_PROTO=OFF envchain tfe-staging go test ./... -v -run TestAccTFEWorkspace

@annawinkler annawinkler marked this pull request as ready for review December 7, 2022 19:05
@annawinkler annawinkler requested a review from a team as a code owner December 7, 2022 19:05
@annawinkler annawinkler force-pushed the aw/support_workspace_resource_count branch from 605ad37 to 432dd15 Compare December 8, 2022 00:49
@annawinkler annawinkler changed the title Aw/support workspace resource count Adding support for the resource_count attribute on tfe_workspace Dec 8, 2022
Copy link
Contributor

@uturunku1 uturunku1 left a comment

Choose a reason for hiding this comment

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

Sorry if I missed this information, but what was the reason not to the use the original PR opened by the external contributor, and push your commit there? If we are merging this PR instead, would it be possible to @ the contributor to the changelog to thank him for these changes? 🙏
This PR looks good to me, thanks for adding the test!

@annawinkler
Copy link
Contributor Author

annawinkler commented Dec 8, 2022

Sorry if I missed this information, but what was the reason not to the use the original PR opened by the external contributor, and push your commit there? If we are merging this PR instead, would it be possible to @ the contributor to the changelog to thank him for these changes? 🙏 This PR looks good to me, thanks for adding the test!

Good questions! For go-tfe, we have a script that rebases forks and opens a new PR so we can run the tests. I wanted to make sure the test issues with the original PR were unrelated to auth problems, so I manually followed a similar process to what we're doing in go-tfe.

I forgot to add the contributor to the changelog entry - I'll do that. Oh - interestingly the TFE provider changelog doesn't indicate who added PRs - do we want to start doing that? I've referenced their PR though, so perhaps that's sufficient?

I added comments to the original PR to rebase and update the changelog.

We had added a comment to the PR indicating that we would help with the tests, and I prefer to either give a diff suggestion or to have a separate branch instead of committing to someone else's branch (unless they explicitly OK me committing to their branch).

@uturunku1 uturunku1 self-requested a review December 8, 2022 16:11
CHANGELOG.md Outdated
@@ -1,6 +1,7 @@
## Unreleased

FEATURES:
* r/tfe_workspace: Add attribute `resource_count` to `tfe_workspace` ([#682](https://github.com/hashicorp/terraform-provider-tfe/pull/682))
Copy link
Contributor

Choose a reason for hiding this comment

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

About your comment interestingly the TFE provider changelog doesn't indicate who added PRs - do we want to start doing that?, yes, I think it would be nice to add the name of the external contributor, if this doesn't get added automatically 😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you! I'll add that.

@annawinkler annawinkler force-pushed the aw/support_workspace_resource_count branch from 432dd15 to e89a2ad Compare December 8, 2022 17:05
@annawinkler annawinkler merged commit ceaab0c into main Dec 8, 2022
@annawinkler annawinkler deleted the aw/support_workspace_resource_count branch December 8, 2022 17:37
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.

2 participants