Skip to content

Commit

Permalink
Fix test flakes related to Kubernetes user cert generation (#52527)
Browse files Browse the repository at this point in the history
#52109 added a dependency on the unified resource cache to user
cert generation to reduce resource consumption. A number of tests
that exercise generating Kubernetes user certs were either not
waiting for the Kubernetes resources to exist prior to authentication
and getting lucky, or checking that the resources were in the auth
cache, but not the unified resource cache.

This attempts to cover any tests which generate Kubernetes user
certificates to verify that the unified resource cache contains
the expected cluster before proceeding.

Fixes #52157.
Fixes #52441.
  • Loading branch information
rosstimothy authored Feb 27, 2025
1 parent f8c3ccb commit 5240263
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 15 deletions.
16 changes: 9 additions & 7 deletions e2e/aws/eks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ func awsEKSDiscoveryMatchedCluster(t *testing.T) {
)
// Get the auth server.
authC := teleport.Process.GetAuthServer()
expectedClusterName := os.Getenv(eksClusterNameEnv)
// Wait for the discovery service to discover the cluster and create a
// KubernetesCluster resource.
// Discovery service will scan the AWS account each minutes.
Expand All @@ -105,7 +106,7 @@ func awsEKSDiscoveryMatchedCluster(t *testing.T) {
// Fail fast if the discovery service creates more than one cluster.
assert.Len(t, clusters, 1)
// Fail fast if the discovery service creates a cluster with a different name.
assert.Equal(t, os.Getenv(eksClusterNameEnv), clusters[0].GetName())
assert.Equal(t, expectedClusterName, clusters[0].GetName())
return true
}, 3*time.Minute, 10*time.Second, "wait for the discovery service to create a cluster")

Expand All @@ -116,13 +117,14 @@ func awsEKSDiscoveryMatchedCluster(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

kubeServers, err := authC.GetKubernetesServers(ctx)
return err == nil && len(kubeServers) == 1
for ks := range authC.UnifiedResourceCache.KubernetesServers(ctx, services.UnifiedResourcesIterateParams{}) {
if ks.GetCluster().GetName() == expectedClusterName {
return true
}
}
return false
}, 2*time.Minute, time.Second, "wait for the kubernetes service to create a KubernetesServer")

clusters, err := authC.GetKubernetesClusters(context.Background())
require.NoError(t, err)

// kubeClient is a Kubernetes client for the user created above
// that will be used to verify that the user can access the cluster and
// the permissions are correct.
Expand All @@ -131,7 +133,7 @@ func awsEKSDiscoveryMatchedCluster(t *testing.T) {
Username: hostUser,
KubeUsers: kubeUsers,
KubeGroups: kubeGroups,
KubeCluster: clusters[0].GetName(),
KubeCluster: expectedClusterName,
})
require.NoError(t, err)

Expand Down
10 changes: 10 additions & 0 deletions lib/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,16 @@ func TestAuthenticateSSHUser(t *testing.T) {
_, err = s.a.UpsertKubernetesServer(ctx, kubeServer)
require.NoError(t, err)

// Wait for cache propagation of the kubernetes resources before proceeding with the tests.
require.Eventually(t, func() bool {
for ks := range s.a.UnifiedResourceCache.KubernetesServers(ctx, services.UnifiedResourcesIterateParams{}) {
if ks.GetCluster().GetName() == kubeCluster.GetName() {
return true
}
}
return false
}, 10*time.Second, 100*time.Millisecond)

// Login specifying a valid kube cluster. It should appear in the TLS cert.
resp, err = s.a.AuthenticateSSHUser(ctx, authclient.AuthenticateSSHRequest{
AuthenticateUserRequest: authclient.AuthenticateUserRequest{
Expand Down
17 changes: 12 additions & 5 deletions tool/tsh/common/kube_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,18 @@ func setupKubeTestPack(t *testing.T, withMultiplexMode bool) *kubeTestPack {
},
),
withValidationFunc(func(s *suite) bool {
rootClusters, err := s.root.GetAuthServer().GetKubernetesServers(ctx)
require.NoError(t, err)
leafClusters, err := s.leaf.GetAuthServer().GetKubernetesServers(ctx)
require.NoError(t, err)
return len(rootClusters) >= 2 && len(leafClusters) >= 1
// Wait for cache propagation of the kubernetes resources before proceeding with the tests.
var foundRoot1, foundRoot2, foundLeaf bool
for ks := range s.root.GetAuthServer().UnifiedResourceCache.KubernetesServers(ctx, services.UnifiedResourcesIterateParams{}) {
foundRoot1 = foundRoot1 || ks.GetCluster().GetName() == rootKubeCluster1
foundRoot2 = foundRoot2 || ks.GetCluster().GetName() == rootKubeCluster2
}

for ks := range s.leaf.GetAuthServer().UnifiedResourceCache.KubernetesServers(ctx, services.UnifiedResourcesIterateParams{}) {
foundLeaf = foundLeaf || ks.GetCluster().GetName() == leafKubeCluster
}

return foundRoot1 && foundRoot2 && foundLeaf
}),
)

Expand Down
5 changes: 2 additions & 3 deletions tool/tsh/common/tsh_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import (
"github.com/gravitational/teleport/lib/config"
"github.com/gravitational/teleport/lib/service"
"github.com/gravitational/teleport/lib/service/servicecfg"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/utils"
)

Expand Down Expand Up @@ -482,10 +483,8 @@ func mustRegisterKubeClusters(t *testing.T, ctx context.Context, authSrv *auth.S
require.NoError(t, wg.Wait())

require.EventuallyWithT(t, func(c *assert.CollectT) {
servers, err := authSrv.GetKubernetesServers(ctx)
assert.NoError(c, err)
gotNames := map[string]struct{}{}
for _, ks := range servers {
for ks := range authSrv.UnifiedResourceCache.KubernetesServers(ctx, services.UnifiedResourcesIterateParams{}) {
gotNames[ks.GetName()] = struct{}{}
}
for _, name := range wantNames {
Expand Down

0 comments on commit 5240263

Please sign in to comment.