compute_disk and compute_instance depends on resolveImageImageExists and resolveImageFamilyExists

[yukinying]

The configurations of compute_disk and compute_instance contain the image field which would trigger a call to resolveImageImageExists and resolveImageFamilyExists for images in the form of xyz/xyz.

The logic on resolveImage is described in https://github.com/GoogleCloudPlatform/terraform-google-conversion/blob/af8ba4e287cf4b1f5b631dc16ed7b74fb4aff136/google/image.go#L73

// If the given name is a URL, return it.
// If it's in the form projects/{project}/global/images/{image}, return it
// If it's in the form projects/{project}/global/images/family/{family}, return it
// If it's in the form global/images/{image}, return it
// If it's in the form global/images/family/{family}, return it
// If it's in the form family/{family}, check if it's a family in the current project. If it is, return it as global/images/family/{family}.
//    If not, check if it could be a GCP-provided family, and if it exists. If it does, return it as projects/{project}/global/images/family/{family}.
// If it's in the form {project}/{family-or-image}, check if it's an image in the named project. If it is, return it as projects/{project}/global/images/{image}.
//    If not, check if it's a family in the named project. If it is, return it as projects/{project}/global/images/family/{family}.
// If it's in the form {family-or-image}, check if it's an image in the current project. If it is, return it as global/images/{image}.
//    If not, check if it could be a GCP-provided image, and if it exists. If it does, return it as projects/{project}/global/images/{image}.
//    If not, check if it's a family in the current project. If it is, return it as global/images/family/{family}.
//    If not, check if it could be a GCP-provided family, and if it exists. If it does, return it as projects/{project}/global/images/family/{family}

Since GCP API direct fetching in terraform-google-conversion is being discouraged (@danawillow , @chrisst please correct me if I am wrong), terraform-validator currently does not have the service client initialized, and will cause a panic when the resolveImage*Exists calls are triggered.

I understand that developers may found it useful if image path can be provided using shorthand. But on the other hand, to make a analyser (terraform-validator) to be deterministic against a configuration, I think it is important to guide the developers to provide the full path of an image.

Consider the fact that an image like debian-8-jessie-v20170523 could mean an image in the project or an image in projects/debian-cloud/global/images/debian-8-jessie-v20170523, a potential attacker to the system can create an image inside the project with the same name and then override all the potential deployments afterwards. I think it is important to make the image resolution be deterministic to encourage developers to put a full path there. It is possible to do that by making resolveImage*Exists returns an error if c.clientCompute is not configured, and prompt the user to change the image path to full path format.

I want to understand if this is the way to go because this may break existing users (but from the fact that no one reported the issues, I suspect this path is not hit by most users). If so, I can provide the pull request to the upstream dependencies.

[yukinying]

@morgante PTAL
@onetwopunch this is related to the issue you mentioned in #85

[morgante]

Since GCP API direct fetching in terraform-google-conversion is being discouraged (@danawillow , @chrisst please correct me if I am wrong), terraform-validator currently does not have the service client initialized, and will cause a panic when the resolveImage*Exists calls are triggered.

I don't know if that's accurate. API calls make some things more complex but I don't think we can avoid them entirely unfortunately.

I think it is important to make the image resolution be deterministic to encourage developers to put a full path there.

Perhaps, but probably only for users who are actually enforcing image policies.

It is possible to do that by making resolveImage*Exists returns an error if c.clientCompute is not configured, and prompt the user to change the image path to full path format.

Instead of doing this, what if we simply returned back what the user provided?

If they only provided a relative path, we simply use that. Some users won't care, while others who do care could simply write a constraint which requires full paths.

[melinath]

Terraform validator supports an "offline" mode which purposely does not configure the provider for GCP access. However, the expandBootDisk function for compute_instance assumes that the provider is configured for GCP access. We should silently skip this verification (at least when offline), because terraform validator is not responsible for validating that the configuration is correct, just that it complies with policies.

This is somewhat complicated by the fact that expandBootDisk is currently manually "shared" between terraform-provider-google and terraform-google-conversion (i.e. copy-pasted from one to the other.)