Skip to content

Commit

Permalink
Handle kubernetes style config directories (neo4j#7363)
Browse files Browse the repository at this point in the history 
* Handle kubernetes style config directories

* File permissions tests for config directories and addresses other feedback

* ensure no logs or warnings for valid conf dir

* setting file permissions on a symlink is only permitted on a mac
  • Loading branch information
@eastlondoner
eastlondoner committed Oct 9, 2020
1 parent fc51efc commit 4d687c6
Show file tree
Hide file tree
Showing 2 changed files with 265 additions and 13 deletions.
103 changes: 93 additions & 10 deletions public/community/configuration/src/main/java/org/neo4j/configuration/Config.java
View file
Expand Up @@ -29,12 +29,15 @@
import java.io.InputStream;
import java.io.InputStreamReader;
import java.lang.reflect.Constructor;
import java.nio.file.FileVisitResult;
import java.nio.file.FileVisitor;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.AclEntry;
import java.nio.file.attribute.AclEntryPermission;
import java.nio.file.attribute.AclEntryType;
import java.nio.file.attribute.AclFileAttributeView;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFileAttributes;
import java.nio.file.attribute.PosixFilePermission;
Expand Down Expand Up @@ -228,19 +231,26 @@ private Builder fromFile( Path file, boolean allowThrow )

try
{
try ( InputStream stream = Files.newInputStream( file ) )
if ( Files.isDirectory( file ) )
{
new Properties()
Files.walkFileTree( file, new ConfigDirectoryFileVisitor( file ) );
}
else
{
try ( InputStream stream = Files.newInputStream( file ) )
{
@Override
public synchronized Object put( Object key, Object value )
new Properties()
{
setRaw( key.toString(), value.toString() );
return null;
}
}.load( stream );
@Override
public synchronized Object put( Object key, Object value )
{
setRaw( key.toString(), value.toString() );
return null;
}
}.load( stream );
}
configFiles.add( file );
}
configFiles.add( file );
}
catch ( IOException e )
{
Expand Down Expand Up @@ -390,6 +400,78 @@ else if ( SystemUtils.IS_OS_WINDOWS )
throw new IllegalStateException( "Configuration command expansion not supported for " + SystemUtils.OS_NAME );
}
}

private class ConfigDirectoryFileVisitor implements FileVisitor<Path>
{
private final Path root;

ConfigDirectoryFileVisitor( Path root )
{
this.root = root;
}

private boolean isRoot( Path dir )
{
return root.equals( dir );
}

private boolean isNotHidden( Path file )
{
return !file.getFileName().toString().startsWith( "." );
}

private boolean isFile( Path file, BasicFileAttributes attrs )
{
return attrs.isRegularFile() || Files.isRegularFile( file );
}

@Override
public FileVisitResult preVisitDirectory( Path dir, BasicFileAttributes attrs )
{
if ( isRoot( dir ) )
{
return FileVisitResult.CONTINUE;
}
else
{
// We don't go into subdirectories, it's too risky
if ( isNotHidden( dir ) )
{
log.warn( "Ignoring subdirectory in config directory [" + dir + "]." );
}
return FileVisitResult.SKIP_SUBTREE;
}
}

@Override
public FileVisitResult visitFile( Path file, BasicFileAttributes attrs ) throws IOException
{
if ( isNotHidden( file ) && isFile( file, attrs ) )
{
String key = file.getFileName().toString();
String value = Files.readString( file );
setRaw( key, value );
configFiles.add( file );
}
return FileVisitResult.CONTINUE;
}

@Override
public FileVisitResult visitFileFailed( Path file, IOException exc ) throws IOException
{
throw exc != null ? exc : new IOException( "Unknown failure loading config file [" + file.toAbsolutePath() + "]" );
}

@Override
public FileVisitResult postVisitDirectory( Path dir, IOException exc ) throws IOException
{
if ( exc != null )
{
throw exc;
}
return FileVisitResult.CONTINUE;
}
}
}

