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.

Sign up for GitHub

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

Auto Discover Server must not overwrite manual changes #44637

Merged
merged 4 commits into from
Jul 30, 2024
Merged

Auto Discover Server must not overwrite manual changes #44637

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
28efbad
Auto Discover Server must not overwrite manual changes
marcoandredinis Jul 25, 2024
c36ed86
Update lib/srv/server/installer/autodiscover.go
marcoandredinis Jul 26, 2024
6702bf5
Update lib/srv/server/installer/autodiscover.go
marcoandredinis Jul 30, 2024
0c340ba
Update lib/srv/server/installer/autodiscover.go
GavinFrazar Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions lib/srv/server/installer/autodiscover.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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, either remove the teleport configuration or create the discover notice file ($ touch /etc/teleport.yaml.discover).",
marcoandredinis marked this conversation as resolved.
Show resolved Hide resolved
"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
}

Expand Down
156 changes: 155 additions & 1 deletion lib/srv/server/installer/autodiscover_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
}
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
Loading