From b1351d0fdb8118679bedf13a15d83eb5efdb0665 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 15 May 2020 06:25:36 -0700 Subject: [PATCH] Remove code which overrides "diffusion.ssh-username" when instanced Summary: Ref T13529. Now that instances can be renamed, an instance may have multiple valid SSH usernames and the preferred SSH username may not be the intenal instance name. `PhacilitySiteSource` should already always set `diffusion.ssh-username` correctly, to the current preferred SSH username (which may be "new-name" after a rename from "old-name"), so we should never be able to reach this code without an accurate `diffusion.ssh-username` value available. The code to resolve names into instances also already works for both "ssh old-name@..." and "ssh new-name@...". So I believe this code has no beneficial effects and only causes harm: it may force us to return "old-name" when falling through would correctly return "new-name". Test Plan: - Previously: renamed an instance, then SSH'd to it using both the old and new names. Both work. - Previously: verified that `diffusion.ssh-username` is set correctly after a rename. - Verified that Diffusion "Clone" UI now shows "new-name" after an instance rename. - The real question here is: does this break something I'm not thinking of? And the change probably has to go to production to answer that. Maniphest Tasks: T13529 Differential Revision: https://secure.phabricator.com/D21259 --- src/applications/almanac/util/AlmanacKeys.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/applications/almanac/util/AlmanacKeys.php b/src/applications/almanac/util/AlmanacKeys.php index 7ed9098e97..6ed0f187c1 100644 --- a/src/applications/almanac/util/AlmanacKeys.php +++ b/src/applications/almanac/util/AlmanacKeys.php @@ -57,15 +57,6 @@ final class AlmanacKeys extends Phobject { } public static function getClusterSSHUser() { - // NOTE: When instancing, we currently use the SSH username to figure out - // which instance you are connecting to. We can't use the host name because - // we have no way to tell which host you think you're reaching: the SSH - // protocol does not have a mechanism like a "Host" header. - $username = PhabricatorEnv::getEnvConfig('cluster.instance'); - if (strlen($username)) { - return $username; - } - $username = PhabricatorEnv::getEnvConfig('diffusion.ssh-user'); if (strlen($username)) { return $username;