docker icon indicating copy to clipboard operation
docker copied to clipboard

Significant Refactor

Open Leopere opened this issue 2 years ago • 19 comments

Significant Refactor.

  • added numerous environment variable changes such as implied defaults that can be overriden.
  • skipped out on using git modules and just pull repo into build/launch step. Adherance to license requires no repackaging and this solves this.
  • cleaned up now unnecessary .env file.
  • recycled environment section using yaml features.
  • writing a few strings to config path to persist data between container starts that focus on cryptography and secrets.
  • writing installed commit to the config path in case the end user needs to change the upstream git commit ID to a newer version for detection and automagic upgrades.
  • added docker-compose.override.yml pattern to .gitignore to allow users to customize their local dev environment if they use docker-compose.yml
  • wrote a dockerfile/container image which allows for uploading the base container to a private or public docker container registry without breaking the license rules.
  • left .env ignore in case users wish to continue to use the old method.
  • updated README.md to include updated simplified instructions.
  • added start.sh script and wait-for-it.sh into the shell $PATH to allow for a potential future of allowing the main executable (node JS app) to run under a limited privilege user while still allowing the init scripts to be executed securely.
  • added some input sanitation for certain critical variables.
  • by default disabled/commented out the studio service as its not to typically be run to enforce better default deployment practices. I would like to figure out what specific query to execute via the CLI instead of running a whole container to establish the first user in the end.
  • wrote relatively unopinionated docker-compose.yml file to avoid causing problems for people trying to deploy this behind a reverse proxy for potential features such as TLS/HTTPS termination.
  • upgraded compose version to latest '3.9' to be sure to enable most modern feature set.

Fixes https://github.com/calcom/docker/issues/87 by providing a working baseline with sober defaults. Fixes https://github.com/calcom/docker/issues/88 by ensuring consistency across all containers Environment vars. Fixes https://github.com/calcom/docker/issues/93 by allowing users to mount the application files within their IDE workspace, however, this will never solve for any times you will need to run yarn build steps. Fixes https://github.com/calcom/docker/issues/99 by no longer using git submodules and just pulling a single commit depth copy of the ORIGIN repository on app bootstrap/first boot. Fixes https://github.com/calcom/docker/issues/113 by no longer requiring build locally if the community maintainer of the Cal docker repository on GitHub will push this image to the hub. Fixes https://github.com/calcom/docker/issues/121 by removing dependency on BuildKit this is done by simply deploying the app if its detected to be the first execution of this container be it due to no container persistence or a commit version upgrade from ORIGIN. Fixes https://github.com/calcom/docker/issues/128 by removing dep on BuildKit Fixes https://github.com/calcom/docker/issues/123 not replicatable and confirmed to be working in repository shipped state. Fixes https://github.com/calcom/docker/issues/136 by building app on first launch from user define-able envvars which can be defined in numerous ways.

Leopere avatar Aug 01 '22 18:08 Leopere

If you can pull this fork to your local to test the only thing I'm unable to figure out is why after configuring everything successfully based off of the original why the end result is.

yarn run v1.22.19
$ turbo run start --scope="@calcom/web"
• Packages in scope:
• Running start in 0 packages

 Tasks:    0 successful, 0 total
Cached:    0 cached, 0 total
  Time:    140ms

Done in 0.30s.

Leopere avatar Aug 01 '22 18:08 Leopere

I also did my best to avoid changing how it would possibly deploy historically but yet enabling future changes to be made which may break backward compatibility to make name changes and such.

Leopere avatar Aug 01 '22 18:08 Leopere

wow what a huge refactor! amazing. Whats missing to go from draft to ready for review?

PeerRich avatar Aug 01 '22 20:08 PeerRich

cc @krumware and @zomars

PeerRich avatar Aug 01 '22 20:08 PeerRich

@PeerRich, for the most part, I'm just stuck on a few more minor details. I would recommend maybe validating my README.MD and committing any changes you think I should make or clarify. This was just a weekend for a fun project for me as I was passing through GitHub. All I can say is it bootstraps okay, it accepts and seems to validate the environment variable settings ok, but I don't know anything about NodeJS apps or Yarn to figure out how to debug what's not letting the web app itself launch. I think for the most part I managed to get 90+% of the original dockerfile and start stuff worked out in a relatively sober way but I'll see what you all think.

Leopere avatar Aug 01 '22 22:08 Leopere

@zomars, I mean, if you aren't the docker guru, that's almost better if it works for you, then I hit my target.

Leopere avatar Aug 01 '22 23:08 Leopere

@Leopere thanks for the contributions here. There's a lot to unpack here. It would be helpful if you could join the cal.com slack and we can chat about it

krumware avatar Aug 02 '22 13:08 krumware

@krumware I absolutely expected this to be declined haha I'll have a look for the Slack.

Leopere avatar Aug 02 '22 14:08 Leopere

@leog on Slack discussion it surged the idea of being about to create the first admin user via a custom seeder TS file as well so we don't expose unconfigured instances in the web. What do you think?

zomars avatar Aug 02 '22 16:08 zomars

@leog on Slack discussion it surged the idea of being about to create the first admin user via a custom seeder TS file as well so we don't expose unconfigured instances in the web. What do you think?

@zomars that makes sense, we can remove the step in the wizard for now until app credentials gets implemented

leog avatar Aug 02 '22 17:08 leog

No need for removal. If the db has a user already the wizard won't show anyways

zomars avatar Aug 02 '22 17:08 zomars

@leog on Slack discussion it surged the idea of being about to create the first admin user via a custom seeder TS file as well so we don't expose unconfigured instances in the web. What do you think?

@zomars that makes sense, we can remove the step in the wizard for now until app credentials gets implemented

Don't get me wrong; it's OK to allow the user to be created via the interface in case they don't deploy it via a standard container. I'm just suggesting it could be an excellent alternative to presenting user configuration to the internet.

Leopere avatar Aug 02 '22 18:08 Leopere

Don't get me wrong; it's OK to allow the user to be created via the interface in case they don't deploy it via a standard container. I'm just suggesting it could be an excellent alternative to presenting user configuration to the internet.

I see. So we would be covering all fronts. Great!

leog avatar Aug 02 '22 18:08 leog

Also, for added clarity, there are two modes for the actual boot invocation of the app itself. Invoked from this case switch here the start.sh script's preflight checks determine what boot mode is to be used.

bootstrap_start is a function that was added for first-time operation in a deployment where it simply detects if there is a package.json inside of the $APP_PATH, and if it doesn't detect $APP_PATH/package.json, it will simply clone and compile the exact git commit requested from the specifically requested upstream git repository. Then it automatically runs the following boot function, basic_start, where it starts the app as defined in the previous docker container iteration.

basic_start simply changes the working directory into the $APP_PATH and executes the former containers' start functionality.

One of the biggest concerns with how this operates is simply that the git repository isn't automatically packaged in the Immutable Container itself; however, licensing of the Cal app itself dictates that repackaging for distribution is not allowed. Written consent would need to be provided to allow for this repackaging to be done via the community.

Leopere avatar Aug 02 '22 18:08 Leopere

Another detail that was brought up, and I will adjust the README.md and docker-compose.yml to accommodate, is that its no longer required to deploy with Prisma Studio as there is now a first install wizard. This said further lines of questions may need to be answered before this is added. Perhaps if this PR gets accepted, a new TODO list item will need to be created in lieu of this PR's acceptance.

Leopere avatar Aug 02 '22 18:08 Leopere

Finally, there may be blocking issues that were raised by @krumware about the immutability of the container and how the container starts.

It would be difficult to exceed the boot speed of the previous configuration with how basic_start is effectively identical to the original container, and now with far fewer container layers, the initial pull speed should be lightning fast.

However, the concern about completely traditional container immutability may be a completely blocking issue if the Cal organization officials don't provide consent to allow repackaging the code base for pushing to the Docker Hub public Docker Registry. However, security-wise, there is a fully cryptographically secure chain of custody between the git repositories and any deployed container as we're using https/tls and the cryptographically signed byte-perfect git fetch feature.

Leopere avatar Aug 02 '22 18:08 Leopere

Thank you for your review!

Admittedly I made the mistake of not checking the find and replace on docker-compose in the documentation. I think a good presumption could be that users of docker compose tend currently to already be wise to the change and lack of need to use the hyphenated version that's installed separately.

Leopere avatar Aug 10 '22 05:08 Leopere

Well I'd still really like to get this working or even help elsewhere.

Leopere avatar Aug 16 '22 17:08 Leopere

@Leopere, help us out here in the cheap seats: Can we use your work to create a mostly-working production environment? If so (and again, sorry for the very basic question) do we simply git clone your repository and head off to the races? Or does it combine with calcom's main branch in order to work?

gregwbrooks avatar Sep 06 '22 20:09 gregwbrooks