Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/clientconfig: fix TLS configs with only CA #7634

Conversation

simonpasquier
Copy link
Contributor

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

065e3dd introduced a regression: TLS configurations with only a CA file failed to load.

For instance, the following snippet is a valid query configuration:

- static_configs:
  - prometheus.example.com:9090
  scheme: https
  http_config:
    tls_config:
      ca_file: /etc/ssl/cert.pem

The test fixtures (CA, certificate and key files) are copied from prometheus/common and are valid until 2072.

Verification

Added unit tests.

@simonpasquier simonpasquier force-pushed the fix-error-loading-ca-file-only-for-thanos-ruler branch from 1e14dbb to dda10d1 Compare August 13, 2024 06:57
saswatamcode
saswatamcode previously approved these changes Aug 13, 2024
Copy link
Member

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for catching this. Will release v0.36.1 with this then, cc: @MichaHoffmann

@simonpasquier simonpasquier changed the base branch from release-0.36 to main August 13, 2024 07:35
@simonpasquier simonpasquier dismissed saswatamcode’s stale review August 13, 2024 07:35

The base branch was changed.

@simonpasquier simonpasquier force-pushed the fix-error-loading-ca-file-only-for-thanos-ruler branch from dda10d1 to 56e2b19 Compare August 13, 2024 07:36
065e3dd introduced a regression: TLS configurations for Thanos Ruler
query and alerting with only a CA file failed to load.

For instance, the following snippet is a valid query configuration:

```
- static_configs:
  - prometheus.example.com:9090
  scheme: https
  http_config:
    tls_config:
      ca_file: /etc/ssl/cert.pem
```

The test fixtures (CA, certificate and key files) are copied from
prometheus/common and are valid until 2072.

Signed-off-by: Simon Pasquier <[email protected]>
@simonpasquier simonpasquier force-pushed the fix-error-loading-ca-file-only-for-thanos-ruler branch from 56e2b19 to f23b1cd Compare August 13, 2024 07:38
@saswatamcode saswatamcode enabled auto-merge (squash) August 13, 2024 07:38
@saswatamcode saswatamcode merged commit 4fd2d8a into thanos-io:main Aug 13, 2024
20 checks passed
saswatamcode pushed a commit to saswatamcode/thanos that referenced this pull request Aug 13, 2024
065e3dd introduced a regression: TLS configurations for Thanos Ruler
query and alerting with only a CA file failed to load.

For instance, the following snippet is a valid query configuration:

```
- static_configs:
  - prometheus.example.com:9090
  scheme: https
  http_config:
    tls_config:
      ca_file: /etc/ssl/cert.pem
```

The test fixtures (CA, certificate and key files) are copied from
prometheus/common and are valid until 2072.

