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

Update: migration to distroless #42

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

itninja-hue
Copy link
Collaborator

Migration to Distroless base image on release stage

@itninja-hue itninja-hue requested a review from 0xb4lamx May 15, 2021 20:57
@itninja-hue itninja-hue self-assigned this May 15, 2021
Copy link
Owner

@0xb4lamx 0xb4lamx left a comment

Choose a reason for hiding this comment

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

2MB diff between the 2 versions? xD
image
but AFAIU "Distroless is for Security if not for Size" 🤔

Dockerfile Outdated Show resolved Hide resolved
@@ -390,8 +390,8 @@ Temporary Items

=======
# Local
docker-compose.yml
*.env
Copy link
Owner

Choose a reason for hiding this comment

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

.env files should not be pushed to git.

we prepared an .env.example to clone from locally 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that env file is loaded by docker-compose in build phase, it evals the docker compose and change the ${VARS} by its equivalent.

@@ -129,7 +129,7 @@ export class ConfigService {
transports: [
new DailyRotateFile({
level: 'debug',
filename: `./logs/${this.nodeEnv}/debug-%DATE%.log`,
filename: `${process.env.HOME}/${this.nodeEnv}/debug-%DATE%.log`,
Copy link
Owner

Choose a reason for hiding this comment

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

we need to find a better location for this.

we can't keep ./logs/* and mount it in docker to be consistent? 🤔 (from developer perspective who uses docker and local env interchangeably)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can play with an env var and default it to env.home if not defined what do you think about it ?

@@ -0,0 +1,149 @@
#!/bin/sh
Copy link
Owner

Choose a reason for hiding this comment

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

Good stuff, we need to update README accoding to this? for better experience using the new CLI 🤔

(didn't test it intensively yet)

@0xb4lamx 0xb4lamx added the good first issue Good for newcomers label Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants