Adding in validation for trailing dot at end of DNS record name #6494

NA2047 · 2022-08-31T21:10:40Z

Potential fix for issue #9074 I have added a validator to ensure the trailing dot is not missed during terraform plan
hashicorp/terraform-provider-google#9074

If this PR is for Terraform, I acknowledge that I have:

[ X] Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

dns: Added in validation for trailing dot at end of DNS record name

modular-magician · 2022-08-31T21:11:41Z

Hello! I am a robot who works on Magic Modules PRs.

I've detected that you're a community contributor. @ScottSuarez, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

Your assigned reviewer will help review your code by:

Ensuring it's backwards compatible, covers common error cases, etc.
Summarizing the change into a user-facing changelog note.
Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.

modular-magician · 2022-08-31T21:22:08Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 13 insertions(+))
Terraform Beta: Diff ( 1 file changed, 13 insertions(+))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-08-31T22:06:59Z

Tests analytics

Total tests: 2151
Passed tests 1910
Skipped tests: 233
Failed tests: 8

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccCloudRunService_cloudRunServiceStaticOutboundExample

modular-magician · 2022-08-31T22:14:46Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[Debug log]

Tests failed during RECORDING mode:
TestAccSqlDatabaseInstance_SqlServerAuditConfig[Error message] [Debug log]
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccComputeGlobalForwardingRule_internalLoadBalancing[Error message] [Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Error message] [Debug log]
TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange[Error message] [Debug log]
TestAccCloudRunService_cloudRunServiceStaticOutboundExample[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

ScottSuarez · 2022-09-01T18:02:35Z

Could you add unit tests for this feature?

ref

modular-magician · 2022-09-01T19:45:09Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 13 insertions(+))
Terraform Beta: Diff ( 1 file changed, 13 insertions(+))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-01T20:30:15Z

Tests analytics

Total tests: 2156
Passed tests 1916
Skipped tests: 234
Failed tests: 6

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample

modular-magician · 2022-09-01T20:44:37Z

Tests passed during RECORDING mode:
TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[Debug log]

Tests failed during RECORDING mode:
TestAccSqlDatabaseInstance_SqlServerAuditConfig[Error message] [Debug log]
TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange[Error message] [Debug log]
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccComputeGlobalForwardingRule_internalLoadBalancing[Error message] [Debug log]
TestAccCloudRunService_cloudRunServiceStaticOutboundExample[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

NA2047 · 2022-09-02T15:39:20Z

Could you add unit tests for this feature?

ref

modular-magician · 2022-09-02T15:51:13Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 13 insertions(+))
Terraform Beta: Diff ( 1 file changed, 13 insertions(+))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-02T16:54:48Z

Tests analytics

Total tests: 2156
Passed tests 1915
Skipped tests: 234
Failed tests: 7

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccSqlDatabaseInstance_SqlServerAuditConfig

modular-magician · 2022-09-02T17:01:15Z

Tests passed during RECORDING mode:
TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccComputeGlobalForwardingRule_internalLoadBalancing[Error message] [Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Error message] [Debug log]
TestAccCloudRunService_cloudRunServiceStaticOutboundExample[Error message] [Debug log]
TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange[Error message] [Debug log]
TestAccSqlDatabaseInstance_SqlServerAuditConfig[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

ScottSuarez · 2022-09-02T19:11:47Z

I notice this is in draft state. Please let me know when it is ready for review

ScottSuarez · 2022-09-07T23:50:46Z

mmv1/third_party/terraform/resources/resource_dns_record_set.go

+	value := v.(string)
+	last1 := value[len(value)-1:]


there is potential to crash here if we are out of bounds. Could you add empty string as a edge case in your test suite?

https://go.dev/play/p/At9a9rVQlRL

Will do @ScottSuarez

modular-magician · 2022-09-08T14:14:14Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 33 insertions(+))
Terraform Beta: Diff ( 2 files changed, 33 insertions(+))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-08T15:15:26Z

Tests analytics

Total tests: 2169
Passed tests 1930
Skipped tests: 235
Failed tests: 4

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample

modular-magician · 2022-09-08T15:20:47Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Debug log]
TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

ScottSuarez · 2022-09-08T17:11:12Z

Thanks ! This pr is still in draft state. If it is ready to be checked in could you convert it to a normal pull request.

ScottSuarez · 2022-09-08T17:39:59Z

Looks like the linter is failing for your files. Could you run make lint downstream?

ScottSuarez · 2022-09-08T17:40:45Z

Here is the error from the linter

make: *** [fmtcheck] Error 1
You can use the command: `make fmt` to reformat code.
./google-beta/resource_dns_record_set.go
gofmt needs running on the following files:
==> Checking that code complies with gofmt requirements...
make: *** [fmtcheck] Error 1
GNUmakefile:23: recipe for target 'fmtcheck' failed
You can use the command: `make fmt` to reformat code.
./google-beta/resource_dns_record_set.go
gofmt needs running on the following files:
==> Checking that code complies with gofmt requirements...
make: *** No rule to make target 'tools'.  Stop.
make: *** [fmtcheck] Error 1
GNUmakefile:23: recipe for target 'fmtcheck' failed
You can use the command: `make fmt` to reformat code.
./google-beta/resource_dns_record_set.go
gofmt needs running on the following files:
==> Checking that code complies with gofmt requirements.

NA2047 · 2022-09-09T15:47:11Z

Yes I have been seeing this locally having issue with goimports installing in correct directory I believe this is fixed now

Here is the error from the linter

make: *** [fmtcheck] Error 1
You can use the command: `make fmt` to reformat code.
./google-beta/resource_dns_record_set.go
gofmt needs running on the following files:
==> Checking that code complies with gofmt requirements...
make: *** [fmtcheck] Error 1
GNUmakefile:23: recipe for target 'fmtcheck' failed
You can use the command: `make fmt` to reformat code.
./google-beta/resource_dns_record_set.go
gofmt needs running on the following files:
==> Checking that code complies with gofmt requirements...
make: *** No rule to make target 'tools'.  Stop.
make: *** [fmtcheck] Error 1
GNUmakefile:23: recipe for target 'fmtcheck' failed
You can use the command: `make fmt` to reformat code.
./google-beta/resource_dns_record_set.go
gofmt needs running on the following files:
==> Checking that code complies with gofmt requirements.

modular-magician · 2022-09-09T15:59:43Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 33 insertions(+))
Terraform Beta: Diff ( 2 files changed, 33 insertions(+))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-09T16:49:13Z

Tests analytics

Total tests: 2170
Passed tests 1931
Skipped tests: 236
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccComputeInstance_soleTenantNodeAffinities

modular-magician · 2022-09-09T16:51:19Z

Tests passed during RECORDING mode:
TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2022-09-09T18:09:00Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 2 files changed, 36 insertions(+), 4 deletions(-))
Terraform Beta: Diff ( 2 files changed, 36 insertions(+), 4 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-09T19:13:09Z

Tests analytics

Total tests: 2170
Passed tests 1931
Skipped tests: 236
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccComputeInstance_soleTenantNodeAffinities|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample

modular-magician · 2022-09-09T19:18:20Z

Tests passed during RECORDING mode:
TestAccCGCSnippet_eventarcWorkflowsExample[Debug log]
TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

ScottSuarez

looks good

NA2047 · 2022-09-09T20:22:42Z

looks good

Thanks Scott, I assume you will be merging these in to main then? I don't have the ability to merge

ScottSuarez · 2022-09-09T21:01:02Z

correct, I didn't want to merge until you marked the change ready for review. Merging now.

NA2047 marked this pull request as draft August 31, 2022 21:11

modular-magician requested a review from ScottSuarez August 31, 2022 21:11

NA2047 force-pushed the b/237099616 branch from 81a6a27 to 8e56e42 Compare September 1, 2022 19:34

NA2047 closed this Sep 2, 2022

NA2047 reopened this Sep 2, 2022

NA2047 force-pushed the b/237099616 branch 3 times, most recently from 46ebd43 to c2c49a2 Compare September 6, 2022 18:02

ScottSuarez reviewed Sep 7, 2022

View reviewed changes

NA2047 force-pushed the b/237099616 branch from c2c49a2 to a2b6dea Compare September 8, 2022 14:02

modular-magician requested a review from ScottSuarez September 8, 2022 14:04

NA2047 force-pushed the b/237099616 branch from a2b6dea to b7cb086 Compare September 9, 2022 15:46

NA2047 closed this Sep 9, 2022

NA2047 reopened this Sep 9, 2022

Adding in validation for trailing dot at end of DNS record name

aa7db89

NA2047 force-pushed the b/237099616 branch from b7cb086 to aa7db89 Compare September 9, 2022 17:56

ScottSuarez approved these changes Sep 9, 2022

View reviewed changes

NA2047 marked this pull request as ready for review September 9, 2022 20:09

ScottSuarez merged commit 276f1c7 into GoogleCloudPlatform:main Sep 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding in validation for trailing dot at end of DNS record name #6494

Adding in validation for trailing dot at end of DNS record name #6494

NA2047 commented Aug 31, 2022 •

edited

Loading

modular-magician commented Aug 31, 2022

modular-magician commented Aug 31, 2022

modular-magician commented Aug 31, 2022

modular-magician commented Aug 31, 2022

ScottSuarez commented Sep 1, 2022

modular-magician commented Sep 1, 2022

modular-magician commented Sep 1, 2022

modular-magician commented Sep 1, 2022

NA2047 commented Sep 2, 2022

modular-magician commented Sep 2, 2022

modular-magician commented Sep 2, 2022

modular-magician commented Sep 2, 2022

ScottSuarez commented Sep 2, 2022

ScottSuarez Sep 7, 2022

NA2047 Sep 8, 2022

modular-magician commented Sep 8, 2022

modular-magician commented Sep 8, 2022

modular-magician commented Sep 8, 2022

ScottSuarez commented Sep 8, 2022

ScottSuarez commented Sep 8, 2022

ScottSuarez commented Sep 8, 2022

NA2047 commented Sep 9, 2022 •

edited

Loading

modular-magician commented Sep 9, 2022

modular-magician commented Sep 9, 2022

modular-magician commented Sep 9, 2022

modular-magician commented Sep 9, 2022

modular-magician commented Sep 9, 2022

modular-magician commented Sep 9, 2022

ScottSuarez left a comment

NA2047 commented Sep 9, 2022

ScottSuarez commented Sep 9, 2022

Adding in validation for trailing dot at end of DNS record name #6494

Adding in validation for trailing dot at end of DNS record name #6494

Conversation

NA2047 commented Aug 31, 2022 • edited Loading

modular-magician commented Aug 31, 2022

modular-magician commented Aug 31, 2022

Diff report:

modular-magician commented Aug 31, 2022

Tests analytics

Action taken

modular-magician commented Aug 31, 2022

ScottSuarez commented Sep 1, 2022

modular-magician commented Sep 1, 2022

Diff report:

modular-magician commented Sep 1, 2022

Tests analytics

Action taken

modular-magician commented Sep 1, 2022

NA2047 commented Sep 2, 2022

modular-magician commented Sep 2, 2022

Diff report:

modular-magician commented Sep 2, 2022

Tests analytics

Action taken

modular-magician commented Sep 2, 2022

ScottSuarez commented Sep 2, 2022

ScottSuarez Sep 7, 2022

Choose a reason for hiding this comment

NA2047 Sep 8, 2022

Choose a reason for hiding this comment

modular-magician commented Sep 8, 2022

Diff report:

modular-magician commented Sep 8, 2022

Tests analytics

Action taken

modular-magician commented Sep 8, 2022

ScottSuarez commented Sep 8, 2022

ScottSuarez commented Sep 8, 2022

ScottSuarez commented Sep 8, 2022

NA2047 commented Sep 9, 2022 • edited Loading

modular-magician commented Sep 9, 2022

Diff report:

modular-magician commented Sep 9, 2022

Tests analytics

Action taken

modular-magician commented Sep 9, 2022

modular-magician commented Sep 9, 2022

Diff report:

modular-magician commented Sep 9, 2022

Tests analytics

Action taken

modular-magician commented Sep 9, 2022

ScottSuarez left a comment

Choose a reason for hiding this comment

NA2047 commented Sep 9, 2022

ScottSuarez commented Sep 9, 2022

NA2047 commented Aug 31, 2022 •

edited

Loading

NA2047 commented Sep 9, 2022 •

edited

Loading