From e426693330a89d98a586d17c1fc60b1acf0a05c2 Mon Sep 17 00:00:00 2001 From: Sudarsan Reddy Date: Wed, 19 Apr 2023 12:41:01 +0100 Subject: [PATCH] TUN-7361: Add a label to override hostname It might make sense for users to sometimes name their cloudflared connectors to make identification easier than relying on hostnames that TUN-7360 provides. This PR provides a new --label option to cloudflared tunnel that a user could provide to give custom names to their connectors. --- cmd/cloudflared/tunnel/cmd.go | 15 ++++++++++++++- component-tests/test_logging.py | 6 +++--- component-tests/test_pq.py | 4 ++-- component-tests/test_proxy_dns.py | 2 +- component-tests/test_quicktunnels.py | 6 +++--- component-tests/test_reconnect.py | 4 ++-- component-tests/test_termination.py | 6 +++--- component-tests/test_tunnel.py | 18 +++++++++--------- management/service.go | 23 ++++++++++++++++++++--- orchestration/orchestrator_test.go | 2 +- 10 files changed, 58 insertions(+), 28 deletions(-) diff --git a/cmd/cloudflared/tunnel/cmd.go b/cmd/cloudflared/tunnel/cmd.go index 3c7aafce..be11f24e 100644 --- a/cmd/cloudflared/tunnel/cmd.go +++ b/cmd/cloudflared/tunnel/cmd.go @@ -96,6 +96,7 @@ 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/) ` + connectorLabelFlag = "label" ) var ( @@ -407,7 +408,14 @@ func StartServer( } } - mgmt := management.New(c.String("management-hostname"), serviceIP, clientID, logger.ManagementLogger.Log, logger.ManagementLogger) + mgmt := management.New( + c.String("management-hostname"), + serviceIP, + clientID, + c.String(connectorLabelFlag), + logger.ManagementLogger.Log, + logger.ManagementLogger, + ) localRules = []ingress.Rule{ingress.NewManagementRule(mgmt)} } orchestrator, err := orchestration.NewOrchestrator(ctx, orchestratorConfig, tunnelConfig.Tags, localRules, tunnelConfig.Log) @@ -675,6 +683,11 @@ func tunnelFlags(shouldHide bool) []cli.Flag { Value: 4, Hidden: true, }), + altsrc.NewStringFlag(&cli.StringFlag{ + Name: connectorLabelFlag, + Usage: "Use this option to give a meaningful label to a specific connector. When a tunnel starts up, a connector id unique to the tunnel is generated. This is a uuid. To make it easier to identify a connector, we will use the hostname of the machine the tunnel is running on along with the connector ID. This option exists if one wants to have more control over what their individual connectors are called.", + Value: "", + }), altsrc.NewDurationFlag(&cli.DurationFlag{ Name: "grace-period", Usage: "When cloudflared receives SIGINT/SIGTERM it will stop accepting new requests, wait for in-progress requests to terminate, then shutdown. Waiting for in-progress requests will timeout after this grace period, or when a second SIGTERM/SIGINT is received.", diff --git a/component-tests/test_logging.py b/component-tests/test_logging.py index e9b0bfbd..91af2e58 100644 --- a/component-tests/test_logging.py +++ b/component-tests/test_logging.py @@ -74,7 +74,7 @@ def assert_log_to_dir(config, log_dir): class TestLogging: def test_logging_to_terminal(self, tmp_path, component_tests_config): config = component_tests_config() - with start_cloudflared(tmp_path, config, new_process=True) as cloudflared: + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], new_process=True) as cloudflared: wait_tunnel_ready(tunnel_url=config.get_url()) assert_log_to_terminal(cloudflared) @@ -85,7 +85,7 @@ class TestLogging: "logfile": str(log_file), } config = component_tests_config(extra_config) - with start_cloudflared(tmp_path, config, new_process=True, capture_output=False): + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], new_process=True, capture_output=False): wait_tunnel_ready(tunnel_url=config.get_url(), cfd_logs=str(log_file)) assert_log_in_file(log_file) assert_json_log(log_file) @@ -98,6 +98,6 @@ class TestLogging: "log-directory": str(log_dir), } config = component_tests_config(extra_config) - with start_cloudflared(tmp_path, config, new_process=True, capture_output=False): + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], new_process=True, capture_output=False): wait_tunnel_ready(tunnel_url=config.get_url(), cfd_logs=str(log_dir)) assert_log_to_dir(config, log_dir) diff --git a/component-tests/test_pq.py b/component-tests/test_pq.py index e897f293..a7b2ed50 100644 --- a/component-tests/test_pq.py +++ b/component-tests/test_pq.py @@ -12,6 +12,6 @@ class TestPostQuantum: def test_post_quantum(self, tmp_path, component_tests_config): config = component_tests_config(self._extra_config()) LOGGER.debug(config) - with start_cloudflared(tmp_path, config, cfd_args=["run", "--post-quantum"], new_process=True): + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], cfd_args=["run", "--post-quantum"], new_process=True): wait_tunnel_ready(tunnel_url=config.get_url(), - require_min_connections=4) + require_min_connections=1) diff --git a/component-tests/test_proxy_dns.py b/component-tests/test_proxy_dns.py index 7c805c0e..aecfa0ae 100644 --- a/component-tests/test_proxy_dns.py +++ b/component-tests/test_proxy_dns.py @@ -25,7 +25,7 @@ def run_test_scenario(tmp_path, component_tests_config, cfd_mode, run_proxy_dns) if cfd_mode == CfdModes.NAMED: expect_tunnel = True - pre_args = ["tunnel"] + pre_args = ["tunnel", "--ha-connections", "1"] args = ["run"] elif cfd_mode == CfdModes.PROXY_DNS: expect_proxy_dns = True diff --git a/component-tests/test_quicktunnels.py b/component-tests/test_quicktunnels.py index 17445a5b..fd2cf7aa 100644 --- a/component-tests/test_quicktunnels.py +++ b/component-tests/test_quicktunnels.py @@ -7,7 +7,7 @@ class TestQuickTunnels: 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): + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], cfd_args=["--hello-world"], new_process=True): wait_tunnel_ready(require_min_connections=1) url = get_quicktunnel_url() send_requests(url, 3, True) @@ -15,8 +15,8 @@ class TestQuickTunnels: 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() + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], cfd_args=["--url", f"http://localhost:{METRICS_PORT}/"], new_process=True): + wait_tunnel_ready(require_min_connections=1) url = get_quicktunnel_url() send_requests(url+"/ready", 3, True) diff --git a/component-tests/test_reconnect.py b/component-tests/test_reconnect.py index 811e99b7..ece68bdc 100644 --- a/component-tests/test_reconnect.py +++ b/component-tests/test_reconnect.py @@ -13,7 +13,7 @@ from util import start_cloudflared, wait_tunnel_ready, check_tunnel_not_connecte @flaky(max_runs=3, min_passes=1) class TestReconnect: - default_ha_conns = 4 + default_ha_conns = 1 default_reconnect_secs = 15 extra_config = { "stdin-control": True, @@ -29,7 +29,7 @@ class TestReconnect: @pytest.mark.parametrize("protocol", protocols()) def test_named_reconnect(self, tmp_path, component_tests_config, protocol): config = component_tests_config(self._extra_config(protocol)) - with start_cloudflared(tmp_path, config, new_process=True, allow_input=True, capture_output=False) as cloudflared: + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], new_process=True, allow_input=True, capture_output=False) as cloudflared: # Repeat the test multiple times because some issues only occur after multiple reconnects self.assert_reconnect(config, cloudflared, 5) diff --git a/component-tests/test_termination.py b/component-tests/test_termination.py index ef12edc8..26f4fea4 100644 --- a/component-tests/test_termination.py +++ b/component-tests/test_termination.py @@ -34,7 +34,7 @@ class TestTermination: def test_graceful_shutdown(self, tmp_path, component_tests_config, signal, protocol): config = component_tests_config(self._extra_config(protocol)) with start_cloudflared( - tmp_path, config, new_process=True, capture_output=False) as cloudflared: + tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], new_process=True, capture_output=False) as cloudflared: wait_tunnel_ready(tunnel_url=config.get_url()) connected = threading.Condition() @@ -56,7 +56,7 @@ class TestTermination: def test_shutdown_once_no_connection(self, tmp_path, component_tests_config, signal, protocol): config = component_tests_config(self._extra_config(protocol)) with start_cloudflared( - tmp_path, config, new_process=True, capture_output=False) as cloudflared: + tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], new_process=True, capture_output=False) as cloudflared: wait_tunnel_ready(tunnel_url=config.get_url()) connected = threading.Condition() @@ -76,7 +76,7 @@ class TestTermination: def test_no_connection_shutdown(self, tmp_path, component_tests_config, signal, protocol): config = component_tests_config(self._extra_config(protocol)) with start_cloudflared( - tmp_path, config, new_process=True, capture_output=False) as cloudflared: + tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], new_process=True, capture_output=False) as cloudflared: wait_tunnel_ready(tunnel_url=config.get_url()) with self.within_grace_period(): self.terminate_by_signal(cloudflared, signal) diff --git a/component-tests/test_tunnel.py b/component-tests/test_tunnel.py index 82f566c0..447b40e3 100644 --- a/component-tests/test_tunnel.py +++ b/component-tests/test_tunnel.py @@ -13,9 +13,9 @@ class TestTunnel: def test_tunnel_hello_world(self, tmp_path, component_tests_config): config = component_tests_config(cfd_mode=CfdModes.NAMED, run_proxy_dns=False, provide_ingress=False) LOGGER.debug(config) - with start_cloudflared(tmp_path, config, cfd_args=["run", "--hello-world"], new_process=True): + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], cfd_args=["run", "--hello-world"], new_process=True): wait_tunnel_ready(tunnel_url=config.get_url(), - require_min_connections=4) + require_min_connections=1) """ test_get_host_details does the following: @@ -34,9 +34,9 @@ class TestTunnel: headers = {} headers["Content-Type"] = "application/json" config_path = write_config(tmp_path, config.full_config) - with start_cloudflared(tmp_path, config, cfd_args=["run", "--hello-world"], new_process=True): + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1", "--label" , "test"], cfd_args=["run", "--hello-world"], new_process=True): wait_tunnel_ready(tunnel_url=config.get_url(), - require_min_connections=4) + require_min_connections=1) cfd_cli = CloudflaredCli(config, config_path, LOGGER) access_jwt = cfd_cli.get_management_token(config, config_path) connector_id = cfd_cli.get_connector_id(config)[0] @@ -45,7 +45,7 @@ class TestTunnel: # Assert response json. assert resp.status_code == 200, "Expected cloudflared to return 200 for host details" - assert resp.json()["hostname"] != "", "Expected cloudflared to return hostname" + assert resp.json()["hostname"] == "custom:test", "Expected cloudflared to return hostname" assert resp.json()["ip"] != "", "Expected cloudflared to return ip" assert resp.json()["connector_id"] == connector_id, "Expected cloudflared to return connector_id" @@ -53,8 +53,8 @@ class TestTunnel: def test_tunnel_url(self, tmp_path, component_tests_config): config = component_tests_config(cfd_mode=CfdModes.NAMED, run_proxy_dns=False, provide_ingress=False) LOGGER.debug(config) - with start_cloudflared(tmp_path, config, cfd_args=["run", "--url", f"http://localhost:{METRICS_PORT}/"], new_process=True): - wait_tunnel_ready(require_min_connections=4) + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], cfd_args=["run", "--url", f"http://localhost:{METRICS_PORT}/"], new_process=True): + wait_tunnel_ready(require_min_connections=1) send_requests(config.get_url()+"/ready", 3, True) def test_tunnel_no_ingress(self, tmp_path, component_tests_config): @@ -64,8 +64,8 @@ class TestTunnel: ''' config = component_tests_config(cfd_mode=CfdModes.NAMED, run_proxy_dns=False, provide_ingress=False) LOGGER.debug(config) - with start_cloudflared(tmp_path, config, cfd_args=["run"], new_process=True): - wait_tunnel_ready(require_min_connections=4) + with start_cloudflared(tmp_path, config, cfd_pre_args=["tunnel", "--ha-connections", "1"], cfd_args=["run"], new_process=True): + wait_tunnel_ready(require_min_connections=1) resp = send_request(config.get_url()+"/") assert resp.status_code == 503, "Expected cloudflared to return 503 for all requests with no ingress defined" resp = send_request(config.get_url()+"/test") diff --git a/management/service.go b/management/service.go index 96fe9048..9e6f9ad3 100644 --- a/management/service.go +++ b/management/service.go @@ -2,6 +2,7 @@ package management import ( "context" + "fmt" "net" "net/http" "os" @@ -32,8 +33,10 @@ type ManagementService struct { // The management tunnel hostname Hostname string + // Host details related configurations serviceIP string clientID uuid.UUID + label string log *zerolog.Logger router chi.Router @@ -51,6 +54,7 @@ type ManagementService struct { func New(managementHostname string, serviceIP string, clientID uuid.UUID, + label string, log *zerolog.Logger, logger LoggerListener, ) *ManagementService { @@ -60,6 +64,7 @@ func New(managementHostname string, logger: logger, serviceIP: serviceIP, clientID: clientID, + label: label, } r := chi.NewRouter() r.Get("/ping", ping) @@ -94,15 +99,27 @@ func (m *ManagementService) getHostDetails(w http.ResponseWriter, r *http.Reques if ip, err := getPrivateIP(m.serviceIP); err == nil { getHostDetailsResponse.IP = ip } - if hostname, err := os.Hostname(); err == nil { - getHostDetailsResponse.HostName = hostname - } + getHostDetailsResponse.HostName = m.getLabel() w.Header().Set("Content-Type", "application/json") w.WriteHeader(200) json.NewEncoder(w).Encode(getHostDetailsResponse) } +func (m *ManagementService) getLabel() string { + if m.label != "" { + return fmt.Sprintf("custom:%s", m.label) + } + + // If no label is provided we return the system hostname. This is not + // a fqdn hostname. + hostname, err := os.Hostname() + if err != nil { + return "unknown" + } + return hostname +} + // Get preferred private ip of this machine func getPrivateIP(addr string) (string, error) { conn, err := net.DialTimeout("tcp", addr, 1*time.Second) diff --git a/orchestration/orchestrator_test.go b/orchestration/orchestrator_test.go index e3051ab3..17d54f47 100644 --- a/orchestration/orchestrator_test.go +++ b/orchestration/orchestrator_test.go @@ -51,7 +51,7 @@ func TestUpdateConfiguration(t *testing.T) { initConfig := &Config{ Ingress: &ingress.Ingress{}, } - orchestrator, err := NewOrchestrator(context.Background(), initConfig, testTags, []ingress.Rule{ingress.NewManagementRule(management.New("management.argotunnel.com", "1.1.1.1:80", uuid.Nil, &testLogger, nil))}, &testLogger) + orchestrator, err := NewOrchestrator(context.Background(), initConfig, testTags, []ingress.Rule{ingress.NewManagementRule(management.New("management.argotunnel.com", "1.1.1.1:80", uuid.Nil, "", &testLogger, nil))}, &testLogger) require.NoError(t, err) initOriginProxy, err := orchestrator.GetOriginProxy() require.NoError(t, err)