dongjoon-hyun commented on code in PR #827:
URL:
https://github.com/apache/spark-kubernetes-operator/pull/827#discussion_r4017177655
##########
.github/workflows/build_and_test.yml:
##########
@@ -277,6 +285,12 @@ jobs:
build-tools/helm/spark-kubernetes-operator/
minikube docker-env --unset
helm test spark
+ - name: Verify Kueue RBAC is denied by default
+ if: matrix.test-group == 'kueue'
+ run: |
+ helm upgrade spark -f
build-tools/helm/spark-kubernetes-operator/values.yaml \
+ build-tools/helm/spark-kubernetes-operator/
+ if kubectl auth can-i create workloads.kueue.x-k8s.io
--as=system:serviceaccount:default:spark-operator; then exit 1; fi
Review Comment:
Done in e53b287. The same impersonated `kubectl auth can-i create
workloads.kueue.x-k8s.io --as=...` now runs before the `helm upgrade` and must
answer "yes", so a wrong subject fails there instead of making the denial
vacuous.
##########
docs/operations.md:
##########
@@ -108,6 +108,7 @@ following table:
| operatorRbac.configManagement.create | Enable
this to create a Role for operator configuration management (hot property
loading and leader election).
| true
|
| operatorRbac.configManagement.roleName | Role name
for operator configuration management.
| `spark-operator-config-role`
|
| operatorRbac.configManagement.roleBinding |
RoleBinding name for operator configuration management.
| `"spark-operator-config-monitor-role-binding"`
|
+| operatorRbac.kueue.enabled | Grant the
operator access to Kueue `workloads`, `resourceflavors`,
`workloadpriorityclasses` and to `priorityclasses`. The cluster-scoped ones
need `clusterRole.create`. Also register `SparkApplication.v1.spark.apache.org`
and `SparkCluster.v1.spark.apache.org` in Kueue's
`integrations.externalFrameworks`.| false
|
Review Comment:
Done in e53b287. Added the Kueue bullet to `Optional Prerequisites` right
after the Gateway API one, moved the `integrations.externalFrameworks`
registration there, and shrank the table row to the grant itself with a link to
that section. The missing space before the closing pipe is fixed too.
##########
build-tools/helm/spark-kubernetes-operator/templates/tests/test-rbac.yaml:
##########
@@ -77,6 +77,20 @@ spec:
kubectl auth can-i watch grpcroutes.gateway.networking.k8s.io
--all-namespaces
kubectl auth can-i create grpcroutes.gateway.networking.k8s.io
--all-namespaces
fi
+ {{- if .Values.operatorRbac.kueue.enabled }}
+
+ # The Kueue grant is opt-in and, like Gateway API, can only be
asserted
+ # where the Kueue CRDs are installed.
+ if kubectl api-resources --api-group=kueue.x-k8s.io --no-headers -o
name | grep -q workloads; then
+ kubectl auth can-i watch workloads.kueue.x-k8s.io --all-namespaces
+ kubectl auth can-i create workloads.kueue.x-k8s.io --all-namespaces
+ kubectl auth can-i patch workloads.kueue.x-k8s.io
--subresource=status --all-namespaces
+ kubectl auth can-i update workloads.kueue.x-k8s.io
--subresource=finalizers --all-namespaces
+ kubectl auth can-i watch resourceflavors.kueue.x-k8s.io
+ kubectl auth can-i watch workloadpriorityclasses.kueue.x-k8s.io
+ fi
+ kubectl auth can-i watch priorityclasses.scheduling.k8s.io
Review Comment:
Done in e53b287. Added the comment explaining that `PriorityClass` is built
in and so needs no CRD-presence guard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
