From 93f8f6b55cf3270a65f58094ffd8a0bcc080bac2 Mon Sep 17 00:00:00 2001 From: Devin Carr Date: Mon, 6 Mar 2023 15:19:10 -0800 Subject: [PATCH] TUN-7245: Add bastion flag to origin service check --- cmd/cloudflared/tunnel/cmd.go | 7 +++-- cmd/cloudflared/tunnel/configuration.go | 19 +++++++------- component-tests/config.py | 33 +++++------------------- component-tests/conftest.py | 10 +++----- component-tests/constants.py | 2 +- component-tests/test_quicktunnels.py | 34 +++++++++++++++++++++++++ component-tests/util.py | 15 +++++++++-- ingress/ingress.go | 4 +-- ingress/origin_service.go | 1 + 9 files changed, 75 insertions(+), 50 deletions(-) create mode 100644 component-tests/test_quicktunnels.py diff --git a/cmd/cloudflared/tunnel/cmd.go b/cmd/cloudflared/tunnel/cmd.go index 4edc0690..e9c372f5 100644 --- a/cmd/cloudflared/tunnel/cmd.go +++ b/cmd/cloudflared/tunnel/cmd.go @@ -199,7 +199,7 @@ func TunnelCommand(c *cli.Context) error { // Run a quick tunnel // A unauthenticated named tunnel hosted on ..com // We don't support running proxy-dns and a quick tunnel at the same time as the same process - shouldRunQuickTunnel := c.IsSet("url") || c.IsSet("hello-world") + shouldRunQuickTunnel := c.IsSet("url") || c.IsSet(ingress.HelloWorldFlag) if !c.IsSet("proxy-dns") && c.String("quick-service") != "" && shouldRunQuickTunnel { return RunQuickTunnel(sc) } @@ -215,6 +215,9 @@ func TunnelCommand(c *cli.Context) error { } if c.IsSet("proxy-dns") { + if shouldRunQuickTunnel { + return fmt.Errorf("running a quick tunnel with `proxy-dns` is not supported") + } // NamedTunnelProperties are nil since proxy dns server does not need it. // This is supported for legacy reasons: dns proxy server is not a tunnel and ideally should // not run as part of cloudflared tunnel. @@ -786,7 +789,7 @@ func configureProxyFlags(shouldHide bool) []cli.Flag { Hidden: shouldHide, }), altsrc.NewBoolFlag(&cli.BoolFlag{ - Name: "hello-world", + Name: ingress.HelloWorldFlag, Value: false, Usage: "Run Hello World Server", EnvVars: []string{"TUNNEL_HELLO_WORLD"}, diff --git a/cmd/cloudflared/tunnel/configuration.go b/cmd/cloudflared/tunnel/configuration.go index dec85fc5..7f1488ab 100644 --- a/cmd/cloudflared/tunnel/configuration.go +++ b/cmd/cloudflared/tunnel/configuration.go @@ -124,7 +124,7 @@ func isSecretEnvVar(key string) bool { func dnsProxyStandAlone(c *cli.Context, namedTunnel *connection.NamedTunnelProperties) bool { return c.IsSet("proxy-dns") && !(c.IsSet("name") || // adhoc-named tunnel - c.IsSet("hello-world") || // quick or named tunnel + c.IsSet(ingress.HelloWorldFlag) || // quick or named tunnel namedTunnel != nil) // named tunnel } @@ -231,17 +231,16 @@ func prepareTunnelConfig( if err != nil && err != ingress.ErrNoIngressRules { return nil, nil, err } - if c.IsSet("url") { - // Ingress rules cannot be provided with --url flag + if c.IsSet("url") || c.IsSet(ingress.HelloWorldFlag) || c.IsSet(config.BastionFlag) { + // Ingress rules cannot be provided with --url, --hello-world or --bastion flag if !ingressRules.IsEmpty() { return nil, nil, ingress.ErrURLIncompatibleWithIngress - } else { - // Only for quick or adhoc tunnels will we attempt to parse: - // --url, --hello-world, or --unix-socket flag for a tunnel ingress rule - ingressRules, err = ingress.NewSingleOrigin(c, false) - if err != nil { - return nil, nil, err - } + } + // Only for quick or adhoc tunnels will we attempt to parse: + // --url, --hello-world, --bastion, or --unix-socket flag for a tunnel ingress rule + ingressRules, err = ingress.NewSingleOrigin(c, false) + if err != nil { + return nil, nil, err } } diff --git a/component-tests/config.py b/component-tests/config.py index dd549081..fc48623f 100644 --- a/component-tests/config.py +++ b/component-tests/config.py @@ -41,8 +41,10 @@ class NamedTunnelBaseConfig(BaseConfig): def merge_config(self, additional): config = super(NamedTunnelBaseConfig, self).merge_config(additional) - config['tunnel'] = self.tunnel - config['credentials-file'] = self.credentials_file + if 'tunnel' not in config: + config['tunnel'] = self.tunnel + if 'credentials-file' not in config: + config['credentials-file'] = self.credentials_file # In some cases we want to override default ingress, such as in config tests if 'ingress' not in config: config['ingress'] = self.ingress @@ -84,28 +86,9 @@ class NamedTunnelConfig(NamedTunnelBaseConfig): def get_credentials_json(self): with open(self.credentials_file) as json_file: return json.load(json_file) - - + @dataclass(frozen=True) -class ClassicTunnelBaseConfig(BaseConfig): - hostname: str = None - origincert: str = None - - def __post_init__(self): - if self.hostname is None: - raise TypeError("Field tunnel is not set") - if self.origincert is None: - raise TypeError("Field credentials_file is not set") - - def merge_config(self, additional): - config = super(ClassicTunnelBaseConfig, self).merge_config(additional) - config['hostname'] = self.hostname - config['origincert'] = self.origincert - return config - - -@dataclass(frozen=True) -class ClassicTunnelConfig(ClassicTunnelBaseConfig): +class QuickTunnelConfig(BaseConfig): full_config: dict = None additional_config: InitVar[dict] = {} @@ -115,10 +98,6 @@ class ClassicTunnelConfig(ClassicTunnelBaseConfig): object.__setattr__(self, 'full_config', self.merge_config(additional_config)) - def get_url(self): - return "https://" + self.hostname - - @dataclass(frozen=True) class ProxyDnsConfig(BaseConfig): full_config = { diff --git a/component-tests/conftest.py b/component-tests/conftest.py index 709ab39a..3e6a7603 100644 --- a/component-tests/conftest.py +++ b/component-tests/conftest.py @@ -5,14 +5,14 @@ from time import sleep import pytest import yaml -from config import NamedTunnelConfig, ClassicTunnelConfig, ProxyDnsConfig +from config import NamedTunnelConfig, ProxyDnsConfig, QuickTunnelConfig from constants import BACKOFF_SECS, PROXY_DNS_PORT from util import LOGGER class CfdModes(Enum): NAMED = auto() - CLASSIC = auto() + QUICK = auto() PROXY_DNS = auto() @@ -42,12 +42,10 @@ def component_tests_config(): tunnel=config['tunnel'], credentials_file=config['credentials_file'], ingress=config['ingress']) - elif cfd_mode is CfdModes.CLASSIC: - return ClassicTunnelConfig( - additional_config=additional_config, cloudflared_binary=config['cloudflared_binary'], - hostname=config['classic_hostname'], origincert=config['origincert']) elif cfd_mode is CfdModes.PROXY_DNS: return ProxyDnsConfig(cloudflared_binary=config['cloudflared_binary']) + elif cfd_mode is CfdModes.QUICK: + return QuickTunnelConfig(additional_config=additional_config, cloudflared_binary=config['cloudflared_binary']) else: raise Exception(f"Unknown cloudflared mode {cfd_mode}") diff --git a/component-tests/constants.py b/component-tests/constants.py index e46bac31..2d0dc405 100644 --- a/component-tests/constants.py +++ b/component-tests/constants.py @@ -7,4 +7,4 @@ PROXY_DNS_PORT = 9053 def protocols(): - return ["h2mux", "http2", "quic"] + return ["http2", "quic"] diff --git a/component-tests/test_quicktunnels.py b/component-tests/test_quicktunnels.py new file mode 100644 index 00000000..f6d32259 --- /dev/null +++ b/component-tests/test_quicktunnels.py @@ -0,0 +1,34 @@ +#!/usr/bin/env python +import requests +from conftest import CfdModes +from constants import METRICS_PORT +from util import LOGGER, start_cloudflared, wait_tunnel_ready, get_quicktunnel_url, send_requests + +class TestCLI: + def test_quick_tunnel(self, tmp_path, component_tests_config): + config = component_tests_config(cfd_mode=CfdModes.QUICK, run_proxy_dns=False) + LOGGER.debug(config) + with start_cloudflared(tmp_path, config, cfd_args=["--hello-world"], new_process=True): + wait_tunnel_ready(require_min_connections=4) + url = get_quicktunnel_url() + send_requests(url, 3, True) + + def test_quick_tunnel_url(self, tmp_path, component_tests_config): + config = component_tests_config(cfd_mode=CfdModes.QUICK, run_proxy_dns=False) + LOGGER.debug(config) + with start_cloudflared(tmp_path, config, cfd_args=["--url", f"http://localhost:{METRICS_PORT}/"], new_process=True): + wait_tunnel_ready() + url = get_quicktunnel_url() + send_requests(url+"/ready", 3, True) + + def test_quick_tunnel_proxy_dns_url(self, tmp_path, component_tests_config): + config = component_tests_config(cfd_mode=CfdModes.QUICK, run_proxy_dns=True) + LOGGER.debug(config) + failed_start = start_cloudflared(tmp_path, config, cfd_args=["--url", f"http://localhost:{METRICS_PORT}/"], expect_success=False) + assert failed_start.returncode == 1, "Expected cloudflared to fail to run with `proxy-dns` and `hello-world`" + + def test_quick_tunnel_proxy_dns_hello_world(self, tmp_path, component_tests_config): + config = component_tests_config(cfd_mode=CfdModes.QUICK, run_proxy_dns=True) + LOGGER.debug(config) + failed_start = start_cloudflared(tmp_path, config, cfd_args=["--hello-world"], expect_success=False) + assert failed_start.returncode == 1, "Expected cloudflared to fail to run with `proxy-dns` and `url`" diff --git a/component-tests/util.py b/component-tests/util.py index 02a9f1fd..dce25681 100644 --- a/component-tests/util.py +++ b/component-tests/util.py @@ -35,7 +35,7 @@ def write_config(directory, config): def start_cloudflared(directory, config, cfd_args=["run"], cfd_pre_args=["tunnel"], new_process=False, - allow_input=False, capture_output=True, root=False, skip_config_flag=False): + allow_input=False, capture_output=True, root=False, skip_config_flag=False, expect_success=True): config_path = None if not skip_config_flag: @@ -46,7 +46,7 @@ def start_cloudflared(directory, config, cfd_args=["run"], cfd_pre_args=["tunnel if new_process: return run_cloudflared_background(cmd, allow_input, capture_output) # By setting check=True, it will raise an exception if the process exits with non-zero exit code - return subprocess.run(cmd, check=True, capture_output=capture_output) + return subprocess.run(cmd, check=expect_success, capture_output=capture_output) def cloudflared_cmd(config, config_path, cfd_args, cfd_pre_args, root): @@ -77,7 +77,18 @@ def run_cloudflared_background(cmd, allow_input, capture_output): cfd.terminate() if capture_output: LOGGER.info(f"cloudflared log: {cfd.stderr.read()}") + +def get_quicktunnel_url(): + quicktunnel_url = f'http://localhost:{METRICS_PORT}/quicktunnel' + with requests.Session() as s: + resp = send_request(s, quicktunnel_url, True) + + hostname = resp.json()["hostname"] + assert hostname, \ + f"Quicktunnel endpoint returned {hostname} but we expected a url" + + return f"https://{hostname}" def wait_tunnel_ready(tunnel_url=None, require_min_connections=1, cfd_logs=None): try: diff --git a/ingress/ingress.go b/ingress/ingress.go index 904d2887..ccee3978 100644 --- a/ingress/ingress.go +++ b/ingress/ingress.go @@ -112,7 +112,7 @@ func NewWarpRoutingService(config WarpRoutingConfig) *WarpRoutingService { // Get a single origin service from the CLI/config. func parseSingleOriginService(c *cli.Context, allowURLFromArgs bool) (OriginService, error) { - if c.IsSet("hello-world") { + if c.IsSet(HelloWorldFlag) { return new(helloWorld), nil } if c.IsSet(config.BastionFlag) { @@ -206,7 +206,7 @@ func validateIngress(ingress []config.UnvalidatedIngressRule, defaults OriginReq } srv := newStatusCode(statusCode) service = &srv - } else if r.Service == HelloWorldService || r.Service == "hello-world" || r.Service == "helloworld" { + } else if r.Service == HelloWorldFlag || r.Service == HelloWorldService { service = new(helloWorld) } else if r.Service == ServiceSocksProxy { rules := make([]ipaccess.Rule, len(r.OriginRequest.IPRules)) diff --git a/ingress/origin_service.go b/ingress/origin_service.go index 85fd29cd..7fc8201e 100644 --- a/ingress/origin_service.go +++ b/ingress/origin_service.go @@ -23,6 +23,7 @@ import ( const ( HelloWorldService = "hello_world" + HelloWorldFlag = "hello-world" HttpStatusService = "http_status" )