filesystem: Globally declared filesystems, fs directive

[elee1766]

attempting to address #5057

the current implementation creates a new app with module name "caddy.filesystems". this module is populated with global options

this app is configured by a new filesystem options directive. use multiple filesystem times to define multiple filesystems to add to the map

new route directive fs to declare a fs for the http route context, similar to root. this is shared between try_routes, file_server, and anything else that uses MatchRoutes module

example caddyfile with an fs impl named swim, which uses a remotely downloaded archive as fs, and routes like an SPA

{
  filesystem app swim "https://remote.com/artifact" /dist .zip
}
fs app
try_files {path} /
file_server

i tried to add as little new things as possible, reusing the existing app framework and options framework. im not sure if they are the correct tools for the job, but they seem okay.

design choices that i dont really know how you guys feel about:

1. the filesystem map is pretty "magical", and so i decided to hide it in internal. I think it's better if people aren't allowed to touch it.

2. the filesystem map is thread safe and modifyable at runtime. I dont know if this is a part of the design contract that you guys want to enforce, maybe its better to keep it not thread safe.

3. I couldn't find an easier way besides adding something to the "Context" which allowed me to share runtime objects across apps. is there some framework i am missing? I'm not really sure how this will scale

[CLAassistant]

All committers have signed the CLA.

[elee1766]

adding some context, i originally create a project called swim, which was for hosting spas from ci artifacts.

the project grew, and we started using it for things like prerender redirection, cache configuration, etc. i decided it was better to add the remote archive loading feature to caddy as a plugin, since i remembered caddy existed and it was written in go.

here is the implementation , it uses afero, and can currently load directories, tarballs, and compressed tarballs, from both local file and remote http/https resources.

https://gfx.cafe/open/swim/-/blob/caddy/src/swim/handler.go?ref_type=heads

if you like the general idea, i'm happy to also PR the local+remote archive loader as a module. the only dependency required for it is afero. otherwise i will just continue maintaining it out of tree.

slightly related, i've already implemented s3 as an afero compatible store here https://gfx.cafe/open/s3fs, but i haven't had the need to write the caddy config for it.

[elee1766]

@mholt @francislavoie please let me know how this looks / if i have sinned anywhere.

[mholt]

So sorry for the wait, it's been a busy quarter! I will return to this ASAP. Just letting you know I haven't forgotten about it.

[elee1766]

So sorry for the wait, it's been a busy quarter! I will return to this ASAP. Just letting you know I haven't forgotten about it.

no problem, i've been busy too.

i've actually been thinking about this a bit in my spare time - i added the fs to the global app because that's how certificates were done and certificates were in my mind a similar problem (sharing assets across many sub apps). however, thinking more, i think it might make more sense to perhaps store them as any other module? (and maybe perhaps make certs function in that way as well?)

on a completely unrelated note - working with caddy made me really enjoy the format. we've done something similar to coredns, except for postgres pooling/lb/routing instead of dns. it's here https://gfx.cafe/gfx/pggat

[francislavoie]

This looks awesome ☺️

[francislavoie]

How does this behave with config reloads? Is ctx scoped to only the current config (and not global)? Just making sure that it won't unregister an fs that didn't change keys between config reloads.

[elee1766]

so I assumed caddy would run cleanup before the config reload and provision after, is that not what happens for config reloads?

I also somewhat assumed that global modules (for instance like the bind address/port) would be global namespaced, is that a bad assumption? the idea is a config reload would unload and then reload every fs declared in the global config.

[francislavoie]

See https://caddyserver.com/docs/architecture for an explanation of the lifecycle.

Basically, when reloading there's two configs loaded and started for a moment. The new config is loaded and started before the old config is stopped. Cleanup gets called during stoppage.

I think what you did might be fine, I just wanted to make sure you tested config reloads and confirmed things still work normally after a reload.

[elee1766]

i haven't so i will, and i will test a few cases as well.

is there a way to programmatically test this with some sort of harness as well?

[francislavoie]

