fix(kms): fix disableKeyVersionWaiter to check for proper state

CHANGELOG.md

@@ -7,6 +7,8 @@
   - **Breaking Change:** Removal of unused model structs: `Area`, `AreaConfig`, `AreaPrefixConfigIPv4`, `UpdateAreaIPv4`, `NetworkAreaIPv4`, `CreateAreaAddressFamily`, `CreateAreaIPv4`, `CreateNetworkAddressFamily`, `CreateNetworkIPv4Body`, `CreateNetworkIPv6Body`, `CreateServerPayloadBootVolume`, `CreateServerPayloadNetworking`, `NullableUpdateAreaAddressFamily`, `CreateServerPayloadNetworking`, `UpdateNetworkAddressFamily`, `CreateServerPayloadNetworking`, `CreateServerPayloadNetworking`
 - `cdn`: [v2.1.0](services/cdn/CHANGELOG.md#v210)
   - **Breaking change:** Removal of unused model structs: `GetLogsSearchFiltersResponse`, `PatchLokiLogSink`
+- `kms`: [v1.1.0](services/kms/CHANGELOG.md#v110)
+  - **Bugfix:** Ensure correct state checking in `DisableKeyVersionWaitHandler` and `EnableKeyVersionWaitHandler`
 
 ## Release (2025-10-29)
 - `stackitmarketplace`: [v1.15.0](services/stackitmarketplace/CHANGELOG.md#v1150)

services/kms/CHANGELOG.md

@@ -1,3 +1,6 @@
+## v1.1.0
+- **Bugfix:** Ensure correct state checking in `DisableKeyVersionWaitHandler` and `EnableKeyVersionWaitHandler`
+
 ## v1.0.0
 - Switch to API version `v1` of STACKIT KMS service (previously `v1beta`)
 - **Breaking Change:** Removal of deprecated `Backend` model

services/kms/VERSION

@@ -1 +1 @@
-v1.0.0
+v1.1.0

services/kms/wait/wait.go

@@ -3,6 +3,7 @@ package wait
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net/http"
 	"time"
 
@@ -58,7 +59,7 @@ func CreateKeyRingWaitHandler(ctx context.Context, client ApiKmsClient, projectI
 			return false, nil, err
 		}
 
-		if response.State != nil {
+		if response != nil && response.State != nil {
 			switch *response.State {
 			case kms.KEYRINGSTATE_CREATING:
 				return false, nil, nil
@@ -80,7 +81,7 @@ func CreateOrUpdateKeyWaitHandler(ctx context.Context, client ApiKmsClient, proj
 			return false, nil, err
 		}
 
-		if response.State != nil {
+		if response != nil && response.State != nil {
 			switch *response.State {
 			case kms.KEYSTATE_CREATING:
 				return false, nil, nil
@@ -117,21 +118,25 @@ func EnableKeyVersionWaitHandler(ctx context.Context, client ApiKmsClient, proje
 	handler := wait.New(func() (bool, *kms.Version, error) {
 		response, err := client.GetVersionExecute(ctx, projectId, region, keyRingId, keyId, version)
 		if err != nil {
+			var apiErr *oapierror.GenericOpenAPIError
+			if errors.As(err, &apiErr) {
+				if statusCode := apiErr.StatusCode; statusCode == http.StatusNotFound || statusCode == http.StatusGone {
+					return true, nil, fmt.Errorf("enabling failed for key %s version %d: version or key not found", keyId, version)
+				}
+			}
 			return false, nil, err
 		}
 
-		if response.State != nil {
+		if response != nil && response.State != nil {
 			switch *response.State {
-			case kms.VERSIONSTATE_DESTROYED:
-				return true, response, nil
-			case kms.VERSIONSTATE_KEY_MATERIAL_INVALID:
-				return true, response, nil
-			case kms.VERSIONSTATE_DISABLED:
+			case kms.VERSIONSTATE_ACTIVE:
 				return true, response, nil
 			case kms.VERSIONSTATE_CREATING:
 				return false, nil, nil
+			case kms.VERSIONSTATE_DESTROYED, kms.VERSIONSTATE_KEY_MATERIAL_INVALID:
+				return true, response, fmt.Errorf("enabling failed for key %s version %d: state %s", keyId, version, *response.State)
 			default:
-				return true, response, nil
+				return true, response, fmt.Errorf("key version %d for key %s has unexpected state %s", version, keyId, *response.State)
 			}
 		}
 
@@ -143,16 +148,30 @@ func EnableKeyVersionWaitHandler(ctx context.Context, client ApiKmsClient, proje
 
 func DisableKeyVersionWaitHandler(ctx context.Context, client ApiKmsClient, projectId, region, keyRingId, keyId string, version int64) *wait.AsyncActionHandler[kms.Version] {
 	handler := wait.New(func() (bool, *kms.Version, error) {
-		_, err := client.GetVersionExecute(ctx, projectId, region, keyRingId, keyId, version)
+		response, err := client.GetVersionExecute(ctx, projectId, region, keyRingId, keyId, version)
 		if err != nil {
 			var apiErr *oapierror.GenericOpenAPIError
 			if errors.As(err, &apiErr) {
 				if statusCode := apiErr.StatusCode; statusCode == http.StatusNotFound || statusCode == http.StatusGone {
-					return true, nil, nil
+					return true, nil, fmt.Errorf("disabling failed for key %s version %d: version or key not found", keyId, version)
 				}
 			}
-			return true, nil, err
+			return false, nil, err
 		}
+
+		if response != nil && response.State != nil {
+			switch *response.State {
+			case kms.VERSIONSTATE_DISABLED:
+				return true, response, nil
+			case kms.VERSIONSTATE_ACTIVE, kms.VERSIONSTATE_CREATING, kms.VERSIONSTATE_KEY_MATERIAL_UNAVAILABLE:
+				return false, nil, nil
+			case kms.VERSIONSTATE_DESTROYED, kms.VERSIONSTATE_KEY_MATERIAL_INVALID:
+				return true, response, fmt.Errorf("disabling failed for key %s version %d: state %s", keyId, version, *response.State)
+			default:
+				return true, response, fmt.Errorf("key version %d for key %s has unexpected state %s", version, keyId, *response.State)
+			}
+		}
+
 		return false, nil, nil
 	})
 	handler.SetTimeout(10 * time.Minute)
@@ -166,8 +185,8 @@ func CreateWrappingKeyWaitHandler(ctx context.Context, client ApiKmsClient, proj
 			return false, nil, err
 		}
 
-		if state := response.State; state != nil {
-			switch *state {
+		if response != nil && response.State != nil {
+			switch *response.State {
 			case kms.WRAPPINGKEYSTATE_CREATING:
 				return false, nil, nil
 			default:

services/kms/wait/wait_test.go

@@ -212,7 +212,7 @@ func TestCreateKeyRingWaitHandler(t *testing.T) {
 			}
 
 			if diff := cmp.Diff(tt.want, got); diff != "" {
-				t.Errorf("differing key %s", diff)
+				t.Errorf("differing key ring %s", diff)
 			}
 		})
 	}
@@ -409,15 +409,15 @@ func TestEnableKeyVersionWaitHandler(t *testing.T) {
 			false,
 		},
 		{
-			"create failed delayed",
+			"create failed with invalid key material",
 			[]versionResponse{
 				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
 				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
 				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
 				{fixtureVersion(1, false, kms.VERSIONSTATE_KEY_MATERIAL_INVALID), nil},
 			},
 			fixtureVersion(1, false, kms.VERSIONSTATE_KEY_MATERIAL_INVALID),
-			false,
+			true,
 		},
 		{
 			"timeout",
@@ -433,7 +433,55 @@ func TestEnableKeyVersionWaitHandler(t *testing.T) {
 				{fixtureVersion(1, false, "bogus"), nil},
 			},
 			fixtureVersion(1, false, "bogus"),
-			false,
+			true,
+		},
+		{
+			"version destroyed",
+			[]versionResponse{
+				{fixtureVersion(1, false, kms.VERSIONSTATE_DESTROYED), nil},
+			},
+			fixtureVersion(1, false, kms.VERSIONSTATE_DESTROYED),
+			true,
+		},
+		{
+			"version disabled - unexpected state",
+			[]versionResponse{
+				{fixtureVersion(1, true, kms.VERSIONSTATE_DISABLED), nil},
+			},
+			fixtureVersion(1, true, kms.VERSIONSTATE_DISABLED),
+			true,
+		},
+		{
+			"version key material unavailable - unexpected state",
+			[]versionResponse{
+				{fixtureVersion(1, false, kms.VERSIONSTATE_KEY_MATERIAL_UNAVAILABLE), nil},
+			},
+			fixtureVersion(1, false, kms.VERSIONSTATE_KEY_MATERIAL_UNAVAILABLE),
+			true,
+		},
+		{
+			"version not found (404)",
+			[]versionResponse{
+				{nil, oapierror.NewError(http.StatusNotFound, "not found")},
+			},
+			nil,
+			true,
+		},
+		{
+			"version gone (410)",
+			[]versionResponse{
+				{nil, oapierror.NewError(http.StatusGone, "gone")},
+			},
+			nil,
+			true,
+		},
+		{
+			"error fetching version",
+			[]versionResponse{
+				{nil, oapierror.NewError(http.StatusInternalServerError, "internal error")},
+			},
+			nil,
+			true,
 		},
 		// no special update tests needed as the states are the same
 	}
@@ -454,7 +502,7 @@ func TestEnableKeyVersionWaitHandler(t *testing.T) {
 			}
 
 			if diff := cmp.Diff(tt.want, got); diff != "" {
-				t.Errorf("differing key %s", diff)
+				t.Errorf("differing version %s", diff)
 			}
 		})
 	}
@@ -464,61 +512,84 @@ func TestDisableKeyVersionWaitHandler(t *testing.T) {
 	tests := []struct {
 		name      string
 		responses []versionResponse
+		want      *kms.Version
 		wantErr   bool
 	}{
 		{
-			"Delete with '404' succeeded immediately",
+			"disable succeeded immediately",
 			[]versionResponse{
-				{nil, oapierror.NewError(http.StatusNotFound, "not found")},
+				{fixtureVersion(1, true, kms.VERSIONSTATE_DISABLED), nil},
 			},
+			fixtureVersion(1, true, kms.VERSIONSTATE_DISABLED),
 			false,
 		},
 		{
-			"Delete with '404' delayed",
+			"disable succeeded delayed",
+			[]versionResponse{
+				{fixtureVersion(1, false, kms.VERSIONSTATE_ACTIVE), nil},
+				{fixtureVersion(1, false, kms.VERSIONSTATE_ACTIVE), nil},
+				{fixtureVersion(1, false, kms.VERSIONSTATE_ACTIVE), nil},
+				{fixtureVersion(1, true, kms.VERSIONSTATE_DISABLED), nil},
+			},
+			fixtureVersion(1, true, kms.VERSIONSTATE_DISABLED),
+			false,
+		},
+		{
+			"disable succeeded from creating state",
 			[]versionResponse{
 				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
 				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-				{nil, oapierror.NewError(http.StatusNotFound, "not found")},
+				{fixtureVersion(1, true, kms.VERSIONSTATE_DISABLED), nil},
 			},
+			fixtureVersion(1, true, kms.VERSIONSTATE_DISABLED),
 			false,
 		},
 		{
-			"Delete with 'gone' succeeded immediately",
+			"version already destroyed",
 			[]versionResponse{
-				{nil, oapierror.NewError(http.StatusGone, "gone")},
+				{fixtureVersion(1, false, kms.VERSIONSTATE_DESTROYED), nil},
 			},
-			false,
+			fixtureVersion(1, false, kms.VERSIONSTATE_DESTROYED),
+			true,
 		},
 		{
-			"Delete with 'gone' delayed",
+			"version key material invalid",
 			[]versionResponse{
-				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-				{nil, oapierror.NewError(http.StatusGone, "not found")},
+				{fixtureVersion(1, false, kms.VERSIONSTATE_KEY_MATERIAL_INVALID), nil},
 			},
-			false,
+			fixtureVersion(1, false, kms.VERSIONSTATE_KEY_MATERIAL_INVALID),
+			true,
 		},
 		{
-			"Delete with error delayed",
+			"timeout waiting for disabled state",
 			[]versionResponse{
-				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-
-				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-				{fixtureVersion(1, false, kms.VERSIONSTATE_DESTROYED), oapierror.NewError(http.StatusInternalServerError, "kapow")},
+				{fixtureVersion(1, false, kms.VERSIONSTATE_ACTIVE), nil},
 			},
+			nil,
 			true,
 		},
 		{
-			"Cannot delete",
+			"version not found (404)",
 			[]versionResponse{
-				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-				{fixtureVersion(1, false, kms.VERSIONSTATE_CREATING), nil},
-				{fixtureVersion(1, false, kms.VERSIONSTATE_DESTROYED), oapierror.NewError(http.StatusOK, "ok")},
+				{nil, oapierror.NewError(http.StatusNotFound, "not found")},
+			},
+			nil,
+			true,
+		},
+		{
+			"version gone (410)",
+			[]versionResponse{
+				{nil, oapierror.NewError(http.StatusGone, "gone")},
+			},
+			nil,
+			true,
+		},
+		{
+			"error fetching version",
+			[]versionResponse{
+				{nil, oapierror.NewError(http.StatusInternalServerError, "internal error")},
 			},
+			nil,
 			true,
 		},
 	}
@@ -529,18 +600,16 @@ func TestDisableKeyVersionWaitHandler(t *testing.T) {
 				versionResponses: tt.responses,
 			}
 			handler := DisableKeyVersionWaitHandler(ctx, client, testProject, testRegion, testKeyRingId, testKeyId, 1)
-			_, err := handler.SetTimeout(1 * time.Second).
+			got, err := handler.SetTimeout(1 * time.Second).
 				SetThrottle(250 * time.Millisecond).
 				WaitWithContext(ctx)
 
-			if tt.wantErr != (err != nil) {
-				t.Fatalf("wrong error result. want err: %v got %v", tt.wantErr, err)
+			if (err != nil) != tt.wantErr {
+				t.Fatalf("unexpected error response. want %v but got %v ", tt.wantErr, err)
 			}
-			if tt.wantErr {
-				var apiErr *oapierror.GenericOpenAPIError
-				if !errors.As(err, &apiErr) {
-					t.Fatalf("expected openapi error, got %v", err)
-				}
+
+			if diff := cmp.Diff(tt.want, got); diff != "" {
+				t.Errorf("differing version %s", diff)
 			}
 		})
 	}
@@ -618,7 +687,7 @@ func TestCreateWrappingWaitHandler(t *testing.T) {
 			}
 
 			if diff := cmp.Diff(tt.want, got); diff != "" {
-				t.Errorf("differing key %s", diff)
+				t.Errorf("differing wrapping key %s", diff)
 			}
 		})
 	}