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

providers/heroku: import heroku_addon resource #14508

Merged
merged 1 commit into from
May 24, 2017

Conversation

cmorent
Copy link
Contributor

@cmorent cmorent commented May 15, 2017

Adds support for importing addons with the heroku_addon resource.

Resolves #14504

@cmorent
Copy link
Contributor Author

cmorent commented May 16, 2017

I think we will have the same problem here that the one exposed in #14497.

If I try to import an addon, I get the following error:

$ terraform import heroku_addon.foobar <addon_id>
heroku_addon.foobar: Importing from ID "<addon_id>"...
heroku_addon.foobar: Import complete!
  Imported heroku_addon (ID: <addon_id>)
heroku_addon.foo: Refreshing state... (ID: <addon_id>)
Error importing: 1 error(s) occurred:

* heroku_addon.foo (import id: <addon_id>): 1 error(s) occurred:

* import heroku_addon.foobar result: <addon_id>: heroku_addon.foobar: Error retrieving addon: Get https://api.heroku.com/apps//addons/<addon_id>: The requested API endpoint was not found. Are you using the right HTTP verb (i.e. `GET` vs. `POST`), and did you specify your intended version with the `Accept` header?

The resourceHerokuAddonRetrieve function is using the heroku.Service.AddOnInfo function from the heroku-go/v3 package and this function is calling the heroku endpoint requiring both App ID and Add-on ID to retrieve the add-on: /apps/:app_id/addons/:addon_id.

The solution, to me, would be to patch the code from heroku-go/v3 and make it call this endpoint instead. It requires the addon_id only and produces the same result:

$ curl -H "Accept: application/vnd.heroku+json; version=3" -n https://api.heroku.com/apps/<app_id>/addons/<addon_id>                            
{
  "actions":[],
  "config_vars":[],
  "created_at":"2016-04-13T17:26:57Z",
  "id":"<addon_id>",
  "name":"<addon_name>",
  "addon_service":{
    "id":"<addon_service_id>",
    "name":"<addon_service_name>"
  },
  "plan":{
    "id":"<plan_id>",
    "name":"<plan_name>"
  },
  "app":{
    "id":"<app_id>",
    "name":"<app_name>"
  },
  "provider_id":"<provider_id>",
  "state":"<addon_state>",
  "updated_at":"2016-08-02T23:58:42Z",
  "web_url":"<weburl>"
}

Produces the exact same result that the following:

$ curl -H "Accept: application/vnd.heroku+json; version=3" -n https://api.heroku.com/apps/<app_id>/addons/<addon_id>
{
  "actions":[],
  "config_vars":[],
  "created_at":"2016-04-13T17:26:57Z",
  "id":"<addon_id>",
  "name":"<addon_name>",
  "addon_service":{
    "id":"<addon_service_id>",
    "name":"<addon_service_name>"
  },
  "plan":{
    "id":"<plan_id>",
    "name":"<plan_name>"
  },
  "app":{
    "id":"<app_id>",
    "name":"<app_name>"
  },
  "provider_id":"<provider_id>",
  "state":"<addon_state>",
  "updated_at":"2016-08-02T23:58:42Z",
  "web_url":"<weburl>"
}

@cmorent
Copy link
Contributor Author

cmorent commented May 16, 2017

I created the following PR heroku-go/pull/16 that allows to pass an empty appIdentity.

@grubernaut
Copy link
Contributor

Awesome work @cmorent! Once that PR is accepted, you can update the vendored dependency in a separate commit (same pr is fine).

@cmorent
Copy link
Contributor Author

cmorent commented May 16, 2017

Roger that, thanks a lot !

@cmorent
Copy link
Contributor Author

cmorent commented May 17, 2017

The PR cyberdelia/heroku-go/pull/17 has been merged yesterday and is supposed to fix the problem! I'll test it today and bring some feedback as soon as possible !

@cmorent cmorent force-pushed the cm-import-heroku-addon branch from c229942 to 32ac7a4 Compare May 17, 2017 07:40
@cmorent
Copy link
Contributor Author

cmorent commented May 17, 2017

This will be a bit more complicated than expected... The last revisions of github.com/cyberdelia/heroku-go are breaking the tests:

$ make test TEST=./builtin/providers/heroku
==> Checking that code complies with gofmt requirements...
==> Checking AWS provider for unchecked errors...
==> NOTE: at this time we only look for uncheck errors in the AWS package
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/17 17:12:15 Generated command/internal_plugin_list.go
go test -i ./builtin/providers/heroku || exit 1
echo ./builtin/providers/heroku | \
		xargs -t -n4 go test  -timeout=60s -parallel=4
go test -timeout=60s -parallel=4 ./builtin/providers/heroku
# github.com/cmorent/terraform/builtin/providers/heroku
builtin/providers/heroku/resource_heroku_addon.go:181: undefined: heroku.AddOnInfoResult
builtin/providers/heroku/resource_heroku_cert.go:125: undefined: heroku.SSLEndpointInfoResult
builtin/providers/heroku/resource_heroku_addon_test.go:94: undefined: heroku.AddOnInfoResult
builtin/providers/heroku/resource_heroku_addon_test.go:105: undefined: heroku.AddOnInfoResult
builtin/providers/heroku/resource_heroku_app_feature_test.go:65: undefined: heroku.AppFeatureInfoResult
builtin/providers/heroku/resource_heroku_app_feature_test.go:98: undefined: heroku.AppFeatureInfoResult
builtin/providers/heroku/resource_heroku_app_test.go:255: undefined: heroku.AppInfoResult
builtin/providers/heroku/resource_heroku_app_test.go:284: undefined: heroku.AppInfoResult
builtin/providers/heroku/resource_heroku_app_test.go:311: undefined: heroku.AppInfoResult
builtin/providers/heroku/resource_heroku_app_test.go:427: undefined: heroku.AppInfoResult
builtin/providers/heroku/resource_heroku_app_test.go:427: too many errors
FAIL	github.com/cmorent/terraform/builtin/providers/heroku [build failed]
make: *** [test] Error 1

In fact, it seems that it most of these structures where either renamed or removed with the 801c76f revision (the client is generated given a schema). It means that if we want to update the vendored github.com/cyberdelia/heroku-go/v3 we have to rewrite a lot of things.

What do you think @grubernaut ?

@grubernaut
Copy link
Contributor

Interesting... Think we should work on the refactor of the updated vendor in a separate pr, then rebase this work ontop of it.

@cmorent
Copy link
Contributor Author

cmorent commented May 19, 2017

My bad, closed it by mistake, I reopen it with a basic commit and while push everything as soon as #14672 will be merged!

@cmorent cmorent reopened this May 19, 2017
@cmorent cmorent force-pushed the cm-import-heroku-addon branch 2 times, most recently from d1dd1bc to d8d6a5f Compare May 24, 2017 12:00
@cmorent cmorent force-pushed the cm-import-heroku-addon branch from d8d6a5f to d0a0fd2 Compare May 24, 2017 13:18
@cmorent cmorent changed the title [WIP] providers/heroku: import heroku_addon resource providers/heroku: import heroku_addon resource May 24, 2017
@cmorent
Copy link
Contributor Author

cmorent commented May 24, 2017

It works for me after the merge of #14672 and a small fix from my side on this PR:

$ make test TEST=./builtin/providers/heroku
==> Checking that code complies with gofmt requirements...
==> Checking AWS provider for unchecked errors...
==> NOTE: at this time we only look for uncheck errors in the AWS package
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/24 15:18:07 Generated command/internal_plugin_list.go
go test -i ./builtin/providers/heroku || exit 1
echo ./builtin/providers/heroku | \
		xargs -t -n4 go test  -timeout=60s -parallel=4
go test -timeout=60s -parallel=4 ./builtin/providers/heroku
ok  	github.com/cmorent/terraform/builtin/providers/heroku	0.042s
$ terraform import heroku_addon.foobar <addon_id>
heroku_addon.foobar: Importing from ID "<addon_id>"...
heroku_addon.foobar: Import complete!
  Imported heroku_addon (ID: <addon_id>)
heroku_addon.foobar: Refreshing state... (ID: <addon_id>)

Import success! The resources imported are shown above. These are
now in your Terraform state. Import does not currently generate
configuration, so you must do this next. If you do not create configuration
for the above resources, then the next `terraform plan` will mark
them for destruction.

Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@catsby catsby merged commit e676813 into hashicorp:master May 24, 2017
catsby added a commit that referenced this pull request May 24, 2017
* 'master' of github.com:hashicorp/terraform:
  Update CHANGELOG.md
  providers/heroku: import heroku_addon resource (#14508)
vanstee pushed a commit to vanstee/terraform that referenced this pull request Sep 28, 2017
@ghost
Copy link

ghost commented Apr 12, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

heroku_addon does not support import
3 participants