caddy icon indicating copy to clipboard operation
caddy copied to clipboard

Not possible to omit a {block} in a snippet import if it's declared in the snippet

Open dword-design opened this issue 10 months ago • 8 comments

If I have this snippet:

(foo) {
  reverse_proxy localhost:3000
  {block}
}

This works:

example.com {
  import foo {
    file_server
  }
}

But this doesn't:

example.com {
  import foo
}

So basically the block is always mandatory and it's not possible to not declare it.

(The example doesn't make any sense, it's basically to show the issue.)

dword-design avatar Feb 01 '25 02:02 dword-design

FYI @elee1766

francislavoie avatar Feb 01 '25 04:02 francislavoie

hmmm yes. I guess this is something I didnt think of.

so I guess In my head I wanted to make the block required since I didn't want my sre/developers to "forget" that they needed to supply a block, because it's not so clear if a snippet uses a block or not.(without looking at the snippet. which is something they usually wouldn't have written)

for now of course you should be able to put just an empty block like {}, and it will work I hope?

I think you're right that probably it should just silently substitute to nothing? I can see a use case where you want to provide optional options and therefore would want to skip the block.

elee1766 avatar Feb 01 '25 04:02 elee1766

#@elee1766 {} doesn't work but { log } does 😋.

I come from the Vue world and there are slots for components where you can render additional stuff and when the user doesn't implement them, they won't be shown.

Apart from that it's a great project and I'm happy now to have a server now that I can maintain as a frontend dev without all the admin hassle.

dword-design avatar Feb 01 '25 14:02 dword-design

I hadn't seen snippet blocks until looking through the docs recently and agree it'd be helpful to have optional blocks. Forcing an empty block instead of allowing complete omission sounds reasonable to me. I took a quick look and wasn't sure how exactly to handle the case where a snippet had {block} before this feature since that's the case which breaks having an optional block. It seems like there'd need to be a token which explicitly means to skip to avoid a breaking change.

https://github.com/caddyserver/caddy/blob/092913a7a568a5eb4b28c06e12c1274bd5eb1140/caddyconfig/caddyfile/parse.go#L557-L560

mpmckinney avatar Jun 07 '25 05:06 mpmckinney

Do you have an example of how {block}/{blocks.*} would have been used before this feature? Removing the above case would allow for optional snippet blocks where {block} will be ignored when not passed in.

Something like { log } will only work in certain contexts depending on where the snippet is used and doesn't seem to be following the current definition of a block requiring newlines.

mpmckinney avatar Jun 11 '25 02:06 mpmckinney

Do you have an example of how {block}/{blocks.*} would have been used before this feature?

{block} didn't exist before this so I'm not sure what you're asking for.

Something like { log } will only work in certain contexts

That should never work, it's a violation of the block syntax https://caddyserver.com/docs/caddyfile/concepts#blocks ({ must be followed by a newline). If it happens to work somewhere you can find, it should be rejected by the parser.

francislavoie avatar Jun 11 '25 02:06 francislavoie

{block} didn't exist before this so I'm not sure what you're asking for.

The existing comment implies it was possible but I wasn't sure how (if at all).

 	// this allows snippets which contained {block}/{block.*} before this change to continue functioning as normal 

That should never work, it's a violation of the block syntax https://caddyserver.com/docs/caddyfile/concepts#blocks ({ must be followed by a newline). If it happens to work somewhere you can find, it should be rejected by the parser.

It appears that it's currently only a warning from the parser.

mpmckinney avatar Jun 11 '25 03:06 mpmckinney

The existing comment implies it was possible but I wasn't sure how (if at all).

I think they were just trying to cover their bases in case someone happened to have that exact text in their config, although extremely unlikely.

francislavoie avatar Jun 11 '25 03:06 francislavoie