feat: irsa checks #98
Conversation
| @@ -55,6 +58,43 @@ Options: | |||
|
|
|||
| - `kubectl`: Required for interacting with Kubernetes clusters. | |||
|
|
|||
| ### IRSA Configuration Check (`/checks/kube/aws-irsa.sh`) | |||
There was a problem hiding this comment.
I'd add somewhere that a Helm chart release install is a requirement for this script.
E.g. helm template + kustomize or ArgoCD (uses helm template) would not result in Helm chart releases.
| Usage: ./checks/kube/aws-irsa.sh [-h] [-n NAMESPACE] [-e EXCLUDE_COMPONENTS] [-p COMPONENTS_PG] [-l COMPONENTS_OS] [-s] | ||
| Options: | ||
| -h Display this help message | ||
| -n NAMESPACE Specify the namespace to use |
There was a problem hiding this comment.
since that's the only value that is required, should this be marked?
| -e EXCLUDE_COMPONENTS Comma-separated list of components to exclude from the check (reference of the component is the root key used in the chart) | ||
| -p COMPONENTS_PG Comma-separated list of components to check IRSA for PostgreSQL (overrides default list) | ||
| -l COMPONENTS_OS Comma-separated list of components to check IRSA for OpenSearch (overrides default list) | ||
| -s Disable pod spawn for IRSA and network flow verification |
There was a problem hiding this comment.
maybe we could explain in a bit more detail what this means?
There was a problem hiding this comment.
Agreed, it was supposed to be part of the documentation, but will not hurt to add some context here aswell
|
|
||
| ##### Example: | ||
| ```bash | ||
| ./checks/kube/aws-irsa.sh -n camunda-primary -p "identity,webModeler" -l "zeebe,operate" |
There was a problem hiding this comment.
curious how precise does the naming have to be?
Woudln't it be easier to just convert everything to lowercase, so customers can supply it however they want and we also don't have to care too much about special cases à la webModeler.
Maybe easier for the helm chart handling, just curious.
There was a problem hiding this comment.
very precise (case sensitive) as we use the name of the component for jq queries.
Introducing key insensitivity in jq directly could be challenging (https://stackoverflow.com/questions/67725386/jq-unsensitive-case-key-filter), that said, we could have a static map at the beggining that does that for us.
I'll implement it
There was a problem hiding this comment.
was just an idea, I'm also fine with keeping it as is.
| if [[ -z "$identity_keycloak_port" ]]; then | ||
| echo "[FAIL] identityKeycloak.externalDatabase.port is not defined in your helm values." 1>&2 | ||
| SCRIPT_STATUS_OUTPUT=86 |
There was a problem hiding this comment.
afaik I don't have to explicitly set it since the port by default is 5432
There was a problem hiding this comment.
good point, I've not tested the setup without explicit port so I can't tell you if the deployment works or not.
Also the chart does not have a default port set, so it will certainly fail.
I'll try to deploy without the port, if it works, then I'll fix this part not to throw an error and rather assume that's 5432 set as default port.
| if [[ -z "$role_arn" ]]; then | ||
| echo "[FAIL] The service account $service_account_name does not have a valid eks.amazonaws.com/role-arn annotation. You must add it in the chart, see https://docs.camunda.io/docs/self-managed/setup/deploy/amazon/amazon-eks/eks-helm/" 1>&2 | ||
| SCRIPT_STATUS_OUTPUT=110 | ||
| else | ||
| echo "[OK] The service account $service_account_name is bound to the role $role_arn by the eks.amazonaws.com/role-arn annotation." |
There was a problem hiding this comment.
I didn't deploy it with IRSA, just wanted to see what happens.
[OK] The service account camunda-identity is bound to the role null by the eks.amazonaws.com/role-arn annotation
[INFO] Verifying role ARN (component=identity,serviceAccount=camunda-identity): null
[INFO] Running command: aws iam get-role --role-name "null"
obviously fails after that, but afaik should have already done so before.
[INFO] Running command: kubectl get serviceaccount "camunda-identity" -n "camunda" -o json
[INFO] Command output: {
"apiVersion": "v1",
"automountServiceAccountToken": true,
"kind": "ServiceAccount",
"metadata": {
"annotations": {
"meta.helm.sh/release-name": "camunda",
"meta.helm.sh/release-namespace": "camunda"
},
"creationTimestamp": "2024-10-31T11:52:14Z",
"labels": {
"app": "camunda-platform",
"app.kubernetes.io/component": "identity",
"app.kubernetes.io/instance": "camunda",
"app.kubernetes.io/managed-by": "Helm",
"app.kubernetes.io/name": "camunda-platform",
"app.kubernetes.io/part-of": "camunda-platform",
"app.kubernetes.io/version": "8.6.2",
"helm.sh/chart": "camunda-platform-11.0.2"
},
"name": "camunda-identity",
"namespace": "camunda",
"resourceVersion": "24758",
"uid": "49c75019-c3cc-4088-ac68-2037e55770d1"
}
}
There was a problem hiding this comment.
I need to do a double check of the returned values, good point
There was a problem hiding this comment.
left some smaller comments to look into, nothing critical that has to be fixed in particular, except maybe the null and mentioning that's helm chart required.
I tried it out once without any IRSA and once with IRSA on just Postgres.
Overall worked quite well, I was just a bit overwhelmed by the output but I think if someone really uses it to debug, they would appreciate the verbosity.
This pull request introduces a substantial script designed to verify IRSA (IAM Roles for Service Accounts) configuration for deployments on Kubernetes.
While the script provides essential functionality, it's important to note that the error handling system is currently somewhat primitive could be nice tp plan to reevaluate it later to improve maintainability (
SCRIPT_STATUS_OUTPUT).https://github.com/camunda/team-infrastructure-experience/issues/25 associated with this pull request will reference the usage of the script in the documentation. The doc will also include a detailed explanation of using IRSA, the required format for IAM roles, and instructions on how to associate the necessary permissions.
Regarding testing, we plan to integrate c8-sm-checks into our reference architectures https://github.com/camunda/team-infrastructure-experience/issues/378. I have personally tested the script on a reference clusters of eks.