Various bug fixes for `heroku_app` resource #257

davidji99 · 2020-03-21T09:04:44Z

Resolves #249
Resolves #118

While looking into Imported heroku_app should include buildpacks #249, it didn't seem heroku_app's import function set ALL of the attributes of the app. So I changed this function to use the same read function as when creating/updating a heroku_app.
(EDIT: THIS change was undone) I also found it a good opportunity to change the value of heroku_app.id from the app's name to its UUID. We should always be prioritizing and using the resource's UUID whenever possible as the name is not idempotent. This shouldn't cause any major for users other than needing to reference the heroku_app.name instead of heroku_app.id for a child resource such as heroku_addon.app_id to avoid a destroy and recreate. Example state:

  ID = 89e3fff4-29c9-4362-ad21-2e5edcba1e74
  provider = provider.heroku
  acm = false
  git_url = https://git.heroku.com/<SOME_APP_NAME>.git
  heroku_hostname = <SOME_APP_NAME>.herokuapp.com
  internal_routing = false
  name = <SOME_APP_NAME>
  organization.# = 1
  organization.0.locked = true
  organization.0.name = <SOME_ORG_NAME>
  organization.0.personal = false
  region = us
  space =
  stack = heroku-18
  web_url = https://<SOME_APP_NAME>.herokuapp.com/

Fix heroku_app in private space won't lock #118. I don't see how the lock attribute wouldn't be respected in a heroku_app creation but nevertheless, I fixed how this attribute is set in state.
I removed heroku_app.uuid. It should never have been an optional attribute and also redundant since heroku_app.id is now set to its UUID.
I did some basic refactoring in the heroku_app resource code.

mars · 2020-03-31T01:30:02Z

@davidji99 a bunch of acceptance tests failed already in Hashicorp CI, and the full run hasn't finished yet:

17 Tests Failed

TestAccHerokuAddonAttachment_importBasic
TestAccHerokuBuild_importBasic
TestAccHerokuBuild_importAllOpts
TestAccHerokuBuild_importWithFileUrl
TestAccHerokuPipelineCoupling_importBasic
TestAccHerokuAddonAttachment_Basic
TestAccHerokuAddonAttachment_Named
TestAccHerokuApp_Basic
TestAccHerokuApp_ExternallySetBuildpacks
TestAccHerokuBuild_Basic

Most fail with:

After applying this step and refreshing, the plan was not empty

I think this merits running tests locally to suss out the problems 😬

davidji99 · 2020-03-31T02:35:28Z

@mars, I did run some of the tests (I always do) locally but guess not enough of them. Thanks, back to the drawing board.

davidji99 · 2020-04-03T04:53:48Z

TF_ACC=1 go test ./heroku/ -v  -timeout 120m -ldflags="-X=github.com/terraform-providers/terraform-provider-heroku/version.ProviderVersion=test"
=== RUN   TestAccDatasourceHerokuAddon_Basic
--- PASS: TestAccDatasourceHerokuAddon_Basic (16.17s)
=== RUN   TestAccDatasourceHerokuApp_Basic
--- PASS: TestAccDatasourceHerokuApp_Basic (11.91s)
=== RUN   TestAccDatasourceHerokuApp_Advanced
--- SKIP: TestAccDatasourceHerokuApp_Advanced (0.00s)
    data_source_heroku_app_test.go:61: HEROKU_SPACES_ORGANIZATION is not set; skipping test.
=== RUN   TestAccDatasourceHerokuSpacePeeringInfo_Basic
--- SKIP: TestAccDatasourceHerokuSpacePeeringInfo_Basic (0.00s)
    config.go:64: skipping test: config [HEROKU_SPACES_ORGANIZATION] not set
=== RUN   TestAccDatasourceHerokuSpace_Basic
--- SKIP: TestAccDatasourceHerokuSpace_Basic (0.00s)
    config.go:64: skipping test: config [HEROKU_SPACES_ORGANIZATION] not set
=== RUN   TestAccDatasourceHerokuTeam_Basic
--- SKIP: TestAccDatasourceHerokuTeam_Basic (0.00s)
    config.go:64: skipping test: config [HEROKU_TEAM] not set
