[0.14] Fix nil dereference for cluster import token #4510
Merged
+17
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When creating a cluster import token fails, the agent management controller is no longer vulnerable to crashes.
weyfonk
force-pushed
the
0.14-import-token-fix-nil-dereference
branch
from
b09efcc to
dc70e8f
Compare
aruiz14
approved these changes
Jan 9, 2026
Comment on lines
+378
to
+391
| if err != nil { | ||
| if apierrors.IsAlreadyExists(err) { | ||
| token, err = i.tokens.Get(cluster.Namespace, tokenName) | ||
| if err != nil { | ||
| logrus.Debugf("Failed to get existing ClusterRegistrationToken for cluster %s/%s: %v (requeuing)", cluster.Namespace, cluster.Name, err) | ||
| i.clusters.EnqueueAfter(cluster.Namespace, cluster.Name, durations.TokenClusterEnqueueDelay) | ||
| return status, err | ||
| } | ||
| } else { | ||
| logrus.Debugf("Failed to create ClusterRegistrationToken for cluster %s/%s: %v (requeuing)", cluster.Namespace, cluster.Name, err) | ||
| i.clusters.EnqueueAfter(cluster.Namespace, cluster.Name, durations.TokenClusterEnqueueDelay) | ||
| return status, err | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
nitpick about style (I don't like too many nested if-else):
Suggested change
| if err != nil { | |
| if apierrors.IsAlreadyExists(err) { | |
| token, err = i.tokens.Get(cluster.Namespace, tokenName) | |
| if err != nil { | |
| logrus.Debugf("Failed to get existing ClusterRegistrationToken for cluster %s/%s: %v (requeuing)", cluster.Namespace, cluster.Name, err) | |
| i.clusters.EnqueueAfter(cluster.Namespace, cluster.Name, durations.TokenClusterEnqueueDelay) | |
| return status, err | |
| } | |
| } else { | |
| logrus.Debugf("Failed to create ClusterRegistrationToken for cluster %s/%s: %v (requeuing)", cluster.Namespace, cluster.Name, err) | |
| i.clusters.EnqueueAfter(cluster.Namespace, cluster.Name, durations.TokenClusterEnqueueDelay) | |
| return status, err | |
| } | |
| } | |
| if apierrors.IsAlreadyExists(err) { | |
| token, err = i.tokens.Get(cluster.Namespace, tokenName) | |
| if err != nil { | |
| logrus.Debugf("Failed to get existing ClusterRegistrationToken for cluster %s/%s: %v (requeuing)", cluster.Namespace, cluster.Name, err) | |
| i.clusters.EnqueueAfter(cluster.Namespace, cluster.Name, durations.TokenClusterEnqueueDelay) | |
| return status, err | |
| } | |
| } else if err != nil { | |
| logrus.Debugf("Failed to create ClusterRegistrationToken for cluster %s/%s: %v (requeuing)", cluster.Namespace, cluster.Name, err) | |
| i.clusters.EnqueueAfter(cluster.Namespace, cluster.Name, durations.TokenClusterEnqueueDelay) | |
| return status, err | |
| } |
Contributor
There was a problem hiding this comment.
I missed that this is a backport and not the original PR, nevermind then 😅
Contributor
Author
There was a problem hiding this comment.
Yes, I agree that the style is a bit less than ideal, but this is indeed a backport :)
thardeck
approved these changes
Jan 9, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes a critical nil pointer dereference bug in the cluster import token creation logic. When token creation failed, the previous code discarded the result and continued execution with a nil token variable, causing crashes when the token was later accessed.
Key Changes:
- Properly capture the created token object instead of discarding it
- Add comprehensive error handling for token creation failures, including race condition handling for AlreadyExists errors
- Return errors appropriately instead of silently ignoring them and continuing with nil token
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When creating a cluster import token fails, the agent management controller is no longer vulnerable to crashes.
Refers to #4491.
Backport of #4494 to v0.14.
Additional Information
Checklist
- [ ] I have updated the documentation via a pull request in thefleet-docs repository.