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

Untrack .env file from Git #1106

Closed
wants to merge 4 commits into from

Conversation

Sebbo94BY
Copy link
Contributor

@Sebbo94BY Sebbo94BY commented Oct 1, 2021

This merge request will...

  • remove the .env file from the Git tracking
  • add an .env.example instead of the previous well known .env file
  • update the install script to create the respective .env file at the installation, when no such file exists yet
  • update the scripts/bump-version.sh to update the .env.example file instead
  • add a note regarding the .env file to the readme

Motivation:
The .env file provides environment specific configurations, which can be always different at each installation. Due to this, the .env file - which is actually loaded and used by the application - should never be committed to Git. Instead, you rather should commit and track a template, an example file.

Such an .env file could also contain secrets like usernames, passwords or API tokens, which is one more good reason to avoid committing it to Git as everyone could see your secrets there.

Additional, when you check out this repository using Git, change something in the .env file and do a git status, it shouldn't show any changes as the .env should be handled like normal system environment variables, which are also not checked by Git. Git also won't be that happy about changed and not committed files when you execute certain Git commands. Git will then ask you to undo, commit or stash these changes first.

A clean and working installation also shouldn't report any changes, when you run a git status. Otherwise, the application is not properly checked out.

Also see these articles regarding the DotEnv file:

@BYK BYK self-requested a review October 1, 2021 12:31
Copy link
Member

@BYK BYK left a comment

Choose a reason for hiding this comment

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

Can you share the motivation behind this? The tracking is intentional as we bake in the released versions into the env file and your proposed solution would not work with that when upgrading.

@Sebbo94BY Sebbo94BY marked this pull request as draft October 1, 2021 12:46
@Sebbo94BY
Copy link
Contributor Author

Sure, I'll update it. I've marked this PR as draft for now. :)

Yes, I know. I'll also update everything related to it, so that it's still working as before.

@Sebbo94BY Sebbo94BY changed the title Untrack env file Untrack .env file from Git Oct 1, 2021
@Sebbo94BY Sebbo94BY marked this pull request as ready for review October 1, 2021 16:15
@Sebbo94BY
Copy link
Contributor Author

Ok, I've updated the merge request. Based on the Git workflows, it should still work like before.

@Sebbo94BY Sebbo94BY requested a review from BYK October 1, 2021 16:20
@Sebbo94BY
Copy link
Contributor Author

Any news regarding this change @BYK ?

At least your Github actions seem to be happy about this change: https://github.com/TS3Tools/onpremise/pull/1/checks :)

@BYK
Copy link
Member

BYK commented Oct 6, 2021

@Sebi94nbg, I'm still very skeptical about the cost/benefit ratio of this change. It is possible to pass different env files to docker-compose and as I mentioned before, we intentionally kept the .env file versioned. We expect people to pass secrets through actual env variables instead of using the .env file and anything that needs to persist, such as configuration settings, can be set in the config files themselves (with the exception of SENTRY_EVENT_RETENTION_DAYS).

Do you have a strong motivation for your use case to make this change?

@BYK
Copy link
Member

BYK commented Oct 6, 2021

The .env file provides environment specific configurations, which can be always different at each installation. Due to this, the .env file - which is actually loaded and used by the application - should never be committed to Git. Instead, you rather should commit and track a template, an example file.

This is not how .env is used in docker-compose btw. It serves as a fallback value when the variables are not set in the environment.

@Sebbo94BY
Copy link
Contributor Author

Ok, if there are these dependencies and concerns on the part of the project, how about adding the ability to provide a "custom" .env file, which allows administrators to overwrite options from the original, committed .env file?

Just as an example:

if [[ -f ".env.custom" ]]; then
    docker-compose --env-file .env.custom up
else
    docker-compose --env-file .env up
    # Identical to the line above
    # docker-compose up
fi

Reference: https://docs.docker.com/compose/environment-variables/#using-the---env-file--option

This would allow administrators to change options from the .env file without actually changing it. So Git will still be happy, that there are no changes. :)

@BYK
Copy link
Member

BYK commented Oct 6, 2021

I think this is a much better solution as it keeps the defaults simple, relatively easy to implement (as we alias docker-compose), that said we should tell people to use that option when we tell them to run docker-compose up.

I have to go back to my original question for clarification though: These values are not intended to be changed, at least on disk, what is the exact need you have that you want to change the values here?

@Sebbo94BY
Copy link
Contributor Author

Closing this merge request. I've created a new one with the discussed change: #1113

@Sebbo94BY Sebbo94BY closed this Oct 9, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants