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

Add support for KDL configuration #42

Merged
merged 4 commits into from
Jun 10, 2024
Merged

Add support for KDL configuration #42

merged 4 commits into from
Jun 10, 2024

Conversation

jamesmunns
Copy link
Collaborator

@jamesmunns jamesmunns commented Jun 5, 2024

Experimenting with KDL, as #39 will introduce quite a bit of nested configuration that may be difficult to manage in TOML.

@jamesmunns jamesmunns added the F-Configuration Functionality relating to configuration label Jun 5, 2024
@jamesmunns jamesmunns marked this pull request as draft June 5, 2024 20:45
@jamesmunns
Copy link
Collaborator Author

Here's my current state, that I think I'm happy with. Here's the test_config in kdl (38 lines, 1038 characters):

system {
    threads-per-service 8
}

services {
    Example1 {
        listeners {
            "0.0.0.0:8080"
            "0.0.0.0:4443" cert-path="./assets/test.crt" key-path="./assets/test.key"
            "0.0.0.0:8443" cert-path="./assets/test.crt" key-path="./assets/test.key"
        }

        connectors {
            "91.107.223.4:443" tls-sni="onevariable.com"
        }

        path-control {
            upstream-request {
                filter kind="remove-header-key-regex" pattern=".*SECRET.*"
                filter kind="remove-header-key-regex" pattern=".*secret.*"
                filter kind="upsert-header" key="x-proxy-friend" value="river"
            }
            upstream-response {
                filter kind="remove-header-key-regex" pattern=".*ETag.*"
                filter kind="upsert-header" key="x-with-love-from" value="river"
            }
        }
    }

    Example2 {
        listeners {
            "0.0.0.0:8000"
        }
        connectors {
            "91.107.223.4:80"
        }
    }
}

And what it looks like in TOML (42 lines, 1226 characters):

[system]
threads-per-service = 8

[[basic-proxy]]
name = "Example1"
    [[basic-proxy.listeners]]
        [basic-proxy.listeners.source]
        kind = "Tcp"
            [basic-proxy.listeners.source.value]
            addr = "0.0.0.0:8080"

    [[basic-proxy.listeners]]
        [basic-proxy.listeners.source]
        kind = "Tcp"
        [basic-proxy.listeners.source.value]
        addr = "0.0.0.0:4443"
            [basic-proxy.listeners.source.value.tls]
            cert_path = "./assets/test.crt"
            key_path = "./assets/test.key"

    [basic-proxy.connector]
    proxy_addr = "91.107.223.4:443"
    tls_sni = "onevariable.com"

    [basic-proxy.path-control]
    upstream-request-filters = [
        { kind = "remove-header-key-regex", pattern = ".*(secret|SECRET).*" },
        { kind = "upsert-header", key = "x-proxy-friend", value = "river" },
    ]

    upstream-response-filters = [
        { kind = "remove-header-key-regex", pattern = ".*ETag.*" },
        { kind = "upsert-header", key = "x-with-love-from", value = "river" },
    ]

[[basic-proxy]]
name = "Example2"
listeners = [
    { source = { kind = "Tcp", value = { addr = "0.0.0.0:8000" } } }
]
connector = { proxy_addr = "91.107.223.4:80" }

It's not necessarily as short as I expected it to be, but I think there's a lot of lines that are trailing braces (}), but there's also a LOT less repeated verbosity for nested items.

@jamesmunns
Copy link
Collaborator Author

Interestingly, this brings our configuration syntax MUCH closer to something like NGINX:

http {
    upstream backend {
        server backend1.example.com;
        server backend2.example.com;
        server 192.0.0.1 backup;
    }

    server {
        location / {
            proxy_pass http://backend;
        }
    }
}

HOWEVER, it is challenging in KDL to express a pattern like server 192.0.0.1 backup - essentially KEYWORD KEYWORD KEYWORD, where it is much easier to express it in a way like server addr="192.0.0.1" option=backup or so.

I don't think this is a blocker, but it might end up being a little "uncanny valley" for former NGINX users.

@bebehei
Copy link

bebehei commented Jun 9, 2024

As a heavy nginx user, this looks much better. Visibly, there is more configuration data in the file than configuration markup

HOWEVER, it is challenging in KDL to express a pattern like server 192.0.0.1 backup - essentially KEYWORD KEYWORD KEYWORD, where it is much easier to express it in a way like server addr="192.0.0.1" option=backup or so.

This is a much smaller problem than you think. The server addr="1.2.3.4" might be redundant. You could just drop the server tag completely.

Stating some options explicitly is somewhat okay. I guess, being able to just throw backup in the upstream's server line requires a self-built configuration parser, I guess.


On the other hand: Is there something like configuration inheritance? Specifying a vHost will most likely still give all three listening sockets the same certificate. So, explicitly stating the same certificate for every endpoint somewhat redundant.

services {
    Example1 {
        listeners {
            "0.0.0.0:8080"
            "0.0.0.0:4443" cert-path="./assets/test.crt" key-path="./assets/test.key"
            "0.0.0.0:8443" cert-path="./assets/test.crt" key-path="./assets/test.key"
        }
    }
}

If KDL would not do this, you could also use YAML for this and offload the problem to the configuration language itself:

services:
  Example:
    listeners:
      "0.0.0.0:8443": &Example-default-certs
        tls: true
        crt: ./assets/test.crt
        key: ./assets/test.key
      "0.0.0.0:4443":
        <<: *Example-default-certs

@jamesmunns jamesmunns changed the title WIP: Begin KDL experiment Add support for KDL configuration Jun 10, 2024
@jamesmunns jamesmunns marked this pull request as ready for review June 10, 2024 17:07
@jamesmunns
Copy link
Collaborator Author

This is a much smaller problem than you think. The server addr="1.2.3.4" might be redundant. You could just drop the server tag completely.

That's fair! I don't think this is a blocker, just something I was noting.

On the other hand: Is there something like configuration inheritance?

Built in to KDL: not specifically. We could invent something, but for as long as we can get away with it, I'd prefer to keep the configuration file parsing as simple as possible, to avoid accidentally making it turing complete :)

We will likely also support JSON configuration in the future, and recommend that for anyone that needs to generate larger or dynamic configuration, or would like to build their own templating solution for configuration.

@jamesmunns jamesmunns merged commit 0c32967 into main Jun 10, 2024
5 checks passed
@jamesmunns jamesmunns deleted the james/kdl-experiment branch June 10, 2024 17:11
@bebehei
Copy link

bebehei commented Jun 11, 2024

We will likely also support JSON configuration in the future

On the one hand, this seems nice. But for configuration, I'd like to see for YAML. Main benefit: It allows comments and does not need another external (mostly self-written tool) to generate configuration.

YAML allows 90% of the use users to use the default configuration format in raw.

@jamesmunns
Copy link
Collaborator Author

@bebehei I'm not totally against YAML, but support is likely not planned in the near future. Previous discussions here:

The plan for now is to keep the core of the software flexible to allow us to support multiple configuration languages for now, as we focus on other core development tasks.

@johnpyp
Copy link

johnpyp commented Jul 23, 2024

I'm a very big fan of this syntax using KDL! Looks great - far more readable than the current toml config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-Configuration Functionality relating to configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants