diff --git a/CHANGELOG.md b/CHANGELOG.md index f1b79a04d..58c0e7d2b 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/services/kms/CHANGELOG.md b/services/kms/CHANGELOG.md index 77172ace8..163f09358 100644 --- a/services/kms/CHANGELOG.md +++ b/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 diff --git a/services/kms/VERSION b/services/kms/VERSION index 0ec25f750..795460fce 100644 --- a/services/kms/VERSION +++ b/services/kms/VERSION @@ -1 +1 @@ -v1.0.0 +v1.1.0 diff --git a/services/kms/wait/wait.go b/services/kms/wait/wait.go index ad00630f9..6c77d241e 100644 --- a/services/kms/wait/wait.go +++ b/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: diff --git a/services/kms/wait/wait_test.go b/services/kms/wait/wait_test.go index f122cd124..b61b3436c 100644 --- a/services/kms/wait/wait_test.go +++ b/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,7 +409,7 @@ 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}, @@ -417,7 +417,7 @@ func TestEnableKeyVersionWaitHandler(t *testing.T) { {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) } }) }