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

ServiceAccount Fails to Bind to ClusterRole Due to Mismatch #292

Closed
Yicheng-Lu-llll opened this issue Oct 11, 2024 · 4 comments · Fixed by #453
Closed

ServiceAccount Fails to Bind to ClusterRole Due to Mismatch #292

Yicheng-Lu-llll opened this issue Oct 11, 2024 · 4 comments · Fixed by #453
Assignees
Labels
area/distributed kind/bug Something isn't working priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@Yicheng-Lu-llll
Copy link
Contributor

Yicheng-Lu-llll commented Oct 11, 2024

🐛 Describe the bug

When I follow the instruction and build from the source:

export PATH=$PATH:$(go env GOPATH)/bin
go install golang.org/dl/go1.22.5@latest
go1.22.5 download
export GOROOT=$(go1.22.5 env GOROOT)
export PATH="$GOROOT/bin:$PATH"

kind delete cluster
kind create cluster

helm template kuberay-operator kuberay/kuberay-operator --namespace aibrix-system --version 1.2.1 --include-crds --set env[0].name=ENABLE_PROBES_INJECTION --set env[0].value=\"false\" --set fullnameOverride=kuberay-operator --output-dir ./config/dependency

kubectl create -k config/dependency
kubectl create -k config/default

KubeRay operator seems to lack permission to list Pods.

kubectl auth can-i list pods --as=system:serviceaccount:aibrix-system:aibrix-kuberay-operator
#no

Upon further investigation, I found that the issue occurs because the RoleBinding is assigned to the kuberay-operator service account instead of aibrix-kuberay-operator.

kubectl kustomize config/default
# apiVersion: rbac.authorization.k8s.io/v1
# kind: ClusterRoleBinding
# metadata:
#   labels:
#     app.kubernetes.io/instance: kuberay-operator
#     app.kubernetes.io/managed-by: Helm
#     app.kubernetes.io/name: kuberay-operator
#     helm.sh/chart: kuberay-operator-1.2.1
#   name: aibrix-kuberay-operator
# roleRef:
#   apiGroup: rbac.authorization.k8s.io
#   kind: ClusterRole
#   name: aibrix-kuberay-operator
# subjects:
# - kind: ServiceAccount
#   name: kuberay-operator
#   namespace: aibrix-system

This is because we first generate the config file from the KubeRay Helm chart and then use namePrefix: aibrix- in config/default/kustomization.yaml to add a prefix to all resources.
However, the following internal references won't be automatically updated.

subjects:
- kind: ServiceAccount
  name: kuberay-operator
  namespace: aibrix-system

We may need add a patch for this if we insist want to add prefix.

Steps to Reproduce

No response

Expected behavior

No response

Environment

No response

@Jeffwan
Copy link
Collaborator

Jeffwan commented Oct 11, 2024

BTW, config/default now includes the kuberay deployment, so helm package install can be skipped. in that case, we should check aibrix-kuberay-operator under aibrix-system namespace to check whether the permission is well granted

@Yicheng-Lu-llll
Copy link
Contributor Author

BTW, config/default now includes the kuberay deployment, so helm package install can be skipped. in that case, we should check aibrix-kuberay-operator under aibrix-system namespace to check whether the permission is well granted

It looks like we use Helm templates to generate the configuration files(under ./config/dependency) and use kubectl create -k config/default to apply those files. Most of the names in the configuration files are kuberay-operator.

However, when we run kubectl create -k config/default, a prefix aibrix- is added to all resources, but it doesn't add the prefix to internal references. As a result, the RoleBinding still binds to the ServiceAccount kuberay-operator instead of aibrix-kuberay-operator.

@Jeffwan Jeffwan self-assigned this Nov 19, 2024
@Jeffwan Jeffwan added the kind/bug Something isn't working label Nov 19, 2024
@Jeffwan Jeffwan added this to the v0.2.0 milestone Nov 19, 2024
@Jeffwan Jeffwan added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. area/distributed labels Nov 19, 2024
@Jeffwan Jeffwan added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Dec 1, 2024
@Jeffwan
Copy link
Collaborator

Jeffwan commented Dec 1, 2024

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
  labels:
    app.kubernetes.io/instance: kuberay-operator
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/name: kuberay-operator
    helm.sh/chart: kuberay-operator-1.2.1
  name: aibrix-kuberay-operator
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: aibrix-kuberay-operator
subjects:
- kind: ServiceAccount
  name: kuberay-operator
  namespace: aibrix-system

here's the generate service account, even we moved kuberay installation to config/default to references. this can be reproduced easily.

deployment service account ref, cluster role binding ref and leader-election role binding ref still stick to origin role name


resources:
- namespace.yaml
- ../crd
- ../rbac
- ../manager
- ../gateway
- ../dependency/kuberay-operator

If I render manifest just for ../dependency/kuberay-operator, it's working fine. However, if I add all, it's not working well.

@Jeffwan
Copy link
Collaborator

Jeffwan commented Dec 1, 2024

I decide to patch the object directly to quick fix this issue. #453 We can do more elegant overriding later. for example, using nameReference transformer etc. (I did try it but didn't work well)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/distributed kind/bug Something isn't working priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants