Fix defaults on incomplete AU config or version resources (#47872)

* Fix panic on incomplete AU config or version resources * lint * address tiago's feedback
gravitational · Oct 24, 2024 · bb5740e · bb5740e
1 parent db8cadf
commit bb5740e
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 10 deletions.
diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go
@@ -59,6 +59,7 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 
 	"github.com/gravitational/teleport"
+	"github.com/gravitational/teleport/api"
 	apiclient "github.com/gravitational/teleport/api/client"
 	"github.com/gravitational/teleport/api/client/proto"
 	"github.com/gravitational/teleport/api/client/webclient"
@@ -69,6 +70,7 @@ import (
 	"github.com/gravitational/teleport/api/mfa"
 	apitracing "github.com/gravitational/teleport/api/observability/tracing"
 	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/api/types/autoupdate"
 	apievents "github.com/gravitational/teleport/api/types/events"
 	"github.com/gravitational/teleport/api/types/installers"
 	"github.com/gravitational/teleport/api/utils/keys"
@@ -1544,15 +1546,27 @@ func (h *Handler) find(w http.ResponseWriter, r *http.Request, p httprouter.Para
 	// TODO(vapopov) DELETE IN v18.0.0 check of IsNotImplemented, must be backported to all latest supported versions.
 	if err != nil && !trace.IsNotFound(err) && !trace.IsNotImplemented(err) {
 		h.logger.ErrorContext(r.Context(), "failed to receive AutoUpdateConfig", "error", err)
-	} else if err == nil {
+	}
+	// If we can't get the AU config or tools AU are not configured, we default to "disabled".
+	// This ensures we fail open and don't accidentally update agents if something is going wrong.
+	// If we want to enable AUs by default, it would be better to create a default "autoupdate_config" resource
+	// than changing this logic.
+	if autoUpdateConfig.GetSpec().GetTools() == nil {
+		response.AutoUpdate.ToolsMode = autoupdate.ToolsUpdateModeDisabled
+	} else {
 		response.AutoUpdate.ToolsMode = autoUpdateConfig.GetSpec().GetTools().GetMode()
 	}
 
 	autoUpdateVersion, err := h.cfg.AccessPoint.GetAutoUpdateVersion(r.Context())
 	// TODO(vapopov) DELETE IN v18.0.0 check of IsNotImplemented, must be backported to all latest supported versions.
 	if err != nil && !trace.IsNotFound(err) && !trace.IsNotImplemented(err) {
 		h.logger.ErrorContext(r.Context(), "failed to receive AutoUpdateVersion", "error", err)
-	} else if err == nil {
+	}
+	// If we can't get the AU version or tools AU version is not specified, we default to the current proxy version.
+	// This ensures we always advertise a version compatible with the cluster.
+	if autoUpdateVersion.GetSpec().GetTools() == nil {
+		response.AutoUpdate.ToolsVersion = api.Version
+	} else {
 		response.AutoUpdate.ToolsVersion = autoUpdateVersion.GetSpec().GetTools().GetTargetVersion()
 	}
 

diff --git a/lib/web/apiserver_ping_test.go b/lib/web/apiserver_ping_test.go
@@ -30,6 +30,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/gravitational/teleport/api"
 	"github.com/gravitational/teleport/api/client/webclient"
 	"github.com/gravitational/teleport/api/constants"
 	autoupdatev1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1"
@@ -302,8 +303,11 @@ func TestPing_autoUpdateResources(t *testing.T) {
 		expected webclient.AutoUpdateSettings
 	}{
 		{
-			name:     "resources not defined",
-			expected: webclient.AutoUpdateSettings{},
+			name: "resources not defined",
+			expected: webclient.AutoUpdateSettings{
+				ToolsVersion: api.Version,
+				ToolsMode:    autoupdate.ToolsUpdateModeDisabled,
+			},
 		},
 		{
 			name: "enable auto update",
@@ -312,8 +316,21 @@ func TestPing_autoUpdateResources(t *testing.T) {
 					Mode: autoupdate.ToolsUpdateModeEnabled,
 				},
 			},
-			expected: webclient.AutoUpdateSettings{ToolsMode: "enabled"},
-			cleanup:  true,
+			expected: webclient.AutoUpdateSettings{
+				ToolsMode:    autoupdate.ToolsUpdateModeEnabled,
+				ToolsVersion: api.Version,
+			},
+			cleanup: true,
+		},
+		{
+			name:    "no autoupdate tool config nor version",
+			config:  &autoupdatev1pb.AutoUpdateConfigSpec{},
+			version: &autoupdatev1pb.AutoUpdateVersionSpec{},
+			expected: webclient.AutoUpdateSettings{
+				ToolsVersion: api.Version,
+				ToolsMode:    autoupdate.ToolsUpdateModeDisabled,
+			},
+			cleanup: true,
 		},
 		{
 			name: "set auto update version",
@@ -322,8 +339,11 @@ func TestPing_autoUpdateResources(t *testing.T) {
 					TargetVersion: "1.2.3",
 				},
 			},
-			expected: webclient.AutoUpdateSettings{ToolsVersion: "1.2.3"},
-			cleanup:  true,
+			expected: webclient.AutoUpdateSettings{
+				ToolsVersion: "1.2.3",
+				ToolsMode:    autoupdate.ToolsUpdateModeDisabled,
+			},
+			cleanup: true,
 		},
 		{
 			name: "enable auto update and set version",
@@ -337,7 +357,10 @@ func TestPing_autoUpdateResources(t *testing.T) {
 					TargetVersion: "1.2.3",
 				},
 			},
-			expected: webclient.AutoUpdateSettings{ToolsMode: "enabled", ToolsVersion: "1.2.3"},
+			expected: webclient.AutoUpdateSettings{
+				ToolsMode:    autoupdate.ToolsUpdateModeEnabled,
+				ToolsVersion: "1.2.3",
+			},
 		},
 		{
 			name: "modify auto update config and version",
@@ -351,7 +374,10 @@ func TestPing_autoUpdateResources(t *testing.T) {
 					TargetVersion: "3.2.1",
 				},
 			},
-			expected: webclient.AutoUpdateSettings{ToolsMode: "disabled", ToolsVersion: "3.2.1"},
+			expected: webclient.AutoUpdateSettings{
+				ToolsMode:    autoupdate.ToolsUpdateModeDisabled,
+				ToolsVersion: "3.2.1",
+			},
 		},
 	}
 	for _, tc := range tests {