Update export_disk_ext.sh #1687
Conversation
The test on line 22 introduces unexpected errors by testing for access to something different than what's actually needed. I've modified it to test reachability to the Storage API, not www.googleapis.com. Most enterprise GCP customers will configure firewall and DNS controls for the Restricted VIP. In that environment, the attempt to curl www.googleapis.com will fail, even though Storage and other APIs can be accessed. This forces a weird workaround to change DNS and firewall rules to allow some limited egress to the private VIP, which adds complexity and compromises security. Improving the test condition allows this code to run without forcing weird workarounds.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: eeaton The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @eeaton. Thanks for your PR. I'm waiting for a GoogleCloudPlatform member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
On further review, I realized my initial solution could be brittle because it assumes a GCP public bucket will never change. In this version, "gsutil ls" is a simple check to list buckets that doesn't take any parameters, but if it fails will indicate that network access or permissions to list buckets are not available.
zoran15
requested review from
dntczdx and
EricEdens
and removed request for
gaohannk
Thanks for the contribution. In addition to accessing GCS, image export script also does GCE operations. In particular, disk resize using |
Added a more robust test condition and error message for both Storage and Compute endpoints. This test removes the original curl to the Discovery API, which can be accessed without authentication but tests access to an unrelated API. Instead, this test now checks HTTP Statuscodes for these endpoints. Without specific parameters, the endpoints will return 4XX; that is acceptable because we're yet not testing permission to reach a specific resource. Returning any status code equal to or greater than 200 indicates that at least the VM can communicate to this endpoint. I've also changed the error message to be more useful. The previous message over-emphasized Private Google Access, which made troubleshooting difficult.
|
I've suggested a more robust test that checks both compute.googleapis.com and storage.googleapis.com endpoints by checking HTTP status codes. This test will work regardless of whether an enterprise client has configured Firewall and DNS settings for the Restricted VIP. It also doesn't assume the hack of the public bucket as in my earlier commit. Please review, thanks! |
The test on line 22 introduces unexpected errors by testing for access to something different than what's actually needed. I've modified it to test reachability to the Storage API, not www.googleapis.com.
Most enterprise GCP customers will configure firewall and DNS controls for the Restricted VIP. In that environment, the attempt to curl www.googleapis.com will fail, even though Storage and other APIs can be accessed. This forces a weird workaround to change DNS and firewall rules to allow some limited egress to the private VIP, which adds complexity and compromises security. Improving the test condition allows this code to run without forcing weird workarounds.