Open cmgsj opened 4 months ago
@nywilken thank you so much for taking the time to review!
I initially thought of adding both fields use_internal_ip
and omit_external_ip
in order to be consistent with the fields the googlecompute
source exposes.
I have identified three possible successful combinations of the above fields according to the following truth table:
case | use_internal_ip | omit_external_ip | result |
---|---|---|---|
1 | true |
true |
✅ internal IP is used, instance does not have an external IP |
2 | true |
false |
✅ internal IP is used, instance has an external IP |
3 | false |
true |
❌ cannot use external IP because instance does not have one |
4 | false |
false |
✅ external IP is used, instance has an external IP |
All the statements you made above are true
except the following two, which are indeed possible, but not necessary:
I think you might not have taken case 2
into account.
Having said that, I do think that your suggestion is very reasonable and will prevent any potential "build-time" errors from case 3
, although it would also prevent case 2
.
In the end I think the disjunction is between allowing all cases (with a the potential error case 3
), or allowing only cases 1
and 4
(without the potential success case 2
).
Please let me know if you agree, and/or what your suggestion would be, thanks in advance!
After trying to use the
googlecompute-export
post-processor, it fails when provisioning the exporter instance because it violates the organization policyconstraints/compute.vmExternalIpAccess
.This is the
packer
code used:and a fragment of the build logs:
This happens because the provisioned export instance always has the fields
omit_external_ip
anduse_internal_ip
set tofalse
, with no way to overwrite them.This PR attempts to change that behavior, by exposing the aforementioned fields in the
googlecompute-export
post-processor config and then using them in thegooglecompute.Config
of the export instance.Example:
Please let me know if a way to circumvent this issue already exists. Any other feedback is welcome as well 🙂!