=== RUN   TestAccHerokuAccountFeature_importBasic
--- PASS: TestAccHerokuAccountFeature_importBasic (1.96s)
=== RUN   TestAccHerokuAddonAttachment_importBasic
--- PASS: TestAccHerokuAddonAttachment_importBasic (14.49s)
=== RUN   TestAccHerokuAddon_importBasic
--- PASS: TestAccHerokuAddon_importBasic (15.45s)
=== RUN   TestAccHerokuAppFeature_importBasic
--- PASS: TestAccHerokuAppFeature_importBasic (11.19s)
=== RUN   TestAccHerokuAppRelease_importBasic
--- SKIP: TestAccHerokuAppRelease_importBasic (0.00s)
    config.go:64: skipping test: config [HEROKU_SLUG_ID] not set
=== RUN   TestAccHerokuApp_importBasic
--- PASS: TestAccHerokuApp_importBasic (11.59s)
=== RUN   TestAccHerokuApp_importOrganization
--- PASS: TestAccHerokuApp_importOrganization (12.92s)
=== RUN   TestAccHerokuApp_importBuildpacks
--- PASS: TestAccHerokuApp_importBuildpacks (11.74s)
=== RUN   TestAccHerokuAppWebhook_importBasic
--- PASS: TestAccHerokuAppWebhook_importBasic (13.16s)
=== RUN   TestAccHerokuBuild_importBasic
--- PASS: TestAccHerokuBuild_importBasic (21.74s)
=== RUN   TestAccHerokuBuild_importAllOpts
--- PASS: TestAccHerokuBuild_importAllOpts (28.80s)
=== RUN   TestAccHerokuBuild_importWithFileUrl
--- PASS: TestAccHerokuBuild_importWithFileUrl (23.78s)
=== RUN   TestAccHerokuCert_importBasic
--- PASS: TestAccHerokuCert_importBasic (27.21s)
=== RUN   TestAccHerokuAppConfigAssociation_importBasic
--- PASS: TestAccHerokuAppConfigAssociation_importBasic (12.57s)
=== RUN   TestAccHerokuConfig_importBasic
--- PASS: TestAccHerokuConfig_importBasic (0.04s)
=== RUN   TestAccHerokuDomain_importBasic
--- PASS: TestAccHerokuDomain_importBasic (16.06s)
=== RUN   TestAccHerokuDrain_importBasic
--- PASS: TestAccHerokuDrain_importBasic (12.50s)
=== RUN   TestAccHerokuFormation_importBasic
--- SKIP: TestAccHerokuFormation_importBasic (0.00s)
    config.go:64: skipping test: config [HEROKU_SLUG_ID] not set
=== RUN   TestAccHerokuPipelineConfigVar_importBasic
--- PASS: TestAccHerokuPipelineConfigVar_importBasic (2.87s)
=== RUN   TestAccHerokuPipelineCoupling_importBasic
--- PASS: TestAccHerokuPipelineCoupling_importBasic (13.49s)
=== RUN   TestAccHerokuPipeline_importBasic
--- PASS: TestAccHerokuPipeline_importBasic (2.32s)
=== RUN   TestAccHerokuSlug_importBasic
--- PASS: TestAccHerokuSlug_importBasic (11.49s)
=== RUN   TestAccHerokuSlug_importAllOpts
--- PASS: TestAccHerokuSlug_importAllOpts (11.44s)
=== RUN   TestAccHerokuSlug_importWithFileUrl
--- PASS: TestAccHerokuSlug_importWithFileUrl (13.30s)
=== RUN   TestAccHerokuSpaceAppAccess_importBasic
--- FAIL: TestAccHerokuSpaceAppAccess_importBasic (0.00s)
    config.go:73: stopping test: config [HEROKU_NON_ADMIN_TEST_USER] must be set
=== RUN   TestAccHerokuTeamCollaborator_importBasic
--- FAIL: TestAccHerokuTeamCollaborator_importBasic (0.00s)
    config.go:73: stopping test: config [HEROKU_NON_ADMIN_TEST_USER] must be set
=== RUN   TestAccHerokuTeamMember_importBasic
--- SKIP: TestAccHerokuTeamMember_importBasic (0.00s)
    config.go:64: skipping test: config [HEROKU_SPACES_ORGANIZATION] not set
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestProviderConfigureUsesHeadersForClient
--- PASS: TestProviderConfigureUsesHeadersForClient (0.00s)
=== RUN   TestAccHerokuAccountFeature_Basic
--- PASS: TestAccHerokuAccountFeature_Basic (2.64s)
=== RUN   TestAccHerokuAddonAttachment_Basic
--- PASS: TestAccHerokuAddonAttachment_Basic (13.79s)
=== RUN   TestAccHerokuAddonAttachment_Named
--- PASS: TestAccHerokuAddonAttachment_Named (13.91s)
=== RUN   TestAccHerokuAddon_Basic
--- PASS: TestAccHerokuAddon_Basic (12.17s)
=== RUN   TestAccHerokuAddon_noPlan
--- PASS: TestAccHerokuAddon_noPlan (16.02s)
=== RUN   TestAccHerokuAddon_CustomName
--- PASS: TestAccHerokuAddon_CustomName (12.84s)
=== RUN   TestAccHerokuAddon_CustomName_Invalid
--- PASS: TestAccHerokuAddon_CustomName_Invalid (0.01s)
=== RUN   TestAccHerokuAddon_CustomName_EmptyString
--- PASS: TestAccHerokuAddon_CustomName_EmptyString (0.01s)
=== RUN   TestAccHerokuAddon_CustomName_FirstCharNum
--- PASS: TestAccHerokuAddon_CustomName_FirstCharNum (0.01s)
=== RUN   TestAccHerokuAddon_Disappears
--- PASS: TestAccHerokuAddon_Disappears (11.09s)
=== RUN   TestAccHerokuAppConfigAssociation_Basic
--- PASS: TestAccHerokuAppConfigAssociation_Basic (12.74s)
=== RUN   TestAccHerokuAppConfigAssociation_Advanced
--- PASS: TestAccHerokuAppConfigAssociation_Advanced (12.68s)
=== RUN   TestAccHerokuAppFeature
--- PASS: TestAccHerokuAppFeature (14.37s)
=== RUN   TestAccHerokuAppRelease_Basic
--- SKIP: TestAccHerokuAppRelease_Basic (0.00s)
    config.go:64: skipping test: config [HEROKU_SLUG_ID] not set
=== RUN   TestAccHerokuAppRelease_OrgBasic
--- SKIP: TestAccHerokuAppRelease_OrgBasic (0.00s)
    config.go:64: skipping test: config [HEROKU_SLUG_ID] not set
=== RUN   TestAccHerokuApp_Basic
--- PASS: TestAccHerokuApp_Basic (10.09s)
=== RUN   TestAccHerokuApp_Disappears
--- PASS: TestAccHerokuApp_Disappears (7.82s)
=== RUN   TestAccHerokuApp_Change
--- PASS: TestAccHerokuApp_Change (14.61s)
=== RUN   TestAccHerokuApp_NukeVars
--- PASS: TestAccHerokuApp_NukeVars (14.41s)
=== RUN   TestAccHerokuApp_Buildpacks
--- FAIL: TestAccHerokuApp_Buildpacks (16.38s)
    testing.go:654: Step 2 error: Check failed: Check 2/3 error: Bad buildpacks: [https://github.com/heroku/heroku-buildpack-multi-procfile heroku/go]
=== RUN   TestAccHerokuApp_ExternallySetBuildpacks
--- FAIL: TestAccHerokuApp_ExternallySetBuildpacks (11.91s)
    testing.go:654: Step 1 error: Check failed: Check 3/3 error: heroku_app.foobar: Attribute 'buildpacks.0' found when not expected
=== RUN   TestAccHerokuApp_ACM
--- PASS: TestAccHerokuApp_ACM (21.15s)
=== RUN   TestAccHerokuApp_Organization
--- PASS: TestAccHerokuApp_Organization (10.98s)
=== RUN   TestAccHerokuApp_Space
--- SKIP: TestAccHerokuApp_Space (0.00s)
    config.go:64: skipping test: config [HEROKU_SPACES_ORGANIZATION] not set
=== RUN   TestAccHerokuApp_Space_Internal
--- SKIP: TestAccHerokuApp_Space_Internal (0.00s)
    config.go:64: skipping test: config [HEROKU_SPACES_ORGANIZATION] not set
=== RUN   TestAccHerokuApp_EmptyConfigVars
--- PASS: TestAccHerokuApp_EmptyConfigVars (10.14s)
=== RUN   TestAccHerokuApp_SensitiveConfigVars
--- PASS: TestAccHerokuApp_SensitiveConfigVars (20.52s)
=== RUN   TestAccHerokuApp_Organization_Locked
--- PASS: TestAccHerokuApp_Organization_Locked (10.63s)
=== RUN   TestAccHerokuAppWebhook_Basic
--- PASS: TestAccHerokuAppWebhook_Basic (17.18s)
=== RUN   TestAccHerokuBuild_Basic
--- PASS: TestAccHerokuBuild_Basic (21.36s)
=== RUN   TestAccHerokuBuild_InsecureUrl
--- PASS: TestAccHerokuBuild_InsecureUrl (0.01s)
=== RUN   TestAccHerokuBuild_NoSource
--- PASS: TestAccHerokuBuild_NoSource (8.24s)
=== RUN   TestAccHerokuBuild_AllOpts
--- PASS: TestAccHerokuBuild_AllOpts (28.36s)
=== RUN   TestAccHerokuBuild_LocalSourceTarball
--- PASS: TestAccHerokuBuild_LocalSourceTarball (38.83s)
=== RUN   TestAccHerokuBuild_LocalSourceTarball_SetChecksum
--- PASS: TestAccHerokuBuild_LocalSourceTarball_SetChecksum (8.27s)
=== RUN   TestAccHerokuBuild_LocalSourceTarball_AllOpts
--- PASS: TestAccHerokuBuild_LocalSourceTarball_AllOpts (23.14s)
=== RUN   TestAccHerokuBuild_LocalSourceDirectory
--- PASS: TestAccHerokuBuild_LocalSourceDirectory (38.08s)
=== RUN   TestAccHerokuBuild_LocalSourceDirectorySelfContained
--- PASS: TestAccHerokuBuild_LocalSourceDirectorySelfContained (22.55s)
=== RUN   TestAccHerokuCert_EU
--- PASS: TestAccHerokuCert_EU (44.92s)
=== RUN   TestAccHerokuCert_US
--- PASS: TestAccHerokuCert_US (45.47s)
=== RUN   TestAccHerokuConfig_Single
--- PASS: TestAccHerokuConfig_Single (0.04s)
=== RUN   TestAccHerokuConfig_Both
--- PASS: TestAccHerokuConfig_Both (0.03s)
=== RUN   TestAccHerokuConfig_Dupe
--- PASS: TestAccHerokuConfig_Dupe (0.02s)
=== RUN   TestAccHerokuDomain_Basic
--- PASS: TestAccHerokuDomain_Basic (15.84s)
=== RUN   TestAccHerokuDrain_Basic
--- PASS: TestAccHerokuDrain_Basic (12.21s)
=== RUN   TestAccHerokuFormationSingleUpdate_WithOrg
--- SKIP: TestAccHerokuFormationSingleUpdate_WithOrg (0.00s)
    config.go:64: skipping test: config [HEROKU_SLUG_ID] not set
=== RUN   TestAccHerokuFormationUpdateFreeDyno
--- SKIP: TestAccHerokuFormationUpdateFreeDyno (0.00s)
    config.go:64: skipping test: config [HEROKU_SLUG_ID] not set
=== RUN   TestAccHerokuPipelineConfigVar_TestStage_Basic
--- PASS: TestAccHerokuPipelineConfigVar_TestStage_Basic (3.19s)
=== RUN   TestAccHerokuPipelineConfigVar_ReviewStage_Basic
--- PASS: TestAccHerokuPipelineConfigVar_ReviewStage_Basic (3.05s)
=== RUN   TestAccHerokuPipelineCoupling_Basic
--- PASS: TestAccHerokuPipelineCoupling_Basic (12.78s)
=== RUN   TestAccHerokuPipeline_Basic
--- PASS: TestAccHerokuPipeline_Basic (3.13s)
=== RUN   TestAccHerokuSlug_Basic
--- PASS: TestAccHerokuSlug_Basic (11.51s)
=== RUN   TestAccHerokuSlug_NoFile
--- PASS: TestAccHerokuSlug_NoFile (8.33s)
=== RUN   TestAccHerokuSlug_AllOpts
--- PASS: TestAccHerokuSlug_AllOpts (11.89s)
=== RUN   TestAccHerokuSlug_WithFile
--- PASS: TestAccHerokuSlug_WithFile (14.76s)
=== RUN   TestAccHerokuSlug_WithRemoteFile
--- PASS: TestAccHerokuSlug_WithRemoteFile (13.17s)
=== RUN   TestAccHerokuSlug_WithInsecureRemoteFile
--- PASS: TestAccHerokuSlug_WithInsecureRemoteFile (0.01s)
=== RUN   TestAccHerokuSlug_WithFile_InPrivateSpace
--- SKIP: TestAccHerokuSlug_WithFile_InPrivateSpace (0.00s)
    config.go:64: skipping test: config [HEROKU_SPACES_ORGANIZATION] not set
=== RUN   TestAccHerokuSpaceAppAccess_Basic
--- FAIL: TestAccHerokuSpaceAppAccess_Basic (0.00s)
    config.go:73: stopping test: config [HEROKU_NON_ADMIN_TEST_USER] must be set
=== RUN   TestAccHerokuSpaceInboundRuleset_Basic
--- PASS: TestAccHerokuSpaceInboundRuleset_Basic (463.74s)
=== RUN   TestAccHerokuSpace_Basic
--- PASS: TestAccHerokuSpace_Basic (455.10s)
=== RUN   TestAccHerokuSpace_Shield
--- PASS: TestAccHerokuSpace_Shield (526.48s)
=== RUN   TestAccHerokuSpace_IPRange
--- PASS: TestAccHerokuSpace_IPRange (466.20s)
=== RUN   TestAccHerokuSpace_CIDRs
--- PASS: TestAccHerokuSpace_CIDRs (905.83s)
=== RUN   TestAccHerokuVPNConnection_basic
--- SKIP: TestAccHerokuVPNConnection_basic (0.00s)
    config.go:64: skipping test: config [HEROKU_SPACES_ORGANIZATION] not set
=== RUN   TestAccHerokuTeamCollaborator_Org
--- FAIL: TestAccHerokuTeamCollaborator_Org (0.00s)
    config.go:73: stopping test: config [HEROKU_NON_ADMIN_TEST_USER] must be set
=== RUN   TestAccHerokuTeamCollaboratorPermsOutOfOrder_Org
--- FAIL: TestAccHerokuTeamCollaboratorPermsOutOfOrder_Org (0.00s)
    config.go:73: stopping test: config [HEROKU_NON_ADMIN_TEST_USER] must be set
=== RUN   TestAccHerokuTeamMember_Org
--- SKIP: TestAccHerokuTeamMember_Org (0.00s)
    config.go:64: skipping test: config [HEROKU_TEST_USER] not set
=== RUN   TestValidateUUID
--- PASS: TestValidateUUID (0.00s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-heroku/heroku	3813.235s
FAIL
make: *** [testacc] Error 1

davidji99 · 2020-04-14T10:05:54Z

@mars This is ready for review again.

mars · 2020-04-17T17:40:42Z

@davidji99 would you please rebase on master to get GitHub Action CI running on this branch?

- change how team/org app is detected for the purpose of setting state attributes - move down Update() method - correctly set team/app attributes

davidji99 · 2020-04-20T09:46:01Z

@mars Done. ✅ Please review.

mars

Wonderful fixes & updates here ✨ Thanks @davidji99

ghost added the size/M label Mar 21, 2020

davidji99 self-assigned this Mar 21, 2020

davidji99 changed the title ~~Imported heroku_app should include buildpacks~~ Fix Imported heroku_app should include buildpacks & update heroku_app resource ID Mar 21, 2020

davidji99 requested review from bernerdschaefer and mars March 21, 2020 09:14

davidji99 force-pushed the issues/249/davidji99-imported-herokuapp-shou branch 3 times, most recently from f0d1617 to 7707b4b Compare March 30, 2020 02:42

davidji99 changed the title ~~Fix Imported heroku_app should include buildpacks & update heroku_app resource ID~~ Various bug fixes for heroku_app resource Mar 30, 2020

davidji99 force-pushed the issues/249/davidji99-imported-herokuapp-shou branch from 7707b4b to 71af60f Compare March 30, 2020 03:09

ghost added size/L documentation and removed size/M labels Mar 30, 2020

davidji99 marked this pull request as ready for review March 30, 2020 03:13

ghost added size/XL and removed size/L labels Mar 30, 2020

mars removed the request for review from bernerdschaefer March 31, 2020 01:30

davidji99 force-pushed the issues/249/davidji99-imported-herokuapp-shou branch 2 times, most recently from 723c65f to 4f72eb3 Compare April 3, 2020 02:35

davidji99 marked this pull request as draft April 13, 2020 08:05

davidji99 marked this pull request as ready for review April 14, 2020 10:05

davidji99 added 4 commits April 20, 2020 11:31

Set heroku_app resource ID to its UUID not name

787616f

always set buildpacks in state for heroku_app

5bd2c64

fix tests that were checking if the app existed [#249]

08c1ada

properly lock org/team app if attr is set [#249]

c655351

davidji99 added 7 commits April 20, 2020 11:31

remove heroku_app.uuid attribute as it's unnecessary [#249]

0f28211

Refactoring the heroku_app resource code: [#249]

677124f

- change how team/org app is detected for the purpose of setting state attributes - move down Update() method - correctly set team/app attributes

add tests for importing app with buildpacks [#249]

6e28ed5

Rename 'organization' to 'team' in heroku_app [#249]

d3325f1

undo changing heroku_app.id value to be the UUID [#249]

da25c14

fix buildpack test check [#249]

bbc4214

fix tests [#249]

96d2979

davidji99 force-pushed the issues/249/davidji99-imported-herokuapp-shou branch from b99dcf9 to 96d2979 Compare April 20, 2020 02:31

fix TestAccHerokuFormationUpdateFreeDyno test [#249]

4ecc2f1

mars approved these changes Apr 21, 2020

View reviewed changes

mars merged commit e60392d into master Apr 21, 2020

davidji99 deleted the issues/249/davidji99-imported-herokuapp-shou branch April 21, 2020 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various bug fixes for `heroku_app` resource #257

Various bug fixes for `heroku_app` resource #257

davidji99 commented Mar 21, 2020 •

edited

Loading

mars commented Mar 31, 2020

davidji99 commented Mar 31, 2020

davidji99 commented Apr 3, 2020

davidji99 commented Apr 14, 2020 •

edited

Loading

mars commented Apr 17, 2020

davidji99 commented Apr 20, 2020 •

edited

Loading

mars left a comment

Various bug fixes for heroku_app resource #257

Various bug fixes for heroku_app resource #257

Conversation

davidji99 commented Mar 21, 2020 • edited Loading

mars commented Mar 31, 2020

davidji99 commented Mar 31, 2020

davidji99 commented Apr 3, 2020

davidji99 commented Apr 14, 2020 • edited Loading

mars commented Apr 17, 2020

davidji99 commented Apr 20, 2020 • edited Loading

mars left a comment

Choose a reason for hiding this comment

Various bug fixes for `heroku_app` resource #257

Various bug fixes for `heroku_app` resource #257

davidji99 commented Mar 21, 2020 •

edited

Loading

davidji99 commented Apr 14, 2020 •

edited

Loading

davidji99 commented Apr 20, 2020 •

edited

Loading