From 5914bc257a40eed988024568474f2576722a16b0 Mon Sep 17 00:00:00 2001 From: Oleg Grishanovich Date: Thu, 14 May 2026 00:49:56 +0200 Subject: [PATCH 1/9] fix: improve error handling and logging in system mapping and unmapping functions --- internal/service/mapping.go | 91 +++++++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 35 deletions(-) diff --git a/internal/service/mapping.go b/internal/service/mapping.go index cc91a25..5ff3451 100644 --- a/internal/service/mapping.go +++ b/internal/service/mapping.go @@ -46,14 +46,16 @@ func (m *Mapping) UnmapSystemFromTenant(ctx context.Context, in *mappinggrpc.Unm defer cancel() err := m.repo.Transaction(ctxTimeout, func(ctx context.Context, r repository.Repository) error { - system, err := validateAndGetSystemForUnmap(ctx, r, in) - if err != nil { - return err + system, validateErr := validateAndGetSystemForUnmap(ctx, r, in) + if validateErr != nil { + slogctx.Error(ctx, "validateAndGetSystemForUnmap failed", "error", validateErr) + return validateErr } system.TenantID = &emptyTenantID - ok, err := r.Patch(ctx, system) - if err != nil { + ok, patchErr := r.Patch(ctx, system) + if patchErr != nil { + slogctx.Error(ctx, "failed to patch system during unmap", "error", patchErr) return ErrSystemUpdate } @@ -61,6 +63,7 @@ func (m *Mapping) UnmapSystemFromTenant(ctx context.Context, in *mappinggrpc.Unm return ErrorWithParams(ErrSystemNotFound, "externalID", in.GetExternalId(), "type", in.GetType()) } + slogctx.Debug(ctx, "system successfully unmapped in transaction") return nil }) @@ -70,6 +73,7 @@ func (m *Mapping) UnmapSystemFromTenant(ctx context.Context, in *mappinggrpc.Unm return nil, err } + slogctx.Info(ctx, "system successfully unmapped from tenant") return &mappinggrpc.UnmapSystemFromTenantResponse{Success: true}, nil } @@ -89,22 +93,28 @@ func (m *Mapping) MapSystemToTenant(ctx context.Context, in *mappinggrpc.MapSyst defer cancel() err := m.repo.Transaction(ctxTimeout, func(ctx context.Context, r repository.Repository) error { - system, found, err := isSystemTenantMapAllowed(ctx, r, in) - if err != nil { - return err + system, found, validateErr := isSystemTenantMapAllowed(ctx, r, in) + if validateErr != nil { + slogctx.Error(ctx, "isSystemTenantMapAllowed failed", "error", validateErr) + return validateErr } if !found { - _, err = createSystem(ctx, m.validation, r, in.GetExternalId(), in.GetType(), tenantID) - return err + _, createErr := createSystem(ctx, m.validation, r, in.GetExternalId(), in.GetType(), tenantID) + if createErr != nil { + slogctx.Error(ctx, "failed to create system during map", "error", createErr) + } + return createErr } system.TenantID = &tenantID - _, err = r.Patch(ctx, system) - if err != nil { + _, patchErr := r.Patch(ctx, system) + if patchErr != nil { + slogctx.Error(ctx, "failed to patch system during map", "error", patchErr) return ErrSystemUpdate } + slogctx.Debug(ctx, "system successfully mapped in transaction") return nil }) @@ -114,6 +124,7 @@ func (m *Mapping) MapSystemToTenant(ctx context.Context, in *mappinggrpc.MapSyst return nil, err } + slogctx.Info(ctx, "system successfully mapped to tenant") return &mappinggrpc.MapSystemToTenantResponse{Success: true}, nil } @@ -148,40 +159,45 @@ func (m *Mapping) Get(ctx context.Context, in *mappinggrpc.GetRequest) (*mapping }, nil } -// validateAndGetSystemForUnmap fetched and returns the system it also validates -// iIt checks if the tenantID matches and if the tenant is active and it checks for the regional systems validity. +// validateAndGetSystemForUnmap fetches and returns the system. It also validates +// that the tenantID matches, that the tenant is active, and checks the regional systems validity. func validateAndGetSystemForUnmap(ctx context.Context, r repository.Repository, in *mappinggrpc.UnmapSystemFromTenantRequest) (*model.System, error) { tenantID := in.GetTenantId() system, found, err := getSystem(ctx, r, in.GetExternalId(), in.GetType()) if err != nil { + slogctx.Error(ctx, "failed to get system for unmap", "error", err) return nil, ErrSystemSelect } if !found { + slogctx.Warn(ctx, "system not found for unmap", "externalId", in.GetExternalId(), "type", in.GetType()) return nil, ErrSystemNotFound } if !system.IsLinkedToTenant() { + slogctx.Warn(ctx, "system is not linked to any tenant", "externalId", system.ExternalID, "type", system.Type) return nil, ErrorWithParams(ErrSystemIsNotLinkedToTenant, "externalID", system.ExternalID, "type", system.Type) } if *system.TenantID != tenantID { + slogctx.Warn(ctx, "system is linked to a different tenant", "externalId", system.ExternalID, "type", system.Type, "linkedTenantId", *system.TenantID, "requestedTenantId", tenantID) return nil, ErrorWithParams(ErrSystemIsNotLinkedToTenant, "externalID", system.ExternalID, "type", system.Type) } - tenant, err := getTenant(ctx, r, *system.TenantID) - if err != nil { - return nil, err + tenant, getTenantErr := getTenant(ctx, r, *system.TenantID) + if getTenantErr != nil { + slogctx.Error(ctx, "failed to get tenant for unmap", "tenantId", *system.TenantID, "error", getTenantErr) + return nil, getTenantErr } - err = checkTenantActive(tenant) - if err != nil { - return nil, err + if activeErr := checkTenantActive(tenant); activeErr != nil { + slogctx.Warn(ctx, "tenant is not active for unmap", "tenantId", tenant.ID, "status", tenant.Status, "error", activeErr) + return nil, activeErr } - if err := validateRegionalSystemsForUnmap(ctx, r, system); err != nil { - slogctx.Warn(ctx, "validation failed for UnmapSystemFromTenant request", "error", err) - return nil, err + if regionalErr := validateRegionalSystemsForUnmap(ctx, r, system); regionalErr != nil { + slogctx.Warn(ctx, "regional systems validation failed for unmap", "externalId", system.ExternalID, "type", system.Type, "error", regionalErr) + return nil, regionalErr } return system, nil @@ -207,34 +223,39 @@ func validateRegionalSystemsForUnmap(ctx context.Context, r repository.Repositor } // isSystemTenantMapAllowed checks whether all conditions are met to map the Tenant. -// It returns nil if the provided Tenant exist, the System is found and no linked, and HasL1KeyClaim is false. +// It returns nil if the provided Tenant exist, the System is found and not linked, and HasL1KeyClaim is false. func isSystemTenantMapAllowed(ctx context.Context, r repository.Repository, in *mappinggrpc.MapSystemToTenantRequest) (*model.System, bool, error) { - tenant, err := getTenant(ctx, r, in.GetTenantId()) - if err != nil { - return nil, false, err + tenant, getTenantErr := getTenant(ctx, r, in.GetTenantId()) + if getTenantErr != nil { + slogctx.Error(ctx, "failed to get tenant for map", "tenantId", in.GetTenantId(), "error", getTenantErr) + return nil, false, getTenantErr } - err = checkTenantActive(tenant) - if err != nil { - return nil, false, err + if activeErr := checkTenantActive(tenant); activeErr != nil { + slogctx.Warn(ctx, "tenant is not active for map", "tenantId", tenant.ID, "status", tenant.Status, "error", activeErr) + return nil, false, activeErr } - system, found, err := getSystem(ctx, r, in.GetExternalId(), in.GetType()) - if err != nil { - return nil, false, err + system, found, getSystemErr := getSystem(ctx, r, in.GetExternalId(), in.GetType()) + if getSystemErr != nil { + slogctx.Error(ctx, "failed to get system for map", "externalId", in.GetExternalId(), "type", in.GetType(), "error", getSystemErr) + return nil, false, getSystemErr } if !found { + slogctx.Debug(ctx, "system not found - will create new", "externalId", in.GetExternalId(), "type", in.GetType()) return nil, false, nil } // For linking, each system must not be already linked and must not have an active L1 key claim. if system.IsLinkedToTenant() { + slogctx.Warn(ctx, "system is already linked to a tenant", "externalId", system.ExternalID, "type", system.Type, "linkedTenantId", *system.TenantID) return system, found, ErrorWithParams(ErrSystemIsLinkedToTenant, "externalID", system.ExternalID, "type", system.Type) } - if err := validateRegionalSystemsForLink(ctx, r, system); err != nil { - return system, found, err + if regionalErr := validateRegionalSystemsForLink(ctx, r, system); regionalErr != nil { + slogctx.Warn(ctx, "regional systems validation failed for map", "externalId", system.ExternalID, "type", system.Type, "error", regionalErr) + return system, found, regionalErr } return system, found, nil From 784f19e1727cbe435aa9543bf556f21472f2132f Mon Sep 17 00:00:00 2001 From: Oleg <51953040+OlegGitH@users.noreply.github.com> Date: Thu, 14 May 2026 15:16:46 +0200 Subject: [PATCH 2/9] small fix for comment validation of system Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- internal/service/mapping.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/service/mapping.go b/internal/service/mapping.go index 5ff3451..6112bff 100644 --- a/internal/service/mapping.go +++ b/internal/service/mapping.go @@ -160,7 +160,7 @@ func (m *Mapping) Get(ctx context.Context, in *mappinggrpc.GetRequest) (*mapping } // validateAndGetSystemForUnmap fetches and returns the system. It also validates -// that the tenantID matches, that the tenant is active, and checks the regional systems validity. +// that the tenantID matches, that the tenant is active, and validates the regional systems. func validateAndGetSystemForUnmap(ctx context.Context, r repository.Repository, in *mappinggrpc.UnmapSystemFromTenantRequest) (*model.System, error) { tenantID := in.GetTenantId() From 50a6386af49f64d6b107ac350eda664238a7bf3b Mon Sep 17 00:00:00 2001 From: Oleg Grishanovich Date: Thu, 14 May 2026 15:22:43 +0200 Subject: [PATCH 3/9] add more tests for MapSystemToTenant and UnmapSystemFromTenant, small comment fixes --- integration/mapping_test.go | 65 +++++++++++++++++++++++++++++++++++++ internal/service/mapping.go | 5 ++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/integration/mapping_test.go b/integration/mapping_test.go index 2430dc3..b86360a 100644 --- a/integration/mapping_test.go +++ b/integration/mapping_test.go @@ -126,6 +126,40 @@ func TestMappingService(t *testing.T) { assert.Nil(t, res) assert.Equal(t, status.Code(err), status.Code(service.ErrSystemHasL1KeyClaim)) }) + t.Run("system is not linked to any tenant", func(t *testing.T) { + systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, "", false, allowedSystemType, nil, nil) + defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, "", systemType, region, false) + + res, err := mSubj.UnmapSystemFromTenant(ctx, &mappinggrpc.UnmapSystemFromTenantRequest{ + ExternalId: systemID, + Type: systemType, + TenantId: existingTenantID, + }) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, status.Code(err), status.Code(service.ErrSystemIsNotLinkedToTenant)) + }) + t.Run("tenant is not active", func(t *testing.T) { + blockedTenant := validTenant() + blockedTenant.Status = model.TenantStatus(tenantgrpc.Status_STATUS_BLOCKED.String()) + err := createTenantInDB(ctx, db, blockedTenant) + assert.NoError(t, err) + defer func() { + assert.NoError(t, deleteTenantFromDB(ctx, db, blockedTenant)) + }() + + systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, blockedTenant.ID, false, allowedSystemType, nil, nil) + defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, blockedTenant.ID, systemType, region, false) + + res, err := mSubj.UnmapSystemFromTenant(ctx, &mappinggrpc.UnmapSystemFromTenantRequest{ + ExternalId: systemID, + Type: systemType, + TenantId: blockedTenant.ID, + }) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, status.Code(err), status.Code(service.ErrTenantUnavailable)) + }) }) t.Run("should unmap system from tenant successfully", func(t *testing.T) { systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, existingTenantID, false, allowedSystemType, nil, nil) @@ -222,6 +256,37 @@ func TestMappingService(t *testing.T) { assert.Nil(t, res) assert.Equal(t, status.Code(err), status.Code(service.ErrSystemIsLinkedToTenant)) }) + t.Run("tenant is not active", func(t *testing.T) { + blockedTenant := validTenant() + blockedTenant.Status = model.TenantStatus(tenantgrpc.Status_STATUS_BLOCKED.String()) + err := createTenantInDB(ctx, db, blockedTenant) + assert.NoError(t, err) + defer func() { + assert.NoError(t, deleteTenantFromDB(ctx, db, blockedTenant)) + }() + + res, err := mSubj.MapSystemToTenant(ctx, &mappinggrpc.MapSystemToTenantRequest{ + ExternalId: validRandID(), + Type: allowedSystemType, + TenantId: blockedTenant.ID, + }) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, status.Code(err), status.Code(service.ErrTenantUnavailable)) + }) + t.Run("regional system has active L1 key claim", func(t *testing.T) { + systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, "", true, allowedSystemType, nil, nil) + defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, "", systemType, region, true) + + res, err := mSubj.MapSystemToTenant(ctx, &mappinggrpc.MapSystemToTenantRequest{ + ExternalId: systemID, + Type: systemType, + TenantId: existingTenantID, + }) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, status.Code(err), status.Code(service.ErrSystemHasL1KeyClaim)) + }) }) t.Run("should map system to tenant successfully", func(t *testing.T) { t.Run("when L1 key claims is false", func(t *testing.T) { diff --git a/internal/service/mapping.go b/internal/service/mapping.go index 5ff3451..1353da3 100644 --- a/internal/service/mapping.go +++ b/internal/service/mapping.go @@ -223,7 +223,10 @@ func validateRegionalSystemsForUnmap(ctx context.Context, r repository.Repositor } // isSystemTenantMapAllowed checks whether all conditions are met to map the Tenant. -// It returns nil if the provided Tenant exist, the System is found and not linked, and HasL1KeyClaim is false. +// It returns (nil, false, nil) when the Tenant is valid and the System does not exist yet, +// so the caller can create it. It returns (system, true, nil) when the Tenant is valid, +// the System is found, is not linked, and has no active L1 key claim. The bool indicates +// whether the System was found. func isSystemTenantMapAllowed(ctx context.Context, r repository.Repository, in *mappinggrpc.MapSystemToTenantRequest) (*model.System, bool, error) { tenant, getTenantErr := getTenant(ctx, r, in.GetTenantId()) if getTenantErr != nil { From 311d2921c751be59b5a1ad2639cf5c69bf2b352d Mon Sep 17 00:00:00 2001 From: Oleg Grishanovich Date: Thu, 14 May 2026 15:48:19 +0200 Subject: [PATCH 4/9] fix: add missing tenantgrpc import in mapping_test.go --- integration/mapping_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/integration/mapping_test.go b/integration/mapping_test.go index b86360a..106c5b7 100644 --- a/integration/mapping_test.go +++ b/integration/mapping_test.go @@ -12,7 +12,9 @@ import ( mappinggrpc "github.com/openkcm/api-sdk/proto/kms/api/cmk/registry/mapping/v1" systemgrpc "github.com/openkcm/api-sdk/proto/kms/api/cmk/registry/system/v1" + tenantgrpc "github.com/openkcm/api-sdk/proto/kms/api/cmk/registry/tenant/v1" + "github.com/openkcm/registry/internal/model" "github.com/openkcm/registry/internal/service" ) From f5b90e8e9b0e9470003935868311099214f50923 Mon Sep 17 00:00:00 2001 From: Oleg Grishanovich Date: Thu, 14 May 2026 15:48:19 +0200 Subject: [PATCH 5/9] fix: add missing tenantgrpc import in mapping_test.go --- integration/mapping_test.go | 65 ------------------------------------- 1 file changed, 65 deletions(-) diff --git a/integration/mapping_test.go b/integration/mapping_test.go index b86360a..2430dc3 100644 --- a/integration/mapping_test.go +++ b/integration/mapping_test.go @@ -126,40 +126,6 @@ func TestMappingService(t *testing.T) { assert.Nil(t, res) assert.Equal(t, status.Code(err), status.Code(service.ErrSystemHasL1KeyClaim)) }) - t.Run("system is not linked to any tenant", func(t *testing.T) { - systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, "", false, allowedSystemType, nil, nil) - defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, "", systemType, region, false) - - res, err := mSubj.UnmapSystemFromTenant(ctx, &mappinggrpc.UnmapSystemFromTenantRequest{ - ExternalId: systemID, - Type: systemType, - TenantId: existingTenantID, - }) - assert.Error(t, err) - assert.Nil(t, res) - assert.Equal(t, status.Code(err), status.Code(service.ErrSystemIsNotLinkedToTenant)) - }) - t.Run("tenant is not active", func(t *testing.T) { - blockedTenant := validTenant() - blockedTenant.Status = model.TenantStatus(tenantgrpc.Status_STATUS_BLOCKED.String()) - err := createTenantInDB(ctx, db, blockedTenant) - assert.NoError(t, err) - defer func() { - assert.NoError(t, deleteTenantFromDB(ctx, db, blockedTenant)) - }() - - systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, blockedTenant.ID, false, allowedSystemType, nil, nil) - defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, blockedTenant.ID, systemType, region, false) - - res, err := mSubj.UnmapSystemFromTenant(ctx, &mappinggrpc.UnmapSystemFromTenantRequest{ - ExternalId: systemID, - Type: systemType, - TenantId: blockedTenant.ID, - }) - assert.Error(t, err) - assert.Nil(t, res) - assert.Equal(t, status.Code(err), status.Code(service.ErrTenantUnavailable)) - }) }) t.Run("should unmap system from tenant successfully", func(t *testing.T) { systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, existingTenantID, false, allowedSystemType, nil, nil) @@ -256,37 +222,6 @@ func TestMappingService(t *testing.T) { assert.Nil(t, res) assert.Equal(t, status.Code(err), status.Code(service.ErrSystemIsLinkedToTenant)) }) - t.Run("tenant is not active", func(t *testing.T) { - blockedTenant := validTenant() - blockedTenant.Status = model.TenantStatus(tenantgrpc.Status_STATUS_BLOCKED.String()) - err := createTenantInDB(ctx, db, blockedTenant) - assert.NoError(t, err) - defer func() { - assert.NoError(t, deleteTenantFromDB(ctx, db, blockedTenant)) - }() - - res, err := mSubj.MapSystemToTenant(ctx, &mappinggrpc.MapSystemToTenantRequest{ - ExternalId: validRandID(), - Type: allowedSystemType, - TenantId: blockedTenant.ID, - }) - assert.Error(t, err) - assert.Nil(t, res) - assert.Equal(t, status.Code(err), status.Code(service.ErrTenantUnavailable)) - }) - t.Run("regional system has active L1 key claim", func(t *testing.T) { - systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, "", true, allowedSystemType, nil, nil) - defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, "", systemType, region, true) - - res, err := mSubj.MapSystemToTenant(ctx, &mappinggrpc.MapSystemToTenantRequest{ - ExternalId: systemID, - Type: systemType, - TenantId: existingTenantID, - }) - assert.Error(t, err) - assert.Nil(t, res) - assert.Equal(t, status.Code(err), status.Code(service.ErrSystemHasL1KeyClaim)) - }) }) t.Run("should map system to tenant successfully", func(t *testing.T) { t.Run("when L1 key claims is false", func(t *testing.T) { From bdf26b102abbf3082c446307d70a4a057c2a6fa7 Mon Sep 17 00:00:00 2001 From: Oleg Grishanovich Date: Mon, 18 May 2026 16:08:12 +0200 Subject: [PATCH 6/9] fix: remove unused tenantgrpc import in mapping_test.go --- integration/mapping_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/integration/mapping_test.go b/integration/mapping_test.go index 2d0a306..2430dc3 100644 --- a/integration/mapping_test.go +++ b/integration/mapping_test.go @@ -12,9 +12,7 @@ import ( mappinggrpc "github.com/openkcm/api-sdk/proto/kms/api/cmk/registry/mapping/v1" systemgrpc "github.com/openkcm/api-sdk/proto/kms/api/cmk/registry/system/v1" - tenantgrpc "github.com/openkcm/api-sdk/proto/kms/api/cmk/registry/tenant/v1" - "github.com/openkcm/registry/internal/model" "github.com/openkcm/registry/internal/service" ) From 9e488fcad904b83cd4e2fc7399c09fc5608288b3 Mon Sep 17 00:00:00 2001 From: Oleg Grishanovich Date: Mon, 18 May 2026 17:52:18 +0200 Subject: [PATCH 7/9] test: add cases for inactive tenant handling in mapping functions --- integration/mapping_test.go | 80 +++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/integration/mapping_test.go b/integration/mapping_test.go index 2430dc3..266e4dd 100644 --- a/integration/mapping_test.go +++ b/integration/mapping_test.go @@ -12,7 +12,10 @@ import ( mappinggrpc "github.com/openkcm/api-sdk/proto/kms/api/cmk/registry/mapping/v1" systemgrpc "github.com/openkcm/api-sdk/proto/kms/api/cmk/registry/system/v1" + tenantgrpc "github.com/openkcm/api-sdk/proto/kms/api/cmk/registry/tenant/v1" + "github.com/openkcm/registry/integration/operatortest" + "github.com/openkcm/registry/internal/model" "github.com/openkcm/registry/internal/service" ) @@ -114,6 +117,34 @@ func TestMappingService(t *testing.T) { assert.Nil(t, res) assert.Equal(t, status.Code(err), status.Code(service.ErrSystemIsNotLinkedToTenant)) }) + t.Run("tenant is not active", func(t *testing.T) { + inactiveTenant := &model.Tenant{ + Name: "InactiveTenantUnmap", + ID: validRandID(), + Region: operatortest.Region, + OwnerID: "owner123", + OwnerType: allowedOwnerType, + Status: model.TenantStatus(tenantgrpc.Status_STATUS_BLOCKED.String()), + Role: tenantgrpc.Role_ROLE_LIVE.String(), + } + err := createTenantInDB(ctx, db, inactiveTenant) + assert.NoError(t, err) + defer func() { + assert.NoError(t, deleteTenantFromDB(ctx, db, inactiveTenant)) + }() + + systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, inactiveTenant.ID, false, allowedSystemType, nil, nil) + defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, inactiveTenant.ID, systemType, region, false) + + res, err := mSubj.UnmapSystemFromTenant(ctx, &mappinggrpc.UnmapSystemFromTenantRequest{ + ExternalId: systemID, + Type: systemType, + TenantId: inactiveTenant.ID, + }) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, status.Code(err), status.Code(service.ErrTenantUnavailable)) + }) t.Run("regional system has active L1 key claim", func(t *testing.T) { systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, existingTenantID, true, allowedSystemType, nil, nil) defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, existingTenantID, systemType, region, true) @@ -138,6 +169,16 @@ func TestMappingService(t *testing.T) { }) assert.NoError(t, err) assert.NotNil(t, res) + assert.True(t, res.Success) + + // Verify system is no longer linked via Get + getRes, err := mSubj.Get(ctx, &mappinggrpc.GetRequest{ + ExternalId: systemID, + Type: systemType, + }) + assert.NoError(t, err) + assert.NotNil(t, getRes) + assert.Empty(t, getRes.TenantId) }) }) @@ -203,6 +244,45 @@ func TestMappingService(t *testing.T) { assert.Nil(t, res) assert.ErrorIs(t, err, service.ErrTenantNotFound) }) + t.Run("tenant is not active", func(t *testing.T) { + inactiveTenant := &model.Tenant{ + Name: "InactiveTenantMap", + ID: validRandID(), + Region: operatortest.Region, + OwnerID: "owner123", + OwnerType: allowedOwnerType, + Status: model.TenantStatus(tenantgrpc.Status_STATUS_BLOCKED.String()), + Role: tenantgrpc.Role_ROLE_LIVE.String(), + } + err := createTenantInDB(ctx, db, inactiveTenant) + assert.NoError(t, err) + defer func() { + assert.NoError(t, deleteTenantFromDB(ctx, db, inactiveTenant)) + }() + + res, err := mSubj.MapSystemToTenant(ctx, &mappinggrpc.MapSystemToTenantRequest{ + ExternalId: validRandID(), + Type: allowedSystemType, + TenantId: inactiveTenant.ID, + }) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, status.Code(err), status.Code(service.ErrTenantUnavailable)) + }) + t.Run("system has active L1 key claim during map", func(t *testing.T) { + // Register system without tenant, then unmap, then set L1 key claim and try to map + systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, "", true, allowedSystemType, nil, nil) + defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, "", systemType, region, true) + + res, err := mSubj.MapSystemToTenant(ctx, &mappinggrpc.MapSystemToTenantRequest{ + ExternalId: systemID, + Type: systemType, + TenantId: existingTenantID, + }) + assert.Error(t, err) + assert.Nil(t, res) + assert.Equal(t, status.Code(err), status.Code(service.ErrSystemHasL1KeyClaim)) + }) t.Run("system is already mapped to another tenant", func(t *testing.T) { tenant := validTenant() err = createTenantInDB(ctx, db, tenant) From da6d7582f16af8af02b1246f9278b4298a6af2d7 Mon Sep 17 00:00:00 2001 From: Oleg Grishanovich Date: Mon, 18 May 2026 23:40:04 +0200 Subject: [PATCH 8/9] test: add cases for inactive tenant handling in mapping functions --- integration/mapping_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/integration/mapping_test.go b/integration/mapping_test.go index 266e4dd..1ccb95a 100644 --- a/integration/mapping_test.go +++ b/integration/mapping_test.go @@ -129,13 +129,16 @@ func TestMappingService(t *testing.T) { } err := createTenantInDB(ctx, db, inactiveTenant) assert.NoError(t, err) + + systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, inactiveTenant.ID, false, allowedSystemType, nil, nil) defer func() { + // Change tenant to active so cleanup can proceed + inactiveTenant.Status = model.TenantStatus(tenantgrpc.Status_STATUS_ACTIVE.String()) + db.WithContext(ctx).Save(inactiveTenant) + cleanupSystem(t, ctx, sSubj, mSubj, systemID, inactiveTenant.ID, systemType, region, false) assert.NoError(t, deleteTenantFromDB(ctx, db, inactiveTenant)) }() - systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, inactiveTenant.ID, false, allowedSystemType, nil, nil) - defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, inactiveTenant.ID, systemType, region, false) - res, err := mSubj.UnmapSystemFromTenant(ctx, &mappinggrpc.UnmapSystemFromTenantRequest{ ExternalId: systemID, Type: systemType, From 0c3df13e3274e68d2665471ddcb5cc4059fa5289 Mon Sep 17 00:00:00 2001 From: Oleg Grishanovich Date: Tue, 19 May 2026 09:51:29 +0200 Subject: [PATCH 9/9] fix: improve error handling in isSystemTenantMapAllowed and update test cleanup logic --- integration/mapping_test.go | 10 ++++++++-- internal/service/mapping.go | 7 ++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/integration/mapping_test.go b/integration/mapping_test.go index 1ccb95a..7db0bb0 100644 --- a/integration/mapping_test.go +++ b/integration/mapping_test.go @@ -273,9 +273,15 @@ func TestMappingService(t *testing.T) { assert.Equal(t, status.Code(err), status.Code(service.ErrTenantUnavailable)) }) t.Run("system has active L1 key claim during map", func(t *testing.T) { - // Register system without tenant, then unmap, then set L1 key claim and try to map systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, "", true, allowedSystemType, nil, nil) - defer cleanupSystem(t, ctx, sSubj, mSubj, systemID, "", systemType, region, true) + defer func() { + // System is not linked to any tenant, so UpdateSystemL1KeyClaim would fail. + // Delete the regional system via gRPC, then remove the parent system from DB. + err := deleteSystem(ctx, sSubj, systemID, systemType, region) + assert.NoError(t, err) + err = deleteSystemInDB(ctx, db, systemID, systemType) + assert.NoError(t, err) + }() res, err := mSubj.MapSystemToTenant(ctx, &mappinggrpc.MapSystemToTenantRequest{ ExternalId: systemID, diff --git a/internal/service/mapping.go b/internal/service/mapping.go index b7580c7..52697e4 100644 --- a/internal/service/mapping.go +++ b/internal/service/mapping.go @@ -224,9 +224,10 @@ func validateRegionalSystemsForUnmap(ctx context.Context, r repository.Repositor // isSystemTenantMapAllowed checks whether all conditions are met to map the Tenant. // It returns (nil, false, nil) when the Tenant is valid and the System does not exist yet, -// so the caller can create it. It returns (system, true, nil) when the Tenant is valid, -// the System is found, is not linked, and has no active L1 key claim. The bool indicates -// whether the System was found. +// so the caller can create it. It also returns (nil, false, error) when the Tenant is not +// found or not active. It returns (system, true, nil) when the Tenant is valid, the System +// is found, is not linked, and has no active L1 key claim. The bool indicates whether the +// System was found. func isSystemTenantMapAllowed(ctx context.Context, r repository.Repository, in *mappinggrpc.MapSystemToTenantRequest) (*model.System, bool, error) { tenant, getTenantErr := getTenant(ctx, r, in.GetTenantId()) if getTenantErr != nil {