Adding CHECK_SERVICE_ACCOUNT to khcheck deployment #425
Conversation
Should have done this in kuberhealthy#361 but forgot. This way people don't have to read the code to know about this setting. The default SA is the default account that will be used by default so this won't matter for anyone who dosen't want to overwrite it like me..
|
Looks good from the testing on my end. I think it would also be good to clean up the deployment-sa that is defined in the .yaml files for the deployment check and switch to using the default sa. |
| @@ -471,6 +471,8 @@ spec: | |||
| value: "4" | |||
| - name: CHECK_DEPLOYMENT_ROLLING_UPDATE | |||
| value: "true" | |||
| - name: CHECK_SERVICE_ACCOUNT | |||
There was a problem hiding this comment.
These files under deploy are auto-generated by github actions based on the contents in the helm directory -- there is no need to touch these as they will get overwritten. Placing them in the values.yaml and the khcheck-deployment.yaml files should avoid this overwrite.
There was a problem hiding this comment.
Ahh I have seen the create-flat-files.sh but I never opened it, a nice solution.
I will push a fix that just changes the values.yaml.
These files gets generated from helm by github action.
|
The deployment-sa account should still be needed to be able to deploy the pods. The reason why I need the CHECK_SERVICE_ACCOUNT function in Openshift is due to that I create a SA account that I allow to run containers as root which is needed since the deploy check deploys nginx and sadly nginx always is run as root. I could of course use deployment-sa to do this but since this SA don't need to be able to run containers that runs as root after the changes that was recently done to the Dockerfiles I don't want to give this account any extra access that isn't needed. Or did i misunderstand you? |
|
I think I misunderstood you here. Would you suggest moving away from using nginx servers? |
|
I don't think it's worth the job to move away from nginx. The good thing with nginx is that everyone knows what it is so it's a very simple deployment and service to debug if something strange would happen. |
Should have done this in #361 but forgot.
This way people don't have to read the code to know about this setting.
The default SA is the default account that will be used by default so this won't matter for anyone who doesn't want to overwrite it like me..