From 2a077d17d82dbc60cb999c32b41e4daf432f41fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ruben=20H=C3=B6nle?= Date: Wed, 15 Oct 2025 14:47:16 +0200 Subject: [PATCH] fix(docs): store IDs of resource after privisioning in contribution guide (#1028) also adjust the contribution guide to show the new multi-region implementation relates to STACKITTPR-374 --- .github/docs/contribution-guide/resource.go | 130 ++++++++++++++++---- CONTRIBUTION.md | 6 +- stackit/internal/utils/regions.go | 3 - 3 files changed, 109 insertions(+), 30 deletions(-) diff --git a/.github/docs/contribution-guide/resource.go b/.github/docs/contribution-guide/resource.go index c87908c2..ff44c7c6 100644 --- a/.github/docs/contribution-guide/resource.go +++ b/.github/docs/contribution-guide/resource.go @@ -5,7 +5,6 @@ import ( "fmt" "strings" - "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" @@ -18,7 +17,8 @@ import ( fooUtils "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/services/foo/utils" "github.com/stackitcloud/terraform-provider-stackit/stackit/internal/utils" - "github.com/stackitcloud/stackit-sdk-go/services/foo" // Import service "foo" from the STACKIT SDK for Go + "github.com/stackitcloud/stackit-sdk-go/services/foo" // Import service "foo" from the STACKIT SDK for Go + "github.com/stackitcloud/stackit-sdk-go/services/foo/wait" // Import service "foo" waiters from the STACKIT SDK for Go (in case the service API has asynchronous endpoints) // (...) ) @@ -27,13 +27,15 @@ var ( _ resource.Resource = &barResource{} _ resource.ResourceWithConfigure = &barResource{} _ resource.ResourceWithImportState = &barResource{} + _ resource.ResourceWithModifyPlan = &barResource{} // not needed for global APIs ) -// Provider's internal model +// Model is the internal model of the terraform resource type Model struct { Id types.String `tfsdk:"id"` // needed by TF ProjectId types.String `tfsdk:"project_id"` BarId types.String `tfsdk:"bar_id"` + Region types.String `tfsdk:"region"` MyRequiredField types.String `tfsdk:"my_required_field"` MyOptionalField types.String `tfsdk:"my_optional_field"` MyReadOnlyField types.String `tfsdk:"my_read_only_field"` @@ -46,7 +48,8 @@ func NewBarResource() resource.Resource { // barResource is the resource implementation. type barResource struct { - client *foo.APIClient + client *foo.APIClient + providerData core.ProviderData // not needed for global APIs } // Metadata returns the resource type name. @@ -54,6 +57,37 @@ func (r *barResource) Metadata(_ context.Context, req resource.MetadataRequest, resp.TypeName = req.ProviderTypeName + "_foo_bar" } +// ModifyPlan implements resource.ResourceWithModifyPlan. +// Use the modifier to set the effective region in the current plan. - FYI: This isn't needed for global APIs. +func (r *barResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) { // nolint:gocritic // function signature required by Terraform + // FYI: the ModifyPlan implementation is not needed for global APIs + var configModel Model + // skip initial empty configuration to avoid follow-up errors + if req.Config.Raw.IsNull() { + return + } + resp.Diagnostics.Append(req.Config.Get(ctx, &configModel)...) + if resp.Diagnostics.HasError() { + return + } + + var planModel Model + resp.Diagnostics.Append(req.Plan.Get(ctx, &planModel)...) + if resp.Diagnostics.HasError() { + return + } + + utils.AdaptRegion(ctx, configModel.Region, &planModel.Region, r.providerData.GetRegion(), resp) + if resp.Diagnostics.HasError() { + return + } + + resp.Diagnostics.Append(resp.Plan.Set(ctx, planModel)...) + if resp.Diagnostics.HasError() { + return + } +} + // Configure adds the provider configured client to the resource. func (r *barResource) Configure(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { providerData, ok := conversion.ParseProviderData(ctx, req.ProviderData, &resp.Diagnostics) @@ -76,6 +110,7 @@ func (r *barResource) Schema(_ context.Context, _ resource.SchemaRequest, resp * "id": "Terraform's internal resource identifier. It is structured as \"`project_id`,`bar_id`\".", "project_id": "STACKIT Project ID to which the bar is associated.", "bar_id": "The bar ID.", + "region": "The resource region. If not defined, the provider region is used.", "my_required_field": "My required field description.", "my_optional_field": "My optional field description.", "my_read_only_field": "My read-only field description.", @@ -108,6 +143,15 @@ func (r *barResource) Schema(_ context.Context, _ resource.SchemaRequest, resp * Description: descriptions["bar_id"], Computed: true, }, + "region": schema.StringAttribute{ // not needed for global APIs + Optional: true, + // must be computed to allow for storing the override value from the provider + Computed: true, + Description: descriptions["region"], + PlanModifiers: []planmodifier.String{ + stringplanmodifier.RequiresReplace(), + }, + }, "my_required_field": schema.StringAttribute{ Description: descriptions["my_required_field"], Required: true, @@ -136,36 +180,57 @@ func (r *barResource) Schema(_ context.Context, _ resource.SchemaRequest, resp * // Create creates the resource and sets the initial Terraform state. func (r *barResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { // nolint:gocritic // function signature required by Terraform var model Model - diags := req.Plan.Get(ctx, &model) - resp.Diagnostics.Append(diags...) + resp.Diagnostics.Append(req.Plan.Get(ctx, &model)...) if resp.Diagnostics.HasError() { return } projectId := model.ProjectId.ValueString() - barId := model.BarId.ValueString() + region := model.Region.ValueString() // not needed for global APIs ctx = tflog.SetField(ctx, "project_id", projectId) + ctx = tflog.SetField(ctx, "region", region) - // Create new bar + // prepare the payload struct for the create bar request payload, err := toCreatePayload(&model) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating credential", fmt.Sprintf("Creating API payload: %v", err)) return } - resp, err := r.client.CreateBar(ctx, projectId, barId).CreateBarPayload(*payload).Execute() + + // Create new bar + barResp, err := r.client.CreateBar(ctx, projectId, region).CreateBarPayload(*payload).Execute() if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating bar", fmt.Sprintf("Calling API: %v", err)) return } + + // only in case the create bar API call is asynchronous (Make sure to include *ALL* fields which are part of the + // internal terraform resource id! And please include the comment below in your code): + // Write id attributes to state before polling via the wait handler - just in case anything goes wrong during the wait handler + utils.SetAndLogStateFields(ctx, &resp.Diagnostics, &resp.State, map[string]interface{}{ + "project_id": projectId, + "region": region, + "bar_id": resp.BarId, + }) + if resp.Diagnostics.HasError() { + return + } + // only in case the create bar API request is synchronous: just log the bar id field instead ctx = tflog.SetField(ctx, "bar_id", resp.BarId) - // Map response body to schema + // only in case the create bar API call is asynchronous: use a wait handler to wait for the create process to complete + barResp, err := wait.CreateBarWaitHandler(ctx, r.client, projectId, region, resp.BarId).WaitWithContext(ctx) + if err != nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating bar", fmt.Sprintf("Bar creation waiting: %v", err)) + return + } + + // No matter if the API request is synchronous or asynchronous: Map response body to schema err = mapFields(resp, &model) if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating bar", fmt.Sprintf("Processing API payload: %v", err)) return } - diags = resp.State.Set(ctx, model) - resp.Diagnostics.Append(diags...) + resp.Diagnostics.Append(resp.State.Set(ctx, model)...) if resp.Diagnostics.HasError() { return } @@ -175,17 +240,18 @@ func (r *barResource) Create(ctx context.Context, req resource.CreateRequest, re // Read refreshes the Terraform state with the latest data. func (r *barResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { // nolint:gocritic // function signature required by Terraform var model Model - diags := req.State.Get(ctx, &model) - resp.Diagnostics.Append(diags...) + resp.Diagnostics.Append(req.State.Get(ctx, &model)...) if resp.Diagnostics.HasError() { return } projectId := model.ProjectId.ValueString() + region := r.providerData.GetRegionWithOverride(model.Region) barId := model.BarId.ValueString() ctx = tflog.SetField(ctx, "project_id", projectId) + ctx = tflog.SetField(ctx, "region", region) ctx = tflog.SetField(ctx, "bar_id", barId) - barResp, err := r.client.GetBar(ctx, projectId, barId).Execute() + barResp, err := r.client.GetBar(ctx, projectId, region, barId).Execute() if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error reading bar", fmt.Sprintf("Calling API: %v", err)) return @@ -199,8 +265,7 @@ func (r *barResource) Read(ctx context.Context, req resource.ReadRequest, resp * } // Set refreshed state - diags = resp.State.Set(ctx, model) - resp.Diagnostics.Append(diags...) + resp.Diagnostics.Append(resp.State.Set(ctx, model)...) if resp.Diagnostics.HasError() { return } @@ -209,7 +274,7 @@ func (r *barResource) Read(ctx context.Context, req resource.ReadRequest, resp * // Update updates the resource and sets the updated Terraform state on success. func (r *barResource) Update(ctx context.Context, _ resource.UpdateRequest, resp *resource.UpdateResponse) { // nolint:gocritic // function signature required by Terraform - // Similar to Create method, calls r.client.UpdateBar instead + // Similar to Create method, calls r.client.UpdateBar (and wait.UpdateBarWaitHandler if needed) instead } // Delete deletes the resource and removes the Terraform state on success. @@ -221,16 +286,25 @@ func (r *barResource) Delete(ctx context.Context, req resource.DeleteRequest, re return } projectId := model.ProjectId.ValueString() + region := model.Region.ValueString() barId := model.BarId.ValueString() ctx = tflog.SetField(ctx, "project_id", projectId) + ctx = tflog.SetField(ctx, "region", region) ctx = tflog.SetField(ctx, "bar_id", barId) // Delete existing bar - _, err := r.client.DeleteBar(ctx, projectId, barId).Execute() + _, err := r.client.DeleteBar(ctx, projectId, region, barId).Execute() if err != nil { core.LogAndAddError(ctx, &resp.Diagnostics, "Error deleting bar", fmt.Sprintf("Calling API: %v", err)) } + // only in case the bar delete API endpoint is asynchronous: use a wait handler to wait for the delete operation to complete + _, err = wait.DeleteBarWaitHandler(ctx, r.client, projectId, region, barId).WaitWithContext(ctx) + if err != nil { + core.LogAndAddError(ctx, &resp.Diagnostics, "Error deleting bar", fmt.Sprintf("Bar deletion waiting: %v", err)) + return + } + tflog.Info(ctx, "Foo bar deleted") } @@ -238,16 +312,20 @@ func (r *barResource) Delete(ctx context.Context, req resource.DeleteRequest, re // The expected format of the bar resource import identifier is: project_id,bar_id func (r *barResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { idParts := strings.Split(req.ID, core.Separator) - if len(idParts) != 2 || idParts[0] == "" || idParts[1] == "" { + if len(idParts) != 3 || idParts[0] == "" || idParts[1] == "" || idParts[2] == "" { core.LogAndAddError(ctx, &resp.Diagnostics, "Error importing bar", - fmt.Sprintf("Expected import identifier with format [project_id],[bar_id], got %q", req.ID), + fmt.Sprintf("Expected import identifier with format [project_id],[region],[bar_id], got %q", req.ID), ) return } - resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("project_id"), idParts[0])...) - resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("bar_id"), idParts[1])...) + utils.SetAndLogStateFields(ctx, &resp.Diagnostics, &resp.State, map[string]any{ + "project_id": idParts[0], + "region": idParts[1], + "bar_id": idParts[2], + }) + tflog.Info(ctx, "Foo bar state imported") } @@ -265,7 +343,11 @@ func mapFields(barResp *foo.GetBarResponse, model *Model) error { bar := barResp.Bar model.BarId = types.StringPointerValue(bar.BarId) - model.Id = utils.BuildInternalTerraformId(model.ProjectId.ValueString(), model.BarId.ValueString()) + model.Id = utils.BuildInternalTerraformId( + model.ProjectId.ValueString(), + model.Region.ValueString(), + model.BarId.ValueString(), + ) model.MyRequiredField = types.StringPointerValue(bar.MyRequiredField) model.MyOptionalField = types.StringPointerValue(bar.MyOptionalField) diff --git a/CONTRIBUTION.md b/CONTRIBUTION.md index c4e0e142..78566b44 100644 --- a/CONTRIBUTION.md +++ b/CONTRIBUTION.md @@ -53,7 +53,7 @@ Let's suppose you want to want to implement a new resource `bar` of service `foo } ``` -Please remeber to always add unit tests for the helper functions (in this case `mapFields` and `toCreatePayload`), as well implementing/extending the acceptance (end-to-end) tests. Our acceptance tests are implemented using Hashicorp's [terraform-plugin-testing](https://developer.hashicorp.com/terraform/plugin/testing/acceptance-tests) package. +Please remember to always add unit tests for the helper functions (in this case `mapFields` and `toCreatePayload`), as well implementing/extending the acceptance (end-to-end) tests. Our acceptance tests are implemented using Hashicorp's [terraform-plugin-testing](https://developer.hashicorp.com/terraform/plugin/testing/acceptance-tests) package. Additionally, remember to run `make generate-docs` after your changes to keep the commands' documentation in `docs/` updated, which is used as a source for the [Terraform Registry documentation page](https://registry.terraform.io/providers/stackitcloud/stackit/latest/docs). @@ -61,7 +61,7 @@ Additionally, remember to run `make generate-docs` after your changes to keep th Below is a typical structure of a STACKIT Terraform provider resource: -https://github.com/stackitcloud/terraform-provider-stackit/blob/1b9225598a007cda8d8bcadf0db1836e96451353/.github/docs/contribution-guide/resource.go#L26-L295 +https://github.com/stackitcloud/terraform-provider-stackit/blob/main/.github/docs/contribution-guide/resource.go If the new resource `bar` is the first resource in the TFP using a STACKIT service `foo`, please refer to [Onboarding a new STACKIT service](./CONTRIBUTION.md/#onboarding-a-new-stackit-service). @@ -86,7 +86,7 @@ If you want to onboard resources of a STACKIT service `foo` that was not yet in ``` 4. Create a utils package, for service `foo` it would be `stackit/internal/foo/utils`. Add a `ConfigureClient()` func and use it in your resource and datasource implementations. -https://github.com/stackitcloud/terraform-provider-stackit/blob/1b9225598a007cda8d8bcadf0db1836e96451353/.github/docs/contribution-guide/utils/util.go#L14-L31 +https://github.com/stackitcloud/terraform-provider-stackit/blob/main/.github/docs/contribution-guide/utils/util.go ### Local development diff --git a/stackit/internal/utils/regions.go b/stackit/internal/utils/regions.go index 7af581c7..89dbdae9 100644 --- a/stackit/internal/utils/regions.go +++ b/stackit/internal/utils/regions.go @@ -33,7 +33,4 @@ func AdaptRegion(ctx context.Context, configRegion types.String, planRegion *typ *planRegion = intendedRegion } resp.Diagnostics.Append(resp.Plan.SetAttribute(ctx, p, *planRegion)...) - if resp.Diagnostics.HasError() { - return - } }