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(kubernetes-plugin): sanitize volumes configuration for helm and kubernetes type pod runners #6251

Merged
merged 15 commits into from
Jul 5, 2024

Conversation

twelvemo
Copy link
Collaborator

@twelvemo twelvemo commented Jul 2, 2024

What this PR does / why we need it:

The kubernetes-type and helm pod runners for Run and Test actions are using the podSpec from a Kubernetes manifest or helm chart if a resource is specified. If these resources e.g. a statefulSet include volumes, these pods may fail to be deployed, because the volumes are already attached (most volumes are read-write-once).

We therefore sanitize the podSpec by removing all volumes except configMaps and secrets which may be crucial for the pod to contain the correct configuration or credentials to run. These volume types can also be mounted several times and are less likely to cause issues.

In case of configMaps we also make sure that the defaultMode of the file permission is in octal.

If the podSpec is explicitly configured in a kubernetes-pod we keep all kinds of volumes, even persistentVolumeClaims since the user has explicitly configured them and can make sure they exist and can be mounted. This PR only removes persistentVolumeClaim type volumes if they are implicitly set through a helm chart or Kubernetes manifest.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Volume mounts in pod runners were allowed in #6112. We might need to re-visit those changes and apply the necessary amendments and restrictions.

@twelvemo twelvemo marked this pull request as draft July 2, 2024 16:04
@twelvemo twelvemo force-pushed the fix-helm-pod-runner branch 4 times, most recently from 872f8fb to 7b49c00 Compare July 4, 2024 12:30
@vvagaytsev vvagaytsev force-pushed the fix-helm-pod-runner branch from 7b49c00 to 12f1a33 Compare July 4, 2024 15:44
@vvagaytsev vvagaytsev force-pushed the fix-helm-pod-runner branch from 3347cb4 to 2e58c54 Compare July 5, 2024 10:36
@vvagaytsev vvagaytsev self-requested a review July 5, 2024 10:56
@vvagaytsev vvagaytsev force-pushed the fix-helm-pod-runner branch from d52d194 to 4bd7da8 Compare July 5, 2024 11:56
@twelvemo twelvemo marked this pull request as ready for review July 5, 2024 14:08
@twelvemo twelvemo enabled auto-merge July 5, 2024 14:27
@twelvemo twelvemo added this pull request to the merge queue Jul 5, 2024
Merged via the queue into main with commit 0a12df4 Jul 5, 2024
41 checks passed
@twelvemo twelvemo deleted the fix-helm-pod-runner branch July 5, 2024 15:04
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.

None yet

2 participants