From d8cc4e5665eea9a4952226dc449c81e30593dad9 Mon Sep 17 00:00:00 2001 From: Chris Sng Date: Thu, 19 Sep 2019 00:44:32 +0800 Subject: [PATCH] Allow updating of resource policies in compute_disk (#2228) * Allow updating of resource policies in compute_disk Signed-off-by: Chris Sng * Address add and removeResourcePolicies API via an attachment resource instead - custom pre_delete code to include resourcePolicies in POST body * Apply suggestions from code review Co-Authored-By: Riley Karson * Fixes based on suggestions from PR review * exclude inspec and ansible. change resource name to reference singular * Remove unused code * Add resource policy attachment test * Use custom checker * Exclude resource policy for ansible and inspec * Update products/compute/ansible.yaml Co-Authored-By: Riley Karson * make imports work * exclude resource policy in ansible overrides also * fix importing of tf resource * resource policy example use google provider, default zone * fix async operation timeout --- products/compute/ansible.yaml | 8 ++ products/compute/api.yaml | 63 ++++++++++++++- products/compute/inspec.yaml | 4 + products/compute/terraform.yaml | 12 +++ ...e_disk_resource_policies_attachment.go.erb | 2 + ...e_disk_resource_policies_attachment.go.erb | 11 +++ .../examples/resource_policy_basic.tf.erb | 6 -- .../examples/resource_policy_full.tf.erb | 7 -- ...e_disk_resource_policies_attachment.go.erb | 11 +++ ...te_disk_resource_policy_attachment_test.go | 80 +++++++++++++++++++ 10 files changed, 190 insertions(+), 14 deletions(-) create mode 100644 templates/terraform/decoders/compute_disk_resource_policies_attachment.go.erb create mode 100644 templates/terraform/encoders/compute_disk_resource_policies_attachment.go.erb create mode 100644 templates/terraform/pre_delete/compute_disk_resource_policies_attachment.go.erb create mode 100644 third_party/terraform/tests/resource_compute_disk_resource_policy_attachment_test.go diff --git a/products/compute/ansible.yaml b/products/compute/ansible.yaml index b2a5c1fb5249..164008c8fda2 100644 --- a/products/compute/ansible.yaml +++ b/products/compute/ansible.yaml @@ -22,6 +22,8 @@ datasources: !ruby/object:Overrides::ResourceOverrides exclude: true RegionAutoscaler: !ruby/object:Overrides::Ansible::ResourceOverride exclude: true + DiskResourcePolicyAttachment: !ruby/object:Overrides::Ansible::ResourceOverride + exclude: true # Readonly resources. DiskType: !ruby/object:Overrides::Ansible::ResourceOverride exclude: true @@ -45,6 +47,8 @@ datasources: !ruby/object:Overrides::ResourceOverrides exclude: true Reservation: !ruby/object:Overrides::Ansible::ResourceOverride exclude: true + ResourcePolicy: !ruby/object:Overrides::Ansible::ResourceOverride + exclude: true RouterNat: !ruby/object:Overrides::Ansible::ResourceOverride exclude: true TargetInstance: !ruby/object:Overrides::Ansible::ResourceOverride @@ -81,6 +85,8 @@ overrides: !ruby/object:Overrides::ResourceOverrides - timeout_seconds RegionBackendService: !ruby/object:Overrides::Ansible::ResourceOverride exclude: true + DiskResourcePolicyAttachment: !ruby/object:Overrides::Ansible::ResourceOverride + exclude: true Disk: !ruby/object:Overrides::Ansible::ResourceOverride properties: sourceSnapshot: !ruby/object:Overrides::Ansible::PropertyOverride @@ -247,6 +253,8 @@ overrides: !ruby/object:Overrides::ResourceOverrides description: | The source snapshot used to create this disk. You can provide this as a partial or full URL to the resource. + ResourcePolicy: !ruby/object:Overrides::Ansible::ResourceOverride + exclude: true Route: !ruby/object:Overrides::Ansible::ResourceOverride properties: nextHopGateway: !ruby/object:Overrides::Ansible::PropertyOverride diff --git a/products/compute/api.yaml b/products/compute/api.yaml index 05e33e759338..6211c0340c35 100644 --- a/products/compute/api.yaml +++ b/products/compute/api.yaml @@ -1303,6 +1303,68 @@ objects: An optional textual description of the valid disk size, such as "10GB-10TB". output: true + - !ruby/object:Api::Resource + name: 'DiskResourcePolicyAttachment' + input: true + base_url: projects/{{project}}/zones/{{zone}}/disks/{{disk}} + create_verb: :POST + create_url: projects/{{project}}/zones/{{zone}}/disks/{{disk}}/addResourcePolicies + delete_verb: :POST + delete_url: projects/{{project}}/zones/{{zone}}/disks/{{disk}}/removeResourcePolicies + self_link: projects/{{project}}/zones/{{zone}}/disks/{{disk}} + nested_query: !ruby/object:Api::Resource::NestedQuery + keys: + - resourcePolicies + is_list_of_ids: true + identity: + - name + description: | + Disk resource policies define a schedule for taking snapshots and a + retention period for these snapshots. + async: !ruby/object:Api::Async + operation: !ruby/object:Api::Async::Operation + kind: 'compute#operation' + path: 'name' + base_url: 'projects/{{project}}/zones/{{zone}}/operations/{{op_id}}' + wait_ms: 1000 + timeouts: !ruby/object:Api::Timeouts + insert_minutes: 4 + delete_minutes: 4 + result: !ruby/object:Api::Async::Result + path: 'targetLink' + status: !ruby/object:Api::Async::Status + path: 'status' + complete: 'DONE' + allowed: + - 'PENDING' + - 'RUNNING' + - 'DONE' + error: !ruby/object:Api::Async::Error + path: 'error/errors' + message: 'message' + parameters: + - !ruby/object:Api::Type::ResourceRef + name: 'disk' + resource: 'Disk' + imports: 'name' + description: | + The name of the disk in which the resource policies are attached to. + required: true + url_param_only: true + - !ruby/object:Api::Type::ResourceRef + name: 'zone' + resource: 'Zone' + imports: 'name' + description: 'A reference to the zone where the disk resides.' + required: true + url_param_only: true + properties: + - !ruby/object:Api::Type::String + name: 'name' + description: | + The resource policy to be attached to the disk for scheduling snapshot + creation. Do not specify the self link. + required: true - !ruby/object:Api::Resource name: 'Disk' # TODO(nelsonjr): Implement disk special actions as defined in the API: @@ -6639,7 +6701,6 @@ objects: - :USE_SERVING_PORT - !ruby/object:Api::Resource name: 'ResourcePolicy' - min_version: beta kind: 'compute#resourcePolicy' base_url: projects/{{project}}/regions/{{region}}/resourcePolicies input: true diff --git a/products/compute/inspec.yaml b/products/compute/inspec.yaml index 363d5069367d..0bc48da65cc2 100644 --- a/products/compute/inspec.yaml +++ b/products/compute/inspec.yaml @@ -19,6 +19,8 @@ overrides: !ruby/object:Overrides::ResourceOverrides exclude: true BackendServiceSignedUrlKey: !ruby/object:Overrides::Inspec::ResourceOverride exclude: true + DiskResourcePolicyAttachment: !ruby/object:Overrides::Inspec::ResourceOverride + exclude: true DiskType: !ruby/object:Overrides::Inspec::ResourceOverride exclude: true ExternalVpnGateway: !ruby/object:Overrides::Inspec::ResourceOverride @@ -72,6 +74,8 @@ overrides: !ruby/object:Overrides::ResourceOverrides exclude: true Reservation: !ruby/object:Overrides::Inspec::ResourceOverride exclude: true + ResourcePolicy: !ruby/object:Overrides::Inspec::ResourceOverride + exclude: true RouterNat: !ruby/object:Overrides::Inspec::ResourceOverride exclude: true Subnetwork: !ruby/object:Overrides::Inspec::ResourceOverride diff --git a/products/compute/terraform.yaml b/products/compute/terraform.yaml index f7b40ebddeb5..0cb6d3f426b9 100644 --- a/products/compute/terraform.yaml +++ b/products/compute/terraform.yaml @@ -311,6 +311,16 @@ overrides: !ruby/object:Overrides::ResourceOverrides state as plain-text. [Read more about sensitive data in state](/docs/state/sensitive-data.html). Because the API does not return the sensitive key value, we cannot confirm or reverse changes to a key outside of Terraform. + DiskResourcePolicyAttachment: !ruby/object:Overrides::Terraform::ResourceOverride + id_format: "{{project}}/{{zone}}/{{disk}}/{{name}}" + custom_code: !ruby/object:Provider::Terraform::CustomCode + encoder: templates/terraform/encoders/compute_disk_resource_policies_attachment.go.erb + decoder: templates/terraform/decoders/compute_disk_resource_policies_attachment.go.erb + pre_delete: templates/terraform/pre_delete/compute_disk_resource_policies_attachment.go.erb + properties: + zone: !ruby/object:Overrides::Terraform::PropertyOverride + required: false + default_from_api: true Disk: !ruby/object:Overrides::Terraform::ResourceOverride # Larger disks were timing out at creation. Bumping up to 5 minutes. # https://github.com/terraform-providers/terraform-provider-google/issues/703 @@ -387,6 +397,8 @@ overrides: !ruby/object:Overrides::ResourceOverrides https://cloud.google.com/compute/docs/disks/customer-managed-encryption#encrypt_a_new_persistent_disk_with_your_own_keys physicalBlockSizeBytes: !ruby/object:Overrides::Terraform::PropertyOverride default_from_api: true + resourcePolicies: !ruby/object:Overrides::Terraform::PropertyOverride + default_from_api: true custom_code: !ruby/object:Provider::Terraform::CustomCode pre_delete: templates/terraform/pre_delete/detach_disk.erb constants: templates/terraform/constants/disk.erb diff --git a/templates/terraform/decoders/compute_disk_resource_policies_attachment.go.erb b/templates/terraform/decoders/compute_disk_resource_policies_attachment.go.erb new file mode 100644 index 000000000000..c981b8c3702f --- /dev/null +++ b/templates/terraform/decoders/compute_disk_resource_policies_attachment.go.erb @@ -0,0 +1,2 @@ +res["name"] = GetResourceNameFromSelfLink(res["name"].(string)) +return res, nil diff --git a/templates/terraform/encoders/compute_disk_resource_policies_attachment.go.erb b/templates/terraform/encoders/compute_disk_resource_policies_attachment.go.erb new file mode 100644 index 000000000000..584c6462f9dd --- /dev/null +++ b/templates/terraform/encoders/compute_disk_resource_policies_attachment.go.erb @@ -0,0 +1,11 @@ +config := meta.(*Config) +project, err := getProject(d, config) +if err != nil { + return nil, err +} + +region := getRegionFromZone(d.Get("zone").(string)) + +obj["resourcePolicies"] = []interface{}{fmt.Sprintf("projects/%s/regions/%s/resourcePolicies/%s", project, region, obj["name"])} +delete(obj, "name") +return obj, nil diff --git a/templates/terraform/examples/resource_policy_basic.tf.erb b/templates/terraform/examples/resource_policy_basic.tf.erb index ff18bc672f42..e3b2fa0b5038 100644 --- a/templates/terraform/examples/resource_policy_basic.tf.erb +++ b/templates/terraform/examples/resource_policy_basic.tf.erb @@ -1,10 +1,4 @@ -provider "google-beta" { - region = "us-central1" - zone = "us-central1-a" -} - resource "google_compute_resource_policy" "foo" { - provider = "google-beta" name = "<%= ctx[:vars]['name'] %>" region = "us-central1" snapshot_schedule_policy { diff --git a/templates/terraform/examples/resource_policy_full.tf.erb b/templates/terraform/examples/resource_policy_full.tf.erb index 1bf9b44000b7..b7de3477b841 100644 --- a/templates/terraform/examples/resource_policy_full.tf.erb +++ b/templates/terraform/examples/resource_policy_full.tf.erb @@ -1,10 +1,4 @@ -provider "google-beta" { - region = "us-central1" - zone = "us-central1-a" -} - resource "google_compute_resource_policy" "bar" { - provider = "google-beta" name = "<%= ctx[:vars]['name'] %>" region = "us-central1" snapshot_schedule_policy { @@ -27,4 +21,3 @@ resource "google_compute_resource_policy" "bar" { } } } - diff --git a/templates/terraform/pre_delete/compute_disk_resource_policies_attachment.go.erb b/templates/terraform/pre_delete/compute_disk_resource_policies_attachment.go.erb new file mode 100644 index 000000000000..82bee6607463 --- /dev/null +++ b/templates/terraform/pre_delete/compute_disk_resource_policies_attachment.go.erb @@ -0,0 +1,11 @@ +obj = make(map[string]interface{}) + +// projects/{project}/regions/{region}/resourcePolicies/{resourceId} +region := getRegionFromZone(d.Get("zone").(string)) + +name, err := expandComputeDiskResourcePolicyAttachmentName(d.Get("name"), d, config) +if err != nil { + return err +} else if v, ok := d.GetOkExists("name"); !isEmptyValue(reflect.ValueOf(name)) && (ok || !reflect.DeepEqual(v, name)) { + obj["resourcePolicies"] = []interface{}{fmt.Sprintf("projects/%s/regions/%s/resourcePolicies/%s", project, region, name)} +} diff --git a/third_party/terraform/tests/resource_compute_disk_resource_policy_attachment_test.go b/third_party/terraform/tests/resource_compute_disk_resource_policy_attachment_test.go new file mode 100644 index 000000000000..fbfbeab07149 --- /dev/null +++ b/third_party/terraform/tests/resource_compute_disk_resource_policy_attachment_test.go @@ -0,0 +1,80 @@ +package google + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform/helper/acctest" + "github.com/hashicorp/terraform/helper/resource" +) + +func TestAccComputeDiskResourcePolicyAttachment_basic(t *testing.T) { + t.Parallel() + + diskName := fmt.Sprintf("tf-test-%s", acctest.RandString(10)) + policyName := fmt.Sprintf("tf-test-policy-%s", acctest.RandString(10)) + policyName2 := fmt.Sprintf("tf-test-policy-%s", acctest.RandString(10)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccComputeDiskResourcePolicyAttachment_basic(diskName, policyName), + }, + { + ResourceName: "google_compute_disk_resource_policy_attachment.foobar", + // ImportStateId: fmt.Sprintf("projects/%s/regions/%s/resourcePolicies/%s", getTestProjectFromEnv(), "us-central1", policyName), + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccComputeDiskResourcePolicyAttachment_basic(diskName, policyName2), + }, + { + ResourceName: "google_compute_disk_resource_policy_attachment.foobar", + // ImportStateId: fmt.Sprintf("projects/%s/regions/%s/resourcePolicies/%s", getTestProjectFromEnv(), "us-central1", policyName), + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func testAccComputeDiskResourcePolicyAttachment_basic(diskName, policyName string) string { + return fmt.Sprintf(` +data "google_compute_image" "my_image" { + family = "debian-9" + project = "debian-cloud" +} + +resource "google_compute_disk" "foobar" { + name = "%s" + image = "${data.google_compute_image.my_image.self_link}" + size = 50 + type = "pd-ssd" + zone = "us-central1-a" + labels = { + my-label = "my-label-value" + } +} + +resource "google_compute_resource_policy" "foobar" { + name = "%s" + region = "us-central1" + snapshot_schedule_policy { + schedule { + daily_schedule { + days_in_cycle = 1 + start_time = "04:00" + } + } + } +} + +resource "google_compute_disk_resource_policy_attachment" "foobar" { + name = google_compute_resource_policy.foobar.name + disk = google_compute_disk.foobar.name + zone = "us-central1-a" +}`, diskName, policyName) +}