[ https://issues.apache.org/jira/browse/CASSANDRA-17772?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17570827#comment-17570827 ]
Andres de la Peña commented on CASSANDRA-17772: ----------------------------------------------- Looks good to me, just dropped a couple of nits on [the commit|https://github.com/josh-mckenzie/cassandra/commit/cc81184461476b558c4440a7cd00f9015f899ca8]. > Improve documentation around query methods in CQLTester and GuardrailTester > --------------------------------------------------------------------------- > > Key: CASSANDRA-17772 > URL: https://issues.apache.org/jira/browse/CASSANDRA-17772 > Project: Cassandra > Issue Type: Improvement > Components: Feature/Guardrails > Reporter: Josh McKenzie > Assignee: Josh McKenzie > Priority: Normal > > Anything that relies on CQLTester.executeFormattedQuery (the > assertInvalidThrowMessage methods for instance) will use internal client > state rather than the client state specified for the query, thus nullifying > any guardrail or systems keyspace permission checks as the > ClientState.isInternal flag overrides all such permission checking. > Reference: > [link|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/service/ClientState.java#L385-L388] > > See chain of CQLTester -> ClientState.isInternal here if interested: > [CQLTester|https://github.com/apache/cassandra/blob/trunk/test/unit/org/apache/cassandra/cql3/CQLTester.java#L1325] > [QueryProcessor|https://github.com/apache/cassandra/blob/8451acc8d8dcfee20d692d1e70ae11b60d2f004e/src/java/org/apache/cassandra/cql3/QueryProcessor.java#L447] > > This is pretty easy to stumble across when testing guardrails as > GuardrailTester adds a variety of assertFails and assertThrows signatures > that _do_ respect the client state and thus apply guardrails > ([example|https://github.com/apache/cassandra/blob/trunk/test/unit/org/apache/cassandra/db/guardrails/GuardrailTester.java#L271]) > > We should add documentation to the method calls in CQLTester and > GuardrailTester to reflect this discrepancy as it can easily trip someone up > writing tests for guardrails. -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org