kvass icon indicating copy to clipboard operation
kvass copied to clipboard

make remote host a real url

Open CosmicToast opened this issue 2 years ago • 4 comments

this means you can do things like prefix-based reverse-proxying and reverse-proxying in general, actually it also knows about proxying to subfolders and thus uses a url-base, meaning you can do something like kvass config remote https://my.normal.website/secret/kvass/

example setup with caddy server: kvass --bind=localhost:8081 (you can also bind to 0.0.0.0 if the reverse proxy is off-system) proxy:

my.domain {
	// ... the rest of the domain
	handle_path /secret/prefix/kvass/* {
		reverse_proxy http://localhost:8081 # match to above
	}
}

clients: kvass config key ... && kvass config remote https://my.domain/secret/prefix/kvass/

Note that the trailing / is important.

CosmicToast avatar Nov 27 '23 19:11 CosmicToast

Hi Cosmic!

Thanks for your work, making kvass work properly with reverse proxies was on the todo list for (too) long and I'm happy you made it happen :)

I'm too tired for a proper review today, but will look at it tomorrow. Some quick questions in the meantime:

  1. How does the migration look for existing users? I thinks it's important that current users can upgrade without having to change the config. I'd image NewSqlitePersistance() checks/updates the SchemaVersion field and take care of it.
  2. There are some places with // this should never fail comments. Could you add if err != nil {panic(err)} checks around them to be sure? Feel free to pack them into a convenience function if you find the explicit version to be ugly to read.
  3. Are there cases, where we would not want a trailing /? If not, lets add a warning or even an automatic conversion so users avoid the foot gun.

maxmunzel avatar Nov 27 '23 20:11 maxmunzel

re: migrations, I don't typically work with/on databases (I put points into shell instead), so I'm not entirely sure what that would look like the rest is all done, as well as slightly improved URL parsing

CosmicToast avatar Nov 28 '23 09:11 CosmicToast

Thanks for the quick adjustments. The migration would actually have little db logic in it and would be at the end of NewSqlitePersistance(). Basically:

  1. Check if state.SchemaVersion == 1
  2. do some magic to make a RemoteURL from a RemoteHostname
  3. increment state.SchemaVersion
  4. write the new state struct to the db

There's already some code for the last schema migration, but this one is even simpler. If you want, you can just give me a snippet for step 2 and I'll do the reset.

maxmunzel avatar Nov 28 '23 10:11 maxmunzel