magic-modules icon indicating copy to clipboard operation
magic-modules copied to clipboard

Add type field to DNS authorization reosurce

Open Hamzawy63 opened this issue 1 year ago • 14 comments

This PR:

  • Added type, a new optional field, to Certificate Manager DNS authorization resource.
  • Added an example for creating regional DNS authorization resource (as it's now supported + in public review)
  • Added an example for creating regional certificate manager certificate with regional DNS auth.

Tests that I ran:

make testacc TEST=./google/services/certificatemanager TESTARGS='-run=TestAccCertificateManagerDnsAuthorization_certificateManagerDnsAuthorizationRegionalExample'

make testacc TEST=./google/services/certificatemanager TESTARGS='-run=TestAccCertificateManagerCertificate_certificateManagerGoogleManagedRegionalCertificateDnsAuthExample'

Release Note Template for Downstream PRs (will be copied)

certificatemanager: added `type` field to `google_certificate_manager_dns_authorization` resource

Hamzawy63 avatar Feb 22 '24 08:02 Hamzawy63

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@BBBmau, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

github-actions[bot] avatar Feb 23 '24 15:02 github-actions[bot]

Based on the API docs i see no reference to a type attribute being added for google_certificate_manager_dns_authorization resource. Can you provide where it is that you saw that this was added

BBBmau avatar Feb 23 '24 19:02 BBBmau

Based on the API docs i see no reference to a type attribute being added for google_certificate_manager_dns_authorization resource. Can you provide where it is that you saw that this was added

The API doc page is not updated yet. Though, you will be able to find the field in gcloud : gcloud certificate-manager dns-authorizations create --help

Hamzawy63 avatar Feb 26 '24 07:02 Hamzawy63

@BBBmau Let me know if you have any questions or inquires. If everything is good with the PR, it would be nice to get it merged today so that it becomes available next week to customers.

Thank you in advance!

Hamzawy63 avatar Feb 27 '24 08:02 Hamzawy63

/gcbrun

BBBmau avatar Feb 27 '24 16:02 BBBmau

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 5 files changed, 173 insertions(+)) Terraform Beta: Diff ( 5 files changed, 173 insertions(+)) TF Conversion: Diff ( 1 file changed, 10 insertions(+)) TF OiCS: Diff ( 8 files changed, 227 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_certificate_manager_dns_authorization (12 total tests) Please add an acceptance test which includes these fields. The test should include the following:

resource "google_certificate_manager_dns_authorization" "primary" {
  type = # value needed
}

modular-magician avatar Feb 27 '24 17:02 modular-magician

Tests analytics

Total tests: 19 Passed tests: 15 Skipped tests: 2 Affected tests: 2

Click here to see the affected service packages
  • certificatemanager

Action taken

Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccCertificateManagerCertificate_certificateManagerGoogleManagedRegionalCertificateDnsAuthExample|TestAccCertificateManagerDnsAuthorization_certificateManagerDnsAuthorizationRegionalExample

Get to know how VCR tests work

modular-magician avatar Feb 27 '24 17:02 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccCertificateManagerCertificate_certificateManagerGoogleManagedRegionalCertificateDnsAuthExample[Debug log] TestAccCertificateManagerDnsAuthorization_certificateManagerDnsAuthorizationRegionalExample[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$ View the build log or the debug log for each test

modular-magician avatar Feb 27 '24 17:02 modular-magician

Your PR includes resource fields which are not covered by any test. Resource: google_certificate_manager_dns_authorization (12 total tests) Please add an acceptance test which includes these fields. The test should include the following:

I will modify one of the examples to use the type field so that we get rid of the warning.

Hamzawy63 avatar Feb 27 '24 19:02 Hamzawy63

/gcbrun

BBBmau avatar Feb 27 '24 19:02 BBBmau

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

Terraform GA: Diff ( 5 files changed, 175 insertions(+)) Terraform Beta: Diff ( 5 files changed, 175 insertions(+)) TF Conversion: Diff ( 1 file changed, 10 insertions(+)) TF OiCS: Diff ( 8 files changed, 228 insertions(+))

modular-magician avatar Feb 27 '24 20:02 modular-magician

Tests analytics

Total tests: 19 Passed tests: 16 Skipped tests: 2 Affected tests: 1

Click here to see the affected service packages
  • certificatemanager

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccCertificateManagerDnsAuthorization_certificateManagerDnsAuthorizationRegionalExample

Get to know how VCR tests work

modular-magician avatar Feb 27 '24 20:02 modular-magician

$\textcolor{green}{\textsf{Tests passed during RECORDING mode:}}$ TestAccCertificateManagerDnsAuthorization_certificateManagerDnsAuthorizationRegionalExample[Debug log]

$\textcolor{green}{\textsf{No issues found for passed tests after REPLAYING rerun.}}$


$\textcolor{green}{\textsf{All tests passed!}}$ View the build log or the debug log for each test

modular-magician avatar Feb 27 '24 20:02 modular-magician

the tests included seem good, i'll do a more thorough test tomorrow before approving 👍🏼

BBBmau avatar Feb 28 '24 05:02 BBBmau

Just for my own interest, once a PR is approved, how long does it usually take before it's available within the provider? Is there a release cycle, or is it simply once merged?

We're using PER_PROJECT_RECORD types, and so just trying to gauge if I wait for this PR, or if I use a local-exec provisioner to set up the dns-authorizations via gcloud in the short term so i'm not blocked.

adamstrawson avatar Mar 01 '24 09:03 adamstrawson

@adamstrawson we make a new release weekly, so this should be in part of the release coming next week!

BBBmau avatar Mar 01 '24 09:03 BBBmau