fix: allow import of same service subject from different accounts

nats-server already allows importing the same (service) subject from different account, while the jwt does not. This becomes an issue when you, for instance, source JetStreams from different accounts and need to import service `$JS.FC.>` from both. Before this could be solved by importing e.g. `$JS.FC.STREAM_FROM_ACC_X.>` and `$JS.FC.STREAM_FROM_ACC_Y.>`, but this requires globally unique stream names. If those streams are both called `ORDERS` then you wouldn't be able to source both.

Signed-off-by: Thobias Karlsson <thobias.karlsson@gmail.com>

Author: thobiaskarlsson

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
+	subjectsByAccount := 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 subjectsByAccount[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 subjectsByAccount[v.Account] == nil {
+				subjectsByAccount[v.Account] = make(map[Subject]struct{}, len(*i))
 			}
-			toSet[sub] = struct{}{}
+			if _, ok := subjectsByAccount[v.Account][sub]; ok {
+				vr.AddError("overlapping subject namespace for %q in account %q", sub, v.Account)
+			}
+			subjectsByAccount[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) {
+	accountA := publicKey(createAccountNKey(t), t)
+	accountB := publicKey(createAccountNKey(t), t)
+
+	testCases := []struct {
+		test    string
+		imports []*Import
+		valid   bool
+	}{
+		{
+			test: "Same subject from same account",
+			imports: []*Import{
+				{Account: accountA, Subject: "order.create", Type: Service},
+				{Account: accountA, Subject: "order.create", Type: Service},
+			},
+			valid: false,
+		},
+		{
+			test: "Same subject from same account, to same local subject",
+			imports: []*Import{
+				{Account: accountA, Subject: "order.create", LocalSubject: "external.order.create", Type: Service},
+				{Account: accountA, 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: accountA, Subject: "order.create", To: "external.order.create", Type: Service},
+				{Account: accountA, 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: accountA, Subject: "order.create", To: "external.order.create", Type: Service},
+				{Account: accountA, Subject: "order.create", LocalSubject: "external.order.create", Type: Service},
+			},
+			valid: false,
+		},
+		{
+			test: "Same subject from same account, but different local subject",
+			imports: []*Import{
+				{Account: accountA, Subject: "order.create", LocalSubject: "service-a.order.create", Type: Service},
+				{Account: accountA, 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: accountA, Subject: "service-a.ping", LocalSubject: "deps.ping", Type: Service},
+				{Account: accountA, 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: accountA, Subject: "$JS.FC.>", Type: Service},
+				{Account: accountB, Subject: "$JS.FC.>", Type: Service},
+			},
+			valid: true,
+		},
+		{
+			test: "Same subject from different accounts, to same local subject",
+			imports: []*Import{
+				{Account: accountA, Subject: "order.create", LocalSubject: "external.order.create", Type: Service},
+				{Account: accountB, Subject: "order.create", LocalSubject: "external.order.create", Type: Service},
+			},
+			valid: true,
+		},
+		{
+			test: "Same subject from different accounts, but different local subject",
+			imports: []*Import{
+				{Account: accountA, Subject: "order.create", LocalSubject: "external.a.order.create", Type: Service},
+				{Account: accountB, 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: accountA, Subject: "service-a.ping", LocalSubject: "deps.ping", Type: Service},
+				{Account: accountB, 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)