fix(search): honor the gRPC client TLS mode in the index command

services/search/MIGRATION.md

@@ -5,15 +5,20 @@ leaves the old one untouched. The service starts normally, but the new index is
 empty: search finds nothing until it is filled. The old index stays around
 until you remove it.
 
-## v7.x.x to v8.0.0
+## v7.x.x to v8.x.x
+
+> [!IMPORTANT]
+> On 8.0.x and 8.1.x, `opencloud search index` needs `--insecure` in the
+> default setup, where the search service runs gRPC without TLS: add it to the
+> commands below. Later releases don't need it.
 
 ### OpenSearch
 
 Fill the new index by indexing all spaces again:
 
 ```shell
 # the service keeps running while it happens
-opencloud search index --all-spaces --insecure
+opencloud search index --all-spaces
 ```
 
 Once the new index is filled, every index but the one with the highest
@@ -31,7 +36,7 @@ The new index is a directory next to the old `bleve` one, both in
 bleve index cannot be copied, index all spaces again:
 
 ```shell
-opencloud search index --all-spaces --insecure
+opencloud search index --all-spaces
 ```
 
 Once the new index is filled, every directory but the one with the highest

services/search/README.md

@@ -124,16 +124,18 @@ opencloud search index --space $SPACE_ID
 It can also be used to re-index all spaces:
 
 ```shell
-opencloud search index --all-spaces --insecure
+opencloud search index --all-spaces
 ```
 
 Please note that a reindex only picks up new or changed files. Files that have already been indexed are not scanned again, even if the configuration or the whole extractor has been changed. To force a full rescan (re-running the extractor on every file) you need to use the `force-rescan` flag:
 
 
 ```shell
-opencloud search index --all-spaces --force-rescan --insecure
+opencloud search index --all-spaces --force-rescan
 ```
 
+The command connects to the service's gRPC address (`SEARCH_GRPC_ADDR`) unless `--endpoint` is given, with the TLS mode set in `OC_GRPC_CLIENT_TLS_MODE`. With `on`, the server certificate is verified against that address, so if `SEARCH_GRPC_ADDR` is a bind address such as `0.0.0.0:9220`, pass `--endpoint` with a host name the certificate is valid for.
+
 ## Metrics
 
 The search service exposes the following prometheus metrics at `<debug_endpoint>/metrics` (as configured using the `SEARCH_DEBUG_ADDR` env var):

services/search/pkg/command/command_suite_test.go

@@ -0,0 +1,13 @@
+package command_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestCommand(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Command Suite")
+}

services/search/pkg/command/index.go

@@ -2,7 +2,6 @@ package command
 
 import (
 	"context"
-	"crypto/tls"
 	"errors"
 	"fmt"
 	"io"
@@ -15,10 +14,8 @@ import (
 	"github.com/opencloud-eu/opencloud/services/search/pkg/config"
 	"github.com/opencloud-eu/opencloud/services/search/pkg/config/parser"
 
+	"github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool"
 	"github.com/spf13/cobra"
-	"google.golang.org/grpc"
-	"google.golang.org/grpc/credentials"
-	"google.golang.org/grpc/credentials/insecure"
 )
 
 // Index is the entrypoint for the server command.
@@ -45,16 +42,23 @@ func Index(cfg *config.Config) *cobra.Command {
 				return fmt.Errorf("concurrency %d exceeds max allowed %d", concurrencyFlag, cfg.ReindexMaxConcurrency)
 			}
 
-			var dialOpts []grpc.DialOption
-			if cfg.GRPCClientTLS.Mode == "insecure" || insecureFlag {
-				dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
-			} else {
-				dialOpts = append(dialOpts, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{
-					MinVersion: tls.VersionTLS12,
-				})))
+			if !cmd.Flags().Changed("endpoint") {
+				endpointFlag = cfg.GRPC.Addr
 			}
 
-			conn, err := grpc.NewClient(endpointFlag, dialOpts...)
+			tlsMode := pool.TLSOff
+			if !insecureFlag {
+				mode, err := pool.StringToTLSMode(cfg.GRPCClientTLS.Mode)
+				if err != nil {
+					return err
+				}
+				tlsMode = mode
+			}
+
+			conn, err := pool.NewConn(endpointFlag,
+				pool.WithTLSMode(tlsMode),
+				pool.WithTLSCACert(cfg.GRPCClientTLS.CACert),
+			)
 			if err != nil {
 				return fmt.Errorf("failed to dial %s: %w", endpointFlag, err)
 			}
@@ -122,8 +126,8 @@ func Index(cfg *config.Config) *cobra.Command {
 	)
 	indexCmd.Flags().String(
 		"endpoint",
-		"127.0.0.1:9220",
-		"the address of the search service gRPC endpoint.",
+		"",
+		"the address of the search service gRPC endpoint. Defaults to the service's configured gRPC address (SEARCH_GRPC_ADDR). With OC_GRPC_CLIENT_TLS_MODE=on the server certificate must be valid for this address.",
 	)
 	indexCmd.Flags().Bool(
 		"insecure",

services/search/pkg/command/index_test.go

@@ -0,0 +1,74 @@
+package command_test
+
+import (
+	"net"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
+
+	"github.com/opencloud-eu/opencloud/pkg/shared"
+	searchsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/search/v0"
+	"github.com/opencloud-eu/opencloud/services/search/pkg/command"
+	"github.com/opencloud-eu/opencloud/services/search/pkg/config"
+)
+
+type fakeSearchProvider struct {
+	searchsvc.UnimplementedSearchProviderServer
+}
+
+func (fakeSearchProvider) IndexSpace(_ *searchsvc.IndexSpaceRequest, stream grpc.ServerStreamingServer[searchsvc.IndexSpaceResponse]) error {
+	return stream.Send(&searchsvc.IndexSpaceResponse{SpaceId: "space-1", IndexedSpaces: 1, TotalSpaces: 1})
+}
+
+var _ = Describe("Index", func() {
+	var addr string
+
+	// runIndex runs the index command against a search service without TLS
+	// that listens on the configured gRPC address.
+	runIndex := func(mode string, args ...string) error {
+		cfg := &config.Config{
+			GRPC:                  config.GRPCConfig{Addr: addr},
+			GRPCClientTLS:         &shared.GRPCClientTLS{Mode: mode},
+			ReindexMaxConcurrency: 3,
+		}
+		cmd := command.Index(cfg)
+		Expect(cmd.ParseFlags(append([]string{"--all-spaces"}, args...))).To(Succeed())
+		return cmd.RunE(cmd, nil)
+	}
+
+	BeforeEach(func() {
+		lis, err := net.Listen("tcp", "127.0.0.1:0")
+		Expect(err).ToNot(HaveOccurred())
+		srv := grpc.NewServer(grpc.Creds(insecure.NewCredentials()))
+		searchsvc.RegisterSearchProviderServer(srv, fakeSearchProvider{})
+		go func() { _ = srv.Serve(lis) }()
+		DeferCleanup(srv.Stop)
+		addr = lis.Addr().String()
+	})
+
+	DescribeTable("connects without TLS",
+		func(mode string, args ...string) {
+			Expect(runIndex(mode, args...)).To(Succeed())
+		},
+		Entry("when the mode is unset", ""),
+		Entry("when the mode is off", "off"),
+		Entry("when --insecure is passed", "", "--insecure"),
+		Entry("when --insecure is passed with an unknown mode", "bogus", "--insecure"),
+	)
+
+	It("uses TLS when the mode is insecure", func() {
+		Expect(runIndex("insecure")).To(MatchError(ContainSubstring("first record does not look like a TLS handshake")))
+	})
+
+	It("rejects an unknown mode", func() {
+		Expect(runIndex("bogus")).To(MatchError(ContainSubstring("unknown TLS mode")))
+	})
+
+	It("prefers --endpoint over the configured gRPC address", func() {
+		endpoint := addr
+		addr = "127.0.0.1:1"
+		Expect(runIndex("", "--endpoint", endpoint)).To(Succeed())
+	})
+})

services/search/pkg/mapping/reconcile.go

@@ -27,7 +27,7 @@ func Reconcile(index string, r SchemaReconciler, logger log.Logger) (Classificat
 	case VerdictAdditive:
 		persisted, err := r.ApplyAdditive()
 		if persisted {
-			logger.Warn().Strs("fields", classification.NewFields).Str("index", index).Msg("extended the search index mapping with new fields; documents indexed before the upgrade do not contain them and queries on these fields will miss those documents until they are re-indexed; to re-index everything run: opencloud search index --all-spaces --force-rescan --insecure")
+			logger.Warn().Strs("fields", classification.NewFields).Str("index", index).Msg("extended the search index mapping with new fields; documents indexed before the upgrade do not contain them and queries on these fields will miss those documents until they are re-indexed; to re-index everything run: opencloud search index --all-spaces --force-rescan")
 		}
 		if err != nil {
 			return classification, err
@@ -40,5 +40,5 @@ func Reconcile(index string, r SchemaReconciler, logger log.Logger) (Classificat
 // LogNewIndexCreated logs that a fresh, empty index was created and how to
 // backfill it. The create path does not run through Reconcile.
 func LogNewIndexCreated(logger log.Logger, index string) {
-	logger.Info().Str("index", index).Msg("created a new empty search index; if this OpenCloud instance already held files, they are not in it yet, index them by running: opencloud search index --all-spaces --force-rescan --insecure")
+	logger.Info().Str("index", index).Msg("created a new empty search index; if this OpenCloud instance already held files, they are not in it yet, index them by running: opencloud search index --all-spaces --force-rescan")
 }