I think technically this is a breaking change, right? Because we now need to define an fs name instead of an fs module? I think this is ok because it's an improvement, we just need to document it in the release notes.

[elee1766]

yes it is. I think this makes a lot more sense.

I tried playing around to declare filesystems inline but it was a nightmare so I gave up...

my primary use case anyways is to route multiple SPA's loaded from archives, oci artifacts, or s3 object stores. we download spas from our gitlab artifact storage and serve them for branch and development deployments

the second use case would be to mount existing apis implemented through filesystems, for instance, the goproxy protocol.

so loading once at the start (do all the funky networking) and reusing for both cases made the most sense.

[francislavoie]

This'll need a rebase at this point.

For some context, we just merged caddyserver/xcaddy#160 this week which will allow xcaddy to wire up a filesystem with files embedded in the binary a build-time. We were talking about what else Caddy would need to make config work well, but I totally forgot we had this PR pending 🤦‍♂️

Looking to get this merged soon! It'll pair very well with embedding with xcaddy.

[elee1766]

This'll need a rebase at this point.

For some context, we just merged caddyserver/xcaddy#160 this week which will allow xcaddy to wire up a filesystem with files embedded in the binary a build-time. We were talking about what else Caddy would need to make config work well, but I totally forgot we had this PR pending 🤦‍♂️

Looking to get this merged soon! It'll pair very well with embedding with xcaddy.

ah i see.

would you like me to convert the embed filesystem to this format as well ?

[francislavoie]

I don't think we need to do anything, embed is a caddy.fs module so it'll still be compatible

[elee1766]

@francislavoie i did some testing and it seems to function correctly on config reload, when keys are added, removed, or unchanged.

[elee1766]

ran fumpt and updated master

[mholt]

Thanks for maintaining this -- I'm working through a holiday backlog and will get around to this soon :)

[elee1766]

https://github.com/gfx-labs/swim/tree/master

example caddyfile with a functional dockerfile is here. (we will likely try using this image at work in our ci pipelines for testing soon)

[francislavoie]

Error: modules/caddyhttp/fileserver/matcher_test.go:119:4: unknown field fileSystem in struct literal of type MatchFile
Error: modules/caddyhttp/fileserver/matcher_test.go:119:16: undefined: osFS
Error: modules/caddyhttp/fileserver/matcher_test.go:228:4: unknown field fileSystem in struct literal of type MatchFile
Error: modules/caddyhttp/fileserver/matcher_test.go:228:16: undefined: osFS

[elee1766]

Error: modules/caddyhttp/fileserver/matcher_test.go:119:4: unknown field fileSystem in struct literal of type MatchFile
Error: modules/caddyhttp/fileserver/matcher_test.go:119:16: undefined: osFS
Error: modules/caddyhttp/fileserver/matcher_test.go:228:4: unknown field fileSystem in struct literal of type MatchFile
Error: modules/caddyhttp/fileserver/matcher_test.go:228:16: undefined: osFS

fixed, but i had to add a nil check for filesystems here so that the CEL matcher would pass

// Filesystems returns a ref to the FilesystemMap
func (ctx *Context) Filesystems() FileSystems {
	// if no config is loaded, we use a default filesystemmap, which includes the osfs
	if ctx.cfg == nil {
		return &filesystems.FilesystemMap{}
	}
	return ctx.cfg.filesystems
}

i feel this is bad though? is this safe for runtime?

i saw in AppIfConfigured the following comment, but i dont really understand what can cause this to happen, or if it is okay.

if ctx.cfg == nil {
	// this can happen if the currently-active context
	// is being accessed, but no config has successfully
	// been loaded yet
        return nil
}

[francislavoie]

I think the ctx.cfg == nil happens in the tests because we're trying to unit test the matcher outside of a real Caddy config context. That fix is probably fine, it should only happen in tests.

Also, looks like gci needs to be run again in context.go

[elee1766]

sounds good, and done.

[francislavoie]

This looks really good to me! Excited to get it merged.