Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions test/e2e/util/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ func GetClient() (crclient.Client, error) {
if err != nil {
return nil, fmt.Errorf("unable to get kubernetes config: %w", err)
}
return GetClientFromConfig(config)
}

// GetClientFromConfig creates a controller-runtime client from the given REST config.
func GetClientFromConfig(config *rest.Config) (crclient.Client, error) {
if config == nil {
return nil, fmt.Errorf("unable to get kubernetes client: nil REST config")
}
client, err := crclient.New(config, crclient.Options{Scheme: scheme})
if err != nil {
return nil, fmt.Errorf("unable to get kubernetes client: %w", err)
Expand Down
66 changes: 66 additions & 0 deletions test/e2e/v2/internal/test_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,17 @@ import (
"context"
"fmt"
"sync"
"time"

hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
"github.com/openshift/hypershift/hypershift-operator/controllers/manifests"
e2eutil "github.com/openshift/hypershift/test/e2e/util"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/discovery"
"k8s.io/client-go/tools/clientcmd"

crclient "sigs.k8s.io/controller-runtime/pkg/client"
)

Expand All @@ -41,6 +47,8 @@ type TestContext struct {
ControlPlaneNamespace string
hostedCluster *hyperv1.HostedCluster
hostedClusterOnce sync.Once
guestClient crclient.Client
guestClientOnce sync.Once
}

// GetHostedCluster returns the HostedCluster associated with this test context.
Expand Down Expand Up @@ -72,6 +80,64 @@ func (tc *TestContext) GetHostedCluster() *hyperv1.HostedCluster {
return tc.hostedCluster
}

// GetGuestClient returns a controller-runtime client for the guest (hosted) cluster.
// It fetches the admin kubeconfig from the HostedCluster status lazily on first call via sync.Once.
// Retries handle transient DNS propagation failures when connecting to the guest API server.
// Panics if the guest client cannot be created, as tests cannot proceed without it.
func (tc *TestContext) GetGuestClient() crclient.Client {
tc.guestClientOnce.Do(func() {
hc := tc.GetHostedCluster()
if hc == nil || hc.Status.KubeConfig == nil {
panic("HostedCluster has no kubeconfig in status")
}

var secret corev1.Secret
err := tc.MgmtClient.Get(context.Background(), crclient.ObjectKey{
Namespace: hc.Namespace,
Name: hc.Status.KubeConfig.Name,
}, &secret)
Comment on lines +90 to +98
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Tighten precondition checks before secret lookup.

The current guard conflates different failures and does not validate an empty kubeconfig secret name, which makes triage harder when this fails.

Suggested change
 		hc := tc.GetHostedCluster()
-		if hc == nil || hc.Status.KubeConfig == nil {
-			panic("HostedCluster has no kubeconfig in status")
+		if hc == nil {
+			panic("HostedCluster is not available in test context")
+		}
+		if hc.Status.KubeConfig == nil || hc.Status.KubeConfig.Name == "" {
+			panic("HostedCluster status kubeconfig secret reference is missing")
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/e2e/v2/internal/test_context.go` around lines 90 - 98, The precondition
check in test_context.go conflates a nil KubeConfig with a missing secret name;
update the guard around hc and hc.Status.KubeConfig (in the code surrounding
variable hc and the call to tc.MgmtClient.Get) to explicitly check and fail fast
with distinct messages: first panic if hc == nil, then panic if
hc.Status.KubeConfig == nil, then panic if hc.Status.KubeConfig.Name == ""
(empty string) before attempting the secret lookup via tc.MgmtClient.Get; ensure
the panic/log messages clearly mention which condition failed (nil
HostedCluster, nil KubeConfig, or empty KubeConfig.Name) to make triage easier.

if err != nil {
panic(fmt.Sprintf("failed to get kubeconfig secret: %v", err))
}

kubeconfigData, ok := secret.Data["kubeconfig"]
if !ok {
panic("kubeconfig secret does not contain 'kubeconfig' key")
}

restConfig, err := clientcmd.RESTConfigFromKubeConfig(kubeconfigData)
if err != nil {
panic(fmt.Sprintf("failed to parse guest kubeconfig: %v", err))
}
restConfig.QPS = -1
restConfig.Burst = -1

var guestClient crclient.Client
var lastErr error
err = wait.PollUntilContextTimeout(tc.Context, 5*time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) {
guestClient, err = e2eutil.GetClientFromConfig(restConfig)
if err != nil {
lastErr = fmt.Errorf("build guest client: %w", err)
return false, nil
}
_, apiErr := discovery.NewDiscoveryClientForConfigOrDie(restConfig).ServerVersion()
if apiErr != nil {
lastErr = fmt.Errorf("discover guest API server version: %w", apiErr)
return false, nil
}
return true, nil
})
if err != nil {
if lastErr != nil {
panic(fmt.Sprintf("failed to connect to guest cluster: %v (last error: %v)", err, lastErr))
}
panic(fmt.Sprintf("failed to connect to guest cluster: %v", err))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
tc.guestClient = guestClient
})
return tc.guestClient
}

var (
// Global test context - set in BeforeSuite
testCtx *TestContext
Expand Down