Skip to content

Conversation

@hengfeiyang
Copy link
Contributor

fixed #9359

@hengfeiyang hengfeiyang requested review from YashodhanJoshi1 and uddhavdave and removed request for uddhavdave November 27, 2025 13:15
@github-actions
Copy link
Contributor

Failed to generate code suggestions for PR

@github-actions github-actions bot added the ☢️ Bug Something isn't working label Nov 27, 2025
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 27, 2025

Greptile Overview

Greptile Summary

Fixes a critical blocker bug where OpenObserve crashes on startup when TLS is enabled by installing the rustls ring CryptoProvider before any TLS initialization occurs.

  • Adds early initialization of rustls ring crypto provider when HTTP or gRPC TLS is enabled
  • Properly handles the rustls 0.23 requirement for process-level CryptoProvider installation
  • Places initialization at the correct point in startup sequence (after config load, before server init)

Confidence Score: 5/5

  • This PR is safe to merge - it's a minimal, targeted fix for a critical blocker
  • The fix directly addresses the root cause by installing the required CryptoProvider before any TLS usage. The implementation is correct: it's placed early in startup, properly checks TLS config flags, and handles errors appropriately. The change is minimal and cannot introduce regressions.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/main.rs 5/5 Adds rustls CryptoProvider initialization before TLS usage - correctly fixes the panic issue

Sequence Diagram

sequenceDiagram
    participant Main as main()
    participant Config as Configuration
    participant Rustls as rustls::crypto::ring
    participant HTTP as HTTP Server
    participant GRPC as gRPC Server
    participant TLS as TLS Module

    Main->>Config: Load config from CLI/env
    Config-->>Main: Return cfg

    Note over Main,Rustls: NEW: Install CryptoProvider early
    alt TLS enabled
        Main->>Rustls: default_provider().install_default()
        Rustls-->>Main: Provider installed
    end

    Main->>Main: Initialize backend jobs
    Main->>Main: Initialize search service
    
    alt HTTP TLS enabled
        Main->>HTTP: Start HTTP server
        HTTP->>TLS: http_tls_config()
        TLS->>Rustls: ServerConfig::builder()
        Note over Rustls: Uses already-installed provider
        Rustls-->>TLS: TLS config
        TLS-->>HTTP: ServerConfig
        HTTP->>HTTP: bind_rustls_0_23()
    else HTTP non-TLS
        Main->>HTTP: Start HTTP server
        HTTP->>HTTP: bind()
    end

    alt gRPC TLS enabled
        Main->>GRPC: Start gRPC server
        GRPC->>GRPC: Load cert/key
        GRPC->>GRPC: tls_config()
        Note over GRPC: Uses tonic TLS (also uses rustls internally)
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@YashodhanJoshi1 YashodhanJoshi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey one concern I have is that here we specify the ring as default provider, but for certain features like email we use aws_lc as the crypto provider (via feature flag) ; can the conflict and cause runtime error if someone tries to send report email (or our cloud related email invite code as well)

Copy link
Contributor

@YashodhanJoshi1 YashodhanJoshi1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested with email client setup, works

@hengfeiyang hengfeiyang merged commit 0c00876 into main Nov 27, 2025
108 of 125 checks passed
@hengfeiyang hengfeiyang deleted the fix/http-tls branch November 27, 2025 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

☢️ Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Listening on TLS port stopped working with v0.16.0

5 participants