Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panic: slice bounds out of range when runner spec contains volumeMounts. #2720

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

fatmcgav
Copy link
Contributor

@fatmcgav fatmcgav commented Jul 5, 2023

As observed in #2719, if a Runner contains a work volume mount, this
will cause a panic in newPod() where the code is assuming that the
work volume mount will be present in the pod definition generated by
newRunnerPodWithContainerMode().

However that isn't always the case, and if it's not present the -1
returned by workVolumeMountPresent results in the slice bounds out of range exception.

So instead, check the bool return value, and only attempt to remove
the work volume mount if it's actually present.

Also apply the same fix to the volumes logic.

Fixes #2719

Signed-off-by: Gavin Williams gavin.williams@machinemax.com

…umeMounts`.

As observed in actions#2719, if a `Runner` contains a `work` volume mount, this
will cause a panic in `newPod()` where the code is assuming that the
`work` volume mount will be present in the `pod` definition generated by
`newRunnerPodWithContainerMode()`.

However that isn't always the case, and if it's not present the `-1`
returned by `workVolumeMountPresent` results in the `slice bounds out of
range` exception.

So instead, check the `bool` return value, and only attempt to remove
the `work` volume mount if it's actually present.

Also apply the same fix to the `volumes` logic.

Fixes actions#2719

Signed-off-by: Gavin Williams <gavin.williams@machinemax.com>
Copy link
Collaborator

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

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

Ah, good catch! Thank you very much for your contribution @fatmcgav!

@mumoshu mumoshu merged commit cd996e7 into actions:master Jul 25, 2023
14 of 15 checks passed
Link- pushed a commit that referenced this pull request Jul 26, 2023
…umeMounts`. (#2720)

Signed-off-by: Gavin Williams <gavin.williams@machinemax.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic if RunnerDeployment contains volumeMounts
2 participants