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

Start celery without uid #2923

Merged
merged 4 commits into from
Oct 4, 2020
Merged

Start celery without uid #2923

merged 4 commits into from
Oct 4, 2020

Conversation

uncycler
Copy link
Contributor

When celery is started with "--uid" argument, it needs root access. In docker, it superfluous since the Dockerfile.django already start the process with the good uid. This cause issue when starting celery-beat in K8s with a securityContext.

  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is flake8 compliant.
  • Your code is python 3.6 compliant (specific python >3.6 syntax is currently not accepted).
  • If this is a new feature and not a bug fix, you've included the proper documentation in the ReadTheDocs documentation folder. https://github.com/DefectDojo/Documentation/tree/master/docs or provide feature documentation in the PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

@madchap
Copy link
Collaborator

madchap commented Sep 28, 2020

I have to check again why I have done that. It was to remove the C_FORCE_ROOT but maybe I missed something.

Does docker-compose still work without https://github.com/DefectDojo/django-DefectDojo/blob/dev/docker-compose.yml#L42 then?

@uncycler
Copy link
Contributor Author

@madchap I just disabled root from the docker-compose since root is only need when celery is started with "--uid". Celery beat is started with the user defined in Dockerfile in both docker-compose and k8s.

@madchap
Copy link
Collaborator

madchap commented Sep 29, 2020

Doing a quick run on docker-compose, applying your change, beat exits with the following error

$ docker logs madchap_ddojo_celerybeat_1          
wait-for-it.sh: waiting 30 seconds for mysql:3306
wait-for-it.sh: mysql:3306 is available after 14 seconds
uid=1001(defectdojo) gid=65534(nogroup) groups=65534(nogroup)
enabling audit logging
Waiting for database to be reachable 
Traceback (most recent call last):
  File "/usr/local/bin/celery", line 8, in <module>
    sys.exit(main())
  File "/usr/local/lib/python3.6/site-packages/celery/__main__.py", line 16, in main
    _main()
  File "/usr/local/lib/python3.6/site-packages/celery/bin/celery.py", line 322, in main
    cmd.execute_from_commandline(argv)
  File "/usr/local/lib/python3.6/site-packages/celery/bin/celery.py", line 499, in execute_from_commandline
    super(CeleryCommand, self).execute_from_commandline(argv)))
  File "/usr/local/lib/python3.6/site-packages/celery/bin/base.py", line 305, in execute_from_commandline
    return self.handle_argv(self.prog_name, argv[1:])
  File "/usr/local/lib/python3.6/site-packages/celery/bin/celery.py", line 491, in handle_argv
    return self.execute(command, argv)
  File "/usr/local/lib/python3.6/site-packages/celery/bin/celery.py", line 419, in execute
    ).run_from_argv(self.prog_name, argv[1:], command=argv[0])
  File "/usr/local/lib/python3.6/site-packages/celery/bin/base.py", line 309, in run_from_argv
    sys.argv if argv is None else argv, command)
  File "/usr/local/lib/python3.6/site-packages/celery/bin/base.py", line 393, in handle_argv
    return self(*args, **options)
  File "/usr/local/lib/python3.6/site-packages/celery/bin/base.py", line 253, in __call__
    ret = self.run(*args, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/celery/bin/beat.py", line 100, in run
    maybe_drop_privileges(uid=uid, gid=gid)
  File "/usr/local/lib/python3.6/site-packages/celery/platforms.py", line 543, in maybe_drop_privileges
    _setuid(uid, gid)
  File "/usr/local/lib/python3.6/site-packages/celery/platforms.py", line 564, in _setuid
    initgroups(uid, gid)
  File "/usr/local/lib/python3.6/site-packages/celery/platforms.py", line 507, in initgroups
    return os.initgroups(username, gid)
PermissionError: [Errno 1] Operation not permitted

@uncycler
Copy link
Contributor Author

@madchap Don't forget to rebuild django image with the modified entrypoint-celery-beat.sh. _setuid is only called when "--uid" is set. docker exec -it madchap_ddojo_celerybeat_1 cat /entrypoint-celery-beat.sh?

@madchap
Copy link
Collaborator

madchap commented Sep 29, 2020

Indeed, I should not attempt to do too many things at once. Works now.

Copy link
Collaborator

@madchap madchap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this.

@madchap
Copy link
Collaborator

madchap commented Sep 29, 2020

@valentijnscholten
Copy link
Member

I have no idea how all this k8s/helm stuff works, so cannot review this one.

@uncycler
Copy link
Contributor Author

Since, securityContext is not defined for other containers (django, nginx, worker), celery-beat-deployment.yaml should do the same . If not, the container will be started as root.

@madchap
Copy link
Collaborator

madchap commented Sep 29, 2020

Spawned up minikube, rebuilt images removing the deployment's securityContext for celerybeat, seems to work. id on beat container returns expected 1001 and beat is not crashing.

$ k exec -ti defectdojo-celery-beat-564444dcfc-v4d5f -- bash -c 'id'
uid=1001 gid=0(root) groups=0(root)

@uncycler do you mind removing https://github.com/DefectDojo/django-DefectDojo/blob/dev/helm/defectdojo/templates/celery-beat-deployment.yaml#L49-L52 as part of your PR?

Thanks.

@uncycler
Copy link
Contributor Author

It's done.

@valentijnscholten valentijnscholten merged commit c805f79 into DefectDojo:dev Oct 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants