add insecure to search reindex - #3505
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Coverage variation | ✅ +0.00% coverage variation (-1.00%) |
| Diff coverage | ✅ 50.00% diff coverage |
Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (994086c) 88539 20995 23.71% Head commit (f511b1a) 88539 (+0) 20997 (+2) 23.71% (+0.00%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>
Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3505) 2 1 50.00% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
I'm not sure about this. Actually I wouldn't want to encourage usage of the --insecure flag, especially not as a default. Could we instead improve the error message (or even deduct the --insecure flag automatically from the config file?)? |
|
I have a history with this. The search index command uses the configured search endpoint. which is an internal grps endpoint. which is not tls encrypted (unless you enable tls for grpc ... try finding the docs). we default to grpc over h2 ... not http2 ... which is why we need to use --insecure. Sucks ... I know ... and annoys me ... but also ... is not really a security issue because we just want to trigger a rescan. which happens in the opencloud process, not the IMO the error message should just use the |
|
documenting it like in opencloud-eu/docs#1153 (review) is the least hassle. |
|
That's hella confusing 😂 I think requiring --insecure always and having it all over the docs is the worst of all options. So maybe just enable it always.. or.. why not the following? Here's what my claude suggests: The CLI already has the server's gRPC TLS config, it just evaluates it the wrong way round.
The config is available in the CLI via Proposed fix, mirroring the go-micro client (the endpoint hunk is optional, today it is a hardcoded flag default): --- a/services/search/pkg/command/index.go
+++ b/services/search/pkg/command/index.go
@@ -46,10 +46,20 @@ func Index(cfg *config.Config) *cobra.Command {
return fmt.Errorf("concurrency %d exceeds max allowed %d", concurrencyFlag, cfg.ReindexMaxConcurrency)
}
+ if !cmd.Flags().Changed("endpoint") {
+ endpointFlag = cfg.GRPC.Addr
+ }
+
var dialOpts []grpc.DialOption
- if cfg.GRPCClientTLS.Mode == "insecure" || insecureFlag {
+ // mirror pkg/service/grpc.NewClient: ""/"off" -> no TLS, "insecure" -> TLS without verification, "on" -> TLS
+ switch {
+ case insecureFlag, cfg.GRPCClientTLS.Mode == "", cfg.GRPCClientTLS.Mode == "off":
dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
- } else {
+ case cfg.GRPCClientTLS.Mode == "insecure":
+ dialOpts = append(dialOpts, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{
+ InsecureSkipVerify: true, //nolint:gosec
+ })))
+ default:
dialOpts = append(dialOpts, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{
MinVersion: tls.VersionTLS12,
})))Verified against a running server built from main with an |
4 participants
during the testing #3197 I noticed that
after added
--insecureworks fine:also added to log:
{"level":"info","service":"search","index":"/var/lib/opencloud/search/bleve-v4","time":"2026-09-10T09:08:19Z","line":"github.com/opencloud-eu/opencloud/services/search/pkg/mapping/reconcile.go:43","message":"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"}