fix(iaas): read write only field correctly from the config model

CONTRIBUTING.md

@@ -9,6 +9,7 @@ Your contribution is welcome! Thank you for your interest in contributing to the
   - [Repository structure](#repository-structure)
   - [Implementing a new resource](#implementing-a-new-resource)
   	- [Resource file structure](#resource-file-structure)
+  	- [Implementing write-only attributes](#implementing-write-only-attributes)
   - [Implementing a new datasource](#implementing-a-new-datasource)
   - [Onboarding a new STACKIT service](#onboarding-a-new-stackit-service)
   - [Implementing IAM Role Bindings](#implementing-iam-role-bindings)
@@ -66,6 +67,14 @@ https://github.com/stackitcloud/terraform-provider-stackit/blob/main/.github/doc
 
 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](./CONTRIBUTING.md/#onboarding-a-new-stackit-service).
 
+#### Implementing write-only attributes
+
+When implementing [write-only attributes](https://developer.hashicorp.com/terraform/language/resources/ephemeral#write-only-arguments) (supported in Terraform 1.11.0 and later), keep in mind that Terraform never populates write-only values in the plan or state models. They are only available in the config model, i.e. they must be read via `req.Config.Get(...)` in the `Create`/`Update` handlers, while all other values are read from the plan model as usual.
+
+You can find a reference implementation of write-only attributes (including the accompanying `<attribute>_wo_version` rotation counter pattern) in the VPN connection resource:
+
+https://github.com/stackitcloud/terraform-provider-stackit/blob/main/stackit/internal/services/vpn/connection/resource.go
+
 ### Implementing a new datasource
 
 The process to implement a new datasource is similar to [implementing a new resource](#implementing-a-new-resource). Some differences worth noting are:

stackit/internal/services/iaas/volume/resource.go

@@ -433,6 +433,16 @@ func (r *volumeResource) Create(ctx context.Context, req resource.CreateRequest,
 		return
 	}
 
+	// The config model - this has to be used because Terraform doesn't include write-only field values in the
+	// plan and state models - for security measures. Write-only values should be only kept in the config model
+	// so that they never end up in the state (or plan).
+	var configModel Model
+	diags = req.Config.Get(ctx, &configModel)
+	resp.Diagnostics.Append(diags...)
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
 	ctx = core.InitProviderContext(ctx)
 
 	projectId := model.ProjectId.ValueString()
@@ -450,7 +460,7 @@ func (r *volumeResource) Create(ctx context.Context, req resource.CreateRequest,
 	}
 
 	// Generate API request body from model
-	payload, err := toCreatePayload(ctx, &model, source)
+	payload, err := toCreatePayload(ctx, &model, &configModel, source)
 	if err != nil {
 		core.LogAndAddError(ctx, &resp.Diagnostics, "Error creating volume", fmt.Sprintf("Creating API payload: %v", err))
 		return
@@ -753,10 +763,13 @@ func mapFields(ctx context.Context, volumeResp *iaas.Volume, model *Model, regio
 	return nil
 }
 
-func toCreatePayload(ctx context.Context, model *Model, source *sourceModel) (*iaas.CreateVolumePayload, error) {
+func toCreatePayload(ctx context.Context, model, configModel *Model, source *sourceModel) (*iaas.CreateVolumePayload, error) {
 	if model == nil {
 		return nil, fmt.Errorf("nil model")
 	}
+	if configModel == nil {
+		return nil, fmt.Errorf("nil config model")
+	}
 
 	labels, err := conversion.ToStringInterfaceMap(ctx, model.Labels)
 	if err != nil {
@@ -782,12 +795,19 @@ func toCreatePayload(ctx context.Context, model *Model, source *sourceModel) (*i
 		Source:           sourcePayload,
 	}
 
-	if model.EncryptionParameters != nil {
+	if model.EncryptionParameters != nil && configModel.EncryptionParameters != nil {
+		// Terraform keeps the write-only field values in the config model - and they shouldn't leave this config model
+		// to make sure they don't end up being stored in the state. In the plan model the write-only field values are just
+		// empty. That's why write-only field values must be read from the config model. Everything else comes from the
+		// plan model.
 		var keyPayload *string
-		if !utils.IsUndefined(model.EncryptionParameters.KeyPayloadBase64WriteOnly) {
-			keyPayload = conversion.StringValueToPointer(model.EncryptionParameters.KeyPayloadBase64WriteOnly)
-		} else if !utils.IsUndefined(model.EncryptionParameters.KeyPayloadBase64) {
+		if !utils.IsUndefined(model.EncryptionParameters.KeyPayloadBase64) {
+			// handle the legacy fallback logic
 			keyPayload = conversion.StringValueToPointer(model.EncryptionParameters.KeyPayloadBase64)
+		} else if !utils.IsUndefined(configModel.EncryptionParameters.KeyPayloadBase64WriteOnly) &&
+			!utils.IsUndefined(model.EncryptionParameters.KeyPayloadBase64WriteOnlyVersion) {
+			// the user is using the write-only field
+			keyPayload = conversion.StringValueToPointer(configModel.EncryptionParameters.KeyPayloadBase64WriteOnly)
 		}
 
 		payload.EncryptionParameters = &iaas.VolumeEncryptionParameter{

stackit/internal/services/iaas/volume/resource_create_test.go

@@ -0,0 +1,135 @@
+package volume_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+
+	iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api"
+
+	"github.com/stackitcloud/terraform-provider-stackit/stackit/internal/testutil"
+)
+
+// TestCreateWriteOnlyKeyPayload is a regression test for the bug where the write-only key payload
+// was read from the plan model instead of the config model.
+// The test asserts that the value configured via key_payload_base64_wo is actually
+// sent to the API in the create request.
+func TestCreateWriteOnlyKeyPayload(t *testing.T) {
+	projectId := uuid.NewString()
+	volumeId := uuid.NewString()
+	kekKeyId := uuid.NewString()
+	kekKeyringId := uuid.NewString()
+	const (
+		region              = "eu01"
+		availabilityZone    = "eu01-1"
+		name                = "test-volume"
+		size                = 16
+		serviceAccount      = "test-sa@sa.stackit.cloud"
+		testKeyPayload      = "VGhlIHF1aWNrIGJyb3duIGZveCBqdW1wcyBvdmVyIDEzIGxhenkgZG9ncy4="
+		volumeStatusCreated = "AVAILABLE"
+	)
+	s := testutil.NewMockServer(t)
+	t.Cleanup(s.Server.Close)
+	tfConfig := fmt.Sprintf(`
+provider "stackit" {
+	default_region = "%s"
+	iaas_custom_endpoint = "%s"
+	service_account_token = "mock-server-needs-no-auth"
+}
+
+resource "stackit_volume" "volume" {
+	project_id = "%s"
+	availability_zone = "%s"
+	name = "%s"
+	size = %d
+	encryption_parameters = {
+		kek_key_id = "%s"
+		kek_key_version = 1
+		kek_keyring_id = "%s"
+		key_payload_base64_wo = "%s"
+		key_payload_base64_wo_version = 1
+		service_account = "%s"
+	}
+}
+`, region, s.Server.URL, projectId, availabilityZone, name, size, kekKeyId, kekKeyringId, testKeyPayload, serviceAccount)
+
+	volumeName := name
+	volumeSize := int64(size)
+	volume := iaas.Volume{
+		Id:               &volumeId,
+		Status:           new(volumeStatusCreated),
+		AvailabilityZone: availabilityZone,
+		Name:             &volumeName,
+		Size:             &volumeSize,
+	}
+
+	var capturedKeyPayload *string
+	createCalled := false
+	createVolume := testutil.MockResponse{
+		Description: "create",
+		Handler: func(w http.ResponseWriter, req *http.Request) {
+			expected := fmt.Sprintf("/v2/projects/%s/regions/%s/volumes", projectId, region)
+			if req.URL.Path != expected {
+				t.Errorf("expected request to %s, got %s", expected, req.URL.Path)
+			}
+			createCalled = true
+			body, err := io.ReadAll(req.Body)
+			if err != nil {
+				t.Errorf("failed to read create request body: %v", err)
+				w.WriteHeader(http.StatusBadRequest)
+				return
+			}
+			var payload iaas.CreateVolumePayload
+			if err := json.Unmarshal(body, &payload); err != nil {
+				t.Errorf("failed to unmarshal create request body: %v", err)
+				w.WriteHeader(http.StatusBadRequest)
+				return
+			}
+			if payload.EncryptionParameters != nil {
+				capturedKeyPayload = payload.EncryptionParameters.KeyPayload
+			}
+
+			w.Header().Set("content-type", "application/json")
+			_ = json.NewEncoder(w).Encode(iaas.Volume{Id: &volumeId})
+		},
+	}
+
+	resource.UnitTest(t, resource.TestCase{
+		ProtoV6ProviderFactories: testutil.TestAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				PreConfig: func() {
+					s.Reset(
+						createVolume,
+						testutil.MockResponse{Description: "create waiter", ToJsonBody: volume},
+						testutil.MockResponse{Description: "get", ToJsonBody: volume},
+						testutil.MockResponse{Description: "delete", StatusCode: http.StatusAccepted},
+						testutil.MockResponse{Description: "delete waiter", StatusCode: http.StatusNotFound},
+					)
+				},
+				Config: tfConfig,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("stackit_volume.volume", "volume_id", volumeId),
+					resource.TestCheckResourceAttr("stackit_volume.volume", "region", region),
+					resource.TestCheckNoResourceAttr("stackit_volume.volume", "encryption_parameters.key_payload_base64_wo"),
+					resource.TestCheckResourceAttr("stackit_volume.volume", "encryption_parameters.key_payload_base64_wo_version", "1"),
+				),
+			},
+		},
+	})
+
+	if !createCalled {
+		t.Fatalf("Expected the create endpoint to be called")
+	}
+	if capturedKeyPayload == nil {
+		t.Fatalf("Expected key payload %q to be sent to the API, but none was sent", testKeyPayload)
+	}
+	if *capturedKeyPayload != testKeyPayload {
+		t.Fatalf("Wrong key payload sent to the API: expected %q, got %q", testKeyPayload, *capturedKeyPayload)
+	}
+}