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 project level auto destroy #1550

Merged
merged 44 commits into from
Feb 3, 2025
Merged

Conversation

simonxmh
Copy link
Contributor

@simonxmh simonxmh commented Dec 19, 2024

Description

Add support in the tfe provider for the project level ephemeral workspace settings.

Remember to:

Testing plan

  1. Set up a workspace inside a project like such
resource "tfe_project" "test" {
  organization = var.organization
  name = "test-project"
  auto_destroy_activity_duration = "9h"
}

resource "tfe_workspace" "overriding_workspace" {
  organization = var.organization
  name = "test_workspace"
  project_id = tfe_project.test.id

  vcs_repo {
    identifier = var.repo
    branch = var.branch
    oauth_token_id = tfe_oauth_client.github.oauth_token_id
  }
}
  1. The workspace should pick up the setting from the project. You can verify this by going to /app/<orgname>/workspaces/<workspace_name>/settings/delete
  2. Set up an overriding workspace
resource "tfe_project" "test" {
  organization = var.organization
  name = "test-project"
  auto_destroy_activity_duration = "9h"
}

resource "tfe_workspace" "overriding_workspace" {
  organization = var.organization
  name = "test_workspace"
  project_id = tfe_project.test.id
  auto_destroy_activity_duration = "10d"
  inherits_project_auto_destroy = false

  vcs_repo {
    identifier = var.repo
    branch = var.branch
    oauth_token_id = tfe_oauth_client.github.oauth_token_id
  }
}
  1. The workspace should override the setting from the project.

External links

Include any links here that might be helpful for people reviewing your PR. If there are none, feel free to delete this section.

Output from acceptance tests

Please run applicable acceptance tests locally and include the output here. See testing.md to learn how to run acceptance tests.

Project Resource

Workspace Resource

Project Data source

