Skip to content

Commit

Permalink
use new 'neo4j dry-run' mechanism to start neo4j process in 4.3 (#295)
Browse files Browse the repository at this point in the history
* use new 'neo4j dry-run' mechanism to start neo4j process in 4.3

do not run  as root

change neo4j dry-run from a command to an argument. Minor tidying of the script

* reduce use of eval

* Revert "temporarily revert tests for command expansion bugs because those bugs are present but tolerated in drop04 (#297)"

This reverts commit 315b333.

* handle errors from --dry-run correctly
  • Loading branch information
eastlondoner authored Jun 2, 2021
1 parent 340f758 commit b3f3a26
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 37 deletions.
27 changes: 21 additions & 6 deletions docker-image-src/4.3/docker-entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -501,15 +501,30 @@ if [ "${cmd}" == "dump-config" ]; then
exit 0
fi

# this prints out a command for us to run.
# the command is something like: `java ...[lots of java options]... neo4j.mainClass ...[some neo4j options]...`
function get_neo4j_run_cmd {

local extraArgs=()

if [ "${EXTENDED_CONF+"yes"}" == "yes" ]; then
extraArgs+=("--expand-commands")
fi

if running_as_root; then
gosu neo4j:neo4j neo4j console --dry-run "${extraArgs[@]}"
else
neo4j console --dry-run "${extraArgs[@]}"
fi
}

# Use su-exec to drop privileges to neo4j user
# Note that su-exec, despite its name, does not replicate the
# functionality of exec, so we need to use both
if [ "${cmd}" == "neo4j" ]; then
if [ "${EXTENDED_CONF+"yes"}" == "yes" ]; then
${exec_cmd} neo4j console --expand-commands
else
${exec_cmd} neo4j console
fi
# separate declaration and use of get_neo4j_run_cmd so that error codes are correctly surfaced
neo4j_console_cmd="$(get_neo4j_run_cmd)"
eval "${exec_cmd} ${neo4j_console_cmd?:No Neo4j command was generated}"
else
${exec_cmd} "$@"
${exec_cmd} "$@"
fi
160 changes: 129 additions & 31 deletions src/test/java/com/neo4j/docker/TestExtendedConf.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,18 @@
import com.neo4j.docker.utils.SetContainerUser;
import com.neo4j.docker.utils.TestSettings;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testcontainers.containers.BindMode;
import org.testcontainers.containers.GenericContainer;
import org.testcontainers.containers.output.OutputFrame;
import org.testcontainers.containers.output.Slf4jLogConsumer;
import org.testcontainers.containers.wait.strategy.LogMessageWaitStrategy;
import org.testcontainers.containers.wait.strategy.Wait;

import java.io.BufferedReader;
Expand All @@ -24,47 +26,72 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.PosixFilePermission;
import java.time.Instant;
import java.util.HashSet;
import java.util.Optional;
import java.util.concurrent.TimeoutException;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class TestExtendedConf
{
private static final Logger log = LoggerFactory.getLogger( TestExtendedConf.class );

@BeforeAll
static void ensureFeaturePresent()
{
Assumptions.assumeTrue( TestSettings.NEO4J_VERSION.isAtLeastVersion( new Neo4jVersion( 4,2,0 ) ),
Assumptions.assumeTrue( TestSettings.NEO4J_VERSION.isAtLeastVersion( new Neo4jVersion( 4,2,1 ) ),
"Extended configuration feature not available before 4.2" );
}

protected GenericContainer createContainer()
protected GenericContainer createContainerNoWait(String password)
{
GenericContainer container = createContainer( password );
// we have to override the default wait strategy with something that will return more quickly
container.setWaitStrategy( new LogMessageWaitStrategy().withRegEx( ".+" ) );
return container;
}

protected GenericContainer createContainer(String password)
{
return new GenericContainer(TestSettings.IMAGE_ID)
.withEnv("NEO4J_AUTH", "none")
.withEnv("NEO4J_ACCEPT_LICENSE_AGREEMENT", "yes")
.withEnv("NEO4J_AUTH", password == null || password.isEmpty() ? "none" : "neo4j/" + password)
.withEnv("NEO4J_ACCEPT_LICENSE_AGREEMENT", "yes")
.withEnv( "EXTENDED_CONF", "yeppers" )
.withExposedPorts(7474, 7687)
.withExposedPorts(7474, 7687)
.waitingFor( Wait.forHttp( "/" ).forPort( 7474 ).forStatusCode( 200 ) )
.withLogConsumer(new Slf4jLogConsumer( log));
.withLogConsumer(new Slf4jLogConsumer( log ));
}


@Test
public void shouldStartWithExtendedConf()
@ParameterizedTest
@ValueSource(strings = {"", "secretN30"})
public void shouldStartWithExtendedConf(String password)
{
try(GenericContainer container = createContainer())
try(GenericContainer container = createContainer(password))
{
container.setWaitStrategy( Wait.forHttp( "/" ).forPort( 7474 ).forStatusCode( 200 ) );
container.start();

Assertions.assertTrue( container.isRunning() );
}
assertPasswordChangedLogIsCorrect( password, container );
}
}

private void assertPasswordChangedLogIsCorrect( String password, GenericContainer container )
{
if ( password.isEmpty()) {
Assertions.assertFalse( container.getLogs( OutputFrame.OutputType.STDOUT).contains( "Changed password for user 'neo4j'." ) );
} else {
Assertions.assertTrue( container.getLogs( OutputFrame.OutputType.STDOUT).contains( "Changed password for user 'neo4j'." ) );
}
}

@Ignore
@Test
void testReadsTheExtendedConfFile_defaultUser() throws Exception
@ParameterizedTest
@ValueSource(strings = {"", "secretN30"})
void testReadsTheExtendedConfFile_defaultUser(String password) throws Exception
{
// set up test folders
Path testOutputFolder = HostFileSystemOperations.createTempFolder( "extendedConfIsRead-" );
Expand All @@ -75,17 +102,79 @@ void testReadsTheExtendedConfFile_defaultUser() throws Exception
Path confFile = Paths.get( "src", "test", "resources", "confs", "ExtendedConf.conf" );
Files.copy( confFile, confFolder.resolve( "neo4j.conf" ) );
setFileOwnerToNeo4j( confFolder.resolve( "neo4j.conf" ) );
chmod600( confFolder.resolve( "neo4j.conf" ) );
chmodConfFilePermissions( confFolder.resolve( "neo4j.conf" ) );

// start container
try(GenericContainer container = createContainer())
try(GenericContainer container = createContainer(password))
{
runContainerAndVerify( container, confFolder, logsFolder, password );
}
}

@ParameterizedTest
@ValueSource( strings = {"", "secretN30"} )
void testInvalidExtendedConfFile_nonRootUser( String password ) throws Exception
{
// set up test folders
Path testOutputFolder = HostFileSystemOperations.createTempFolder( "extendedConfIsRead-" );
Path confFolder = HostFileSystemOperations.createTempFolder( "conf-", testOutputFolder );
Path logsFolder = HostFileSystemOperations.createTempFolder( "logs-", testOutputFolder );

// copy configuration file and set permissions
Path confFile = Paths.get( "src", "test", "resources", "confs", "InvalidExtendedConf.conf" );
Files.copy( confFile, confFolder.resolve( "neo4j.conf" ) );
chmodConfFilePermissions( confFolder.resolve( "neo4j.conf" ) );

try ( GenericContainer container = createContainerNoWait( password ) )
{
SetContainerUser.nonRootUser( container );
container.withFileSystemBind( "/etc/passwd", "/etc/passwd", BindMode.READ_ONLY );
container.withFileSystemBind( "/etc/group", "/etc/group", BindMode.READ_ONLY );
HostFileSystemOperations.mountHostFolderAsVolume( container, confFolder, "/conf" );
container.start();

// expect the container to exit promptly
Instant deadline = Instant.now().plusSeconds( 60 );
while ( container.isRunning() )
{
Thread.sleep( 1000 );
if ( Instant.now().isAfter( deadline ) )
{
throw new TimeoutException( "Timed out waiting for container to exit" );
}
}
Assertions.assertFalse( container.isRunning() );

// An error occurred so we expect a non-zero exit code
Assertions.assertNotEquals( 0, container.getCurrentContainerInfo().getState().getExitCodeLong() );

String logs = container.getLogs();

// check that error messages from neo4j are visible in docker logs
Assertions.assertTrue( logs.contains( "Error evaluating value for setting 'dbms.logs.http.rotation.keep_number'" ) );

// check that error messages from the command that failed are visible in docker logs
Assertions.assertTrue( logs.contains( "this is an error message from inside neo4j config command expansion" ) );

// check that the error is only encountered once (i.e. we quit the docker entrypoint the first time it was encountered)
Assertions.assertEquals( 1, countOccurrences( Pattern.compile( "Error evaluating value for setting" ), logs ) );
}
}

private int countOccurrences( Pattern pattern, String inString )
{
Matcher matcher = pattern.matcher( inString );
int count = 0;
while ( matcher.find() )
{
runContainerAndVerify( container, confFolder, logsFolder );
count = count + 1;
}
return count;
}

@Test
void testReadsTheExtendedConfFile_nonRootUser() throws Exception
@ParameterizedTest
@ValueSource(strings = {"", "secretN30"})
void testReadsTheExtendedConfFile_nonRootUser(String password) throws Exception
{
// set up test folders
Path testOutputFolder = HostFileSystemOperations.createTempFolder( "extendedConfIsRead-" );
Expand All @@ -95,18 +184,18 @@ void testReadsTheExtendedConfFile_nonRootUser() throws Exception
// copy configuration file and set permissions
Path confFile = Paths.get( "src", "test", "resources", "confs", "ExtendedConf.conf" );
Files.copy( confFile, confFolder.resolve( "neo4j.conf" ) );
chmod600( confFolder.resolve( "neo4j.conf" ) );
chmodConfFilePermissions( confFolder.resolve( "neo4j.conf" ) );

try(GenericContainer container = createContainer())
try(GenericContainer container = createContainer(password))
{
SetContainerUser.nonRootUser( container );
container.withFileSystemBind( "/etc/passwd", "/etc/passwd", BindMode.READ_ONLY );
container.withFileSystemBind( "/etc/group", "/etc/group", BindMode.READ_ONLY );
runContainerAndVerify( container, confFolder, logsFolder );
runContainerAndVerify( container, confFolder, logsFolder, password );
}
}

private void runContainerAndVerify(GenericContainer container, Path confFolder, Path logsFolder) throws Exception
private void runContainerAndVerify(GenericContainer container, Path confFolder, Path logsFolder, String password) throws Exception
{
HostFileSystemOperations.mountHostFolderAsVolume( container, confFolder, "/conf" );
HostFileSystemOperations.mountHostFolderAsVolume( container, logsFolder, "/logs" );
Expand All @@ -121,16 +210,25 @@ private void runContainerAndVerify(GenericContainer container, Path confFolder,
Optional<String> isMatch = lines.filter( s -> s.contains("dbms.logs.http.rotation.keep_number=20")).findFirst();
lines.close();
Assertions.assertTrue( isMatch.isPresent(), "dbms.max_databases was not set correctly");

//Check the password was changed if set
assertPasswordChangedLogIsCorrect( password, container );
}

private void chmod600(Path file) throws IOException
private void chmodConfFilePermissions( Path file ) throws IOException
{
Files.setPosixFilePermissions( file,
new HashSet<PosixFilePermission>()
{{
add( PosixFilePermission.OWNER_READ );
add( PosixFilePermission.OWNER_WRITE );
}} );

HashSet<PosixFilePermission> permissions = new HashSet<PosixFilePermission>()
{{
add( PosixFilePermission.OWNER_READ );
add( PosixFilePermission.OWNER_WRITE );
}};

if ( TestSettings.NEO4J_VERSION.isAtLeastVersion( new Neo4jVersion( 4, 3, 0 ) ) )
{
permissions.add( PosixFilePermission.GROUP_READ );
}
Files.setPosixFilePermissions( file, permissions );
}

private void setFileOwnerToNeo4j(Path file) throws Exception
Expand Down
1 change: 1 addition & 0 deletions src/test/resources/confs/InvalidExtendedConf.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
dbms.logs.http.rotation.keep_number=$(bash -c '>&2 echo "this is an error message from inside neo4j config command expansion" && exit 1')

0 comments on commit b3f3a26

Please sign in to comment.