Add google_apigee_custom_report resource#16955
Conversation
|
Googlers: For automatic test runs see go/terraform-auto-test-runs. @zli82016, 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. |
|
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.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_apigee_custom_report" "primary" {
chart_type = # value needed
comments = # value needed
filter = # value needed
from_time = # value needed
limit = # value needed
offset = # value needed
properties {
property = # value needed
value = # value needed
}
sort_by_cols = # value needed
sort_order = # value needed
tags = # value needed
time_unit = # value needed
to_time = # value needed
topk = # value needed
}
|
Non-exercised tests🔴 Tests were added that are skipped in VCR:
Tests analyticsTotal tests: 73 Click here to see the affected service packages
🟢 All tests passed! View the build log |
|
The |
|
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: 74 Click here to see the affected service packages
🟢 All tests passed! View the build log |
zli82016
left a comment
There was a problem hiding this comment.
Can you also add the update tests for the updatable fields?
Doc: https://googlecloudplatform.github.io/magic-modules/test/test/#add-an-update-test
Thanks
|
Thanks for the thorough review @zli82016! Pushed changes addressing all feedback: Copyright: Fixed to 2026. Vars: Added Update test: Added a handwritten update test (
|
|
@zli82016 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
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: 79 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. |
|
The @zli82016 all your review feedback was addressed in the last push (copyright, vars, update test, sweeper/VCR explanations). Happy to make further changes if needed. |
|
@ogormans-deptstack, this PR is waiting for action from you. If no action is taken, this PR will be closed in 28 days. Please address any comments or change requests, or re-request review from a core reviewer if no action is required. This notification can be disabled with the |
| The internal resource name of the custom report, a UUID assigned | ||
| by the server on creation. Used as the identifier in the API URL path. | ||
| output: true | ||
| - name: 'displayName' |
There was a problem hiding this comment.
Should the field displayName be immutable?
There was a problem hiding this comment.
This comment is still pending.
|
TestAccApigeeCustomReport_apigeeCustomReportFullTestExample failed |
Change properties.value from ["true"] to ["yes"] in the Apigee CustomReport full test. The API rejects boolean-like strings in custom_report.properties[0].values[0]. Fixes GoogleCloudPlatform#16955
- Fix properties.value schema: API expects Attribute objects (name/value pairs), not flat strings. This resolves the FullTest failure reported by @zli82016. - Simplify custom_import template to use standard ParseImportId patterns per reviewer suggestion.
|
Thanks @zli82016 — pushed addressing the remaining feedback: FullTest failure (properties.value): Root cause was a schema mismatch. The API's Custom import template: Simplified to use standard displayName immutability: |
Use {{index $.Vars "project_id"}} in test templates instead of hardcoded tf-test%{random_suffix}, matching the ControlPlaneAccess pattern.
|
Thanks for your patience and thorough reviews @zli82016 — really appreciate the time you've put into this. Pushed two commits addressing the remaining feedback: 1. FullTest failure fix (properties.value schema) 2. Custom import simplified 3. project_id declared as var Re: displayName immutability — it's intentionally updatable. The handwritten update test exercises this (changes |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit e674203: 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 replaying VCR 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 recording VCR build log or the debug logs folder for detailed results. @ogormans-deptstack, @zli82016 VCR tests complete for e674203! |
These fields causing diffs are not in the GET response. |
|
Pushed fixes for both test failures:
Re: displayName immutability — it's intentionally updatable. The PUT API accepts changes, and the handwritten update test exercises this (changes |
- Shorten project_id var to 'proj' to stay under 30-char GCP limit - Add ignore_read to fromTime, toTime, topk, limit, offset, comments (API accepts on write but doesn't return on GET)
Thanks for the fix. Will the API return them on GET in the future? If the API is not ready to return them and will return them in the future, these fields can be removed now and then be added once the API returns them. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 5cd354d: 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 replaying VCR 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 recording VCR build log or the debug logs folder for detailed results. @ogormans-deptstack, @zli82016 VCR tests complete for 5cd354d! |
|
Honestly, I'm not sure whether the API will return them in the future — they're documented in the CustomReport resource spec as top-level fields, but the GET response doesn't include them even after they're set via POST/PUT. I'll remove them entirely for now. That's cleaner than Removing all 6: |
Remove fromTime, toTime, topk, limit, offset, comments — the API accepts these on create/update but does not return them on GET, causing plan drift. Can be re-added if the API starts returning them.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes for commit 45ec085: Diff reportYour PR generated the following diffs in downstream repositories:
Test reportAnalytics
Affected Service Packages
Step 1: Replaying Mode 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
View the replaying VCR 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 recording VCR build log or the debug logs folder for detailed results. @ogormans-deptstack, @zli82016 VCR tests complete for 45ec085! |
zli82016
left a comment
There was a problem hiding this comment.
More fields sort_by_cols, sort_order, tags need to be removed from the support.
=== CONT TestAccApigeeCustomReport_apigeeCustomReportFullTestExample
resource_apigee_custom_report_generated_test.go:199: Step 1/3 error: After applying this test step, the non-refresh plan was not empty.
stdout:
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
~ update in-place
Terraform will perform the following actions:
# google_apigee_custom_report.apigee_custom_report will be updated in-place
~ resource "google_apigee_custom_report" "apigee_custom_report" {
id = "organizations/tf-test-proji0xlclte63/reports/c58ac590-d37e-4ce0-ad82-f58c48bb99c0"
name = "c58ac590-d37e-4ce0-ad82-f58c48bb99c0"
~ sort_by_cols = [
+ "message_count",
]
+ sort_order = "DESC"
~ tags = [
+ "production",
+ "analytics",
]
# (12 unchanged attributes hidden)
# (3 unchanged blocks hidden)
}
Plan: 0 to add, 1 to change, 0 to destroy.

New resource
google_apigee_custom_reportfor managing Apigee Analytics custom report definitions via theorganizations.reportsAPI.Custom reports let users define reusable analytics views with custom metrics, dimensions, filters, and grouping. This resource supports all fields from the API including metrics with aggregation functions, dimensions, time units, sorting, and report properties.
Tested all CRUD operations against a live Apigee evaluation org:
Uses
PUTwith no update mask (same pattern asDeveloperApp). Thenamefield is a server-assigned UUID used as the path identifier.displayNameis user-specified and mutable.Related to PR #16954 (
google_apigee_data_collector).Release Note Template for Downstream PRs (will be copied)