TESTARGS="-run TestAccTFEProjectDataSource_*" envchain tf-provider make testacc          $! simonxmh/add_project_level_auto_destroy 
TF_ACC=1 TF_LOG_SDK_PROTO=OFF go test $(go list ./... |grep -v 'vendor') -v -run TestAccTFEProjectDataSource_* -timeout 15m
?       github.com/hashicorp/terraform-provider-tfe     [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/client     0.293s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/logging    0.470s [no tests to run]
?       github.com/hashicorp/terraform-provider-tfe/internal/provider/validators        [no test files]
?       github.com/hashicorp/terraform-provider-tfe/version     [no test files]
=== RUN   TestAccTFEProjectDataSource_basic
--- PASS: TestAccTFEProjectDataSource_basic (9.52s)
=== RUN   TestAccTFEProjectDataSource_caseInsensitive
--- PASS: TestAccTFEProjectDataSource_caseInsensitive (7.30s)
=== RUN   TestAccTFEProjectDataSource_basicWithAutoDestroy
--- PASS: TestAccTFEProjectDataSource_basicWithAutoDestroy (9.16s)
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/provider   26.446s

Workspace Data source

TESTARGS="-run TestAccTFEWorkspaceDataSource_*" envchain tf-provider make testacc        
TF_ACC=1 TF_LOG_SDK_PROTO=OFF go test $(go list ./... |grep -v 'vendor') -v -run TestAccTFEWorkspaceDataSource_* -timeout 15m
?       github.com/hashicorp/terraform-provider-tfe     [no test files]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/client     0.471s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/logging    (cached) [no tests to run]
?       github.com/hashicorp/terraform-provider-tfe/internal/provider/validators        [no test files]
?       github.com/hashicorp/terraform-provider-tfe/version     [no test files]
=== RUN   TestAccTFEWorkspaceDataSource_remoteStateConsumers
--- PASS: TestAccTFEWorkspaceDataSource_remoteStateConsumers (11.11s)
=== RUN   TestAccTFEWorkspaceDataSource_basic
--- PASS: TestAccTFEWorkspaceDataSource_basic (6.16s)
=== RUN   TestAccTFEWorkspaceDataSourceWithTriggerPatterns
--- PASS: TestAccTFEWorkspaceDataSourceWithTriggerPatterns (4.11s)
=== RUN   TestAccTFEWorkspaceDataSource_readAutoDestroyAt
--- PASS: TestAccTFEWorkspaceDataSource_readAutoDestroyAt (10.25s)
=== RUN   TestAccTFEWorkspaceDataSource_readAutoDestroyDuration
--- PASS: TestAccTFEWorkspaceDataSource_readAutoDestroyDuration (10.36s)
=== RUN   TestAccTFEWorkspaceDataSource_readProjectIDDefault
--- PASS: TestAccTFEWorkspaceDataSource_readProjectIDDefault (6.05s)
=== RUN   TestAccTFEWorkspaceDataSource_readProjectIDNonDefault
--- PASS: TestAccTFEWorkspaceDataSource_readProjectIDNonDefault (7.61s)
PASS
ok      github.com/hashicorp/terraform-provider-tfe/internal/provider   56.248s
TESTARGS="-run TestAccTFEWorkspaceDataSource_*" envchain tf-provider make   7.37s user 4.54s system 20% cpu 59.017 total

...

@simonxmh simonxmh requested a review from a team as a code owner December 19, 2024 20:29
@simonxmh simonxmh force-pushed the simonxmh/add_project_level_auto_destroy branch from 91a6fb5 to 01a4069 Compare December 19, 2024 20:34
@simonxmh simonxmh requested a review from a team December 20, 2024 18:52
go.mod Outdated Show resolved Hide resolved
CHANGELOG.md Outdated
@@ -1,5 +1,13 @@
## Unreleased

BREAKING_CHANGES:
* `r/tfe_workspace`: Add attribute `inherits_project_auto_destroy` to tfe_workspace, existing workspaces with auto-destroy settings should add `inherits_project_auto_destroy: false` to tfe_workspace resource [1550](https://github.com/hashicorp/terraform-provider-tfe/pull/1550)
Copy link
Member

Choose a reason for hiding this comment

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

IMO we should do fill this out for the user the same way we do on the frontend. Basically if they have one defined in config then we should automatically set this to false

Add business org delegation to the datasource test

Run go mod tidy

Add interface stub

Use datasource for organzation

Reference data source

Add missing reference

Modify resource to directly reference orgname
@simonxmh simonxmh force-pushed the simonxmh/add_project_level_auto_destroy branch from 15be161 to 54858bc Compare January 9, 2025 21:33
CHANGELOG.md Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
@simonxmh simonxmh requested a review from notchairmk January 27, 2025 17:05
Update latest version to v0.63.0

cdktf: update documentation

empty

Fix merged conflicts
emailnitram
emailnitram previously approved these changes Jan 27, 2025
Copy link

@emailnitram emailnitram left a comment

Choose a reason for hiding this comment

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

tested provider locally and looks good

emailnitram
emailnitram previously approved these changes Jan 28, 2025
@notchairmk
Copy link
Member

Just tried this out, it looks like removing an auto_destroy_at override from a workspace that's in a project with an auto destroy duration defined doesn't work.

@simonxmh
Copy link
Contributor Author

Just tried this out, it looks like removing an auto_destroy_at override from a workspace that's in a project with an auto destroy duration defined doesn't work.

Will sync with you on this. Cannot replicate this at the moment.

@simonxmh simonxmh requested a review from a team January 29, 2025 21:28
d.Set("auto_destroy_activity_duration", v)
} else {
d.Set("auto_destroy_activity_duration", nil)
Copy link
Member

Choose a reason for hiding this comment

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

very minor nit, imitating the logic above for other nilable strings (and what we do in the project data source) would probably be better

		var autoDestroyDuration string
		if proj.AutoDestroyActivityDuration.IsSpecified() {
			autoDestroyDuration, err = proj.AutoDestroyActivityDuration.Get()
			if err != nil {
				return diag.Errorf("Error reading auto destroy activity duration: %v", err)
			}
		}

notchairmk
notchairmk previously approved these changes Jan 29, 2025
Copy link
Member

@notchairmk notchairmk left a comment

Choose a reason for hiding this comment

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

Thanks, looks good! 🚢

Copy link
Contributor

@sebasslash sebasslash left a comment

Choose a reason for hiding this comment

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

Implementation is looking good. We're missing the doc changes for r/tfe_project, r/tfe_workspace and d/tfe_workspace however

@@ -62,6 +62,33 @@ func TestAccTFEProjectDataSource_caseInsensitive(t *testing.T) {
})
}

func TestAccTFEProjectDataSource_basicWithAutoDestroy(t *testing.T) {
rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this feature requires a subscription upgrade we should skip this test for our nightly TFE CI using skipUnlessCloud(t).

Copy link
Contributor Author

@simonxmh simonxmh Feb 3, 2025

Choose a reason for hiding this comment

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

I'll use skipIfEnterprise if that's the flag you mean? I don't believe skipUnlessCloud is a thing. Unless you mean the opposite in skipIfCloud?

@simonxmh
Copy link
Contributor Author

simonxmh commented Feb 3, 2025

Implementation is looking good. We're missing the doc changes for r/tfe_project, r/tfe_workspace and d/tfe_workspace however

I added the docs for r/tfe_project,and d/tfe_workspace, but not for r/tfe_workspace because the interface didn't really change for the resource. The inherits_project_auto_destroy attribute is also read-only so not a configuring attribute.

Edit: My bad, thanks for leting me know that we should still add it to the attribute reference section.

@simonxmh simonxmh requested a review from sebasslash February 3, 2025 16:37
@simonxmh simonxmh merged commit 404d250 into main Feb 3, 2025
5 checks passed
@simonxmh simonxmh deleted the simonxmh/add_project_level_auto_destroy branch February 3, 2025 21:05
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.

4 participants