skywire icon indicating copy to clipboard operation
skywire copied to clipboard

Check usage of omitted flags in certain conditions

Open ersonp opened this issue 3 years ago • 1 comments

Currently, we omit some flags when certain conditions are not met which causes some commands to throw flag not available error which is misleading and hard to debug.

As mentioned my @0pcom the flag should only be

  • hidden from help, not omitted if the config doesn't exist
  • generate a clear message or error if the flag is still invoked in the absence of the config

We will have to look at all the flags to ensure the same behavior everywhere.

ersonp avatar Sep 01 '22 12:09 ersonp

These are the different scenarios and flags which are currently affected

Only available as root and if the config was generated with the -p flag (or the config exists at the default location that flag generates /opt/skywire/skywire.json)

skywire-visor -p

Root is needed to actually write the config to that default package config path with the skywire-cli config gen -p flag, but the flag is still available without root because the config file can be output to a non-root owned path or even to stdout.

Similarly; the -u flag should only be available as non-root and if the config was generated with the -u flag (or the config exists at the default location that flag generates ~/skywire-config.json)

skywire-visor -u

The appropriate user is needed to actually write the config to that default userspace config path with the skywire-cli config gen -u flag; the visor should be run not as root against that config (with the -u flag) or else the userspace will be polluted with root-owned files and directories. In that case, upon subsequent runs, the visor will fail to start because of insufficient permissions.

it may be possible to combine the -u and -p flags for the visor; and to use any config which exists in the appropriate path for the permissions granted to the visor on invocation. I personally prefer more explicit flags such as we already have. It should be easily possible to change the help text for the flags when they would have been hidden and to generate a better error when the flag is called without the needed conditions being met.

Furthermore, it may be possible to go ahead and generate the config if it does not exist when invoking these flags, but i would not update it and i would suggest the use of bitfield/script directly to generate the config n that instance.

I am reluctant for any config gen to happen from the visor itself, really.


Visor flags for the autopeering are only available if SKYBIAN=true

skywire-visor -ml

additionally, the default value of the -m flag will be true if AUTOPEER=1 in addition to SKYBIAN=true

There is a further lockout mechanism for the autopeering where the -m flag is ignored if local or remote hypervisors are present in the config.

Additional notes:

  • switching the default value of the -m flag on the AUTOPEER=1 env can be reverted as i was able to make the service work without that (rendering the -m flag itself at the end of ExecStart= from a drop in systemd service config file provided by skybian)

  • some elements of the HVUI are only available or displayed as well if SKYBIAN=true

  • The SKYBIAN env should eventually be changed from true to 1 for brevity and conformity with typical implementations seen in other scripts (i.e. makepkg)

Its possible there are more instances where flags are hidden based on certain conditions not being met, I will update this comment as i run across them.

0pcom avatar Sep 01 '22 15:09 0pcom

this was fixed at some point

$ skywire-cli config gen --all
Generate a config file

Usage:
  skywire-cli config gen [flags] 

Flags:
  -a, --url string           services conf
      --log-level string     level of logging in config (default "info")
  -b, --bestproto            best protocol (dmsg | direct) based on location
  -c, --noauth               disable authentication for hypervisor UI
  -d, --dmsghttp             use dmsg connection to skywire services
  -e, --auth                 enable auth on hypervisor UI
  -f, --force                remove pre-existing config
  -g, --disableapps string   comma separated list of apps to disable
  -i, --ishv                 local hypervisor configuration
  -j, --hvpks string         list of public keys to use as hypervisor
  -k, --os string            (linux / mac / win) paths (default "linux")
  -l, --publicip             allow display node ip in services
  -m, --example-apps         add example apps to the config
  -n, --stdout               write config to stdout
  -o, --out string           output config: skywire-config.json
  -p, --pkg                  use path for package: /opt/skywire
  -u, --user                 use paths for user space: /home/d0mo
  -q, --publicrpc            allow rpc requests from LAN
  -r, --regen                re-generate existing config & retain keys
  -s, --sk cipher.SecKey     a random key is generated if unspecified
 (default 0000000000000000000000000000000000000000000000000000000000000000)
  -t, --testenv              use test deployment conf.skywire.dev
  -v, --servevpn             enable vpn server
  -w, --hide                 dont print the config to the terminal
  -x, --retainhv             retain existing hypervisors with regen
  -y, --autoconn             disable autoconnect to public visors
  -z, --public               publicize visor in service discovery
      --version string       custom version testing override
      --binpath string       set bin_path

both --pkg and --usr flags are shown on the extended help with skywire-cli config gen --all

0pcom avatar May 10 '23 13:05 0pcom