-
Notifications
You must be signed in to change notification settings - Fork 31
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
feat(discovery): enable OpenShift cross-namespace discovery #1290
Conversation
Test image available:
|
Test image available:
|
Test image available:
|
@ebaron thoughts on this approach? I hacked together something quick for https://github.com/andrewazores/cryostat-helm/tree/crossns To test this out I used In $ helm install --set core.image.repository=quay.io/andrewazores/cryostat --set core.image.tag="2.3.0-crossns" cryostat ./cryostat I did the usual kubectl -n default set env deploy --containers=cryostat cryostat CRYOSTAT_K8S_NAMESPACES=default,cryostat1,cryostat2 This failed as expected with the following in the Cryostat container logs:
so it is trying to start an Endpoints Informer for each of the listed namespaces, but it fails due to lack of permissions since |
Test image available:
|
I repeated the setup above, but this time I only created the additional In the Back in namespace
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
annotations:
kubectl.kubernetes.io/last-applied-configuration: |
{"apiVersion":"rbac.authorization.k8s.io/v1","kind":"Role","metadata":{"annotations":{"meta.helm.sh/release-name":"cryostat","meta.helm.sh/release-namespace":"default"},"creationTimestamp":"2023-01-12T12:31:19Z","labels":{"app.kubernetes.io/instance":"cryostat","app.kubernetes.io/managed-by":"Helm","app.kubernetes.io/name":"cryostat","app.kubernetes.io/version":"latest","helm.sh/chart":"cryostat-0.2.0"},"name":"cryostat1","namespace":"cryostat1","resourceVersion":"1414","uid":"41a8748c-18e5-4846-8670-e28308d367f8"},"rules":[{"apiGroups":[""],"resources":["endpoints"],"verbs":["get","list","watch"]},{"apiGroups":[""],"resources":["pods","replicationcontrollers"],"verbs":["get"]},{"apiGroups":["apps"],"resources":["replicasets","deployments","daemonsets","statefulsets"],"verbs":["get"]},{"apiGroups":["apps.openshift.io"],"resources":["deploymentconfigs"],"verbs":["get"]}]}
meta.helm.sh/release-name: cryostat
meta.helm.sh/release-namespace: default
creationTimestamp: "2023-01-12T12:40:03Z"
labels:
app.kubernetes.io/instance: cryostat
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: cryostat
app.kubernetes.io/version: latest
helm.sh/chart: cryostat-0.2.0
name: cryostat1
namespace: cryostat1
resourceVersion: "2429"
uid: 71ff90cc-5ca5-4a69-9f44-244d16266658
rules:
- apiGroups:
- ""
resources:
- endpoints
verbs:
- get
- list
- watch
- apiGroups:
- ""
resources:
- pods
- replicationcontrollers
verbs:
- get
- apiGroups:
- apps
resources:
- replicasets
- deployments
- daemonsets
- statefulsets
verbs:
- get
- apiGroups:
- apps.openshift.io
resources:
- deploymentconfigs
verbs:
- get
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
annotations:
kubectl.kubernetes.io/last-applied-configuration: |
{"apiVersion":"rbac.authorization.k8s.io/v1","kind":"RoleBinding","metadata":{"annotations":{"meta.helm.sh/release-name":"cryostat","meta.helm.sh/release-namespace":"default"},"creationTimestamp":"2023-01-12T12:31:19Z","labels":{"app.kubernetes.io/instance":"cryostat","app.kubernetes.io/managed-by":"Helm","app.kubernetes.io/name":"cryostat","app.kubernetes.io/version":"latest","helm.sh/chart":"cryostat-0.2.0"},"name":"cryostat1","namespace":"cryostat1","resourceVersion":"1415","uid":"62004294-924f-43fe-af43-72445d06aa51"},"roleRef":{"apiGroup":"rbac.authorization.k8s.io","kind":"Role","name":"cryostat1"},"subjects":[{"kind":"ServiceAccount","name":"cryostat","namespace":"default"}]}
meta.helm.sh/release-name: cryostat
meta.helm.sh/release-namespace: default
creationTimestamp: "2023-01-12T12:40:09Z"
labels:
app.kubernetes.io/instance: cryostat
app.kubernetes.io/managed-by: Helm
app.kubernetes.io/name: cryostat
app.kubernetes.io/version: latest
helm.sh/chart: cryostat-0.2.0
name: cryostat1
namespace: cryostat1
resourceVersion: "2438"
uid: a4785739-b463-4454-a00d-47b36ab235ec
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: cryostat1
subjects:
- kind: ServiceAccount
name: cryostat
namespace: default These are basically just copies of the existing role/rolebinding that the helm chart sets up, just modified to create a mirrored role and binding to the same serviceaccount within After that, the logs show Cryostat starting successfully, and these messages appear:
API responses show cross-namespace discovery working:
[
{
"jvmId": "-WRGlnsyV__ze4pBW6eKUxvF62L6YVmGWTa7Xgf5Itk=",
"connectUrl": "service:jmx:rmi:///jndi/rmi://10.244.0.12:9091/jmxrmi",
"alias": "cryostat-ffb67db94-jpx68",
"labels": {
"pod-template-hash": "ffb67db94",
"app.kubernetes.io/name": "cryostat",
"app.kubernetes.io/instance": "cryostat"
},
"annotations": {
"platform": {},
"cryostat": {
"HOST": "10.244.0.12",
"PORT": "9091",
"NAMESPACE": "cryostat1",
"POD_NAME": "cryostat-ffb67db94-jpx68",
"REALM": "KubernetesApi"
}
}
},
{
"jvmId": "a-NTvJxVlVQ2MjH-aX_PRWL17_MabOPjj1nDNS9ypus=",
"connectUrl": "service:jmx:rmi:///jndi/rmi://10.244.0.11:9091/jmxrmi",
"alias": "cryostat-6f4c6b58fc-hr4jg",
"labels": {
"pod-template-hash": "6f4c6b58fc",
"app.kubernetes.io/name": "cryostat",
"app.kubernetes.io/instance": "cryostat"
},
"annotations": {
"platform": {
"kubectl.kubernetes.io/restartedAt": "2023-01-12T07:40:11-05:00"
},
"cryostat": {
"HOST": "10.244.0.11",
"PORT": "9091",
"NAMESPACE": "default",
"POD_NAME": "cryostat-6f4c6b58fc-hr4jg",
"REALM": "KubernetesApi"
}
}
}
]
{
"meta": {
"type": "application/json",
"status": "OK"
},
"data": {
"result": {
"children": [
{
"name": "Custom Targets",
"nodeType": "Realm",
"labels": {
"REALM": "a3308ab3-d16c-4473-9809-2041a956fba9"
},
"children": []
},
{
"name": "KubernetesApi",
"nodeType": "Realm",
"labels": {
"REALM": "b15783b3-34d0-4646-a99e-2f75e581eb1d"
},
"children": [
{
"name": "cryostat1",
"nodeType": "Namespace",
"labels": {},
"children": [
{
"name": "cryostat",
"nodeType": "Deployment",
"labels": {
"helm.sh/chart": "cryostat-0.2.0",
"app.kubernetes.io/managed-by": "Helm",
"app.kubernetes.io/name": "cryostat",
"app.kubernetes.io/instance": "cryostat",
"app.kubernetes.io/version": "latest"
},
"children": [
{
"name": "cryostat-ffb67db94",
"nodeType": "ReplicaSet",
"labels": {
"pod-template-hash": "ffb67db94",
"app.kubernetes.io/name": "cryostat",
"app.kubernetes.io/instance": "cryostat"
},
"children": [
{
"name": "cryostat-ffb67db94-jpx68",
"nodeType": "Pod",
"labels": {
"pod-template-hash": "ffb67db94",
"app.kubernetes.io/name": "cryostat",
"app.kubernetes.io/instance": "cryostat"
},
"children": [
{
"name": "service:jmx:rmi:///jndi/rmi://10.244.0.12:9091/jmxrmi",
"nodeType": "Endpoint",
"labels": {},
"target": {
"jvmId": "-WRGlnsyV__ze4pBW6eKUxvF62L6YVmGWTa7Xgf5Itk=",
"connectUrl": "service:jmx:rmi:///jndi/rmi://10.244.0.12:9091/jmxrmi",
"alias": "cryostat-ffb67db94-jpx68",
"labels": {
"pod-template-hash": "ffb67db94",
"app.kubernetes.io/name": "cryostat",
"app.kubernetes.io/instance": "cryostat"
},
"annotations": {
"platform": {},
"cryostat": {
"NAMESPACE": "cryostat1",
"PORT": "9091",
"REALM": "KubernetesApi",
"HOST": "10.244.0.12",
"POD_NAME": "cryostat-ffb67db94-jpx68"
}
}
}
}
]
}
]
}
]
}
]
},
{
"name": "default",
"nodeType": "Namespace",
"labels": {},
"children": [
{
"name": "cryostat",
"nodeType": "Deployment",
"labels": {
"helm.sh/chart": "cryostat-0.2.0",
"app.kubernetes.io/managed-by": "Helm",
"app.kubernetes.io/name": "cryostat",
"app.kubernetes.io/instance": "cryostat",
"app.kubernetes.io/version": "latest"
},
"children": [
{
"name": "cryostat-6f4c6b58fc",
"nodeType": "ReplicaSet",
"labels": {
"pod-template-hash": "6f4c6b58fc",
"app.kubernetes.io/name": "cryostat",
"app.kubernetes.io/instance": "cryostat"
},
"children": [
{
"name": "cryostat-6f4c6b58fc-hr4jg",
"nodeType": "Pod",
"labels": {
"pod-template-hash": "6f4c6b58fc",
"app.kubernetes.io/name": "cryostat",
"app.kubernetes.io/instance": "cryostat"
},
"children": [
{
"name": "service:jmx:rmi:///jndi/rmi://10.244.0.11:9091/jmxrmi",
"nodeType": "Endpoint",
"labels": {},
"target": {
"jvmId": "a-NTvJxVlVQ2MjH-aX_PRWL17_MabOPjj1nDNS9ypus=",
"connectUrl": "service:jmx:rmi:///jndi/rmi://10.244.0.11:9091/jmxrmi",
"alias": "cryostat-6f4c6b58fc-hr4jg",
"labels": {
"pod-template-hash": "6f4c6b58fc",
"app.kubernetes.io/name": "cryostat",
"app.kubernetes.io/instance": "cryostat"
},
"annotations": {
"platform": {
"kubectl.kubernetes.io/restartedAt": "2023-01-12T07:40:11-05:00"
},
"cryostat": {
"NAMESPACE": "default",
"PORT": "9091",
"REALM": "KubernetesApi",
"HOST": "10.244.0.11",
"POD_NAME": "cryostat-6f4c6b58fc-hr4jg"
}
}
}
}
]
}
]
}
]
}
]
}
]
}
],
"name": "Universe",
"nodeType": "Universe",
"labels": {}
}
}
} As expected, port-forwarding to expose the instance in |
Test image available:
|
Your approach to test the PR makes sense to me and I'm glad it worked! I'll give the code a closer look shortly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good and works as expected. Very nice!
Test image available:
|
Welcome to Cryostat! 👋
Before contributing, make sure you have:
main
branch[chore, ci, docs, feat, fix, test]
git commit --amend --signoff
Related to #1188
Related to #760
Related to cryostatio/cryostat-operator#501
Description of the change:
The primary change is a one-liner:k8sClient.endpoints().inNamespace(namespace) -> k8sClient.endpoints().inAnyNamespace()
. This also does a bit of refactoring.This updates the
KubeApiPlatformClient
class, which is the internal implementation that handles theEndpoints
querying discovery behaviour, to not be restricted to only querying forEndpoints
objects within the sameNamespace
that the Cryostat container is running within. A newCRYSOTAT_K8S_NAMESPACES
environment variable is added. The variable's value is expected to be a comma-separated list of namespace names. TheKubeApiPlatformClient
sets up anInformer
for each of these namespaces, and collates results from all of the informers when handling operations like listing targets or constructing the discovery tree. If this environment variable is unset/blank then the namespace Cryostat is running in is used as a singleton list value, which maintains the existing behaviour.Motivation for the change:
If the Operator has deployed Cryostat in anAllNamespaces
mode, then Cryostat's built-in OpenShift discovery mechanism should look in all namespaces, not only the one which Cryostat is deployed within. This is still subject to the hardcodedsvc.port.name == 'jfr-jmx' || svc.port.number == 9091
behaviour as before. The previous Discovery Plugin API and the upcomingcryostat-agent
address that. This PR also does not handle the "multi-tenant" case of multi-namespace installations, ie cases where not only are applications divided between different Namespaces but where users have differing permissions between namespaces. The combination of OperatorAllNamespaces
and this PR only enables an all-privileged model across multiple namespaces - any user with access to Cryostat has access to all of the recording data within Cryostat, regardless of where the data originated and what the user's permissions are within that origin (namespace).The Operator may deploy Cryostat into its own namespace and expect it to monitor targets across multiple namespaces which all have the same or similar permissions levels (same levels of user and service account access etc). This PR does not add any handling or security considerations for varying access levels across the listed namespaces. If Cryostat's service account has access to those namespaces then it will be able to discover targets there, collect JFR data from those targets, and store it in its archives. Any user with access to Cryostat will then be able to read those files from the archives, even if they do not have permission to access the targets in the namespace where the data originated. See #1188 for work toward enforcing namespace separation. These two PRs together would enable a proper cluster-wide/
AllNamespaces
Cryostat installation.TODO later, to hook in with #1188, is to support some syntax like
CRYOSTAT_K8S_NAMESPACES='*'
, which would cause theKubeApiPlatformClient
to install anInformer
with.inAnyNamespace()
for full cluster-wideEndpoints
discovery. This is slightly trickier when it comes to constructing the discovery tree since the namespaces are not known in advance but found while creating the tree, so some transient caching of namespaceEnvironmentNode
s may be needed to ensure that differentEndpoints
objects and their ownership chains do not cause duplicate namespaces to be registered into the final tree.How to manually test:
See comments below.