-
Notifications
You must be signed in to change notification settings - Fork 93
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
feat: add support for http_interface
and http_bind_address
#431
Conversation
523b817
to
86f59d3
Compare
dd847dc
to
6075355
Compare
http_interface
http_interface
and http_bind_address
6075355
to
6c76adc
Compare
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.
Just a couple of comments on the new code, but overall LGTM, thanks for taking the time to delve into this @tenthirtyam
6c76adc
to
8d5fd53
Compare
8d5fd53
to
f1af754
Compare
aff18b8
to
1dedcea
Compare
@nywilken This one is ready now, but need some help with the failing est on |
So looking into the failure it looks like the .web-docs were committed to this branch with a previous version of the packer-sdc. Rebasing onto the latest main doesn't fix it because your changes are being applied on top of what is on main. So in order to fix this issue you must run make generate again and commit the updated .web-doc files. |
That did the trick @nywilken - update in inbound now. |
1dedcea
to
bcabfdc
Compare
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 have a few suggestions on the code. But the approach looks good.
2cccb46
to
4242f44
Compare
@nywilken and @lbajolet-hashicorp - this one has been updated but I still need to run another round of testing before any merge. Let me know how the updates look to you both. |
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 rerolls @tenthirtyam, LGTM!
Marked as draft pending e2e testing post refactor. |
6574850
to
bca604f
Compare
@lbajolet-hashicorp @nywilken - all comments have been addressed and implemented. I've tested various scenarios above with the console and log outputs. Note that the log will also show from where the IP address is used. Ryan |
bca604f
to
c55ba97
Compare
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.
Just left a small nit/question on the hostIP
function, but aside from that, LGTM!
Pre-approving so not to be a blocker later
} | ||
for _, addr := range addrs { | ||
if ipnet, ok := addr.(*net.IPNet); ok && !ipnet.IP.IsLoopback() { | ||
if ipnet.IP.To4() != nil { |
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.
Are we only supporting IPv4 here?
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.
Good catch! Resolved in 4f68e62..
Adds support for `http_interface` and `http_bind_address`. Signed-off-by: Ryan Johnson <ryan@tenthirtyam.org>
c55ba97
to
4f68e62
Compare
Summary
Adds support for
http_interface
andhttp_bind_interface
from the SDK.Note
http_bind_address
andhttp_interface
are mutually exclusive, per the SDK.http_bind_address
andhttp_interface
have higher priority thanhttp_ip
.http_bind_address
is matched against the IP addresses of the host's network interfaces. If no match is found, the plugin will terminate.http_interface
is compared with the host's network interfaces. If there's no corresponding network interface, the plugin will also terminate.http_bind_address
,http_interface
, andhttp_ip
are provided, the plugin will automatically find and use the IP address of the first non-loopback interface forhttp_ip
.http_ip
is provided and the IP address does not exist on any interface, the build will exit.Testing
Basic
✅ PASS :
http_ip
not provided. (Default)✅ PASS :
http_ip
provided with a valid IP address.✅ PASS :
http_ip
provided with an invalid IP address.--> vsphere-iso.appliance: error using IP address 192.168.86.57: 192.168.86.57 is not assigned to an interface
✅ PASS :
http_bind_address
provided with matching interface.✅ PASS :
http_bind_address
provided with non-matching interface.--> vsphere-iso.appliance: 192.168.86.57 is not assigned to an interface
✅ PASS :
http_bind_address
provided withhttp_interface
.Error: 1 error(s) occurred: * either http_interface or http_bind_address can be specified
✅ PASS :
http_bind_address
provided withhttp_ip
.http_bind_address
takes precedence.✅ PASS :
http_interface
provided.🟢 Interface
en0
:🟢 Interface
en1
:✅ PASS :
http_interface
provided with no matching interface.--> vsphere-iso.appliance: error using interface foo: route ip+net: no such network interface
✅ PASS :
http_interface
provided withhttp_ip
.http_interface
takes precedence.