Add google_apigee_data_collector resource#16954
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Googlers: For automatic test runs see go/terraform-auto-test-runs. @NickElliot, 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. |
|
I signed the CLA after the fact please rerun |
#16955 passed on my other contribution so we should be good |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Non-exercised tests🔴 Tests were added that are skipped in VCR:
Tests analyticsTotal tests: 73 Click here to see the affected service packages
Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
|
@NickElliot This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @NickElliot This PR has been waiting for review for 1 week. Please take a look! Use the label |
NickElliot
left a comment
There was a problem hiding this comment.
Hi, sorry for delays I was on PTO!
Could you share logs of the tests running locally?
|
@NickElliot just wanted to flag the CI status here — all checks pass cleanly:
The only red is the VCR test — which failed on I've also addressed the same class of review feedback that @zli82016 raised on the sibling PR #16955 (vars for hardcoded names, update tests, sweeper/VCR skip explanations) — happy to apply any of those patterns here too if you'd like. Let me know if you have any questions or feedback. |
Local acceptance test resultsFull CRUD lifecycle test against a live GCP Apigee org (EVALUATION tier, Test provisions a new GCP project, enables APIs, creates a compute network + service networking connection, provisions an Apigee org, creates the data collector, verifies the import step, then tears everything down. ~15 minutes end-to-end, all steps clean. |
NickElliot
left a comment
There was a problem hiding this comment.
I was looking for the test results because both of the added tests are skipped/excluded, I am aware the checks were all green :)
LGTM!
NickElliot
left a comment
There was a problem hiding this comment.
I double checked other apigee tests and saw we are able to provision Orgs within those tests without skipping them in CI/VCR e.g. TestAccApigeeOrganization_apigeeOrganizationCloudBasicTestExample, is there any reason we can't do the same here?
|
Thanks for the review @NickElliot, and no worries about the PTO! Good question on VCR. I initially added That said, you're right that the Organization test itself runs without skipping VCR, and other resources like EndpointAttachment don't skip either. I'll remove Will push shortly. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit c2bbd2f: Diff reportYour PR generated the following diffs in downstream repositories:
Test reportAnalytics
Affected Service Packages
Step 1: Replaying Mode Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
View the build log Step 2: Recording Mode
Caution Issues requiring attention before PR completion 🔴 Initial Recording Failed: Some tests failed during the recording step. See the table above for details. Please address these issues to complete your PR. If you believe these detections are incorrect or unrelated to your change, please raise the concern with your reviewer. View the build log or the debug logs folder for detailed results. @ogormans-deptstack, @NickElliot VCR tests complete for c2bbd2f! |
|
@NickElliot This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
- Remove exclude_sweeper to enable resource cleanup - Use vars for network_name and range_name in doc template
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 0d3c2b0: Diff reportYour PR generated the following diffs in downstream repositories:
Test reportAnalytics
Affected Service Packages
Step 1: Replaying Mode Action takenFound 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
View the build log Step 2: Recording Mode
Caution Issues requiring attention before PR completion 🔴 Initial Recording Failed: Some tests failed during the recording step. See the table above for details. Please address these issues to complete your PR. If you believe these detections are incorrect or unrelated to your change, please raise the concern with your reviewer. View the build log or the debug logs folder for detailed results. @ogormans-deptstack, @NickElliot VCR tests complete for 0d3c2b0! |
|
@NickElliot VCR results are in and the DataCollector tests look clean. VCR replay (Step 1): Only 2 tests were flagged as affected — both are the pre-existing VCR recording (Step 2): The only failures are those same 2 pre-existing Organization tests — they fail on every Apigee PR (same failures appear on #16955 and #16957, and on other Apigee PRs from before our changes). So to summarize:
Happy to address any other concerns. This one should be ready for approval. |
NickElliot
left a comment
There was a problem hiding this comment.
LGTM now, thanks for adding the tests!
e1b97d6
Adds the
google_apigee_data_collectorresource to the Google provider, enabling Terraform management of Apigee DataCollectors for custom analytics and API monetization.This is a rewrite of #11655, which was opened under the old Ruby generator format. As noted by @rileykarson in that PR, the generator has since moved to Go with generic YAML types. This PR follows the current conventions.
Resource Details
apigee.googleapis.com/v1/organizations/{org}/datacollectorsorg_id(required, immutable) - Apigee organizationdata_collector_id(required, immutable) - Must begin withdc_, validated via regextype(required, immutable) - Enum: INTEGER, FLOAT, STRING, BOOLEAN, DATETIMEdescription(optional, mutable) - Only updatable fieldname,created_at,last_modified_at(computed, output-only)Testing
Built the provider locally and tested all CRUD operations against a real Apigee eval organization:
organizations/{org}/datacollectors/{id}) worksFiles
mmv1/products/apigee/DataCollector.yaml- Resource definitionmmv1/templates/terraform/custom_import/apigee_data_collector.go.tmpl- Import logic for org-scoped resourcesmmv1/templates/terraform/examples/apigee_data_collector_basic.tf.tmpl- Docs examplemmv1/templates/terraform/examples/apigee_data_collector_basic_test.tf.tmpl- Acceptance test exampleModeled on existing Apigee resources (
AppGroup,Envgroup) for consistency.