Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new/update flags on skywire-cli #1052

Merged
merged 13 commits into from
Jan 25, 2022
Merged

new/update flags on skywire-cli #1052

merged 13 commits into from
Jan 25, 2022

Conversation

mrpalide
Copy link
Contributor

@mrpalide mrpalide commented Jan 12, 2022

Did you run make format && make check?

Fixes #1046 #1050 #1054 #1061

Changes:

  • added --public-rpc flag to change CLIAddr from localhost:3435 to :3435.
  • added comparing step on adding hypervisors keys on --hypervisor-pks <visor-pk> flag, that if key equal to visor PK, then change this visor to hypervisor.
  • added --vpn-server-enable flag to make vpn-server app autostart attribute true
  • improve --package flag to distinguished between hypervisor and visor, and change its config output file name
  • add a checking --output or -o flag, and if set was used then override -p and -s flag

How to test this PR:

  • build at this PR by make build
  • generate config with ./skywire-cli config gen --public-rpc
  • check for CLIAddr value, should be :3435
  • add visor pk as hypervisor pks by skywire-cli config gen --hypervisor-pks <visor-pk> -ro skywire-config.json
  • check config file that hypervisors should be empty [], and hypervisor config part should added to config file.
  • generate/update config with ./skywire-cli config gen -r --vpn-server-enable
  • check config file to confirm that vpn-server autostart is true
  • generate config twice with -pi and -p to check their different name
  • generate config with -pir or -sr with -o config.json flag to see override functionality

@0pcom
Copy link
Collaborator

0pcom commented Jan 13, 2022

cli_addr should actually be ":3545"

I think it's equivalent to 0.0.0.0 for the ip address to be blank but the port is listed

@0pcom
Copy link
Collaborator

0pcom commented Jan 15, 2022

We need consistency in the default behavior for some things.

Is enable-auth to be considered default? If so, can you make sure it is evenly implemented, it was changed to not enabled by default in the -p flag but the default when you give no arguments is enabled

and can you put a flag for the config generation to --disable-auth also, if we go with the aforementioned default?

@mrpalide
Copy link
Contributor Author

mrpalide commented Jan 20, 2022

As had conversation with Moses on matrix, we want these changes:

  • disable OS detection in runtime, and set flag instead
    • as default set to linux, and can set by --os windows or --os macos flag.
  • disable auth in hypervisor as default, and be enabled by -p flag, or set OS to windows or macos
  • set enable-auth and disable-auth flag separately
  • remove -s flag completely
  • all apps be available as default, and remove each by flags
    • you should separate apps by ,, same as --disable-apps skysocks,vpn-server

Copy link
Member

@jdknives jdknives left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks alright. Only need to double check we are not using the deprecated flag in Skybian still.

@jdknives jdknives merged commit c888b87 into skycoin:develop Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants