googleapis · dbolduc · Apr 1, 2022 · Apr 1, 2022
@@ -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:

@@ -32,8 +32,6 @@
 #include "absl/memory/memory.h"
 #include <google/protobuf/text_format.h>
 #include <gmock/gmock.h>
-// TODO(#5929) - remove once deprecated functions are removed
-#include "google/cloud/internal/disable_deprecation_warnings.inc"
 
 namespace google {
 namespace cloud {

@@ -16,8 +16,6 @@
 #include <sstream>
 #include <string>
 #include <type_traits>
-// 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<google::cloud::IamPolicy> 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<google::iam::v1::Policy> InstanceAdmin::GetNativeIamPolicy(
     std::string const& instance_id) {
   google::cloud::internal::OptionsSpan span(policies_);
@@ -210,29 +198,6 @@ StatusOr<google::iam::v1::Policy> InstanceAdmin::GetNativeIamPolicy(
   return connection_->GetIamPolicy(request);
 }
 
-StatusOr<google::cloud::IamPolicy> 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<google::iam::v1::Policy> 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<std::vector<std::string>> InstanceAdmin::TestIamPermissions(
   return result;
 }
 
-StatusOr<google::cloud::IamPolicy> 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<google::protobuf::FieldDescriptor const*> 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

@@ -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<google::cloud::IamPolicy> GetIamPolicy(
-      std::string const& instance_id);
-
   /**
    * Gets the native policy for @p instance_id.
    *
@@ -663,43 +638,6 @@ class InstanceAdmin {
   StatusOr<google::iam::v1::Policy> 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<google::cloud::IamPolicy> 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.
    *

@@ -17,8 +17,6 @@
 #include "google/cloud/bigtable/testing/mock_policies.h"
 #include "google/cloud/testing_util/status_matchers.h"
 #include <gmock/gmock.h>
-// 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;

@@ -26,8 +26,6 @@
 #include "absl/memory/memory.h"
 #include <google/protobuf/text_format.h>
 #include <gmock/gmock.h>
-// 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 =

@@ -20,13 +20,6 @@
 #include "google/cloud/version.h"
 #include <string>
 
-#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.