Enable health checks for Kubernetes with virtual defaults

constants.go

@@ -775,9 +775,18 @@ const (
 var PresetRoles = []string{PresetEditorRoleName, PresetAccessRoleName, PresetAuditorRoleName}
 
 const (
-	// PresetDefaultHealthCheckConfigName is the name of a preset
-	// default health_check_config that enables health checks for all resources.
-	PresetDefaultHealthCheckConfigName = "default"
+	// VirtualDefaultHealthCheckConfigDBName is the name of a virtual
+	// health_check_config that enables health checks for all database
+	// resources. For historical reasons, it's value is "default" even
+	// though it applies to databases only.
+	VirtualDefaultHealthCheckConfigDBName = "default"
+	// VirtualDefaultHealthCheckConfigKubeName is the name of a virtual
+	// health_check_config that enables health checks for all Kubernetes
+	// resources.
+	VirtualDefaultHealthCheckConfigKubeName = "default-kube"
+	// VirtualDefaultHealthCheckConfigCount is the number of virtual
+	// health_check_config resources.
+	VirtualDefaultHealthCheckConfigCount = 2
 )
 
 const (

docs/pages/enroll-resources/kubernetes-access/health-checks.mdx

@@ -177,14 +177,12 @@ spec:
     kubernetes_labels_expression: 'labels["owner"] == "platform-team"'
 ```
 
-A `default_kube` `health_check_config` is introduced in version (= kubernetes.health_check_min_version =), and enables all Kubernetes clusters to participate in health checks.
+A `default-kube` `health_check_config` is introduced in version (= kubernetes.health_check_min_version =), and enables all Kubernetes clusters to participate in health checks.
 ```yaml
 kind: health_check_config
 metadata:
   description: Enables all health checks by default
-  labels:
-    teleport.internal/resource-type: preset
-  name: default_kube
+  name: default-kube
 spec:
   match:
     kubernetes_labels:
@@ -194,6 +192,8 @@ spec:
 version: v1
 ```
 
+`default-kube` may be disabled, but not permanently deleted. Deleting with `tctl rm health_check_config/default-kube` has the effect of resetting the config to its default settings and matching all Kubernetes clusters.
+
 A different `default` `health_check_config` also exists, and focuses on matching databases for health checks.
 
 Multiple different `health_check_config` resources may be created for different groups of Kubernetes clusters. When multiple `health_check_config` match the same Kubernetes cluster, configs are sorted in ascending order by name, and only the first config applies (e.g., the name "00-my-config" has greater precedence than "10-my-config").
@@ -202,14 +202,12 @@ Multiple different `health_check_config` resources may be created for different
 
 Set the `match.disabled` field to `true` on any `health_check_config`. 
 
-For example, use `tctl edit health_check_config/default_kube`
+For example, use `tctl edit health_check_config/default-kube`
 ```yaml
 kind: health_check_config
 metadata:
   description: Enables all health checks by default
-  labels:
-    teleport.internal/resource-type: preset
-  name: default_kube
+  name: default-kube
 spec:
   match:
     disable: true
@@ -226,7 +224,7 @@ Any defined labels, such as `kubernetes_labels`, are ignored when `disable: true
 
 Read the default health check config with `tctl get`:
 ```bash
-$ tctl get health_check_config/default_kube
+$ tctl get health_check_config/default-kube
 ```
 
 Create a new health check config with `tctl create`:
@@ -236,7 +234,7 @@ $ tctl create health_check_config.yaml
 
 Update an existing config interactively with `tctl edit`:
 ```bash
-$ tctl edit health_check_config/default_kube
+$ tctl edit health_check_config/default-kube
 ```
 
 Delete a health check config with `tctl rm`:

lib/auth/export_test.go

@@ -286,10 +286,6 @@ func CreatePresetRoles(ctx context.Context, um PresetRoleManager) error {
 	return createPresetRoles(ctx, um)
 }
 
-func CreatePresetHealthCheckConfig(ctx context.Context, svc services.HealthCheckConfig) error {
-	return createPresetHealthCheckConfig(ctx, svc)
-}
-
 func GetPresetUsers() []types.User {
 	return getPresetUsers()
 }

lib/auth/healthcheckconfig/healthcheckconfigv1/service_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/require"
 
+	"github.com/gravitational/teleport"
 	healthcheckconfigv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/healthcheckconfig/v1"
 	labelv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/label/v1"
 	"github.com/gravitational/teleport/api/types"
@@ -98,7 +99,8 @@ func TestHealthCheckConfigCRUD(t *testing.T) {
 				resp, err := clt.ServiceUnderTest.ListHealthCheckConfigs(ctx, &healthcheckconfigv1.ListHealthCheckConfigsRequest{})
 				if err == nil {
 					require.NotNil(t, resp)
-					require.Len(t, resp.Configs, 2, "the test bootstrapped exactly 2 health_check_config resources")
+					require.Len(t, resp.Configs, 2+teleport.VirtualDefaultHealthCheckConfigCount,
+						"expected 2 inserted and virtual defaults")
 				}
 				return err
 			},
@@ -199,7 +201,7 @@ func (c *accessTest) run(t *testing.T) {
 		ctx, clt := c.setup(t, spec)
 		err := c.actionFn(t, ctx, clt)
 		require.Error(t, err)
-		require.IsType(t, trace.AccessDenied(""), err)
+		require.True(t, trace.IsAccessDenied(err))
 	})
 
 	t.Run(fmt.Sprintf("%s is denied", c.name), func(t *testing.T) {
@@ -210,7 +212,7 @@ func (c *accessTest) run(t *testing.T) {
 		ctx, clt := c.setup(t, spec)
 		err := c.actionFn(t, ctx, clt)
 		require.Error(t, err)
-		require.IsType(t, trace.AccessDenied(""), err)
+		require.True(t, trace.IsAccessDenied(err))
 	})
 }
 

lib/auth/init.go

@@ -682,12 +682,6 @@ func initCluster(ctx context.Context, cfg InitConfig, asrv *Server) error {
 			asrv.logger.WarnContext(ctx, "error creating preset database object import rules", "error", err)
 		}
 		span.AddEvent("completed creating database object import rules")
-
-		span.AddEvent("creating preset health check config")
-		if err := createPresetHealthCheckConfig(ctx, asrv); err != nil {
-			return trace.Wrap(err)
-		}
-		span.AddEvent("completed creating preset health check config")
 	} else {
 		asrv.logger.InfoContext(ctx, "skipping preset role and user creation")
 	}
@@ -1489,27 +1483,6 @@ func createPresetDatabaseObjectImportRule(ctx context.Context, rules services.Da
 	return nil
 }
 
-// createPresetHealthCheckConfig creates a default preset health check config
-// resource that enables health checks on all resources.
-func createPresetHealthCheckConfig(ctx context.Context, svc services.HealthCheckConfig) error {
-	page, _, err := svc.ListHealthCheckConfigs(ctx, 0, "")
-	if err != nil {
-		return trace.Wrap(err, "failed listing available health check configs")
-	}
-	if len(page) > 0 {
-		return nil
-	}
-	preset := services.NewPresetHealthCheckConfig()
-	_, err = svc.CreateHealthCheckConfig(ctx, preset)
-	if err != nil && !trace.IsAlreadyExists(err) {
-		return trace.Wrap(err,
-			"failed creating preset health_check_config %s",
-			preset.GetMetadata().GetName(),
-		)
-	}
-	return nil
-}
-
 // isFirstStart returns 'true' if the auth server is starting for the 1st time
 // on this server.
 func isFirstStart(ctx context.Context, authServer *Server, cfg InitConfig) (bool, error) {

lib/auth/init_test.go

@@ -45,7 +45,6 @@ import (
 	"golang.org/x/crypto/ssh"
 	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/testing/protocmp"
-	"google.golang.org/protobuf/types/known/durationpb"
 	kyaml "k8s.io/apimachinery/pkg/util/yaml"
 
 	"github.com/gravitational/teleport"
@@ -141,7 +140,7 @@ func TestBadIdentity(t *testing.T) {
 
 	// bad cert type
 	_, err = state.ReadSSHIdentityFromKeyPair(priv, pub)
-	require.IsType(t, trace.BadParameter(""), err)
+	require.ErrorAs(t, err, new(*trace.BadParameterError))
 
 	// missing authority domain
 	cert, err := a.GenerateHostCert(sshca.HostCertificateRequest{
@@ -158,7 +157,7 @@ func TestBadIdentity(t *testing.T) {
 	require.NoError(t, err)
 
 	_, err = state.ReadSSHIdentityFromKeyPair(priv, cert)
-	require.IsType(t, trace.BadParameter(""), err)
+	require.ErrorAs(t, err, new(*trace.BadParameterError))
 
 	// missing host uuid
 	cert, err = a.GenerateHostCert(sshca.HostCertificateRequest{
@@ -175,7 +174,7 @@ func TestBadIdentity(t *testing.T) {
 	require.NoError(t, err)
 
 	_, err = state.ReadSSHIdentityFromKeyPair(priv, cert)
-	require.IsType(t, trace.BadParameter(""), err)
+	require.ErrorAs(t, err, new(*trace.BadParameterError))
 
 	// unrecognized role
 	cert, err = a.GenerateHostCert(sshca.HostCertificateRequest{
@@ -192,7 +191,7 @@ func TestBadIdentity(t *testing.T) {
 	require.NoError(t, err)
 
 	_, err = state.ReadSSHIdentityFromKeyPair(priv, cert)
-	require.IsType(t, trace.BadParameter(""), err)
+	require.ErrorAs(t, err, new(*trace.BadParameterError))
 }
 
 func TestSignatureAlgorithmSuite(t *testing.T) {
@@ -969,25 +968,15 @@ func TestPresets(t *testing.T) {
 		err := auth.CreatePresetRoles(ctx, as)
 		require.NoError(t, err)
 
-		err = auth.CreatePresetHealthCheckConfig(ctx, as)
-		require.NoError(t, err)
-
 		// Second call should not fail
 		err = auth.CreatePresetRoles(ctx, as)
 		require.NoError(t, err)
 
-		err = auth.CreatePresetHealthCheckConfig(ctx, as)
-		require.NoError(t, err)
-
 		// Presets were created
 		for _, role := range presetRoleNames {
 			_, err := as.GetRole(ctx, role)
 			require.NoError(t, err)
 		}
-
-		cfg, err := as.GetHealthCheckConfig(ctx, teleport.PresetDefaultHealthCheckConfigName)
-		require.NoError(t, err)
-		require.NotNil(t, cfg)
 	})
 
 	// Makes sure that existing role with the same name is not modified
@@ -1015,26 +1004,6 @@ func TestPresets(t *testing.T) {
 		require.Equal(t, access.GetLogins(types.Allow), out.GetLogins(types.Allow))
 	})
 
-	t.Run("ExistingHealthCheckConfig", func(t *testing.T) {
-		as := newTestAuthServer(ctx, t)
-		clock := clockwork.NewFakeClock()
-		as.SetClock(clock)
-
-		// an existing health check config should not be modified by init
-		cfg := services.NewPresetHealthCheckConfig()
-		cfg.Spec.Interval = durationpb.New(42 * time.Second)
-		cfg, err := as.CreateHealthCheckConfig(ctx, cfg)
-		require.NoError(t, err)
-
-		err = auth.CreatePresetHealthCheckConfig(ctx, as)
-		require.NoError(t, err)
-
-		// Preset was created. Ensure it didn't overwrite the existing config
-		got, err := as.GetHealthCheckConfig(ctx, cfg.GetMetadata().GetName())
-		require.NoError(t, err)
-		require.Equal(t, cfg.Spec.Interval.AsDuration(), got.Spec.Interval.AsDuration())
-	})
-
 	// If a default allow condition is not present, ensure it gets added.
 	t.Run("AddDefaultAllowConditions", func(t *testing.T) {
 		as := newTestAuthServer(ctx, t)

lib/cache/cache_test.go

@@ -1363,6 +1363,11 @@ func testResources[T types.Resource](t *testing.T, p *testPack, funcs testFuncs[
 
 // testResources153 is a wrapper for testing resources conforming to types.Resource153
 func testResources153[T types.Resource153](t *testing.T, p *testPack, funcs testFuncs[T], opts ...optionsFunc) {
+	// TODO(rana): Add broader support for virtual resources in list operations.
+	// Virtual resources change the total count returned by list operations,
+	// and is unexpected for the current test. When updated, we can remove virtual
+	// resource filtering and paging from lib/cache/health_check_config_test.go.
+	opts = append(opts, withSkipPaginationTest())
 	funcs.resource = defaultResource153Ops[T]()
 	testResourcesInternal(t, p, funcs, opts...)
 }

lib/cache/health_check_config_test.go

@@ -25,6 +25,8 @@ import (
 	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/require"
 
+	"github.com/gravitational/teleport"
+	"github.com/gravitational/teleport/api/defaults"
 	healthcheckconfigv1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/healthcheckconfig/v1"
 	"github.com/gravitational/teleport/api/types/healthcheckconfig"
 )
@@ -57,13 +59,84 @@ func TestHealthCheckConfig(t *testing.T) {
 			_, err := p.healthCheckConfig.CreateHealthCheckConfig(ctx, cfg)
 			return trace.Wrap(err)
 		},
-		list: p.healthCheckConfig.ListHealthCheckConfigs,
+		list: filterHealthCfgNonVirtual(p.healthCheckConfig.ListHealthCheckConfigs),
 		update: func(ctx context.Context, cfg *healthcheckconfigv1.HealthCheckConfig) error {
 			_, err := p.healthCheckConfig.UpdateHealthCheckConfig(ctx, cfg)
 			return trace.Wrap(err)
 		},
 		deleteAll: p.healthCheckConfig.DeleteAllHealthCheckConfigs,
-		cacheList: p.cache.ListHealthCheckConfigs,
+		cacheList: filterHealthCfgNonVirtual(p.cache.ListHealthCheckConfigs),
 		cacheGet:  p.cache.GetHealthCheckConfig,
 	})
 }
+
+type listHealthCfgFunc func(ctx context.Context, pageSize int, pageToken string) ([]*healthcheckconfigv1.HealthCheckConfig, string, error)
+
+// filterHealthCfgNonVirtual excludes virtual defaults while maintaining pagination.
+func filterHealthCfgNonVirtual(listFn listHealthCfgFunc) listHealthCfgFunc {
+	return func(ctx context.Context, pageSize int, pageToken string) ([]*healthcheckconfigv1.HealthCheckConfig, string, error) {
+		var allNonVirtual []*healthcheckconfigv1.HealthCheckConfig
+		var token string
+		for {
+			items, nextPageToken, err := listFn(ctx, defaults.DefaultChunkSize, token)
+			if err != nil {
+				return nil, "", trace.Wrap(err)
+			}
+			for _, item := range items {
+				if !isVirtualDefaultHealthCheckConfig(item.GetMetadata().GetName()) {
+					allNonVirtual = append(allNonVirtual, item)
+				}
+			}
+			if nextPageToken == "" {
+				break
+			}
+			token = nextPageToken
+		}
+		page, nextPageToken := pageHealthCfg(allNonVirtual, pageSize, pageToken)
+		return page, nextPageToken, nil
+	}
+}
+
+// pageHealthCfg creates a page from a slice.
+func pageHealthCfg(
+	items []*healthcheckconfigv1.HealthCheckConfig,
+	pageSize int,
+	pageToken string,
+) ([]*healthcheckconfigv1.HealthCheckConfig, string) {
+	if len(items) == 0 {
+		return nil, ""
+	}
+	// look for the start index
+	var idxStart int
+	if pageToken != "" {
+		for n, item := range items {
+			if item.GetMetadata().GetName() == pageToken {
+				idxStart = n + 1
+				break
+			}
+		}
+	}
+	if idxStart >= len(items) {
+		return nil, ""
+	}
+	// look for the end index
+	idxEnd := len(items)
+	if pageSize > 0 && idxStart+pageSize < len(items) {
+		idxEnd = idxStart + pageSize
+	}
+	page := items[idxStart:idxEnd]
+	var nextPageToken string
+	if idxEnd < len(items) {
+		nextPageToken = page[len(page)-1].GetMetadata().GetName()
+	}
+	return page, nextPageToken
+}
+
+func isVirtualDefaultHealthCheckConfig(name string) bool {
+	switch name {
+	case teleport.VirtualDefaultHealthCheckConfigDBName,
+		teleport.VirtualDefaultHealthCheckConfigKubeName:
+		return true
+	}
+	return false
+}