From 2370960399ee696c27a9c9a958670608c56dc9d0 Mon Sep 17 00:00:00 2001 From: Jan Schreier Date: Tue, 8 Sep 2026 10:23:01 +0200 Subject: [PATCH] fix(iaas): remove routing table from state when it no longer exists The IaaS API answers HTTP 404 for routing tables and routes of a network area whose region (stackit_network_area_region) or the area itself has been deleted. Read() of stackit_routing_table turned that 404 into an error diagnostic (utils.LogError followed by RemoveResource); Terraform keeps the prior state when a read reports an error, so every refresh of the stale resource failed. The RemoveResource call after any other API error was dead code for the same reason. Read() now removes the resource from state on 404 without an error diagnostic and reports every other error while keeping the state, the pattern stackit_routing_table_route.Read() already uses. A resource- level test covers 200, 404, 403, 500 and a transport error. The schema descriptions of both resources document the platform's cascade and the provider's behaviour. Fixes #1749 --- docs/resources/routing_table.md | 3 + docs/resources/routing_table_route.md | 3 + .../iaas/routingtable/route/resource.go | 5 +- .../iaas/routingtable/table/resource.go | 22 ++- .../iaas/routingtable/table/resource_test.go | 144 ++++++++++++++++++ 5 files changed, 164 insertions(+), 13 deletions(-) diff --git a/docs/resources/routing_table.md b/docs/resources/routing_table.md index 1314dbb4b..03f26b867 100644 --- a/docs/resources/routing_table.md +++ b/docs/resources/routing_table.md @@ -5,6 +5,7 @@ subcategory: "" description: |- Routing table resource schema. Must have a region specified in the provider configuration. This resource is for SNA, not VPC, based networks. + The platform deletes routing tables and their routes together with the network area region (stackit_network_area_region) they belong to. When the network area or its region no longer exists, the provider treats the routing table as deleted: it is removed from the Terraform state on refresh, and destroying it succeeds. ~> This resource is part of the experimental feature routing-tables and is likely going to undergo significant changes or be removed in the future. Use it at your own discretion. --- @@ -14,6 +15,8 @@ Routing table resource schema. Must have a `region` specified in the provider co This resource is for SNA, not VPC, based networks. +The platform deletes routing tables and their routes together with the network area region (`stackit_network_area_region`) they belong to. When the network area or its region no longer exists, the provider treats the routing table as deleted: it is removed from the Terraform state on refresh, and destroying it succeeds. + ~> This resource is part of the experimental feature routing-tables and is likely going to undergo significant changes or be removed in the future. Use it at your own discretion. ## Example Usage diff --git a/docs/resources/routing_table_route.md b/docs/resources/routing_table_route.md index 19f66a859..c1c7000ba 100644 --- a/docs/resources/routing_table_route.md +++ b/docs/resources/routing_table_route.md @@ -5,6 +5,7 @@ subcategory: "" description: |- Routing table route resource schema. Must have a region specified in the provider configuration. This resource is for SNA, not VPC, based networks. + The platform deletes routing tables and their routes together with the network area region (stackit_network_area_region) they belong to. When the network area or its region no longer exists, the provider treats the route as deleted: it is removed from the Terraform state on refresh, and destroying it succeeds. ~> This resource is part of the experimental feature routing-tables and is likely going to undergo significant changes or be removed in the future. Use it at your own discretion. --- @@ -14,6 +15,8 @@ Routing table route resource schema. Must have a `region` specified in the provi This resource is for SNA, not VPC, based networks. +The platform deletes routing tables and their routes together with the network area region (`stackit_network_area_region`) they belong to. When the network area or its region no longer exists, the provider treats the route as deleted: it is removed from the Terraform state on refresh, and destroying it succeeds. + ~> This resource is part of the experimental feature routing-tables and is likely going to undergo significant changes or be removed in the future. Use it at your own discretion. ## Example Usage diff --git a/stackit/internal/services/iaas/routingtable/route/resource.go b/stackit/internal/services/iaas/routingtable/route/resource.go index 66a60d77d..d71384924 100644 --- a/stackit/internal/services/iaas/routingtable/route/resource.go +++ b/stackit/internal/services/iaas/routingtable/route/resource.go @@ -108,7 +108,10 @@ func (r *routeResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanR // Schema defines the schema for the resource. func (r *routeResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { description := "Routing table route resource schema. Must have a `region` specified in the provider configuration.\n\n" + - "This resource is for SNA, not VPC, based networks." + "This resource is for SNA, not VPC, based networks.\n\n" + + "The platform deletes routing tables and their routes together with the network area region (`stackit_network_area_region`) they belong to. " + + "When the network area or its region no longer exists, the provider treats the route as deleted: " + + "it is removed from the Terraform state on refresh, and destroying it succeeds." resp.Schema = schema.Schema{ Description: description, MarkdownDescription: features.AddExperimentDescription(description, features.RoutingTablesExperiment, core.Resource), diff --git a/stackit/internal/services/iaas/routingtable/table/resource.go b/stackit/internal/services/iaas/routingtable/table/resource.go index f63fcefeb..ade600b0e 100644 --- a/stackit/internal/services/iaas/routingtable/table/resource.go +++ b/stackit/internal/services/iaas/routingtable/table/resource.go @@ -123,7 +123,10 @@ func (r *routingTableResource) ModifyPlan(ctx context.Context, req resource.Modi func (r *routingTableResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { description := "Routing table resource schema. Must have a `region` specified in the provider configuration.\n\n" + - "This resource is for SNA, not VPC, based networks." + "This resource is for SNA, not VPC, based networks.\n\n" + + "The platform deletes routing tables and their routes together with the network area region (`stackit_network_area_region`) they belong to. " + + "When the network area or its region no longer exists, the provider treats the routing table as deleted: " + + "it is removed from the Terraform state on refresh, and destroying it succeeds." resp.Schema = schema.Schema{ Description: description, MarkdownDescription: features.AddExperimentDescription(description, features.RoutingTablesExperiment, core.Resource), @@ -302,17 +305,12 @@ func (r *routingTableResource) Read(ctx context.Context, req resource.ReadReques routingTableResp, err := r.client.DefaultAPI.GetRoutingTableOfArea(ctx, organizationId, networkAreaId, region, routingTableId).Execute() if err != nil { - utils.LogError( - ctx, - &resp.Diagnostics, - err, - "Reading routing table", - fmt.Sprintf("routing table with ID %q does not exist in organization %q.", routingTableId, organizationId), - map[int]string{ - http.StatusForbidden: fmt.Sprintf("Organization with ID %q not found or forbidden access", organizationId), - }, - ) - resp.State.RemoveResource(ctx) + var oapiErr *oapierror.GenericOpenAPIError + if errors.As(err, &oapiErr) && oapiErr.StatusCode == http.StatusNotFound { + resp.State.RemoveResource(ctx) + return + } + core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading routing table", fmt.Sprintf("Calling API: %v", err)) return } diff --git a/stackit/internal/services/iaas/routingtable/table/resource_test.go b/stackit/internal/services/iaas/routingtable/table/resource_test.go index 8c0b7a3c4..6e443ca5e 100644 --- a/stackit/internal/services/iaas/routingtable/table/resource_test.go +++ b/stackit/internal/services/iaas/routingtable/table/resource_test.go @@ -3,12 +3,21 @@ package table import ( "context" "fmt" + "net/http" + "net/http/httptest" "testing" "github.com/google/go-cmp/cmp" + "github.com/gorilla/mux" "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/stackitcloud/stackit-sdk-go/core/config" iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + + "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/core" + "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils" ) func TestMapFields(t *testing.T) { @@ -215,3 +224,138 @@ func TestToUpdatePayload(t *testing.T) { }) } } + +func TestRead(t *testing.T) { + const ( + organizationId = "0f0f0f0f-0f0f-0f0f-0f0f-0f0f0f0f0f0f" + networkAreaId = "1e1e1e1e-1e1e-1e1e-1e1e-1e1e1e1e1e1e" + routingTableId = "2d2d2d2d-2d2d-2d2d-2d2d-2d2d2d2d2d2d" + region = "eu01" + ) + priorState := Model{ + Id: utils.BuildInternalTerraformId(organizationId, region, networkAreaId, routingTableId), + OrganizationId: types.StringValue(organizationId), + RoutingTableId: types.StringValue(routingTableId), + NetworkAreaId: types.StringValue(networkAreaId), + Name: types.StringValue("example"), + Region: types.StringValue(region), + Labels: types.MapNull(types.StringType), + SystemRoutes: types.BoolValue(true), + DynamicRoutes: types.BoolValue(true), + } + + tests := []struct { + name string + statusCode int + body string + closeServer bool // close the mocked server before Read to provoke a transport error + wantErr bool + wantRemoved bool + wantState *Model // expected state after Read; nil keeps the prior state + }{ + { + name: "routing table exists", + statusCode: http.StatusOK, + body: fmt.Sprintf(`{"id": %q, "name": "renamed", "description": "d", "systemRoutes": false, "dynamicRoutes": true, "labels": {"k": "v"}}`, routingTableId), + wantState: &Model{ + Id: utils.BuildInternalTerraformId(organizationId, region, networkAreaId, routingTableId), + OrganizationId: types.StringValue(organizationId), + RoutingTableId: types.StringValue(routingTableId), + NetworkAreaId: types.StringValue(networkAreaId), + Name: types.StringValue("renamed"), + Description: types.StringValue("d"), + Region: types.StringValue(region), + Labels: types.MapValueMust(types.StringType, map[string]attr.Value{"k": types.StringValue("v")}), + SystemRoutes: types.BoolValue(false), + DynamicRoutes: types.BoolValue(true), + CreatedAt: types.StringNull(), + UpdatedAt: types.StringNull(), + }, + }, + { + // The IaaS API answers 404 for routing tables of a deleted network area or network area region + name: "404 removes the routing table from state without an error", + statusCode: http.StatusNotFound, + body: `{"code": 404, "msg": "resource not found: area"}`, + wantRemoved: true, + }, + { + name: "403 reports an error and keeps the state", + statusCode: http.StatusForbidden, + body: `{"code": 403, "msg": "forbidden"}`, + wantErr: true, + }, + { + name: "500 reports an error and keeps the state", + statusCode: http.StatusInternalServerError, + body: `{"code": 500, "msg": "internal error"}`, + wantErr: true, + }, + { + name: "transport error reports an error and keeps the state", + closeServer: true, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + router := mux.NewRouter() + router.HandleFunc("/v2/organizations/{organizationId}/network-areas/{areaId}/regions/{region}/routing-tables/{routingTableId}", func(w http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) + if r.Method != http.MethodGet || vars["organizationId"] != organizationId || vars["areaId"] != networkAreaId || vars["region"] != region || vars["routingTableId"] != routingTableId { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tt.statusCode) + if _, err := w.Write([]byte(tt.body)); err != nil { + t.Errorf("Get routing table handler: failed to write response: %v", err) + } + }) + mockedServer := httptest.NewServer(router) + defer mockedServer.Close() + client, err := iaas.NewAPIClient( + config.WithEndpoint(mockedServer.URL), + config.WithoutAuthentication(), + ) + if err != nil { + t.Fatalf("Failed to initialize client: %v", err) + } + if tt.closeServer { + mockedServer.Close() + } + r := &routingTableResource{client: client, providerData: core.ProviderData{DefaultRegion: region}} + + ctx := context.Background() + var schemaResp resource.SchemaResponse + r.Schema(ctx, resource.SchemaRequest{}, &schemaResp) + state := tfsdk.State{Schema: schemaResp.Schema} + if diags := state.Set(ctx, priorState); diags.HasError() { + t.Fatalf("Failed to build state: %v", diags) + } + + resp := resource.ReadResponse{State: state} + r.Read(ctx, resource.ReadRequest{State: state}, &resp) + + if resp.Diagnostics.HasError() != tt.wantErr { + t.Errorf("Read() error diagnostics = %v, wantErr %v: %v", resp.Diagnostics.HasError(), tt.wantErr, resp.Diagnostics) + } + if resp.State.Raw.IsNull() != tt.wantRemoved { + t.Errorf("Read() removed resource from state = %v, want %v", resp.State.Raw.IsNull(), tt.wantRemoved) + } + if tt.wantRemoved { + return + } + var got Model + if diags := resp.State.Get(ctx, &got); diags.HasError() { + t.Fatalf("Failed to read state back: %v", diags) + } + want := priorState + if tt.wantState != nil { + want = *tt.wantState + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Read() state mismatch (-want +got):\n%s", diff) + } + }) + } +}