[#2404] Custom ServiceAccount in Infinispan CR#2524
Conversation
tristantarrant
force-pushed
the
2404/service_account
branch
from
ca58ca2 to
15c4cc7
Compare
| + | ||
| The `spec.serviceAccountName` field sets the service account for {brandname} server pods and GossipRouter pods. | ||
| The `spec.configListener.serviceAccountName` field sets the service account for the ConfigListener pod. | ||
| Because the ConfigListener uses the Operator image rather than the {brandname} server image, it may require a different service account. |
There was a problem hiding this comment.
Different image may be one reason if for whatever reason the images are in different repositories but mainly the config listener service account needs access (roles) to manage Cache CRs. Cluster and Gossip service accounts do not require any roles (at least at the moment)
| Because the ConfigListener uses the Operator image rather than the {brandname} server image, it may require a different service account. | ||
| + | ||
| When you specify `spec.configListener.serviceAccountName`, {ispn_operator} does not create its own service account for the ConfigListener. | ||
| However, it still creates the necessary Role and RoleBinding resources bound to the specified service account to ensure the ConfigListener has the required API permissions. |
There was a problem hiding this comment.
This is anti pattern, you are giving away authorization secretly. If user want's to use his own service account, he needs to give it correct rbac roles and we need to document which ones are required and how to do that.
| Annotations: annotationsForPod, | ||
| }, | ||
| Spec: corev1.PodSpec{ | ||
| ServiceAccountName: i.Spec.ServiceAccountName, |
There was a problem hiding this comment.
We might want to do a validation of SA existence and either emit an event if the SA does not exist or set a condition the SA is missing and loop reconciliation till it exists.
At the moment it just creates a StatefulSet with a error pod with missing ServiceAccount (Not 100% sure). Which is okay as well and we may keep it as is for now but generally I'd say it's better to accumulate issues directly at the Infinispan CR if we are able to detect them.
| assert.Equal(saName, pod.Spec.ServiceAccountName) | ||
| } | ||
|
|
||
| func TestServiceAccountNameUpdate(t *testing.T) { |
There was a problem hiding this comment.
This test is failing because required changes to pkg/reconcile/pipeline/infinispan/handler/manage/statefulset_updates.go reponsible for taking care of updates are missing
| @@ -0,0 +1,6 @@ | |||
| kind: Infinispan | |||
There was a problem hiding this comment.
Let's not forget on ServiceAccount for Batches, Backups and Restores which spin up pods as well.
For those I'd say the general strategy should be to use any serviceAccount configured for target cluster (Infinispan CR) if present and maybe add an option to override with spec.serviceAccountName. Question is should we add an option to override the serviceAccount for the GossipRouter as well? I'd say not mandatory, at least not now.
tristantarrant
force-pushed
the
2404/service_account
branch
from
15c4cc7 to
ebcbe4b
Compare
2 participants
Closes #2404