cloudflare / cloudflare/pingora

Support starting a `Server` with a `ServerConf`, and make `ServerConf` fields public

Open
#159 3 comments 0 reactions 1 assignee Claimed by @drcaramelsyrup View on GitHub
enhancement
Dominant language
Rust
Stars
27.4k
Forks
1.7k
Avg merge
6h 22m
Merged PRs (30d)
3

Description

## What is the problem your feature solves, or the need it fulfills?

At the moment, there is no constructor for `Server` that allows you to fully set the contents of `ServerConf`. You can create a server with an `Opt` that has a `None` field for a conf path, and the `configuration` field of `Server` is public, but not all fields of `ServerConf` are public:

https://github.com/cloudflare/pingora/blob/0de54eb9071a9c4baccc6bad7acad11e9c54186f/pingora-core/src/server/configuration/mod.rs#L36-L69

This is a little silly though - `ServerConf` implements `Deserialize`, which means I could write my own contents in JSON or YAML and deserialize it myself, meaning that the non-public fields are not effective if the intent is to prevent user modification.

For the `river` application, we will likely have our own configuration file format, so it would be good to be able to disable (or at least skip) the built-in YAML configuration format used by the `pingora` crate. It may be good to split this functionality into an optional `pingora-config` crate, as "frontends" to pingora like river may want to use their own.

## Describe the solution you'd like

1. Create a new constructor to `Server` that takes `ServerConf` directly (and maybe not `Opt` at all?)
2. Make all fields of `ServerConf` public, or provide some methods to be able to set all fields
3. (optional) split out the configuration file handling into a separate crate, to reduce dependencies (such as StructOpt) and build time

## Describe alternatives you've considered

If this is not implemented, I will need to either:

1. be forced to use and extend the YAML file used by pingora, making that a stability issue for river to ensure compatibility with new items added by pingora in the future
2. Use a somewhat hacky "deserialize to sneak in fields" workflow

## Additional context

This is for the `river` application.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.