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

docs for PR caddyserver/caddy#6573 #424

Open
wants to merge 17 commits into
base: 2.9
Choose a base branch
from

Conversation

MayCXC
Copy link

@MayCXC MayCXC commented Oct 10, 2024

```

- **<hosts...>** is the list of host interfaces to bind which to bind the listener.
- **<protocols...>** is an optional override of the HTTP protocols to enable for the listener.
Copy link
Member

Choose a reason for hiding this comment

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

This should probably list the allowed options

@@ -16,10 +16,13 @@ Note that binding sites inconsistently may result in unintended consequences. Fo
## Syntax

```caddy-d
bind <hosts...>
bind <hosts...> {
protocols <protocol> ...
Copy link
Member

Choose a reason for hiding this comment

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

Is it moreseo like this?

Suggested change
protocols <protocol> ...
protocols <protocols...>

<protocol> ... implies a single protocol is given, which can itself have additional options, but from my reading of the examples, it's just a list of protocols.

Copy link
Author

Choose a reason for hiding this comment

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

with the ... after, I thought this was the notation for at least one protocol, then possibly more. there are other options that use both, but I thought the distinction was >=1 vs >=0.

Copy link
Member

@francislavoie francislavoie Oct 10, 2024

Choose a reason for hiding this comment

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

We use <args...> to to mean "one or more". See https://caddyserver.com/docs/caddyfile/directives#syntax. If it was optional, it would be [<args...>].

@@ -192,21 +194,44 @@ Default: `443`


##### `default_bind`
The default bind address(es) to be used for all sites, if the [`bind` directive](/docs/caddyfile/directives/bind) is not used in the site. Default: empty, which binds to all interfaces.
The default bind address(es) and optional HTTP protocol(s) to serve with them for all sites, if the [`bind` directive](/docs/caddyfile/directives/bind) is not used in the site. If multiple `default_bind` directives are present, each will be applied to servers with no `bind` directive in the order they were given. Default: empty, which binds to all interfaces, and serves the default protocols (h1+h2+h3) on them.
Copy link
Member

Choose a reason for hiding this comment

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

Prefer to show the options in the way they would appear in the Caddyfile

Suggested change
The default bind address(es) and optional HTTP protocol(s) to serve with them for all sites, if the [`bind` directive](/docs/caddyfile/directives/bind) is not used in the site. If multiple `default_bind` directives are present, each will be applied to servers with no `bind` directive in the order they were given. Default: empty, which binds to all interfaces, and serves the default protocols (h1+h2+h3) on them.
The default bind address(es) and optional HTTP protocol(s) to serve with them for all sites, if the [`bind` directive](/docs/caddyfile/directives/bind) is not used in the site. If multiple `default_bind` directives are present, each will be applied to servers with no `bind` directive in the order they were given. Default: empty, which binds to all interfaces, and serves the default protocols (`h1 h2 h3`) on them.

Also this section should probably mention the list of allowed options

src/docs/markdown/caddyfile/options.md Outdated Show resolved Hide resolved
src/docs/markdown/caddyfile/options.md Outdated Show resolved Hide resolved
@@ -32,6 +32,7 @@ The network can be any of the following; ones suffixed with `4` or `6` are IPv4
- UDP: `udp`, `udp4`, `udp6`
- IP: `ip`, `ip4`, `ip6`
- Unix: `unix`, `unixgram`, `unixpacket`
- File descriptors: `fd`, `fdgram`
Copy link
Member

Choose a reason for hiding this comment

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

I think a short paragraph to explain what a "file descriptor" means in this context would help

@MayCXC MayCXC requested a review from francislavoie October 10, 2024 13:14

In the case of IPv6 addresses, the address must be enclosed in square brackets `[]`. The zone identifier (starting with `%`) is optional (often used for link-local addresses).

In the case of file descriptors, the host must be an unsigned [integer literal](https://go.dev/ref/spec#Integer_literals).
Copy link
Member

Choose a reason for hiding this comment

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

Could we explain here a bit how this works? Like what does the number actually represent? I don't have a good understanding of how file descriptors are useful, and I assume most users wouldn't understand the point of it either.

In all these docs, I only see examples with {env.*} but no actual concrete values, so as a user I would have no idea what the env var values should even look like.

}
```

To bind to inherited file descriptors specified with [environment placeholders](/docs/conventions#placeholders):
Copy link
Member

Choose a reason for hiding this comment

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

I think this should explain a bit why you'd want to use file descriptors. As a reader, I think "but why?"

@francislavoie francislavoie changed the base branch from master to 2.9 October 11, 2024 14:55
@francislavoie
Copy link
Member

Changed the base to the 2.9 branch (to queue up docs for when we release)

@francislavoie francislavoie changed the title docs for PR #6573 docs for PR caddyserver/caddy#6573 Oct 11, 2024
@MayCXC
Copy link
Author

MayCXC commented Oct 11, 2024

Changed the base to the 2.9 branch (to queue up docs for when we release)

I will add a paragraph about socket activation, fds, how/why they are used, and link all mentions of fd and fdgram to it. it would be a good place to link to https://github.com/eriksjolund/podman-caddy-socket-activation too.

@francislavoie
Copy link
Member

francislavoie commented Oct 11, 2024

I think we could move most of the contents of https://github.com/eriksjolund/podman-caddy-socket-activation to an article, or to the wiki (FYI @eriksjolund)

@MayCXC
Copy link
Author

MayCXC commented Oct 11, 2024

an article works well, then I can link to that from the fd examples.

@eriksjolund
Copy link

I'll try to adjust the material into an article. I'll keep you updated.

@eriksjolund
Copy link

Some updates:
I wrote an introduction paragraph "Using Caddy with socket activation" (https://github.com/eriksjolund/podman-caddy-socket-activation/?tab=readme-ov-file#using-caddy-with-socket-activation)

It would be nice to have an example that is using a systemd system service (so that there is no need to lower the number /proc/sys/net/ipv4/ip_unprivileged_port_start). I started writing a PR for that

but it's currently work in progress. I am still learning about what is best practice. It looks like --userns=auto is the preferred way when using rootful Podman (see discussion containers/podman#13728).

Anyway, if there is a need to use another open source license for https://github.com/eriksjolund/podman-caddy-socket-activation I'm open for changing the license (or maybe just add a second license).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants