pokt-network / pocket-core

Official implementation of the Pocket Network Protocol
http://www.pokt.network
MIT License
208 stars 101 forks source link

Docker Image enhancement and better CLI support. #1603

Closed jorgecuesta closed 2 months ago

jorgecuesta commented 2 months ago

THIS PR INCLUDES BREAKING CHANGES

Why?

A few Geo-Mesh users report to POKTscan about an issue with the new image after we adopt the one on the pokt-network/pocket-core repository on the latest RC.

They report that this image is using root as the user which is recommended to be avoided. There are a lot of blogs and documentation about this, here one of them from a well-known docker image user/company.

Also, we detected a few things that could be enhanced on both, entry point and docker context.

The problem with having a public image using root right now is that pocket binary generates folders and files that now belong to the root user, so they will need to modify those permissions to belong to the proper app user and group. To this, I added another optional entry point that could be used once to fix the permission issue and then start the container as before.

Here you can see how to use it with docker-compose or docker

Changes:

Olshansk commented 2 months ago

@okdas Please review this when you're back in office next week and see if any of the learnings / suggestions can transfer over to #495 which we should pick up then as well.

nodiesBlade commented 2 months ago

(Reviewed this on Geomesh branch as well), looks good to me.

Olshansk commented 2 months ago

@okdas PTAL

Olshansk commented 2 months ago

Re-running to make sure this passes but otherwise lgtm:

Screenshot 2024-05-17 at 3 38 42 PM
okdas commented 2 months ago

@Olshansk it is not going to pass because the origin of the PR is a branch from pokt-scan organization which doesn't have permissions to push to our container registry.

Olshansk commented 2 months ago

@okdas Appreciate the context. What do you recommend as the solution here?

For example:

  1. We squash & merge as is?
  2. Update permissions to allow their org to run CI on our repo?
  3. Validate in some other way?
  4. Etc?

I understand the risk is low but just looking to you to make a call here.

okdas commented 2 months ago

@Olshansk the workaround would be to open a new PR under pokt-network org, but I don't think we need to do that and should proceed with merging as is. The CI job seems to be failing on the very last step - and the image itself is built successfully.