Skip to content

fix: restore missing batch/jobs permissions and add PVC permissions to executor role #553

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jul 10, 2025
Merged

fix: restore missing batch/jobs permissions and add PVC permissions to executor role #553

merged 4 commits into from
Jul 10, 2025

Conversation

@alexAtSourcegraph
Copy link
Contributor

@alexAtSourcegraph alexAtSourcegraph commented Sep 23, 2024 •
edited by enriquegh
Loading

Problem

Customer reported that batch changes are failing with the error:

running command "step.kubernetes.step.0.pre": persistentvolumeclaims is 
forbidden: User "system:serviceaccount:sourcegraph-test:sg-executor" cannot 
create resource "persistentvolumeclaims" in API group "" in the namespace 
"sourcegraph-test"

Solution

Add missing persistentvolumeclaims permissions (create/delete / get / list) that the executor needs for batch change job execution, based on customer's working role configuration.

Checklist

Test plan

@jdpleiness jdpleiness requested a review from a team April 23, 2025 13:02
@jdpleiness
Copy link
Contributor

@alexAtSourcegraph thanks for the PR! Would you mind filling in a description on what the intention of this PR is? We have some guidelines here for more info. Thanks 🙏

jdpleiness
jdpleiness requested changes May 7, 2025
View reviewed changes
Copy link
Contributor

@jdpleiness jdpleiness left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just a ping on this, I'm not really sure what the context of this PR is or what it's aimed at accomplishing. See #553 (comment)

@enriquegh enriquegh force-pushed the ajb-update-executors branch from 34ccc68 to d5e8f1e Compare July 10, 2025 14:55
@enriquegh
fix: restore missing batch/jobs permissions and add PVC permissions …
12a2d67 
…to executor role

    - Re-add batch/jobs create/delete permissions that were accidentally removed
    - Add persistentvolumeclaims create/delete permissions for job workspaces
    - Right-size pod permissions to get/list/watch only
    - Fixes customer-reported "cannot create resource persistentvolumeclaims" errors
    - Resolves batch changes functionality breaking due to missing permissions
@enriquegh enriquegh changed the title Ajb update executors fix: restore missing batch/jobs permissions and add PVC permissions to executor role Jul 10, 2025
@enriquegh
Copy link
Contributor

Hey @jdpleiness sorry for the lull on this. Helping Alex with this PR.

I added some context on what the PR is trying to accomplish. One of our customer's is having an issue when creating batch changes via executors.

Let me know if you have any thoughts/questions.

@enriquegh enriquegh requested a review from jdpleiness July 10, 2025 15:28
@enriquegh
Copy link
Contributor

If curious you can see the ticket (ask for Entitle Zendesk Light Agent access)
https://sourcegraph.zendesk.com/agent/tickets/19783

@jdpleiness Graphite App
Copy link
Contributor

Hey @jdpleiness sorry for the lull on this. Helping Alex with this PR.

I added some context on what the PR is trying to accomplish. One of our customer's is having an issue when creating batch changes via executors.

Let me know if you have any thoughts/questions.

Thanks for filling in the details 🙏

@enriquegh enriquegh merged commit 4031ec0 into main Jul 10, 2025
9 checks passed
@enriquegh enriquegh deleted the ajb-update-executors branch July 10, 2025 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdpleiness jdpleiness jdpleiness approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alexAtSourcegraph @jdpleiness @enriquegh