Skip to content
9 changes: 9 additions & 0 deletions mmv1/third_party/terraform/envvar/envvar_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ var UniverseDomainEnvVars = []string{
"GOOGLE_UNIVERSE_DOMAIN",
}

var ProjectPrefixEnvVars = []string{
"GOOGLE_PROJECT_PREFIX",
}

// This is the billing account that will be charged for the infrastructure used during testing. For
// that reason, it is also the billing account used for creating new projects.
var BillingAccountEnvVars = []string{
Expand Down Expand Up @@ -146,6 +150,11 @@ func GetTestUniverseDomainFromEnv(t *testing.T) string {
return transport_tpg.MultiEnvSearch(UniverseDomainEnvVars)
}

// Project Prefix of different universes
func GetProjectPrefixFromEnv() string {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a followup PR could you change the env var to include "Universe" in the namespace? i.e. GOOGLE_UNIVERSE_PROJECT_PREFIX / GetUniverseProjectPrefixFromEnv

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do

return transport_tpg.MultiEnvSearch(ProjectPrefixEnvVars)
}

// AccTestPreCheck ensures at least one of the region env variables is set.
func GetTestRegionFromEnv() string {
return transport_tpg.MultiEnvSearch(RegionEnvVars)
Expand Down
7 changes: 7 additions & 0 deletions mmv1/third_party/terraform/provider/provider.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,13 @@ func ProviderConfigure(ctx context.Context, d *schema.ResourceData, p *schema.Pr
return nil, diag.FromErr(err)
}

// Bypass checkDestroyer failure with universe domain set
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@c2thorn FYI, this is added to bypass check destroy error

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this work exactly? I don't think we're generally expected to call d.Set inside the provider as it's not a full resource, so I'm a bit surprised it even works.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this works, this is the exact same as removing the check later in https://github.com/hashicorp/terraform-provider-google/blob/main/google/provider/provider.go#L1266. There's no point in having both.

I was hoping to either set the universe domain in a test-only setup function, or skip the check altogether by adding something to

else if config.UniverseDomain != "" && config.UniverseDomain != "googleapis.com"

that verifies if it is a test run.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the flow:

  1. https://github.com/hashicorp/terraform-provider-google/blob/main/google/provider/universe/universe_domain_compute_test.go#L100
  2. https://github.com/hashicorp/terraform-provider-google/blob/main/google/acctest/provider_test_utils.go#L45
  3. The configure function then calls https://github.com/hashicorp/terraform-provider-google/blob/ff829fe23a4235e6566fd07be74c52e32e376b69/google/provider/provider.go#L921

Finally an error occurs because universe_domain is not set in d (schema.ResourceData). And the code right below will return an error, then the check destroyer function fails.

Adding this block of code works around this problem

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline; we can likely set the value on https://github.com/hashicorp/terraform-provider-google/blob/8ef7e5fbcf4f8d5a2a141dfff29bffc52d119e65/google/acctest/provider_test_utils.go#L44 to later successfully call configure.

More broadly, we should figure out how the two initialization paths (through the provider factory and there) are different, and ideally align them.

Copy link
Copy Markdown
Contributor Author

@hao-nan-li hao-nan-li May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied fix in provider_test_utils. And this solves the error in check destroy.

Will take a look at the second suggestion.

if _, ok := d.GetOk("universe_domain"); !ok {
if config.UniverseDomain != "" && config.UniverseDomain != "googleapis.com" {
d.Set("universe_domain", config.UniverseDomain)
}
}

// Verify that universe domains match between credentials and configuration
if v, ok := d.GetOk("universe_domain"); ok {
if config.UniverseDomain == "" && v.(string) != "googleapis.com" { // v can't be "", as it wouldn't pass `ok` above
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,31 @@ func TestAccUniverseDomainDisk(t *testing.T) {
})
}

func TestAccUniverseDomainDiskImage(t *testing.T) {

universeDomain := envvar.GetTestUniverseDomainFromEnv(t)
zone := envvar.GetTestZoneFromEnv()
prefix := envvar.GetProjectPrefixFromEnv()
image_project := ""

if prefix != "" {
image_project = prefix + ":debian-cloud"
} else {
image_project = "debian-cloud"
}

acctest.VcrTest(t, resource.TestCase{
PreCheck: func() { acctest.AccTestPreCheck(t) },
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
CheckDestroy: testAccCheckComputeDiskDestroyProducer(t),
Steps: []resource.TestStep{
{
Config: testAccUniverseDomain_basic_disk_image(universeDomain, zone, image_project),
},
},
})
}

func TestAccDefaultUniverseDomainDisk(t *testing.T) {
universeDomain := "googleapis.com"

Expand Down Expand Up @@ -85,6 +110,28 @@ resource "google_compute_instance_template" "instance_template" {
`, universeDomain)
}

func testAccUniverseDomain_basic_disk_image(universeDomain, zone, image_project string) string {
return fmt.Sprintf(`
provider "google" {
universe_domain = "%s"
}

data "google_compute_images" "debian" {
project = "%s"
filter = "name=debian-12*"
}

resource "google_compute_disk" "primary" {
name = "async-test-disk"
type = "pd-ssd"
zone = "%s"

physical_block_size_bytes = 4096
image = "projects/%s/global/images/${data.google_compute_images.debian.images[0].name}"
}
`, universeDomain, image_project, zone, image_project)
}

func testAccCheckComputeDiskDestroyProducer(t *testing.T) func(s *terraform.State) error {
return func(s *terraform.State) error {
for name, rs := range s.RootModule().Resources {
Expand Down
3 changes: 3 additions & 0 deletions mmv1/third_party/terraform/services/compute/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ func ResolveImage(c *transport_tpg.Config, project, name, userAgent string) (str
break
}
}
if c.UniverseDomain != "" && c.UniverseDomain != "googleapis.com" {
resolveImageLink = regexp.MustCompile(fmt.Sprintf("^https://compute.%s/compute/[a-z0-9]+/projects/(%s)/global/images/(%s)", c.UniverseDomain, verify.ProjectRegex, resolveImageImageRegex))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix works but we should probably make it more clear. Out of band, could we move resolveImageLink into a function that returns the appropriate link for the current universe? That'll guard against direct access of the raw value, move the logic together, and give a good spot to document the differences with a function comment.

}
switch {
case resolveImageLink.MatchString(name): // https://www.googleapis.com/compute/v1/projects/xyz/global/images/xyz
return name, nil
Expand Down
Loading