fix(iaas): IaaS acceptance tests fixes by Fyusel · Pull Request #1543 · stackitcloud/terraform-provider-stackit

Fyusel · 2026-06-26T07:59:03Z

Create a project to network interface and server acceptance tests. ipv4_nameservers from List to Set. Set is an unordered list and detects changes based on a hash. Update image_v2 acceptance test.

Description

STACKITTPR-673

Checklist

Issue was linked above
Code format was applied: make fmt
Examples were added / adjusted (see examples/ directory)
Docs are up-to-date: make generate-docs (will be checked by CI)
Unit tests got implemented or updated
Acceptance tests got implemented or updated (see e.g. here)
Unit tests are passing: make test (will be checked by CI)
No linter issues: make lint (will be checked by CI)

Create a project to network interface and server acceptance tests. ipv4_nameservers from List to Set. Set is an unordered list and detects changes based on a hash. Update image_v2 acceptance test. STACKITTPR-673 Signed-off-by: Alexander Dahmen <alexander.dahmen@inovex.de>

github-actions · 2026-06-26T09:00:17Z

Merging this branch will increase overall coverage

Impacted Packages	Coverage Δ	🤖
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas	0.00% (ø)
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/imagev2	44.31% (ø)
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/network	46.15% (ø)
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils	87.26% (+0.59%)	👍

Coverage by file

Changed files (no unit tests)

Changed File	Coverage Δ	Total	Covered	Missed	🤖
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/imagev2/datasource.go	44.31% (ø)	167	74	93
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/network/datasource.go	57.98% (ø)	119	69	50
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/network/resource.go	42.27% (ø)	362	153	209
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/utils.go	84.95% (+1.23%)	93 (+7)	79 (+7)	14	👍

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/iaas_acc_test.go
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/network/datasource_test.go
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/iaas/network/resource_test.go
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils/utils_test.go

marceljk · 2026-06-26T11:52:38Z

+		},
+		{


Can you add here a test case for a null set? Want to have it here, to ensure the implementation doesn't change. Because if during an update the set was removed from the config, we usually want to send an explicit empty slice to the API. Otherwise the API wouldn't recognize during the patch call, that the value should be removed and Terraform results in a state drift.

Suggested change

},

{

},

{

description: "null set",

input: types.SetNull(types.StringType),

expected: []string{},

isValid: true,

},

