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

feat: emit explicit errors for the service command on unsupported OSes #1291

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

micah-yeager
Copy link

Per the contribution guidelines, this seemed to me like a small enough change to not warrant an issue before creating this pull request. Let me know if you'd like me to create one anyway.

Background

While working with cloudflared on FreeBSD recently, I noticed that there's an inconsistency with the available CLI commands on that OS versus others — namely that the service command doesn't exist at all for operating systems other than Linux, macOS, and Windows.

Contrast cloudflared --help output on macOS versus FreeBSD (truncated to focus on the COMMANDS section):

  • Current help output on macOS:

    COMMANDS:
       update     Update the agent if a new version exists
       version    Print the version
       proxy-dns  Run a DNS over HTTPS proxy server.
       tail       Stream logs from a remote cloudflared
       service    Manages the cloudflared launch agent
       help, h    Shows a list of commands or help for one command
       Access:
         access, forward  access <subcommand>
       Tunnel:
         tunnel  Use Cloudflare Tunnel to expose private services to the Internet or to   Cloudflare connected private users.
    
  • Current help output on FreeBSD:

    COMMANDS:
       update     Update the agent if a new version exists
       version    Print the version
       proxy-dns  Run a DNS over HTTPS proxy server.
       tail       Stream logs from a remote cloudflared
       help, h    Shows a list of commands or help for one command
       Access:
         access, forward  access <subcommand>
       Tunnel:
         tunnel  Use Cloudflare Tunnel to expose private services to the Internet or to   Cloudflare connected private users.
    

This omission has caused confusion for users (including me), especially since the provided command in the Cloudflare Zero Trust dashboard returns a seemingly-unrelated error message:

$ sudo cloudflared service install ...
You did not specify any valid additional argument to the cloudflared tunnel command.

If you are trying to run a Quick Tunnel then you need to explicitly pass the --url flag.
Eg. cloudflared tunnel --url localhost:8080/.

Please note that Quick Tunnels are meant to be ephemeral and should only be used for testing purposes.
For production usage, we recommend creating Named Tunnels. (https://developers.cloudflare.com/cloudflare-one/connections/connect-apps/install-and-setup/tunnel-guide/)

Contribution

This pull request adds a "stub" service command (including the usual subcommands available on other OSes) to explicitly declare it as unsupported on the operating system.

New help output on FreeBSD (and other operating systems where service management is unsupported):

COMMANDS:
   update     Update the agent if a new version exists
   version    Print the version
   proxy-dns  Run a DNS over HTTPS proxy server.
   tail       Stream logs from a remote cloudflared
   service    Manages the cloudflared system service (not supported on this operating system)
   help, h    Shows a list of commands or help for one command
   Access:
     access, forward  access <subcommand>
   Tunnel:
     tunnel  Use Cloudflare Tunnel to expose private services to the Internet or to   Cloudflare connected private users.

New outputs when running the service management subcommands:

$ sudo cloudflared service install ...
service installation is not supported on this operating system
$ sudo cloudflared service uninstall ...
service uninstallation is not supported on this operating system

This keeps the available commands consistent until proper service management support can be added for these otherwise-supported operating systems.

I'm more than happy to make any adjustments to the pull request — just let me know!

@chungthuang
Copy link
Contributor

Hi @micah-yeager, thank you for submitting the fix. Can you rebase on the latest master and we are good to merge?

@micah-yeager
Copy link
Author

micah-yeager commented Aug 6, 2024

@chungthuang I used GitHub's "sync fork" feature, but it looks like that did a merge instead of a rebase. Should I redo it, or is that acceptable? Never mind, went ahead and fixed by rebasing instead.

@micah-yeager
Copy link
Author

Rebased onto latest.

@micah-yeager
Copy link
Author

Rebased onto latest.

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.

4 participants