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

[changelog skip] Quote buildpack's export values #967

Merged
merged 1 commit into from Mar 19, 2020

Conversation

schneems
Copy link
Contributor

@schneems schneems commented Mar 18, 2020

[changelog skip] Quote buildpack's export values

This is a followup from #965.

In #888 the values in the buildpack's export file dropped their quoting for paths causing a few failures that were not caught by our tests.

This PR fixes the underlying problem and adds a test that would have caught the regression. The test uses a buildpack that intentionally adds a quoted newline to it's export file https://github.com/sharpstone/export_path_with_newlines_buildpack/blob/master/bin/compile this gets picked up by the Ruby buildpack and put back into Ruby's export file. Since ruby after #888 did not quote these values it would result in this failure:

  1) Buildpack internals handles PATH with a newline in it correctly
     Failure/Error:
       Hatchet::Runner.new("default_ruby", buildpacks: buildpacks).deploy do |app|
         expect(app.output).to_not match("No such file or directory")
       end
     Hatchet::App::FailedDeploy:
       Could not deploy 'hatchet-t-3e6bacb18a' (default_ruby) using 'Hatchet::GitApp' at path: './repos/rack/default_ruby'
        if this was expected add `allow_failure: true` to your deploy hash.
       output:
       Buildpack: nil
       Repo: https://git.heroku.com/hatchet-t-3e6bacb18a.git
       remote: Compressing source files... done.        
       remote: Building source:        
       remote: 
       remote: -----> I add newlines to the export file app detected        
       remote: -----> Ruby app detected        
       remote: -----> Installing bundler 1.17.3        
       remote: -----> Removing BUNDLED WITH version in the Gemfile.lock        
       remote: -----> Compiling Ruby/Rack        
       remote: -----> Using Ruby version: ruby-2.6.5        
       remote: -----> Installing dependencies using bundler 1.17.3        
       remote:        Running: bundle install --without development:test --path vendor/bundle --binstubs vendor/bundle/bin -j4 --deployment        
       remote:        Fetching gem metadata from https://rubygems.org/...............        
       remote:        Using bundler 1.17.3        
       remote:        Fetching rack 1.5.0        
       remote:        Installing rack 1.5.0        
       remote:        Bundle complete! 1 Gemfile dependency, 2 gems now installed.        
       remote:        Gems in the groups development and test were not installed.        
       remote:        Bundled gems are installed into `./vendor/bundle`        
       remote:        Bundle completed (3.14s)        
       remote:        Cleaning up the bundler cache.        
       remote: -----> Writing config/database.yml to read from DATABASE_URL        
       remote: -----> Detecting rake tasks        
       remote: 
       remote: ###### WARNING:        
       remote: 
       remote:        You have not declared a Ruby version in your Gemfile.        
       remote:        To set your Ruby version add this line to your Gemfile:        
       remote:        ruby '2.6.5'        
       remote:        # See https://devcenter.heroku.com/articles/ruby-versions for more information.        
       remote: 
       remote: ###### WARNING:        
       remote: 
       remote:        No Procfile detected, using the default web server.        
       remote:        We recommend explicitly declaring how to boot your server process via a Procfile.        
       remote:        https://devcenter.heroku.com/articles/ruby-default-web-server        
       remote: 
       remote: 
       remote: -----> Null Buildpack app detected        
       remote: /app/tmp/buildpacks/6ccd2b9f41db985dbeffeeafa11877f50b22197988b0d84a055bdf7fe48efeff0c1fbccefc0b0ef5dd833ede16523bd9cec929eeebc3e0e3bfe67c0f12460db2/export: line 2: /dog:/tmp/build_fdb4f5eaca8a5eb93660ff3af68703df/hello/there:/usr/local/bin:/usr/bin:/bin:/tmp/codon/vendor/bin:/tmp/build_fdb4f5eaca8a5eb93660ff3af68703df/bin:/usr/local/bin:/usr/bin:/bin:/tmp/build_fdb4f5eaca8a5eb93660ff3af68703df/./vendor/bundle/bin:/tmp/build_fdb4f5eaca8a5eb93660ff3af68703df/./vendor/bundle/ruby/2.6.0/bin:/tmp/build_fdb4f5eaca8a5eb93660ff3af68703df/vendor/ruby-2.6.5/bin:/tmp/tmp.HN41keQoC9/bin/:cinco/: No such file or directory        
       remote: /usr/bin/env: ‘bash’: No such file or directory        
       remote:  !     Push rejected, failed to compile Null Buildpack app.        
       remote: 
       remote:  !     Push failed        
       remote: Verifying deploy...        
       remote: 
       remote: !	Push rejected to hatchet-t-3e6bacb18a.        
       remote: 

This is a followup from #965.

In #888 the values in the buildpack's `export` file dropped their quoting for paths causing a few failures that were not caught by our tests.

