Sorry for the delay in reviewing this PR.

This is based on the structure that the prometheus/exporter-toolkit web-config uses.

Why not pull in the prometheus/exporter-toolkits TLSConfig instead of rolling our own here?

I agree that the functionality added in this PR is useful (exporting zrepl prometheus metrics over TLS) but I don't want to maintain a YAML <=> tls.Config mapping.

I notice that this project is still stuck on the ancient Go 1.12

Ah, I see, the go 1.12 line in the go.mod file.
Bump it as you need to get this PR done.
zrepl is behind on bumping its Go dependencies, so, possibly there might be trouble upgrading to >= go 1.21 , which is when the Go toolchain starts enforcing

At go 1.21 or higher:

• The go line declares a required minimum version of Go to use with this module.
• The go line must be greater than or equal to the go line of all dependencies.
• The go command no longer attempts to maintain compatibility with the previous older version of Go.
• The go command is more careful about keeping checksums of go.mod files in the go.sum file.

[https://go.dev/ref/mod#go-mod-file-go]

I'm happy to rework this and pull in prometheus/exporter-toolkit, but it's going to mean a bump of minimum golang version to 1.22 https://github.com/prometheus/exporter-toolkit/blob/master/go.mod#L3

If you're happy with that, then I'll change it up- let me know

Hmm, I guess I'll have to bump a lot of dependencies first.

I will do that in a separate PR, but to unblock you, just do whatever bumping you need to do to pull in the exporter-toolkit.

I'll rebase your PR when I'm done.

For the record, WIP branch with dependency updates here: #819

FYI, #819 was merged a couple of days ago, so, minimum Go version is 1.22 now.

Also, I restructured the repository and moved virtually all of the code into internal/.
Hierarchy is the same, so, should be straight-forward to update your PR.

Are you still planning to work on this?

Thanks for the update. Yes, I plan to pick this up at some point when time allows

Dropping the TLSConfig struct made this patch a lot smaller. So small in fact that I'm having a hard time believing it 😆 Testing it now

Seems like it's gonna need some more work. Even ignoring the subtle differences I introduced, it doesn't seem to accept my config

could not parse config: yaml: unmarshal errors:
  line 19: cannot unmarshal !!str `TLS12` into web.TLSVersion
  line 20: cannot unmarshal !!str `TLS13` into web.TLSVersion
  line 23: cannot unmarshal !!str `TLS_ECD...` into web.Cipher
  line 25: cannot unmarshal !!str `TLS_AES...` into web.Cipher
  line 26: cannot unmarshal !!str `TLS_CHA...` into web.Cipher
  line 14: field cert_file is required but not given
  line 14: field key_file is required but not given
  line 14: field client_ca_file is required but not given
  line 14: field cipher_suites is required but has zero value
  line 14: field curve_preferences is required but not given
  line 14: field min_version is required but has zero value
  line 14: field max_version is required but has zero value
  line 14: field prefer_server_cipher_suites is required but not given
  line 14: field client_allowed_sans is required but not given

Seems to be some side-effect of the strict unmarshaling?

Transitioning this back to draft state, please reopen and ping me once ready.

Looks like the problem is zrepl's custom YAML decoder: zrepl/yaml-config@3fc339b
The upstream prometheus/exporter-toolkit config structs are written to work with go-yaml/yaml and not forks that change unmarshaling behaviour.

We could revert this patch to the initial implementation and carry the burden of having to re-implement TLSConfig

Why not pull in the prometheus/exporter-toolkits TLSConfig instead of rolling our own here?

I guess we found a reason why 😁

Alternatively we could change zrepl to use the upstream go-yaml implementation, or maybe revert optional=true to required=false to be more compatible with other external structs that expect to be parsed from yaml.

edit: one more option I think might be viable (and maybe a reasonable middle-ground) is to reimplement the TLSConfig struct just for unmarshaling, then copy all fields into the prometheus/exporter-toolkit struct for use in the application?

As-is, I don't see any other ways forward. Let me know what you want to do @problame

Apologies for letting this sit for so long, life got in the way.

edit: one more option I think might be viable (and maybe a reasonable middle-ground) is to reimplement the TLSConfig struct just for unmarshaling, then copy all fields into the prometheus/exporter-toolkit struct for use in the application?

This sounds acceptable, let me know if you want to pick this up, or close this PR if you (understandably) lost interest in contributing.