feat(iaas): support for v2 API #1070
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rubenhoenle
force-pushed
the
feat/STACKITTPR-313-iaas-v2
branch
2 times, most recently
from
5cfe536 to
29cbe25
Compare
|
This PR was marked as stale after 7 days of inactivity and will be closed after another 7 days of further inactivity. If this PR should be kept open, just add a comment, remove the stale label or push new commits to it. |
relates to STACKITTPR-313
rubenhoenle
force-pushed
the
feat/STACKITTPR-313-iaas-v2
branch
from
2b753bf to
c72a460
Compare
Merging this branch changes the coverage (15 decrease, 4 increase)
Coverage by fileChanged files (no unit tests)
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
|
marceljk
reviewed
Dec 8, 2025
|
|
||
| - `affinity_group_id` (String) The affinity group ID. | ||
| - `id` (String) Terraform's internal resource identifier. It is structured as "`project_id`,`affinity_group_id`". | ||
| - `id` (String) Terraform's internal resource identifier. It is structured as "`project_id`,`region`,`affinity_group_id`". |
Contributor
There was a problem hiding this comment.
The example import statement must be adjusted as well. Currently it has the old id structure (${var.project_id},${var.affinity_group_id}), but it has changed to ${var.project_id},${var.region},${var.affinity_group_id}
marceljk
reviewed
Dec 8, 2025
|
|
||
| - `checksum` (Attributes) Representation of an image checksum. (see [below for nested schema](#nestedatt--checksum)) | ||
| - `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`image_id`". | ||
| - `id` (String) Terraform's internal resource ID. It is structured as "`project_id`,`region`,`image_id`". |
Contributor
There was a problem hiding this comment.
Import example must be adjusted here as well
marceljk
reviewed
Dec 8, 2025
| - `routing_table_id` (String) The ID of the routing table associated with the network. | ||
|
|
||
| ### Read-Only | ||
|
|
Contributor
There was a problem hiding this comment.
In the network resource the documentation of the id structure must be updated and the import example. Probably this relates to most of the modified resources / datasources
marceljk
reviewed
Dec 8, 2025
| # Only use the import statement, if you want to import an existing network area region | ||
| import { | ||
| to = stackit_network_area_region.import-example | ||
| id = "${var.organization_id},${var.network_area_id},eu01" |
Contributor
There was a problem hiding this comment.
nitpick: we usually structure the id only with vars, so I suggest to change eu01 to var.region, to keep it consistent with our other examples
Suggested change
| id = "${var.organization_id},${var.network_area_id},eu01" | |
| id = "${var.organization_id},${var.network_area_id},${var.region}" |
marceljk
reviewed
Dec 8, 2025
| github.com/stackitcloud/stackit-sdk-go/services/dns v0.17.1 | ||
| github.com/stackitcloud/stackit-sdk-go/services/git v0.8.0 | ||
| github.com/stackitcloud/stackit-sdk-go/services/iaas v0.31.0 | ||
| github.com/stackitcloud/stackit-sdk-go/services/iaas v1.0.0 |
Contributor
There was a problem hiding this comment.
Version can be bumped to v1.2.4
marceljk
reviewed
Dec 9, 2025
| if err != nil { | ||
| var oapiErr *oapierror.GenericOpenAPIError | ||
| ok := errors.As(err, &oapiErr) | ||
| if !(ok && (oapiErr.StatusCode == http.StatusNotFound || oapiErr.StatusCode == http.StatusBadRequest)) { // TODO: iaas api returns http 400 in case network area region is not found |
Member
Author
There was a problem hiding this comment.
well, it's an API issue. What should I do then? 😂
| } | ||
|
|
||
| // Deprecated: Will be removed in May 2026. Only introduced to make the IaaS v1 -> v2 API migration non-breaking in the Terraform provider. | ||
| networkAreaRegionResp, err := d.client.GetNetworkAreaRegion(ctx, organizationId, networkAreaId, "eu01").Execute() |
Contributor
There was a problem hiding this comment.
it's intended that eu01 is hardcoded, right? If so, I would suggest that the deprecated fields has a note, that they only load the data from eu01 during the deprecation period
Member
Author
There was a problem hiding this comment.
Yes. I left that out because the behavior of the resource stays unchanged from a user perspective. It was eu01 before, it is eu01 now. But I can still add it if you want 😄
|
|
||
| // Create new network area | ||
| area, err := r.client.CreateNetworkArea(ctx, organizationId).CreateNetworkAreaPayload(*payload).Execute() | ||
| networkArea, err := r.client.CreateNetworkArea(ctx, organizationId).CreateNetworkAreaPayload(*payload).Execute() |
Contributor
There was a problem hiding this comment.
If I remember correctly, we decided to store the partial state of the network area, because in case the second request fails (in legacy mode) the network area will still exists, but from terraform perspective the network area is not created.
I wanted to test something right now and misconfigured the second request accidentally. The result was a few network areas, which I had to delete manually in the portal
Comment on lines
+262
to
+264
| ctx = tflog.SetField(ctx, "region", region) | ||
|
|
||
| // Generate API request body from model |
Contributor
There was a problem hiding this comment.
InitProviderContext can be added
Suggested change
| ctx = tflog.SetField(ctx, "region", region) | |
| // Generate API request body from model | |
| ctx = tflog.SetField(ctx, "region", region) | |
| ctx = core.InitProviderContext(ctx) | |
| // Generate API request body from model |
| core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating network area region", fmt.Sprintf("Calling API: %v", err)) | ||
| return | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Log response / trace id
Suggested change
| ctx = core.LogResponse(ctx) | |
| ctx = tflog.SetField(ctx, "organization_id", organizationId) | ||
| ctx = tflog.SetField(ctx, "network_area_id", networkAreaId) | ||
| ctx = tflog.SetField(ctx, "region", region) | ||
|
|
Contributor
There was a problem hiding this comment.
add initProviderContext and trace id logging
| ctx = tflog.SetField(ctx, "organization_id", organizationId) | ||
| ctx = tflog.SetField(ctx, "network_area_id", networkAreaId) | ||
| ctx = tflog.SetField(ctx, "region", region) | ||
|
|
Contributor
There was a problem hiding this comment.
add initProviderContext and trace id logging
| ctx = tflog.SetField(ctx, "organization_id", organizationId) | ||
| ctx = tflog.SetField(ctx, "network_area_id", networkAreaId) | ||
| ctx = tflog.SetField(ctx, "region", region) | ||
|
|
Contributor
There was a problem hiding this comment.
add initProviderContext and trace id logging
| Nexthop: conversion.StringValueToPointer(model.NextHop), | ||
| Labels: &labels, | ||
| Destination: &iaas.RouteDestination{ | ||
| DestinationCIDRv4: &iaas.DestinationCIDRv4{ |
Contributor
There was a problem hiding this comment.
The v1 iaas api supported only a CIDRv4 as destination, but the v2 api supports v4 and v6. When we already need to deprecate some fields in the iaas resources and change things, I would suggest, that we do this for the network area route and the prefix/destination field as well.
| DestinationCIDRv6: nil, | ||
| }, | ||
| Labels: &labels, | ||
| Nexthop: &iaas.RouteNexthop{ |
Contributor
There was a problem hiding this comment.
Same like for the destination. Nexthop supports beside ipv4 also ipv6, internet and blackhole in the iaas v2 api
marceljk
reviewed
Dec 9, 2025
| "network_interfaces": schema.ListAttribute{ | ||
| Description: "The IDs of network interfaces which should be attached to the server. Updating it will recreate the server.", | ||
| Optional: true, | ||
| Required: true, |
Contributor
There was a problem hiding this comment.
This can lead to migration issues. If some hasn't defined the network_interfaces in the server resource and uses instead the server_network_interface_attach resource (or hasn't attached any network_interface, but then the server is probably useless), an migration would result in a replace at the moment.
Use for example this tf-file with the latest released version of our provider:
resource "stackit_server" "server-01" {
project_id = var.project_id
name = "test"
boot_volume = {
source_type = "image"
size = 32
source_id = "a2c127b2-b1b5-4aee-986f-41cd11b41279" // Ubuntu 24.04 image
delete_on_termination = true
}
machine_type = "t1.1"
availability_zone = "eu01-1"
# network_interfaces = [ stackit_network_interface.nic.network_interface_id ]
}
resource "stackit_network" "network" {
project_id = var.project_id
name = "test"
ipv4_nameservers = ["8.8.8.8", "8.8.4.4"]
ipv4_prefix_length = 24
routed = true
}
resource "stackit_network_interface" "nic" {
project_id = var.project_id
network_id = stackit_network.network.network_id
}
resource "stackit_server_network_interface_attach" "name" {
project_id = var.project_id
server_id = stackit_server.server-01.server_id
network_interface_id = stackit_network_interface.nic.network_interface_id
}- Run $ terraform apply
- Build locally the provider from this PR
- Run again $ terraform apply
-> Error because network_interfaces isn't set
-> Adding a network_interface will result in an Replace
network_interfaces currently stores only the network_interfaces which are provided in the plan, because otherwise it results in state drifts. See STACKITTPR-101 and #679. To provide a smooth migration, we probably need to extend the filter logic, which was introduced in #679
marceljk
reviewed
Dec 10, 2025
|
|
||
| model.Id = utils.BuildInternalTerraformId(projectId, serverId, networkInterfaceId) | ||
| model.Id = utils.BuildInternalTerraformId(projectId, region, serverId, networkInterfaceId) | ||
|
|
Contributor
There was a problem hiding this comment.
Region must be set in the model
Suggested change
| model.Region = types.StringValue(region) | |
Contributor
There was a problem hiding this comment.
In the Read() function it's also missing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
relates to STACKITTPR-313
Checklist
make fmtexamples/directory)make generate-docs(will be checked by CI)make test(will be checked by CI)make lint(will be checked by CI)