This PR fixes the underlying problem and adds a test that would have caught the regression. The test uses a buildpack that intentionally adds a quoted newline to it's export file https://github.com/sharpstone/export_path_with_newlines_buildpack/blob/master/bin/compile this gets picked up by the Ruby buildpack and put back into Ruby's `export` file. Since ruby after #888 did not quote these values it would result in this failure:

```
  1) Buildpack internals handles PATH with a newline in it correctly
     Failure/Error:
       Hatchet::Runner.new("default_ruby", buildpacks: buildpacks).deploy do |app|
         expect(app.output).to_not match("No such file or directory")
       end
     Hatchet::App::FailedDeploy:
       Could not deploy 'hatchet-t-3e6bacb18a' (default_ruby) using 'Hatchet::GitApp' at path: './repos/rack/default_ruby'
        if this was expected add `allow_failure: true` to your deploy hash.
       output:
       Buildpack: nil
       Repo: https://git.heroku.com/hatchet-t-3e6bacb18a.git
       remote: Compressing source files... done.        
       remote: Building source:        
       remote: 
       remote: -----> I add newlines to the export file app detected        
       remote: -----> Ruby app detected        
       remote: -----> Installing bundler 1.17.3        
       remote: -----> Removing BUNDLED WITH version in the Gemfile.lock        
       remote: -----> Compiling Ruby/Rack        
       remote: -----> Using Ruby version: ruby-2.6.5        
       remote: -----> Installing dependencies using bundler 1.17.3        
       remote:        Running: bundle install --without development:test --path vendor/bundle --binstubs vendor/bundle/bin -j4 --deployment        
       remote:        Fetching gem metadata from https://rubygems.org/...............        
       remote:        Using bundler 1.17.3        
       remote:        Fetching rack 1.5.0        
       remote:        Installing rack 1.5.0        
       remote:        Bundle complete! 1 Gemfile dependency, 2 gems now installed.        
       remote:        Gems in the groups development and test were not installed.        
       remote:        Bundled gems are installed into `./vendor/bundle`        
       remote:        Bundle completed (3.14s)        
       remote:        Cleaning up the bundler cache.        
       remote: -----> Writing config/database.yml to read from DATABASE_URL        
       remote: -----> Detecting rake tasks        
       remote: 
       remote: ###### WARNING:        
       remote: 
       remote:        You have not declared a Ruby version in your Gemfile.        
       remote:        To set your Ruby version add this line to your Gemfile:        
       remote:        ruby '2.6.5'        
       remote:        # See https://devcenter.heroku.com/articles/ruby-versions for more information.        
       remote: 
       remote: ###### WARNING:        
       remote: 
       remote:        No Procfile detected, using the default web server.        
       remote:        We recommend explicitly declaring how to boot your server process via a Procfile.        
       remote:        https://devcenter.heroku.com/articles/ruby-default-web-server        
       remote: 
       remote: 
       remote: -----> Null Buildpack app detected        
       remote: /app/tmp/buildpacks/6ccd2b9f41db985dbeffeeafa11877f50b22197988b0d84a055bdf7fe48efeff0c1fbccefc0b0ef5dd833ede16523bd9cec929eeebc3e0e3bfe67c0f12460db2/export: line 2: /dog:/tmp/build_fdb4f5eaca8a5eb93660ff3af68703df/hello/there:/usr/local/bin:/usr/bin:/bin:/tmp/codon/vendor/bin:/tmp/build_fdb4f5eaca8a5eb93660ff3af68703df/bin:/usr/local/bin:/usr/bin:/bin:/tmp/build_fdb4f5eaca8a5eb93660ff3af68703df/./vendor/bundle/bin:/tmp/build_fdb4f5eaca8a5eb93660ff3af68703df/./vendor/bundle/ruby/2.6.0/bin:/tmp/build_fdb4f5eaca8a5eb93660ff3af68703df/vendor/ruby-2.6.5/bin:/tmp/tmp.HN41keQoC9/bin/:cinco/: No such file or directory        
       remote: /usr/bin/env: ‘bash’: No such file or directory        
       remote:  !     Push rejected, failed to compile Null Buildpack app.        
       remote: 
       remote:  !     Push failed        
       remote: Verifying deploy...        
       remote: 
       remote: !	Push rejected to hatchet-t-3e6bacb18a.        
       remote: 

```
Copy link

@CaseyFaist CaseyFaist left a comment

Choose a reason for hiding this comment

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

I like the testing approach here! 🎉 approved

@schneems schneems merged commit be6cd5f into master Mar 19, 2020
@schneems schneems deleted the schneems/quote-export-file branch March 19, 2020 16:32
schneems added a commit to heroku/buildpacks-ruby that referenced this pull request Oct 29, 2020
From heroku/heroku-buildpack-ruby#967. I'm moving the behavior from an integration test to a unit test.
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

2 participants