-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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: add heroku_space resource #13921
Conversation
d.SetId(space.ID) | ||
log.Printf("[INFO] Space ID: %s", d.Id()) | ||
|
||
setSpaceAttributes(d, (*heroku.Space)(space)) |
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.
If the vendored version of heroku-go is updated, this type conversion can be dropped, as the Space
methods have been updated to all return *heroku.Space
.
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.
Mind adding an inline comment here noting this?
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.
Certainly! Done in e7c904a.
# Create a new Heroku app in test-space | ||
resource "heroku_app" "default" { | ||
name = "test-app" | ||
space = "${heroku_space.default.name}" |
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.
This bit of documentation is only accurate if #13862 is also merged.
if org == "" { | ||
t.Skip("HEROKU_ORGANIZATION is not set; skipping test.") | ||
} | ||
}, |
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.
I also wonder if this needs an additional flag, like HEROKU_ENABLE_SPACES=1
, since not all organizations can currently create private spaces.
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.
This will return an API error if the organization cannot create a private space, yeah?
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.
That's right, this should return a 403 Forbidden error for organizations without space management access.
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.
Okay cool, if the error returned from the API is sufficiently descriptive enough, I don't think we'll need to add the env-var.
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.
LGTM, few minor nits though, nothing major. Thanks!
d.SetId(space.ID) | ||
log.Printf("[INFO] Space ID: %s", d.Id()) | ||
|
||
setSpaceAttributes(d, (*heroku.Space)(space)) |
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.
Mind adding an inline comment here noting this?
if org == "" { | ||
t.Skip("HEROKU_ORGANIZATION is not set; skipping test.") | ||
} | ||
}, |
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.
This will return an API error if the organization cannot create a private space, yeah?
## Example Usage | ||
|
||
```hcl | ||
# Create a new Heroku space |
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.
Please use //
for inline comments in examples
region = "virginia" | ||
} | ||
|
||
# Create a new Heroku app in test-space |
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.
Here as well
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.
Done in 26d6a56.
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.
LGTM! Thanks!
providers/heroku: add heroku_space resource
providers/heroku: add heroku_space resource
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. |
This introduces a new
heroku_space
resource for managing Heroku Private Spaces.It creates, renames, and destroys private spaces in the configured region.
Combined with #13862, this should resolve #7059.
Acceptance test results