Skip to content
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

fix: retries for GetIamPolicy, TestIamPermissions #10957

Merged
merged 3 commits into from
Feb 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,11 @@ Idempotency GoldenThingAdminConnectionIdempotencyPolicy::SetIamPolicy(
}

Idempotency GoldenThingAdminConnectionIdempotencyPolicy::GetIamPolicy(google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency GoldenThingAdminConnectionIdempotencyPolicy::TestIamPermissions(google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency GoldenThingAdminConnectionIdempotencyPolicy::CreateBackup(google::test::admin::database::v1::CreateBackupRequest const&) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ TEST_F(GoldenIdempotencyPolicyTest, SetIamPolicy) {

TEST_F(GoldenIdempotencyPolicyTest, GetIamPolicy) {
google::iam::v1::GetIamPolicyRequest request;
EXPECT_EQ(policy_->GetIamPolicy(request), Idempotency::kNonIdempotent);
EXPECT_EQ(policy_->GetIamPolicy(request), Idempotency::kIdempotent);
}

TEST_F(GoldenIdempotencyPolicyTest, TestIamPermissions) {
google::iam::v1::TestIamPermissionsRequest request;
EXPECT_EQ(policy_->TestIamPermissions(request), Idempotency::kNonIdempotent);
EXPECT_EQ(policy_->TestIamPermissions(request), Idempotency::kIdempotent);
}

TEST_F(GoldenIdempotencyPolicyTest, CreateBackup) {
Expand Down
4 changes: 4 additions & 0 deletions generator/internal/descriptor_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,10 @@ void SetHttpGetQueryParameters(

std::string DefaultIdempotencyFromHttpOperation(
google::protobuf::MethodDescriptor const& method) {
if (method.name() == "GetIamPolicy" ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: there is prior code to detect SetIamPolicy methods, maybe you should have followed the same structure as those?

method.name() == "TestIamPermissions") {
return "kIdempotent";
}
if (method.options().HasExtension(google::api::http)) {
google::api::HttpRule http_rule =
method.options().GetExtension(google::api::http);
Expand Down
21 changes: 20 additions & 1 deletion generator/internal/descriptor_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,20 @@ char const* const kServiceProto =
" option (google.api.method_signature) = \"name,parent,number\";\n"
" option (google.api.method_signature) = \"name,title,number\";\n"
" }\n"
" // Test that the method defaults to kIdempotent.\n"
" rpc GetIamPolicy(Empty) returns (Empty) {\n"
" option (google.api.http) = {\n"
" post: \"/v1/foo\"\n"
" body: \"*\"\n"
" };\n"
" }\n"
" // Test that the method defaults to kIdempotent.\n"
" rpc TestIamPermissions(Empty) returns (Empty) {\n"
" option (google.api.http) = {\n"
" post: \"/v1/foo\"\n"
" body: \"*\"\n"
" };\n"
" }\n"
"}\n";

char const* const kMethod6Deprecated1 = // name,not_used_anymore
Expand Down Expand Up @@ -952,7 +966,12 @@ INSTANTIATE_TEST_SUITE_P(
R"""(,
{std::make_pair("page_size", std::to_string(request.page_size())),
std::make_pair("page_token", request.page_token()),
std::make_pair("name", request.name())})""")),
std::make_pair("name", request.name())})"""),
// IAM idempotency defaults
MethodVarsTestValues("google.protobuf.Service.GetIamPolicy",
"idempotency", "kIdempotent"),
MethodVarsTestValues("google.protobuf.Service.TestIamPermissions",
"idempotency", "kIdempotent")),
[](testing::TestParamInfo<CreateMethodVarsTest::ParamType> const& info) {
std::vector<std::string> pieces = absl::StrSplit(info.param.method, '.');
return pieces.back() + "_" + info.param.vars_key;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,12 @@ Idempotency AccessContextManagerConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency AccessContextManagerConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency AccessContextManagerConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<AccessContextManagerConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ Idempotency ArtifactRegistryConnectionIdempotencyPolicy::GetIamPolicy(

Idempotency ArtifactRegistryConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency ArtifactRegistryConnectionIdempotencyPolicy::GetProjectSettings(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ Idempotency AnalyticsHubServiceConnectionIdempotencyPolicy::SubscribeListing(

Idempotency AnalyticsHubServiceConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency AnalyticsHubServiceConnectionIdempotencyPolicy::SetIamPolicy(
Expand All @@ -115,7 +115,7 @@ Idempotency AnalyticsHubServiceConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency AnalyticsHubServiceConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<AnalyticsHubServiceConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ Idempotency ConnectionServiceConnectionIdempotencyPolicy::DeleteConnection(

Idempotency ConnectionServiceConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency ConnectionServiceConnectionIdempotencyPolicy::SetIamPolicy(
Expand All @@ -74,7 +74,7 @@ Idempotency ConnectionServiceConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency ConnectionServiceConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<ConnectionServiceConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Idempotency DataPolicyServiceConnectionIdempotencyPolicy::ListDataPolicies(

Idempotency DataPolicyServiceConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency DataPolicyServiceConnectionIdempotencyPolicy::SetIamPolicy(
Expand All @@ -79,7 +79,7 @@ Idempotency DataPolicyServiceConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency DataPolicyServiceConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<DataPolicyServiceConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ Idempotency BigtableInstanceAdminConnectionIdempotencyPolicy::DeleteAppProfile(

Idempotency BigtableInstanceAdminConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency BigtableInstanceAdminConnectionIdempotencyPolicy::SetIamPolicy(
Expand All @@ -137,7 +137,7 @@ Idempotency BigtableInstanceAdminConnectionIdempotencyPolicy::SetIamPolicy(
Idempotency
BigtableInstanceAdminConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency BigtableInstanceAdminConnectionIdempotencyPolicy::ListHotTablets(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ Idempotency BigtableTableAdminConnectionIdempotencyPolicy::RestoreTable(

Idempotency BigtableTableAdminConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency BigtableTableAdminConnectionIdempotencyPolicy::SetIamPolicy(
Expand All @@ -130,7 +130,7 @@ Idempotency BigtableTableAdminConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency BigtableTableAdminConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<BigtableTableAdminConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ Idempotency CloudBillingConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency CloudBillingConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<CloudBillingConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ Idempotency ContainerAnalysisConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency ContainerAnalysisConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency ContainerAnalysisConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency ContainerAnalysisConnectionIdempotencyPolicy::
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,12 @@ Idempotency DataCatalogConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency DataCatalogConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency DataCatalogConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<DataCatalogConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ Idempotency PolicyTagManagerConnectionIdempotencyPolicy::GetPolicyTag(

Idempotency PolicyTagManagerConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency PolicyTagManagerConnectionIdempotencyPolicy::SetIamPolicy(
Expand All @@ -98,7 +98,7 @@ Idempotency PolicyTagManagerConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency PolicyTagManagerConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<PolicyTagManagerConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Idempotency ContentServiceConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency ContentServiceConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency ContentServiceConnectionIdempotencyPolicy::ListContent(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ Idempotency CloudFunctionsServiceConnectionIdempotencyPolicy::GetIamPolicy(
Idempotency
CloudFunctionsServiceConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<CloudFunctionsServiceConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ Idempotency IAMConnectionIdempotencyPolicy::EnableServiceAccountKey(

Idempotency IAMConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency IAMConnectionIdempotencyPolicy::SetIamPolicy(
Expand All @@ -122,7 +122,7 @@ Idempotency IAMConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency IAMConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency IAMConnectionIdempotencyPolicy::QueryGrantableRoles(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ Idempotency IAMPolicyConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency IAMPolicyConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency IAMPolicyConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<IAMPolicyConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ IdentityAwareProxyAdminServiceConnectionIdempotencyPolicy::SetIamPolicy(
Idempotency
IdentityAwareProxyAdminServiceConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency
IdentityAwareProxyAdminServiceConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,12 @@ Idempotency DeviceManagerConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency DeviceManagerConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency DeviceManagerConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency DeviceManagerConnectionIdempotencyPolicy::SendCommandToDevice(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Idempotency FoldersConnectionIdempotencyPolicy::UndeleteFolder(

Idempotency FoldersConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency FoldersConnectionIdempotencyPolicy::SetIamPolicy(
Expand All @@ -88,7 +88,7 @@ Idempotency FoldersConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency FoldersConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<FoldersConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Idempotency OrganizationsConnectionIdempotencyPolicy::SearchOrganizations(

Idempotency OrganizationsConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency OrganizationsConnectionIdempotencyPolicy::SetIamPolicy(
Expand All @@ -58,7 +58,7 @@ Idempotency OrganizationsConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency OrganizationsConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<OrganizationsConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Idempotency ProjectsConnectionIdempotencyPolicy::UndeleteProject(

Idempotency ProjectsConnectionIdempotencyPolicy::GetIamPolicy(
google::iam::v1::GetIamPolicyRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

Idempotency ProjectsConnectionIdempotencyPolicy::SetIamPolicy(
Expand All @@ -88,7 +88,7 @@ Idempotency ProjectsConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency ProjectsConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<ProjectsConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ Idempotency ServicesConnectionIdempotencyPolicy::SetIamPolicy(

Idempotency ServicesConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<ServicesConnectionIdempotencyPolicy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ Idempotency SecretManagerServiceConnectionIdempotencyPolicy::GetIamPolicy(

Idempotency SecretManagerServiceConnectionIdempotencyPolicy::TestIamPermissions(
google::iam::v1::TestIamPermissionsRequest const&) {
return Idempotency::kNonIdempotent;
return Idempotency::kIdempotent;
}

std::unique_ptr<SecretManagerServiceConnectionIdempotencyPolicy>
Expand Down
Loading