Add support for projects field in Developer Connect Insights InsightsConfig resource#16184
Conversation
|
Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: For automatic test runs see go/terraform-auto-test-runs. @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. |
mmv1/openapi_generate/openapi/developerconnect_public_openapi3_0_v1.json
Outdated
Show resolved
Hide resolved
|
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.
|
Tests analyticsTotal tests: 33 Click here to see the affected service packages
🟢 All tests passed! View the build log |
|
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.
|
Tests analyticsTotal tests: 33 Click here to see the affected service packages
🟢 All tests passed! View the build log |
|
Test is skipped in VCR. Running test on TC and I'll update the test result here when it completes |
shuyama1
left a comment
There was a problem hiding this comment.
Test failed with error:
=== RUN TestAccDeveloperConnectInsightsConfig_developerConnectInsightsConfigBasicExample
=== PAUSE TestAccDeveloperConnectInsightsConfig_developerConnectInsightsConfigBasicExample
=== CONT TestAccDeveloperConnectInsightsConfig_developerConnectInsightsConfigBasicExample
resource_developer_connect_insights_config_generated_test.go:63: Step 1/2 error: After applying this test step, the 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_developer_connect_insights_config.insights_config_projects will be updated in-place
~ resource "google_developer_connect_insights_config" "insights_config_projects" {
id = "projects/dci-tf-1aap91xhyt/locations/us-central1/insightsConfigs/tf-test-ic-projects-1aap91xhyt"
name = "projects/dci-tf-1aap91xhyt/locations/us-central1/insightsConfigs/tf-test-ic-projects-1aap91xhyt"
# (13 unchanged attributes hidden)
~ projects {
~ project_ids = [
~ "projects/337985338891" -> "dci-tf-1aap91xhyt",
]
}
}
Plan: 0 to add, 1 to change, 0 to destroy.
--- FAIL: TestAccDeveloperConnectInsightsConfig_developerConnectInsightsConfigBasicExample (265.60s)
FAIL
It seems the API does not return the values for the field project_ids as sent, which causes a permadiff in Terraform. See details at https://googlecloudplatform.github.io/magic-modules/develop/diffs/#fix-diffs
|
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.
|
Tests analyticsTotal tests: 33 Click here to see the affected service packages
🟢 All tests passed! View the build log |
|
Added DiffSupress function (diff_suppress_func: 'tpgresource.ProjectNumberDiffSuppress') to handle the VCR error. |
e83ea88 to
503b123
Compare
|
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.
|
Tests analyticsTotal tests: 33 Click here to see the affected service packages
🟢 All tests passed! View the build log |
|
Running tests on TC and I'll update the test result here when it completes |
|
@shuyama1 This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
shuyama1
left a comment
There was a problem hiding this comment.
Test results form TC
TestAccDeveloperConnectInsightsConfig_update:
resource_developer_connect_insights_config_test.go:55: Step 3/4 error: Pre-apply plan check(s) failed:
'google_developer_connect_insights_config.insights_config' - expected DestroyBeforeCreate, got action(s): [delete]
--- FAIL: TestAccDeveloperConnectInsightsConfig_update (260.05s)
TestAccDeveloperConnectInsightsConfig_developerConnectInsightsConfigBasicExample:
resource_developer_connect_insights_config_generated_test.go:63: Resource specified by ResourceName couldn't be found: google_developer_connect_insights_config.insights_config
--- FAIL: TestAccDeveloperConnectInsightsConfig_developerConnectInsightsConfigBasicExample (262.17s)
| depends_on = [time_sleep.wait_for_propagation] | ||
| } | ||
|
|
||
| resource "google_developer_connect_insights_config" "insights_config_projects" { |
There was a problem hiding this comment.
Sorry, somehow missed this in the previous reviews. We'd want to target one primary resource per test. Therefore, could you move this to a new test?
| resource "google_developer_connect_insights_config" "insights_config_apphub" { | ||
| location = "us-central1" | ||
| insights_config_id = "tf-test-ic%{random_suffix}" | ||
| insights_config_id = "tf-test-ic-apphub-%{random_suffix}" |
There was a problem hiding this comment.
This is changing the identifier of the resource in the update step which will cause the resource to recreate.
| @@ -337,5 +337,16 @@ func testAccDeveloperConnectInsightsConfig_update(context map[string]interface{} | |||
| } | |||
| depends_on = [time_sleep.wait_for_propagation] | |||
| } | |||
| resource "google_developer_connect_insights_config" "insights_config_projects" { | |||
There was a problem hiding this comment.
Same here, we may want to move this to a new test instead of creating two google_developer_connect_insights_config resources in one test. Or if the resource can update in-place from using app_hub_application to projects, you could add a new test step to update it to use projects.
https://googlecloudplatform.github.io/magic-modules/test/test/#add-resource-tests covers more details on adding tests if it helps.
Let me know if you have any questions, thanks!
There was a problem hiding this comment.
Yes, you're right. I've added a separate test for an insights_config resource created with an AppHub application and another for an insights_config resource created with projectIds.
…pp and 1 with projects
|
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: 35 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
|
|
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: 34 Click here to see the affected service packages
🟢 All tests passed! View the build log |
shuyama1
left a comment
There was a problem hiding this comment.
I double-checked with the team regarding the naming of this field: Let's rename this to target_projects. This helps distinguish it from the standard project field and aligns with how it's handled in gcloud.
I've added the suggested changes inline, which you can apply directly. I've also verified this in #16353 and tests passed. This should be good to go after applying the changes. Thanks!
mmv1/templates/terraform/examples/developer_connect_insights_config_projects.tf.tmpl
Outdated
Show resolved
Hide resolved
Co-authored-by: Shuya Ma <87669292+shuyama1@users.noreply.github.com>
Co-authored-by: Shuya Ma <87669292+shuyama1@users.noreply.github.com>
…onfig_projects.tf.tmpl Co-authored-by: Shuya Ma <87669292+shuyama1@users.noreply.github.com>
|
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.
|
|
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.
|
|
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: 34 Click here to see the affected service packages
🟢 All tests passed! View the build log |
Non-exercised tests🔴 Tests were added that are skipped in VCR:
Tests analyticsTotal tests: 34 Click here to see the affected service packages
🟢 All tests passed! View the build log |
Non-exercised tests🔴 Tests were added that are skipped in VCR:
Tests analyticsTotal tests: 34 Click here to see the affected service packages
🟢 All tests passed! View the build log |
eacb125
This PR adds support for the
project_idsfield within theprojectsblock for thegoogle_developer_connect_insights_configresource. This enables users to specify target projects when configuring Developer Connect Insights.The following changes are included:
mmv1/products/developerconnect/InsightsConfig.yamlto include the new field.project_idsfield.Addresses hashicorp/terraform-provider-google#25855