add use_internal_ip and omit_external_ip fields to export post-processor

[cmgsj]

After trying to use the googlecompute-export post-processor, it fails when provisioning the exporter instance because it violates the organization policy constraints/compute.vmExternalIpAccess.

This is the packer code used:

source "googlecompute" "vm" {
  // ...
}

build {
  name = "main"
  sources = ["sources.googlecompute.vm"]
  // ...
  post-processor "googlecompute-export" {
    paths = [
      "gs://$BUCKET_NAME/$IMAGE_NAME.tar.gz",
    ]
    keep_input_artifact = true
  }
}

and a fragment of the build logs:

==> main.googlecompute.vm: Running post-processor: (type googlecompute-export)
==> main.googlecompute.vm (googlecompute-export): Exporting image $IMAGE_NAME to destination: [gs://$BUCKET_NAME/$IMAGE_NAME.tar.gz]
==> main.googlecompute.vm (googlecompute-export): Creating temporary RSA SSH key for instance...
==> main.googlecompute.vm (googlecompute-export): Using image: debian-9-worker-v20200616
==> main.googlecompute.vm (googlecompute-export): Creating instance...
    main.googlecompute.vm (googlecompute-export): Loading zone: $ZONE
    main.googlecompute.vm (googlecompute-export): Loading machine type: n1-highcpu-4
    main.googlecompute.vm (googlecompute-export): Requesting instance creation...
==> main.googlecompute.vm (googlecompute-export): Error creating instance: googleapi: Error 412: Constraint constraints/compute.vmExternalIpAccess violated for project $PROJECT_NUMBER. Add instance projects/$PROJECT_ID/zones/$ZONE/instances/$IMAGE_NAME-exporter to the constraint to use external IP with it., conditionNotMet

This happens because the provisioned export instance always has the fields omit_external_ip and use_internal_ip set to false, 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 the googlecompute.Config of the export instance.

Example:

source "googlecompute" "vm" {
  // ...
}

build {
  name = "main"
  sources = ["sources.googlecompute.vm"]
  // ...
  post-processor "googlecompute-export" {
    paths = [
      "gs://$BUCKET_NAME/$IMAGE_NAME.tar.gz",
    ]
    keep_input_artifact = true
    omit_external_ip    = true
    use_internal_ip     = true
  }
}

Please let me know if a way to circumvent this issue already exists. Any other feedback is welcome as well 🙂!

[hashicorp-cla]

All committers have signed the CLA.

[nywilken]

Hi there apologies for the delayed review here. Our focus has been on Packer itself trying to resolve issues with plugin loading. Looking at the changes and reference issue I can understand the need for this feature.

I left a few suggestions on the change based on my understanding on how Omit and Use internal are working. Please let me know if you have any questions or if my understanding is off.

[cmgsj]

@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:

• When UseInternalIP is true OmitExternalIP must be True.
• OmitExternalIP can not be false when UseInternal is True.

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!

[cmgsj]

@nywilken whenever you have some time, please let me know if my comment above makes more sense, or if you think it would be better to go forward with the changes you had originally suggested.

Thanks.