add resources to support apigee proxy deployments#15126
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. @c2thorn, 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. |
Non-exercised tests🔴 Tests were added that are skipped in VCR:
Tests analyticsTotal tests: 67 Click here to see the affected service packages
🟢 All tests passed! View the build log |
c2thorn
left a comment
There was a problem hiding this comment.
The test is skipped in VCR so I'm running a manual test run in our CI. I'll post the results later.
For now, can you tell me why this resource is handwritten instead of generated via yaml in the apigee product folder: https://github.com/GoogleCloudPlatform/magic-modules/tree/main/mmv1/products/apigee
Looking through the schema, I don't believe I'm seeing anything out of the ordinary. If its possible, we'd prefer to have the yaml files be the source of truth for resource schemas.
| resource "google_apigee_api" "test_apigee_api" { | ||
| name = "tf-test-apigee-api" | ||
| org_id = google_project.project.project_id | ||
| config_bundle = "%{config_bundle}" |
There was a problem hiding this comment.
By changing the config_bundle here, are we expecting the api_deployment resource to force recreate?
There was a problem hiding this comment.
We are - when I ran this acceptance test I saw that both a revision 1 and 2 were deployed. This is akin to the shared flow test.
There was a problem hiding this comment.
This resource is handwritten because the related google_apigee_sharedflow_deployment is also handwritten, and the use case is very similar. I can look into the MMv1 format if you'd prefer that.
There was a problem hiding this comment.
Thanks for sharing the similar resource. Looking at its creation PR
#7202, I see
Using handwritten approach for sharedflow as create API requires a file to be uploaded as `octet-stream` and mmv1 does not support this pattern
It looks like Sharedflow needed be handwritten, but I'm very doubtful SharedflowDeployment needed to be. It does have a non-standard create URL but it seems google_apigee_api_deployment does not.
Handwritten resources are considered only to be created when absolutely necessary and make it harder for maintainability over time (divergence from large sweeping changes to the engine, harder for our metric tracking). I appreciate that this resource is already complete, but I have to ask you to please recreate it following the standard "MMv1" method here: https://googlecloudplatform.github.io/magic-modules/develop/add-resource/. This resource is fairly small, so hopefully it should be quick.
This will reduce the need for the handwritten documentation file as well. You could keep the test file you already have, but there would need to be at least one create test example for the generated documentation.
There was a problem hiding this comment.
It took me some time to figure out but I believe I have constructed a new resource using the MMv1 templating format. It looks like the generated test file passes as well. Please take a look and let me know if there's anything else you'll need!
|
@c2thorn This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
Does it look successful from your end? |
|
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: 67 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
|
|
I see there is a failing check, that seems to be a test. I cannot see the reason behind the issue, do I need to make a change? |
|
/gcbrun |
Feel free to ignore the failed google conversion integration tests. Sorry for the confusion. |
|
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: 67 Click here to see the affected service packages
🟢 All tests passed! View the build log |
c92357c
Fixes hashicorp/terraform-provider-google#24218
Provides a resource called
google_apigee_api_deploymentthat manages Apigee proxy deployments, similar to the existing resourcegoogle_apigee_sharedflow_deployment.Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.