Fix nil pointer dereference panic in sidecar mounter#1285
Fix nil pointer dereference panic in sidecar mounter#1285alleaditya wants to merge 2 commits intoGoogleCloudPlatform:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alleaditya The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a nil check for the MountConfig object in cmd/sidecar_mounter/main.go to prevent potential panics when the configuration fails to initialize. The review feedback points out that while this addition is correct, it makes a subsequent check on the same object redundant, suggesting further cleanup to improve code clarity.
| time.Sleep(1500 * time.Millisecond) | ||
| mc := sidecarmounter.NewMountConfig(sp, flagsFromDriver) | ||
| if mc == nil { | ||
| klog.Errorf("failed to create mount config for %s", sp) |
There was a problem hiding this comment.
Should we write this error to the error file so node publish can report it to the customer? Otherwise I'm not sure what would happen if you just skip the mount, requires a small repro to find out.
2 participants
What type of PR is this?
/kind bug
What this PR does / why we need it:
This PR fixes a
nil pointer dereferencepanic in thegke-gcsfuse-sidecarcontainer. The panic occurs atcmd/sidecar_mounter/main.go:86whenmc(MountConfig) is accessed immediately afterNewMountConfigwithout checking if it isnil. The fix adds anilcheck to ensure safe execution.Which issue(s) this PR fixes:
Fixes (Internal Bug b/498411736)
Special notes for your reviewer:
https://b.corp.google.com/issues/498411736
Does this PR introduce a user-facing change?: