Skip to content

fix: push-mode informers pick up rotated bearer token without restart#7663

Open
zhuyulicfc49 wants to merge 1 commit into
karmada-io:masterfrom
zhuyulicfc49:fix/informer-token-rotation
Open

fix: push-mode informers pick up rotated bearer token without restart#7663
zhuyulicfc49 wants to merge 1 commit into
karmada-io:masterfrom
zhuyulicfc49:fix/informer-token-rotation

Conversation

@zhuyulicfc49

@zhuyulicfc49 zhuyulicfc49 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind bug

What this PR does / why we need it:

Push-mode informers hold a frozen bearer token from startup. After rotation, long-lived connections (resource watches) silently break while short-lived connections (health checks) keep working — the cluster shows Ready=True but is partially blind.

Fix: Remove the static BearerToken from BuildClusterConfig(). Add a WrapTransport that re-reads the token from the Secret on a 30s TTL — so long-lived clients pick up rotated credentials without a restart.

Scope:

BuildClusterConfig() is the single entry point for all push-mode member cluster connections. The fix covers both categories:

Category Controllers Impact of fix
Long-lived (informers — resource watches; broken without fix) WorkStatusController, ExecutionController (informer cache), EndpointSliceCollectController, FederatedHPAController, ServiceExportController Token refreshes on TTL → watches reconnect with valid credential → fixed
Short-lived (fresh client each cycle; already work) ClusterStatusController (health checks), ObjectWatcher (work execution) Also goes through the RoundTripper now — harmless, consistent

Backward compatibility: Static-token users see no behavior change — the RoundTripper re-reads the same value every 30s.

Which issue(s) this PR fixes:

Fixes #7562

Special notes for your reviewer:

Testing

Strategy:

I approached this using a shift-left mindset: first design an end-to-end scenario that fails before the fix (RED), then verify the same scenario passes after the fix (GREEN). For this issue, I also wanted the test to validate the suspected root cause by contrasting Karmada's two connection types: before the fix, token expiry should leave short-lived requests working while long-lived informers become stale; after the fix, both should continue working without a restart.

Once the manual RED/GREEN behavior was validated, I converted a similar scenario into a permanent e2e regression test for CI. The automated version uses token revocation and a Docker restart instead of waiting for natural token expiry, reducing the runtime from ~20 minutes to ~65 seconds while preserving the same coverage.

The diagram below shows the test breakdown (T1 = old 10-min token, T2 = new token):

  1. Token rotation — write T2 to Secret; informer still reads expired T1 (before fix) or reads T2 (after fix)
  2. Scale replicas — trigger a state change on the member cluster
  3. Checkpoint on collected replica number — before fix it returns the cached (stale) value; after fix it correctly returns the new value
image

Manual test results

Manual RED (without fix) — informer goes blind after natural token expiry

Environment: local-up-karmada cluster (kind-based).

Helpers: kcp=karmada-apiserver, kh=karmada-host, km1=member1 direct. The key one:

# proxy_ready: queries member1 THROUGH Karmada's aggregated proxy (short-lived path,
# always reads fresh token). This gives the LIVE truth of what's running on member1.
proxy_ready() {
  kcp get --raw '/apis/cluster.karmada.io/v1alpha1/clusters/member1/proxy/apis/apps/v1/namespaces/default/deployments/nginx' \
    | python3 -c 'import sys,json; print(json.load(sys.stdin)["status"].get("readyReplicas"))'
}

Setup:

  1. Propagate a 2-replica nginx to member1
  2. Mint a 600s token (T_short), write it to the Secret, restart CM so the informer builds holding T_short
  3. Write a fresh 24h token (T_long) to the Secret (simulates rotation)
  4. Wait for T_short to naturally expire (~10 min)

Baseline (both paths healthy, before expiry):

$ echo "LIVE (proxy/short):        $(proxy_ready)"
  echo "COLLECTED (informer/long): $(kcp get deploy nginx -o jsonpath='{.status.readyReplicas}')"
LIVE (proxy/short):        2
COLLECTED (informer/long): 2

Wait for T_short to expire (~10 min):

