Address review comments from markdroth.

include/grpcpp/security/tls_certificate_provider.h

@@ -40,11 +40,6 @@ class GRPCXX_DLL CertificateProviderInterface {
  public:
   virtual ~CertificateProviderInterface() = default;
   virtual grpc_tls_certificate_provider* c_provider() = 0;
-
-  // Returns an OK status if the credentials held by the provider are valid.
-  // What it means for a credential to be valid is determined by the provider
-  // implementation.
-  virtual absl::Status ValidateCredentials() const { return absl::OkStatus(); }
 };
 
 // A struct that stores the credential data presented to the peer in handshake
@@ -80,7 +75,7 @@ class GRPCXX_DLL StaticDataCertificateProvider
   // - the root certificates consist of one or more valid PEM blocks, and
   // - every identity key-cert pair has a certificate chain that consists of
   //   valid PEM blocks and has a private key is a valid PEM block.
-  absl::Status ValidateCredentials() const override;
+  absl::Status ValidateCredentials() const;
 
  private:
   grpc_tls_certificate_provider* c_provider_ = nullptr;
@@ -138,7 +133,7 @@ class GRPCXX_DLL FileWatcherCertificateProvider final
   // - every currently-loaded identity key-cert pair, if any, has a certificate
   //   chain that consists of valid PEM blocks and has a private key is a valid
   //   PEM block.
-  absl::Status ValidateCredentials() const override;
+  absl::Status ValidateCredentials() const;
 
  private:
   grpc_tls_certificate_provider* c_provider_ = nullptr;

src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h

@@ -77,11 +77,6 @@ struct grpc_tls_certificate_provider
   // instances of that provider implementation.
   virtual grpc_core::UniqueTypeName type() const = 0;
 
