Migrate dns data sources back to the SDK to solve auth issue resulting from migration to plugin framework

.changelog/10368.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+dns: fixed bug where some methods of authentication didn't work when using `dns` data sources
+```

google-beta/acctest/provider_test_utils.go

@@ -73,6 +73,35 @@ func AccTestPreCheck(t *testing.T) {
 	}
 }
 
+// AccTestPreCheck_AdcCredentialsOnly is a PreCheck function for acceptance tests that use ADCs when
+func AccTestPreCheck_AdcCredentialsOnly(t *testing.T) {
+	if v := os.Getenv("GOOGLE_CREDENTIALS_FILE"); v != "" {
+		t.Log("Ignoring GOOGLE_CREDENTIALS_FILE; acceptance test doesn't use credentials other than ADCs")
+	}
+
+	// Fail on set creds
+	if v := transport_tpg.MultiEnvSearch(envvar.CredsEnvVarsExcludingAdcs()); v != "" {
+		t.Fatalf("This acceptance test only uses ADCs, so all of %s must be unset", strings.Join(envvar.CredsEnvVarsExcludingAdcs(), ", "))
+	}
+
+	// Fail on ADC ENV not set
+	if v := os.Getenv("GOOGLE_APPLICATION_CREDENTIALS"); v == "" {
+		t.Fatalf("GOOGLE_APPLICATION_CREDENTIALS must be set for acceptance tests that are dependent on ADCs")
+	}
+
+	if v := transport_tpg.MultiEnvSearch(envvar.ProjectEnvVars); v == "" {
+		t.Fatalf("One of %s must be set for acceptance tests", strings.Join(envvar.ProjectEnvVars, ", "))
+	}
+
+	if v := transport_tpg.MultiEnvSearch(envvar.RegionEnvVars); v == "" {
+		t.Fatalf("One of %s must be set for acceptance tests", strings.Join(envvar.RegionEnvVars, ", "))
+	}
+
+	if v := transport_tpg.MultiEnvSearch(envvar.ZoneEnvVars); v == "" {
+		t.Fatalf("One of %s must be set for acceptance tests", strings.Join(envvar.ZoneEnvVars, ", "))
+	}
+}
+
 // GetTestRegion has the same logic as the provider's GetRegion, to be used in tests.
 func GetTestRegion(is *terraform.InstanceState, config *transport_tpg.Config) (string, error) {
 	if res, ok := is.Attributes["region"]; ok {

google-beta/envvar/envvar_utils.go

@@ -21,6 +21,18 @@ var CredsEnvVars = []string{
 	"GOOGLE_USE_DEFAULT_CREDENTIALS",
 }
 
+// CredsEnvVarsExcludingAdcs returns the contents of CredsEnvVars excluding GOOGLE_APPLICATION_CREDENTIALS
+func CredsEnvVarsExcludingAdcs() []string {
+	envs := CredsEnvVars
+	var filtered []string
+	for _, e := range envs {
+		if e != "GOOGLE_APPLICATION_CREDENTIALS" {
+			filtered = append(filtered, e)
+		}
+	}
+	return filtered
+}
+
 var ProjectNumberEnvVars = []string{
 	"GOOGLE_PROJECT_NUMBER",
 }

google-beta/fwprovider/framework_provider.go

@@ -19,7 +19,6 @@ import (
 	"github.com/hashicorp/terraform-provider-google-beta/google-beta/functions"
 	"github.com/hashicorp/terraform-provider-google-beta/google-beta/fwmodels"
 	"github.com/hashicorp/terraform-provider-google-beta/google-beta/fwtransport"
-	"github.com/hashicorp/terraform-provider-google-beta/google-beta/services/dns"
 	"github.com/hashicorp/terraform-provider-google-beta/google-beta/services/firebase"
 	"github.com/hashicorp/terraform-provider-google-beta/google-beta/services/resourcemanager"
 
@@ -1045,10 +1044,6 @@ func (p *FrameworkProvider) DataSources(_ context.Context) []func() datasource.D
 	return []func() datasource.DataSource{
 		resourcemanager.NewGoogleClientConfigDataSource,
 		resourcemanager.NewGoogleClientOpenIDUserinfoDataSource,
-		dns.NewGoogleDnsManagedZoneDataSource,
-		dns.NewGoogleDnsManagedZonesDataSource,
-		dns.NewGoogleDnsRecordSetDataSource,
-		dns.NewGoogleDnsKeysDataSource,
 		firebase.NewGoogleFirebaseAndroidAppConfigDataSource,
 		firebase.NewGoogleFirebaseAppleAppConfigDataSource,
 		firebase.NewGoogleFirebaseWebAppConfigDataSource,

google-beta/provider/provider_mmv1_resources.go

@@ -233,6 +233,10 @@ var handwrittenDatasources = map[string]*schema.Resource{
 	"google_container_registry_repository":                containeranalysis.DataSourceGoogleContainerRepo(),
 	"google_dataproc_metastore_service":                   dataprocmetastore.DataSourceDataprocMetastoreService(),
 	"google_datastream_static_ips":                        datastream.DataSourceGoogleDatastreamStaticIps(),
+	"google_dns_keys":                                     dns.DataSourceDNSKeys(),
+	"google_dns_managed_zone":                             dns.DataSourceDnsManagedZone(),
+	"google_dns_managed_zones":                            dns.DataSourceDnsManagedZones(),
+	"google_dns_record_set":                               dns.DataSourceDnsRecordSet(),
 	"google_filestore_instance":                           filestore.DataSourceGoogleFilestoreInstance(),
 	"google_iam_policy":                                   resourcemanager.DataSourceGoogleIamPolicy(),
 	"google_iam_role":                                     resourcemanager.DataSourceGoogleIamRole(),

google-beta/services/dns/data_source_dns_key_test.go

@@ -12,88 +12,46 @@ import (
 )
 
 func TestAccDataSourceDNSKeys_basic(t *testing.T) {
-	// TODO: https://github.com/hashicorp/terraform-provider-google/issues/14158
-	acctest.SkipIfVcr(t)
 	t.Parallel()
 
 	dnsZoneName := fmt.Sprintf("tf-test-dnskey-test-%s", acctest.RandString(t, 10))
 
-	var kskDigest1, kskDigest2, zskPubKey1, zskPubKey2, kskAlg1, kskAlg2 string
-
 	acctest.VcrTest(t, resource.TestCase{
-		PreCheck:     func() { acctest.AccTestPreCheck(t) },
-		CheckDestroy: testAccCheckDNSManagedZoneDestroyProducerFramework(t),
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccCheckDNSManagedZoneDestroyProducer(t),
 		Steps: []resource.TestStep{
 			{
-				ExternalProviders: map[string]resource.ExternalProvider{
-					"google": {
-						VersionConstraint: "4.58.0",
-						Source:            "hashicorp/google-beta",
-					},
-				},
-				Config: testAccDataSourceDNSKeysConfigWithOutputs(dnsZoneName, "on"),
+				Config: testAccDataSourceDNSKeysConfig(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"),
 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "zone_signing_keys.#", "1"),
 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key_id", "key_signing_keys.#", "1"),
 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key_id", "zone_signing_keys.#", "1"),
-					acctest.TestExtractResourceAttr("data.google_dns_keys.foo_dns_key", "key_signing_keys.0.digests.0.digest", &kskDigest1),
-					acctest.TestExtractResourceAttr("data.google_dns_keys.foo_dns_key_id", "zone_signing_keys.0.public_key", &zskPubKey1),
-					acctest.TestExtractResourceAttr("data.google_dns_keys.foo_dns_key_id", "key_signing_keys.0.algorithm", &kskAlg1),
-				),
-			},
-			{
-				ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
-				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"),
-					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "zone_signing_keys.#", "1"),
-					acctest.TestExtractResourceAttr("data.google_dns_keys.foo_dns_key", "key_signing_keys.0.digests.0.digest", &kskDigest2),
-					acctest.TestExtractResourceAttr("data.google_dns_keys.foo_dns_key_id", "zone_signing_keys.0.public_key", &zskPubKey2),
-					acctest.TestExtractResourceAttr("data.google_dns_keys.foo_dns_key_id", "key_signing_keys.0.algorithm", &kskAlg2),
-					acctest.TestCheckAttributeValuesEqual(&kskDigest1, &kskDigest2),
-					acctest.TestCheckAttributeValuesEqual(&zskPubKey1, &zskPubKey2),
-					acctest.TestCheckAttributeValuesEqual(&kskAlg1, &kskAlg2),
 				),
 			},
 		},
 	})
 }
 
 func TestAccDataSourceDNSKeys_noDnsSec(t *testing.T) {
-	// TODO: https://github.com/hashicorp/terraform-provider-google/issues/14158
-	acctest.SkipIfVcr(t)
 	t.Parallel()
 
 	dnsZoneName := fmt.Sprintf("tf-test-dnskey-test-%s", acctest.RandString(t, 10))
 
 	acctest.VcrTest(t, resource.TestCase{
-		PreCheck:     func() { acctest.AccTestPreCheck(t) },
-		CheckDestroy: testAccCheckDNSManagedZoneDestroyProducerFramework(t),
+		PreCheck:                 func() { acctest.AccTestPreCheck(t) },
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+		CheckDestroy:             testAccCheckDNSManagedZoneDestroyProducer(t),
 		Steps: []resource.TestStep{
 			{
-				ExternalProviders: map[string]resource.ExternalProvider{
-					"google": {
-						VersionConstraint: "4.58.0",
-						Source:            "hashicorp/google-beta",
-					},
-				},
 				Config: testAccDataSourceDNSKeysConfig(dnsZoneName, "off"),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "key_signing_keys.#", "0"),
 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "zone_signing_keys.#", "0"),
 				),
 			},
-			{
-				ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
-				Config:                   testAccDataSourceDNSKeysConfig(dnsZoneName, "off"),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "key_signing_keys.#", "0"),
-					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "zone_signing_keys.#", "0"),
-				),
-			},
 		},
 	})
 }
@@ -117,7 +75,7 @@ func testAccDataSourceDNSKeysConfig(dnsZoneName, dnssecStatus string) string {
 	return fmt.Sprintf(`
 resource "google_dns_managed_zone" "foo" {
   name     = "%s"
-  dns_name = "%s.hashicorptest.com."
+  dns_name = "dnssec.gcp.tfacc.hashicorptest.com."
 
   dnssec_config {
     state         = "%s"
@@ -132,27 +90,90 @@ data "google_dns_keys" "foo_dns_key" {
 data "google_dns_keys" "foo_dns_key_id" {
   managed_zone = google_dns_managed_zone.foo.id
 }
-`, dnsZoneName, dnsZoneName, dnssecStatus)
+`, dnsZoneName, dnssecStatus)
+}
+
+// TestAccDataSourceDNSKeys_basic_AdcAuth is the same as TestAccDataSourceDNSKeys_basic but the test enforces that a developer runs this using
+// ADCs, supplied via GOOGLE_APPLICATION_CREDENTIALS. If any other credentials ENVs are set the PreCheck will fail.
+// Commented out until this test can run in TeamCity/CI.
+// func TestAccDataSourceDNSKeys_basic_AdcAuth(t *testing.T) {
+// 	acctest.SkipIfVcr(t) // Uses external providers
+// 	t.Parallel()
+
+// 	creds := os.Getenv("GOOGLE_APPLICATION_CREDENTIALS") // PreCheck assertion handles checking this is set
+
+// 	dnsZoneName := fmt.Sprintf("tf-test-dnskey-test-%s", acctest.RandString(t, 10))
+
+// 	context := map[string]interface{}{
+// 		"credentials_path": creds,
+// 		"dns_zone_name":    dnsZoneName,
+// 		"dnssec_status":    "on",
+// 	}
+
+// 	acctest.VcrTest(t, resource.TestCase{
+// 		PreCheck:     func() { acctest.AccTestPreCheck_AdcCredentialsOnly(t) }, // Note different than default
+// 		CheckDestroy: testAccCheckDNSManagedZoneDestroyProducer(t),
+// 		Steps: []resource.TestStep{
+// 			// Check test fails with version of provider where data source is implemented with PF
+// 			{
+// 				ExternalProviders: map[string]resource.ExternalProvider{
+// 					"google": {
+// 						VersionConstraint: "4.60.0", // Muxed provider with dns data sources migrated to PF
+// 						Source:            "hashicorp/google",
+// 					},
+// 				},
+// 				ExpectError: regexp.MustCompile("Post \"https://oauth2.googleapis.com/token\": context canceled"),
+// 				Config:      testAccDataSourceDNSKeysConfig_AdcCredentials(context),
+// 				Check: resource.ComposeTestCheckFunc(
+// 					testAccDataSourceDNSKeysDSRecordCheck("data.google_dns_keys.foo_dns_key"),
+// 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "key_signing_keys.#", "1"),
+// 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "zone_signing_keys.#", "1"),
+// 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key_id", "key_signing_keys.#", "1"),
+// 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key_id", "zone_signing_keys.#", "1"),
+// 				),
+// 			},
+// 			// Test should pass with more recent code
+// 			{
+// 				ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
+// 				Config:                   testAccDataSourceDNSKeysConfig_AdcCredentials(context),
+// 				Check: resource.ComposeTestCheckFunc(
+// 					testAccDataSourceDNSKeysDSRecordCheck("data.google_dns_keys.foo_dns_key"),
+// 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "key_signing_keys.#", "1"),
+// 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key", "zone_signing_keys.#", "1"),
+// 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key_id", "key_signing_keys.#", "1"),
+// 					resource.TestCheckResourceAttr("data.google_dns_keys.foo_dns_key_id", "zone_signing_keys.#", "1"),
+// 				),
+// 			},
+// 		},
+// 	})
+// }
+
+func testAccDataSourceDNSKeysConfig_AdcCredentials(context map[string]interface{}) string {
+	return acctest.Nprintf(`
+
+// The auth problem isn't triggered unless provider block is
+// present in the test config.
+
+provider "google" {
+  credentials = "%{credentials_path}"
 }
 
-// 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 {
+resource "google_dns_managed_zone" "foo" {
+  name     = "%{dns_zone_name}"
+  dns_name = "dnssec.gcp.tfacc.hashicorptest.com."
 
-	config := testAccDataSourceDNSKeysConfig(dnsZoneName, dnssecStatus)
-	config = config + `
-# These outputs will cause an error if google_dns_managed_zone.foo.dnssec_config.state == "off"
+  dnssec_config {
+    state         = "%{dnssec_status}"
+    non_existence = "nsec3"
+  }
+}
 
-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
+data "google_dns_keys" "foo_dns_key" {
+  managed_zone = google_dns_managed_zone.foo.name
 }
 
-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
+data "google_dns_keys" "foo_dns_key_id" {
+  managed_zone = google_dns_managed_zone.foo.id
 }
-`
-	return config
+`, context)
 }