$ for i in $(seq 1 40); do
    c=$(curl -sk -o /dev/null -w '%{http_code}' -H "Authorization: Bearer $(cat /tmp/tshort.jwt)" "$M1URL/api/v1/nodes")
    echo "$(date '+%H:%M:%S') T_short->$c"
    [ "$c" = 401 ] && { echo ">> T_short EXPIRED"; break; }; sleep 20
  done
02:56:14 T_short->200
02:56:34 T_short->200
02:56:54 T_short->200
02:57:14 T_short->200
02:57:34 T_short->200
02:57:54 T_short->401
>> T_short EXPIRED

Watch naturally re-establishes with expired token → sustained Unauthorized:

$ kh -n karmada-system logs -l app=karmada-controller-manager --prefix -f --tail=0 \
    | grep --line-buffered -iE 'Failed to watch.*(Pod|Node|deployment|Unauthorized)'
"Failed to watch" type="*v1.Pod"  err="failed to list *v1.Pod: Unauthorized"
"Failed to watch" type="rbac.../clusterrolebindings" err="...: Unauthorized"
"Failed to watch" type="rbac.../clusterroles"        err="...: Unauthorized"
"Failed to watch" type="apps/v1, Resource=deployments" err="...: Unauthorized"
"Failed to watch" type="*v1.Node" err="failed to list *v1.Node: Unauthorized"

Cluster still shows Ready (silent failure — health checks use the short-lived path):

$ echo "member1 Ready = $(kcp get cluster member1 -o jsonpath='{.status.conditions[?(@.type=="Ready")].status}')"
member1 Ready = True

Scale on member1 directly, then compare the two views:

$ km1 scale deployment nginx --replicas=4; sleep 8
  echo "LIVE (proxy/short):        $(proxy_ready)"
  echo "COLLECTED (informer/long): $(kcp get deploy nginx -o jsonpath='{.status.readyReplicas}')"
LIVE (proxy/short):        4
COLLECTED (informer/long): 2

Verdict: LIVE=4, COLLECTED=2 → DIVERGED — the informer is frozen on the expired token. Karmada reports Ready=True but is blind to member state.

Manual GREEN (with fix) — informer recovers after natural token expiry

Same setup, same natural re-watch, same cluster (rebuilt fresh with fix image). The CM was never restarted after token expiry — recovery is purely via the RoundTripper.

Baseline (before expiry):

$ echo "LIVE (proxy/short):        $(proxy_ready)"
  echo "COLLECTED (informer/long): $(kcp get deploy nginx -o jsonpath='{.status.readyReplicas}')"
LIVE (proxy/short):        2
COLLECTED (informer/long): 2

T_short expires at 04:03:57 (same ~10 min wait). Then monitor for auth failures:

$ for i in $(seq 1 30); do
    ue=$(kh -n karmada-system logs -l app=karmada-controller-manager --since=25m 2>/dev/null \
      | grep -c 'Failed to watch.*Unauthorized')
    echo "$(date '+%H:%M:%S') cumulative watch-Unauthorized=$ue"
    sleep 30
  done
04:08:10 cumulative watch-Unauthorized=0
04:08:40 cumulative watch-Unauthorized=0
...
04:22:41 cumulative watch-Unauthorized=0

Zero auth failures across 14.5 minutes post-expiry (vs RED which 401s within ~2 min).

Informer healthy — no sustained errors:

$ echo "member1 Ready = $(kcp get cluster member1 -o jsonpath='{.status.conditions[?(@.type=="Ready")].status}')"
  echo "watch-errs in last 150 lines = $(kh -n karmada-system logs -l app=karmada-controller-manager --tail=150 2>/dev/null \
    | grep -ic 'Failed to watch.*Unauthorized')"
member1 Ready = True
watch-errs in last 150 lines = 0

Scale through Karmada and confirm the informer is actively collecting:

$ kcp scale deployment nginx --replicas=4
  for i in $(seq 1 24); do
    L=$(proxy_ready); C=$(kcp get deploy nginx -o jsonpath='{.status.readyReplicas}')
    echo "$(date '+%H:%M:%S') LIVE(proxy)=$L  COLLECTED(informer)=$C"
    [ "$L" = "4" ] && [ "$C" = "4" ] && { echo ">> CONVERGED"; break; }; sleep 5
  done
04:27:05 LIVE(proxy)=2  COLLECTED(informer)=2
04:27:10 LIVE(proxy)=4  COLLECTED(informer)=4
>> CONVERGED

