Skip to content

feat(vpn): Onboarding VPN gateway#1453

Open
s-inter wants to merge 16 commits into
mainfrom
si/onboard-VPN-gateway
Open

feat(vpn): Onboarding VPN gateway#1453
s-inter wants to merge 16 commits into
mainfrom
si/onboard-VPN-gateway

Conversation

@s-inter
Copy link
Copy Markdown
Contributor

@s-inter s-inter commented May 11, 2026

Description

relates to STACKITTPR-633)

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)

@s-inter s-inter changed the title Si/onboard vpn gateway feat(vpn): Onboarding VPN gateway May 11, 2026
Comment thread stackit/internal/services/vpn/vpn_acc_test.go
Comment thread stackit/internal/services/vpn/vpn_acc_test.go
Comment thread stackit/internal/services/vpn/gateway/datasource.go Outdated
@s-inter s-inter marked this pull request as ready for review May 13, 2026 12:14
@s-inter s-inter requested a review from a team as a code owner May 13, 2026 12:14
gatewaysToDestroy = append(gatewaysToDestroy, gatewayId)
}

gatewaysResp, err := client.DefaultAPI.ListGateways(ctx, testutil.ProjectId, vpn.REGION_EU01).Execute()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
gatewaysResp, err := client.DefaultAPI.ListGateways(ctx, testutil.ProjectId, vpn.REGION_EU01).Execute()
gatewaysResp, err := client.DefaultAPI.ListGateways(ctx, testutil.ProjectId, testutil.Region).Execute()

resource.TestCheckResourceAttr("stackit_vpn_gateway.gateway", "display_name", testutil.ConvertConfigVariable(gatewayMaxVars["display_name"])),
resource.TestCheckResourceAttr("stackit_vpn_gateway.gateway", "routing_type", testutil.ConvertConfigVariable(gatewayMaxVars["routing_type"])),
resource.TestCheckResourceAttr("stackit_vpn_gateway.gateway", "bgp.local_asn", testutil.ConvertConfigVariable(gatewayMaxVars["local_asn"])),
resource.TestCheckResourceAttrSet("stackit_vpn_gateway.gateway", "gateway_id"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about availability_zones, region, labels, ...

You should check all fields here, even if it's a negative check that they are not set.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Applies everywhere

resource.TestCheckResourceAttrPair("data.stackit_vpn_gateway.gateway", "project_id", "stackit_vpn_gateway.gateway", "project_id"),
resource.TestCheckResourceAttrPair("data.stackit_vpn_gateway.gateway", "region", "stackit_vpn_gateway.gateway", "region"),
resource.TestCheckResourceAttrPair("data.stackit_vpn_gateway.gateway", "gateway_id", "stackit_vpn_gateway.gateway", "gateway_id"),
resource.TestCheckResourceAttrPair("data.stackit_vpn_gateway.gateway", "display_name", "stackit_vpn_gateway.gateway", "display_name"),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Duplicate check: Didn't you check the "display_name" field already above with the following line?

resource.TestCheckResourceAttr("data.stackit_vpn_gateway.gateway", "display_name", testutil.ConvertConfigVariable(gatewayMaxVars["display_name"])),

Applies to most of the fields here, don't duplicate checks. It blows the test up without any advantage IMO.

Config: fmt.Sprintf("%s\n%s", testutil.NewConfigBuilder().BuildProviderConfig(), gatewayMaxConfig),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("stackit_vpn_gateway.gateway", "display_name", testutil.ConvertConfigVariable(gatewayMaxVarsUpdated["display_name"])),
resource.TestCheckResourceAttr("stackit_vpn_gateway.gateway", "bgp.local_asn", testutil.ConvertConfigVariable(gatewayMaxVarsUpdated["local_asn"])),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Check all fields here to detect potential drifts

Config: fmt.Sprintf("%s\n%s", testutil.NewConfigBuilder().BuildProviderConfig(), gatewayMinConfig),
Check: resource.ComposeAggregateTestCheckFunc(
// Gateway data
resource.TestCheckResourceAttr("stackit_vpn_gateway.gateway", "project_id", testutil.ConvertConfigVariable(gatewayMinVars["project_id"])),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here

Comment thread stackit/internal/services/vpn/gateway/datasource.go Outdated
Comment thread stackit/internal/services/vpn/gateway/datasource.go Outdated
Comment thread stackit/internal/services/vpn/gateway/datasource.go Outdated
Comment thread stackit/internal/services/vpn/gateway/datasource.go Outdated
Comment thread stackit/internal/services/vpn/gateway/resource.go
Co-authored-by: rubenhoenle <Ruben.Hoenle@digits.schwarz>
@github-actions
Copy link
Copy Markdown

Merging this branch changes the coverage (1 decrease, 2 increase)

Impacted Packages Coverage Δ 🤖
github.com/stackitcloud/terraform-provider-stackit/stackit 1.23% (-0.02%) 👎
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core 21.62% (ø)
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/vpn 0.00% (ø)
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/vpn/gateway 18.97% (+18.97%) 🎉
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/vpn/utils 77.78% (+77.78%) 🌟
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/testutil 39.83% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core/core.go 21.62% (ø) 37 8 29
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/vpn/gateway/datasource.go 0.00% (ø) 42 (+42) 0 42 (+42)
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/vpn/gateway/resource.go 22.18% (+22.18%) 248 (+248) 55 (+55) 193 (+193) 🌟
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/vpn/utils/utils.go 77.78% (+77.78%) 9 (+9) 7 (+7) 2 (+2) 🌟
github.com/stackitcloud/terraform-provider-stackit/stackit/internal/testutil/testutil.go 51.09% (ø) 92 47 45
github.com/stackitcloud/terraform-provider-stackit/stackit/provider.go 1.23% (-0.02%) 163 (+2) 2 161 (+2) 👎

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/vpn/gateway/datasource_test.go
  • github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/vpn/gateway/resource_test.go
  • github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/vpn/utils/utils_test.go
  • github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/vpn/vpn_acc_test.go

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