diff --git a/integration/mapping_test.go b/integration/mapping_test.go index 2430dc3..7db0bb0 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,37 @@ 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) + + 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)) + }() + + 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 +172,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 +247,51 @@ 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) { + systemID, systemType, region := registerRegionalSystem(t, ctx, sSubj, "", true, allowedSystemType, nil, nil) + 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, + 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) diff --git a/internal/service/mapping.go b/internal/service/mapping.go index cc91a25..52697e4 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 validates the regional systems. 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,43 @@ 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, false, nil) when the Tenant is valid and the System does not exist yet, +// 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, 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