Verdict: LIVE=4, COLLECTED=4 → CONVERGED — the RoundTripper supplied T_long on the natural re-watch. The informer recovered without any restart.

Test coverage

Type What it proves
Unit (6) Token injection, TTL refresh, error fallback, empty token guard, proxy chaining, concurrent deduplication
Integration (1) Full BuildClusterConfig → client → rotated token appears on wire
E2E (1, ~65s) End-to-end informer recovery after rotation (requires docker)
Manual (RED + GREEN) Natural token expiry on real cluster (above)

Does this PR introduce a user-facing change?:

`karmada-controller-manager`: Fixed push-mode informers silently breaking after member cluster token rotation. Long-lived resource watches now pick up rotated credentials automatically via a token-refreshing transport layer.

Copilot AI review requested due to automatic review settings June 24, 2026 17:20
@karmada-bot karmada-bot added the kind/bug Categorizes issue or PR as related to a bug. label Jun 24, 2026
@karmada-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign xishanyongye-chang for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@karmada-bot karmada-bot requested a review from zhzhuang-zju June 24, 2026 17:21
@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical issue where push-mode informers in Karmada would stop functioning after a member cluster's bearer token was rotated, as they held a stale, frozen token from startup. By implementing a token-refreshing transport layer, the system now ensures that long-lived connections periodically re-fetch credentials, allowing for seamless rotation and preventing silent failures in cluster monitoring.

Highlights

  • Token Refresh Mechanism: Introduced a tokenRefreshingRoundTripper that dynamically re-reads the bearer token from the associated Secret on a 30s TTL, ensuring long-lived connections (like informers) automatically pick up rotated credentials.
  • Configuration Update: Modified BuildClusterConfig to remove the static BearerToken and instead inject the new token-refreshing transport layer, maintaining backward compatibility for static-token users.
  • Testing Coverage: Added comprehensive unit tests for the token-refreshing logic and a new E2E test suite to verify that informers correctly recover after member cluster token rotation without requiring a controller manager restart.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@zhuyulicfc49 zhuyulicfc49 marked this pull request as draft June 24, 2026 17:21
