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.

Sign up for GitHub

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

Jump to bottom

config: add docs on permissions for public IP #372

Merged
merged 2 commits into from
Apr 24, 2023
Merged

config: add docs on permissions for public IP #372

merged 2 commits into from
Apr 24, 2023

Conversation

lbajolet-hashicorp
Copy link
Contributor

When the `associate_public_ip_address' configuration attribute is set in a config without a VPC or a Subnet, we now attempt to infer those from the default VPC and subnets.

These actions require some extra permissions, so we reflect this in the docs for the attribute.

@lbajolet-hashicorp lbajolet-hashicorp requested a review from a team as a code owner April 21, 2023 15:50
nywilken
nywilken suggested changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

I expect to see the generated partials updated for this change. But it looks like the check is not completing the call to make generate before completing. Feel free to run locally and commit the changes.

builder/common/run_config.go Outdated Show resolved Hide resolved
@lbajolet-hashicorp
config: add docs on permissions for public IP
6cf772c 
When the `associate_public_ip_address' configuration attribute is set in
a config without a VPC or a Subnet, we now attempt to infer those from
the default VPC and subnets.

These actions require some extra permissions, so we reflect this in the
docs for the attribute.
@lbajolet-hashicorp
docs: commit generated changes to fast-launch
44c7898 
When the fast-launch configuration was committed to the plugin's code,
we forgot to generate the partials, so they where not up-to-date
compared to the code.
@lbajolet-hashicorp lbajolet-hashicorp force-pushed the doc_associate_public_ip_permissions branch from 919bf13 to 44c7898 Compare April 24, 2023 13:31
@lbajolet-hashicorp
Copy link
Contributor Author

Good call regarding the partials. When re-compiling them locally I noticed the fast-launch ones were not up-to-date as well, presumably I amended them and forgot to run make generate.
I'll look into the workflow, it should report such discrepancies.

nywilken
nywilken approved these changes Apr 24, 2023
View reviewed changes
Copy link
Contributor

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

Changes look good. Feel free to update the docs.zip associated with the 1.2.4 release once merged. Otherwise you can wait until #373 is merged and released.

@lbajolet-hashicorp
Copy link
Contributor Author

I think we can wait until we've released 1.2.5 if we're to do it quickly.
However, I'll add some notes to the 1.2.3 and 1.2.4 releases, unless you have an objection against that.

@nywilken
Copy link
Contributor

I think we can wait until we've released 1.2.5 if we're to do it quickly.
However, I'll add some notes to the 1.2.3 and 1.2.4 releases, unless you have an objection against that.

sounds good. looking at the other PR now. Adding to the release notes is a good call as well.

@lbajolet-hashicorp
Copy link
Contributor Author

Good, thanks for the confirmation! I'll add a paragraph to both notes right now.

@lbajolet-hashicorp lbajolet-hashicorp merged commit 29176e2 into main Apr 24, 2023
@lbajolet-hashicorp lbajolet-hashicorp deleted the doc_associate_public_ip_permissions branch April 24, 2023 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nywilken nywilken nywilken approved these changes

Assignees
No one assigned
Labels
docs enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lbajolet-hashicorp @nywilken