Records for configuring Remote

[dag]

TLDR: Should I use separate types for client and server configuration, or unify the configuration in a shared type?

I'm working on replacing the arguments to acidServer and openRemoteState with data records instead, as this will allow us to provide default configurations and the ability to add new options without necessarily breaking existing code.

However, I'm stuck at a design decision: should there be a single unified record type that's used both for servers and clients, or should there be separate record types?

Single type:

• More convenient in simple cases: we only need one nullConf-like function, we only need one sharedSecretConf function, and the user only needs to make one configuration instance that they can use both for starting their server and for connecting to it.
• The function pairs for authentication are unified in a single configuration, reflecting the fact that they should be used together.
• More complex setups are still possible.

Separate types:

• Added type safety makes it impossible to set up a configuration intended for clients that simply skips server authentication, and then accidentally using it for configuring the server and ending up running it unprotected. This problem can be avoided with a single type too as long as the user uses the provided functions for generating configurations rather than using the record constructors directly.

• Even though the function pairs for authentication should be used together, their configuration can in fact differ.

  When using shared secret authentication, the server can be configured to accept a set of secrets, but the client only needs one of them to authenticate. Thus with a unified configuration, we end up having to either provide two functions for generating configuration for shared secret authentication anyway, or a single but weird function Set Secret -> Secret -> Config. It is weird because you can now make a shared configuration with server-side secrets that differ from the one secret the client offers.

  In the case where you only use one secret on the server, a unified configuration would make more sense because you could just provide a single function that simply takes the single secret as an argument.

In my own usage so far, I just use the remote backend to share a state between processes on the same system and as the same user, and as a way to inspect the state of a running server. In this type of use, a single configuration is more convenient and even makes more sense: things like the port number is kept in one place.

However, the fact that sharedSecretCheck takes a set of secrets suggests that acid-state wants to support more complex setups where you have multiple clients connecting from multiple systems. In this type of use, I think separate configurations better reflect reality, at the expense of complicating the simpler cases a bit.

I'm probably over-thinking this but I'm not confident to make design decisions like this for a project that isn't mine. I'm experiencing some cognitive dissonance because I originally wanted to do this in part to simplify the user facing API, but at the same time I think separate types are probably the more "correct" option.

What sayeth you?

CC @stepcut

[nikita-volkov]

TLDR completely, as suggested )
I actually have something "in the spirit" already implemented in private projects. Haven't worked on integrating it into the acid-state yet though. The configurations are separated for client and server, the code explains why. Besides solving the configuration issues it also abstracts over connecting to local or remote server or running the server itself with openExisting.
@lemmih what do you think about that?

startServer 
  :: IsAcidic a
  => Either (Int, Set ByteString) FilePath 
  -- ^ Port and secrets or a socket file
  -> AcidState a
  -- ^ State
  -> IO ()
startServer options acid = case options of
  Left (port, secrets) -> do
    Acid.acidServer 
      (Acid.sharedSecretCheck secrets) 
      (Network.PortNumber $ fromIntegral port) 
      acid
  Right socket -> do
    Acid.acidServer 
      Acid.skipAuthenticationCheck
      (Network.UnixSocket $ FilePath.encodeString socket) 
      acid



data URL 
  = Socket FilePath
  -- ^ A path to a socket file
  | Host Text Int (Maybe ByteString) 
  -- ^ Host, port and secret
  | Path FilePath
  -- ^ A path to local state
  | Name Text 
  -- ^ A name of state located under standard local directory
  deriving (Eq, Ord, Show)

-- | Open an existing state by "URL".
-- TODO: handle connection failure and probably return 'Either'.
openExisting :: IsAcidic a => URL -> IO (AcidState a)
openExisting url = case url of
  Host host port (Just secret) -> 
    Acid.openRemoteState 
      (Acid.sharedSecretPerform secret) 
      (Text.unpack host)
      (Network.PortNumber $ fromIntegral port)
  Host host port _ -> 
    Acid.openRemoteState 
      Acid.skipAuthenticationPerform 
      (Text.unpack host)
      (Network.PortNumber $ fromIntegral port)
  Socket path -> do
    path' <- FilePath.resolve path
    Acid.openRemoteState
      Acid.skipAuthenticationPerform 
      ""
      (Network.UnixSocket $ FilePath.encodeString path')
  Path path -> do
    path' <- FilePath.resolve path
    Acid.openLocalStateFrom (FilePath.encodeString $ path') $
      error $ "No initialized local state found at following path: " <> FilePath.encodeString path
  Name name -> do
    home <- FilePath.getHomeDirectory
    let path = home <> ".acid-databases" <> FilePath.fromText name
    openLocalStateFrom (FilePath.encodeString path) $
      error $ "No initialized local state found under following name: " <> Text.unpack name

[lemmih]

I defer all judgement regarding the remote backend to @stepcut.

[dag]

@nikita-volkov Your ideas inspired me to rethink some things, so I think I have an API I'm satisfied with now.

[stepcut]

However, the fact that sharedSecretCheck takes a set of secrets suggests that acid-state wants to support more complex setups where you have multiple clients connecting from multiple systems. In this type of use, I think separate configurations better reflect reality, at the expense of complicating the simpler cases a bit.

Correct -- it is setup so that there could be multiple clients and then you could revoke access to one of them. It is also setup so that other authentication schemes could be used besides shared secrets.

Although not currently supported, it could also be nice if there were two permission levels -- read vs read/write. Or perhaps a scheme where each client has a whitelist of which specific methods they can invoke.