@karmada-bot karmada-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jun 24, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to handle member cluster token rotation for long-lived clients by implementing a TTL-based tokenRefreshingRoundTripper that periodically re-reads the token from the Karmada Secret. It also adds comprehensive unit and E2E tests to verify this behavior. The reviewer's feedback suggests optimizing the locking mechanism in tokenRefreshingRoundTripper to prevent head-of-line blocking during network I/O, adding defensive nil checks to avoid potential panics, and formatting a function signature on a single line to adhere to the repository style guide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +80 to +89
type tokenRefreshingRoundTripper struct {
inner http.RoundTripper
secretGetter func(namespace, name string) (*corev1.Secret, error)
secretNS string
secretName string

mu sync.RWMutex
cachedToken string
cacheExpiry time.Time
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

To prevent head-of-line blocking for all concurrent requests when the token expires, we should avoid holding the read/write lock (t.mu) during the network I/O call to fetch the secret. Adding a separate fetchMu mutex allows us to synchronize the token fetching process independently of token reading.

Suggested change
type tokenRefreshingRoundTripper struct {
inner http.RoundTripper
secretGetter func(namespace, name string) (*corev1.Secret, error)
secretNS string
secretName string
mu sync.RWMutex
cachedToken string
cacheExpiry time.Time
}
type tokenRefreshingRoundTripper struct {
inner http.RoundTripper
secretGetter func(namespace, name string) (*corev1.Secret, error)
secretNS string
secretName string
mu sync.RWMutex
cachedToken string
cacheExpiry time.Time
fetchMu sync.Mutex
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a good point! But I feel secretGetter reads from the controller-runtime in-memory informer cache, which is a map lookup and already fast enough(~100ns). And the write lock is held for that duration only, so concurrent requests are not meaningfully blocked I feel

Comment on lines +131 to +152
func (t *tokenRefreshingRoundTripper) forceRefresh() string {
t.mu.Lock()
defer t.mu.Unlock()

if time.Now().Before(t.cacheExpiry) {
return t.cachedToken
}

secret, err := t.secretGetter(t.secretNS, t.secretName)
if err != nil {
klog.Warningf("tokenRefreshingRoundTripper: failed to refresh token from secret %s/%s, keeping last known token: %v",
t.secretNS, t.secretName, err)
t.cacheExpiry = time.Now().Add(tokenCacheTTL)
return t.cachedToken
}

if token := string(secret.Data[clusterv1alpha1.SecretTokenKey]); token != "" {
t.cachedToken = token
}
t.cacheExpiry = time.Now().Add(tokenCacheTTL)
return t.cachedToken
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Currently, forceRefresh holds the write lock t.mu during the entire network I/O call (t.secretGetter). This blocks all concurrent requests (even those just reading the token) for the duration of the API call.

By using t.fetchMu.TryLock(), we can ensure that only one goroutine performs the network I/O while all other concurrent requests immediately fall back to using the cached token instead of blocking. Additionally, we should add defensive checks to ensure secret and secret.Data are not nil before accessing them to prevent potential nil pointer dereference panics. Also, please add a doc comment to this unexported function to improve maintainability.

// forceRefresh handles the token refresh logic by fetching the secret and updating the cached token.
func (t *tokenRefreshingRoundTripper) forceRefresh() string {
	if !t.fetchMu.TryLock() {
		t.mu.RLock()
		token := t.cachedToken
		t.mu.RUnlock()
		return token
	}
	defer t.fetchMu.Unlock()

	t.mu.RLock()
	if time.Now().Before(t.cacheExpiry) {
		token := t.cachedToken
		t.mu.RUnlock()
		return token
	}
	t.mu.RUnlock()

	secret, err := t.secretGetter(t.secretNS, t.secretName)

	t.mu.Lock()
	defer t.mu.Unlock()

	if err != nil {
		klog.Warningf("tokenRefreshingRoundTripper: failed to refresh token from secret %s/%s, keeping last known token: %v",
			t.secretNS, t.secretName, err)
		t.cacheExpiry = time.Now().Add(tokenCacheTTL)
		return t.cachedToken
	}

	if secret != nil && secret.Data != nil {
		if token := string(secret.Data[clusterv1alpha1.SecretTokenKey]); token != "" {
			t.cachedToken = token
		}
	}
	t.cacheExpiry = time.Now().Add(tokenCacheTTL)
	return t.cachedToken
}
References
  1. Add doc comments to unexported functions that contain key logic to improve maintainability.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same reason as above I feel

Comment on lines +97 to +100
func NewTokenRefreshingRoundTripperWrapperConstructor(
secretGetter func(string, string) (*corev1.Secret, error),
secretNS, secretName, initialToken string,
) transport.WrapperFunc {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

According to the repository style guide, function signatures should preferably be written on a single line, including the parameter list and return types.

func NewTokenRefreshingRoundTripperWrapperConstructor(secretGetter func(string, string) (*corev1.Secret, error), secretNS, secretName, initialToken string) transport.WrapperFunc {
References
  1. Function signatures should preferably be written on a single line, including the parameter list and return types. (link)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure if I should apply this... the existing NewProxyHeaderRoundTripperWrapperConstructor in the same file uses the same multi-line format. And no linting error with multi-line format..

@zhuyulicfc49 zhuyulicfc49 force-pushed the fix/informer-token-rotation branch from abf0414 to a8091a4 Compare June 24, 2026 17:24

Copilot AI left a comment

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.

Pull request overview

This PR fixes push-mode member cluster clients (notably informer-backed long-lived watches) holding a stale bearer token after Secret rotation by moving token injection out of rest.Config.BearerToken and into a transport wrapper that periodically re-reads the token from the Secret (TTL-based refresh).

Changes:

  • Removed the static BearerToken field from BuildClusterConfig() and added a token-refreshing WrapTransport layer.
  • Introduced tokenRefreshingRoundTripper with a 30s cache TTL to refresh the bearer token from the member cluster Secret.
  • Added unit tests for the round tripper behavior, an integration-style test for BuildClusterConfig() token rotation, and an e2e test covering informer recovery after rotation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/e2e/suites/base/token_rotation_test.go New e2e covering token rotation + forced watch reconnection and validating informer-based collection recovers.
pkg/util/round_trippers.go Adds the TTL-based token-refreshing http.RoundTripper wrapper used to inject updated bearer tokens.
pkg/util/round_trippers_token_test.go Unit tests for token injection, TTL refresh, error handling, chaining, and concurrency dedup.
pkg/util/membercluster_client.go Removes static bearer token config and wires the new token-refreshing wrapper into cluster client config construction.
pkg/util/membercluster_client_rotation_test.go Integration-style test validating a long-lived client created via BuildClusterConfig() picks up a rotated Secret token without rebuild.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +99 to +100
saNamespace, saName := parseSAFromToken(string(secret.Data["token"]))
klog.Infof("cluster %s uses SA %s/%s on the member", targetCluster, saNamespace, saName)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed both:

  • Added gomega.Expect(saName).ShouldNot(gomega.BeEmpty(), "cluster token must be a parseable SA JWT") right after parseSAFromToken so the test fails early with a clear message.
  • Replaced the literal "token" with clusterv1alpha1.SecretTokenKey in both places.

@codecov-commenter

codecov-commenter commented Jun 24, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 97.91667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 42.10%. Comparing base (7edc6e6) to head (0f1a5ea).
⚠️ Report is 6 commits behind head on master.

Files with missing lines Patch % Lines
pkg/util/round_trippers.go 97.50% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7663      +/-   ##
==========================================
+ Coverage   42.04%   42.10%   +0.05%     
==========================================
  Files         879      879              
  Lines       54827    54872      +45     
==========================================
+ Hits        23053    23103      +50     
+ Misses      30027    30024       -3     
+ Partials     1747     1745       -2     
Flag Coverage Δ
unittests 42.10% <97.91%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zhuyulicfc49 zhuyulicfc49 force-pushed the fix/informer-token-rotation branch 5 times, most recently from 910aa4a to b684d46 Compare June 25, 2026 02:02
Signed-off-by: zhuyulicfc49 <zyliw49@gmail.com>
@zhuyulicfc49 zhuyulicfc49 force-pushed the fix/informer-token-rotation branch from b684d46 to 0f1a5ea Compare June 25, 2026 02:14
@zhuyulicfc49 zhuyulicfc49 marked this pull request as ready for review June 25, 2026 03:12
@karmada-bot karmada-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 25, 2026
tokenRotatePollRate = 5 * time.Millisecond
)

func TestBuildClusterConfig_LongLivedClientPicksUpRotatedToken(t *testing.T) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

confirmed it fails without the fix :)

--- FAIL: TestBuildClusterConfig_LongLivedClientPicksUpRotatedToken (0.04s)
  membercluster_client_rotation_test.go:112:
    Not equal:
      expected: "Bearer token-B"
      actual  : "Bearer token-A"
    Messages: the long-lived client must put the ROTATED token on the wire after rotation
  membercluster_client_rotation_test.go:116:
    Received unexpected error: the server has asked for the client to provide credentials (get nodes foo)
    Messages: after rotation the long-lived client must keep working (pick up token-B without a rebuild)
FAIL  github.com/karmada-io/karmada/pkg/util  0.409s

// - Token revocation (instant) instead of waiting for natural token expiry.
// - Docker restart to force watch reconnection — an open watch survives
// revocation since the API server only re-validates auth on reconnection.
var _ = framework.SerialDescribe("push-mode token rotation", func() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. based on the CI logs the time spent of this test is TODO
  2. confirmed it failed without the fix
E2E Suite
Running Suite: E2E Suite - /home/ubuntu/projects/karmada/test/e2e/suites/base
=============================================================================
Random Seed: 1782267580

Will run 1 of 274 specs
------------------------------
[SynchronizedBeforeSuite] 
/home/ubuntu/projects/karmada/test/e2e/suites/base/suite_test.go:142
I0624 02:19:52.538719  182298 cluster.go:332] Use context karmada-host
I0624 02:19:52.540695  182298 cluster.go:332] Use context karmada-apiserver
I0624 02:19:52.571884  182298 cluster.go:198] Got 3 member cluster and all in ready state.
I0624 02:19:52.668807  182298 cluster.go:332] Use context member3
  STEP: Check if namespace(karmadatest-clpl4) present on member clusters @ 06/24/26 02:19:52.7