public static Config defaults()
Expand Down Expand Up @@ -673,7 +755,6 @@ else if ( overriddenDefaultStrings.containsKey( key ) )
if ( settingValueObjects.containsKey( key ) )
{
value = settingValueObjects.get( key );

}
else if ( settingValueStrings.containsKey( key ) ) // Map value
{
Expand Down Expand Up @@ -933,6 +1014,7 @@ public Setting<Object> getSetting( String name )
}
return (Setting<Object>) settings.get( name ).setting;
}

@SuppressWarnings( "unchecked" )
public Map<String,Setting<Object>> getDeclaredSettings()
{
Expand Down Expand Up @@ -1023,6 +1105,7 @@ public <T> void removeListener( Setting<T> setting, SettingChangeListener<T> lis
private class DepEntry<T> extends Entry<T>
{
private volatile T solved;

private DepEntry( SettingImpl<T> setting, T value, T defaultValue, T solved )
{
super( setting, value, defaultValue, false );
Expand Down
175 changes: 172 additions & 3 deletions public/community/configuration/src/test/java/org/neo4j/configuration/ConfigTest.java
View file
Expand Up @@ -23,6 +23,7 @@
import org.apache.commons.lang3.mutable.MutableInt;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.EnabledOnOs;
import org.junit.jupiter.api.condition.OS;

import java.io.IOException;
Expand All @@ -32,12 +33,15 @@
import java.nio.file.attribute.AclEntryPermission;
import java.nio.file.attribute.AclEntryType;
import java.nio.file.attribute.AclFileAttributeView;
import java.nio.file.attribute.FileAttribute;
import java.nio.file.attribute.PosixFilePermissions;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Stream;

import org.neo4j.configuration.connectors.BoltConnector;
import org.neo4j.configuration.connectors.HttpConnector;
Expand All @@ -52,8 +56,14 @@
import org.neo4j.test.rule.TestDirectory;
import org.neo4j.util.FeatureToggles;

import static java.nio.file.attribute.PosixFilePermission.GROUP_READ;
import static java.nio.file.attribute.PosixFilePermission.GROUP_WRITE;
import static java.nio.file.attribute.PosixFilePermission.OTHERS_READ;
import static java.nio.file.attribute.PosixFilePermission.OTHERS_WRITE;
import static java.nio.file.attribute.PosixFilePermission.OWNER_EXECUTE;
import static java.nio.file.attribute.PosixFilePermission.OWNER_READ;
import static java.nio.file.attribute.PosixFilePermission.OWNER_WRITE;
import static org.apache.commons.lang3.SystemUtils.IS_OS_MAC;
import static org.apache.commons.lang3.SystemUtils.IS_OS_WINDOWS;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -421,10 +431,12 @@ void testResolveBrokenSettingDependency()
private static final class SingleSettingGroup extends GroupSetting
{
final Setting<String> singleSetting = getBuilder( STRING, null ).build();

static SingleSettingGroup group( String name )
{
return new SingleSettingGroup( name );
}

private SingleSettingGroup( String name )
{
super( name );
Expand Down Expand Up @@ -490,9 +502,132 @@ void canReadConfigFile() throws IOException
Path confFile = testDirectory.file( "test.conf" );
Files.write( confFile, Collections.singletonList( GraphDatabaseSettings.default_database.name() + "=foo" ) );

assertEquals( "foo", Config.newBuilder().fromFile( confFile ).build().get( GraphDatabaseSettings.default_database ) );
assertEquals( "foo", Config.newBuilder().fromFileNoThrow( confFile ).build().get( GraphDatabaseSettings.default_database ) );
assertEquals( "foo", Config.newBuilder().fromFileNoThrow( confFile ).build().get( GraphDatabaseSettings.default_database ) );
Config config1 = buildWithoutErrorsOrWarnings( Config.newBuilder().fromFile( confFile )::build );
Config config2 = buildWithoutErrorsOrWarnings( Config.newBuilder().fromFileNoThrow( confFile )::build );
Stream.of( config1, config2 ).forEach(
c -> assertEquals( "foo", c.get( GraphDatabaseSettings.default_database ) )
);
}

@Test
void canReadConfigDir() throws IOException
{
Path confDir = testDirectory.directory( "test.conf" );
Path defaultDatabase = confDir.resolve( GraphDatabaseSettings.default_database.name() );
Files.write( defaultDatabase, "foo".getBytes() );

Config config1 = buildWithoutErrorsOrWarnings( Config.newBuilder().fromFile( confDir )::build );
Config config2 = buildWithoutErrorsOrWarnings( Config.newBuilder().fromFileNoThrow( confDir )::build );
Stream.of( config1, config2 ).forEach(
c -> assertEquals( "foo", c.get( GraphDatabaseSettings.default_database ) )
);
}

@Test
void ignoreSubdirsInConfigDir() throws IOException
{
Path confDir = testDirectory.directory( "test.conf" );
Path subDir = Files.createDirectory( confDir.resolve( "more" ) );

Path defaultDatabase = subDir.resolve( GraphDatabaseSettings.default_database.name() );
Files.write( defaultDatabase, "foo".getBytes() );

Config config1 = Config.newBuilder().fromFile( confDir ).build();
Config config2 = Config.newBuilder().fromFileNoThrow( confDir ).build();

Stream.of( config1, config2 )
.forEach( c ->
{
AssertableLogProvider logProvider = new AssertableLogProvider();
c.setLogger( logProvider.getLog( Config.class ) );
assertThat( logProvider ).forLevel( AssertableLogProvider.Level.WARN )
.containsMessages( "Ignoring subdirectory in config directory [" + subDir + "]." );
assertThat( logProvider ).forLevel( AssertableLogProvider.Level.ERROR ).doesNotHaveAnyLogs();

assertThat( c.get( GraphDatabaseSettings.default_database ) ).isNotEqualTo( "foo" );
}
);
}

@Test
void canReadK8sStyleConfigDir() throws IOException
{
Path confDir = createK8sStyleConfigDir();

Config config = buildWithoutErrorsOrWarnings( Config.newBuilder().fromFile( confDir )::build );
Config config2 = buildWithoutErrorsOrWarnings( Config.newBuilder().fromFileNoThrow( confDir )::build );

Stream.of( config, config2 ).forEach( c ->
{
assertEquals( "foo", c.get( GraphDatabaseSettings.default_database ) );
assertEquals( true, c.get( GraphDatabaseSettings.auth_enabled ) );
assertEquals( 4, c.get( GraphDatabaseSettings.auth_max_failed_attempts ) );
} );
}

/**
* This tests some of the unusual arrangements with links and metadata files/directories that can exist in Kubernetes mounted volumes
*
* @param fileAttributes
* @throws IOException
*/
private Path createK8sStyleConfigDir( FileAttribute<?>... fileAttributes ) throws IOException
{
// Create and populate a directory for files and directories that we will target using links
Path targetDir = testDirectory.directory( "links" );

Path dotFile = Files.createFile( targetDir.resolve( "..data" ) );
Path dotDir = Files.createDirectory( targetDir.resolve( "..metadata" ) );

Path defaultDatabase = targetDir.resolve( GraphDatabaseSettings.default_database.name() );
Files.createFile( defaultDatabase, fileAttributes );
Files.write( defaultDatabase, "foo".getBytes() );

Path authEnabled = targetDir.resolve( GraphDatabaseSettings.auth_enabled.name() );
Files.createFile( authEnabled, fileAttributes );
Files.write( authEnabled, "true".getBytes() );

// Create and populate the actual conf dir
Path confDir = testDirectory.directory( "neo4j.conf" );

// -- Set up all the links --
// Setting file permissions on a symlink is only supported on Mac
FileAttribute<?>[] symlinkAttributes = IS_OS_MAC ? new FileAttribute[]{
PosixFilePermissions.asFileAttribute( Set.of( OWNER_EXECUTE, OWNER_READ, OWNER_WRITE, GROUP_READ, GROUP_WRITE, OTHERS_READ, OTHERS_WRITE ) )}
: new FileAttribute[0];
// Symbolic link to a dot file
Files.createSymbolicLink( confDir.resolve( dotFile.getFileName() ), dotFile, symlinkAttributes );
// Symbolic link to a dot directory
Files.createSymbolicLink( confDir.resolve( dotDir.getFileName() ), dotDir, symlinkAttributes );
// Symbolic link to an actual setting file we want read
Files.createSymbolicLink( confDir.resolve( defaultDatabase.getFileName() ), defaultDatabase, symlinkAttributes );
// Hard link to an actual setting file we want read
Files.createLink( confDir.resolve( authEnabled.getFileName() ), authEnabled );

// -- Set up regular files/dirs in the conf dir --
// A dot file (this one doesn't show up on K8s, but better safe than sorry)
Files.createFile( confDir.resolve( ".DS_STORE" ) );
// A dot dir (this one doesn't show up on K8s, but better safe than sorry)
Files.createDirectory( confDir.resolve( "..version" ) );
// An actual settings file we want to read
Path authMaxFailedAttempts = confDir.resolve( GraphDatabaseSettings.auth_max_failed_attempts.name() );
Files.createFile( authMaxFailedAttempts, fileAttributes );
Files.write( authMaxFailedAttempts, "4".getBytes() );
return confDir;
}

private Config buildWithoutErrorsOrWarnings( Supplier<Config> buildConfig )
{
AssertableLogProvider lp = new AssertableLogProvider();

Config config = buildConfig.get();

// The config uses a buffering log, when you supply it with a log (i.e. our mock) it replays the buffered log into it
config.setLogger( lp.getLog( Config.class ) );
assertThat( lp ).forLevel( AssertableLogProvider.Level.WARN ).doesNotHaveAnyLogs();
assertThat( lp ).forLevel( AssertableLogProvider.Level.ERROR ).doesNotHaveAnyLogs();

return config;
}

@Test
Expand Down Expand Up @@ -794,6 +929,39 @@ void shouldNotEvaluateWithIncorrectFilePermission() throws IOException
assertThat( msg ).contains( expectedErrorMessage );
}

@Test
@EnabledOnOs( {OS.LINUX, OS.MAC} )
void shouldNotEvaluateK8sConfDirWithIncorrectFilePermission() throws IOException
{
//Given
Path confDir = createK8sStyleConfigDir( PosixFilePermissions.asFileAttribute( PosixFilePermissions.fromString( "rwx------" ) ) );
Config.Builder builder = Config.newBuilder().allowCommandExpansion().addSettingsClass( TestSettings.class ).fromFile( confDir );

//Then
String msg = assertThrows( IllegalArgumentException.class, builder::build ).getMessage();
String expectedErrorMessage = "does not have the correct file permissions";
assertThat( msg ).contains( expectedErrorMessage );
}

@Test
@EnabledOnOs( {OS.LINUX, OS.MAC} )
void shouldEvaluateK8sConfDirWithCorrectFilePermission() throws IOException
{
var acceptablePermissions = PosixFilePermissions.asFileAttribute( Set.of( OWNER_READ, OWNER_WRITE ) );

// Given
Path confDir = createK8sStyleConfigDir( acceptablePermissions );

var testSetting = Files.createFile( confDir.resolve( TestSettings.intSetting.name() ), acceptablePermissions );
Files.write( testSetting, "$(expr 3 + 3)".getBytes() );

Config.Builder configBuilder = Config.newBuilder().allowCommandExpansion().addSettingsClass( TestSettings.class ).fromFile( confDir );

//Then
Config config = buildWithoutErrorsOrWarnings( configBuilder::build );
assertEquals( 6, config.get( TestSettings.intSetting ) );
}

@Test
void shouldTimeoutOnSlowCommands()
{
Expand Down Expand Up @@ -897,6 +1065,7 @@ public static TestConnectionGroupSetting group( String name )
{
return new TestConnectionGroupSetting( name );
}

@Override
public String getPrefix()
{
Expand Down

0 comments on commit 4d687c6

Please sign in to comment.