Merge pull request #1463 from hashicorp/b-query-name-alias

Adds a check to make sure query names can't be registered twice.

consul/state/prepared_query.go

@@ -76,6 +76,19 @@ func (s *StateStore) preparedQuerySetTxn(tx *memdb.Txn, idx uint64, query *struc
 		query.ModifyIndex = idx
 	}
 
+	// Verify that the query name doesn't already exist, or that we are
+	// updating the same instance that has this name.
+	if query.Name != "" {
+		alias, err := tx.First("prepared-queries", "name", query.Name)
+		if err != nil {
+			return fmt.Errorf("failed prepared query lookup: %s", err)
+		}
+		if alias != nil && (existing == nil ||
+			existing.(*structs.PreparedQuery).ID != alias.(*structs.PreparedQuery).ID) {
+			return fmt.Errorf("name '%s' aliases an existing query name", query.Name)
+		}
+	}
+
 	// Verify that the name doesn't alias any existing ID. We allow queries
 	// to be looked up by ID *or* name so we don't want anyone to try to
 	// register a query with a name equal to some other query's ID in an
@@ -86,12 +99,12 @@ func (s *StateStore) preparedQuerySetTxn(tx *memdb.Txn, idx uint64, query *struc
 	// index will complain if we look up something that's not formatted
 	// like one.
 	if isUUID(query.Name) {
-		existing, err := tx.First("prepared-queries", "id", query.Name)
+		alias, err := tx.First("prepared-queries", "id", query.Name)
 		if err != nil {
 			return fmt.Errorf("failed prepared query lookup: %s", err)
 		}
-		if existing != nil {
-			return fmt.Errorf("name '%s' aliases an existing query id", query.Name)
+		if alias != nil {
+			return fmt.Errorf("name '%s' aliases an existing query ID", query.Name)
 		}
 	}
 

consul/state/prepared_query_test.go

@@ -175,36 +175,65 @@ func TestStateStore_PreparedQuerySet_PreparedQueryGet(t *testing.T) {
 		t.Fatalf("bad: %v", actual)
 	}
 
-	// Finally, try to abuse the system by trying to register a query whose
-	// name aliases a real query ID.
-	evil := &structs.PreparedQuery{
-		ID:   testUUID(),
-		Name: query.ID,
-		Service: structs.ServiceQuery{
-			Service: "redis",
-		},
+	// Try to register a query with the same name and make sure it fails.
+	{
+		evil := &structs.PreparedQuery{
+			ID:   testUUID(),
+			Name: query.Name,
+			Service: structs.ServiceQuery{
+				Service: "redis",
+			},
+		}
+		err := s.PreparedQuerySet(7, evil)
+		if err == nil || !strings.Contains(err.Error(), "aliases an existing query name") {
+			t.Fatalf("bad: %v", err)
+		}
+
+		// Sanity check to make sure it's not there.
+		idx, actual, err := s.PreparedQueryGet(evil.ID)
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		if idx != 6 {
+			t.Fatalf("bad index: %d", idx)
+		}
+		if actual != nil {
+			t.Fatalf("bad: %v", actual)
+		}
 	}
-	err = s.PreparedQuerySet(7, evil)
-	if err == nil || !strings.Contains(err.Error(), "aliases an existing query") {
-		t.Fatalf("bad: %v", err)
+
+	// Try to abuse the system by trying to register a query whose name
+	// aliases a real query ID.
+	{
+		evil := &structs.PreparedQuery{
+			ID:   testUUID(),
+			Name: query.ID,
+			Service: structs.ServiceQuery{
+				Service: "redis",
+			},
+		}
+		err := s.PreparedQuerySet(8, evil)
+		if err == nil || !strings.Contains(err.Error(), "aliases an existing query ID") {
+			t.Fatalf("bad: %v", err)
+		}
+
+		// Sanity check to make sure it's not there.
+		idx, actual, err := s.PreparedQueryGet(evil.ID)
+		if err != nil {
+			t.Fatalf("err: %s", err)
+		}
+		if idx != 6 {
+			t.Fatalf("bad index: %d", idx)
+		}
+		if actual != nil {
+			t.Fatalf("bad: %v", actual)
+		}
 	}
 
 	// Index is not updated if nothing is saved.
 	if idx := s.maxIndex("prepared-queries"); idx != 6 {
 		t.Fatalf("bad index: %d", idx)
 	}
-
-	// Sanity check to make sure it's not there.
-	idx, actual, err = s.PreparedQueryGet(evil.ID)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if idx != 6 {
-		t.Fatalf("bad index: %d", idx)
-	}
-	if actual != nil {
-		t.Fatalf("bad: %v", actual)
-	}
 }
 
 func TestStateStore_PreparedQueryDelete(t *testing.T) {