-
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
Added new resource for managing envirommnent add-ons configuration #12417
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @melinath, 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. |
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.
thanks for your PR! It looks like downstream generation is currently failing. I have some change suggestions below. I'd also recommend generating the providers locally after you make changes to make sure that's working, to speed up the review process: https://googlecloudplatform.github.io/magic-modules/develop/generate-providers/
async: | ||
actions: ['update'] | ||
type: 'OpAsync' | ||
operation: | ||
base_url: '{{op_id}}' | ||
path: 'name' | ||
wait_ms: 1000 | ||
result: | ||
path: 'response' | ||
resource_inside_response: false | ||
error: | ||
path: 'error' | ||
message: 'message' |
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.
Most of these are unused - see hashicorp/terraform-provider-google#17098 for more details.
async: | |
actions: ['update'] | |
type: 'OpAsync' | |
operation: | |
base_url: '{{op_id}}' | |
path: 'name' | |
wait_ms: 1000 | |
result: | |
path: 'response' | |
resource_inside_response: false | |
error: | |
path: 'error' | |
message: 'message' | |
async: | |
actions: ['update'] | |
type: 'OpAsync' | |
operation: | |
base_url: '{{op_id}}' |
description: Enable/Disable add-ons for an Apigee environment. | ||
references: | ||
guides: | ||
'Creating an environment': 'https://cloud.google.com/apigee/docs/api-platform/get-started/create-environment' |
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.
this doesn't seem to be specific to EnvironmentAddonsConfig; any chance there's a more specific guide?
base_url: '{{env_id}}/addonsConfig' | ||
self_link: '{{env_id}}/addonsConfig' | ||
update_url: '{{env_id}}/addonsConfig:setAddonEnablement' | ||
update_verb: 'POST' |
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.
It looks like you might need delete_url and delete_verb similar to AddonsConfig? Not sure though. This is a weird resource.
magic-modules/mmv1/products/apigee/AddonsConfig.yaml
Lines 28 to 29 in 3bde930
delete_url: 'organizations/{{org}}:setAddons' | |
delete_verb: 'POST' |
update_url: '{{env_id}}/addonsConfig:setAddonEnablement' | ||
update_verb: 'POST' | ||
timeouts: | ||
update_minutes: 5 |
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.
why is this timeout so short?
message: 'message' | ||
custom_code: | ||
custom_import: 'templates/terraform/custom_import/apigee_env_addons.go.tmpl' | ||
test_check_destroy: 'templates/terraform/custom_check_destroy/apigee_env_addons_override.go.tmpl' |
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.
This is currently causing downstream generation to fail with the error:
open templates/terraform/custom_check_destroy/apigee_env_addons_override.go.tmpl: no such file or directory
It looks like you might need a custom test_check_destroy function since this is such a weird resource, possibly similar to https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/templates/terraform/custom_check_destroy/apigee_addons_override.go.tmpl
test_env_vars: | ||
org_id: 'ORG_ID' | ||
billing_account: 'BILLING_ACCT' | ||
exclude_docs: true |
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.
Right now both examples are excluding docs - but we do need docs. Would it be okay to remove this from one (or both) of them, or to add a separate docs-only example similar to addons_config?
} | ||
|
||
resource "google_apigee_environment_addons_config" "{{$.PrimaryResourceId}}" { | ||
org = google_apigee_organization.org.name |
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 think this should be the env rather than the org
} | ||
|
||
resource "google_apigee_environment_addons_config" "{{$.PrimaryResourceId}}" { | ||
org = google_apigee_organization.org.name |
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.
same here
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.
marking as changes requested
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.