fix(AIP-123): prevent panic in getParentIDVariable with single-variable patterns (#1565)

Author: eddiesoller

rules/aip0123/aip0123.go

@@ -115,6 +115,13 @@ func getParentIDVariable(pattern string) string {
 		return variables[len(variables)-1]
 	}
 
+	// If there are fewer than 2 variables for a non-singleton resource, there is no parent ID variable.
+	// This can happen with invalid patterns like "prefix/collection/{id}",
+	// which will be caught by the resource-name-components-alternate rule.
+	if len(variables) < 2 {
+		return ""
+	}
+
 	return variables[len(variables)-2]
 }
 

rules/aip0123/aip0123_test.go

@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"github.com/googleapis/api-linter/v2/lint"
+	"github.com/googleapis/api-linter/v2/rules/internal/testutils"
 	apb "google.golang.org/genproto/googleapis/api/annotations"
 )
 
@@ -250,3 +251,57 @@ func TestGetParentIDVariable(t *testing.T) {
 		})
 	}
 }
+
+func TestAIP0123_InvalidPatternsDontPanic(t *testing.T) {
+	registry := lint.NewRuleRegistry()
+	if err := AddRules(registry); err != nil {
+		t.Fatalf("Failed to add AIP-0123 rules: %v", err)
+	}
+	linter := lint.New(registry, nil)
+
+	for _, test := range []struct {
+		name    string
+		Pattern string
+	}{
+		{
+			name:    "prefixed top level",
+			Pattern: "nonCollectionPrefix/projects/{project}",
+		},
+		{
+			name:    "prefixed child collection",
+			Pattern: "nonCollectionPrefix/projects/{project}/locations/{location}",
+		},
+		{
+			name:    "prefixed singleton",
+			Pattern: "nonCollectionPrefix/projects/{project}/config",
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			f := testutils.ParseProto3Tmpl(t, `
+			import "google/api/resource.proto";
+			message TestResource {
+				option (google.api.resource) = {
+					type: "library.googleapis.com/TestResource"
+					pattern: "{{ .Pattern }}"
+					singular: "testResource"
+					plural: "testResources"
+				};
+				string name = 1;
+			}
+			`, test)
+
+			responses, err := linter.LintProtos(f)
+			if err != nil {
+				t.Fatalf("Linter returned error (possible panic): %v", err)
+			}
+
+			problemCount := 0
+			for _, resp := range responses {
+				problemCount += len(resp.Problems)
+			}
+			if problemCount == 0 {
+				t.Errorf("Expected at least one problem for invalid pattern %q, got none", test.Pattern)
+			}
+		})
+	}
+}

rules/aip0123/resource_name_components_alternate_test.go

@@ -30,6 +30,9 @@ func TestResourceNameComponentsAlternate(t *testing.T) {
 		{"ValidSingleton", "user/{user}/config", testutils.Problems{}},
 		{"InvalidDoubleCollection", "author/books/{book}", testutils.Problems{{Message: "must alternate"}}},
 		{"InvalidDoubleIdentifier", "books/{author}/{book}", testutils.Problems{{Message: "must alternate"}}},
+		{"InvalidPrefixedSingleVariable", "nonCollectionPrefix/projects/{project}", testutils.Problems{{Message: "must alternate"}}},
+		{"InvalidPrefixedChildCollection", "nonCollectionPrefix/projects/{project}/locations/{location}", testutils.Problems{{Message: "must alternate"}}},
+		{"InvalidPrefixedSingleton", "nonCollectionPrefix/projects/{project}/config", testutils.Problems{{Message: "must alternate"}}},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			f := testutils.ParseProto3Tmpl(t, `