sarankk commented on code in PR #276:
URL: https://github.com/apache/cassandra-sidecar/pull/276#discussion_r2535762978
##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/InvalidateCacheHandler.java:
##########
@@ -78,34 +80,85 @@ public Set<Authorization> requiredAuthorizations()
}
@Override
- protected String extractParamsOrThrow(RoutingContext context)
+ protected Params extractParamsOrThrow(RoutingContext context)
{
- return context.pathParam("cacheName");
+ String cacheName = context.pathParam("cacheName");
+ List<String> keys = context.queryParam("keys");
+ return new Params(cacheName, keys);
+ }
+
+ /**
+ * Simple holder class for cache invalidation parameters
+ */
+ protected static class Params
+ {
+ final String cacheName;
+ final List<String> keys;
+
+ Params(String cacheName, List<String> keys)
+ {
+ this.cacheName = cacheName;
+ this.keys = keys;
+ }
+
+ boolean invalidateAll()
+ {
+ return keys == null || keys.isEmpty();
+ }
}
@Override
protected void handleInternal(RoutingContext context,
HttpServerRequest httpRequest,
@NotNull String host,
SocketAddress remoteAddress,
- String cacheName)
+ Params params)
{
+ String cacheName = params.cacheName;
+ boolean invalidateAll = params.invalidateAll();
+
switch (cacheName.toLowerCase())
{
case "identity_to_role_cache":
Review Comment:
Can you reuse cache `NAME` here
https://github.com/apache/cassandra-sidecar/blob/f41b3ca8cc8b3f89fb5f5fa5ba760c42fd4df8e0/server/src/main/java/org/apache/cassandra/sidecar/acl/IdentityToRoleCache.java#L37.
Also can you add static constants for names without underscore? incase we want
to reuse name.
##########
integration-tests/src/integrationTest/org/apache/cassandra/sidecar/routes/InvalidateCacheIntegrationTest.java:
##########
@@ -344,11 +368,12 @@ private void createTestRole(Session session)
session.execute("CREATE ROLE IF NOT EXISTS \"test_role\" WITH
SUPERUSER = false AND LOGIN = true");
session.execute(String.format("ADD IDENTITY IF NOT EXISTS '%s' TO ROLE
'test_role'", TEST_USER_IDENTITY));
- // Create a test superuser role for testing SuperUserCache
Review Comment:
Nit: Why are we removing this comment?
##########
server/src/main/java/org/apache/cassandra/sidecar/handlers/InvalidateCacheHandler.java:
##########
@@ -78,34 +80,85 @@ public Set<Authorization> requiredAuthorizations()
}
@Override
- protected String extractParamsOrThrow(RoutingContext context)
+ protected Params extractParamsOrThrow(RoutingContext context)
{
- return context.pathParam("cacheName");
+ String cacheName = context.pathParam("cacheName");
+ List<String> keys = context.queryParam("keys");
+ return new Params(cacheName, keys);
+ }
+
+ /**
+ * Simple holder class for cache invalidation parameters
+ */
+ protected static class Params
+ {
+ final String cacheName;
+ final List<String> keys;
+
+ Params(String cacheName, List<String> keys)
+ {
+ this.cacheName = cacheName;
+ this.keys = keys;
+ }
+
+ boolean invalidateAll()
+ {
+ return keys == null || keys.isEmpty();
+ }
}
@Override
protected void handleInternal(RoutingContext context,
HttpServerRequest httpRequest,
@NotNull String host,
SocketAddress remoteAddress,
- String cacheName)
+ Params params)
{
+ String cacheName = params.cacheName;
+ boolean invalidateAll = params.invalidateAll();
+
switch (cacheName.toLowerCase())
{
case "identity_to_role_cache":
case "identitytorolecache":
- identityToRoleCache.invalidateAll();
- break;
- case "role_permissions_cache":
- case "roleauthorizationscache":
- roleAuthorizationsCache.invalidateAll();
+ if (invalidateAll)
+ {
+ identityToRoleCache.invalidateAll();
+ }
+ else
+ {
+ identityToRoleCache.invalidateAll(params.keys);
+ }
break;
case "super_user_cache":
case "superusercache":
- superUserCache.invalidateAll();
+ if (invalidateAll)
+ {
+ superUserCache.invalidateAll();
+ }
+ else
+ {
+ superUserCache.invalidateAll(params.keys);
+ }
+ break;
+ case "role_authorization_cache":
Review Comment:
Nit: `RoleAuthorizationsCache` missing plural in cache name.
##########
integration-tests/src/integrationTest/org/apache/cassandra/sidecar/routes/InvalidateCacheIntegrationTest.java:
##########
@@ -144,7 +154,7 @@ protected Function<SidecarConfigurationImpl.Builder,
SidecarConfigurationImpl.Bu
Map.of());
CacheConfiguration permissionCacheConfiguration =
CacheConfigurationImpl.builder()
-
.expireAfterAccess(MillisecondBoundConfiguration.parse("5s"))
+
.expireAfterAccess(MillisecondBoundConfiguration.parse("5m"))
Review Comment:
Let's revert time change? tests do not seem to require this.
##########
integration-tests/src/integrationTest/org/apache/cassandra/sidecar/routes/InvalidateCacheIntegrationTest.java:
##########
@@ -0,0 +1,625 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.sidecar.routes;
+
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Function;
+
+import org.junit.jupiter.api.Test;
+
+import com.datastax.driver.core.SSLOptions;
+import com.datastax.driver.core.Session;
+import com.github.benmanes.caffeine.cache.AsyncCache;
+import com.github.benmanes.caffeine.cache.stats.CacheStats;
+import com.google.inject.AbstractModule;
+import com.google.inject.Provides;
+import com.google.inject.Singleton;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.vertx.core.buffer.Buffer;
+import io.vertx.core.http.HttpMethod;
+import io.vertx.ext.web.client.HttpResponse;
+import io.vertx.ext.web.client.WebClient;
+import org.apache.cassandra.distributed.api.ICluster;
+import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.sidecar.acl.IdentityToRoleCache;
+import org.apache.cassandra.sidecar.acl.authorization.AuthorizationCacheKey;
+import org.apache.cassandra.sidecar.acl.authorization.RoleAuthorizationsCache;
+import org.apache.cassandra.sidecar.acl.authorization.SuperUserCache;
+import org.apache.cassandra.sidecar.common.server.CQLSessionProvider;
+import
org.apache.cassandra.sidecar.common.server.utils.MillisecondBoundConfiguration;
+import
org.apache.cassandra.sidecar.common.server.utils.SecondBoundConfiguration;
+import org.apache.cassandra.sidecar.config.AccessControlConfiguration;
+import org.apache.cassandra.sidecar.config.CacheConfiguration;
+import org.apache.cassandra.sidecar.config.KeyStoreConfiguration;
+import org.apache.cassandra.sidecar.config.ParameterizedClassConfiguration;
+import org.apache.cassandra.sidecar.config.yaml.AccessControlConfigurationImpl;
+import org.apache.cassandra.sidecar.config.yaml.CacheConfigurationImpl;
+import org.apache.cassandra.sidecar.config.yaml.KeyStoreConfigurationImpl;
+import
org.apache.cassandra.sidecar.config.yaml.ParameterizedClassConfigurationImpl;
+import org.apache.cassandra.sidecar.config.yaml.SidecarConfigurationImpl;
+import org.apache.cassandra.sidecar.config.yaml.SslConfigurationImpl;
+import org.apache.cassandra.sidecar.testing.MtlsTestHelper;
+import
org.apache.cassandra.sidecar.testing.SharedClusterSidecarIntegrationTestBase;
+import org.apache.cassandra.sidecar.testing.TemporaryCqlSessionProvider;
+import org.apache.cassandra.sidecar.utils.CacheFactory;
+import org.apache.cassandra.sidecar.utils.SimpleCassandraVersion;
+import org.apache.cassandra.testing.ClusterBuilderConfiguration;
+
+import static org.apache.cassandra.testing.DriverTestUtils.buildContactPoints;
+import static org.apache.cassandra.testing.TestUtils.DC1_RF1;
+import static org.apache.cassandra.testing.TlsTestUtils.getSSLOptions;
+import static
org.apache.cassandra.testing.TlsTestUtils.withAuthenticatedSession;
+import static org.apache.cassandra.testing.utils.AssertionUtils.getBlocking;
+import static org.apache.cassandra.testing.utils.AssertionUtils.loopAssert;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assumptions.assumeThat;
+
+/**
+ * Integration test for cache invalidation endpoints
+ */
+class InvalidateCacheIntegrationTest extends
SharedClusterSidecarIntegrationTestBase
+{
+ protected static final int MIN_VERSION_WITH_MTLS = 5;
+ private static final String CASSANDRA_IDENTITY =
"spiffe://cassandra/sidecar/cassandra_role";
+ private static final String SIDECAR_ROLE_IDENTITY =
"spiffe://cassandra/sidecar/sidecar_role";
+ private static final String TEST_USER_IDENTITY =
"spiffe://cassandra/sidecar/test_user";
+ private static final String TEST_SUPERUSER_IDENTITY =
"spiffe://cassandra/sidecar/test_superuser";
+ private static final String TEST_SUPERUSER2_IDENTITY =
"spiffe://cassandra/sidecar/test_superuser2";
+ private static final String SCHEMA_ROUTE = "/api/v1/cassandra/schema";
+ private static final String CACHE_INVALIDATE_ROUTE_TEMPLATE =
"/api/v1/caches/%s/invalidate";
+
+ // Cache name constants
+ private static final String IDENTITY_TO_ROLE_CACHE =
"identity_to_role_cache";
+ private static final String ROLE_AUTHORIZATION_CACHE =
"role_authorization_cache";
+ private static final String SUPER_USER_CACHE = "super_user_cache";
+ private static final String ENDPOINT_AUTHORIZATION_CACHE =
"endpoint_authorization_cache";
+
+ private Path testUserKeystorePath;
+ private Path superuserKeystorePath;
+ private Path superuser2KeystorePath;
+
+ @Override
+ protected void beforeClusterProvisioning()
+ {
+ // mTLS authentication was added in Cassandra starting 5.0 version
+ assumeThat(SimpleCassandraVersion.create(testVersion.version()).major)
+ .as("mTLS authentication is not supported in 4.0 and 4.1 Cassandra
versions")
+ .isGreaterThanOrEqualTo(MIN_VERSION_WITH_MTLS);
+ }
+
+ @Override
+ protected ClusterBuilderConfiguration testClusterConfiguration()
+ {
+ return super.testClusterConfiguration()
+ .additionalInstanceConfig(Map.of("authenticator",
"org.apache.cassandra.auth.PasswordAuthenticator"));
+ }
+
+ @Override
+ protected void afterClusterProvisioned()
+ {
+ IInstance instance = cluster.getFirstRunningInstance();
+ configureAdminAndSidecarIdentity(instance);
+
+ cluster.stopUnchecked(instance);
+
+ var instanceConfig = instance.config();
+ instanceConfig.set("authenticator.class_name",
"org.apache.cassandra.auth.MutualTlsAuthenticator");
+ instanceConfig.set("authenticator.parameters",
Map.of("validator_class_name",
+
"org.apache.cassandra.auth.SpiffeCertificateValidator"));
+ instanceConfig.set("role_manager", "CassandraRoleManager");
+ instanceConfig.set("authorizer", "CassandraAuthorizer");
+ instanceConfig.set("client_encryption_options.enabled", "true");
+ instanceConfig.set("client_encryption_options.optional", "true");
+ instanceConfig.set("client_encryption_options.require_client_auth",
"true");
+
instanceConfig.set("client_encryption_options.require_endpoint_verification",
"false");
+ instanceConfig.set("client_encryption_options.keystore",
mtlsTestHelper.serverKeyStorePath());
+ instanceConfig.set("client_encryption_options.keystore_password",
mtlsTestHelper.serverKeyStorePassword());
+ instanceConfig.set("client_encryption_options.truststore",
mtlsTestHelper.trustStorePath());
+ instanceConfig.set("client_encryption_options.truststore_password",
mtlsTestHelper.trustStorePassword());
+ instanceConfig.set("credentials_update_interval", "50ms");
+
+ instance.startup();
+ }
+
+ @Override
+ protected Function<SidecarConfigurationImpl.Builder,
SidecarConfigurationImpl.Builder> configurationOverrides()
+ {
+ return builder -> {
+ Map<String, String> params = Map.of("certificate_validator",
"io.vertx.ext.auth.mtls.impl.CertificateValidatorImpl",
+
"certificate_identity_extractor",
"org.apache.cassandra.sidecar.acl.authentication.CassandraIdentityExtractor");
+ ParameterizedClassConfiguration mTLSConfig
+ = new
ParameterizedClassConfigurationImpl("org.apache.cassandra.sidecar.acl.authentication.MutualTlsAuthenticationHandlerFactory",
+ params);
+ ParameterizedClassConfiguration rbacConfig
+ = new
ParameterizedClassConfigurationImpl("org.apache.cassandra.sidecar.acl.authorization.RoleBasedAuthorizationProvider",
+ Map.of());
+
+ CacheConfiguration permissionCacheConfiguration =
CacheConfigurationImpl.builder()
+
.expireAfterAccess(MillisecondBoundConfiguration.parse("5m"))
+
.build();
+
+ AccessControlConfiguration accessControlConfiguration =
AccessControlConfigurationImpl.builder()
+
.enabled(true)
+
.authenticatorsConfiguration(List.of(mTLSConfig))
+
.authorizerConfiguration(rbacConfig)
+
.permissionCacheConfiguration(permissionCacheConfiguration)
+
.build();
+
+ KeyStoreConfiguration truststoreConfiguration = new
KeyStoreConfigurationImpl(mtlsTestHelper.trustStorePath(),
+
mtlsTestHelper.trustStorePassword(),
+
mtlsTestHelper.trustStoreType(),
+
SecondBoundConfiguration.parse("1d"));
+
+ KeyStoreConfiguration keyStoreConfiguration = new
KeyStoreConfigurationImpl(mtlsTestHelper.serverKeyStorePath(),
+
mtlsTestHelper.serverKeyStorePassword(),
+
mtlsTestHelper.serverKeyStoreType(),
+
SecondBoundConfiguration.parse("1d"));
+
+ SslConfigurationImpl sslConfiguration =
SslConfigurationImpl.builder()
+
.enabled(true)
+
.clientAuth("REQUEST")
+
.keystore(keyStoreConfiguration)
+
.truststore(truststoreConfiguration)
+
.build();
+
+ return
builder.accessControlConfiguration(accessControlConfiguration)
+ .sslConfiguration(sslConfiguration);
+ };
+ }
+
+ @Override
+ protected void startSidecar(ICluster<? extends IInstance> cluster) throws
InterruptedException
+ {
+ serverWrapper = startSidecarWithInstances(cluster, new
TestModule(mtlsTestHelper, cluster));
+ }
+
+ @Override
+ protected void beforeTestStart()
+ {
+ waitForSchemaReady(10, TimeUnit.SECONDS);
+ try
+ {
+ testUserKeystorePath =
mtlsTestHelper.issueClientKeyStore(certificateBuilder ->
+
certificateBuilder.addSanUriName(TEST_USER_IDENTITY));
+ superuserKeystorePath =
mtlsTestHelper.issueClientKeyStore(certificateBuilder ->
+
certificateBuilder.addSanUriName(TEST_SUPERUSER_IDENTITY));
+ superuser2KeystorePath =
mtlsTestHelper.issueClientKeyStore(certificateBuilder ->
+
certificateBuilder.addSanUriName(TEST_SUPERUSER2_IDENTITY));
+ }
+ catch (Exception e)
+ {
+ throw new RuntimeException("Failed to create test keystores", e);
+ }
+ }
+
+ @Override
+ protected void initializeSchemaForTest()
+ {
+ Path clientKeystorePath = cassandraIdentityClientKeyStore();
+
+ SSLOptions sslOptions = getSSLOptions(clientKeystorePath.toString(),
+
mtlsTestHelper.clientKeyStorePassword(),
+ mtlsTestHelper.trustStorePath(),
+
mtlsTestHelper.trustStorePassword());
+ withAuthenticatedSession(cluster.get(1), "cassandra", "cassandra",
session -> {
+ createTestKeyspace(session, "sidecar_internal", DC1_RF1);
+ createRolesPermissionsTable(session);
+ createTestRole(session);
+ }, sslOptions);
+ }
+
+ @Test
+ void testInvalidateIdentityToRoleCache()
+ {
+ IdentityToRoleCache identityToRoleCache =
serverWrapper.injector.getInstance(IdentityToRoleCache.class);
+ verifyFullCacheInvalidation(IDENTITY_TO_ROLE_CACHE,
+ identityToRoleCache::getAll,
+ testUserKeystorePath,
+ TEST_USER_IDENTITY,
+ 1,
+ TEST_SUPERUSER_IDENTITY);
+ }
+
+ @Test
+ void testInvalidateIdentityToRoleCacheWithKeys()
+ {
+ IdentityToRoleCache identityToRoleCache =
serverWrapper.injector.getInstance(IdentityToRoleCache.class);
+ verifySelectiveKeyInvalidation(IDENTITY_TO_ROLE_CACHE,
+ identityToRoleCache::getAll,
+ testUserKeystorePath,
+ superuserKeystorePath,
+ TEST_USER_IDENTITY,
Review Comment:
Can you add multiple keys here for testing
##########
integration-tests/src/integrationTest/org/apache/cassandra/sidecar/routes/InvalidateCacheIntegrationTest.java:
##########
@@ -82,10 +83,19 @@ class InvalidateCacheIntegrationTest extends
SharedClusterSidecarIntegrationTest
private static final String SIDECAR_ROLE_IDENTITY =
"spiffe://cassandra/sidecar/sidecar_role";
private static final String TEST_USER_IDENTITY =
"spiffe://cassandra/sidecar/test_user";
private static final String TEST_SUPERUSER_IDENTITY =
"spiffe://cassandra/sidecar/test_superuser";
+ private static final String TEST_SUPERUSER2_IDENTITY =
"spiffe://cassandra/sidecar/test_superuser2";
private static final String SCHEMA_ROUTE = "/api/v1/cassandra/schema";
+ private static final String CACHE_INVALIDATE_ROUTE_TEMPLATE =
"/api/v1/caches/%s/invalidate";
+
+ // Cache name constants
+ private static final String IDENTITY_TO_ROLE_CACHE =
"identity_to_role_cache";
Review Comment:
Nit: Reuse cache names from respective classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
