diff --git a/lib/srv/server/installer/autodiscover.go b/lib/srv/server/installer/autodiscover.go index 287677b3aa68b..a9b8901127972 100644 --- a/lib/srv/server/installer/autodiscover.go +++ b/lib/srv/server/installer/autodiscover.go @@ -52,6 +52,13 @@ import ( "github.com/gravitational/teleport/lib/utils/packagemanager" ) +const ( + discoverNotice = "" + + "Teleport Discover has successfully configured /etc/teleport.yaml for this server to join the cluster.\n" + + "Discover might replace the configuration if the server fails to join the cluster.\n" + + "Please remove this file if you are managing this instance using another tool or doing so manually.\n" +) + // AutoDiscoverNodeInstallerConfig installs and configures a Teleport Server into the current system. type AutoDiscoverNodeInstallerConfig struct { Logger *slog.Logger @@ -312,6 +319,7 @@ func (ani *AutoDiscoverNodeInstaller) configureTeleportNode(ctx context.Context, _ = os.Remove(teleportYamlConfigurationPathNew) }() + discoverNoticeFile := teleportYamlConfigurationPath + ".discover" // Check if file already exists and has the same content that we are about to write if _, err := os.Stat(teleportYamlConfigurationPath); err == nil { hashExistingFile, err := checksum(teleportYamlConfigurationPath) @@ -325,14 +333,38 @@ func (ani *AutoDiscoverNodeInstaller) configureTeleportNode(ctx context.Context, } if hashExistingFile == hashNewFile { + if err := os.WriteFile(discoverNoticeFile, []byte(discoverNotice), 0o644); err != nil { + return trace.Wrap(err) + } + return trace.AlreadyExists("teleport.yaml is up to date") } + + // If a previous /etc/teleport.yaml configuration file exists and is different from the target one, it might be because one of the following reasons: + // - discover installation params (eg token name) were changed + // - `$ teleport node configure` command produces a different output + // - teleport was manually installed / configured + // + // For the first two scenarios, it's fine, and even desired in most cases, to restart teleport with the new configuration. + // + // However, for the last scenario (teleport was manually installed), this flow must not replace the currently running teleport service configuration. + // To prevent this, this flow checks for the existence of the discover notice file, and only allows replacement if it does exist. + if _, err := os.Stat(discoverNoticeFile); err != nil { + ani.Logger.InfoContext(ctx, "Refusing to replace the existing teleport configuration. For the script to replace the existing configuration remove the Teleport configuration.", + "teleport_configuration", teleportYamlConfigurationPath, + "discover_notice_file", discoverNoticeFile) + return trace.BadParameter("missing discover notice file") + } } if err := os.Rename(teleportYamlConfigurationPathNew, teleportYamlConfigurationPath); err != nil { return trace.Wrap(err) } + if err := os.WriteFile(discoverNoticeFile, []byte(discoverNotice), 0o644); err != nil { + return trace.Wrap(err) + } + return nil } diff --git a/lib/srv/server/installer/autodiscover_test.go b/lib/srv/server/installer/autodiscover_test.go index 5d36a036e1db6..0d02cb918f2cf 100644 --- a/lib/srv/server/installer/autodiscover_test.go +++ b/lib/srv/server/installer/autodiscover_test.go @@ -201,6 +201,8 @@ func TestAutoDiscoverNode(t *testing.T) { for binName, mockBin := range mockBins { require.True(t, mockBin.Check(t), "mismatch between expected invocations and actual calls for %q", binName) } + require.FileExists(t, testTempDir+"/etc/teleport.yaml") + require.FileExists(t, testTempDir+"/etc/teleport.yaml.discover") }) } } @@ -277,6 +279,8 @@ func TestAutoDiscoverNode(t *testing.T) { for binName, mockBin := range mockBins { require.True(t, mockBin.Check(t), "mismatch between expected invocations and actual calls for %q", binName) } + require.FileExists(t, testTempDir+"/etc/teleport.yaml") + require.FileExists(t, testTempDir+"/etc/teleport.yaml.discover") }) t.Run("fails when imds server is not available", func(t *testing.T) { @@ -314,9 +318,11 @@ func TestAutoDiscoverNode(t *testing.T) { for binName, mockBin := range mockBins { require.True(t, mockBin.Check(t), "mismatch between expected invocations and actual calls for %q", binName) } + require.NoFileExists(t, testTempDir+"/etc/teleport.yaml") + require.NoFileExists(t, testTempDir+"/etc/teleport.yaml.discover") }) - t.Run("reconfigures and restarts if target teleport.yaml is different", func(t *testing.T) { + t.Run("reconfigures and restarts if target teleport.yaml is different and discover file exists", func(t *testing.T) { distroConfig := wellKnownOS["ubuntu"]["24.04"] testTempDir := t.TempDir() @@ -345,6 +351,8 @@ func TestAutoDiscoverNode(t *testing.T) { // create an existing teleport.yaml configuration file require.NoError(t, os.WriteFile(testTempDir+"/etc/teleport.yaml", []byte("has wrong config"), 0o644)) + // create a teleport.yaml.discover to indicate that this host is controlled by the discover flow + require.NoError(t, os.WriteFile(testTempDir+"/etc/teleport.yaml.discover", []byte(""), 0o644)) // package manager is not called in this scenario because teleport binary already exists in the system require.FileExists(t, mockBins["teleport"].Path) @@ -374,11 +382,72 @@ func TestAutoDiscoverNode(t *testing.T) { require.NoFileExists(t, testTempDir+"/etc/teleport.yaml.new") require.FileExists(t, testTempDir+"/etc/teleport.yaml") + require.FileExists(t, testTempDir+"/etc/teleport.yaml.discover") bs, err := os.ReadFile(testTempDir + "/etc/teleport.yaml") require.NoError(t, err) require.Equal(t, "teleport.yaml configuration bytes", string(bs)) }) + t.Run("does not reconfigure if teleport.yaml exists but discover file does not exists", func(t *testing.T) { + distroConfig := wellKnownOS["ubuntu"]["24.04"] + + testTempDir := t.TempDir() + + setupDirsForTest(t, testTempDir, distroConfig) + + installerConfig := &AutoDiscoverNodeInstallerConfig{ + RepositoryChannel: "stable/rolling", + AutoUpgrades: false, + ProxyPublicAddr: "proxy.example.com", + TeleportPackage: "teleport", + TokenName: "my-token", + AzureClientID: "azure-client-id", + + fsRootPrefix: testTempDir, + imdsProviders: mockIMDSProviders, + binariesLocation: binariesLocation, + aptPublicKeyEndpoint: mockRepoKeys.URL, + } + + teleportInstaller, err := NewAutoDiscoverNodeInstaller(installerConfig) + require.NoError(t, err) + + // package manager is not called in this scenario because teleport binary already exists in the system + require.FileExists(t, mockBins["teleport"].Path) + + // create an existing teleport.yaml configuration file + require.NoError(t, os.WriteFile(testTempDir+"/etc/teleport.yaml", []byte("has wrong config"), 0o644)) + + // package manager is not called in this scenario because teleport binary already exists in the system + require.FileExists(t, mockBins["teleport"].Path) + + mockBins["teleport"].Expect("node", + "configure", + "--output=file://"+testTempDir+"/etc/teleport.yaml.new", + "--proxy=proxy.example.com", + "--join-method=azure", + "--token=my-token", + "--labels=teleport.internal/region=eastus,teleport.internal/resource-group=TestGroup,teleport.internal/subscription-id=5187AF11-3581-4AB6-A654-59405CD40C44,teleport.internal/vm-id=ED7DAC09-6E73-447F-BD18-AF4D1196C1E4", + "--azure-client-id=azure-client-id", + ).AndCallFunc(func(c *bintest.Call) { + // create a teleport.yaml configuration file + require.NoError(t, os.WriteFile(testTempDir+"/etc/teleport.yaml.new", []byte("teleport.yaml configuration bytes"), 0o644)) + c.Exit(0) + }) + + require.ErrorContains(t, teleportInstaller.Install(ctx), "missing discover notice file") + + for binName, mockBin := range mockBins { + require.True(t, mockBin.Check(t), "mismatch between expected invocations and actual calls for %q", binName) + } + + require.NoFileExists(t, testTempDir+"/etc/teleport.yaml.new") + require.FileExists(t, testTempDir+"/etc/teleport.yaml") + bs, err := os.ReadFile(testTempDir + "/etc/teleport.yaml") + require.NoError(t, err) + require.Equal(t, "has wrong config", string(bs)) + }) + t.Run("does nothing if teleport is already installed and target teleport.yaml configuration already exists", func(t *testing.T) { distroName := "ubuntu" distroVersion := "24.04" @@ -440,6 +509,91 @@ func TestAutoDiscoverNode(t *testing.T) { require.NoError(t, err) require.Equal(t, "teleport.yaml configuration bytes", string(bs)) }) + + t.Run("does nothing if target teleport.yaml configuration exists but was manually created/edited", func(t *testing.T) { + distroName := "ubuntu" + distroVersion := "24.04" + distroConfig := wellKnownOS[distroName][distroVersion] + + testTempDir := t.TempDir() + + setupDirsForTest(t, testTempDir, distroConfig) + + installerConfig := &AutoDiscoverNodeInstallerConfig{ + RepositoryChannel: "stable/rolling", + AutoUpgrades: false, + ProxyPublicAddr: "proxy.example.com", + TeleportPackage: "teleport", + TokenName: "my-token", + AzureClientID: "azure-client-id", + + fsRootPrefix: testTempDir, + imdsProviders: mockIMDSProviders, + binariesLocation: binariesLocation, + aptPublicKeyEndpoint: mockRepoKeys.URL, + } + + teleportInstaller, err := NewAutoDiscoverNodeInstaller(installerConfig) + require.NoError(t, err) + + // package manager is not called in this scenario because teleport binary already exists in the system + require.FileExists(t, mockBins["teleport"].Path) + + // create an existing teleport.yaml configuration file + require.NoError(t, os.WriteFile(testTempDir+"/etc/teleport.yaml", []byte("teleport.yaml bytes"), 0o644)) + + // package manager is not called in this scenario because teleport binary already exists in the system + require.FileExists(t, mockBins["teleport"].Path) + + mockBins["teleport"].Expect("node", + "configure", + "--output=file://"+testTempDir+"/etc/teleport.yaml.new", + "--proxy=proxy.example.com", + "--join-method=azure", + "--token=my-token", + "--labels=teleport.internal/region=eastus,teleport.internal/resource-group=TestGroup,teleport.internal/subscription-id=5187AF11-3581-4AB6-A654-59405CD40C44,teleport.internal/vm-id=ED7DAC09-6E73-447F-BD18-AF4D1196C1E4", + "--azure-client-id=azure-client-id", + ).AndCallFunc(func(c *bintest.Call) { + // create a teleport.yaml configuration file + require.NoError(t, os.WriteFile(testTempDir+"/etc/teleport.yaml.new", []byte("teleport.yaml configuration bytes"), 0o644)) + c.Exit(0) + }) + + require.ErrorContains(t, teleportInstaller.Install(ctx), "missing discover notice file") + + for binName, mockBin := range mockBins { + require.True(t, mockBin.Check(t), "mismatch between expected invocations and actual calls for %q", binName) + } + + t.Run("even if it runs multiple times", func(t *testing.T) { + + // create an existing teleport.yaml configuration file + require.NoError(t, os.WriteFile(testTempDir+"/etc/teleport.yaml", []byte("manual configuration already exists"), 0o644)) + + // package manager is not called in this scenario because teleport binary already exists in the system + require.FileExists(t, mockBins["teleport"].Path) + + mockBins["teleport"].Expect("node", + "configure", + "--output=file://"+testTempDir+"/etc/teleport.yaml.new", + "--proxy=proxy.example.com", + "--join-method=azure", + "--token=my-token", + "--labels=teleport.internal/region=eastus,teleport.internal/resource-group=TestGroup,teleport.internal/subscription-id=5187AF11-3581-4AB6-A654-59405CD40C44,teleport.internal/vm-id=ED7DAC09-6E73-447F-BD18-AF4D1196C1E4", + "--azure-client-id=azure-client-id", + ).AndCallFunc(func(c *bintest.Call) { + // create a teleport.yaml configuration file + require.NoError(t, os.WriteFile(testTempDir+"/etc/teleport.yaml.new", []byte("teleport.yaml configuration bytes"), 0o644)) + c.Exit(0) + }) + + require.ErrorContains(t, teleportInstaller.Install(ctx), "missing discover notice file") + + for binName, mockBin := range mockBins { + require.True(t, mockBin.Check(t), "mismatch between expected invocations and actual calls for %q", binName) + } + }) + }) } // wellKnownOS lists the officially supported repositories for Linux Distros