Expose runtime configuration during slugbuilder execution #960

gabrtv · 2014-05-12T18:01:33Z

Fixes #462.

This PR uses a new Config hook to grab the latest app configuration from the controller and expose it to slugbuilder using -e flags. While this works, it is not a very good mechanism for handling user-defined inputs on the builder. We must consider this as we move to refactor the builder in Go.

Example of setting a custom BUILDPACK_URL:

$ deis create
Creating application... done, created humble-autoharp
Git remote deis added
$ deis config:set BUILDPACK_URL=https://github.com/dpiddy/heroku-buildpack-ruby-minimal
Creating config... done, v2

=== humble-autoharp
BUILDPACK_URL: https://github.com/dpiddy/heroku-buildpack-ruby-minimal
$ git push deis master
Counting objects: 92, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (50/50), done.
Writing objects: 100% (92/92), 19.97 KiB | 0 bytes/s, done.
Total 92 (delta 40), reused 85 (delta 37)
-----> Fetching custom buildpack
Cloning into '/tmp/buildpacks/custom'...
-----> Ruby app detected
-----> Using bundler-1.5.2
-----> Using ruby-1.9.3
-----> Installing dependencies
       Fetching gem metadata from http://rubygems.org/..........
       Fetching additional metadata from http://rubygems.org/..
       Using bundler (1.5.2)
       Installing rack (1.5.2)
       Installing tilt (1.3.6)
       Installing rack-protection (1.5.0)
       Installing sinatra (1.4.2)
       Your bundle is complete!
       Gems in the groups development and test were not installed.
       It was installed into ./vendor/bundle

Note this particular buildpack doesn't contain a bin/release and errors with:

/tmp/builder/build.sh: line 94: /tmp/buildpacks/custom/bin/release: No such file or directory
Slugbuilder returned error code

.. but that's not our problem.

bacongobbler · 2014-05-12T18:14:08Z

Gonna need to be careful with 9a2312a, but otherwise looks great! @gabrtv could you please add docs for this PR at http://docs.deis.io/en/latest/developer/? It could fall under http://docs.deis.io/en/latest/developer/deploy-application/, but I think it needs its own page much like https://devcenter.heroku.com/articles/config-vars.

https://github.com/bacongobbler/heroku-buildpack-jekyll might be a better example with this PR as it should work out of the box.

gabrtv · 2014-05-12T18:47:21Z

@bacongobbler I am going to address documentation holistically in the next PR, which will be the improved Dockerfile handling -- coming soon to a theater near you.

bacongobbler · 2014-05-12T18:50:09Z

controller/api/views.py

+        user = get_object_or_404(
+            User, username=request.DATA['receive_user'])
+        # check the user is authorized for this app
+        if user == app.owner or user in get_users_with_perms(app):


Shouldn't we be utilizing a permissions class for this logic instead?

We are actually using a permissions class specified in the parent (HasBuilderAuth). The more granular check (user is authorized for this app) could be ported to a PermissionsClass, but it's a bit more work and would require refactoring the other hook views. Again I'm open to it, just not in this PR.

gabrtv · 2014-05-12T19:36:04Z

Added back scale command found by @bacongobbler. Ready for testing.

carmstrong · 2014-05-12T20:12:11Z

What is the best way to test this? Is setting a custom buildpack URL as you did adequate, @gabrtv?

bacongobbler · 2014-05-12T20:32:47Z

Still need to test, but LGTM from my point of view :)

gabrtv · 2014-05-12T20:33:17Z

@carmstrong yes, custom buildpack URL is the easiest test.

carmstrong · 2014-05-12T20:37:40Z

@gabrtv Could you please rebase off master so we have the /etc/hosts fix for the Docker CDN?

This uses a new Config API Hook to grab the latest app configuration and expose it to slugbuilder using `-e` flags. While this works, it is a very good mechanism for handling user-defined inputs on the builder. In other words, command-injection is possible through malformed inputs. We must consider this as we move to refactor the builder in Go.

As part of the effort to move code from views into models, we would always scale an app to an initial structure on every deploy. This made it impossible to, for example, set a BUILDPACK_URL without triggering a false deploy and timeout. Moving the logic back into the BuildView where it belongs.

gabrtv · 2014-05-12T20:43:14Z

@carmstrong done.

carmstrong · 2014-05-12T23:04:07Z

Finally completed testing this. LGTM - works as advertised.

bacongobbler · 2014-05-13T16:10:09Z

Works as advertised for me as well:

><> deis config
=== twenty-uptowner Config
BUILDPACK_URL      asdf; pwd
><> git push deis aster
error: src refspec aster does not match any.
error: failed to push some refs to 'ssh://git@local.deisapp.com:2222/twenty-uptowner.git'
-----> Fetching custom buildpack
fatal: repository 'asdf; pwd' does not exist
Slugbuilder returned error code

hehehe... LGTM!

gabrtv · 2014-05-13T16:12:11Z

Nice test. Single quotes ftw.

Expose runtime configuration during slugbuilder execution

bacongobbler · 2014-05-13T16:24:46Z

http://xkcd.com/327/

bacongobbler reviewed May 12, 2014
View reviewed changes

Gabriel Monroy added 4 commits May 12, 2014 14:38

fix(flake8): fix spaces and a docstring

fd88251

fix(builder): add scale() back to first build

1d46330

gabrtv pushed a commit that referenced this pull request May 13, 2014

Merge pull request #960 from deis/config-during-build

886551f

Expose runtime configuration during slugbuilder execution

gabrtv merged commit 886551f into master May 13, 2014

gabrtv deleted the config-during-build branch May 13, 2014 16:12

bacongobbler mentioned this pull request Feb 25, 2015

pass env vars to buildpack #3145

Closed

bacongobbler mentioned this pull request Jul 14, 2016

Pass config to slugbuilder/dockerbuilder during builds deis/builder#388

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose runtime configuration during slugbuilder execution #960

Expose runtime configuration during slugbuilder execution #960

gabrtv commented May 12, 2014

bacongobbler commented May 12, 2014

gabrtv commented May 12, 2014

bacongobbler May 12, 2014

gabrtv May 12, 2014

gabrtv commented May 12, 2014

carmstrong commented May 12, 2014

bacongobbler commented May 12, 2014

gabrtv commented May 12, 2014

carmstrong commented May 12, 2014

gabrtv commented May 12, 2014

carmstrong commented May 12, 2014

bacongobbler commented May 13, 2014

gabrtv commented May 13, 2014

bacongobbler commented May 13, 2014

Expose runtime configuration during slugbuilder execution #960

Expose runtime configuration during slugbuilder execution #960

Conversation

gabrtv commented May 12, 2014

bacongobbler commented May 12, 2014

gabrtv commented May 12, 2014

bacongobbler May 12, 2014

Choose a reason for hiding this comment

gabrtv May 12, 2014

Choose a reason for hiding this comment

gabrtv commented May 12, 2014

carmstrong commented May 12, 2014

bacongobbler commented May 12, 2014

gabrtv commented May 12, 2014

carmstrong commented May 12, 2014

gabrtv commented May 12, 2014

carmstrong commented May 12, 2014

bacongobbler commented May 13, 2014

gabrtv commented May 13, 2014

bacongobbler commented May 13, 2014