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

Docker: Introduce .env.dist file to store sensitive data #13

Closed
wants to merge 13 commits into from
Closed

Docker: Introduce .env.dist file to store sensitive data #13

wants to merge 13 commits into from

Conversation

dev7ch
Copy link
Contributor

@dev7ch dev7ch commented Jan 9, 2018

Due we have sensitive data such as a Github I would suggest to store sensitive data in a file which is .gitignored automatically.

@dev7ch dev7ch changed the title Introduce .env.dist file to store sensitive data Docker: Introduce .env.dist file to store sensitive data Jan 9, 2018
@rainerCH
Copy link
Contributor

Hey @dev7ch have you tested everything with a fresh installation of all docker containers? (rebuild)

@dev7ch
Copy link
Contributor Author

dev7ch commented Jan 10, 2018

@rainerCH yes I tested it. ( all images and container removed before testing)

@nadar
Copy link
Member

nadar commented Jan 10, 2018

I changed GITHUB_TOKEN="ADD_YOUR_SECRET_TOKEN_HERE" to GITHUB_TOKEN=ADD_YOUR_SECRET_TOKEN_HERE otherwise it wont work.

@dev7ch
Copy link
Contributor Author

dev7ch commented Jan 10, 2018

ok, it worked for me fine with the ".." as well, on OSX and if the "" are removed form .env they should be added in docker-compose.yml probably.

@nadar did you test it with "" ?

@rainerCH
Copy link
Contributor

rainerCH commented Jan 10, 2018

doesn't work for me with "" as well.

@dev7ch
Copy link
Contributor Author

dev7ch commented Jan 10, 2018

ok, my bad it seem not to work with the "" which is far away from logic, maybe only for me ;)

@nadar
Copy link
Member

nadar commented Jan 10, 2018

So we could merge, what do you think @rainerCH?

@rainerCH
Copy link
Contributor

rainerCH commented Jan 10, 2018

I have another Idea to make the installation steps more simple:

step 1: docker-compose build --build-arg GITHUB_TOKEN=token luya_composer
step 2: docker-compose up
step 3: setup

so in fact people do not have to copy (and edit) any files.

--> I suggest to remove the .env.dist, remove the args section in docker-compose and add the build command to readme.md

what do you guys think? @dev7ch @nadar

@dev7ch
Copy link
Contributor Author

dev7ch commented Jan 10, 2018

@rainerCH yes it think that´s a good alternative as well.

But what i am not sure about is, if the image has been destroyed, is on rebuild is a new token required? What you think?

just guessing it could be causes the token will be stored in the image i think. And you can view this token only once.

@rainerCH
Copy link
Contributor

rainerCH commented Jan 10, 2018

thats true, you have to pass the token again to the build process but:

  • normally you build the image just once
  • you can still add a file called .env (and args line in yml file) with the github token if you want later
  • kickstart should be as easy as possible

i made another pull request with my proposal

@dev7ch
Copy link
Contributor Author

dev7ch commented Jan 11, 2018

@rainerCH yes normally an images should not been rebuild but image you would like to test an apache image besides nginx, this would mean you have to generate a new token ( which is less simple then store it in a .gitignored file)

And personally, i like the way of copy a file and adding more then read the docs,which feels more simple for me then adding argument via command line.

@nadar what do you think?

@nadar
Copy link
Member

nadar commented Jan 11, 2018

@dev7ch I am going to close this PR, but please make another PR which extends the docker README with this information, so it could be "another way" to solve this problem.

@dev7ch
Copy link
Contributor Author

dev7ch commented Jan 11, 2018

ok

@dev7ch dev7ch closed this Jan 11, 2018
@nadar
Copy link
Member

nadar commented Jan 11, 2018

@dev7ch i think after yii 2.1 release, there is no need for FXP plugin anymore which currently hits the api request limit. so maybe there is not even any need to provide an api key anymore.

@dev7ch
Copy link
Contributor Author

dev7ch commented Jan 11, 2018

ok, yes thats an argument to leave it that way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants