-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add location field in DNS authorization resource. #9968
Merged
SarahFrench
merged 2 commits into
GoogleCloudPlatform:main
from
Hamzawy63:dns-authorization-regionalization
Feb 22, 2024
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 changes: 2 additions & 1 deletion
3
mmv1/templates/terraform/examples/certificate_manager_dns_authorization_basic.tf.erb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
91 changes: 91 additions & 0 deletions
91
mmv1/templates/terraform/state_migrations/certificate_manager_dns_authorization.go.erb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
func ResourceCertificateManagerDnsAuthorizationUpgradeV0(_ context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) { | ||
log.Printf("[DEBUG] Attributes before migration: %#v", rawState) | ||
// Version 0 didn't support location. Default it to global. | ||
rawState["location"] = "global" | ||
log.Printf("[DEBUG] Attributes after migration: %#v", rawState) | ||
return rawState, nil | ||
} | ||
|
||
func resourceCertificateManagerDnsAuthorizationResourceV0() *schema.Resource { | ||
return &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"domain": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
Description: `A domain which is being authorized. A DnsAuthorization resource covers a | ||
single domain and its wildcard, e.g. authorization for "example.com" can | ||
be used to issue certificates for "example.com" and "*.example.com".`, | ||
}, | ||
"name": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
Description: `Name of the resource; provided by the client when the resource is created. | ||
The name must be 1-64 characters long, and match the regular expression [a-zA-Z][a-zA-Z0-9_-]* which means the first character must be a letter, | ||
and all following characters must be a dash, underscore, letter or digit.`, | ||
}, | ||
"description": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Description: `A human-readable description of the resource.`, | ||
}, | ||
"labels": { | ||
Type: schema.TypeMap, | ||
Optional: true, | ||
Description: `Set of label tags associated with the DNS Authorization resource. | ||
|
||
**Note**: This field is non-authoritative, and will only manage the labels present in your configuration. | ||
Please refer to the field 'effective_labels' for all of the labels present on the resource.`, | ||
Elem: &schema.Schema{Type: schema.TypeString}, | ||
}, | ||
"dns_resource_record": { | ||
Type: schema.TypeList, | ||
Computed: true, | ||
Description: `The structure describing the DNS Resource Record that needs to be added | ||
to DNS configuration for the authorization to be usable by | ||
certificate.`, | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"data": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
Description: `Data of the DNS Resource Record.`, | ||
}, | ||
"name": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
Description: `Fully qualified name of the DNS Resource Record. | ||
E.g. '_acme-challenge.example.com'.`, | ||
}, | ||
"type": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
Description: `Type of the DNS Resource Record.`, | ||
}, | ||
}, | ||
}, | ||
}, | ||
"effective_labels": { | ||
Type: schema.TypeMap, | ||
Computed: true, | ||
Description: `All of labels (key/value pairs) present on the resource in GCP, including the labels configured through Terraform, other clients and services.`, | ||
Elem: &schema.Schema{Type: schema.TypeString}, | ||
}, | ||
"terraform_labels": { | ||
Type: schema.TypeMap, | ||
Computed: true, | ||
Description: `The combination of labels configured directly on the resource | ||
and default labels configured on the provider.`, | ||
Elem: &schema.Schema{Type: schema.TypeString}, | ||
}, | ||
"project": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Computed: true, | ||
ForceNew: true, | ||
}, | ||
}, | ||
} | ||
|
||
} |
78 changes: 78 additions & 0 deletions
78
...ervices/certificatemanager/resource_certificate_manager_dns_authorization_upgrade_test.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
package certificatemanager_test | ||
|
||
import ( | ||
"fmt" | ||
"testing" | ||
|
||
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" | ||
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" | ||
"github.com/hashicorp/terraform-provider-google/google/acctest" | ||
) | ||
|
||
// Tests schema version migration by creating a dns authorization with an old version of the provider (5.15.0) | ||
// and then updating it with the current version the provider. | ||
func TestAccCertificateManagerDnsAuthorization_migration(t *testing.T) { | ||
acctest.SkipIfVcr(t) | ||
t.Parallel() | ||
name := fmt.Sprintf("tf-test-%d", acctest.RandInt(t)) | ||
|
||
oldVersion := map[string]resource.ExternalProvider{ | ||
"google": { | ||
VersionConstraint: "5.15.0", // a version that doesn't support location yet. | ||
Source: "registry.terraform.io/hashicorp/google", | ||
}, | ||
} | ||
newVersion := map[string]func() (*schema.Provider, error){ | ||
"mynewprovider": func() (*schema.Provider, error) { return acctest.TestAccProviders["google"], nil }, | ||
} | ||
|
||
acctest.VcrTest(t, resource.TestCase{ | ||
PreCheck: func() { acctest.AccTestPreCheck(t) }, | ||
CheckDestroy: testAccCheckCertificateManagerDnsAuthorizationDestroyProducer(t), | ||
Steps: []resource.TestStep{ | ||
{ | ||
Config: dnsAuthorizationResourceConfig(name), | ||
ExternalProviders: oldVersion, | ||
}, | ||
{ | ||
ResourceName: "google_certificate_manager_dns_authorization.default", | ||
ImportState: true, | ||
ImportStateVerifyIgnore: []string{"location"}, | ||
SarahFrench marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ExternalProviders: oldVersion, | ||
}, | ||
{ | ||
Config: dnsAuthorizationResourceConfigUpdated(name), | ||
ProviderFactories: newVersion, | ||
}, | ||
{ | ||
ResourceName: "google_certificate_manager_dns_authorization.default", | ||
ImportState: true, | ||
ImportStateVerifyIgnore: []string{"location"}, | ||
ProviderFactories: newVersion, | ||
}, | ||
}, | ||
}) | ||
} | ||
|
||
func dnsAuthorizationResourceConfig(name string) string { | ||
return fmt.Sprintf(` | ||
resource "google_certificate_manager_dns_authorization" "default" { | ||
name = "%s" | ||
description = "The default dns" | ||
domain = "domain.hashicorptest.com" | ||
} | ||
`, name) | ||
} | ||
|
||
func dnsAuthorizationResourceConfigUpdated(name string) string { | ||
return fmt.Sprintf(` | ||
provider "mynewprovider" {} | ||
|
||
resource "google_certificate_manager_dns_authorization" "default" { | ||
provider = mynewprovider | ||
name = "%s" | ||
description = "The migrated default dns" | ||
domain = "domain.hashicorptest.com" | ||
} | ||
`, name) | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that this log is not printed in while running the migration test, when it will be used?
Also what can I do to mimic a customer that has a configuration in an old version and then upgrades it? I didn't know how to define the resource to be in an older version.
I had an old journey with a similar change and you suggested some solutions for testing: #7661 (comment), I would appreciate if you could elaborate more on both of the mentioned solutions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran your
TestAccCertificateManagerDnsAuthorization_migration
test locally with the environment variable TF_LOG set to "DEBUG" to make debug logs visible and I also didn't see the log. I believe this shows that the test isn't using the local code frommynewprovider
- see other comment for suggested changes!You can achieve this with the
TestAccCertificateManagerDnsAuthorization_migration
test (following the fix in my other comment). In that test what's happening is:You could add further test steps if they were needed. For example you could insert a step between 2 and 3 of the steps above where you use the first config with the new provider and check that the plan is empty (the snippet below has
ExpectNonEmptyPlan: false,
implied). This would mimic a user just upgrading their provider version and not changing their config: