Skip to content

Commit

Permalink
Update authz tests to use callback (grpc#30058)
Browse files Browse the repository at this point in the history 
* Update authz core tests to use callback
  • Loading branch information
@ashithasantosh
ashithasantosh authored Jun 30, 2022
1 parent 0fc0384 commit 9bc16ed
Show file tree
Hide file tree
Showing 5 changed files with 243 additions and 40 deletions.
24 changes: 21 additions & 3 deletions src/core/lib/security/authorization/grpc_authorization_policy_provider.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,25 +130,43 @@ FileWatcherAuthorizationPolicyProvider::FileWatcherAuthorizationPolicyProvider(
refresh_thread_->Start();
}

void FileWatcherAuthorizationPolicyProvider::SetCallbackForTesting(
std::function<void(bool contents_changed, absl::Status status)> cb) {
MutexLock lock(&mu_);
cb_ = std::move(cb);
}

absl::Status FileWatcherAuthorizationPolicyProvider::ForceUpdate() {
bool contents_changed = false;
auto done_early = [&](absl::Status status) {
MutexLock lock(&mu_);
if (cb_ != nullptr) {
cb_(contents_changed, status);
}
return status;
};
absl::StatusOr<std::string> file_contents =
ReadPolicyFromFile(authz_policy_path_);
if (!file_contents.ok()) {
return file_contents.status();
return done_early(file_contents.status());
}
if (file_contents_ == *file_contents) {
return absl::OkStatus();
return done_early(absl::OkStatus());
}
file_contents_ = std::move(*file_contents);
contents_changed = true;
auto rbac_policies_or = GenerateRbacPolicies(file_contents_);
if (!rbac_policies_or.ok()) {
return rbac_policies_or.status();
return done_early(rbac_policies_or.status());
}
MutexLock lock(&mu_);
allow_engine_ = MakeRefCounted<GrpcAuthorizationEngine>(
std::move(rbac_policies_or->allow_policy));
deny_engine_ = MakeRefCounted<GrpcAuthorizationEngine>(
std::move(rbac_policies_or->deny_policy));
if (cb_ != nullptr) {
cb_(contents_changed, absl::OkStatus());
}
if (GRPC_TRACE_FLAG_ENABLED(grpc_authz_trace)) {
gpr_log(GPR_INFO,
"authorization policy reload status: successfully loaded new "
Expand Down
7 changes: 7 additions & 0 deletions src/core/lib/security/authorization/grpc_authorization_policy_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <grpc/support/port_platform.h>

#include <functional>
#include <memory>
#include <string>

Expand Down Expand Up @@ -83,6 +84,9 @@ class FileWatcherAuthorizationPolicyProvider
unsigned int refresh_interval_sec,
absl::Status* status);

void SetCallbackForTesting(
std::function<void(bool contents_changed, absl::Status Status)> cb);

void Orphan() override;

AuthorizationEngines engines() override {
Expand All @@ -102,6 +106,9 @@ class FileWatcherAuthorizationPolicyProvider
gpr_event shutdown_event_;

Mutex mu_;
// Callback is executed on every reload. This is useful for testing purpose.
std::function<void(bool contents_changed, absl::Status status)> cb_
ABSL_GUARDED_BY(mu_) = nullptr;
// Engines created using authz_policy_.
RefCountedPtr<AuthorizationEngine> allow_engine_ ABSL_GUARDED_BY(mu_);
RefCountedPtr<AuthorizationEngine> deny_engine_ ABSL_GUARDED_BY(mu_);
Expand Down
83 changes: 65 additions & 18 deletions test/core/end2end/tests/grpc_authz.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,6 @@ static gpr_timespec five_seconds_from_now(void) {
return n_seconds_from_now(5);
}

static void wait_for_policy_reload(void) {
// Wait for the provider's refresh thread to read the updated files.
// TODO(jtattermusch): Refactor the tests to use a more reliable mechanism of
// detecting that the policy has been reloaded. See b/204329811
gpr_sleep_until(grpc_timeout_seconds_to_deadline(5));
}

static void drain_cq(grpc_completion_queue* cq) {
grpc_event ev;
do {
Expand All @@ -70,8 +63,7 @@ static void shutdown_server(grpc_end2end_test_fixture* f) {
grpc_server_shutdown_and_notify(f->server, f->cq, tag(1000));
grpc_event ev;
do {
ev = grpc_completion_queue_next(f->cq, grpc_timeout_seconds_to_deadline(5),
nullptr);
ev = grpc_completion_queue_next(f->cq, five_seconds_from_now(), nullptr);
} while (ev.type != GRPC_OP_COMPLETE || ev.tag != tag(1000));
grpc_server_destroy(f->server);
f->server = nullptr;
Expand Down Expand Up @@ -557,6 +549,17 @@ static void test_file_watcher_valid_policy_reload(
config, "test_file_watcher_valid_policy_reload", nullptr, &server_args);
grpc_authorization_policy_provider_release(provider);
test_allow_authorized_request(f);
gpr_event on_reload_done;
gpr_event_init(&on_reload_done);
std::function<void(bool contents_changed, absl::Status status)> callback =
[&on_reload_done](bool contents_changed, absl::Status status) {
if (contents_changed) {
GPR_ASSERT(status.ok());
gpr_event_set(&on_reload_done, reinterpret_cast<void*>(1));
}
};
dynamic_cast<grpc_core::FileWatcherAuthorizationPolicyProvider*>(provider)
->SetCallbackForTesting(std::move(callback));
// Replace existing policy in file with a different authorization policy.
authz_policy =
"{"
Expand All @@ -583,8 +586,12 @@ static void test_file_watcher_valid_policy_reload(
" ]"
"}";
tmp_policy.RewriteFile(authz_policy);
wait_for_policy_reload();
GPR_ASSERT(
reinterpret_cast<void*>(1) ==
gpr_event_wait(&on_reload_done, gpr_inf_future(GPR_CLOCK_MONOTONIC)));
test_deny_unauthorized_request(f);
dynamic_cast<grpc_core::FileWatcherAuthorizationPolicyProvider*>(provider)
->SetCallbackForTesting(nullptr);

end_test(&f);
config.tear_down_data(&f);
Expand Down Expand Up @@ -626,17 +633,32 @@ static void test_file_watcher_invalid_policy_skip_reload(
nullptr, &server_args);
grpc_authorization_policy_provider_release(provider);
test_allow_authorized_request(f);
gpr_event on_reload_done;
gpr_event_init(&on_reload_done);
std::function<void(bool contents_changed, absl::Status status)> callback =
[&on_reload_done](bool contents_changed, absl::Status status) {
if (contents_changed) {
GPR_ASSERT(absl::StatusCode::kInvalidArgument == status.code());
GPR_ASSERT("\"name\" field is not present." == status.message());
gpr_event_set(&on_reload_done, reinterpret_cast<void*>(1));
}
};
dynamic_cast<grpc_core::FileWatcherAuthorizationPolicyProvider*>(provider)
->SetCallbackForTesting(std::move(callback));
// Replace exisiting policy in file with an invalid policy.
authz_policy = "{}";
tmp_policy.RewriteFile(authz_policy);
wait_for_policy_reload();
GPR_ASSERT(
reinterpret_cast<void*>(1) ==
gpr_event_wait(&on_reload_done, gpr_inf_future(GPR_CLOCK_MONOTONIC)));
test_allow_authorized_request(f);
dynamic_cast<grpc_core::FileWatcherAuthorizationPolicyProvider*>(provider)
->SetCallbackForTesting(nullptr);

end_test(&f);
config.tear_down_data(&f);
}

#ifndef GPR_APPLE
static void test_file_watcher_recovers_from_failure(
grpc_end2end_test_config config) {
const char* authz_policy =
Expand Down Expand Up @@ -672,11 +694,36 @@ static void test_file_watcher_recovers_from_failure(
config, "test_file_watcher_recovers_from_failure", nullptr, &server_args);
grpc_authorization_policy_provider_release(provider);
test_allow_authorized_request(f);
gpr_event on_first_reload_done;
gpr_event_init(&on_first_reload_done);
std::function<void(bool contents_changed, absl::Status status)> callback1 =
[&on_first_reload_done](bool contents_changed, absl::Status status) {
if (contents_changed) {
GPR_ASSERT(absl::StatusCode::kInvalidArgument == status.code());
GPR_ASSERT("\"name\" field is not present." == status.message());
gpr_event_set(&on_first_reload_done, reinterpret_cast<void*>(1));
}
};
dynamic_cast<grpc_core::FileWatcherAuthorizationPolicyProvider*>(provider)
->SetCallbackForTesting(std::move(callback1));
// Replace exisiting policy in file with an invalid policy.
authz_policy = "{}";
tmp_policy.RewriteFile(authz_policy);
wait_for_policy_reload();
GPR_ASSERT(reinterpret_cast<void*>(1) ==
gpr_event_wait(&on_first_reload_done,
gpr_inf_future(GPR_CLOCK_MONOTONIC)));
test_allow_authorized_request(f);
gpr_event on_second_reload_done;
gpr_event_init(&on_second_reload_done);
std::function<void(bool contents_changed, absl::Status status)> callback2 =
[&on_second_reload_done](bool contents_changed, absl::Status status) {
if (contents_changed) {
GPR_ASSERT(status.ok());
gpr_event_set(&on_second_reload_done, reinterpret_cast<void*>(1));
}
};
dynamic_cast<grpc_core::FileWatcherAuthorizationPolicyProvider*>(provider)
->SetCallbackForTesting(std::move(callback2));
// Recover from reload errors, by replacing invalid policy in file with a
// valid policy.
authz_policy =
Expand Down Expand Up @@ -704,13 +751,16 @@ static void test_file_watcher_recovers_from_failure(
" ]"
"}";
tmp_policy.RewriteFile(authz_policy);
wait_for_policy_reload();
GPR_ASSERT(reinterpret_cast<void*>(1) ==
gpr_event_wait(&on_second_reload_done,
gpr_inf_future(GPR_CLOCK_MONOTONIC)));
test_deny_unauthorized_request(f);
dynamic_cast<grpc_core::FileWatcherAuthorizationPolicyProvider*>(provider)
->SetCallbackForTesting(nullptr);

end_test(&f);
config.tear_down_data(&f);
}
#endif

void grpc_authz(grpc_end2end_test_config config) {
test_static_init_allow_authorized_request(config);
Expand All @@ -721,10 +771,7 @@ void grpc_authz(grpc_end2end_test_config config) {
test_file_watcher_init_deny_request_no_match_in_policy(config);
test_file_watcher_valid_policy_reload(config);
test_file_watcher_invalid_policy_skip_reload(config);
#ifndef GPR_APPLE // test case highly flaky on Mac
// TODO(jtattermusch): reenable the test once b/204329811 is fixed.
test_file_watcher_recovers_from_failure(config);
#endif
}

void grpc_authz_pre_init(void) {}
76 changes: 68 additions & 8 deletions test/core/security/grpc_authorization_policy_provider_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,23 @@ TEST(AuthorizationPolicyProviderTest, FileWatcherSuccessValidPolicyRefresh) {
ASSERT_NE(deny_engine, nullptr);
EXPECT_EQ(deny_engine->action(), Rbac::Action::kDeny);
EXPECT_EQ(deny_engine->num_policies(), 1);
gpr_event on_reload_done;
gpr_event_init(&on_reload_done);
std::function<void(bool contents_changed, absl::Status status)> callback =
[&on_reload_done](bool contents_changed, absl::Status status) {
if (contents_changed) {
EXPECT_TRUE(status.ok());
gpr_event_set(&on_reload_done, reinterpret_cast<void*>(1));
}
};
dynamic_cast<FileWatcherAuthorizationPolicyProvider*>(provider->get())
->SetCallbackForTesting(std::move(callback));
// Rewrite the file with a different valid authorization policy.
tmp_authz_policy->RewriteFile(testing::GetFileContents(VALID_POLICY_PATH_2));
// Wait 2 seconds for the provider's refresh thread to read the updated files.
gpr_sleep_until(grpc_timeout_seconds_to_deadline(2));
// Wait for the provider's refresh thread to read the updated files.
ASSERT_EQ(
gpr_event_wait(&on_reload_done, gpr_inf_future(GPR_CLOCK_MONOTONIC)),
reinterpret_cast<void*>(1));
engines = (*provider)->engines();
allow_engine =
dynamic_cast<GrpcAuthorizationEngine*>(engines.allow_engine.get());
Expand All @@ -121,6 +134,8 @@ TEST(AuthorizationPolicyProviderTest, FileWatcherSuccessValidPolicyRefresh) {
ASSERT_NE(deny_engine, nullptr);
EXPECT_EQ(deny_engine->action(), Rbac::Action::kDeny);
EXPECT_EQ(deny_engine->num_policies(), 0);
dynamic_cast<FileWatcherAuthorizationPolicyProvider*>(provider->get())
->SetCallbackForTesting(nullptr);
}

TEST(AuthorizationPolicyProviderTest,
Expand All @@ -141,10 +156,24 @@ TEST(AuthorizationPolicyProviderTest,
ASSERT_NE(deny_engine, nullptr);
EXPECT_EQ(deny_engine->action(), Rbac::Action::kDeny);
EXPECT_EQ(deny_engine->num_policies(), 1);
gpr_event on_reload_done;
gpr_event_init(&on_reload_done);
std::function<void(bool contents_changed, absl::Status status)> callback =
[&on_reload_done](bool contents_changed, absl::Status status) {
if (contents_changed) {
EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument);
EXPECT_EQ(status.message(), "\"name\" field is not present.");
gpr_event_set(&on_reload_done, reinterpret_cast<void*>(1));
}
};
dynamic_cast<FileWatcherAuthorizationPolicyProvider*>(provider->get())
->SetCallbackForTesting(std::move(callback));
// Skips the following policy update, and continues to use the valid policy.
tmp_authz_policy->RewriteFile(testing::GetFileContents(INVALID_POLICY_PATH));
// Wait 2 seconds for the provider's refresh thread to read the updated files.
gpr_sleep_until(grpc_timeout_seconds_to_deadline(2));
// Wait for the provider's refresh thread to read the updated files.
ASSERT_EQ(
gpr_event_wait(&on_reload_done, gpr_inf_future(GPR_CLOCK_MONOTONIC)),
reinterpret_cast<void*>(1));
engines = (*provider)->engines();
allow_engine =
dynamic_cast<GrpcAuthorizationEngine*>(engines.allow_engine.get());
Expand All @@ -156,6 +185,8 @@ TEST(AuthorizationPolicyProviderTest,
ASSERT_NE(deny_engine, nullptr);
EXPECT_EQ(deny_engine->action(), Rbac::Action::kDeny);
EXPECT_EQ(deny_engine->num_policies(), 1);
dynamic_cast<FileWatcherAuthorizationPolicyProvider*>(provider->get())
->SetCallbackForTesting(nullptr);
}

TEST(AuthorizationPolicyProviderTest, FileWatcherRecoversFromFailure) {
Expand All @@ -175,10 +206,24 @@ TEST(AuthorizationPolicyProviderTest, FileWatcherRecoversFromFailure) {
ASSERT_NE(deny_engine, nullptr);
EXPECT_EQ(deny_engine->action(), Rbac::Action::kDeny);
EXPECT_EQ(deny_engine->num_policies(), 1);
gpr_event on_first_reload_done;
gpr_event_init(&on_first_reload_done);
std::function<void(bool contents_changed, absl::Status status)> callback1 =
[&on_first_reload_done](bool contents_changed, absl::Status status) {
if (contents_changed) {
EXPECT_EQ(status.code(), absl::StatusCode::kInvalidArgument);
EXPECT_EQ(status.message(), "\"name\" field is not present.");
gpr_event_set(&on_first_reload_done, reinterpret_cast<void*>(1));
}
};
dynamic_cast<FileWatcherAuthorizationPolicyProvider*>(provider->get())
->SetCallbackForTesting(std::move(callback1));
// Skips the following policy update, and continues to use the valid policy.
tmp_authz_policy->RewriteFile(testing::GetFileContents(INVALID_POLICY_PATH));
// Wait 2 seconds for the provider's refresh thread to read the updated files.
gpr_sleep_until(grpc_timeout_seconds_to_deadline(2));
// Wait for the provider's refresh thread to read the updated files.
ASSERT_EQ(gpr_event_wait(&on_first_reload_done,
gpr_inf_future(GPR_CLOCK_MONOTONIC)),
reinterpret_cast<void*>(1));
engines = (*provider)->engines();
allow_engine =
dynamic_cast<GrpcAuthorizationEngine*>(engines.allow_engine.get());
Expand All @@ -190,10 +235,23 @@ TEST(AuthorizationPolicyProviderTest, FileWatcherRecoversFromFailure) {
ASSERT_NE(deny_engine, nullptr);
EXPECT_EQ(deny_engine->action(), Rbac::Action::kDeny);
EXPECT_EQ(deny_engine->num_policies(), 1);
gpr_event on_second_reload_done;
gpr_event_init(&on_second_reload_done);
std::function<void(bool contents_changed, absl::Status status)> callback2 =
[&on_second_reload_done](bool contents_changed, absl::Status status) {
if (contents_changed) {
EXPECT_TRUE(status.ok());
gpr_event_set(&on_second_reload_done, reinterpret_cast<void*>(1));
}
};
dynamic_cast<FileWatcherAuthorizationPolicyProvider*>(provider->get())
->SetCallbackForTesting(std::move(callback2));
// Rewrite the file with a valid authorization policy.
tmp_authz_policy->RewriteFile(testing::GetFileContents(VALID_POLICY_PATH_2));
// Wait 2 seconds for the provider's refresh thread to read the updated files.
gpr_sleep_until(grpc_timeout_seconds_to_deadline(2));
// Wait for the provider's refresh thread to read the updated files.
ASSERT_EQ(gpr_event_wait(&on_second_reload_done,
gpr_inf_future(GPR_CLOCK_MONOTONIC)),
reinterpret_cast<void*>(1));
engines = (*provider)->engines();
allow_engine =
dynamic_cast<GrpcAuthorizationEngine*>(engines.allow_engine.get());
Expand All @@ -205,6 +263,8 @@ TEST(AuthorizationPolicyProviderTest, FileWatcherRecoversFromFailure) {
ASSERT_NE(deny_engine, nullptr);
EXPECT_EQ(deny_engine->action(), Rbac::Action::kDeny);
EXPECT_EQ(deny_engine->num_policies(), 0);
dynamic_cast<FileWatcherAuthorizationPolicyProvider*>(provider->get())
->SetCallbackForTesting(nullptr);
}

} // namespace grpc_core
Expand Down
Loading

0 comments on commit 9bc16ed

Please sign in to comment.