HYPERFLEET-1345 - fix: Address bugs from PR#288

pkg/api/resource_reference.go

@@ -13,7 +13,7 @@ func (ResourceReference) TableName() string {
 	return "resource_references"
 }
 
-// ResourceSummary carries kind + name for error messages (e.g. FindReferencer 409).
+// ResourceSummary carries kind + name for error messages (e.g. FindReferencers 409).
 type ResourceSummary struct {
 	Kind string
 	Name string

pkg/dao/mocks/resource.go

@@ -127,7 +127,7 @@ func (d *resourceDaoMock) ReplaceReferences(_ context.Context, _ string, _ []api
 	return nil
 }
 
-func (d *resourceDaoMock) FindReferencer(_ context.Context, _ string) (*api.ResourceSummary, error) {
+func (d *resourceDaoMock) FindReferencers(_ context.Context, _ string) ([]api.ResourceSummary, error) {
 	return nil, nil
 }
 

pkg/dao/resource.go

@@ -24,7 +24,7 @@ type ResourceDao interface {
 	FindByKindAndOwnerForUpdate(ctx context.Context, kind, ownerID string) (api.ResourceList, error)
 	GetByID(ctx context.Context, id string) (*api.Resource, error)
 	ReplaceReferences(ctx context.Context, sourceID string, refs []api.ResourceReference) error
-	FindReferencer(ctx context.Context, targetID string) (*api.ResourceSummary, error)
+	FindReferencers(ctx context.Context, targetID string) ([]api.ResourceSummary, error)
 	ClearTargetReferences(ctx context.Context, targetID string) error
 	FindSourceIDsByRef(ctx context.Context, refType, targetID string) ([]string, error)
 }
@@ -196,26 +196,22 @@ func (d *sqlResourceDao) ReplaceReferences(
 	return nil
 }
 
-// FindReferencer returns the first non-deleted resource that references targetID,
+// FindReferencers returns the list of resources that references targetID,
 // or nil if none exists. Used as an existence check for 409 conflict responses.
-func (d *sqlResourceDao) FindReferencer(
+func (d *sqlResourceDao) FindReferencers(
 	ctx context.Context, targetID string,
-) (*api.ResourceSummary, error) {
+) ([]api.ResourceSummary, error) {
 	g2 := d.sessionFactory.New(ctx)
-	var summary api.ResourceSummary
+	var summaries []api.ResourceSummary
 	err := g2.Model(&api.ResourceReference{}).
 		Select("resources.kind, resources.name").
 		Joins("JOIN resources ON resource_references.source_id = resources.id").
 		Where("resource_references.target_id = ? AND resources.deleted_time IS NULL", targetID).
-		Limit(1).
-		Scan(&summary).Error
+		Scan(&summaries).Error
 	if err != nil {
 		return nil, err
 	}
-	if summary.Kind == "" {
-		return nil, nil
-	}
-	return &summary, nil
+	return summaries, nil
 }
 
 // ClearTargetReferences removes all inbound references pointing at targetID.

pkg/services/resource.go

@@ -286,15 +286,19 @@ func (s *sqlResourceService) deleteResourceTree(
 	}
 
 	// Check if other resources reference this one before any deletion.
-	referencer, refErr := s.resourceDao.FindReferencer(ctx, resource.ID)
+	referencers, refErr := s.resourceDao.FindReferencers(ctx, resource.ID)
 	if refErr != nil {
 		return errors.GeneralError("failed to check references: %s", refErr)
 	}
-	if referencer != nil {
+	// List out all the references for a specific resource
+	if len(referencers) > 0 {
+		names := make([]string, len(referencers))
+		for i, r := range referencers {
+			names[i] = fmt.Sprintf("%s %q", r.Kind, r.Name)
+		}
 		return errors.ConflictState(
-			"cannot delete %s %q: referenced by %s %q — remove the reference before deleting",
-			resource.Kind, resource.ID, referencer.Kind, referencer.Name,
-		)
+			"cannot delete %s %q: referenced by %s — remove the reference(s) before deleting",
+			resource.Kind, resource.Name, strings.Join(names, ", "))
 	}
 
 	shouldSoftDelete, svcErr := s.shouldSoftDelete(ctx, resource, children)
@@ -979,6 +983,12 @@ func (s *sqlResourceService) validateReferences(
 				)
 			}
 			seen[*ref.Id] = true
+			if ref.Kind != rd.TargetKind {
+				return errors.Validation(
+					"reference type %q: kind %q does not match expected target kind %q",
+					refType, ref.Kind, rd.TargetKind,
+				)
+			}
 			target, err := s.resourceDao.Get(ctx, rd.TargetKind, *ref.Id)
 			if err != nil {
 				return errors.Validation(

pkg/services/resource_test.go

@@ -51,7 +51,7 @@ type mockResourceDao struct {
 	deleteErr                   error
 	existsSoftDeletedByOwnerErr error
 	replaceRefsErr              error
-	findReferencerResult        *api.ResourceSummary
+	findReferencersResult       []api.ResourceSummary
 	lastReplacedRefs            []api.ResourceReference
 	replaceRefsCalled           bool
 }
@@ -194,8 +194,8 @@ func (d *mockResourceDao) ReplaceReferences(_ context.Context, _ string, refs []
 	return nil
 }
 
-func (d *mockResourceDao) FindReferencer(_ context.Context, _ string) (*api.ResourceSummary, error) {
-	return d.findReferencerResult, nil
+func (d *mockResourceDao) FindReferencers(_ context.Context, _ string) ([]api.ResourceSummary, error) {
+	return d.findReferencersResult, nil
 }
 
 func (d *mockResourceDao) ClearTargetReferences(_ context.Context, _ string) error {
@@ -2196,7 +2196,7 @@ func TestResourceService_Delete_ReferencedResource_Returns409(t *testing.T) {
 
 	target := testResource("Target", "t-1", "target-1")
 	mockDao.addResource(target)
-	mockDao.findReferencerResult = &api.ResourceSummary{Kind: "Parent", Name: "parent-1"}
+	mockDao.findReferencersResult = []api.ResourceSummary{{Kind: "Parent", Name: "parent-1"}}
 
 	result, svcErr := svc.Delete(context.Background(), "Target", "t-1")
 	Expect(result).To(BeNil())
@@ -2215,7 +2215,7 @@ func TestResourceService_Delete_UnreferencedResource_Succeeds(t *testing.T) {
 
 	target := testResource("Target", "t-1", "target-1")
 	mockDao.addResource(target)
-	mockDao.findReferencerResult = nil
+	mockDao.findReferencersResult = nil
 
 	result, svcErr := svc.Delete(context.Background(), "Target", "t-1")
 	Expect(svcErr).To(BeNil())

test/integration/resource_references_test.go

@@ -2,6 +2,7 @@ package integration
 
 import (
 	"fmt"
+	"strings"
 	"sync"
 	"testing"
 
@@ -516,6 +517,37 @@ func TestResourceReferences_CreateDuplicateRefTarget(t *testing.T) {
 	Expect(svcErr.HTTPCode).To(Equal(400))
 	Expect(svcErr.Reason).To(ContainSubstring("duplicate target id"))
 }
+func TestResourceReferences_CreateInvalidKind(t *testing.T) {
+	testCases := []struct {
+		name        string
+		kind        string
+		description string
+	}{
+		{"WrongKind", "WrongKind", "wrong kind should fail validation"},
+		{"EmptyKind", "", "empty kind should fail validation"},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			RegisterTestingT(t)
+			svc, _ := setupRefTest(t)
+
+			target, svcErr := svc.Create(t.Context(), "RefTarget",
+				newRefTestResource("RefTarget",
+					fmt.Sprintf("target-%s-%s", strings.ToLower(tc.name[:2]), uuid.NewString()[:8])),
+				nil)
+			Expect(svcErr).To(BeNil())
+
+			sourceName := fmt.Sprintf("source-%s-%s", strings.ToLower(tc.name[:2]), uuid.NewString()[:8])
+			refs := makeRefs("dep", struct{ id, kind string }{target.ID, tc.kind})
+
+			_, svcErr = svc.Create(t.Context(), "RefSource", newRefTestResource("RefSource", sourceName), refs)
+			Expect(svcErr).NotTo(BeNil(), tc.description)
+			Expect(svcErr.HTTPCode).To(Equal(400))
+			Expect(svcErr.Reason).To(ContainSubstring("does not match expected target kind"))
+		})
+	}
+}
 
 func TestResourceReferences_CreateRefToSoftDeletedTarget(t *testing.T) {
 	RegisterTestingT(t)