Signed-off-by: Simon Pasquier <[email protected]>
saswatamcode added a commit that referenced this pull request Aug 13, 2024
* Proxy: Query goroutine leak when `store.response-timeout` is set (#7618)

time.AfterFunc() returns a time.Timer object whose C field is nil,
accroding to the documentation. A goroutine blocks forever on reading
from a `nil` channel, leading to a goroutine leak on random slow
queries.

Signed-off-by: Mikhail Nozdrachev <[email protected]>

* pkg/clientconfig: fix TLS configs with only CA (#7634)

065e3dd introduced a regression: TLS configurations for Thanos Ruler
query and alerting with only a CA file failed to load.

For instance, the following snippet is a valid query configuration:

```
- static_configs:
  - prometheus.example.com:9090
  scheme: https
  http_config:
    tls_config:
      ca_file: /etc/ssl/cert.pem
```

The test fixtures (CA, certificate and key files) are copied from
prometheus/common and are valid until 2072.

Signed-off-by: Simon Pasquier <[email protected]>

* Cut patch release v0.36.1

Signed-off-by: Saswata Mukherjee <[email protected]>

* Fix failing e2e test (#7620)

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>

---------

Signed-off-by: Mikhail Nozdrachev <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Co-authored-by: Mikhail Nozdrachev <[email protected]>
Co-authored-by: Simon Pasquier <[email protected]>
Co-authored-by: Harry John <[email protected]>
saswatamcode added a commit that referenced this pull request Aug 14, 2024
* CHANGELOG: Mark 0.36 as in progress

Signed-off-by: Michael Hoffmann <[email protected]>

* Cut release candidate v0.36.0-rc.0 (#7490)

Signed-off-by: Michael Hoffmann <[email protected]>

* Cut release candidate 0.36.0 rc.1 (#7510)

* *: fix server grpc histograms (#7493)

Signed-off-by: Michael Hoffmann <[email protected]>

* Close endpoints after the gRPC server has terminated (#7509)

Endpoints are currently closed as soon as we receive a SIGTERM or SIGINT.
This causes in-flight queries to get cancelled since outgoing connections
get closed instantly.

This commit moves the endpoints.Close call after the grpc server shutdown
to make sure connections are available as long as the server is running.

Signed-off-by: Filip Petkovski <[email protected]>

* Cut release candidate v0.36.0-rc.1

Signed-off-by: Michael Hoffmann <[email protected]>

---------

Signed-off-by: Michael Hoffmann <[email protected]>
Signed-off-by: Filip Petkovski <[email protected]>
Co-authored-by: Filip Petkovski <[email protected]>

* Cut release v0.36.0 (#7578)

Signed-off-by: Michael Hoffmann <[email protected]>

* Cut patch release `v0.36.1` (#7636)

* Proxy: Query goroutine leak when `store.response-timeout` is set (#7618)

time.AfterFunc() returns a time.Timer object whose C field is nil,
accroding to the documentation. A goroutine blocks forever on reading
from a `nil` channel, leading to a goroutine leak on random slow
queries.

Signed-off-by: Mikhail Nozdrachev <[email protected]>

* pkg/clientconfig: fix TLS configs with only CA (#7634)

065e3dd introduced a regression: TLS configurations for Thanos Ruler
query and alerting with only a CA file failed to load.

For instance, the following snippet is a valid query configuration:

```
- static_configs:
  - prometheus.example.com:9090
  scheme: https
  http_config:
    tls_config:
      ca_file: /etc/ssl/cert.pem
```

The test fixtures (CA, certificate and key files) are copied from
prometheus/common and are valid until 2072.

Signed-off-by: Simon Pasquier <[email protected]>

* Cut patch release v0.36.1

Signed-off-by: Saswata Mukherjee <[email protected]>

* Fix failing e2e test (#7620)

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>

---------

Signed-off-by: Mikhail Nozdrachev <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Co-authored-by: Mikhail Nozdrachev <[email protected]>
Co-authored-by: Simon Pasquier <[email protected]>
Co-authored-by: Harry John <[email protected]>

---------

Signed-off-by: Michael Hoffmann <[email protected]>
Signed-off-by: Filip Petkovski <[email protected]>
Signed-off-by: Mikhail Nozdrachev <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Co-authored-by: Michael Hoffmann <[email protected]>
Co-authored-by: Filip Petkovski <[email protected]>
Co-authored-by: Mikhail Nozdrachev <[email protected]>
Co-authored-by: Simon Pasquier <[email protected]>
Co-authored-by: Harry John <[email protected]>
hczhu-db pushed a commit to databricks/thanos that referenced this pull request Aug 22, 2024
* Proxy: Query goroutine leak when `store.response-timeout` is set (thanos-io#7618)

time.AfterFunc() returns a time.Timer object whose C field is nil,
accroding to the documentation. A goroutine blocks forever on reading
from a `nil` channel, leading to a goroutine leak on random slow
queries.

Signed-off-by: Mikhail Nozdrachev <[email protected]>

* pkg/clientconfig: fix TLS configs with only CA (thanos-io#7634)

065e3dd introduced a regression: TLS configurations for Thanos Ruler
query and alerting with only a CA file failed to load.

For instance, the following snippet is a valid query configuration:

```
- static_configs:
  - prometheus.example.com:9090
  scheme: https
  http_config:
    tls_config:
      ca_file: /etc/ssl/cert.pem
```

The test fixtures (CA, certificate and key files) are copied from
prometheus/common and are valid until 2072.

Signed-off-by: Simon Pasquier <[email protected]>

* Cut patch release v0.36.1

Signed-off-by: Saswata Mukherjee <[email protected]>

* Fix failing e2e test (thanos-io#7620)

Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>

---------

Signed-off-by: Mikhail Nozdrachev <[email protected]>
Signed-off-by: Simon Pasquier <[email protected]>
Signed-off-by: Saswata Mukherjee <[email protected]>
Signed-off-by: 🌲 Harry 🌊 John 🏔 <[email protected]>
Co-authored-by: Mikhail Nozdrachev <[email protected]>
Co-authored-by: Simon Pasquier <[email protected]>
Co-authored-by: Harry John <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants