fix: allow import of same service subject from different accounts

v2/imports.go

@@ -126,7 +126,8 @@ type Imports []*Import
 
 // Validate checks if an import is valid for the wrapping account
 func (i *Imports) Validate(acctPubKey string, vr *ValidationResults) {
-	toSet := make(map[Subject]struct{}, len(*i))
+	// Group subjects by account to check for overlaps only within the same account
+	subsByAcct := make(map[string]map[Subject]struct{}, len(*i))
 	for _, v := range *i {
 		if v == nil {
 			vr.AddError("null import is not allowed")
@@ -140,15 +141,19 @@ func (i *Imports) Validate(acctPubKey string, vr *ValidationResults) {
 			if sub == "" {
 				sub = v.Subject
 			}
-			for k := range toSet {
-				if sub.IsContainedIn(k) || k.IsContainedIn(sub) {
-					vr.AddError("overlapping subject namespace for %q and %q", sub, k)
+			// Check for overlapping subjects only within the same account
+			for subOther := range subsByAcct[v.Account] {
+				if sub.IsContainedIn(subOther) || subOther.IsContainedIn(sub) {
+					vr.AddError("overlapping subject namespace for %q and %q in same account %q", sub, subOther, v.Account)
 				}
 			}
-			if _, ok := toSet[sub]; ok {
-				vr.AddError("overlapping subject namespace for %q", v.To)
+			if subsByAcct[v.Account] == nil {
+				subsByAcct[v.Account] = make(map[Subject]struct{}, len(*i))
 			}
-			toSet[sub] = struct{}{}
+			if _, ok := subsByAcct[v.Account][sub]; ok {
+				vr.AddError("overlapping subject namespace for %q in account %q", sub, v.Account)
+			}
+			subsByAcct[v.Account][sub] = struct{}{}
 		}
 		v.Validate(acctPubKey, vr)
 	}

v2/imports_test.go

@@ -499,6 +499,118 @@ func TestImports_Validate(t *testing.T) {
 	}
 }
 
+func TestImports_Validate_ServiceConflicts(t *testing.T) {
+	acctA := publicKey(createAccountNKey(t), t)
+	acctB := publicKey(createAccountNKey(t), t)
+
+	testCases := []struct {
+		test    string
+		imports []*Import
+		valid   bool
+	}{
+		{
+			test: "Same subject from same account",
+			imports: []*Import{
+				{Account: acctA, Subject: "order.create", Type: Service},
+				{Account: acctA, Subject: "order.create", Type: Service},
+			},
+			valid: false,
+		},
+		{
+			test: "Same subject from same account, to same local subject",
+			imports: []*Import{
+				{Account: acctA, Subject: "order.create", LocalSubject: "external.order.create", Type: Service},
+				{Account: acctA, Subject: "order.create", LocalSubject: "external.order.create", Type: Service},
+			},
+			valid: false,
+		},
+		{
+			test: "Same subject from same account, to same local subject (using To)",
+			imports: []*Import{
+				{Account: acctA, Subject: "order.create", To: "external.order.create", Type: Service},
+				{Account: acctA, Subject: "order.create", To: "external.order.create", Type: Service},
+			},
+			valid: false,
+		},
+		{
+			test: "Same subject from same account, to same local subject (using To and LocalSubject)",
+			imports: []*Import{
+				{Account: acctA, Subject: "order.create", To: "external.order.create", Type: Service},
+				{Account: acctA, Subject: "order.create", LocalSubject: "external.order.create", Type: Service},
+			},
+			valid: false,
+		},
+		{
+			test: "Same subject from same account, but different local subject",
+			imports: []*Import{
+				{Account: acctA, Subject: "order.create", LocalSubject: "service-a.order.create", Type: Service},
+				{Account: acctA, Subject: "order.create", LocalSubject: "service-b.order.create", Type: Service},
+			},
+			valid: true,
+		},
+		{
+			test: "Different subjects from same account, to same local subject",
+			imports: []*Import{
+				{Account: acctA, Subject: "service-a.ping", LocalSubject: "deps.ping", Type: Service},
+				{Account: acctA, Subject: "service-b.ping", LocalSubject: "deps.ping", Type: Service},
+			},
+			valid: false, // nats-server does not allow this today, hence it should be rejected here as well.
+		},
+		{
+			test: "Same subject from different accounts",
+			imports: []*Import{
+				{Account: acctA, Subject: "$JS.FC.>", Type: Service},
+				{Account: acctB, Subject: "$JS.FC.>", Type: Service},
+			},
+			valid: true,
+		},
+		{
+			test: "Same subject from different accounts, to same local subject",
+			imports: []*Import{
+				{Account: acctA, Subject: "order.create", LocalSubject: "external.order.create", Type: Service},
+				{Account: acctB, Subject: "order.create", LocalSubject: "external.order.create", Type: Service},
+			},
+			valid: true,
+		},
+		{
+			test: "Same subject from different accounts, but different local subject",
+			imports: []*Import{
+				{Account: acctA, Subject: "order.create", LocalSubject: "external.a.order.create", Type: Service},
+				{Account: acctB, Subject: "order.create", LocalSubject: "external.b.order.create", Type: Service},
+			},
+			valid: true,
+		},
+		{
+			test: "Different subjects from different accounts, to same local subject",
+			imports: []*Import{
+				{Account: acctA, Subject: "service-a.ping", LocalSubject: "deps.ping", Type: Service},
+				{Account: acctB, Subject: "service-a.ping", LocalSubject: "deps.ping", Type: Service},
+			},
+			valid: true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.test, func(t *testing.T) {
+			// Given
+			var imports Imports
+			imports.Add(testCase.imports...)
+			vr := ValidationResults{}
+
+			// When
+			imports.Validate("", &vr)
+
+			// Then
+			valid := !vr.IsBlocking(true)
+			if testCase.valid && !valid {
+				t.Fatalf("expected valid, got: %+v", vr.Issues)
+			} else if !testCase.valid && valid {
+				t.Fatalf("expected error, got no blocking validation issues")
+			}
+		})
+	}
+}
+
 func TestImportAllowTrace(t *testing.T) {
 	ak2 := createAccountNKey(t)
 	akp2 := publicKey(ak2, t)