I0624 02:19:52.700444  182298 namespace.go:63] Waiting for namespace present on cluster(member1)
I0624 02:19:57.721375  182298 namespace.go:63] Waiting for namespace present on cluster(member2)
I0624 02:19:57.727812  182298 namespace.go:63] Waiting for namespace present on cluster(member3)
[SynchronizedBeforeSuite] PASSED [5.224 seconds]
------------------------------
[SSS… 273 specs skipped …SSS]
------------------------------
push-mode token rotation informers recover after member cluster token rotates [Serial]
/home/ubuntu/projects/karmada/test/e2e/suites/base/token_rotation_test.go:86
  STEP: Creating PropagationPolicy(karmadatest-clpl4/deploy-5gzc4) @ 06/24/26 02:19:57.747
  STEP: Creating Deployment(karmadatest-clpl4/deploy-5gzc4) @ 06/24/26 02:19:57.762
  STEP: 1. verify workload propagated and status collected (baseline) @ 06/24/26 02:19:57.77
I0624 02:19:57.770902  182298 deployment.go:91] Waiting for deployment(karmadatest-clpl4/deploy-5gzc4) synced on cluster(member1)
E0624 02:19:57.773685  182298 deployment.go:95] Failed to get Deployment(karmadatest-clpl4/deploy-5gzc4) on cluster(member1), err: deployments.apps "deploy-5gzc4" not found
I0624 02:20:07.787185  182298 token_rotation_test.go:92] baseline: deployment karmadatest-clpl4/deploy-5gzc4 present on member1 with ready replicas
I0624 02:20:07.795420  182298 token_rotation_test.go:106] cluster member1 uses SA karmada-cluster/karmada-member1 on the member
  STEP: 2. rotate: revoke old token, mint new, write to Secret @ 06/24/26 02:20:07.795
