-
Notifications
You must be signed in to change notification settings - Fork 770
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
Support for environment variables substitution #67
Conversation
Now user can put environment variables in docker-compose file and it will be read by kompose from environment Fixes # 56
The code in this PR is from https://github.com/docker/libcompose/blob/master/docker/project.go#L25 So I needed some discussion around this PR, so I checked the code in kompose here, it is similar to code in libcompose here. So I was hoping we can use the function So the downside of using |
@surajssd I see you are changing libcompose code in |
@ngtuna I did |
@surajssd Ah okay you changed libcompose revision to HEAD. OK let me check it. |
@ngtuna @kadel How can I convert a |
@surajssd The reason I didn't use NewProject function because I don't want kompose to be constrained by libcompose. Basically we only need its parsing function in order to parse docker compose file into komposeObject, nothing more. Otherwise, we have to follow libcompose's constraints such as it's required to have a Did you check whether your changes in cli/app/app.go still works with the current libcompose revision at HEAD master? If yes then I suggest to keep the vendoring stable and just apply your changes in app.go. I can't check it now but will do tomorrow morning. |
@ngtuna I checked with the vendored libcompose available I am not able to build, because I need https://github.com/skippbox/kompose/pull/67/files#diff-41d801ef80f1858d5e8e9695667e4dafR749 and it is not in the vendored |
Checked! Thanks @surajssd . Please go ahead to merge upstream. |
@ngtuna thank you :) |
Now user can put environment variables in docker-compose file and it will be read by kompose from environment
Fixes #56