-
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 certificate manager certificates datasource #11543
Add certificate manager certificates datasource #11543
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @shuyama1, 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. |
Tests analyticsTotal tests: 3911 Click here to see the affected service packages
Action takenFound 5 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
@shuyama1 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
Hi @shuyama1 👋 , anything I can do to help get this merged? Would love to use this next release! |
Thanks for the ping. Taking a look now. Sorry for the delay on review. |
/gcbrun |
Type: schema.TypeList, | ||
Computed: true, | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ |
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 wonder if we can use Schema: datasourceSchemaFromResourceSchema(ResourceCertificateManagerCertificate().Schema),
to mirror the fields in the google_certificate_manager_certificate resource, avoiding the need to manually add new ones.
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.
Good suggestion, I was hesitant to do this at first because there are fields that aren't relevant.
I changed it and deleted the irrelevant fields for the data source.
While I know it's not necessary, I added a remove helper for fields that aren't relevant for the data source.
I prefer having it explicit on fields that can't be part of the data-source instead of having floating fields that exist in the schema but will never be used.
mmv1/third_party/terraform/website/docs/d/certificate_manager_certificates.html.markdown
Show resolved
Hide resolved
Tests analyticsTotal tests: 3255 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
@shuyama1 I processed your suggestions and ran the acceptance tests, ran them for resource as well as I added an additional output field to be in-line with the data source. % make testacc TEST=./google/services/certificatemanager TESTARGS='-run=TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificate'
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google/services/certificatemanager -v -run=TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificate -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsExample
=== PAUSE TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsExample
=== RUN TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateIssuanceConfigExample
=== PAUSE TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateIssuanceConfigExample
=== RUN TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateIssuanceConfigAllRegionsExample
=== PAUSE TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateIssuanceConfigAllRegionsExample
=== RUN TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsAllRegionsExample
=== PAUSE TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsAllRegionsExample
=== CONT TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsExample
=== CONT TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateIssuanceConfigAllRegionsExample
=== CONT TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateIssuanceConfigExample
=== CONT TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsAllRegionsExample
--- PASS: TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsAllRegionsExample (57.98s)
--- PASS: TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateDnsExample (58.47s)
--- PASS: TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateIssuanceConfigAllRegionsExample (90.61s)
--- PASS: TestAccCertificateManagerCertificate_certificateManagerGoogleManagedCertificateIssuanceConfigExample (90.61s)
PASS
ok github.com/hashicorp/terraform-provider-google/google/services/certificatemanager 91.735s % make testacc TEST=./google/services/certificatemanager TESTARGS='-run=TestAccDataSourceGoogleCertificateManagerCertificates_'
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google/services/certificatemanager -v -run=TestAccDataSourceGoogleCertificateManagerCertificates_ -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google/version.ProviderVersion=acc"
=== RUN TestAccDataSourceGoogleCertificateManagerCertificates_basic
=== PAUSE TestAccDataSourceGoogleCertificateManagerCertificates_basic
=== RUN TestAccDataSourceGoogleCertificateManagerCertificates_full
=== PAUSE TestAccDataSourceGoogleCertificateManagerCertificates_full
=== RUN TestAccDataSourceGoogleCertificateManagerCertificates_regionBasic
=== PAUSE TestAccDataSourceGoogleCertificateManagerCertificates_regionBasic
=== RUN TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificate
=== PAUSE TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificate
=== RUN TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificateDNSAuthorization
=== PAUSE TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificateDNSAuthorization
=== RUN TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificateIssuerConfig
=== PAUSE TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificateIssuerConfig
=== CONT TestAccDataSourceGoogleCertificateManagerCertificates_basic
=== CONT TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificate
=== CONT TestAccDataSourceGoogleCertificateManagerCertificates_regionBasic
=== CONT TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificateIssuerConfig
=== CONT TestAccDataSourceGoogleCertificateManagerCertificates_full
=== CONT TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificateDNSAuthorization
--- PASS: TestAccDataSourceGoogleCertificateManagerCertificates_regionBasic (25.29s)
--- PASS: TestAccDataSourceGoogleCertificateManagerCertificates_basic (33.56s)
--- PASS: TestAccDataSourceGoogleCertificateManagerCertificates_full (33.60s)
--- PASS: TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificate (34.27s)
--- PASS: TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificateDNSAuthorization (58.96s)
--- PASS: TestAccDataSourceGoogleCertificateManagerCertificates_managedCertificateIssuerConfig (92.87s)
PASS
ok github.com/hashicorp/terraform-provider-google/google/services/certificatemanager 94.017s |
Tests analyticsTotal tests: 3815 Click here to see the affected service packages
Action takenFound 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
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.
The failing test is not related to the change in this PR.
Hello, I have a problem with this datasource. I'm trying to use:
but this error occured on terraform validate:
When I removed project field:
I got this error on terraform plan:
I don't know if I'm doing something wrong or there's a bug in implementation.
|
This PR adds the Certificate Manager certificates data source.
Tests:
Release Note Template for Downstream PRs (will be copied)