From fffc70438cba37cb4f190822e2970b89bd6fda72 Mon Sep 17 00:00:00 2001 From: Sarah French <15078782+SarahFrench@users.noreply.github.com> Date: Tue, 25 Apr 2023 16:53:04 +0100 Subject: [PATCH] Update list implementation in `google_dns_keys` data source (#7748) * Change computed blocks fields to `schema.ListAttribute` This is required to fix plan-time issues like : https://github.com/hashicorp/terraform-provider-google/issues/14298 * Update acceptance tests to check computed blocks are accessible at plan time --- .../data_sources/data_source_dns_keys.go | 113 +++++------------- .../tests/data_source_dns_key_test.go.erb | 26 +++- 2 files changed, 55 insertions(+), 84 deletions(-) diff --git a/mmv1/third_party/terraform/data_sources/data_source_dns_keys.go b/mmv1/third_party/terraform/data_sources/data_source_dns_keys.go index 54e1cd07ffe4..b4d33497e444 100644 --- a/mmv1/third_party/terraform/data_sources/data_source_dns_keys.go +++ b/mmv1/third_party/terraform/data_sources/data_source_dns_keys.go @@ -3,6 +3,7 @@ package google import ( "context" "fmt" + "google.golang.org/api/dns/v1" "github.com/hashicorp/terraform-plugin-framework/attr" @@ -101,17 +102,21 @@ func (d *GoogleDnsKeysDataSource) Schema(ctx context.Context, req datasource.Sch MarkdownDescription: "DNS keys identifier", Computed: true, }, - }, - Blocks: map[string]schema.Block{ - "zone_signing_keys": schema.ListNestedBlock{ + // Issue with using computed blocks in the plugin framework with protocol 5 + // See: https://developer.hashicorp.com/terraform/plugin/framework/migrating/attributes-blocks/blocks-computed#framework + "zone_signing_keys": schema.ListAttribute{ Description: "A list of Zone-signing key (ZSK) records.", MarkdownDescription: "A list of Zone-signing key (ZSK) records.", - NestedObject: dnsKeyObject(), + ElementType: dnsKeyObject(), + Computed: true, }, - "key_signing_keys": schema.ListNestedBlock{ + // Issue with using computed blocks in the plugin framework with protocol 5 + // See: https://developer.hashicorp.com/terraform/plugin/framework/migrating/attributes-blocks/blocks-computed#framework + "key_signing_keys": schema.ListAttribute{ Description: "A list of Key-signing key (KSK) records.", MarkdownDescription: "A list of Key-signing key (KSK) records.", - NestedObject: kskObject(), + ElementType: kskObject(), + Computed: true, }, }, } @@ -204,76 +209,24 @@ func (d *GoogleDnsKeysDataSource) Read(ctx context.Context, req datasource.ReadR // dnsKeyObject is a helper function for the zone_signing_keys schema and // is also used by key_signing_keys schema (called in kskObject defined below) -func dnsKeyObject() schema.NestedBlockObject { - return schema.NestedBlockObject{ - Attributes: map[string]schema.Attribute{ - "algorithm": schema.StringAttribute{ - Description: "String mnemonic specifying the DNSSEC algorithm of this key. Immutable after creation time. " + - "Possible values are `ecdsap256sha256`, `ecdsap384sha384`, `rsasha1`, `rsasha256`, and `rsasha512`.", - MarkdownDescription: "String mnemonic specifying the DNSSEC algorithm of this key. Immutable after creation time. " + - "Possible values are `ecdsap256sha256`, `ecdsap384sha384`, `rsasha1`, `rsasha256`, and `rsasha512`.", - Computed: true, - }, - "creation_time": schema.StringAttribute{ - Description: "The time that this resource was created in the control plane. This is in RFC3339 text format.", - MarkdownDescription: "The time that this resource was created in the control plane. This is in RFC3339 text format.", - Computed: true, - }, - "description": schema.StringAttribute{ - Description: "A mutable string of at most 1024 characters associated with this resource for the user's convenience.", - MarkdownDescription: "A mutable string of at most 1024 characters associated with this resource for the user's convenience.", - Computed: true, - }, - "id": schema.StringAttribute{ - Description: "Unique identifier for the resource; defined by the server.", - MarkdownDescription: "Unique identifier for the resource; defined by the server.", - Computed: true, - }, - "is_active": schema.BoolAttribute{ - Description: "Active keys will be used to sign subsequent changes to the ManagedZone. " + - "Inactive keys will still be present as DNSKEY Resource Records for the use of resolvers validating existing signatures.", - MarkdownDescription: "Active keys will be used to sign subsequent changes to the ManagedZone. " + - "Inactive keys will still be present as DNSKEY Resource Records for the use of resolvers validating existing signatures.", - Computed: true, - }, - "key_length": schema.Int64Attribute{ - Description: "Length of the key in bits. Specified at creation time then immutable.", - MarkdownDescription: "Length of the key in bits. Specified at creation time then immutable.", - Computed: true, - }, - "key_tag": schema.Int64Attribute{ - Description: "The key tag is a non-cryptographic hash of the a DNSKEY resource record associated with this DnsKey. " + - "The key tag can be used to identify a DNSKEY more quickly (but it is not a unique identifier). " + - "In particular, the key tag is used in a parent zone's DS record to point at the DNSKEY in this child ManagedZone. " + - "The key tag is a number in the range [0, 65535] and the algorithm to calculate it is specified in RFC4034 Appendix B.", - MarkdownDescription: "The key tag is a non-cryptographic hash of the a DNSKEY resource record associated with this DnsKey. " + - "The key tag can be used to identify a DNSKEY more quickly (but it is not a unique identifier). " + - "In particular, the key tag is used in a parent zone's DS record to point at the DNSKEY in this child ManagedZone. " + - "The key tag is a number in the range [0, 65535] and the algorithm to calculate it is specified in RFC4034 Appendix B.", - Computed: true, - }, - "public_key": schema.StringAttribute{ - Description: "Base64 encoded public half of this key.", - MarkdownDescription: "Base64 encoded public half of this key.", - Computed: true, - }, - }, - Blocks: map[string]schema.Block{ - "digests": schema.ListNestedBlock{ - Description: "A list of cryptographic hashes of the DNSKEY resource record associated with this DnsKey. These digests are needed to construct a DS record that points at this DNS key.", - MarkdownDescription: "A list of cryptographic hashes of the DNSKEY resource record associated with this DnsKey. These digests are needed to construct a DS record that points at this DNS key.", - NestedObject: schema.NestedBlockObject{ - Attributes: map[string]schema.Attribute{ - "digest": schema.StringAttribute{ - Description: "The base-16 encoded bytes of this digest. Suitable for use in a DS resource record.", - MarkdownDescription: "The base-16 encoded bytes of this digest. Suitable for use in a DS resource record.", - Computed: true, - }, - "type": schema.StringAttribute{ - Description: "Specifies the algorithm used to calculate this digest. Possible values are `sha1`, `sha256` and `sha384`", - MarkdownDescription: "Specifies the algorithm used to calculate this digest. Possible values are `sha1`, `sha256` and `sha384`", - Computed: true, - }, +func dnsKeyObject() types.ObjectType { + // See comments in Schema function + // Also: https://github.com/hashicorp/terraform-plugin-framework/issues/214#issuecomment-1194666110 + return types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "algorithm": types.StringType, + "creation_time": types.StringType, + "description": types.StringType, + "id": types.StringType, + "is_active": types.BoolType, + "key_length": types.Int64Type, + "key_tag": types.Int64Type, + "public_key": types.StringType, + "digests": types.ListType{ + ElemType: types.ObjectType{ + AttrTypes: map[string]attr.Type{ + "digest": types.StringType, + "type": types.StringType, }, }, }, @@ -282,14 +235,10 @@ func dnsKeyObject() schema.NestedBlockObject { } // kskObject is a helper function for the key_signing_keys schema -func kskObject() schema.NestedBlockObject { +func kskObject() types.ObjectType { nbo := dnsKeyObject() - nbo.Attributes["ds_record"] = schema.StringAttribute{ - Description: "The DS record based on the KSK record.", - MarkdownDescription: "The DS record based on the KSK record.", - Computed: true, - } + nbo.AttrTypes["ds_record"] = types.StringType return nbo } diff --git a/mmv1/third_party/terraform/tests/data_source_dns_key_test.go.erb b/mmv1/third_party/terraform/tests/data_source_dns_key_test.go.erb index 3c23a0be71bc..0b21f6a1d226 100644 --- a/mmv1/third_party/terraform/tests/data_source_dns_key_test.go.erb +++ b/mmv1/third_party/terraform/tests/data_source_dns_key_test.go.erb @@ -29,7 +29,7 @@ func TestAccDataSourceDNSKeys_basic(t *testing.T) { Source: "hashicorp/google<%= "-" + version unless version == 'ga' -%>", }, }, - Config: testAccDataSourceDNSKeysConfig(dnsZoneName, "on"), + Config: testAccDataSourceDNSKeysConfigWithOutputs(dnsZoneName, "on"), Check: resource.ComposeTestCheckFunc( testAccDataSourceDNSKeysDSRecordCheck("data.google_dns_keys.foo_dns_key"), resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "key_signing_keys.#", "1"), @@ -43,7 +43,7 @@ func TestAccDataSourceDNSKeys_basic(t *testing.T) { }, { ProtoV5ProviderFactories: ProtoV5ProviderFactories(t), - Config: testAccDataSourceDNSKeysConfig(dnsZoneName, "on"), + Config: testAccDataSourceDNSKeysConfigWithOutputs(dnsZoneName, "on"), Check: resource.ComposeTestCheckFunc( testAccDataSourceDNSKeysDSRecordCheck("data.google_dns_keys.foo_dns_key"), resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "key_signing_keys.#", "1"), @@ -132,3 +132,25 @@ data "google_dns_keys" "foo_dns_key_id" { } `, dnsZoneName, dnsZoneName, dnssecStatus) } + +// This function extends the config returned from the `testAccDataSourceDNSKeysConfig` function +// to include output blocks that access the `key_signing_keys` and `zone_signing_keys` attributes. +// These are null if DNSSEC is not enabled. +func testAccDataSourceDNSKeysConfigWithOutputs(dnsZoneName, dnssecStatus string) string { + + config := testAccDataSourceDNSKeysConfig(dnsZoneName, dnssecStatus) + config = config + ` +# These outputs will cause an error if google_dns_managed_zone.foo.dnssec_config.state == "off" + +output "test_access_google_dns_keys_key_signing_keys" { + description = "Testing that we can access a value in key_signing_keys ok as a computed block" + value = data.google_dns_keys.foo_dns_key_id.key_signing_keys[0].ds_record +} + +output "test_access_google_dns_keys_zone_signing_keys" { + description = "Testing that we can access a value in zone_signing_keys ok as a computed block" + value = data.google_dns_keys.foo_dns_key_id.zone_signing_keys[0].id +} +` + return config +}