Skip to content

Commit

Permalink
Allow app.build_stack to be container for ensureContainerStack (h…
Browse files Browse the repository at this point in the history
…eroku#2952)

* Allow `app.build_stack` to be `container` for `ensureContainerStack`

When migrating from  a non-container stack to a container stack, the `app.build_stack` is set to `container`. This situation should be allowed in `ensureContainerStack` to avoid an error when `container:*` commands are run.

* fixup! Allow `app.build_stack` to be `container` for `ensureContainerStack`

* Update messaging to offer switching stacks as an additional resolution.

* Fix tests to match new error message

---------

Co-authored-by: Eric Black <[email protected]>
  • Loading branch information
jabrown85 and eablack authored Jul 26, 2024
1 parent 718ceb8 commit 3bba8e1
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 5 deletions.
10 changes: 7 additions & 3 deletions packages/cli/src/lib/container/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@ const stackLabelMap: { [key: string]: string} = {
* @returns {null} null
*/
export function ensureContainerStack(app: Heroku.App, cmd: string): void {
if (app.stack?.name !== 'container') {
const appLabel = (app.stack?.name && stackLabelMap[app.stack.name]) || app.stack?.name
const buildStack = app.build_stack?.name
const appStack = app.stack?.name
const allowedStack = 'container'

// either can be container stack and are allowed
if (buildStack !== allowedStack && appStack !== allowedStack) {
let message = 'This command is for Docker apps only.'
if (['push', 'release'].includes(cmd)) {
message += ` Run ${color.cyan('git push heroku main')} to deploy your ${color.cyan(app.name)} ${color.cyan(appLabel)} app instead.`
message += ` Switch stacks by running ${color.cmd('heroku stack:set container')}. Or, to deploy ${color.app(app.name)} with ${color.yellow(appStack)}, run ${color.cmd('git push heroku main')} instead.`
}

ux.error(message, {exit: 1})
Expand Down
31 changes: 30 additions & 1 deletion packages/cli/test/unit/commands/container/push.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,40 @@ describe('container push', function () {
error = error_
})
const {message, oclif} = error as unknown as CLIError
expect(message).to.equal(`This command is for Docker apps only. Run ${color.cyan('git push heroku main')} to deploy your ${color.cyan('testapp')} ${color.cyan('heroku-24')} app instead.`)
expect(message).to.equal(`This command is for Docker apps only. Switch stacks by running ${color.cmd('heroku stack:set container')}. Or, to deploy ${color.app('testapp')} with ${color.yellow('heroku-24')}, run ${color.cmd('git push heroku main')} instead.`)
expect(oclif.exit).to.equal(1)
})
})

context('when the app build_stack is container', function () {
beforeEach(function () {
api
.get('/apps/testapp')
.reply(200, {name: 'testapp', stack: {name: 'heroku-22'}, build_stack: {name: 'container'}})
})

it('allows push to the docker registry', async function () {
const dockerfiles = sandbox.stub(DockerHelper, 'getDockerfiles')
.returns(['/path/to/Dockerfile'])
const build = sandbox.stub(DockerHelper, 'buildImage')
.withArgs('/path/to/Dockerfile', 'registry.heroku.com/testapp/web', [])
const push = sandbox.stub(DockerHelper, 'pushImage')
.withArgs('registry.heroku.com/testapp/web')

await runCommand(Cmd, [
'--app',
'testapp',
'web',
])

expect(stdout.output).to.contain('Building web (/path/to/Dockerfile)')
expect(stdout.output).to.contain('Pushing web (/path/to/Dockerfile)')
sandbox.assert.calledOnce(dockerfiles)
sandbox.assert.calledOnce(build)
sandbox.assert.calledOnce(push)
})
})

context('when the app is a container app', function () {
beforeEach(function () {
api
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('container release', function () {
})

const {message, oclif} = error as unknown as CLIError
expect(message).to.equal(`This command is for Docker apps only. Run ${color.cyan('git push heroku main')} to deploy your ${color.cyan('testapp')} ${color.cyan('heroku-24')} app instead.`)
expect(message).to.equal(`This command is for Docker apps only. Switch stacks by running ${color.cmd('heroku stack:set container')}. Or, to deploy ${color.app('testapp')} with ${color.yellow('heroku-24')}, run ${color.cmd('git push heroku main')} instead.`)
expect(oclif.exit).to.equal(1)
})

Expand Down

0 comments on commit 3bba8e1

Please sign in to comment.