-
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
Add manual converter and tests for service_account #7146
Add manual converter and tests for service_account #7146
Conversation
Hello! I am a robot who works on Magic Modules PRs. I've detected that you're a community contributor. @rileykarson, a repository maintainer, has been assigned to assist you and help review your changes. ❓ First time contributing? Click here for more detailsYour assigned reviewer will help review your code by:
You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails. If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox. |
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. TF Validator: Diff ( 11 files changed, 525 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccRegionInstanceGroupManager_stateful|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccVertexAIIndex_updated|TestAccLoggingBucketConfigProject_cmekSettings|TestAccFirebaserulesRelease_BasicRelease |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
I will investigate why integration-test are not passing |
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. TF Validator: Diff ( 8 files changed, 317 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccCloudfunctions2function_cloudfunctions2BasicGcsExample |
Tests failed during RECORDING mode: Please fix these to complete your PR |
// This file is copied here by Magic Modules. The code for building up a | ||
// storage bucket object is copied from the manually implemented | ||
// provider file: | ||
// third_party/terraform/resources/resource_storage_bucket.go |
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.
should this be updated as well?
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.
Fix and removed
The generate-diffs are failing for some 500 error, would be good if you can trigger a rerun by doing a minor change to see whether it can work next time. |
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. TF Validator: Diff ( 9 files changed, 324 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccCloudfunctions2function_cloudfunctions2BasicGcsExample |
Tests failed during RECORDING mode: Please fix these to complete your PR |
Minor change has been pushed. Checks and their results are visible now |
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. TF Validator: Diff ( 13 files changed, 560 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccContainerCluster_failedCreation|TestAccRegionInstanceGroupManager_stateful |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
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. TF Validator: Diff ( 13 files changed, 555 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccContainerCluster_failedCreation |
@iyabchen as discussed I have added a test case for service account existing. |
@@ -0,0 +1,35 @@ | |||
/** | |||
* Copyright 2021 Google LLC |
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.
2023?
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.
Fixed
magic-modules/mmv1/third_party/validator/tests/data/example_service_account_update.tf
Line 2 in 2c65ec7
* Copyright 2023 Google LLC |
Looks good since the test case is proving it works well. @melinath could you take a look? since I don't have permission to merge. |
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. TF Validator: Diff ( 13 files changed, 555 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccFirebaserulesRelease_BasicRelease|TestAccContainerCluster_failedCreation|TestAccRegionInstanceGroupManager_stateful |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
/gcbrun |
This LGTM but I'd like to re-run just to make sure there weren't any changes that broke something in the last two weeks. Since this only changes TFV, we don't need to wait for the full VCR tests. Apologies for the delayed response, I've been catching up on old reviews. |
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. TF Validator: Diff ( 13 files changed, 555 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccFirebaserulesRelease_BasicRelease|TestAccRegionInstanceGroupManager_stateful|TestAccFrameworkProviderMeta_setModuleName|TestAccDataSourceDnsRecordSet_basic |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
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.
LGTM
…rm#7146) * Add manual converter and tests for service_account * Remove test expecting resource to be already existing * Fix compute_instance test with service account * Remove not needed comments * Adding test for service account update * Correct tf state file to match version 0.12.31 * Fixed copyright header year
…rm#7146) * Add manual converter and tests for service_account * Remove test expecting resource to be already existing * Fix compute_instance test with service account * Remove not needed comments * Adding test for service account update * Correct tf state file to match version 0.12.31 * Fixed copyright header year
Add google_service_account for terraform validator
The converter is not existing and has to be created manually. Also, adding information into the converter list and add test cases.
Fixed GoogleCloudPlatform/terraform-validator#1290
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)