-  // Returns an OK status if the credentials held by the provider are valid.
-  // What it means for a credential to be valid is determined by the provider
-  // implementation.
-  virtual absl::Status ValidateCredentials() const { return absl::OkStatus(); }
-
   static absl::string_view ChannelArgName();
   static int ChannelArgsCompare(const grpc_tls_certificate_provider* a,
                                 const grpc_tls_certificate_provider* b) {
@@ -113,7 +108,7 @@ class StaticDataCertificateProvider final
 
   UniqueTypeName type() const override;
 
-  absl::Status ValidateCredentials() const override;
+  absl::Status ValidateCredentials() const;
 
  private:
   struct WatcherInfo {
@@ -154,7 +149,7 @@ class FileWatcherCertificateProvider final
 
   UniqueTypeName type() const override;
 
-  absl::Status ValidateCredentials() const override;
+  absl::Status ValidateCredentials() const;
 
   int64_t TestOnlyGetRefreshIntervalSecond() const;
 

src/cpp/common/tls_certificate_provider.cc

@@ -47,7 +47,9 @@ StaticDataCertificateProvider::~StaticDataCertificateProvider() {
 };
 
 absl::Status StaticDataCertificateProvider::ValidateCredentials() const {
-  return c_provider_->ValidateCredentials();
+  auto* provider =
+      reinterpret_cast<grpc_core::StaticDataCertificateProvider*>(c_provider_);
+  return provider->ValidateCredentials();
 }
 
 FileWatcherCertificateProvider::FileWatcherCertificateProvider(
@@ -65,7 +67,9 @@ FileWatcherCertificateProvider::~FileWatcherCertificateProvider() {
 };
 
 absl::Status FileWatcherCertificateProvider::ValidateCredentials() const {
-  return c_provider_->ValidateCredentials();
+  auto* provider =
+      reinterpret_cast<grpc_core::FileWatcherCertificateProvider*>(c_provider_);
+  return provider->ValidateCredentials();
 }
 
 }  // namespace experimental

test/cpp/server/credentials_test.cc

@@ -36,7 +36,6 @@
 #define SERVER_KEY_PATH "src/core/tsi/test_creds/server1.key"
 #define CRL_DIR_PATH "test/core/tsi/test_creds/crl_data/crls"
 #define MALFORMED_CERT_PATH "src/core/tsi/test_creds/malformed-cert.pem"
-#define MALFORMED_KEY_PATH "src/core/tsi/test_creds/malformed-key.pem"
 
 namespace {
 
@@ -130,22 +129,6 @@ TEST(CredentialsTest,
   EXPECT_EQ(provider.ValidateCredentials(), absl::OkStatus());
 }
 
-TEST(CredentialsTest,
-     StaticDataCertificateProviderValidationSuccessWithRootOnly) {
-  std::string root_certificates = GetFileContents(CA_CERT_PATH);
-  StaticDataCertificateProvider provider(root_certificates);
-  EXPECT_EQ(provider.ValidateCredentials(), absl::OkStatus());
-}
-
-TEST(CredentialsTest,
-     StaticDataCertificateProviderValidationSuccessWithIdentityOnly) {
-  experimental::IdentityKeyCertPair key_cert_pair;
-  key_cert_pair.private_key = GetFileContents(SERVER_KEY_PATH);
-  key_cert_pair.certificate_chain = GetFileContents(SERVER_CERT_PATH);
-  StaticDataCertificateProvider provider({key_cert_pair});
-  EXPECT_EQ(provider.ValidateCredentials(), absl::OkStatus());
-}
-
 TEST(CredentialsTest, StaticDataCertificateProviderWithMalformedRoot) {
   std::string root_certificates = GetFileContents(MALFORMED_CERT_PATH);
   experimental::IdentityKeyCertPair key_cert_pair;
@@ -156,66 +139,20 @@ TEST(CredentialsTest, StaticDataCertificateProviderWithMalformedRoot) {
             absl::FailedPreconditionError("Invalid PEM."));
 }
 
-TEST(CredentialsTest, StaticDataCertificateProviderWithMalformedIdentityCert) {
-  std::string root_certificates = GetFileContents(CA_CERT_PATH);
-  experimental::IdentityKeyCertPair key_cert_pair;
-  key_cert_pair.private_key = GetFileContents(SERVER_KEY_PATH);
-  key_cert_pair.certificate_chain = GetFileContents(MALFORMED_CERT_PATH);
-  StaticDataCertificateProvider provider(root_certificates, {key_cert_pair});
-  EXPECT_EQ(provider.ValidateCredentials(),
-            absl::FailedPreconditionError("Invalid PEM."));
-}
-
-TEST(CredentialsTest, StaticDataCertificateProviderWithMalformedIdentityKey) {
-  std::string root_certificates = GetFileContents(CA_CERT_PATH);
-  experimental::IdentityKeyCertPair key_cert_pair;
-  key_cert_pair.private_key = GetFileContents(MALFORMED_KEY_PATH);
-  key_cert_pair.certificate_chain = GetFileContents(SERVER_CERT_PATH);
-  StaticDataCertificateProvider provider(root_certificates, {key_cert_pair});
-  EXPECT_EQ(provider.ValidateCredentials(),
-            absl::NotFoundError("No private key found."));
-}
-
 TEST(CredentialsTest,
      FileWatcherCertificateProviderValidationSuccessWithAllCredentials) {
   FileWatcherCertificateProvider provider(SERVER_KEY_PATH, SERVER_CERT_PATH,
                                           CA_CERT_PATH, 1);
   EXPECT_EQ(provider.ValidateCredentials(), absl::OkStatus());
 }
 
-TEST(CredentialsTest,
-     FileWatcherCertificateProviderValidationSuccessWithRootOnly) {
-  FileWatcherCertificateProvider provider(CA_CERT_PATH, 1);
-  EXPECT_EQ(provider.ValidateCredentials(), absl::OkStatus());
-}
-
-TEST(CredentialsTest,
-     FileWatcherCertificateProviderValidationSuccessWithIdentityOnly) {
-  FileWatcherCertificateProvider provider(SERVER_KEY_PATH, SERVER_CERT_PATH, 1);
-  EXPECT_EQ(provider.ValidateCredentials(), absl::OkStatus());
-}
-
 TEST(CredentialsTest, FileWatcherCertificateProviderWithMalformedRoot) {
   FileWatcherCertificateProvider provider(SERVER_KEY_PATH, SERVER_CERT_PATH,
                                           MALFORMED_CERT_PATH, 1);
   EXPECT_EQ(provider.ValidateCredentials(),
             absl::FailedPreconditionError("Invalid PEM."));
 }
 
-TEST(CredentialsTest, FileWatcherCertificateProviderWithMalformedIdentityCert) {
-  FileWatcherCertificateProvider provider(SERVER_KEY_PATH, MALFORMED_CERT_PATH,
-                                          CA_CERT_PATH, 1);
-  EXPECT_EQ(provider.ValidateCredentials(),
-            absl::FailedPreconditionError("Invalid PEM."));
-}
-
-TEST(CredentialsTest, FileWatcherCertificateProviderWithMalformedIdentityKey) {
-  FileWatcherCertificateProvider provider(MALFORMED_KEY_PATH, SERVER_CERT_PATH,
-                                          CA_CERT_PATH, 1);
-  EXPECT_EQ(provider.ValidateCredentials(),
-            absl::NotFoundError("No private key found."));
-}
-
 TEST(CredentialsTest, TlsServerCredentialsWithCrlChecking) {
   auto certificate_provider = std::make_shared<FileWatcherCertificateProvider>(
       SERVER_KEY_PATH, SERVER_CERT_PATH, CA_CERT_PATH, 1);