-
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
Support serviceAccount field for AppEngine flex #6391
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Hello! I am a robot who works on Magic Modules PRs. I've detected that you're a community contributor. @melinath, 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. |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccAccessApprovalSettings|TestAccComputeInstance_networkPerformanceConfig|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccCloudFunctions2Function_fullUpdate |
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.
Thanks for your contribution! Please complete the third checkbox by adding this field to an example or (if that's not possible) to a handwritten test in the third_party folder
Added field to existing example |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccComputeInstance_networkPerformanceConfig|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudFunctions2Function_fullUpdate|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccAppEngineFlexibleAppVersion_appEngineFlexibleAppVersionExample|TestAccSqlUser_mysqlDisabled |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
@melinath i added an example, but unable to view failed test cases and logs. It says I don't have access to Google cloud bucket. |
mmv1/templates/terraform/examples/app_engine_flexible_app_version.tf.erb
Show resolved
Hide resolved
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccComputeInstance_networkPerformanceConfig|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudFunctions2Function_fullUpdate|TestAccAppEngineFlexibleAppVersion_appEngineFlexibleAppVersionExample|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
I think this feature could be easily added the |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccComputeInstance_networkPerformanceConfig|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCloudfunctions2function_cloudfunctions2BasicAuditlogsExample|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudfunctions2function_cloudfunctions2BasicGcsExample|TestAccCloudFunctions2Function_fullUpdate|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccAppEngineFlexibleAppVersion_appEngineFlexibleAppVersionExample |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
It looks like still the same failure - I'm out until next Tuesday but I can take a deeper look next week unless you figure it out in the meantime. |
Looking at this again - it looks like the error message did change slightly, actually - I think this is a configuration issue in our CI/CD environment. I'll take a look tomorrow. |
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.
okay, I see what's happening here: because this test creates a new project, the cloudservices default service account for that project doesn't have permissions on the service account running the tests overall.
Sorry for getting this wrong previously. I recommend the following:
Go back to creating a new service account in this test (like in 2adaef1) but with a few changes:
- set a custom account name like
my-account
that has a hyphen - using the
vars
map (nottest_env_vars
) - make sure you set the project field so that the service account is created inside the project for this test (so that the cloudservices service account will have correct access to it by default)
I think that should do it!
Thanks, I will update PR by EOD today. |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccComputeInstance_advancedMachineFeatures|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccComputeInstanceTemplate_AdvancedMachineFeatures|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccAppEngineFlexibleAppVersion_appEngineFlexibleAppVersionExample|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccComputeInstance_soleTenantNodeAffinities |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
hmm, rerunning tests because the failure looks potentially transient |
/gcbrun |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 62 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccComputeInstance_advancedMachineFeatures|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccComputeInstanceTemplate_AdvancedMachineFeatures|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccAppEngineFlexibleAppVersion_appEngineFlexibleAppVersionExample|TestAccComputeInstance_soleTenantNodeAffinities|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccFirebaserulesRelease_BasicRelease|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
Hmm, this is still failing - now with the following message:
I'm going to rerun just the impacted test ( |
Thanks, but i am not able to open the link to see status. May be we can try adding |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 74 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccSqlDatabaseInstance_SqlServerAuditConfig|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccAppEngineFlexibleAppVersion_appEngineFlexibleAppVersionExample |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
hmm, now it's failing to enable the API - that seems likely a flake. I'm doing a manual run of the test: https://ci-oss.hashicorp.engineering/buildConfiguration/GoogleCloud_ProviderGoogleCloudMmUpstream/333356 |
Thank you, yeah sounds flaky. what was the result? which API? |
The flakeyness was in trying to enable the appengine API. The manual run has passed, so I think your change may have been the last fix. I'm going to trigger another full test run out of an abundance of caution. /gcbrun |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 74 insertions(+), 2 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccPrivatecaCertificateAuthority_privatecaCertificateAuthoritySubordinateExample|TestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeGlobalForwardingRule_internalLoadBalancing|TestAccCloudRunService_cloudRunServiceStaticOutboundExample|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccAppEngineFlexibleAppVersion_appEngineFlexibleAppVersionExample|TestAccSqlDatabaseInstance_withPrivateNetwork_withAllocatedIpRange|TestAccSqlDatabaseInstance_SqlServerAuditConfig |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
Thanks @melinath for helping to get this in, feels nice to contribute back to terraform community. |
for sure, this was a rough one - thanks for sticking with it! |
…6391) * Support serviceAccount field for AppEngine flex * Add service account example * Update account_id * Use vars instead of test env vars * Move account id to right tf example * Update mmv1/templates/terraform/examples/app_engine_flexible_app_version.tf.erb Co-authored-by: Stephen Lewis (Burrows) <stephen.r.burrows@gmail.com> * Add logs writer and network user role * Update mmv1/templates/terraform/examples/app_engine_flexible_app_version.tf.erb Co-authored-by: Stephen Lewis (Burrows) <stephen.r.burrows@gmail.com> * Add objectViewer role Co-authored-by: Stephen Lewis (Burrows) <stephen.r.burrows@gmail.com>
Fixes hashicorp/terraform-provider-google#11813
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)