I0624 02:20:07.804461  182298 token_rotation_test.go:120] revoked old tokens by recreating SA karmada-cluster/karmada-member1
I0624 02:20:07.809051  182298 token_rotation_test.go:132] minted new token bound to recreated SA karmada-cluster/karmada-member1
I0624 02:20:07.816946  182298 token_rotation_test.go:137] wrote rotated token to Secret karmada-cluster/member1
I0624 02:20:07.816970  182298 token_rotation_test.go:140] waiting for revocation cache to flush...
  STEP: 3. force watch reconnection (docker restart member node) @ 06/24/26 02:20:22.829
I0624 02:20:22.829856  182298 token_rotation_test.go:148] restarting kind container member1-control-plane to force watch reconnection
I0624 02:20:33.416766  182298 token_rotation_test.go:156] container member1-control-plane restarted
I0624 02:20:33.425924  182298 token_rotation_test.go:172] cluster Ready again — watch must now re-establish with the (potentially stale) token
  STEP: 4. verify short-lived path still works (cluster stays Ready) @ 06/24/26 02:20:33.425
I0624 02:21:03.426409  182298 token_rotation_test.go:189] short-lived path: cluster stayed Ready throughout
  STEP: 5. verify long-lived path recovered (informer sees member changes) @ 06/24/26 02:21:03.426
I0624 02:21:03.438838  182298 token_rotation_test.go:202] scaled deployment to 5 via Karmada control plane
I0624 02:21:03.442966  182298 token_rotation_test.go:212] karmada collected readyReplicas=3 (target 5)
I0624 02:21:08.450480  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:21:13.458580  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:21:18.466034  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:21:23.472915  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:21:28.477046  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:21:33.485613  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:21:38.492077  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:21:43.498345  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:21:48.505572  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:21:53.512496  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:21:58.517130  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:03.520879  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:08.526643  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:13.534435  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:18.547977  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:23.554826  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:28.558937  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:33.562834  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:38.570023  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:43.579846  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:48.587145  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:53.593676  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
I0624 02:22:58.597876  182298 token_rotation_test.go:212] karmada collected readyReplicas=0 (target 5)
  [FAILED] in [It] - /home/ubuntu/projects/karmada/test/e2e/suites/base/token_rotation_test.go:215 @ 06/24/26 02:23:03.442
  STEP: Removing Deployment(karmadatest-clpl4/deploy-5gzc4) @ 06/24/26 02:23:03.464
  STEP: Removing PropagationPolicy(karmadatest-clpl4/deploy-5gzc4) @ 06/24/26 02:23:03.472
