feat: add tls block in common/net/config to use in http/grpc block #4082

fgouteroux · 2025-07-30T10:21:55Z

PR Description

Add the tls in http and grpc block:

prometheus.receive_http "default" {
  http {
    listen_address = "0.0.0.0"
    listen_port    = 9090

    tls {
      cert_file = "/etc/alloy/ssl/cert.pem"
      key_file  = "/etc/alloy/ssl/key.pem"
    }

  }

Which issue(s) this PR fixes

Fixes #1082

Notes to the Reviewer

PR Checklist

CHANGELOG.md updated
Documentation added
Tests updated
Config converters updated

CLAassistant · 2025-07-30T10:22:02Z

All committers have signed the CLA.

ptodev

Hi @fgouteroux , thank you for the PR! Could you please edit the docs for those components to include the new config arguments? It'd be good to also add tests which start those components with TLS enabled and test sending data to them.

dehaansa · 2025-07-30T18:04:35Z

One note in addition to paulin's request - we'll also need an entry in the changelog. If you don't have capacity to put the finishing touches on the PR w/ docs & changelog, let us know and we'll try to allocate some capacity to update them.

…eceive_http

fgouteroux · 2025-07-31T13:01:20Z

@ptodev @dehaansa I try to update doc but I'm not sure I do it well.

As this PR update internal/component/common/net/config.go, I think it could impact other component than loki.source.api and prometheus.receive_http.

After a quick search into component calling fnet.ServerConfig I found:

aws_firehose
gcplog
heroku
pyroscope.receive_http

I would appreciate a review about this in order to be sure that it doesn't break anything.

Thanks

docs/sources/shared/reference/components/loki-server-http.md

fgouteroux · 2025-08-01T10:03:17Z

I think this is the wrong place for this information. The http block can have arguments to configure the block. If the tls_config block is a child block of the http block, then it should be added as a child block to the components that use this child block... based on a quick grep, that would be:

But I think it's more simple to keep in loki-server-http.md as the http block reference is already defined, is not ?

### `http`

{{< docs/shared lookup="reference/components/loki-server-http.md" source="alloy" version="<ALLOY_VERSION>" >}}

Please guide me on what you expect, I will follow your suggestions.

kalleep

Nice work, left a couple of suggestions just so we keep it consistent with other components.

I think we can also update converter code too support these new arguments. The one I found was promtail converter. Currently we are rejecting configs that include tls config https://github.com/grafana/alloy/blob/main/internal/converter/internal/common/weaveworks_server.go#L55-L66.

I can update these after the pr is merged if you don't want to or don't have the time to do it

internal/component/common/net/config.go

fgouteroux · 2025-08-05T09:19:27Z

Nice work, left a couple of suggestions just so we keep it consistent with other components.

I think we can also update converter code too support these new arguments. The one I found was promtail converter. Currently we are rejecting configs that include tls config https://github.com/grafana/alloy/blob/main/internal/converter/internal/common/weaveworks_server.go#L55-L66.

I can update these after the pr is merged if you don't want to or don't have the time to do it

If you can do it, it will be better, I'm in holidays soon. thx.

fgouteroux · 2025-08-05T10:00:57Z

@kalleep I applied your suggestions, let me know if missing something.

dehaansa · 2025-08-05T19:47:13Z

I think this is the wrong place for this information. The http block can have arguments to configure the block. If the tls_config block is a child block of the http block, then it should be added as a child block to the components that use this child block... based on a quick grep, that would be:

But I think it's more simple to keep in loki-server-http.md as the http block reference is already defined, is not ?
### `http`

{{< docs/shared lookup="reference/components/loki-server-http.md" source="alloy" version="<ALLOY_VERSION>" >}}
Please guide me on what you expect, I will follow your suggestions.

It might be simpler, but not consistent with the rest of our docs. We separate attributes and blocks. Below is an example of what the blocks section looks like for another component with nested blocks.

## Blocks

You can use the following blocks with `discovery.consul`:

| Block                                 | Description                                                | Required |
| ------------------------------------- | ---------------------------------------------------------- | -------- |
| [`authorization`][authorization]      | Configure generic authorization to the endpoint.           | no       |
| [`basic_auth`][basic_auth]            | Configure `basic_auth` for authenticating to the endpoint. | no       |
| [`oauth2`][oauth2]                    | Configure OAuth 2.0 for authenticating to the endpoint.    | no       |
| `oauth2` > [`tls_config`][tls_config] | Configure TLS settings for connecting to the endpoint.     | no       |
| [`tls_config`][tls_config]            | Configure TLS settings for connecting to the endpoint.     | no       |

fgouteroux · 2025-08-06T07:20:08Z

@dehaansa doc updated to use block. I also rename loki-server-http.md to server-http.md as it is used by others components

clayton-cornell · 2025-08-08T17:11:23Z

@fgouteroux There are some doc structure issues with your proposed changes that will cause some issues with the published content. It's a bit difficult to explain in a suggestion box here in the web UI. Do you mind if I check out your fork and make the changes myself and push back to the PR?

fgouteroux · 2025-08-08T17:31:41Z

@fgouteroux There are some doc structure issues with your proposed changes that will cause some issues with the published content. It's a bit difficult to explain in a suggestion box here in the web UI. Do you mind if I check out your fork and make the changes myself and push back to the PR?

@clayton-cornell yes no worries

kalleep

Sorry forgot to submit my review

kalleep · 2025-08-07T07:02:21Z

internal/component/common/net/config.go

+	if h.TLSConfig != nil {
+		h.TLSConfig.Into(&c.HTTPTLSConfig)
+	}
+	if h.TLSConfig.MinVersion != c.MinVersion {
+		c.MinVersion = h.TLSConfig.MinVersion
+	}
+	if h.TLSConfig.CipherSuites != c.CipherSuites {
+		c.CipherSuites = h.TLSConfig.CipherSuites
+	}


Suggested change

if h.TLSConfig != nil {

h.TLSConfig.Into(&c.HTTPTLSConfig)

}

if h.TLSConfig.MinVersion != c.MinVersion {

c.MinVersion = h.TLSConfig.MinVersion

}

if h.TLSConfig.CipherSuites != c.CipherSuites {

c.CipherSuites = h.TLSConfig.CipherSuites

}

if h.TLSConfig != nil {

h.TLSConfig.Into(&c.HTTPTLSConfig)

if h.TLSConfig.MinVersion != c.MinVersion {

c.MinVersion = h.TLSConfig.MinVersion

}

if h.TLSConfig.CipherSuites != c.CipherSuites {

c.CipherSuites = h.TLSConfig.CipherSuites

}

}

This will fix the panic you see in tests

clayton-cornell · 2025-08-08T19:50:41Z

@fgouteroux I've reworked the doc structure a bit. We have to be really careful if we include headings in shared content because we can't always guarantee that the heading levels will be right. It's simpler in the long run to keep shared content as a single block of content at a single heading level.

So, with rework the topics should look like this for topics that just have HTTP:

and this for topics with both gRPC and HTTP:

feat: add tls_config in common/net/config to use in http block

093f6ca

fgouteroux requested a review from a team as a code owner July 30, 2025 10:21

ptodev reviewed Jul 30, 2025

View reviewed changes

ptodev self-assigned this Jul 30, 2025

fgouteroux added 2 commits July 30, 2025 16:11

fix: lint issue

0497c41

feat: add test for prometheus_receive_http

dddee1c

fgouteroux added 2 commits July 31, 2025 12:43

feat: create dedicated TLS tests for loki/source/api and prometheus/r…

38f7b8b

…eceive_http

feat: update doc about tls_config block

4300a1a

fgouteroux requested a review from clayton-cornell as a code owner July 31, 2025 12:52

clayton-cornell reviewed Jul 31, 2025

View reviewed changes

docs/sources/shared/reference/components/loki-server-http.md Outdated Show resolved Hide resolved

doc: update doc and changelog

91298f3

fgouteroux changed the title ~~feat: add tls_config in common/net/config to use in http block~~ feat: add tls_config in common/net/config to use in http/grpc block Aug 1, 2025

Merge branch 'main' into feat/add_tls_config_in_http_block

8240eab

kalleep reviewed Aug 4, 2025

View reviewed changes

internal/component/common/net/config.go Outdated Show resolved Hide resolved

internal/component/common/net/config.go Outdated Show resolved Hide resolved

internal/component/common/net/config.go Outdated Show resolved Hide resolved

fix: apply suggested changes

8f1b9d4

fix: add tls MinVersion and CipherSuites as suggested

6ecb20a

fix: doc update

7a0a0bf

fgouteroux changed the title ~~feat: add tls_config in common/net/config to use in http/grpc block~~ feat: add tls block in common/net/config to use in http/grpc block Aug 6, 2025

Merge branch 'main' into feat/add_tls_config_in_http_block

db16a23

Reconfigure docs to include TLS block

6337be4

Back out some prettier changes

9d517bd

kalleep reviewed Aug 8, 2025

View reviewed changes

clayton-cornell added 3 commits August 8, 2025 12:53

Fix gRPC name

61736f1

Fix more prettier changes

1f3c2d2

Another change from prettier backed out

8b255eb

feat: add tls block in common/net/config to use in http/grpc block #4082

Are you sure you want to change the base?

feat: add tls block in common/net/config to use in http/grpc block #4082

Conversation

fgouteroux commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Description

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

Uh oh!

CLAassistant commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ptodev left a comment

Choose a reason for hiding this comment

Uh oh!

dehaansa commented Jul 30, 2025

Uh oh!

fgouteroux commented Jul 31, 2025

Uh oh!

Uh oh!

fgouteroux commented Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kalleep left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fgouteroux commented Aug 5, 2025

Uh oh!

fgouteroux commented Aug 5, 2025

Uh oh!

dehaansa commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fgouteroux commented Aug 6, 2025

Uh oh!

clayton-cornell commented Aug 8, 2025

Uh oh!

fgouteroux commented Aug 8, 2025

Uh oh!

kalleep left a comment

Choose a reason for hiding this comment

Uh oh!

kalleep Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

kalleep Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

clayton-cornell commented Aug 8, 2025

Uh oh!

Uh oh!

fgouteroux commented Jul 30, 2025 •

edited

Loading

CLAassistant commented Jul 30, 2025 •

edited

Loading

fgouteroux commented Aug 1, 2025 •

edited

Loading

dehaansa commented Aug 5, 2025 •

edited

Loading