You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/06/30 18:58:35 UTC
[GitHub] [flink-kubernetes-operator] jeesmon opened a new pull request, #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
jeesmon opened a new pull request, #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288
Removed un-used apiGroup and resources from required RBAC
Signed-off-by: Jeesmon Jacob <jj...@vmware.com>
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#issuecomment-1172439992
+1 for merging, LGTM
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#issuecomment-1172381184
> Thanks @jeesmon, looks good. I agree with removing endpoints and pvcs - hope you can confirm that we can also remove nodes. If it is only sometimes needed we could make a config value for it in the helm chart.
@mbalassi Yes, working on an update for `nodes` rules. Thanks.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911623792
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
- - persistentvolumeclaims
Review Comment:
The helm chart functionality can be extended with kustomize templates, and PVCs could be a useful to mount plugins and what not for the Operator. Although I'm generally in favour of least privilege principle, I would probably won't remove this one. WDYT?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911621447
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
- - persistentvolumeclaims
- events
- configmaps
- secrets
- - nodes
Review Comment:
I remember we needed it either when playing with Ingress or the `kubernetes.rest-service.exposed.type` fail to recall. Worth double-checking both before removing.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#issuecomment-1172390240
@morhidi @mbalassi I have updated the PR to conditionally create `nodes` rbac. Please check and let me know your feedback. Thanks.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911971961
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
- - persistentvolumeclaims
- events
- configmaps
- secrets
- - nodes
Review Comment:
@morhidi You are right, `kubernetes.rest-service.exposed.type: NodePort` requires `list` permission on `nodes` at the cluster scope.
```
2022-07-01 13:30:20,742 i.j.o.p.e.ReconciliationDispatcher [ERROR][flink-27975/basic-example] Error during event processing ExecutionScope{ resource id: ResourceID{name='basic-example', namespace='flink-27975'}, version: 16843355} failed.
org.apache.flink.kubernetes.operator.exception.ReconciliationException: org.apache.flink.kubernetes.shaded.io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: GET at: https://10.96.0.1/api/v1/nodes. Message: Forbidden!Configured service account doesn't have access. Service account may have been revoked. nodes is forbidden: User "system:serviceaccount:flink-27975:flink-operator" cannot list resource "nodes" in API group "" at the cluster scope.
```
This brings another point that `kubernetes.rest-service.exposed.type: NodePort` will throw error for namespace scoped operator as it requires `list` permission at the cluster scope for `nodes`.
I will update the PR with `nodes` rbac and some comments. Thanks for your pointers.
@gyfora FYI
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911906795
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
- - persistentvolumeclaims
- events
- configmaps
- secrets
- - nodes
Review Comment:
I will double check that config and get back.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911624111
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
Review Comment:
+1 This block seemingly here for no good, can be removed.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#issuecomment-1172579457
@mbalassi Doc updated
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] mbalassi merged pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
mbalassi merged PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] mbalassi commented on pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
mbalassi commented on PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#issuecomment-1172858784
Now that all checks have passed I am merging.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911906327
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
- - persistentvolumeclaims
Review Comment:
@morhidi Even if we are adding PVC to operator, operator is not creating/managing PVC. PVC is created outside of operator as I understood. So mounting a persistentVolumeClaim to operator deployment doesn't require this RBAC. `e2e-tests/data/sessionjob-cr.yaml` is a good example for this and that test is passed without the RBAC. Please correct me if I'm wrong.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] mbalassi commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
mbalassi commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911776904
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
- - persistentvolumeclaims
- events
- configmaps
- secrets
- - nodes
Review Comment:
I think it was needed for NodePort (?), but it would be nice to remove it - having access to nodes can be a bit too much to ask.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911959390
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
Review Comment:
Great, thanks
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#issuecomment-1172444514
@morhidi Could you please trigger the workflow to make sure no CI issues?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#issuecomment-1171583094
@gyfora As there were no code markers or other hints (like in kubebuilder)
1. I searched through the code for any reference to those removed resources
2. Ran examples and made sure operator is not reporting any errors in log
I'm also expecting any CI errors if there is any hidden uses. Not sure how else to confirm :(
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911623792
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
- - persistentvolumeclaims
Review Comment:
The helm chart functionality can be extended with kustomize templates, and PVCs could be useful to mount plugins and what not for the Operator. Although I'm generally in favour of least privilege principle, I would probably won't remove this one. WDYT?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911908077
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
Review Comment:
As far as I can see, `services` RBAC covers it. Operator or flink native integration doesn't seem to be creating or managing endpoints directly. But I will double check.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#issuecomment-1171654135
I think this is another place to look for the kube resources that flink is using: https://github.com/apache/flink/tree/master/flink-kubernetes/src/main/java/org/apache/flink/kubernetes/kubeclient/resources
I see management of only the following resources in native integration:
Out of which only Service/Pod/ConfigMap are the only API resources.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
gyfora commented on PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#issuecomment-1171574415
Could you please share how you came to the conclusion of removing these specific ones?
I am just trying to understand how to verify this properly :)
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] jeesmon commented on pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
jeesmon commented on PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#issuecomment-1171569746
@mbalassi @gyfora Could you please review?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911620631
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
Review Comment:
Not sure about the endpoints, we create quite a bunch:
```
(minikube:default) ➜ flink-kubernetes-operator git:(FLINK-28228) k get endpoints
NAME ENDPOINTS AGE
basic-checkpoint-ha-example-rest 172.17.0.9:8081 8s
basic-ingress 172.17.0.7:6124,172.17.0.7:6123 16s
basic-ingress-rest 172.17.0.7:8081 16s
flink-operator-webhook-service 172.17.0.6:9443 3m36s
```
Does the service permissions include this?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a diff in pull request #288: [FLINK-27975] Remove unnecessary RBAC rules from operator
Posted by GitBox <gi...@apache.org>.
morhidi commented on code in PR #288:
URL: https://github.com/apache/flink-kubernetes-operator/pull/288#discussion_r911922368
##########
helm/flink-kubernetes-operator/templates/rbac.yaml:
##########
@@ -21,23 +21,14 @@ RBAC rules used to create the operator (cluster)role based on the scope
*/}}
{{- define "flink-operator.rbacRules" }}
rules:
- - apiGroups:
- - flink-operator
- resources:
- - "*"
- verbs:
- - "*"
- apiGroups:
- ""
resources:
- pods
- services
- - endpoints
- - persistentvolumeclaims
Review Comment:
Good point, can be removed then
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org