• [FAILED] [185.734 seconds]
push-mode token rotation [It] informers recover after member cluster token rotates [Serial]
/home/ubuntu/projects/karmada/test/e2e/suites/base/token_rotation_test.go:86

  [FAILED] Timed out after 120.003s.
  Karmada must observe the scaled status after rotation (informer recovered)
  Expected
      <bool>: false
  to be true
  In [It] at: /home/ubuntu/projects/karmada/test/e2e/suites/base/token_rotation_test.go:215 @ 06/24/26 02:23:03.442

  Full Stack Trace
    github.com/karmada-io/karmada/test/e2e/suites/base.init.func82.3.5()
    	/home/ubuntu/projects/karmada/test/e2e/suites/base/token_rotation_test.go:215 +0x3be
    github.com/karmada-io/karmada/test/e2e/suites/base.init.func82.3()
    	/home/ubuntu/projects/karmada/test/e2e/suites/base/token_rotation_test.go:192 +0x6d5
------------------------------
[SSS… 38 specs skipped …SSS]
------------------------------
[SynchronizedAfterSuite] 
/home/ubuntu/projects/karmada/test/e2e/suites/base/suite_test.go:203
[SynchronizedAfterSuite] PASSED [0.182 seconds]
------------------------------

Summarizing 1 Failure:
  [FAIL] push-mode token rotation [It] informers recover after member cluster token rotates [Serial]
  /home/ubuntu/projects/karmada/test/e2e/suites/base/token_rotation_test.go:215

Ran 1 of 274 Specs in 191.152 seconds
FAIL! -- 0 Passed | 1 Failed | 0 Pending | 273 Skipped
--- FAIL: TestE2E (191.17s)
FAIL

Ginkgo ran 1 suite in 3m23.033745281s

Test Suite Failed
exit status 1
GINKGO_EXIT:1

Comment on lines +97 to +100
func NewTokenRefreshingRoundTripperWrapperConstructor(
secretGetter func(string, string) (*corev1.Secret, error),
secretNS, secretName, initialToken string,
) transport.WrapperFunc {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure if I should apply this... the existing NewProxyHeaderRoundTripperWrapperConstructor in the same file uses the same multi-line format. And no linting error with multi-line format..

Comment on lines +80 to +89
type tokenRefreshingRoundTripper struct {
inner http.RoundTripper
secretGetter func(namespace, name string) (*corev1.Secret, error)
secretNS string
secretName string

mu sync.RWMutex
cachedToken string
cacheExpiry time.Time
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a good point! But I feel secretGetter reads from the controller-runtime in-memory informer cache, which is a map lookup and already fast enough(~100ns). And the write lock is held for that duration only, so concurrent requests are not meaningfully blocked I feel

Comment on lines +131 to +152
func (t *tokenRefreshingRoundTripper) forceRefresh() string {
t.mu.Lock()
defer t.mu.Unlock()

if time.Now().Before(t.cacheExpiry) {
return t.cachedToken
}

secret, err := t.secretGetter(t.secretNS, t.secretName)
if err != nil {
klog.Warningf("tokenRefreshingRoundTripper: failed to refresh token from secret %s/%s, keeping last known token: %v",
t.secretNS, t.secretName, err)
t.cacheExpiry = time.Now().Add(tokenCacheTTL)
return t.cachedToken
}

if token := string(secret.Data[clusterv1alpha1.SecretTokenKey]); token != "" {
t.cachedToken = token
}
t.cacheExpiry = time.Now().Add(tokenCacheTTL)
return t.cachedToken
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same reason as above I feel

Comment on lines +99 to +100
saNamespace, saName := parseSAFromToken(string(secret.Data["token"]))
klog.Infof("cluster %s uses SA %s/%s on the member", targetCluster, saNamespace, saName)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — fixed both:

  • Added gomega.Expect(saName).ShouldNot(gomega.BeEmpty(), "cluster token must be a parseable SA JWT") right after parseSAFromToken so the test fails early with a clear message.
  • Replaced the literal "token" with clusterv1alpha1.SecretTokenKey in both places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Categorizes issue or PR as related to a bug. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Push-mode member cluster informers break permanently after token rotation

4 participants