-
Notifications
You must be signed in to change notification settings - Fork 91
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
Fix refresh of target if upstream no longer exists #70
Conversation
Provider should not throw error when upstream no longer exists as that means that the target no longer exists either.
Hopefully removes confusion regarding presence of upstream_url on service resource (kevholditch#52 (comment))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the awesome contribution, just a couple of small things to update then its good to go
kong/resource_kong_target_test.go
Outdated
|
||
if len(targets) > 0 { | ||
return fmt.Errorf("expecting zero target resources found %v", len(targets)) | ||
} | ||
|
||
if err != nil { | ||
return fmt.Errorf("Error thrown when trying to read target: %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error string should begin with lowercase letter
|
||
if err != nil { | ||
return fmt.Errorf("could not delete kong upstream: %v", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
combine if and err declaration onto one line...
err := testAccProvider.Meta().(*gokong.KongAdminClient).Upstreams().DeleteById(rs.Primary.ID); if err != nil {
return fmt.Errorf("could not delete kong upstream: %v", err)
}
kong/resource_kong_target.go
Outdated
@@ -55,6 +55,14 @@ func resourceKongTargetCreate(d *schema.ResourceData, meta interface{}) error { | |||
func resourceKongTargetRead(d *schema.ResourceData, meta interface{}) error { | |||
|
|||
var ids = strings.Split(d.Id(), "/") | |||
|
|||
// First check if the upstream exists. If it does not then the target no longer exists either. | |||
upstream, _ := meta.(*gokong.KongAdminClient).Upstreams().GetById(ids[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
combine if and variable declaration onto one line...
upstream, _ := meta.(*gokong.KongAdminClient).Upstreams().GetById(ids[0]); if upstream == nil ...
@kevholditch Thanks for the feedback. Changes have been made. Wasn't familiar with the combination of var initialization and check for result in Go prior to now so have started my day learning something :) |
@onematchfox thanks for the contribution and helping to make the provider better. Cool that you started the day learning something! I'll merge your PR now and the changes will be in |
* Fix refresh of target if upstream no longer exists Provider should not throw error when upstream no longer exists as that means that the target no longer exists either. * Adjust documentation of upstream name to match Kong's description Hopefully removes confusion regarding presence of upstream_url on service resource (#52 (comment)) * goimports * Code review adjustments
Resolves #69