{

marceljk · 2026-06-26T11:58:56Z

+	"image_id":            config.StringVariable("fb5b3fa8-5e20-478a-929a-2b7da1676b18"),
+	"name":                config.StringVariable(fmt.Sprintf("tfe2e-project-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum))),
+	"owner_email":         config.StringVariable(testutil.TestProjectServiceAccountEmail),
+	"parent_container_id": config.StringVariable(testutil.TestProjectParentContainerID),


Can your extend our table, that the parent container id is now also for IaaS required?

terraform-provider-stackit/README.md

Line 235 in 4e42f8c

| `TF_ACC_TEST_PROJECT_PARENT_CONTAINER_ID` | Container ID of the project parent container (folder within an organization or the organization itself) | `organization-d2b7087` | `resourcemanager` |

marceljk · 2026-06-26T12:07:31Z

+	"network_name":        config.StringVariable(fmt.Sprintf("tf-acc-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum))),
+	"machine_type":        config.StringVariable("t1.1"),
+	"image_id":            config.StringVariable("fb5b3fa8-5e20-478a-929a-2b7da1676b18"),
+	"name":                config.StringVariable(fmt.Sprintf("tfe2e-project-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum))),


Personal preference: I think it would be nice, if the project name contains the information, to which test is belongs. Maybe something like server-min. This makes it easier to find the related test in the portal, in case the test stops without cleaning up.

Suggested change

"name": config.StringVariable(fmt.Sprintf("tfe2e-project-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum))),

"name": config.StringVariable(fmt.Sprintf("tfe2e-project-server-min-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum))),

But we should to take care, that the name doesn't reach the limit of 39 chars

Or as alternative, we print the project name within the acc test, like we have it with the network name.

marceljk · 2026-06-26T12:26:27Z

 					// Server
-					resource.TestCheckResourceAttr("stackit_server.server", "project_id", testutil.ConvertConfigVariable(testConfigServerVarsMin["project_id"])),
-					resource.TestCheckResourceAttr("stackit_server.server", "name", testutil.ConvertConfigVariable(testConfigServerVarsMin["name"])),
+					resource.TestCheckResourceAttrSet("stackit_server.server", "project_id"),


I think it would be better, to don't only check if the project_id is set, but also if it's set to the correct value.

Suggested change

resource.TestCheckResourceAttrSet("stackit_server.server", "project_id"),

resource.TestCheckResourceAttrPair(

"stackit_resourcemanager_project.example", "project_id",

"stackit_server.server", "project_id",

),

same applies to the other occurrences

marceljk · 2026-06-26T13:29:52Z

+		testAccCheckResourceManagerProjectsDestroy,
 	}
 	var errs []error



I think we should change two small things in this testAccCheckDestroy function to prevent errors:

I would suggest to remove the waitgroup stuff here and don't call the checkFunctions in goroutines. Most of the resources must be deleted before the next one can be removed (e.g. network interface first, before deleting network). With the goroutine it all happens simultaneously, which is more likely to fail.

We should change the order of the checkFunctions, to take care, that everything will be cleaned up in the correct order. For example first server, then volumes, network interfaces, networks, (then probably everything else) and in the end resource manager, routingTableRoutes, routingTable, networkAreaRegion, networkArea. Not sure if I missed something, but I think this should be almost the correct order.

marceljk · 2026-06-26T13:33:02Z

+	if errors.As(err, &oapiErr) {
+		return oapiErr.StatusCode == http.StatusForbidden ||
+			oapiErr.StatusCode == http.StatusNotFound ||
+			oapiErr.StatusCode == http.StatusBadRequest


Should a BadRequest really be ignored? Usually this means that the sent payload was invalid

marceljk · 2026-06-26T13:43:29Z

+
+	projectsToDestroy := []string{}
+	for _, rs := range s.RootModule().Resources {
+		if rs.Type != "stackit_resourcemanager_project" {
+			continue
+		}
+		// project terraform ID: "[container_id]"
+		containerId := rs.Primary.ID
+		projectsToDestroy = append(projectsToDestroy, containerId)
+	}
+
+	var containerParentId string
+	switch {
+	case testutil.TestProjectParentContainerID != "":
+		containerParentId = testutil.TestProjectParentContainerID
+	case testutil.TestProjectParentUUID != "":
+		containerParentId = testutil.TestProjectParentUUID
+	default:
+		return fmt.Errorf("either TestProjectParentContainerID or TestProjectParentUUID must be set")
+	}
+
+	projectsResp, err := client.DefaultAPI.ListProjects(ctx).ContainerParentId(containerParentId).Execute()
+	if err != nil {
+		return fmt.Errorf("getting projectsResp: %w", err)
+	}
+
+	items := projectsResp.Items
+	for i := range items {
+		if items[i].LifecycleState == resourcemanager.LIFECYCLESTATE_DELETING {
+			continue
+		}
+		if !utils.Contains(projectsToDestroy, items[i].ContainerId) {
+			continue
+		}
+
+		err := client.DefaultAPI.DeleteProject(ctx, items[i].ContainerId).Execute()
+		if err != nil {
+			return fmt.Errorf("destroying project %s during CheckDestroy: %w", items[i].ContainerId, err)
+		}
+
+		_, err = resourcemanager_wait.DeleteProjectWaitHandler(ctx, client.DefaultAPI, items[i].ContainerId).WaitWithContext(ctx)
+		if err != nil {
+			return fmt.Errorf("destroying project %s during CheckDestroy: waiting for deletion %w", items[i].ContainerId, err)
+		}
+	}
+	return nil


I would implement the testDestroy function a bit different. I know that the current implementation was done for a lot of services and also here in IaaS, but I think we can change a few things to make this more stable.

In generel, I think we shouldn't read here any IDs or informations from the testutil vars. Because it could be the case, that we the project got a different parent ID, e.g. from a folder which is part of the test. So we should read everything out of the state.

I also would remove the list endpoint, because theoretically every project could have a different parent ID and then this doesn't work anymore. So instead I would just do a GET request, check the status code (and maybe the state of the resource) and then call the Delete endpoint.

I already wrote this for an upcoming resource in our internal repo. I'll send you a link to it in our internal chat.

Fyusel requested a review from a team as a code owner June 26, 2026 07:59

Fyusel force-pushed the fix-flaky-iaas-tests branch from a8460ac to 6aeb164 Compare June 26, 2026 08:57

marceljk reviewed Jun 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(iaas): IaaS acceptance tests fixes#1543

fix(iaas): IaaS acceptance tests fixes#1543
Fyusel wants to merge 1 commit into
mainfrom
fix-flaky-iaas-tests

Fyusel commented Jun 26, 2026

Uh oh!

github-actions Bot commented Jun 26, 2026

Changed files (no unit tests)

Changed unit test files

Uh oh!

marceljk Jun 26, 2026

Uh oh!

marceljk Jun 26, 2026

Uh oh!

marceljk Jun 26, 2026

Uh oh!

marceljk Jun 26, 2026

Uh oh!

marceljk Jun 26, 2026

Uh oh!

marceljk Jun 26, 2026

Uh oh!

marceljk Jun 26, 2026

Uh oh!

marceljk Jun 26, 2026

Uh oh!

marceljk Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-		},
-		{
+		},
+		{
+			description: "null set",
+			input:       types.SetNull(types.StringType),
+			expected:    []string{},
+			isValid:     true,
+		},
+		{

	"name": config.StringVariable(fmt.Sprintf("tfe2e-project-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum))),
	"name": config.StringVariable(fmt.Sprintf("tfe2e-project-server-min-%s", acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum))),

Uh oh!

Conversation

Fyusel commented Jun 26, 2026

Description

Checklist

Uh oh!

github-actions Bot commented Jun 26, 2026

Merging this branch will increase overall coverage

Changed files (no unit tests)

Changed unit test files

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants