-
Notifications
You must be signed in to change notification settings - Fork 2
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
Self hosting! #67
Self hosting! #67
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe recent updates enhance deployment and configuration processes for the Draft project, focusing on restructuring file paths, refining command parameters, and updating domain configurations. Key tasks include setting up Traefik and managing processes with PM2, utilizing Ansible for provisioning and deployment, and streamlining certificate loading and environment variable setups from AWS Secrets Manager. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
server/package-lock.json
is excluded by!**/package-lock.json
Files selected for processing (15)
- README.md (2 hunks)
- etc/start_helper_sidecar.sh (1 hunks)
- server/.prettierignore (1 hunks)
- server/ansible/deploy.yaml (1 hunks)
- server/ansible/inventory.ini (1 hunks)
- server/ansible/provision.yaml (1 hunks)
- server/package.json (2 hunks)
- server/pm2.json (1 hunks)
- server/traefik/dynamic/dynamic.yaml (1 hunks)
- server/traefik/dynamic/tls.yaml (1 hunks)
- server/traefik/traefik.yaml (1 hunks)
- sidecar/ansible/deploy.yaml (1 hunks)
- sidecar/ansible/inventory.ini (1 hunks)
- sidecar/ansible/provision.yaml (2 hunks)
- sidecar/cli/cli.py (5 hunks)
Files skipped from review due to trivial changes (7)
- etc/start_helper_sidecar.sh
- server/.prettierignore
- server/package.json
- server/pm2.json
- sidecar/ansible/deploy.yaml
- sidecar/ansible/inventory.ini
- sidecar/ansible/provision.yaml
Additional context used
yamllint
server/traefik/dynamic/tls.yaml
[error] 3-3: syntax error: expected , but found ''
(syntax)
server/traefik/dynamic/dynamic.yaml
[error] 7-7: syntax error: expected , but found ''
(syntax)
Markdownlint
README.md
70-70: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (33)
server/traefik/traefik.yaml (1)
1-9
: LGTM!The configuration for entry points and providers is correctly set up.
server/ansible/inventory.ini (1)
1-5
: LGTM!The Ansible inventory configuration is correctly set up.
server/ansible/deploy.yaml (5)
5-9
: LGTM!The task to pull new commits from GitHub is correctly configured.
11-15
: LGTM!The task to install npm packages based on the package-lock.json file is correctly configured.
17-28
: LGTM!The task to load the .env file using AWS Secrets Manager is correctly configured.
31-36
: LGTM!The task to rebuild the draft website using npm is correctly configured.
39-44
: LGTM!The task to restart the draft website using npm is correctly configured.
server/ansible/provision.yaml (19)
4-6
: LGTM!The task to store the HOME directory variable is correctly configured.
8-12
: LGTM!The task to check if Node.js is installed is correctly configured.
14-19
: LGTM!The task to install Node.js using yum is correctly configured.
21-25
: LGTM!The task to check if npm is installed is correctly configured.
27-32
: LGTM!The task to install npm using yum is correctly configured.
34-38
: LGTM!The task to check if Git is installed is correctly configured.
40-45
: LGTM!The task to install Git using yum is correctly configured.
47-51
: LGTM!The task to clone the repository if it doesn't exist is correctly configured.
52-56
: LGTM!The task to install npm packages based on the package-lock.json file is correctly configured.
58-62
: LGTM!The task to check if Traefik is installed is correctly configured.
64-69
: LGTM!The task to download Traefik if it is not installed is correctly configured.
71-75
: LGTM!The task to ensure that the extraction directory for Traefik exists is correctly configured.
77-81
: LGTM!The task to extract the Traefik binary is correctly configured.
83-87
: LGTM!The task to copy the Traefik binary into the draft directory is correctly configured.
91-91
: LGTM!The task to grant the CAP_NET_BIND_SERVICE capability to the Traefik binary is correctly configured.
95-97
: LGTM!The task to create a directory for certificates is correctly configured.
100-108
: LGTM!The task to load the cert.pem file using AWS Secrets Manager is correctly configured.
113-120
: LGTM!The task to load the key.pem file using AWS Secrets Manager is correctly configured.
123-132
: LGTM!The task to start Traefik and Next.js using npm is correctly configured.
README.md (5)
27-27
: LGTM!The change correctly updates the path to the inventory.ini file.
28-28
: LGTM!The change correctly updates the provision command to use the new inventory and provision file paths.
30-30
: LGTM!The change correctly updates the deploy command to use the new inventory and deploy file paths.
71-71
: LGTM!The change correctly updates the
draft start-helper-sidecar
command to use the new domain parameters.
74-74
: LGTM!The change correctly updates the status URL to use the new sidecar domain.
sidecar/cli/cli.py (2)
80-81
: LGTM!The function
start_traefik_command
is correctly updated to require the necessary domain parameters.
136-137
: LGTM!The function
run_helper_sidecar
is correctly updated to require the necessary domain parameters and remove the unnecessaryroot_domain
parameter.
server/ansible/deploy.yaml
Outdated
state: present | ||
ci: true | ||
|
||
- name: Load .env file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please help me understand what is getting stored in .env
file here? According to cli doc
The decrypted secret value, if the secret value was originally provided as a string or through the Secrets Manager console.
If this secret was created by using the console, then Secrets Manager stores the information as a JSON structure of key/value pairs.
If that's true, I am a little nervous about keeping the secret on durable storage. Is it possible to configure env variable or (probably better) assume role on this instance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's variables needed server side like API keys, db passwords, etc. We could certainly set these as environmental variables instead of using the .env
file, and it seems reasonable to avoid writing them to a file.
When you say "assume role on this instance, this does assume a specific AMI role, which is what allows the
aws secretmanager` cli to actually access those secrets. Is there something else you mean by this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, fixed. I added a shell script "load_secrets.sh" which is sourced and adds them into the env. the TLS keys are still written to disk, because traefik expects them there. it's not immediately obvious of a better way to handle that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you!
When you say "assume role on this instance, this does assume a specific AMI role, which is what allows the aws secretmanager` cli to actually access those secrets. Is there something else you mean by this?
Yea I was thinking about using https://awscli.amazonaws.com/v2/documentation/api/latest/reference/sts/assume-role.html to get temporary credentials, but this serves different purpose as far as I can tell.
If we store more than one password, API key, etc, we could consider partitioning this and store one secret with one key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- server/ansible/deploy.yaml (1 hunks)
- server/ansible/load_secrets.sh (1 hunks)
- server/ansible/provision.yaml (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- server/ansible/deploy.yaml
- server/ansible/provision.yaml
Additional comments not posted (5)
server/ansible/load_secrets.sh (5)
1-7
: LGTM!The shebang is correct, and the check for the CERT_DIR environment variable is a good practice to ensure the variable is set.
9-12
: LGTM!Setting the CERT_DIR variable to the first argument and ensuring the directory exists using
mkdir -p
is a good practice.
14-21
: LGTM!The retrieval of the
cert.pem
file from AWS Secrets Manager and writing it to the CERT_DIR directory is correctly implemented.
22-28
: LGTM!The retrieval of the
key.pem
file from AWS Secrets Manager and writing it to the CERT_DIR directory is correctly implemented.
30-36
: LGTM!The retrieval of environment variables from AWS Secrets Manager and setting them in the current shell environment using
jq
and awhile
loop is correctly implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- server/ansible/load_secrets.sh (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- server/ansible/load_secrets.sh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- server/ansible/deploy.yaml (1 hunks)
Files skipped from review due to trivial changes (1)
- server/ansible/deploy.yaml
This moves our frontend deployment to https://draft.ipa-helper.dev hosted on AWS.
while we loose some of the convenience of vercel, this will allow us to do stuff like long polling, which will ultimately be better for the API design.
Summary by CodeRabbit
New Features
deploy.yaml
for automating deployment tasks including GitHub pull, npm package installation, and service management using PM2.Updates
README.md
with accurate file paths and domain configurations.start_helper_sidecar.sh
for better variable assignment and command parameters.traefik/*
andansible/*
files.package.json
with new scripts for PM2 and added PM2 dependency.Bug Fixes
root_domain
parameter from commands and configurations.Refactor
cli.py
to enforce mandatory parameters, removing defaults and cleaning up command usage.