diff --git a/CHANGELOG.md b/CHANGELOG.md index d663b69e442c1..259330db41a79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,15 @@ https://github.com/googleapis/google-cloud-cpp/issues/8234. * This only changes the default C++ version, we continue to test and support C++11. * For more details, see [#6767](https://github.com/googleapis/google-cloud-cpp/issues/6767). +### [Bigtable](https://github.com/googleapis/google-cloud-cpp/blob/main/google/cloud/bigtable/README.md) + +**BREAKING CHANGES** + +* `InstanceAdmin::GetIamPolicy` and `InstanceAdmin::SetIamPolicy` have been + retired. If you are affected by this removal, please use + `InstanceAdmin::GetNativeIamPolicy` and `InstanceAdmin::SetNativeIamPolicy` + instead. See [#5929] for more details. + ### [KMS](https://github.com/googleapis/google-cloud-cpp/blob/main/google/cloud/kms/README.md) The library has been expanded to include the following services: diff --git a/google/cloud/bigtable/admin/integration_tests/instance_admin_integration_test.cc b/google/cloud/bigtable/admin/integration_tests/instance_admin_integration_test.cc index 871ab3f83daa7..cc6b543566c94 100644 --- a/google/cloud/bigtable/admin/integration_tests/instance_admin_integration_test.cc +++ b/google/cloud/bigtable/admin/integration_tests/instance_admin_integration_test.cc @@ -32,8 +32,6 @@ #include "absl/memory/memory.h" #include #include -// TODO(#5929) - remove once deprecated functions are removed -#include "google/cloud/internal/disable_deprecation_warnings.inc" namespace google { namespace cloud { diff --git a/google/cloud/bigtable/instance_admin.cc b/google/cloud/bigtable/instance_admin.cc index 7bd2cf5c3f2ef..6d93c08bfc34a 100644 --- a/google/cloud/bigtable/instance_admin.cc +++ b/google/cloud/bigtable/instance_admin.cc @@ -16,8 +16,6 @@ #include #include #include -// TODO(#5929) - remove after deprecation is completed -#include "google/cloud/internal/disable_deprecation_warnings.inc" namespace btadmin = ::google::bigtable::admin::v2; @@ -192,16 +190,6 @@ Status InstanceAdmin::DeleteAppProfile(std::string const& instance_id, return connection_->DeleteAppProfile(request); } -StatusOr InstanceAdmin::GetIamPolicy( - std::string const& instance_id) { - google::cloud::internal::OptionsSpan span(policies_); - google::iam::v1::GetIamPolicyRequest request; - request.set_resource(InstanceName(instance_id)); - auto sor = connection_->GetIamPolicy(request); - if (!sor) return std::move(sor).status(); - return ProtoToWrapper(*std::move(sor)); -} - StatusOr InstanceAdmin::GetNativeIamPolicy( std::string const& instance_id) { google::cloud::internal::OptionsSpan span(policies_); @@ -210,29 +198,6 @@ StatusOr InstanceAdmin::GetNativeIamPolicy( return connection_->GetIamPolicy(request); } -StatusOr InstanceAdmin::SetIamPolicy( - std::string const& instance_id, - google::cloud::IamBindings const& iam_bindings, std::string const& etag) { - google::cloud::internal::OptionsSpan span(policies_); - google::iam::v1::Policy policy; - policy.set_etag(etag); - auto role_bindings = iam_bindings.bindings(); - for (auto& binding : role_bindings) { - auto& new_binding = *policy.add_bindings(); - new_binding.set_role(binding.first); - for (auto const& member : binding.second) { - new_binding.add_members(member); - } - } - - google::iam::v1::SetIamPolicyRequest request; - request.set_resource(InstanceName(instance_id)); - *request.mutable_policy() = std::move(policy); - auto sor = connection_->SetIamPolicy(request); - if (!sor) return std::move(sor).status(); - return ProtoToWrapper(*std::move(sor)); -} - StatusOr InstanceAdmin::SetIamPolicy( std::string const& instance_id, google::iam::v1::Policy const& iam_policy) { google::cloud::internal::OptionsSpan span(policies_); @@ -260,32 +225,6 @@ StatusOr> InstanceAdmin::TestIamPermissions( return result; } -StatusOr InstanceAdmin::ProtoToWrapper( - google::iam::v1::Policy proto) { - google::cloud::IamPolicy result; - result.version = proto.version(); - result.etag = std::move(*proto.mutable_etag()); - for (auto& binding : *proto.mutable_bindings()) { - std::vector field_descs; - google::iam::v1::Binding::GetReflection()->ListFields(binding, - &field_descs); - for (auto const* field_desc : field_descs) { - if (field_desc->name() != "members" && field_desc->name() != "role") { - std::stringstream os; - os << "IamBinding field \"" << field_desc->name() - << "\" is unknown to Bigtable C++ client. Please use " - "GetNativeIamPolicy() and its" - "SetIamPolicy() overload."; - return Status(StatusCode::kUnimplemented, os.str()); - } - } - for (auto& member : *binding.mutable_members()) { - result.bindings.AddMember(binding.role(), std::move(member)); - } - } - return result; -} - GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END } // namespace bigtable } // namespace cloud diff --git a/google/cloud/bigtable/instance_admin.h b/google/cloud/bigtable/instance_admin.h index 6b946fefbbfd5..0b7eab2c20dcc 100644 --- a/google/cloud/bigtable/instance_admin.h +++ b/google/cloud/bigtable/instance_admin.h @@ -614,31 +614,6 @@ class InstanceAdmin { std::string const& profile_id, bool ignore_warnings = true); - /** - * Gets the policy for @p instance_id. - * - * @param instance_id the instance to query. - * @return Policy the full IAM policy for the instance. - * - * @deprecated this function is deprecated; it doesn't support conditional - * bindings and will not support any other features to come; please use - * `GetNativeIamPolicy` instead. - * - * @par Idempotency - * This operation is read-only and therefore it is always idempotent. - * - * @par Thread-safety - * Two threads concurrently calling this member function on the same instance - * of this class are **not** guaranteed to work. Consider copying the object - * and using different copies in each thread. - * - * @par Example - * Use #GetNativeIamPolicy() instead. - */ - GOOGLE_CLOUD_CPP_BIGTABLE_IAM_DEPRECATED("GetNativeIamPolicy") - StatusOr GetIamPolicy( - std::string const& instance_id); - /** * Gets the native policy for @p instance_id. * @@ -663,43 +638,6 @@ class InstanceAdmin { StatusOr GetNativeIamPolicy( std::string const& instance_id); - /** - * Sets the IAM policy for an instance. - * - * Applications can provide the @p etag to implement optimistic concurrency - * control. If @p etag is not empty, the server will reject calls where the - * provided ETag does not match the ETag value stored in the server. - * - * @param instance_id which instance to set the IAM policy for. - * @param iam_bindings IamBindings object containing role and members. - * @param etag the expected ETag value for the current policy. - * @return Policy the current IAM bindings for the instance. - * - * @deprecated this function is deprecated; it doesn't support conditional - * bindings and will not support any other features to come; please use - * the overload for `google::iam::v1::Policy` instead. - * - * @warning ETags are currently not used by Cloud Bigtable. - * - * @par Idempotency - * This operation is always treated as non-idempotent. - * - * @par Thread-safety - * Two threads concurrently calling this member function on the same instance - * of this class are **not** guaranteed to work. Consider copying the object - * and using different copies in each thread. - * - * @par Example - * Use #SetIamPolicy(std::string const&, google::iam::v1::Policy const&) - * instead. - */ - GOOGLE_CLOUD_CPP_BIGTABLE_IAM_DEPRECATED( - "SetIamPolicy(std::string const&, google::iam::v1::Policy const&)") - StatusOr SetIamPolicy( - std::string const& instance_id, - google::cloud::IamBindings const& iam_bindings, - std::string const& etag = std::string{}); - /** * Sets the IAM policy for an instance. * diff --git a/google/cloud/bigtable/instance_admin_test.cc b/google/cloud/bigtable/instance_admin_test.cc index c692de7e5a672..f744622c282b7 100644 --- a/google/cloud/bigtable/instance_admin_test.cc +++ b/google/cloud/bigtable/instance_admin_test.cc @@ -17,8 +17,6 @@ #include "google/cloud/bigtable/testing/mock_policies.h" #include "google/cloud/testing_util/status_matchers.h" #include -// TODO(#5929) - remove after deprecation is completed -#include "google/cloud/internal/disable_deprecation_warnings.inc" namespace google { namespace cloud { @@ -584,39 +582,6 @@ TEST_F(InstanceAdminTest, DeleteAppProfile) { EXPECT_STATUS_OK(tested.DeleteAppProfile(kInstanceId, kProfileId, true)); } -TEST_F(InstanceAdminTest, GetIamPolicySuccess) { - auto tested = InstanceAdmin(connection_, kProjectId); - - EXPECT_CALL(*connection_, GetIamPolicy) - .WillOnce([&](iamproto::GetIamPolicyRequest const& request) { - CheckPolicies(google::cloud::internal::CurrentOptions()); - EXPECT_EQ(kInstanceName, request.resource()); - iamproto::Policy p; - p.set_version(3); - p.set_etag("tag"); - return p; - }); - - auto policy = tested.GetIamPolicy(kInstanceId); - ASSERT_STATUS_OK(policy); - EXPECT_EQ(3, policy->version); - EXPECT_EQ("tag", policy->etag); -} - -TEST_F(InstanceAdminTest, GetIamPolicyFailure) { - auto tested = InstanceAdmin(connection_, kProjectId); - - EXPECT_CALL(*connection_, GetIamPolicy) - .WillOnce([&](iamproto::GetIamPolicyRequest const& request) { - CheckPolicies(google::cloud::internal::CurrentOptions()); - EXPECT_EQ(kInstanceName, request.resource()); - return FailingStatus(); - }); - - EXPECT_THAT(tested.GetIamPolicy(kInstanceId), - StatusIs(StatusCode::kPermissionDenied)); -} - TEST_F(InstanceAdminTest, GetNativeIamPolicy) { auto tested = InstanceAdmin(connection_, kProjectId); @@ -631,25 +596,6 @@ TEST_F(InstanceAdminTest, GetNativeIamPolicy) { StatusIs(StatusCode::kPermissionDenied)); } -TEST_F(InstanceAdminTest, SetIamPolicy) { - auto tested = InstanceAdmin(connection_, kProjectId); - google::cloud::IamBinding b1({"role1", {"m1", "m2"}}); - google::cloud::IamBinding b2({"role2", {"m1", "m3"}}); - google::cloud::IamBindings bindings({b1, b2}); - - EXPECT_CALL(*connection_, SetIamPolicy) - .WillOnce([&](iamproto::SetIamPolicyRequest const& request) { - CheckPolicies(google::cloud::internal::CurrentOptions()); - EXPECT_EQ(kInstanceName, request.resource()); - EXPECT_EQ("tag", request.policy().etag()); - EXPECT_EQ(2, request.policy().bindings_size()); - return FailingStatus(); - }); - - EXPECT_THAT(tested.SetIamPolicy(kInstanceId, bindings, "tag"), - StatusIs(StatusCode::kPermissionDenied)); -} - TEST_F(InstanceAdminTest, SetNativeIamPolicy) { auto tested = InstanceAdmin(connection_, kProjectId); iamproto::Policy policy; diff --git a/google/cloud/bigtable/tests/instance_admin_integration_test.cc b/google/cloud/bigtable/tests/instance_admin_integration_test.cc index 02a86301c076b..4c0754ab4ef35 100644 --- a/google/cloud/bigtable/tests/instance_admin_integration_test.cc +++ b/google/cloud/bigtable/tests/instance_admin_integration_test.cc @@ -26,8 +26,6 @@ #include "absl/memory/memory.h" #include #include -// TODO(#5929) - remove once deprecated functions are removed -#include "google/cloud/internal/disable_deprecation_warnings.inc" namespace google { namespace cloud { @@ -342,38 +340,6 @@ TEST_F(InstanceAdminIntegrationTest, CreateListGetDeleteClusterTest) { ASSERT_STATUS_OK(instance_admin_->DeleteInstance(instance_id)); } -/// @test Verify that IAM Policy APIs work as expected. -TEST_F(InstanceAdminIntegrationTest, SetGetTestIamAPIsTest) { - auto const instance_id = - "it-" + google::cloud::internal::Sample( - generator_, 8, "abcdefghijklmnopqrstuvwxyz0123456789"); - - // Create instance prerequisites for cluster operations - auto config = IntegrationTestConfig(instance_id, zone_a_, - bigtable::InstanceConfig::PRODUCTION, 3); - ASSERT_STATUS_OK(instance_admin_->CreateInstance(config).get()); - - auto iam_bindings = google::cloud::IamBindings( - "roles/bigtable.reader", {"serviceAccount:" + service_account_}); - - auto initial_policy = - instance_admin_->SetIamPolicy(instance_id, iam_bindings); - ASSERT_STATUS_OK(initial_policy); - - auto fetched_policy = instance_admin_->GetIamPolicy(instance_id); - ASSERT_STATUS_OK(fetched_policy); - - EXPECT_EQ(initial_policy->version, fetched_policy->version); - EXPECT_EQ(initial_policy->etag, fetched_policy->etag); - - auto permission_set = instance_admin_->TestIamPermissions( - instance_id, {"bigtable.tables.list", "bigtable.tables.delete"}); - ASSERT_STATUS_OK(permission_set); - - EXPECT_EQ(2, permission_set->size()); - EXPECT_STATUS_OK(instance_admin_->DeleteInstance(instance_id)); -} - /// @test Verify that IAM Policy Native APIs work as expected. TEST_F(InstanceAdminIntegrationTest, SetGetTestIamNativeAPIsTest) { auto const instance_id = diff --git a/google/cloud/bigtable/version.h b/google/cloud/bigtable/version.h index f3c8fdaa00baf..aa1c661ced0f2 100644 --- a/google/cloud/bigtable/version.h +++ b/google/cloud/bigtable/version.h @@ -20,13 +20,6 @@ #include "google/cloud/version.h" #include -#define GOOGLE_CLOUD_CPP_BIGTABLE_IAM_DEPRECATED(alternative) \ - GOOGLE_CLOUD_CPP_DEPRECATED( \ - "this function predates IAM conditions and does not work with policies " \ - "that include IAM conditions. Please use " alternative \ - " instead. The function will be removed on 2022-04-01 or shortly " \ - "after. See GitHub issue #5929 for more information.") - // This preprocessor symbol is deprecated and should never be used anywhere. It // exists solely for backward compatibility to avoid breaking anyone who may // have been using it.