From 153002ff6b277e9152c5592bc79c8ef5dd789e39 Mon Sep 17 00:00:00 2001 From: fabenan-f <63860771+fabenan-f@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:53:06 +0200 Subject: [PATCH 1/3] feat: add agents upsert key rpc Signed-off-by: fabenan-f <63860771+fabenan-f@users.noreply.github.com> --- Makefile | 1 + api-specs/v1/proto/agents/keys/keys.proto | 22 ++ internal/handler/announcekey/task.go | 4 +- internal/handler/announcekey/task_test.go | 2 +- internal/keylifecycle/keylifecycle.go | 6 + internal/keylifecycle/keylifecycle_test.go | 25 ++ internal/keyoperator/key.go | 46 ++++ internal/keyoperator/key_test.go | 184 +++++++++++++++ pkg/api/v1/proto/agents/keys/errmap.go | 33 +++ pkg/api/v1/proto/agents/keys/keys.pb.go | 236 +++++++++++++++++++ pkg/api/v1/proto/agents/keys/keys_grpc.pb.go | 122 ++++++++++ pkg/api/v1/proto/agents/keys/service.go | 77 ++++++ pkg/api/v1/proto/agents/keys/service_test.go | 144 +++++++++++ pkg/api/v1/proto/agents/keys/setup_test.go | 181 ++++++++++++++ pkg/store/key.go | 2 +- pkg/store/sql/key.go | 21 +- pkg/store/sql/key_test.go | 10 +- pkg/store/sql/migrate.go | 3 +- pkg/validator/key_validator.go | 40 ++++ pkg/validator/key_validator_test.go | 91 +++++++ 20 files changed, 1224 insertions(+), 26 deletions(-) create mode 100644 api-specs/v1/proto/agents/keys/keys.proto create mode 100644 pkg/api/v1/proto/agents/keys/errmap.go create mode 100644 pkg/api/v1/proto/agents/keys/keys.pb.go create mode 100644 pkg/api/v1/proto/agents/keys/keys_grpc.pb.go create mode 100644 pkg/api/v1/proto/agents/keys/service.go create mode 100644 pkg/api/v1/proto/agents/keys/service_test.go create mode 100644 pkg/api/v1/proto/agents/keys/setup_test.go diff --git a/Makefile b/Makefile index a454ed5a..e3c60937 100644 --- a/Makefile +++ b/Makefile @@ -82,6 +82,7 @@ dev: postgres root .PHONY: proto-gen proto-gen: ./scripts/proto-gen.sh "api-specs/v1/proto/agents" + ./scripts/proto-gen.sh "api-specs/v1/proto/agents/keys" ./scripts/proto-gen.sh "api-specs/v1/proto/sealer" ./scripts/proto-gen.sh "api-specs/v1/proto/admin" ./scripts/proto-gen.sh "api-specs/v1/proto/admin/keys" diff --git a/api-specs/v1/proto/agents/keys/keys.proto b/api-specs/v1/proto/agents/keys/keys.proto new file mode 100644 index 00000000..e0e60305 --- /dev/null +++ b/api-specs/v1/proto/agents/keys/keys.proto @@ -0,0 +1,22 @@ +syntax = "proto3"; + +package krypton.v1.agents.keys; + +option go_package = "github.com/openkcm/krypton/pkg/api/v1/proto/agents/keys"; + +service KeyService { + rpc UpsertKey(UpsertKeyRequest) returns (UpsertKeyResponse) {} +} + +message UpsertKeyRequest { + string tenant_id = 1; + string key_id = 2; + string kind = 3; + string name = 4; + string parent_id = 5; + string lifecycle_state = 6; + string managed_by = 7; + map labels = 8; +} + +message UpsertKeyResponse {} diff --git a/internal/handler/announcekey/task.go b/internal/handler/announcekey/task.go index 01cb55ba..26aae2f8 100644 --- a/internal/handler/announcekey/task.go +++ b/internal/handler/announcekey/task.go @@ -60,7 +60,7 @@ func NewTaskHandler(keyStore store.Key) orbital.HandlerFunc { } // Idempotent re-delivery: agent already has the key. - if errors.Is(err, store.ErrKeyAlreadyExists) { + if errors.Is(err, store.ErrKeyInsertConflict) { slogctx.Info(ctx, "key already announced (idempotent)", "keyID", key.ID) resp.Complete() return @@ -76,7 +76,7 @@ func NewTaskHandler(keyStore store.Key) orbital.HandlerFunc { return case pgCodeUniqueViolation: // Belt-and-suspenders: should already be caught above as - // ErrKeyAlreadyExists, but be defensive. + // ErrKeyInsertConflict, but be defensive. resp.Complete() return } diff --git a/internal/handler/announcekey/task_test.go b/internal/handler/announcekey/task_test.go index 9c069c6d..a1b882be 100644 --- a/internal/handler/announcekey/task_test.go +++ b/internal/handler/announcekey/task_test.go @@ -74,7 +74,7 @@ func TestTaskHandler_CorruptPayload_TerminalFail(t *testing.T) { } func TestTaskHandler_AlreadyExists_Idempotent(t *testing.T) { - handler := announcekey.NewTaskHandler(&taskCreateOverride{createErr: store.ErrKeyAlreadyExists}) + handler := announcekey.NewTaskHandler(&taskCreateOverride{createErr: store.ErrKeyInsertConflict}) data := announcekey.TaskData{ KeyID: uuid.New().String(), diff --git a/internal/keylifecycle/keylifecycle.go b/internal/keylifecycle/keylifecycle.go index e982b0b5..09dce882 100644 --- a/internal/keylifecycle/keylifecycle.go +++ b/internal/keylifecycle/keylifecycle.go @@ -61,6 +61,12 @@ var defaultLifecycle = lifecycle{ }, } +// IsKnown reports whether s is one of the defined lifecycle states. +func IsKnown(s model.KeyLifeCycleState) bool { + _, ok := defaultLifecycle.transitions[s] + return ok +} + // ValidateTransition checks whether transitioning from one state to another is allowed. func ValidateTransition(from, to model.KeyLifeCycleState) error { ts, ok := defaultLifecycle.transitions[from] diff --git a/internal/keylifecycle/keylifecycle_test.go b/internal/keylifecycle/keylifecycle_test.go index ac42c442..56137f0d 100644 --- a/internal/keylifecycle/keylifecycle_test.go +++ b/internal/keylifecycle/keylifecycle_test.go @@ -206,3 +206,28 @@ func TestKeyLifecycleKeyUsages(t *testing.T) { } }) } + +func TestIsKnown(t *testing.T) { + t.Parallel() + + tts := []struct { + state model.KeyLifeCycleState + want bool + }{ + {state: "", want: false}, + {state: "bogus", want: false}, + {state: model.KeyLifeCyclePreActivation, want: true}, + {state: model.KeyLifeCycleActive, want: true}, + {state: model.KeyLifeCycleSuspended, want: true}, + {state: model.KeyLifeCycleDeactivated, want: true}, + {state: model.KeyLifeCycleCompromised, want: true}, + {state: model.KeyLifeCycleDestroyed, want: true}, + } + + for _, tt := range tts { + t.Run(fmt.Sprintf("[%s]=%t", tt.state, tt.want), func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, keylifecycle.IsKnown(tt.state)) + }) + } +} diff --git a/internal/keyoperator/key.go b/internal/keyoperator/key.go index 32bd996f..1ce8487e 100644 --- a/internal/keyoperator/key.go +++ b/internal/keyoperator/key.go @@ -34,8 +34,54 @@ var ( // ErrGetKey signals a failed read of the target key. ErrGetKey = errors.New("failed to get key") + + // ErrCreateKey signals a failed key creation. + ErrCreateKey = errors.New("failed to create key") + + // ErrKeyConflict signals that an existing key with the same identity + // (tenant + id or tenant + name) does not match the upsert request: + // one of Name, TenantID, ManagedBy, Kind, or ParentID differs. + ErrKeyConflict = errors.New("existing key does not match upsert request") ) +// UpsertKey inserts newKey; on conflict it reconciles the existing row. +func UpsertKey(newKey model.Key) store.TransactionFunc { + return func(ctx context.Context, stores store.Stores) error { + err := stores.Keys.CreateKey(ctx, newKey) + if err == nil { + return nil + } + if !errors.Is(err, store.ErrKeyInsertConflict) { + return fmt.Errorf("%w: %w", ErrCreateKey, err) + } + + existing, err := stores.Keys.GetKeyByID(ctx, newKey.ID, newKey.TenantID) + if err != nil { + return fmt.Errorf("%w: %w", ErrGetKey, err) + } + if !existing.IsSame(&newKey) { + return ErrKeyConflict + } + + err = stores.Keys.UpdateKeyStates(ctx, store.UpdateKeyStatesQuery{ + ID: existing.ID, + TenantID: existing.TenantID, + ToState: newKey.LifeCycleState, + ToStatus: newKey.KeyProcessingState.Status, + FromState: []model.KeyLifeCycleState{existing.LifeCycleState}, + FromStatus: []model.KeyProcessingStatus{model.KeyProcessingPending, model.KeyProcessingFailed}, + }) + // compare-and-swap matched zero rows: row is already in the target state (idempotent replay). + if errors.Is(err, store.ErrKeyNotFound) { + return nil + } + if err != nil { + return fmt.Errorf("%w: %w", ErrUpdateKeyState, err) + } + return nil + } +} + // UpdateKeyState returns a transaction step that transitions the key's // life cycle and processing status. func UpdateKeyState(tenantID, keyID string, transition Transition) store.TransactionFunc { diff --git a/internal/keyoperator/key_test.go b/internal/keyoperator/key_test.go index bae6bfb7..d2fe5745 100644 --- a/internal/keyoperator/key_test.go +++ b/internal/keyoperator/key_test.go @@ -22,10 +22,15 @@ const ( type stubKeyStore struct { store.Key + createKey func(ctx context.Context, key model.Key) error getKeyByID func(ctx context.Context, id, tenantID string) (*model.Key, error) updateKeyStates func(ctx context.Context, q store.UpdateKeyStatesQuery) error } +func (s *stubKeyStore) CreateKey(ctx context.Context, key model.Key) error { + return s.createKey(ctx, key) +} + func (s *stubKeyStore) GetKeyByID(ctx context.Context, id, tenantID string) (*model.Key, error) { return s.getKeyByID(ctx, id, tenantID) } @@ -94,3 +99,182 @@ func TestUpdateKeyState(t *testing.T) { }) } } + +func TestUpsertKey(t *testing.T) { + errBoom := errors.New("boom") + + parentID := "parent-1" + otherParent := "other-parent" + + newKey := model.Key{ + ID: testKeyID, + Name: "some-name", + TenantID: testTenantID, + Kind: "K1", + ParentID: &parentID, + ManagedBy: "agent-aws", + LifeCycleState: model.KeyLifeCyclePreActivation, + KeyProcessingState: model.KeyProcessingState{Status: model.KeyProcessingCompleted}, + } + + tests := []struct { + name string + + createErr error + existing *model.Key + getKeyErr error + updateErr error + + wantErrIs []error + wantErrIsNot []error + wantNil bool + }{ + { + name: "generic store error on insert", + createErr: errBoom, + wantErrIs: []error{keyoperator.ErrCreateKey, errBoom}, + wantErrIsNot: []error{keyoperator.ErrKeyConflict, store.ErrKeyInsertConflict}, + }, + { + name: "conflict then GetKeyByID fails", + createErr: store.ErrKeyInsertConflict, + getKeyErr: errBoom, + wantErrIs: []error{keyoperator.ErrGetKey, errBoom}, + }, + { + name: "conflict with different name", + createErr: store.ErrKeyInsertConflict, + existing: &model.Key{ + ID: testKeyID, + Name: "different-name", + TenantID: testTenantID, + Kind: "K1", + ParentID: &parentID, + ManagedBy: "agent-aws", + LifeCycleState: model.KeyLifeCyclePreActivation, + KeyProcessingState: model.KeyProcessingState{Status: model.KeyProcessingPending}, + }, + wantErrIs: []error{keyoperator.ErrKeyConflict}, + }, + { + name: "conflict with different managed_by", + createErr: store.ErrKeyInsertConflict, + existing: &model.Key{ + ID: testKeyID, + Name: "some-name", + TenantID: testTenantID, + Kind: "K1", + ParentID: &parentID, + ManagedBy: "other-agent", + LifeCycleState: model.KeyLifeCyclePreActivation, + KeyProcessingState: model.KeyProcessingState{Status: model.KeyProcessingPending}, + }, + wantErrIs: []error{keyoperator.ErrKeyConflict}, + }, + { + name: "conflict with different parent_id", + createErr: store.ErrKeyInsertConflict, + existing: &model.Key{ + ID: testKeyID, + Name: "some-name", + TenantID: testTenantID, + Kind: "K1", + ParentID: &otherParent, + ManagedBy: "agent-aws", + LifeCycleState: model.KeyLifeCyclePreActivation, + KeyProcessingState: model.KeyProcessingState{Status: model.KeyProcessingPending}, + }, + wantErrIs: []error{keyoperator.ErrKeyConflict}, + }, + { + name: "conflict same identity then update store error", + createErr: store.ErrKeyInsertConflict, + existing: &model.Key{ + ID: testKeyID, + Name: "some-name", + TenantID: testTenantID, + Kind: "K1", + ParentID: &parentID, + ManagedBy: "agent-aws", + LifeCycleState: model.KeyLifeCyclePreActivation, + KeyProcessingState: model.KeyProcessingState{Status: model.KeyProcessingPending}, + }, + updateErr: errBoom, + wantErrIs: []error{keyoperator.ErrUpdateKeyState, errBoom}, + wantErrIsNot: []error{keyoperator.ErrKeyTransitionRejected}, + }, + { + name: "happy path insert succeeds", + createErr: nil, + wantNil: true, + }, + { + name: "conflict same identity CAS updates row", + createErr: store.ErrKeyInsertConflict, + existing: &model.Key{ + ID: testKeyID, + Name: "some-name", + TenantID: testTenantID, + Kind: "K1", + ParentID: &parentID, + ManagedBy: "agent-aws", + LifeCycleState: model.KeyLifeCyclePreActivation, + KeyProcessingState: model.KeyProcessingState{Status: model.KeyProcessingPending}, + }, + updateErr: nil, + wantNil: true, + }, + { + name: "conflict same identity idempotent replay (CAS matched zero rows)", + createErr: store.ErrKeyInsertConflict, + existing: &model.Key{ + ID: testKeyID, + Name: "some-name", + TenantID: testTenantID, + Kind: "K1", + ParentID: &parentID, + ManagedBy: "agent-aws", + LifeCycleState: model.KeyLifeCyclePreActivation, + KeyProcessingState: model.KeyProcessingState{Status: model.KeyProcessingCompleted}, + }, + updateErr: store.ErrKeyNotFound, + wantNil: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + keys := &stubKeyStore{ + createKey: func(_ context.Context, _ model.Key) error { + return tc.createErr + }, + getKeyByID: func(_ context.Context, _, _ string) (*model.Key, error) { + if tc.getKeyErr != nil { + return nil, tc.getKeyErr + } + return tc.existing, nil + }, + updateKeyStates: func(_ context.Context, _ store.UpdateKeyStatesQuery) error { + return tc.updateErr + }, + } + + step := keyoperator.UpsertKey(newKey) + err := step(t.Context(), store.Stores{Keys: keys}) + + if tc.wantNil { + assert.NoError(t, err) + return + } + if !assert.Error(t, err) { + return + } + for _, s := range tc.wantErrIs { + assert.ErrorIs(t, err, s) + } + for _, s := range tc.wantErrIsNot { + assert.NotErrorIs(t, err, s, "unexpected: err matches %v", s) + } + }) + } +} diff --git a/pkg/api/v1/proto/agents/keys/errmap.go b/pkg/api/v1/proto/agents/keys/errmap.go new file mode 100644 index 00000000..04db9bf1 --- /dev/null +++ b/pkg/api/v1/proto/agents/keys/errmap.go @@ -0,0 +1,33 @@ +package keys + +import ( + "errors" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/openkcm/krypton/internal/keyoperator" + "github.com/openkcm/krypton/pkg/api/v1/proto" + "github.com/openkcm/krypton/pkg/store" + "github.com/openkcm/krypton/pkg/validator" +) + +func mapToProtoErr(err error) error { + switch { + case errors.Is(err, store.ErrTenantNotFound): + return proto.ErrDetailsWithCode( + status.New(codes.FailedPrecondition, validator.ErrInvalidTenantID.Error()), + proto.Code_ERROR_CODE_ABORT, + ) + case errors.Is(err, keyoperator.ErrKeyConflict): + return proto.ErrDetailsWithCode( + status.New(codes.FailedPrecondition, keyoperator.ErrKeyConflict.Error()), + proto.Code_ERROR_CODE_ABORT, + ) + } + + return proto.ErrDetailsWithCode( + status.New(codes.Internal, err.Error()), + proto.Code_ERROR_CODE_RETRY, + ) +} diff --git a/pkg/api/v1/proto/agents/keys/keys.pb.go b/pkg/api/v1/proto/agents/keys/keys.pb.go new file mode 100644 index 00000000..e44e9083 --- /dev/null +++ b/pkg/api/v1/proto/agents/keys/keys.pb.go @@ -0,0 +1,236 @@ +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.36.12 +// protoc v7.36.1 +// source: keys.proto + +package keys + +import ( + reflect "reflect" + sync "sync" + unsafe "unsafe" + + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +type UpsertKeyRequest struct { + state protoimpl.MessageState `protogen:"open.v1"` + TenantId string `protobuf:"bytes,1,opt,name=tenant_id,json=tenantId,proto3" json:"tenant_id,omitempty"` + KeyId string `protobuf:"bytes,2,opt,name=key_id,json=keyId,proto3" json:"key_id,omitempty"` + Kind string `protobuf:"bytes,3,opt,name=kind,proto3" json:"kind,omitempty"` + Name string `protobuf:"bytes,4,opt,name=name,proto3" json:"name,omitempty"` + ParentId string `protobuf:"bytes,5,opt,name=parent_id,json=parentId,proto3" json:"parent_id,omitempty"` + LifecycleState string `protobuf:"bytes,6,opt,name=lifecycle_state,json=lifecycleState,proto3" json:"lifecycle_state,omitempty"` + ManagedBy string `protobuf:"bytes,7,opt,name=managed_by,json=managedBy,proto3" json:"managed_by,omitempty"` + Labels map[string]string `protobuf:"bytes,8,rep,name=labels,proto3" json:"labels,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *UpsertKeyRequest) Reset() { + *x = UpsertKeyRequest{} + mi := &file_keys_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *UpsertKeyRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*UpsertKeyRequest) ProtoMessage() {} + +func (x *UpsertKeyRequest) ProtoReflect() protoreflect.Message { + mi := &file_keys_proto_msgTypes[0] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use UpsertKeyRequest.ProtoReflect.Descriptor instead. +func (*UpsertKeyRequest) Descriptor() ([]byte, []int) { + return file_keys_proto_rawDescGZIP(), []int{0} +} + +func (x *UpsertKeyRequest) GetTenantId() string { + if x != nil { + return x.TenantId + } + return "" +} + +func (x *UpsertKeyRequest) GetKeyId() string { + if x != nil { + return x.KeyId + } + return "" +} + +func (x *UpsertKeyRequest) GetKind() string { + if x != nil { + return x.Kind + } + return "" +} + +func (x *UpsertKeyRequest) GetName() string { + if x != nil { + return x.Name + } + return "" +} + +func (x *UpsertKeyRequest) GetParentId() string { + if x != nil { + return x.ParentId + } + return "" +} + +func (x *UpsertKeyRequest) GetLifecycleState() string { + if x != nil { + return x.LifecycleState + } + return "" +} + +func (x *UpsertKeyRequest) GetManagedBy() string { + if x != nil { + return x.ManagedBy + } + return "" +} + +func (x *UpsertKeyRequest) GetLabels() map[string]string { + if x != nil { + return x.Labels + } + return nil +} + +type UpsertKeyResponse struct { + state protoimpl.MessageState `protogen:"open.v1"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *UpsertKeyResponse) Reset() { + *x = UpsertKeyResponse{} + mi := &file_keys_proto_msgTypes[1] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *UpsertKeyResponse) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*UpsertKeyResponse) ProtoMessage() {} + +func (x *UpsertKeyResponse) ProtoReflect() protoreflect.Message { + mi := &file_keys_proto_msgTypes[1] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use UpsertKeyResponse.ProtoReflect.Descriptor instead. +func (*UpsertKeyResponse) Descriptor() ([]byte, []int) { + return file_keys_proto_rawDescGZIP(), []int{1} +} + +var File_keys_proto protoreflect.FileDescriptor + +const file_keys_proto_rawDesc = "" + + "\n" + + "\n" + + "keys.proto\x12\x16krypton.v1.agents.keys\"\xdc\x02\n" + + "\x10UpsertKeyRequest\x12\x1b\n" + + "\ttenant_id\x18\x01 \x01(\tR\btenantId\x12\x15\n" + + "\x06key_id\x18\x02 \x01(\tR\x05keyId\x12\x12\n" + + "\x04kind\x18\x03 \x01(\tR\x04kind\x12\x12\n" + + "\x04name\x18\x04 \x01(\tR\x04name\x12\x1b\n" + + "\tparent_id\x18\x05 \x01(\tR\bparentId\x12'\n" + + "\x0flifecycle_state\x18\x06 \x01(\tR\x0elifecycleState\x12\x1d\n" + + "\n" + + "managed_by\x18\a \x01(\tR\tmanagedBy\x12L\n" + + "\x06labels\x18\b \x03(\v24.krypton.v1.agents.keys.UpsertKeyRequest.LabelsEntryR\x06labels\x1a9\n" + + "\vLabelsEntry\x12\x10\n" + + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\x13\n" + + "\x11UpsertKeyResponse2p\n" + + "\n" + + "KeyService\x12b\n" + + "\tUpsertKey\x12(.krypton.v1.agents.keys.UpsertKeyRequest\x1a).krypton.v1.agents.keys.UpsertKeyResponse\"\x00B9Z7github.com/openkcm/krypton/pkg/api/v1/proto/agents/keysb\x06proto3" + +var ( + file_keys_proto_rawDescOnce sync.Once + file_keys_proto_rawDescData []byte +) + +func file_keys_proto_rawDescGZIP() []byte { + file_keys_proto_rawDescOnce.Do(func() { + file_keys_proto_rawDescData = protoimpl.X.CompressGZIP(unsafe.Slice(unsafe.StringData(file_keys_proto_rawDesc), len(file_keys_proto_rawDesc))) + }) + return file_keys_proto_rawDescData +} + +var file_keys_proto_msgTypes = make([]protoimpl.MessageInfo, 3) +var file_keys_proto_goTypes = []any{ + (*UpsertKeyRequest)(nil), // 0: krypton.v1.agents.keys.UpsertKeyRequest + (*UpsertKeyResponse)(nil), // 1: krypton.v1.agents.keys.UpsertKeyResponse + nil, // 2: krypton.v1.agents.keys.UpsertKeyRequest.LabelsEntry +} +var file_keys_proto_depIdxs = []int32{ + 2, // 0: krypton.v1.agents.keys.UpsertKeyRequest.labels:type_name -> krypton.v1.agents.keys.UpsertKeyRequest.LabelsEntry + 0, // 1: krypton.v1.agents.keys.KeyService.UpsertKey:input_type -> krypton.v1.agents.keys.UpsertKeyRequest + 1, // 2: krypton.v1.agents.keys.KeyService.UpsertKey:output_type -> krypton.v1.agents.keys.UpsertKeyResponse + 2, // [2:3] is the sub-list for method output_type + 1, // [1:2] is the sub-list for method input_type + 1, // [1:1] is the sub-list for extension type_name + 1, // [1:1] is the sub-list for extension extendee + 0, // [0:1] is the sub-list for field type_name +} + +func init() { file_keys_proto_init() } +func file_keys_proto_init() { + if File_keys_proto != nil { + return + } + type x struct{} + out := protoimpl.TypeBuilder{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: unsafe.Slice(unsafe.StringData(file_keys_proto_rawDesc), len(file_keys_proto_rawDesc)), + NumEnums: 0, + NumMessages: 3, + NumExtensions: 0, + NumServices: 1, + }, + GoTypes: file_keys_proto_goTypes, + DependencyIndexes: file_keys_proto_depIdxs, + MessageInfos: file_keys_proto_msgTypes, + }.Build() + File_keys_proto = out.File + file_keys_proto_goTypes = nil + file_keys_proto_depIdxs = nil +} diff --git a/pkg/api/v1/proto/agents/keys/keys_grpc.pb.go b/pkg/api/v1/proto/agents/keys/keys_grpc.pb.go new file mode 100644 index 00000000..1e5dc686 --- /dev/null +++ b/pkg/api/v1/proto/agents/keys/keys_grpc.pb.go @@ -0,0 +1,122 @@ +// Code generated by protoc-gen-go-grpc. DO NOT EDIT. +// versions: +// - protoc-gen-go-grpc v1.6.2 +// - protoc v7.36.1 +// source: keys.proto + +package keys + +import ( + context "context" + + grpc "google.golang.org/grpc" + codes "google.golang.org/grpc/codes" + status "google.golang.org/grpc/status" +) + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the grpc package it is being compiled against. +// Requires gRPC-Go v1.64.0 or later. +const _ = grpc.SupportPackageIsVersion9 + +const ( + KeyService_UpsertKey_FullMethodName = "/krypton.v1.agents.keys.KeyService/UpsertKey" +) + +// KeyServiceClient is the client API for KeyService service. +// +// For semantics around ctx use and closing/ending streaming RPCs, please refer to https://pkg.go.dev/google.golang.org/grpc/?tab=doc#ClientConn.NewStream. +type KeyServiceClient interface { + UpsertKey(ctx context.Context, in *UpsertKeyRequest, opts ...grpc.CallOption) (*UpsertKeyResponse, error) +} + +type keyServiceClient struct { + cc grpc.ClientConnInterface +} + +func NewKeyServiceClient(cc grpc.ClientConnInterface) KeyServiceClient { + return &keyServiceClient{cc} +} + +func (c *keyServiceClient) UpsertKey(ctx context.Context, in *UpsertKeyRequest, opts ...grpc.CallOption) (*UpsertKeyResponse, error) { + cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) + out := new(UpsertKeyResponse) + err := c.cc.Invoke(ctx, KeyService_UpsertKey_FullMethodName, in, out, cOpts...) + if err != nil { + return nil, err + } + return out, nil +} + +// KeyServiceServer is the server API for KeyService service. +// All implementations must embed UnimplementedKeyServiceServer +// for forward compatibility. +type KeyServiceServer interface { + UpsertKey(context.Context, *UpsertKeyRequest) (*UpsertKeyResponse, error) + mustEmbedUnimplementedKeyServiceServer() +} + +// UnimplementedKeyServiceServer must be embedded to have +// forward compatible implementations. +// +// NOTE: this should be embedded by value instead of pointer to avoid a nil +// pointer dereference when methods are called. +type UnimplementedKeyServiceServer struct{} + +func (UnimplementedKeyServiceServer) UpsertKey(context.Context, *UpsertKeyRequest) (*UpsertKeyResponse, error) { + return nil, status.Error(codes.Unimplemented, "method UpsertKey not implemented") +} +func (UnimplementedKeyServiceServer) mustEmbedUnimplementedKeyServiceServer() {} +func (UnimplementedKeyServiceServer) testEmbeddedByValue() {} + +// UnsafeKeyServiceServer may be embedded to opt out of forward compatibility for this service. +// Use of this interface is not recommended, as added methods to KeyServiceServer will +// result in compilation errors. +type UnsafeKeyServiceServer interface { + mustEmbedUnimplementedKeyServiceServer() +} + +func RegisterKeyServiceServer(s grpc.ServiceRegistrar, srv KeyServiceServer) { + // If the following call panics, it indicates UnimplementedKeyServiceServer was + // embedded by pointer and is nil. This will cause panics if an + // unimplemented method is ever invoked, so we test this at initialization + // time to prevent it from happening at runtime later due to I/O. + if t, ok := srv.(interface{ testEmbeddedByValue() }); ok { + t.testEmbeddedByValue() + } + s.RegisterService(&KeyService_ServiceDesc, srv) +} + +func _KeyService_UpsertKey_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { + in := new(UpsertKeyRequest) + if err := dec(in); err != nil { + return nil, err + } + if interceptor == nil { + return srv.(KeyServiceServer).UpsertKey(ctx, in) + } + info := &grpc.UnaryServerInfo{ + Server: srv, + FullMethod: KeyService_UpsertKey_FullMethodName, + } + handler := func(ctx context.Context, req interface{}) (interface{}, error) { + return srv.(KeyServiceServer).UpsertKey(ctx, req.(*UpsertKeyRequest)) + } + return interceptor(ctx, in, info, handler) +} + +// KeyService_ServiceDesc is the grpc.ServiceDesc for KeyService service. +// It's only intended for direct use with grpc.RegisterService, +// and not to be introspected or modified (even as a copy) +var KeyService_ServiceDesc = grpc.ServiceDesc{ + ServiceName: "krypton.v1.agents.keys.KeyService", + HandlerType: (*KeyServiceServer)(nil), + Methods: []grpc.MethodDesc{ + { + MethodName: "UpsertKey", + Handler: _KeyService_UpsertKey_Handler, + }, + }, + Streams: []grpc.StreamDesc{}, + Metadata: "keys.proto", +} diff --git a/pkg/api/v1/proto/agents/keys/service.go b/pkg/api/v1/proto/agents/keys/service.go new file mode 100644 index 00000000..7dbd44b0 --- /dev/null +++ b/pkg/api/v1/proto/agents/keys/service.go @@ -0,0 +1,77 @@ +package keys + +import ( + "context" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/openkcm/krypton/internal/clock" + "github.com/openkcm/krypton/internal/keyoperator" + "github.com/openkcm/krypton/pkg/api/v1/proto" + "github.com/openkcm/krypton/pkg/model" + "github.com/openkcm/krypton/pkg/store" + "github.com/openkcm/krypton/pkg/validator" +) + +type Service struct { + UnimplementedKeyServiceServer + + transactor store.Transactor +} + +// NewKeyService constructs the agent-side KeyService. +func NewKeyService(t store.Transactor) *Service { + return &Service{ + transactor: t, + } +} + +// UpsertKey creates or reconciles a key on this agent. +// Repeated calls with the same identity are idempotent. +func (s *Service) UpsertKey(ctx context.Context, req *UpsertKeyRequest) (*UpsertKeyResponse, error) { + err := validator.ValidateKeyUpsert(validator.UpsertKeyInput{ + TenantID: req.GetTenantId(), + KeyID: req.GetKeyId(), + Kind: req.GetKind(), + Name: req.GetName(), + ManagedBy: req.GetManagedBy(), + ParentID: req.GetParentId(), + LifecycleState: req.GetLifecycleState(), + }) + if err != nil { + return nil, proto.ErrDetailsWithCode( + status.New(codes.InvalidArgument, err.Error()), + proto.Code_ERROR_CODE_ABORT, + ) + } + + newKey := newKey(req) + err = store.ChainTransaction(ctx, s.transactor, + validator.ValidateTenant(newKey.TenantID), + keyoperator.UpsertKey(newKey), + ) + if err != nil { + return nil, mapToProtoErr(err) + } + + return &UpsertKeyResponse{}, nil +} + +func newKey(req *UpsertKeyRequest) model.Key { + parentID := req.GetParentId() + now := clock.Now() + return model.Key{ + ID: req.GetKeyId(), + Name: req.GetName(), + TenantID: req.GetTenantId(), + Kind: model.KeyKind(req.GetKind()), + ParentID: &parentID, + ManagedBy: req.GetManagedBy(), + Labels: req.GetLabels(), + LifeCycleState: model.KeyLifeCycleState(req.GetLifecycleState()), + KeyProcessingState: model.KeyProcessingState{Status: model.KeyProcessingCompleted}, + CreatedAt: now, + UpdatedAt: now, + } +} diff --git a/pkg/api/v1/proto/agents/keys/service_test.go b/pkg/api/v1/proto/agents/keys/service_test.go new file mode 100644 index 00000000..4eda22b6 --- /dev/null +++ b/pkg/api/v1/proto/agents/keys/service_test.go @@ -0,0 +1,144 @@ +package keys_test + +import ( + "testing" + "uuid" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/openkcm/krypton/internal/keyoperator" + "github.com/openkcm/krypton/pkg/api/v1/proto" + "github.com/openkcm/krypton/pkg/model" +) + +func TestUpsertKey(t *testing.T) { + ctx := t.Context() + + t.Run("should return InvalidArgument when tenant_id is empty", func(t *testing.T) { + setup := setupServerAndClient(t) + + req := validUpsertRequest(uuid.New().String()) + req.TenantId = "" + + _, err := setup.cli.UpsertKey(ctx, req) + + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err), err.Error()) + assertErrorDetails(t, proto.Code_ERROR_CODE_ABORT, err) + }) + + t.Run("should return InvalidArgument when parent_id is malformed", func(t *testing.T) { + setup := setupServerAndClient(t) + + req := validUpsertRequest(uuid.New().String()) + req.ParentId = "not-a-uuid" + + _, err := setup.cli.UpsertKey(ctx, req) + + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err), err.Error()) + assertErrorDetails(t, proto.Code_ERROR_CODE_ABORT, err) + }) + + t.Run("should return InvalidArgument when lifecycle_state is unknown", func(t *testing.T) { + setup := setupServerAndClient(t) + + req := validUpsertRequest(uuid.New().String()) + req.LifecycleState = "bogus" + + _, err := setup.cli.UpsertKey(ctx, req) + + require.Error(t, err) + assert.Equal(t, codes.InvalidArgument, status.Code(err), err.Error()) + assertErrorDetails(t, proto.Code_ERROR_CODE_ABORT, err) + }) + + t.Run("should return FailedPrecondition when tenant does not exist", func(t *testing.T) { + setup := setupServerAndClient(t) + + req := validUpsertRequest(uuid.New().String()) + + _, err := setup.cli.UpsertKey(ctx, req) + + require.Error(t, err) + assert.Equal(t, codes.FailedPrecondition, status.Code(err), err.Error()) + assertErrorDetails(t, proto.Code_ERROR_CODE_ABORT, err) + }) + + t.Run("should return FailedPrecondition on identity conflict", func(t *testing.T) { + setup := setupServerAndClient(t) + tenant := createTenant(t, setup.tenantStore) + + req := validUpsertRequest(tenant.ID) + + // Pre-insert a row with the same (tenant, key_id) but a different Name. + parentID := req.GetParentId() + existing := model.Key{ + ID: req.GetKeyId(), + Name: "different-name", + TenantID: tenant.ID, + Kind: model.KeyKind(req.GetKind()), + ParentID: &parentID, + ManagedBy: req.GetManagedBy(), + LifeCycleState: model.KeyLifeCyclePreActivation, + KeyProcessingState: model.KeyProcessingState{Status: model.KeyProcessingCompleted}, + } + require.NoError(t, setup.keyStore.CreateKey(ctx, existing)) + + _, err := setup.cli.UpsertKey(ctx, req) + + require.Error(t, err) + assert.Equal(t, codes.FailedPrecondition, status.Code(err), err.Error()) + assertErrorDetails(t, proto.Code_ERROR_CODE_ABORT, err) + assert.Contains(t, status.Convert(err).Message(), keyoperator.ErrKeyConflict.Error()) + }) + + t.Run("should create a new key", func(t *testing.T) { + setup := setupServerAndClient(t) + tenant := createTenant(t, setup.tenantStore) + + req := validUpsertRequest(tenant.ID) + + _, err := setup.cli.UpsertKey(ctx, req) + require.NoError(t, err) + + got, err := setup.keyStore.GetKeyByID(ctx, req.GetKeyId(), tenant.ID) + require.NoError(t, err) + + assert.Equal(t, req.GetKeyId(), got.ID) + assert.Equal(t, req.GetName(), got.Name) + assert.Equal(t, model.KeyKind(req.GetKind()), got.Kind) + assert.Equal(t, req.GetManagedBy(), got.ManagedBy) + require.NotNil(t, got.ParentID) + assert.Equal(t, req.GetParentId(), *got.ParentID) + assert.Equal(t, model.KeyLifeCyclePreActivation, got.LifeCycleState) + assert.Equal(t, model.KeyProcessingCompleted, got.KeyProcessingState.Status) + assert.Equal(t, "test", got.Labels["env"]) + }) + + t.Run("should be idempotent on repeated call with same identity", func(t *testing.T) { + setup := setupServerAndClient(t) + tenant := createTenant(t, setup.tenantStore) + + req := validUpsertRequest(tenant.ID) + + _, err := setup.cli.UpsertKey(ctx, req) + require.NoError(t, err) + + first, err := setup.keyStore.GetKeyByID(ctx, req.GetKeyId(), tenant.ID) + require.NoError(t, err) + + _, err = setup.cli.UpsertKey(ctx, req) + require.NoError(t, err) + + second, err := setup.keyStore.GetKeyByID(ctx, req.GetKeyId(), tenant.ID) + require.NoError(t, err) + + assert.Equal(t, first.ID, second.ID) + assert.Equal(t, first.Name, second.Name) + assert.Equal(t, model.KeyProcessingCompleted, second.KeyProcessingState.Status) + }) +} diff --git a/pkg/api/v1/proto/agents/keys/setup_test.go b/pkg/api/v1/proto/agents/keys/setup_test.go new file mode 100644 index 00000000..820e64ce --- /dev/null +++ b/pkg/api/v1/proto/agents/keys/setup_test.go @@ -0,0 +1,181 @@ +package keys_test + +import ( + "context" + "database/sql" + "net" + "os" + "strings" + "testing" + "uuid" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/testcontainers/testcontainers-go/modules/postgres" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" + "google.golang.org/grpc/test/bufconn" + + _ "github.com/lib/pq" + + "github.com/openkcm/krypton/pkg/api/v1/proto" + "github.com/openkcm/krypton/pkg/api/v1/proto/agents/keys" + "github.com/openkcm/krypton/pkg/model" + "github.com/openkcm/krypton/pkg/store" + storesql "github.com/openkcm/krypton/pkg/store/sql" +) + +var pgConnStr string + +func TestMain(m *testing.M) { + pgCleanup, err := setupPostgres() + if err != nil { + os.Exit(1) + } + + exitCode := m.Run() + pgCleanup() + os.Exit(exitCode) +} + +func setupPostgres() (func(), error) { + ctx := context.Background() + + pgContainer, err := postgres.Run(ctx, + "postgres:18-alpine", + postgres.WithDatabase("postgres"), + postgres.WithUsername("testuser"), + postgres.WithPassword("testpass"), + postgres.BasicWaitStrategies(), + ) + if err != nil { + return nil, err + } + cleanUp := func() { _ = pgContainer.Terminate(ctx) } + + pgConnStr, err = pgContainer.ConnectionString(ctx, "sslmode=disable") + + return cleanUp, err +} + +type serviceSetup struct { + transactor store.Transactor + tenantStore store.Tenant + keyStore store.Key + cli keys.KeyServiceClient +} + +func setupServerAndClient(t *testing.T) *serviceSetup { + t.Helper() + ctx := t.Context() + + db := createDatabase(t) + require.NoError(t, storesql.Migrate(ctx, db)) + + setup := &serviceSetup{ + transactor: storesql.NewTransactor(db), + tenantStore: storesql.NewTenantStore(db), + keyStore: storesql.NewKeyStore(db), + } + + srv := grpc.NewServer() + keys.RegisterKeyServiceServer(srv, keys.NewKeyService(setup.transactor)) + + const bufSize = 1024 * 1024 + lis := bufconn.Listen(bufSize) + go func() { + if err := srv.Serve(lis); err != nil { + assert.Fail(t, "agent key service server error", err) + } + }() + dialer := func(context.Context, string) (net.Conn, error) { + return lis.Dial() + } + + t.Cleanup(func() { + srv.GracefulStop() + }) + + conn, err := grpc.NewClient( + "passthrough:///bufconn", + grpc.WithContextDialer(dialer), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + require.NoError(t, err) + + t.Cleanup(func() { + conn.Close() + }) + + setup.cli = keys.NewKeyServiceClient(conn) + return setup +} + +func createDatabase(t *testing.T) *sql.DB { + t.Helper() + ctx := t.Context() + + db, err := sql.Open("postgres", pgConnStr) + if err != nil { + assert.FailNowf(t, "failed to connect to PostgreSQL", "error: %v", err) + } + + dbName := "test_" + strings.ReplaceAll(uuid.New().String(), "-", "") + _, err = db.ExecContext(ctx, "CREATE DATABASE "+dbName) + if err != nil { + db.Close() + assert.FailNowf(t, "failed to create test database", "error: %v", err) + } + db.Close() + + pgConStr := strings.Replace(pgConnStr, "/postgres?", "/"+dbName+"?", 1) + sqlDB, err := sql.Open("postgres", pgConStr) + if err != nil { + assert.FailNowf(t, "failed to connect to test database", "error: %v", err) + } + + t.Cleanup(func() { + sqlDB.Close() + + db, err := sql.Open("postgres", pgConnStr) + if err == nil { + _, _ = db.ExecContext(context.Background(), "DROP DATABASE "+dbName) + db.Close() + } + }) + return sqlDB +} + +func assertErrorDetails(t *testing.T, expCode proto.Code, actErr error) { + t.Helper() + + st := status.Convert(actErr) + dts := st.Details() + require.Len(t, dts, 1, "expected 1 error detail") + + dt, ok := dts[0].(*proto.ErrorDetails) + require.True(t, ok, "expected error details of type proto.ErrorDetails") + assert.Equal(t, expCode, dt.GetCode()) +} + +func createTenant(t *testing.T, s store.Tenant) model.Tenant { + t.Helper() + tenant := model.NewTenant("test-tenant-"+uuid.New().String(), nil) + result, err := s.CreateTenant(t.Context(), store.CreateTenantQuery{Tenant: tenant}) + require.NoError(t, err) + return result.Tenant +} + +func validUpsertRequest(tenantID string) *keys.UpsertKeyRequest { + return &keys.UpsertKeyRequest{ + TenantId: tenantID, + KeyId: uuid.New().String(), + Kind: "K1", + Name: "test-key-" + uuid.New().String(), + ParentId: uuid.New().String(), + LifecycleState: string(model.KeyLifeCyclePreActivation), + ManagedBy: "agent-aws", + Labels: map[string]string{"env": "test"}, + } +} diff --git a/pkg/store/key.go b/pkg/store/key.go index 840988b3..8d528e46 100644 --- a/pkg/store/key.go +++ b/pkg/store/key.go @@ -9,7 +9,7 @@ import ( var ( ErrKeyNotFound = errors.New("key not found") - ErrKeyAlreadyExists = errors.New("key already exists") + ErrKeyInsertConflict = errors.New("key insert skipped due to conflict") ) type Key interface { diff --git a/pkg/store/sql/key.go b/pkg/store/sql/key.go index 273b1520..5507467b 100644 --- a/pkg/store/sql/key.go +++ b/pkg/store/sql/key.go @@ -8,17 +8,11 @@ import ( "fmt" "strings" - "github.com/lib/pq" - "github.com/openkcm/krypton/internal/clock" "github.com/openkcm/krypton/pkg/model" "github.com/openkcm/krypton/pkg/store" ) -// pgErrCodeUniqueViolation is the SQLSTATE value returned by Postgres on a -// unique-constraint violation (23505). -const pgErrCodeUniqueViolation = "23505" - type KeyStore struct { db DBTX } @@ -33,6 +27,8 @@ func (ks *KeyStore) CreateKey(ctx context.Context, key model.Key) error { stmt := ` INSERT INTO keys (id, tenant_id, kind, name, parent_id, managed_by, labels, life_cycle_state, processing_status, processing_job_id, created_at, updated_at) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) + -- keeps the enclosing transaction alive on conflict so callers can reconcile + ON CONFLICT DO NOTHING ` labelsJSON, err := json.Marshal(key.Labels) @@ -40,7 +36,7 @@ func (ks *KeyStore) CreateKey(ctx context.Context, key model.Key) error { return err } - _, err = ks.db.ExecContext( + res, err := ks.db.ExecContext( ctx, stmt, key.ID, key.TenantID, @@ -56,12 +52,15 @@ func (ks *KeyStore) CreateKey(ctx context.Context, key model.Key) error { key.UpdatedAt, ) if err != nil { - var pqErr *pq.Error - if errors.As(err, &pqErr) && string(pqErr.Code) == pgErrCodeUniqueViolation { - return store.ErrKeyAlreadyExists - } return err } + n, err := res.RowsAffected() + if err != nil { + return err + } + if n == 0 { + return store.ErrKeyInsertConflict + } return nil } diff --git a/pkg/store/sql/key_test.go b/pkg/store/sql/key_test.go index 930d1a54..f44afc12 100644 --- a/pkg/store/sql/key_test.go +++ b/pkg/store/sql/key_test.go @@ -105,14 +105,6 @@ func TestCreateKey(t *testing.T) { assert.NoError(t, err) }) - t.Run("should fail with invalid parent reference", func(t *testing.T) { - badParent := uuid.New().String() - key := model.NewKey(tenant.ID, "orphan-key", "K1", &badParent, "root", nil) - - err := keyStore.CreateKey(ctx, key) - assert.Error(t, err) - }) - t.Run("should fail with invalid tenant reference", func(t *testing.T) { key := model.NewKey(uuid.New().String(), "bad-tenant-key", "K0", nil, "root", nil) @@ -304,7 +296,7 @@ func TestCreateKey_DuplicateName(t *testing.T) { second := model.NewKey(tenant.ID, "dup-name", "K0", nil, "root", nil) err = keyStore.CreateKey(ctx, second) - assert.ErrorIs(t, err, store.ErrKeyAlreadyExists) + assert.ErrorIs(t, err, store.ErrKeyInsertConflict) } func TestGetKeyByName(t *testing.T) { diff --git a/pkg/store/sql/migrate.go b/pkg/store/sql/migrate.go index 7c26a0f7..c923fe0b 100644 --- a/pkg/store/sql/migrate.go +++ b/pkg/store/sql/migrate.go @@ -43,8 +43,7 @@ CREATE TABLE IF NOT EXISTS keys ( updated_at BIGINT NOT NULL, UNIQUE (tenant_id, name), - UNIQUE (tenant_id, id), - FOREIGN KEY (tenant_id, parent_id) REFERENCES keys(tenant_id, id) + UNIQUE (tenant_id, id) ); ` diff --git a/pkg/validator/key_validator.go b/pkg/validator/key_validator.go index c0a9ca81..5eea7fb2 100644 --- a/pkg/validator/key_validator.go +++ b/pkg/validator/key_validator.go @@ -40,6 +40,11 @@ var ( ErrParentKeyTransientState = errors.New("parent key is in a transient state, please wait for the parent key to be completed") ErrKeyTransientState = errors.New("key is in a transient state, please wait for the key to be completed") ErrNoParentKeyRelation = errors.New("key has no parent key relation") + + ErrEmptyManagedBy = errors.New("managed_by cannot be empty") + ErrEmptyLifecycleState = errors.New("lifecycle_state cannot be empty") + ErrInvalidParentID = errors.New("parent_id is invalid") + ErrUnknownLifecycleState = errors.New("lifecycle_state is not a known state") ) func NewValidator(rootSegment spec.HierarchySegment, topology spec.Topology, hierarchy spec.KeyHierarchy, tenants store.Tenant, keys store.Key) KeyValidator { @@ -60,6 +65,41 @@ type AnnounceInput struct { TargetName string } +type UpsertKeyInput struct { + TenantID string + KeyID string + Kind string + Name string + ManagedBy string + ParentID string + LifecycleState string +} + +// ValidateKeyUpsert verifies the shape of an agent-side UpsertKey +// request. All fields are required. +func ValidateKeyUpsert(input UpsertKeyInput) error { + switch { + case !isValidUUID(input.TenantID): + return ErrInvalidTenantID + case !isValidUUID(input.KeyID): + return ErrInvalidKeyID + case input.Kind == "": + return ErrEmptyKeyKind + case input.Name == "": + return ErrEmptyName + case input.ManagedBy == "": + return ErrEmptyManagedBy + case !isValidUUID(input.ParentID): + return ErrInvalidParentID + case input.LifecycleState == "": + return ErrEmptyLifecycleState + } + if !keylifecycle.IsKnown(model.KeyLifeCycleState(input.LifecycleState)) { + return ErrUnknownLifecycleState + } + return nil +} + func (v *keyValidator) ValidateKeyAnnounce(ctx context.Context, input AnnounceInput) *ValidationError { ve := &ValidationError{ code: Invalid, diff --git a/pkg/validator/key_validator_test.go b/pkg/validator/key_validator_test.go index cba2149a..5cde05ee 100644 --- a/pkg/validator/key_validator_test.go +++ b/pkg/validator/key_validator_test.go @@ -282,6 +282,97 @@ func TestValidator_ValidateKeyAnnounce(t *testing.T) { } } +func TestValidator_ValidateKeyUpsert(t *testing.T) { + baseValid := validator.UpsertKeyInput{ + TenantID: validUUID, + KeyID: uuid.New().String(), + Kind: "K1", + Name: "some-name", + ManagedBy: "agent-aws", + ParentID: uuid.New().String(), + LifecycleState: string(model.KeyLifeCyclePreActivation), + } + + withField := func(mut func(*validator.UpsertKeyInput)) validator.UpsertKeyInput { + in := baseValid + mut(&in) + return in + } + + tests := []struct { + name string + input validator.UpsertKeyInput + wantErr error + }{ + { + name: "invalid tenantID", + input: withField(func(in *validator.UpsertKeyInput) { in.TenantID = invalidUUID }), + wantErr: validator.ErrInvalidTenantID, + }, + { + name: "invalid keyID", + input: withField(func(in *validator.UpsertKeyInput) { in.KeyID = invalidUUID }), + wantErr: validator.ErrInvalidKeyID, + }, + { + name: "empty kind", + input: withField(func(in *validator.UpsertKeyInput) { in.Kind = "" }), + wantErr: validator.ErrEmptyKeyKind, + }, + { + name: "empty name", + input: withField(func(in *validator.UpsertKeyInput) { in.Name = "" }), + wantErr: validator.ErrEmptyName, + }, + { + name: "empty managed_by", + input: withField(func(in *validator.UpsertKeyInput) { in.ManagedBy = "" }), + wantErr: validator.ErrEmptyManagedBy, + }, + { + name: "invalid parent_id", + input: withField(func(in *validator.UpsertKeyInput) { in.ParentID = invalidUUID }), + wantErr: validator.ErrInvalidParentID, + }, + { + name: "empty parent_id", + input: withField(func(in *validator.UpsertKeyInput) { in.ParentID = "" }), + wantErr: validator.ErrInvalidParentID, + }, + { + name: "empty lifecycle_state", + input: withField(func(in *validator.UpsertKeyInput) { in.LifecycleState = "" }), + wantErr: validator.ErrEmptyLifecycleState, + }, + { + name: "unknown lifecycle_state", + input: withField(func(in *validator.UpsertKeyInput) { in.LifecycleState = "bogus" }), + wantErr: validator.ErrUnknownLifecycleState, + }, + { + name: "valid with pre-activation", + input: baseValid, + wantErr: nil, + }, + { + name: "valid with active", + input: withField(func(in *validator.UpsertKeyInput) { in.LifecycleState = string(model.KeyLifeCycleActive) }), + wantErr: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := validator.ValidateKeyUpsert(tc.input) + if tc.wantErr == nil { + assert.NoError(t, err) + return + } + assert.ErrorIs(t, err, tc.wantErr) + }) + } +} + func TestValidator_ValidateActivateRequest(t *testing.T) { tests := []struct { name string From ff61dc48b87b2bced248e8deb25bf0d288dcf4f9 Mon Sep 17 00:00:00 2001 From: fabenan-f <63860771+fabenan-f@users.noreply.github.com> Date: Tue, 15 Sep 2026 17:04:50 +0200 Subject: [PATCH 2/3] fix: linting issue Signed-off-by: fabenan-f <63860771+fabenan-f@users.noreply.github.com> --- pkg/store/key.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/store/key.go b/pkg/store/key.go index 8d528e46..2ecd7f66 100644 --- a/pkg/store/key.go +++ b/pkg/store/key.go @@ -8,7 +8,7 @@ import ( ) var ( - ErrKeyNotFound = errors.New("key not found") + ErrKeyNotFound = errors.New("key not found") ErrKeyInsertConflict = errors.New("key insert skipped due to conflict") ) From 007c7268904e57dfa94022252de0d33d94ac02e4 Mon Sep 17 00:00:00 2001 From: fabenan-f <63860771+fabenan-f@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:05:10 +0200 Subject: [PATCH 3/3] fix: obscure internal error message Signed-off-by: fabenan-f <63860771+fabenan-f@users.noreply.github.com> --- pkg/api/v1/proto/admin/keys/errmap.go | 2 +- pkg/api/v1/proto/agents/keys/errmap.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/api/v1/proto/admin/keys/errmap.go b/pkg/api/v1/proto/admin/keys/errmap.go index 0f95d04a..8e2dc3f9 100644 --- a/pkg/api/v1/proto/admin/keys/errmap.go +++ b/pkg/api/v1/proto/admin/keys/errmap.go @@ -74,7 +74,7 @@ func mapToProtoErr(err error) error { errors.Is(err, keyoperator.ErrGetKey), errors.Is(err, keyoperator.ErrGetParentKeyVersion): return proto.ErrDetailsWithCode( - status.New(codes.Internal, err.Error()), + status.New(codes.Internal, "internal error"), proto.Code_ERROR_CODE_RETRY, ) } diff --git a/pkg/api/v1/proto/agents/keys/errmap.go b/pkg/api/v1/proto/agents/keys/errmap.go index 04db9bf1..dd35fb6c 100644 --- a/pkg/api/v1/proto/agents/keys/errmap.go +++ b/pkg/api/v1/proto/agents/keys/errmap.go @@ -27,7 +27,7 @@ func mapToProtoErr(err error) error { } return proto.ErrDetailsWithCode( - status.New(codes.Internal, err.Error()), + status.New(codes.Internal, "internal error"), proto.Code_ERROR_CODE_RETRY, ) }