diff --git a/cmd/zoekt-sourcegraph-indexserver/index_test.go b/cmd/zoekt-sourcegraph-indexserver/index_test.go index 58bf0e774..674e08792 100644 --- a/cmd/zoekt-sourcegraph-indexserver/index_test.go +++ b/cmd/zoekt-sourcegraph-indexserver/index_test.go @@ -1,25 +1,21 @@ package main import ( - "bytes" "context" - "encoding/json" + "errors" "fmt" - "net/http" - "net/http/httptest" "net/url" "os" "os/exec" "path/filepath" - "reflect" "sort" - "strconv" "strings" "testing" "time" "github.com/sourcegraph/log/logtest" proto "github.com/sourcegraph/zoekt/cmd/zoekt-sourcegraph-indexserver/protos/sourcegraph/zoekt/configuration/v1" + "github.com/sourcegraph/zoekt/ctags" "google.golang.org/grpc" "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/timestamppb" @@ -31,625 +27,406 @@ import ( ) func TestIterateIndexOptions_Fingerprint(t *testing.T) { - t.Run("gRPC", func(t *testing.T) { - fingerprintV0 := &proto.Fingerprint{ - Identifier: 100, - GeneratedAt: timestamppb.New(time.Unix(100, 0)), - } - - fingerprintV1 := &proto.Fingerprint{ - Identifier: 101, - GeneratedAt: timestamppb.New(time.Unix(101, 0)), - } - - fingerprintV2 := &proto.Fingerprint{ - Identifier: 102, - GeneratedAt: timestamppb.New(time.Unix(102, 0)), - } + fingerprintV0 := &proto.Fingerprint{ + Identifier: 100, + GeneratedAt: timestamppb.New(time.Unix(100, 0)), + } - mkSearchConfigurationResponse := func(fingerprint *proto.Fingerprint, repoIDs ...int32) *proto.SearchConfigurationResponse { - repositories := make([]*proto.ZoektIndexOptions, 0, len(repoIDs)) - for _, repoID := range repoIDs { - repositories = append(repositories, &proto.ZoektIndexOptions{ - RepoId: repoID, - }) - } + fingerprintV1 := &proto.Fingerprint{ + Identifier: 101, + GeneratedAt: timestamppb.New(time.Unix(101, 0)), + } - return &proto.SearchConfigurationResponse{ - UpdatedOptions: repositories, - Fingerprint: fingerprint, - } - } + fingerprintV2 := &proto.Fingerprint{ + Identifier: 102, + GeneratedAt: timestamppb.New(time.Unix(102, 0)), + } - grpcClient := &mockGRPCClient{ - mockList: func(_ context.Context, in *proto.ListRequest, opts ...grpc.CallOption) (*proto.ListResponse, error) { - return &proto.ListResponse{ - RepoIds: []int32{1, 2, 3}, - }, nil - }, + mkSearchConfigurationResponse := func(fingerprint *proto.Fingerprint, repoIDs ...int32) *proto.SearchConfigurationResponse { + repositories := make([]*proto.ZoektIndexOptions, 0, len(repoIDs)) + for _, repoID := range repoIDs { + repositories = append(repositories, &proto.ZoektIndexOptions{ + RepoId: repoID, + }) } - clientOpts := []SourcegraphClientOption{ - WithBatchSize(1), - WithShouldUseGRPC(true), - WithGRPCClient(grpcClient), + return &proto.SearchConfigurationResponse{ + UpdatedOptions: repositories, + Fingerprint: fingerprint, } + } - testURL := url.URL{Scheme: "http", Host: "does.not.matter", Path: "/"} - sg := newSourcegraphClient(&testURL, "", clientOpts...) - - type step struct { - name string + grpcClient := &mockGRPCClient{ + mockList: func(_ context.Context, in *proto.ListRequest, opts ...grpc.CallOption) (*proto.ListResponse, error) { + return &proto.ListResponse{ + RepoIds: []int32{1, 2, 3}, + }, nil + }, + } - wantFingerprint *proto.Fingerprint - returnFingerprint *proto.Fingerprint - returnErr error - skipCheckingRepoIDs bool - } + clientOpts := []SourcegraphClientOption{ + WithBatchSize(1), + } - for _, step := range []step{ - { - name: "first call", - wantFingerprint: nil, - returnFingerprint: fingerprintV0, - }, - { - name: "second call (should provide fingerprint from last time)", - wantFingerprint: fingerprintV0, - returnFingerprint: fingerprintV1, - }, - { - name: "error", - wantFingerprint: fingerprintV1, - returnFingerprint: fingerprintV2, + testURL := url.URL{Scheme: "http", Host: "does.not.matter", Path: "/"} + sg := newSourcegraphClient(&testURL, "", grpcClient, clientOpts...) - returnErr: fmt.Errorf("boom"), - skipCheckingRepoIDs: true, // don't bother checking repoIDs if we expect an error - }, - { - name: "call after error (should ignore fingerprint from last time, and provide the older one)", - wantFingerprint: fingerprintV1, - returnFingerprint: fingerprintV2, - }, - } { - t.Run(step.name, func(t *testing.T) { - called := false - grpcClient.mockSearchConfiguration = func(_ context.Context, in *proto.SearchConfigurationRequest, opts ...grpc.CallOption) (*proto.SearchConfigurationResponse, error) { - called = true - - diff := cmp.Diff(step.wantFingerprint, in.GetFingerprint(), protocmp.Transform()) - if diff != "" { - t.Fatalf("unexpected fingerprint (-want +got):\n%s", diff) - } - - return mkSearchConfigurationResponse(step.returnFingerprint, in.RepoIds...), step.returnErr - } + type step struct { + name string - result, err := sg.List(context.Background(), nil) - if err != nil { - t.Fatalf("unexpected error from List: %v", err) - } + wantFingerprint *proto.Fingerprint + returnFingerprint *proto.Fingerprint + returnErr error + skipCheckingRepoIDs bool + } - var iteratedIDs []uint32 - result.IterateIndexOptions(func(options IndexOptions) { - iteratedIDs = append(iteratedIDs, options.RepoID) - }) + for _, step := range []step{ + { + name: "first call", + wantFingerprint: nil, + returnFingerprint: fingerprintV0, + }, + { + name: "second call (should provide fingerprint from last time)", + wantFingerprint: fingerprintV0, + returnFingerprint: fingerprintV1, + }, + { + name: "error", + wantFingerprint: fingerprintV1, + returnFingerprint: fingerprintV2, - if !called { - t.Fatal("expected SearchConfiguration to be called") - } + returnErr: fmt.Errorf("boom"), + skipCheckingRepoIDs: true, // don't bother checking repoIDs if we expect an error + }, + { + name: "call after error (should ignore fingerprint from last time, and provide the older one)", + wantFingerprint: fingerprintV1, + returnFingerprint: fingerprintV2, + }, + } { + t.Run(step.name, func(t *testing.T) { + called := false + grpcClient.mockSearchConfiguration = func(_ context.Context, in *proto.SearchConfigurationRequest, opts ...grpc.CallOption) (*proto.SearchConfigurationResponse, error) { + called = true - if step.skipCheckingRepoIDs { - return + diff := cmp.Diff(step.wantFingerprint, in.GetFingerprint(), protocmp.Transform()) + if diff != "" { + t.Fatalf("unexpected fingerprint (-want +got):\n%s", diff) } - sort.Slice(iteratedIDs, func(i, j int) bool { - return iteratedIDs[i] < iteratedIDs[j] - }) + return mkSearchConfigurationResponse(step.returnFingerprint, in.RepoIds...), step.returnErr + } - expectedIDs := []uint32{1, 2, 3} - sort.Slice(expectedIDs, func(i, j int) bool { - return expectedIDs[i] < expectedIDs[j] - }) + result, err := sg.List(context.Background(), nil) + if err != nil { + t.Fatalf("unexpected error from List: %v", err) + } - if diff := cmp.Diff(expectedIDs, iteratedIDs); diff != "" { - t.Fatalf("unexpected repo ids (-want +got):\n%s", diff) - } + var iteratedIDs []uint32 + result.IterateIndexOptions(func(options IndexOptions) { + iteratedIDs = append(iteratedIDs, options.RepoID) }) - } - }) - t.Run("REST", func(t *testing.T) { - fingerprintV0 := "v0" - fingerprintV1 := "v1" - fingerprintV2 := "v2" - - handleList := func(w http.ResponseWriter, _ *http.Request) { - data := struct { - RepoIDs []uint32 - }{ - RepoIDs: []uint32{1, 2, 3}, + if !called { + t.Fatal("expected SearchConfiguration to be called") } - json.NewEncoder(w).Encode(data) - } - - searchConfigurationHandler := func(w http.ResponseWriter, r *http.Request) { - http.Error(w, "this search configuration handler hasn't been overridden", http.StatusForbidden) - } - - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/.internal/search/configuration": - searchConfigurationHandler(w, r) - case "/.internal/repos/index": - handleList(w, r) - default: - t.Fatalf("unexpected path: %s", r.URL.Path) + if step.skipCheckingRepoIDs { + return } - })) - defer server.Close() - - clientOpts := []SourcegraphClientOption{ - WithBatchSize(1), - WithShouldUseGRPC(false), - } - - testURL, err := url.Parse(server.URL) - if err != nil { - t.Fatalf("unexpected error parsing URL: %v", err) - } - - sg := newSourcegraphClient(testURL, "", clientOpts...) - - type step struct { - name string - - wantFingerprint string - returnFingerprint string - returnErr error - skipCheckingRepoIDs bool - } - for _, step := range []step{ - { - name: "first call", - wantFingerprint: "", - returnFingerprint: fingerprintV0, - }, - { - name: "second call (should provide fingerprint from last time)", - wantFingerprint: fingerprintV0, - returnFingerprint: fingerprintV1, - }, - { - name: "error", - wantFingerprint: fingerprintV1, - returnFingerprint: fingerprintV2, - - returnErr: fmt.Errorf("boom"), - skipCheckingRepoIDs: true, // don't bother checking repoIDs if we expect an error - }, - { - name: "call after error (should ignore fingerprint from last time, and provide the older one)", - wantFingerprint: fingerprintV1, - returnFingerprint: fingerprintV2, - }, - } { - t.Run(step.name, func(t *testing.T) { - called := false - - searchConfigurationHandler = func(w http.ResponseWriter, r *http.Request) { - called = true - - fingerprint := r.Header.Get(fingerprintHeader) - if diff := cmp.Diff(step.wantFingerprint, fingerprint); diff != "" { - t.Fatalf("unexpected fingerprint (-want +got):\n%s", diff) - } - - w.Header().Set(fingerprintHeader, step.returnFingerprint) - - if step.returnErr != nil { - // The status code is a bit of a hack, but it prevents - // the retry logic from kicking in and stalling the test for 45 seconds. - http.Error(w, step.returnErr.Error(), http.StatusBadRequest) - return - } - - if err := r.ParseForm(); err != nil { - http.Error(w, fmt.Sprintf("unexpected error parsing form for repoIDs: %v", err.Error()), http.StatusBadRequest) - return - } - - repoIDs := make([]uint32, 0, len(r.Form["repoID"])) - for _, idStr := range r.Form["repoID"] { - id, err := strconv.Atoi(idStr) - if err != nil { - http.Error(w, fmt.Sprintf("invalid repo id %s: %s", idStr, err), http.StatusBadRequest) - return - } - repoIDs = append(repoIDs, uint32(id)) - } - - optionJSONSlice := make([][]byte, 0, len(repoIDs)) - for _, repoID := range repoIDs { - option := IndexOptions{ - RepoID: repoID, - } - - optionJSON, err := json.Marshal(option) - if err != nil { - t.Fatalf("unexpected error marshalling JSON: %v", err) - } - - optionJSONSlice = append(optionJSONSlice, optionJSON) - } - - w.Write(bytes.Join(optionJSONSlice, []byte("\n"))) - } - - result, err := sg.List(context.Background(), nil) - if err != nil { - t.Fatalf("unexpected error from List: %v", err) - } - - var iteratedIDs []uint32 - result.IterateIndexOptions(func(options IndexOptions) { - iteratedIDs = append(iteratedIDs, options.RepoID) - }) - - if !called { - t.Fatal("expected SearchConfiguration to be called") - } - - if step.skipCheckingRepoIDs { - return - } - - sort.Slice(iteratedIDs, func(i, j int) bool { - return iteratedIDs[i] < iteratedIDs[j] - }) - - expectedIDs := []uint32{1, 2, 3} - sort.Slice(expectedIDs, func(i, j int) bool { - return expectedIDs[i] < expectedIDs[j] - }) + sort.Slice(iteratedIDs, func(i, j int) bool { + return iteratedIDs[i] < iteratedIDs[j] + }) - if diff := cmp.Diff(expectedIDs, iteratedIDs); diff != "" { - t.Fatalf("unexpected repo ids (-want +got):\n%s", diff) - } + expectedIDs := []uint32{1, 2, 3} + sort.Slice(expectedIDs, func(i, j int) bool { + return expectedIDs[i] < expectedIDs[j] }) - } - }) + + if diff := cmp.Diff(expectedIDs, iteratedIDs); diff != "" { + t.Fatalf("unexpected repo ids (-want +got):\n%s", diff) + } + }) + } } func TestGetIndexOptions(t *testing.T) { - t.Run("gRPC", func(t *testing.T) { - type testCase struct { - name string - response *proto.SearchConfigurationResponse - want *IndexOptions - wantErr string - } - for _, tc := range []testCase{ - { - name: "symbols, large files", - response: &proto.SearchConfigurationResponse{ - UpdatedOptions: []*proto.ZoektIndexOptions{ - { - Symbols: true, - LargeFiles: []string{"foo", "bar"}, - }, + type testCase struct { + name string + response *proto.SearchConfigurationResponse + want *IndexOptions + wantErr string + } + + for _, tc := range []testCase{ + { + name: "symbols, large files", + response: &proto.SearchConfigurationResponse{ + UpdatedOptions: []*proto.ZoektIndexOptions{ + { + Symbols: true, + LargeFiles: []string{"foo", "bar"}, }, }, - want: &IndexOptions{ - Symbols: true, - LargeFiles: []string{"foo", "bar"}, - }, }, - { - name: "no symbols , large files", - response: &proto.SearchConfigurationResponse{ - UpdatedOptions: []*proto.ZoektIndexOptions{ - { - Symbols: true, - LargeFiles: []string{"foo", "bar"}, - }, + want: &IndexOptions{ + Symbols: true, + LargeFiles: []string{"foo", "bar"}, + }, + }, + { + name: "no symbols , large files", + response: &proto.SearchConfigurationResponse{ + UpdatedOptions: []*proto.ZoektIndexOptions{ + { + Symbols: true, + LargeFiles: []string{"foo", "bar"}, }, }, - want: &IndexOptions{ - Symbols: true, - LargeFiles: []string{"foo", "bar"}, - }, }, - - { - name: "empty", - response: nil, - want: nil, + want: &IndexOptions{ + Symbols: true, + LargeFiles: []string{"foo", "bar"}, }, + }, + + { + name: "empty", + response: nil, + want: nil, + }, - { - name: "symbols", - response: &proto.SearchConfigurationResponse{ - UpdatedOptions: []*proto.ZoektIndexOptions{ - { - Symbols: true, - }, + { + name: "symbols", + response: &proto.SearchConfigurationResponse{ + UpdatedOptions: []*proto.ZoektIndexOptions{ + { + Symbols: true, }, }, - want: &IndexOptions{ - Symbols: true, - }, }, - { - name: "repoID", - response: &proto.SearchConfigurationResponse{ - UpdatedOptions: []*proto.ZoektIndexOptions{ - { - RepoId: 123, - }, + want: &IndexOptions{ + Symbols: true, + }, + }, + { + name: "repoID", + response: &proto.SearchConfigurationResponse{ + UpdatedOptions: []*proto.ZoektIndexOptions{ + { + RepoId: 123, }, }, - want: &IndexOptions{ - RepoID: 123, - }, }, - { - name: "error", - response: &proto.SearchConfigurationResponse{ - UpdatedOptions: []*proto.ZoektIndexOptions{ - { - Error: "boom", - }, + want: &IndexOptions{ + RepoID: 123, + }, + }, + { + name: "error", + response: &proto.SearchConfigurationResponse{ + UpdatedOptions: []*proto.ZoektIndexOptions{ + { + Error: "boom", }, }, - want: nil, - wantErr: "boom", }, - } { - called := false - mockClient := &mockGRPCClient{ - mockSearchConfiguration: func(_ context.Context, _ *proto.SearchConfigurationRequest, _ ...grpc.CallOption) (*proto.SearchConfigurationResponse, error) { - called = true - return tc.response, nil - }, - } - - testURL := &url.URL{ - Scheme: "http", - Host: "does.not.matter", - Path: "/", - } - - sg := newSourcegraphClient( - testURL, - "", - WithShouldUseGRPC(true), - WithGRPCClient(mockClient), - ) - - var got IndexOptions - var err error - sg.ForceIterateIndexOptions(func(o IndexOptions) { - got = o - }, func(_ uint32, e error) { - err = e - }, 123) - - if !called { - t.Fatal("expected mock to be called") - } - - if err != nil { - if tc.wantErr == "" || !strings.Contains(err.Error(), tc.wantErr) { - t.Fatalf("unexpected error: %v", err) - } - } - - if tc.want == nil { - continue - } - - tc.want.CloneURL = sg.getCloneURL(got.Name) - - if diff := cmp.Diff(tc.want, &got, cmpopts.EquateEmpty()); diff != "" { - t.Errorf("mismatch (-want +got):\n%s", diff) - } + want: nil, + wantErr: "boom", + }, + } { + called := false + mockClient := &mockGRPCClient{ + mockSearchConfiguration: func(_ context.Context, _ *proto.SearchConfigurationRequest, _ ...grpc.CallOption) (*proto.SearchConfigurationResponse, error) { + called = true + return tc.response, nil + }, } - // Mimic our fingerprint API, which doesn't return anything if the - // repo hasn't changed. - t.Run("unchanged", func(t *testing.T) { - called := false - mockClient := &mockGRPCClient{ - mockSearchConfiguration: func(_ context.Context, _ *proto.SearchConfigurationRequest, _ ...grpc.CallOption) (*proto.SearchConfigurationResponse, error) { - called = true - return nil, nil - }, - } - - testURL := &url.URL{ - Scheme: "http", - Host: "does.not.matter", - Path: "/", - } + testURL := &url.URL{ + Scheme: "http", + Host: "does.not.matter", + Path: "/", + } - sg := newSourcegraphClient( - testURL, - "", - WithShouldUseGRPC(true), - WithGRPCClient(mockClient)) + sg := newSourcegraphClient( + testURL, + "", + mockClient, + ) - gotAtLeastOneOption := false - var err error - sg.ForceIterateIndexOptions(func(_ IndexOptions) { - gotAtLeastOneOption = true - }, func(_ uint32, e error) { - err = e - }, 123) + var got IndexOptions + var err error + sg.ForceIterateIndexOptions(func(o IndexOptions) { + got = o + }, func(_ uint32, e error) { + err = e + }, 123) - if !called { - t.Fatal("expected mock to be called") - } + if !called { + t.Fatal("expected mock to be called") + } - if err != nil { + if err != nil { + if tc.wantErr == "" || !strings.Contains(err.Error(), tc.wantErr) { t.Fatalf("unexpected error: %v", err) } - - if gotAtLeastOneOption { - t.Fatalf("expected no options, got %v", gotAtLeastOneOption) - } - }) - }) - t.Run("REST", func(t *testing.T) { - var response []byte - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if err := r.ParseForm(); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - if got, want := r.URL.String(), "/.internal/search/configuration"; got != want { - http.Error(w, fmt.Sprintf("got URL %v want %v", got, want), http.StatusBadRequest) - return - } - if got, want := r.Form, (url.Values{"repoID": []string{"123"}}); !reflect.DeepEqual(got, want) { - http.Error(w, fmt.Sprintf("got URL %v want %v", got, want), http.StatusBadRequest) - return - } - _, _ = w.Write(response) - })) - defer server.Close() - - u, err := url.Parse(server.URL) - if err != nil { - t.Fatal(err) } - sg := newSourcegraphClient(u, "", WithBatchSize(0)) + if tc.want == nil { + continue + } - cases := map[string]*IndexOptions{ - `{"Symbols": true, "LargeFiles": ["foo","bar"]}`: { - Symbols: true, - LargeFiles: []string{"foo", "bar"}, - }, + tc.want.CloneURL = sg.getCloneURL(got.Name) - `{"Symbols": false, "LargeFiles": ["foo","bar"]}`: { - LargeFiles: []string{"foo", "bar"}, - }, - - `{}`: {}, + if diff := cmp.Diff(tc.want, &got, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + } - `{"Symbols": true}`: { - Symbols: true, - }, + // Mimic our fingerprint API, which doesn't return anything if the + // repo hasn't changed. + t.Run("unchanged", func(t *testing.T) { - `{"RepoID": 123}`: { - RepoID: 123, + called := false + mockClient := &mockGRPCClient{ + mockSearchConfiguration: func(_ context.Context, _ *proto.SearchConfigurationRequest, _ ...grpc.CallOption) (*proto.SearchConfigurationResponse, error) { + called = true + return nil, nil }, - - `{"Error": "boom"}`: nil, } - for r, want := range cases { - response = []byte(r) - - var got IndexOptions - var err error - sg.ForceIterateIndexOptions(func(o IndexOptions) { - got = o - }, func(_ uint32, e error) { - err = e - }, 123) - - if err != nil && want != nil { - t.Fatalf("unexpected error: %v", err) - } - if want == nil { - continue - } - - want.CloneURL = sg.getCloneURL(got.Name) - - if d := cmp.Diff(*want, got); d != "" { - t.Log("response", r) - t.Errorf("mismatch (-want +got):\n%s", d) - } + testURL := &url.URL{ + Scheme: "http", + Host: "does.not.matter", + Path: "/", } - // Special case our fingerprint API which doesn't return anything if the - // repo hasn't changed. - t.Run("unchanged", func(t *testing.T) { - response = []byte(``) - - got := false - var err error - sg.ForceIterateIndexOptions(func(_ IndexOptions) { - got = true - }, func(_ uint32, e error) { - err = e - }, 123) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - - if got { - t.Fatalf("expected no options, got %v", got) - } - }) - }) + sg := newSourcegraphClient( + testURL, + "", + mockClient, + ) + gotAtLeastOneOption := false + var err error + sg.ForceIterateIndexOptions(func(_ IndexOptions) { + gotAtLeastOneOption = true + }, func(_ uint32, e error) { + err = e + }, 123) - var response []byte - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if err := r.ParseForm(); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return + if !called { + t.Fatal("expected mock to be called") } - if got, want := r.URL.String(), "/.internal/search/configuration"; got != want { - http.Error(w, fmt.Sprintf("got URL %v want %v", got, want), http.StatusBadRequest) - return + + if err != nil { + t.Fatalf("unexpected error: %v", err) } - if got, want := r.Form, (url.Values{"repoID": []string{"123"}}); !reflect.DeepEqual(got, want) { - http.Error(w, fmt.Sprintf("got URL %v want %v", got, want), http.StatusBadRequest) - return + + if gotAtLeastOneOption { + t.Fatalf("expected no options, got %v", gotAtLeastOneOption) } - _, _ = w.Write(response) - })) - defer server.Close() + }) - u, err := url.Parse(server.URL) - if err != nil { - t.Fatal(err) + var response *proto.SearchConfigurationResponse + mockClient := &mockGRPCClient{ + mockSearchConfiguration: func(_ context.Context, req *proto.SearchConfigurationRequest, _ ...grpc.CallOption) (*proto.SearchConfigurationResponse, error) { + if len(req.GetRepoIds()) == 0 || req.GetRepoIds()[0] != 123 { + return nil, errors.New("invalid repo id") + } + return response, nil + }, } - sg := newSourcegraphClient(u, "", WithBatchSize(0)) + sg := newSourcegraphClient(&url.URL{Path: "/"}, "", mockClient, WithBatchSize(0)) - cases := map[string]*IndexOptions{ - `{"Symbols": true, "LargeFiles": ["foo","bar"]}`: { - Symbols: true, - LargeFiles: []string{"foo", "bar"}, + cases := []struct { + Response *proto.SearchConfigurationResponse + *IndexOptions + }{ + { + Response: &proto.SearchConfigurationResponse{ + UpdatedOptions: []*proto.ZoektIndexOptions{ + { + Symbols: true, + LargeFiles: []string{"foo", "bar"}, + }, + }, + }, + IndexOptions: &IndexOptions{ + Symbols: true, + LargeFiles: []string{"foo", "bar"}, + Branches: []zoekt.RepositoryBranch{}, + LanguageMap: map[string]ctags.CTagsParserType{}, + }, }, - `{"Symbols": false, "LargeFiles": ["foo","bar"]}`: { - LargeFiles: []string{"foo", "bar"}, + { + Response: &proto.SearchConfigurationResponse{ + UpdatedOptions: []*proto.ZoektIndexOptions{ + { + Symbols: false, + LargeFiles: []string{"foo", "bar"}, + }, + }, + }, + IndexOptions: &IndexOptions{ + LargeFiles: []string{"foo", "bar"}, + Branches: []zoekt.RepositoryBranch{}, + LanguageMap: map[string]ctags.CTagsParserType{}, + }, }, - `{}`: {}, + { + Response: &proto.SearchConfigurationResponse{}, + }, - `{"Symbols": true}`: { - Symbols: true, + { + Response: &proto.SearchConfigurationResponse{ + UpdatedOptions: []*proto.ZoektIndexOptions{ + { + Symbols: true, + }, + }, + }, + IndexOptions: &IndexOptions{ + Symbols: true, + Branches: []zoekt.RepositoryBranch{}, + LanguageMap: map[string]ctags.CTagsParserType{}, + }, }, - `{"RepoID": 123}`: { - RepoID: 123, + { + Response: &proto.SearchConfigurationResponse{ + UpdatedOptions: []*proto.ZoektIndexOptions{ + { + RepoId: 123, + }, + }, + }, + IndexOptions: &IndexOptions{ + RepoID: 123, + Branches: []zoekt.RepositoryBranch{}, + LanguageMap: map[string]ctags.CTagsParserType{}, + }, }, - `{"Error": "boom"}`: nil, + { + Response: &proto.SearchConfigurationResponse{ + UpdatedOptions: []*proto.ZoektIndexOptions{ + { + Error: "boom", + }, + }, + }, + }, } - for r, want := range cases { - response = []byte(r) + for _, tc := range cases { + response = tc.Response var got IndexOptions var err error @@ -659,17 +436,16 @@ func TestGetIndexOptions(t *testing.T) { err = e }, 123) - if err != nil && want != nil { + if err != nil && tc.IndexOptions != nil { t.Fatalf("unexpected error: %v", err) } - if want == nil { + if tc.IndexOptions == nil { continue } - want.CloneURL = sg.getCloneURL(got.Name) + tc.IndexOptions.CloneURL = sg.getCloneURL(got.Name) - if d := cmp.Diff(*want, got); d != "" { - t.Log("response", r) + if d := cmp.Diff(*tc.IndexOptions, got); d != "" { t.Errorf("mismatch (-want +got):\n%s", d) } } @@ -677,7 +453,7 @@ func TestGetIndexOptions(t *testing.T) { // Special case our fingerprint API which doesn't return anything if the // repo hasn't changed. t.Run("unchanged", func(t *testing.T) { - response = []byte(``) + response = &proto.SearchConfigurationResponse{} got := false var err error diff --git a/cmd/zoekt-sourcegraph-indexserver/main.go b/cmd/zoekt-sourcegraph-indexserver/main.go index 44ee746d0..d26ae5ef4 100644 --- a/cmd/zoekt-sourcegraph-indexserver/main.go +++ b/cmd/zoekt-sourcegraph-indexserver/main.go @@ -108,11 +108,6 @@ var ( Help: "Number of indexed repos by code host", }) - metricNumAssigned = promauto.NewGauge(prometheus.GaugeOpts{ - Name: "index_num_assigned", - Help: "Number of repos assigned to this indexer by code host", - }) - metricFailingTotal = promauto.NewCounter(prometheus.CounterOpts{ Name: "index_failing_total", Help: "Counts failures to index (indexing activity, should be used with rate())", @@ -1139,19 +1134,6 @@ func getEnvWithDefaultDuration(k string, defaultVal time.Duration) time.Duration return d } -func getEnvWithDefaultBool(k string, defaultVal bool) bool { - v := os.Getenv(k) - if v == "" { - return defaultVal - } - - b, err := strconv.ParseBool(v) - if err != nil { - log.Fatalf("error parsing ENV %s to bool: %s", k, err) - } - return b -} - func getEnvWithDefaultEmptySet(k string) map[string]struct{} { set := map[string]struct{}{} for _, v := range strings.Split(os.Getenv(k), ",") { @@ -1224,9 +1206,6 @@ type rootConfig struct { // config values related to backoff indexing repos with one or more consecutive failures backoffDuration time.Duration maxBackoffDuration time.Duration - - // useGRPC is true if we should use the gRPC API to talk to Sourcegraph. - useGRPC bool } func (rc *rootConfig) registerRootFlags(fs *flag.FlagSet) { @@ -1240,7 +1219,6 @@ func (rc *rootConfig) registerRootFlags(fs *flag.FlagSet) { fs.IntVar(&rc.blockProfileRate, "block_profile_rate", getEnvWithDefaultInt("BLOCK_PROFILE_RATE", -1), "Sampling rate of Go's block profiler in nanoseconds. Values <=0 disable the blocking profiler Var(default). A value of 1 includes every blocking event. See https://pkg.go.dev/runtime#SetBlockProfileRate") fs.DurationVar(&rc.backoffDuration, "backoff_duration", getEnvWithDefaultDuration("BACKOFF_DURATION", 10*time.Minute), "for the given duration we backoff from enqueue operations for a repository that's failed its previous indexing attempt. Consecutive failures increase the duration of the delay linearly up to the maxBackoffDuration. A negative value disables indexing backoff.") fs.DurationVar(&rc.maxBackoffDuration, "max_backoff_duration", getEnvWithDefaultDuration("MAX_BACKOFF_DURATION", 120*time.Minute), "the maximum duration to backoff from enqueueing a repo for indexing. A negative value disables indexing backoff.") - fs.BoolVar(&rc.useGRPC, "use_grpc", mustGetBoolFromEnvironmentVariables([]string{"GRPC_ENABLED", "SG_FEATURE_FLAG_GRPC"}, true), "use the gRPC API to talk to Sourcegraph") // flags related to shard merging fs.DurationVar(&rc.vacuumInterval, "vacuum_interval", getEnvWithDefaultDuration("SRC_VACUUM_INTERVAL", 24*time.Hour), "run vacuum this often") @@ -1406,13 +1384,12 @@ func newServer(conf rootConfig) (*Server, error) { if v := os.Getenv("SRC_REPO_CONFIG_BATCH_SIZE"); v != "" { batchSize, err = strconv.Atoi(v) if err != nil { - return nil, fmt.Errorf("Invalid value for SRC_REPO_CONFIG_BATCH_SIZE, must be int") + return nil, fmt.Errorf("invalid value for SRC_REPO_CONFIG_BATCH_SIZE, must be int") } } opts := []SourcegraphClientOption{ WithBatchSize(batchSize), - WithShouldUseGRPC(conf.useGRPC), } logger := sglog.Scoped("zoektConfigurationGRPCClient") @@ -1421,8 +1398,7 @@ func newServer(conf rootConfig) (*Server, error) { return nil, fmt.Errorf("initializing gRPC connection to %q: %w", rootURL.Host, err) } - opts = append(opts, WithGRPCClient(client)) - sg = newSourcegraphClient(rootURL, conf.hostname, opts...) + sg = newSourcegraphClient(rootURL, conf.hostname, client, opts...) } else { sg = sourcegraphFake{ @@ -1633,17 +1609,6 @@ func main() { } } -// mustGetBoolFromEnvironmentVariables is like getBoolFromEnvironmentVariables, but it panics -// if any of the provided environment variables fails to parse as a boolean. -func mustGetBoolFromEnvironmentVariables(envVarNames []string, defaultBool bool) bool { - value, err := getBoolFromEnvironmentVariables(envVarNames, defaultBool) - if err != nil { - panic(err) - } - - return value -} - // getBoolFromEnvironmentVariables returns the boolean defined by the first environment // variable listed in envVarNames that is set in the current process environment, or the defaultBool if none are set. // diff --git a/cmd/zoekt-sourcegraph-indexserver/main_test.go b/cmd/zoekt-sourcegraph-indexserver/main_test.go index 9a7d80815..806a69280 100644 --- a/cmd/zoekt-sourcegraph-indexserver/main_test.go +++ b/cmd/zoekt-sourcegraph-indexserver/main_test.go @@ -6,8 +6,6 @@ import ( "fmt" "io" "log" - "net/http" - "net/http/httptest" "net/url" "os" "path/filepath" @@ -33,7 +31,7 @@ func TestServer_defaultArgs(t *testing.T) { } s := &Server{ - Sourcegraph: newSourcegraphClient(root, "", WithBatchSize(0)), + Sourcegraph: newSourcegraphClient(root, "", nil, WithBatchSize(0)), IndexDir: "/testdata/index", CPUCount: 6, IndexConcurrency: 1, @@ -101,7 +99,7 @@ func TestServer_parallelism(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { s := &Server{ - Sourcegraph: newSourcegraphClient(root, "", WithBatchSize(0)), + Sourcegraph: newSourcegraphClient(root, "", nil, WithBatchSize(0)), IndexDir: "/testdata/index", CPUCount: tt.cpuCount, IndexConcurrency: tt.indexConcurrency, @@ -117,7 +115,7 @@ func TestServer_parallelism(t *testing.T) { t.Run("index option is limited by available CPU", func(t *testing.T) { s := &Server{ - Sourcegraph: newSourcegraphClient(root, "", WithBatchSize(0)), + Sourcegraph: newSourcegraphClient(root, "", nil, WithBatchSize(0)), IndexDir: "/testdata/index", IndexConcurrency: 1, } @@ -133,136 +131,64 @@ func TestServer_parallelism(t *testing.T) { } func TestListRepoIDs(t *testing.T) { - t.Run("gRPC", func(t *testing.T) { - grpcClient := &mockGRPCClient{} + grpcClient := &mockGRPCClient{} - clientOptions := []SourcegraphClientOption{ - WithShouldUseGRPC(true), - WithGRPCClient(grpcClient), - WithBatchSize(0), - } - - testURL := url.URL{Scheme: "http", Host: "does.not.matter"} - testHostname := "test-hostname" - s := newSourcegraphClient(&testURL, testHostname, clientOptions...) - - listCalled := false - grpcClient.mockList = func(ctx context.Context, in *proto.ListRequest, opts ...grpc.CallOption) (*proto.ListResponse, error) { - listCalled = true - - gotRepoIDs := in.GetIndexedIds() - sort.Slice(gotRepoIDs, func(i, j int) bool { - return gotRepoIDs[i] < gotRepoIDs[j] - }) - - wantRepoIDs := []int32{1, 3} - sort.Slice(wantRepoIDs, func(i, j int) bool { - return wantRepoIDs[i] < wantRepoIDs[j] - }) - - if diff := cmp.Diff(wantRepoIDs, gotRepoIDs); diff != "" { - t.Errorf("indexed repoIDs mismatch (-want +got):\n%s", diff) - } - - hostname := in.GetHostname() - if diff := cmp.Diff(testHostname, hostname); diff != "" { - t.Errorf("hostname mismatch (-want +got):\n%s", diff) - } - - return &proto.ListResponse{RepoIds: []int32{1, 2, 3}}, nil - } + clientOptions := []SourcegraphClientOption{ + WithBatchSize(0), + } - ctx := context.Background() - got, err := s.List(ctx, []uint32{1, 3}) - if err != nil { - t.Fatal(err) - } + testURL := url.URL{Scheme: "http", Host: "does.not.matter"} + testHostname := "test-hostname" + s := newSourcegraphClient(&testURL, testHostname, grpcClient, clientOptions...) - if !listCalled { - t.Fatalf("List was not called") - } + listCalled := false + grpcClient.mockList = func(ctx context.Context, in *proto.ListRequest, opts ...grpc.CallOption) (*proto.ListResponse, error) { + listCalled = true - receivedRepoIDs := got.IDs - sort.Slice(receivedRepoIDs, func(i, j int) bool { - return receivedRepoIDs[i] < receivedRepoIDs[j] + gotRepoIDs := in.GetIndexedIds() + sort.Slice(gotRepoIDs, func(i, j int) bool { + return gotRepoIDs[i] < gotRepoIDs[j] }) - expectedRepoIDs := []uint32{1, 2, 3} - sort.Slice(expectedRepoIDs, func(i, j int) bool { - return expectedRepoIDs[i] < expectedRepoIDs[j] + wantRepoIDs := []int32{1, 3} + sort.Slice(wantRepoIDs, func(i, j int) bool { + return wantRepoIDs[i] < wantRepoIDs[j] }) - if diff := cmp.Diff(expectedRepoIDs, receivedRepoIDs); diff != "" { - t.Errorf("mismatch in list of all repoIDs (-want +got):\n%s", diff) + if diff := cmp.Diff(wantRepoIDs, gotRepoIDs); diff != "" { + t.Errorf("indexed repoIDs mismatch (-want +got):\n%s", diff) } - }) - - t.Run("REST", func(t *testing.T) { - var gotBody string - var gotURL *url.URL - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - gotURL = r.URL - - b, err := io.ReadAll(r.Body) - if err != nil { - t.Fatal(err) - } - gotBody = string(b) - _, err = w.Write([]byte(`{"RepoIDs": [1, 2, 3]}`)) - if err != nil { - t.Fatal(err) - } - })) - defer ts.Close() - - u, err := url.Parse(ts.URL) - if err != nil { - t.Fatal(err) - } - - s := newSourcegraphClient(u, "test-indexed-search-1", WithBatchSize(0)) - - gotRepos, err := s.List(context.Background(), []uint32{1, 3}) - if err != nil { - t.Fatal(err) - } - - if want := []uint32{1, 2, 3}; !cmp.Equal(gotRepos.IDs, want) { - t.Errorf("repos mismatch (-want +got):\n%s", cmp.Diff(want, gotRepos.IDs)) - } - if want := `{"Hostname":"test-indexed-search-1","IndexedIDs":[1,3]}`; gotBody != want { - t.Errorf("body mismatch (-want +got):\n%s", cmp.Diff(want, gotBody)) + hostname := in.GetHostname() + if diff := cmp.Diff(testHostname, hostname); diff != "" { + t.Errorf("hostname mismatch (-want +got):\n%s", diff) } - if want := "/.internal/repos/index"; gotURL.Path != want { - t.Errorf("request path mismatch (-want +got):\n%s", cmp.Diff(want, gotURL.Path)) - } - }) -} -func TestListRepoIDs_Error_REST(t *testing.T) { - // Note: There is no gRPC equivalent to this test because gRPC errors are - // always returned as an error to the caller. - - msg := "deadbeaf deadbeaf" - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // This is how Sourcegraph returns error messages to the caller. - http.Error(w, msg, http.StatusInternalServerError) - })) - defer ts.Close() + return &proto.ListResponse{RepoIds: []int32{1, 2, 3}}, nil + } - u, err := url.Parse(ts.URL) + ctx := context.Background() + got, err := s.List(ctx, []uint32{1, 3}) if err != nil { t.Fatal(err) } - s := newSourcegraphClient(u, "test-indexed-search-1", WithBatchSize(0)) - s.restClient.RetryMax = 0 + if !listCalled { + t.Fatalf("List was not called") + } + + receivedRepoIDs := got.IDs + sort.Slice(receivedRepoIDs, func(i, j int) bool { + return receivedRepoIDs[i] < receivedRepoIDs[j] + }) - _, err = s.List(context.Background(), []uint32{1, 3}) + expectedRepoIDs := []uint32{1, 2, 3} + sort.Slice(expectedRepoIDs, func(i, j int) bool { + return expectedRepoIDs[i] < expectedRepoIDs[j] + }) - if !strings.Contains(err.Error(), msg) { - t.Fatalf("%s does not contain %s", err.Error(), msg) + if diff := cmp.Diff(expectedRepoIDs, receivedRepoIDs); diff != "" { + t.Errorf("mismatch in list of all repoIDs (-want +got):\n%s", diff) } } diff --git a/cmd/zoekt-sourcegraph-indexserver/merge.go b/cmd/zoekt-sourcegraph-indexserver/merge.go index 7c3ff0fba..40c87fcff 100644 --- a/cmd/zoekt-sourcegraph-indexserver/merge.go +++ b/cmd/zoekt-sourcegraph-indexserver/merge.go @@ -16,8 +16,6 @@ import ( "github.com/sourcegraph/zoekt" ) -var reCompound = regexp.MustCompile(`compound-.*\.zoekt`) - var metricShardMergingRunning = promauto.NewGauge(prometheus.GaugeOpts{ Name: "index_shard_merging_running", Help: "Set to 1 if indexserver's merge job is running.", diff --git a/cmd/zoekt-sourcegraph-indexserver/sg.go b/cmd/zoekt-sourcegraph-indexserver/sg.go index 061a454cf..8a163d10f 100644 --- a/cmd/zoekt-sourcegraph-indexserver/sg.go +++ b/cmd/zoekt-sourcegraph-indexserver/sg.go @@ -4,14 +4,11 @@ import ( "bufio" "bytes" "context" - "encoding/json" "errors" "fmt" "hash/crc32" - "io" "log" "math/rand" - "net/http" "net/url" "os" "os/exec" @@ -22,11 +19,9 @@ import ( "time" "github.com/go-git/go-git/v5" - retryablehttp "github.com/hashicorp/go-retryablehttp" proto "github.com/sourcegraph/zoekt/cmd/zoekt-sourcegraph-indexserver/protos/sourcegraph/zoekt/configuration/v1" "github.com/sourcegraph/zoekt/ctags" "golang.org/x/net/trace" - "google.golang.org/grpc" "github.com/sourcegraph/zoekt" ) @@ -93,47 +88,12 @@ func WithBatchSize(batchSize int) SourcegraphClientOption { } } -// WithShouldUseGRPC enables or disables the use of gRPC for communicating with Sourcegraph. -func WithShouldUseGRPC(useGRPC bool) SourcegraphClientOption { - return func(c *sourcegraphClient) { - c.useGRPC = useGRPC - } -} - -// WithGRPCClient sets the gRPC client to use for communicating with Sourcegraph. -func WithGRPCClient(client proto.ZoektConfigurationServiceClient) SourcegraphClientOption { - return func(c *sourcegraphClient) { - c.grpcClient = client - } -} - -func newSourcegraphClient(rootURL *url.URL, hostname string, opts ...SourcegraphClientOption) *sourcegraphClient { - httpClient := retryablehttp.NewClient() - httpClient.Logger = debug - - // Sourcegraph might return an error message in the body if StatusCode==500. The - // default behavior of the go-retryablehttp restClient is to drain the body and not - // to propagate the error. Hence, we call ErrorPropagatedRetryPolicy instead of - // DefaultRetryPolicy and augment the error with the response body if possible. - httpClient.CheckRetry = func(ctx context.Context, resp *http.Response, err error) (bool, error) { - shouldRetry, checkErr := retryablehttp.ErrorPropagatedRetryPolicy(ctx, resp, err) - - if resp != nil && resp.StatusCode == http.StatusInternalServerError { - if b, e := io.ReadAll(resp.Body); e == nil { - checkErr = fmt.Errorf("%w: body=%q", checkErr, string(b)) - } - } - - return shouldRetry, checkErr - } - +func newSourcegraphClient(rootURL *url.URL, hostname string, grpcClient proto.ZoektConfigurationServiceClient, opts ...SourcegraphClientOption) *sourcegraphClient { client := &sourcegraphClient{ Root: rootURL, - restClient: httpClient, Hostname: hostname, BatchSize: 0, - grpcClient: noopGRPCClient{}, - useGRPC: false, // disable gRPC by default + grpcClient: grpcClient, } for _, opt := range opts { @@ -157,21 +117,9 @@ type sourcegraphClient struct { // zero a value of 10000 is used. BatchSize int - // restClient is used to make requests to the Sourcegraph instance. Prefer to - // use .doRequest() to ensure the appropriate headers are set. - restClient *retryablehttp.Client - // grpcClient is used to make requests to the Sourcegraph instance if gRPC is enabled. grpcClient proto.ZoektConfigurationServiceClient - // configFingerprint is the last config fingerprint returned from - // Sourcegraph. It can be used for future calls to the configuration - // endpoint. - // - // configFingerprint is mutually exclusive with configFingerprintProto - this field - // will only be used if gRPC is disabled. - configFingerprint string - // configFingerprintProto is the last config fingerprint (as GRPC) returned from // Sourcegraph. It can be used for future calls to the configuration // endpoint. @@ -185,22 +133,11 @@ type sourcegraphClient struct { // configFingerprint logic is faulty. When it is cleared out, we fallback to // calculating everything. configFingerprintReset time.Time - - // useGRPC indicates whether we should use a gRPC client to communicate with Sourcegraph. - useGRPC bool } // GetDocumentRanks asks Sourcegraph for a mapping of file paths to rank // vectors. func (s *sourcegraphClient) GetDocumentRanks(ctx context.Context, repoName string) (RepoPathRanks, error) { - if s.useGRPC { - return s.getDocumentRanksGRPC(ctx, repoName) - } - - return s.getDocumentRanksREST(ctx, repoName) -} - -func (s *sourcegraphClient) getDocumentRanksGRPC(ctx context.Context, repoName string) (RepoPathRanks, error) { resp, err := s.grpcClient.DocumentRanks(ctx, &proto.DocumentRanksRequest{Repository: repoName}) if err != nil { return RepoPathRanks{}, err @@ -212,58 +149,6 @@ func (s *sourcegraphClient) getDocumentRanksGRPC(ctx context.Context, repoName s return out, nil } -func (s *sourcegraphClient) getDocumentRanksREST(ctx context.Context, repoName string) (RepoPathRanks, error) { - u := s.Root.ResolveReference(&url.URL{ - Path: "/.internal/ranks/" + strings.Trim(repoName, "/") + "/documents", - }) - - b, err := s.get(ctx, u) - if err != nil { - return RepoPathRanks{}, err - } - - ranks := RepoPathRanks{} - err = json.Unmarshal(b, &ranks) - if err != nil { - return RepoPathRanks{}, err - } - - return ranks, nil -} - -func (s *sourcegraphClient) get(ctx context.Context, u *url.URL) ([]byte, error) { - req, err := retryablehttp.NewRequestWithContext(ctx, "GET", u.String(), nil) - if err != nil { - return nil, err - } - - resp, err := s.doRequest(req) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - b, err := io.ReadAll(io.LimitReader(resp.Body, 1024)) - _ = resp.Body.Close() - if err != nil { - return nil, err - } - return nil, &url.Error{ - Op: "Get", - URL: u.String(), - Err: fmt.Errorf("%s: %s", resp.Status, string(b)), - } - } - - b, err := io.ReadAll(resp.Body) - if err != nil { - return nil, err - } - - return b, nil -} - func (s *sourcegraphClient) List(ctx context.Context, indexed []uint32) (*SourcegraphListResult, error) { repos, err := s.listRepoIDs(ctx, indexed) if err != nil { @@ -287,7 +172,6 @@ func (s *sourcegraphClient) List(ctx context.Context, indexed []uint32) (*Source s.configFingerprintReset = time.Now().Add(next) s.configFingerprintProto = nil - s.configFingerprint = "" } // getIndexOptionsFunc is a function that can be used to get the index @@ -299,59 +183,30 @@ func (s *sourcegraphClient) List(ctx context.Context, indexed []uint32) (*Source // fail, the old fingerprint is restored. type getIndexOptionsFunc func(repos ...uint32) ([]indexOptionsItem, error) - // default to REST mkGetIndexOptionsFunc := func(tr trace.Trace) getIndexOptionsFunc { - startingFingerPrint := s.configFingerprint - tr.LazyPrintf("fingerprint: %s", startingFingerPrint) + startingFingerPrint := s.configFingerprintProto + tr.LazyPrintf("fingerprint: %s", startingFingerPrint.String()) first := true return func(repos ...uint32) ([]indexOptionsItem, error) { - options, nextFingerPrint, err := s.getIndexOptionsREST(startingFingerPrint, repos...) + options, nextFingerPrint, err := s.getIndexOptions(ctx, startingFingerPrint, repos) if err != nil { first = false - s.configFingerprint = startingFingerPrint + s.configFingerprintProto = startingFingerPrint return nil, err } if first { first = false - s.configFingerprint = nextFingerPrint - - tr.LazyPrintf("new fingerprint: %s", nextFingerPrint) + s.configFingerprintProto = nextFingerPrint + tr.LazyPrintf("new fingerprint: %s", nextFingerPrint.String()) } return options, nil } } - // If we enabled GRPC, use our gRPC client instead. - if s.useGRPC { - mkGetIndexOptionsFunc = func(tr trace.Trace) getIndexOptionsFunc { - startingFingerPrint := s.configFingerprintProto - tr.LazyPrintf("fingerprint: %s", startingFingerPrint.String()) - - first := true - return func(repos ...uint32) ([]indexOptionsItem, error) { - options, nextFingerPrint, err := s.getIndexOptionsGRPC(ctx, startingFingerPrint, repos) - if err != nil { - first = false - s.configFingerprintProto = startingFingerPrint - - return nil, err - } - - if first { - first = false - s.configFingerprintProto = nextFingerPrint - tr.LazyPrintf("new fingerprint: %s", nextFingerPrint.String()) - } - - return options, nil - } - } - } - iterate := func(f func(IndexOptions)) { start := time.Now() tr := trace.New("getIndexOptions", "") @@ -407,20 +262,8 @@ func (s *sourcegraphClient) ForceIterateIndexOptions(onSuccess func(IndexOptions batchSize = 10_000 } - getIndexOptions := func(repos ...uint32) ([]indexOptionsItem, error) { - opts, _, err := s.getIndexOptionsREST("", repos...) - return opts, err - } - - if s.useGRPC { - getIndexOptions = func(repos ...uint32) ([]indexOptionsItem, error) { - opts, _, err := s.getIndexOptionsGRPC(context.Background(), nil, repos) - return opts, err - } - } - for repos := range batched(repos, batchSize) { - opts, err := getIndexOptions(repos...) + opts, _, err := s.getIndexOptions(context.Background(), nil, repos) if err != nil { for _, id := range repos { onError(id, err) @@ -525,7 +368,7 @@ func (o *indexOptionsItem) ToProto() *proto.ZoektIndexOptions { } } -func (s *sourcegraphClient) getIndexOptionsGRPC(ctx context.Context, fingerprint *proto.Fingerprint, repos []uint32) ([]indexOptionsItem, *proto.Fingerprint, error) { +func (s *sourcegraphClient) getIndexOptions(ctx context.Context, fingerprint *proto.Fingerprint, repos []uint32) ([]indexOptionsItem, *proto.Fingerprint, error) { repoIDs := make([]int32, 0, len(repos)) for _, id := range repos { repoIDs = append(repoIDs, int32(id)) @@ -554,77 +397,11 @@ func (s *sourcegraphClient) getIndexOptionsGRPC(ctx context.Context, fingerprint return items, response.GetFingerprint(), nil } -const fingerprintHeader = "X-Sourcegraph-Config-Fingerprint" - -func (s *sourcegraphClient) getIndexOptionsREST(fingerprint string, repos ...uint32) ([]indexOptionsItem, string, error) { - u := s.Root.ResolveReference(&url.URL{ - Path: "/.internal/search/configuration", - }) - - repoIDs := make([]string, len(repos)) - for i, id := range repos { - repoIDs[i] = strconv.Itoa(int(id)) - } - data := url.Values{"repoID": repoIDs} - req, err := retryablehttp.NewRequest("POST", u.String(), []byte(data.Encode())) - if err != nil { - return nil, "", err - } - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - if fingerprint != "" { - req.Header.Set(fingerprintHeader, fingerprint) - } - - resp, err := s.doRequest(req) - if err != nil { - return nil, "", err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - b, err := io.ReadAll(io.LimitReader(resp.Body, 1024)) - _ = resp.Body.Close() - if err != nil { - return nil, "", err - } - return nil, "", &url.Error{ - Op: "Get", - URL: u.String(), - Err: fmt.Errorf("%s: %s", resp.Status, string(b)), - } - } - - dec := json.NewDecoder(resp.Body) - var opts []indexOptionsItem - for { - var opt indexOptionsItem - err := dec.Decode(&opt) - if err == io.EOF { - break - } - if err != nil { - return nil, "", fmt.Errorf("error decoding body: %w", err) - } - opt.CloneURL = s.getCloneURL(opt.Name) - opts = append(opts, opt) - } - - return opts, resp.Header.Get(fingerprintHeader), nil -} - func (s *sourcegraphClient) getCloneURL(name string) string { return s.Root.ResolveReference(&url.URL{Path: path.Join("/.internal/git", name)}).String() } func (s *sourcegraphClient) listRepoIDs(ctx context.Context, indexed []uint32) ([]uint32, error) { - if s.useGRPC { - return s.listRepoIDsGRPC(ctx, indexed) - } - - return s.listRepoIDsREST(ctx, indexed) -} - -func (s *sourcegraphClient) listRepoIDsGRPC(ctx context.Context, indexed []uint32) ([]uint32, error) { var request proto.ListRequest request.Hostname = s.Hostname request.IndexedIds = make([]int32, 0, len(indexed)) @@ -645,46 +422,6 @@ func (s *sourcegraphClient) listRepoIDsGRPC(ctx context.Context, indexed []uint3 return repoIDs, nil } -func (s *sourcegraphClient) listRepoIDsREST(_ context.Context, indexed []uint32) ([]uint32, error) { - body, err := json.Marshal(&struct { - Hostname string - IndexedIDs []uint32 - }{ - Hostname: s.Hostname, - IndexedIDs: indexed, - }) - if err != nil { - return nil, err - } - - u := s.Root.ResolveReference(&url.URL{Path: "/.internal/repos/index"}) - req, err := retryablehttp.NewRequest(http.MethodPost, u.String(), bytes.NewReader(body)) - if err != nil { - return nil, err - } - req.Header.Set("Content-Type", "application/json; charset=utf-8") - - resp, err := s.doRequest(req) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("failed to list repositories: status %s", resp.Status) - } - - var data struct { - RepoIDs []uint32 - } - err = json.NewDecoder(resp.Body).Decode(&data) - if err != nil { - return nil, err - } - - return data.RepoIDs, nil -} - type indexStatus struct { RepoID uint32 Branches []zoekt.RepositoryBranch @@ -752,14 +489,6 @@ func (u *updateIndexStatusRequest) FromProto(x *proto.UpdateIndexStatusRequest) func (s *sourcegraphClient) UpdateIndexStatus(repositories []indexStatus) error { r := updateIndexStatusRequest{Repositories: repositories} - if s.useGRPC { - return s.updateIndexStatusGRPC(r) - } - - return s.updateIndexStatusREST(r) -} - -func (s *sourcegraphClient) updateIndexStatusGRPC(r updateIndexStatusRequest) error { request := r.ToProto() _, err := s.grpcClient.UpdateIndexStatus(context.Background(), request) if err != nil { @@ -769,43 +498,6 @@ func (s *sourcegraphClient) updateIndexStatusGRPC(r updateIndexStatusRequest) er return nil } -func (s *sourcegraphClient) updateIndexStatusREST(r updateIndexStatusRequest) error { - payload, err := json.Marshal(r) - if err != nil { - return err - } - - u := s.Root.ResolveReference(&url.URL{Path: "/.internal/search/index-status"}) - req, err := retryablehttp.NewRequest(http.MethodPost, u.String(), bytes.NewReader(payload)) - if err != nil { - return err - } - req.Header.Set("Content-Type", "application/json; charset=utf-8") - - resp, err := s.doRequest(req) - if err != nil { - return err - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("failed to update index status: status %s", resp.Status) - } - - return nil -} - -// doRequest executes the provided request after adding the appropriate headers -// for interacting with a Sourcegraph instance. -func (s *sourcegraphClient) doRequest(req *retryablehttp.Request) (*http.Response, error) { - // Make all requests as an internal user. - // - // Should match github.com/sourcegraph/sourcegraph/internal/actor.headerKeyActorUID - // and github.com/sourcegraph/sourcegraph/internal/actor.headerValueInternalActor - req.Header.Set("X-Sourcegraph-Actor-UID", "internal") - return s.restClient.Do(req) -} - type sourcegraphFake struct { RootDir string Log *log.Logger @@ -844,21 +536,6 @@ func (sf sourcegraphFake) GetDocumentRanks(ctx context.Context, repoName string) return ranks, nil } -func floats64(s string) []float64 { - parts := strings.Split(s, ",") - - var r []float64 - for _, rank := range parts { - f, err := strconv.ParseFloat(rank, 64) - if err != nil { - return nil - } - r = append(r, f) - } - - return r -} - func (sf sourcegraphFake) List(ctx context.Context, indexed []uint32) (*SourcegraphListResult, error) { repos, err := sf.ListRepoIDs(ctx, indexed) if err != nil { @@ -1084,7 +761,6 @@ func (s sourcegraphNop) List(ctx context.Context, indexed []uint32) (*Sourcegrap } func (s sourcegraphNop) ForceIterateIndexOptions(onSuccess func(IndexOptions), onError func(uint32, error), repos ...uint32) { - return } func (s sourcegraphNop) GetDocumentRanks(ctx context.Context, repoName string) (RepoPathRanks, error) { @@ -1124,23 +800,3 @@ func (r *RepoPathRanks) ToProto() *proto.DocumentRanksResponse { Paths: paths, } } - -type noopGRPCClient struct{} - -func (n noopGRPCClient) SearchConfiguration(ctx context.Context, in *proto.SearchConfigurationRequest, opts ...grpc.CallOption) (*proto.SearchConfigurationResponse, error) { - return nil, fmt.Errorf("grpc client not enabled") -} - -func (n noopGRPCClient) List(ctx context.Context, in *proto.ListRequest, opts ...grpc.CallOption) (*proto.ListResponse, error) { - return nil, fmt.Errorf("grpc client not enabled") -} - -func (n noopGRPCClient) DocumentRanks(ctx context.Context, in *proto.DocumentRanksRequest, opts ...grpc.CallOption) (*proto.DocumentRanksResponse, error) { - return nil, fmt.Errorf("grpc client not enabled") -} - -func (n noopGRPCClient) UpdateIndexStatus(ctx context.Context, in *proto.UpdateIndexStatusRequest, opts ...grpc.CallOption) (*proto.UpdateIndexStatusResponse, error) { - return nil, fmt.Errorf("grpc client not enabled") -} - -var _ proto.ZoektConfigurationServiceClient = noopGRPCClient{} diff --git a/go.mod b/go.mod index d40dab2b9..848184d76 100644 --- a/go.mod +++ b/go.mod @@ -19,7 +19,6 @@ require ( github.com/grafana/regexp v0.0.0-20221123153739-15dc172cd2db github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus v1.0.0-rc.0 github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.0.0 - github.com/hashicorp/go-retryablehttp v0.7.4 github.com/keegancsmith/rpc v1.3.0 github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f github.com/opentracing/opentracing-go v1.2.0 @@ -97,6 +96,7 @@ require ( github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.1 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/hashicorp/go-hclog v0.16.2 // indirect + github.com/hashicorp/go-retryablehttp v0.7.4 // indirect github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect github.com/kevinburke/ssh_config v1.2.0 // indirect github.com/kr/pretty v0.3.1 // indirect