Skip to content

Commit

Permalink
Allow updating of resource policies in compute_disk (#2228)
Browse files Browse the repository at this point in the history
* Allow updating of resource policies in compute_disk

Signed-off-by: Chris Sng <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* 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
  • Loading branch information
chrissng authored and modular-magician committed Sep 18, 2019
1 parent c84606e commit d8cc4e5
Show file tree
Hide file tree
Showing 10 changed files with 190 additions and 14 deletions.
8 changes: 8 additions & 0 deletions products/compute/ansible.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
63 changes: 62 additions & 1 deletion products/compute/api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions products/compute/inspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions products/compute/terraform.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
res["name"] = GetResourceNameFromSelfLink(res["name"].(string))
return res, nil
Original file line number Diff line number Diff line change
@@ -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
6 changes: 0 additions & 6 deletions templates/terraform/examples/resource_policy_basic.tf.erb
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
7 changes: 0 additions & 7 deletions templates/terraform/examples/resource_policy_full.tf.erb
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -27,4 +21,3 @@ resource "google_compute_resource_policy" "bar" {
}
}
}

Original file line number Diff line number Diff line change
@@ -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)}
}
Original file line number Diff line number Diff line change
@@ -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)
}

0 comments on commit d8cc4e5

Please sign in to comment.