Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import java.util.stream.StreamSupport;

import static org.elasticsearch.entitlement.runtime.policy.Platform.LINUX;
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.BaseDir.CONFIG;
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.BaseDir.DATA;
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.BaseDir.SHARED_REPO;
import static org.elasticsearch.entitlement.runtime.policy.entitlements.FilesEntitlement.Mode.READ;
Expand Down Expand Up @@ -182,7 +183,8 @@ private static PolicyManager createPolicyManager() {
FileData.ofPath(bootstrapArgs.libDir(), READ),
FileData.ofRelativePath(Path.of(""), DATA, READ_WRITE),
FileData.ofRelativePath(Path.of(""), SHARED_REPO, READ_WRITE),

// exclusive settings file
FileData.ofRelativePath(Path.of("operator/settings.json"), CONFIG, READ_WRITE).withExclusive(true),
// OS release on Linux
FileData.ofPath(Path.of("/etc/os-release"), READ).withPlatform(LINUX),
FileData.ofPath(Path.of("/etc/system-release"), READ).withPlatform(LINUX),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@
import org.elasticsearch.core.FixForMultiProject;

import java.io.IOException;
import java.io.InputStream;
import java.nio.file.ClosedWatchServiceException;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.StandardWatchEventKinds;
import java.nio.file.WatchEvent;
import java.nio.file.WatchKey;
import java.nio.file.WatchService;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileTime;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -62,8 +63,9 @@ public abstract class AbstractFileWatchingService extends AbstractLifecycleCompo
private WatchKey settingsDirWatchKey;
private WatchKey configDirWatchKey;

@SuppressWarnings("this-escape")
public AbstractFileWatchingService(Path settingsDir) {
if (Files.exists(settingsDir) && Files.isDirectory(settingsDir) == false) {
if (filesExists(settingsDir) && filesIsDirectory(settingsDir) == false) {
throw new IllegalArgumentException("settingsDir should be a directory");
}
this.settingsDir = settingsDir;
Expand Down Expand Up @@ -113,10 +115,10 @@ public final boolean watching() {
return watcherThread != null;
}

private static FileUpdateState readFileUpdateState(Path path) throws IOException {
private FileUpdateState readFileUpdateState(Path path) throws IOException {
BasicFileAttributes attr;
try {
attr = Files.readAttributes(path, BasicFileAttributes.class);
attr = filesReadAttributes(path, BasicFileAttributes.class);
} catch (NoSuchFileException e) {
// file doesn't exist anymore
return null;
Expand All @@ -141,7 +143,7 @@ final boolean fileChanged(Path path) throws IOException {
}

protected final synchronized void startWatcher() {
if (Files.exists(settingsDir.getParent()) == false) {
if (filesExists(settingsDir.getParent()) == false) {
logger.warn("File watcher for [{}] cannot start because parent directory does not exist", settingsDir);
return;
}
Expand All @@ -155,7 +157,7 @@ protected final synchronized void startWatcher() {
*/
try {
this.watchService = settingsDir.getParent().getFileSystem().newWatchService();
if (Files.exists(settingsDir)) {
if (filesExists(settingsDir)) {
settingsDirWatchKey = enableDirectoryWatcher(settingsDirWatchKey, settingsDir);
} else {
logger.debug("watched directory [{}] not found, will watch for its creation...", settingsDir);
Expand Down Expand Up @@ -188,8 +190,8 @@ protected final void watcherThread() {
try {
logger.info("file settings service up and running [tid={}]", Thread.currentThread().getId());

if (Files.exists(settingsDir)) {
try (Stream<Path> files = Files.list(settingsDir)) {
if (filesExists(settingsDir)) {
try (Stream<Path> files = filesList(settingsDir)) {
var f = files.iterator();
if (f.hasNext() == false) {
// no files in directory
Expand Down Expand Up @@ -248,7 +250,7 @@ protected final void watcherThread() {
}
}
} else if (key == configDirWatchKey) {
if (Files.exists(settingsDir)) {
if (filesExists(settingsDir)) {
// We re-register the settings directory watch key, because we don't know
// if the file name maps to the same native file system file id. Symlinks
// are one potential cause of inconsistency here, since their handling by
Expand All @@ -257,7 +259,7 @@ protected final void watcherThread() {
settingsDirWatchKey = enableDirectoryWatcher(settingsDirWatchKey, settingsDir);

// re-read the settings directory, and ping for any changes
try (Stream<Path> files = Files.list(settingsDir)) {
try (Stream<Path> files = filesList(settingsDir)) {
for (var f = files.iterator(); f.hasNext();) {
Path file = f.next();
if (fileChanged(file)) {
Expand Down Expand Up @@ -370,4 +372,19 @@ long retryDelayMillis(int failedCount) {
* class to determine if a file has been changed.
*/
private record FileUpdateState(long timestamp, String path, Object fileKey) {}

// the following methods are a workaround to ensure exclusive access for files
// required by child watchers; this is required because we only check the caller's module
// not the entire stack
protected abstract boolean filesExists(Path path);

protected abstract boolean filesIsDirectory(Path path);

protected abstract <A extends BasicFileAttributes> A filesReadAttributes(Path path, Class<A> clazz) throws IOException;

protected abstract Stream<Path> filesList(Path dir) throws IOException;

protected abstract Path filesSetLastModifiedTime(Path path, FileTime time) throws IOException;

protected abstract InputStream filesNewInputStream(Path path) throws IOException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.elasticsearch.gateway.GatewayService;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
import java.time.Instant;
Expand All @@ -43,7 +42,7 @@ protected void doStart() {
// We start the file watcher when we know we are master from a cluster state change notification.
// We need the additional active flag, since cluster state can change after we've shutdown the service
// causing the watcher to start again.
this.active = Files.exists(watchedFileDir().getParent());
this.active = filesExists(watchedFileDir().getParent());
if (active == false) {
// we don't have a config directory, we can't possibly launch the file settings service
return;
Expand Down Expand Up @@ -92,10 +91,10 @@ public final void clusterChanged(ClusterChangedEvent event) {
@FixForMultiProject // do we want to re-process everything all at once?
private void refreshExistingFileStateIfNeeded(ClusterState clusterState) {
if (shouldRefreshFileState(clusterState)) {
try (Stream<Path> files = Files.list(watchedFileDir())) {
try (Stream<Path> files = filesList(watchedFileDir())) {
FileTime time = FileTime.from(Instant.now());
for (var it = files.iterator(); it.hasNext();) {
Files.setLastModifiedTime(it.next(), time);
filesSetLastModifiedTime(it.next(), time);
}
} catch (IOException e) {
logger.warn("encountered I/O error trying to update file settings timestamp", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,12 @@
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileTime;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.stream.Stream;

import static org.elasticsearch.health.HealthStatus.GREEN;
import static org.elasticsearch.health.HealthStatus.YELLOW;
Expand Down Expand Up @@ -128,7 +131,7 @@ public void handleSnapshotRestore(ClusterState clusterState, Metadata.Builder md
// since we don't know the current operator configuration, e.g. file settings could be disabled
// on the target cluster. If file settings exist and the cluster state has lost it's reserved
// state for the "file_settings" namespace, we touch our file settings file to cause it to re-process the file.
if (watching() && Files.exists(watchedFile)) {
if (watching() && filesExists(watchedFile)) {
if (fileSettingsMetadata != null) {
ReservedStateMetadata withResetVersion = new ReservedStateMetadata.Builder(fileSettingsMetadata).version(0L).build();
mdBuilder.put(withResetVersion);
Expand Down Expand Up @@ -201,7 +204,7 @@ protected XContentParser createParser(InputStream stream) throws IOException {

private void processFileChanges(ReservedStateVersionCheck versionCheck) throws IOException, InterruptedException, ExecutionException {
PlainActionFuture<Void> completion = new PlainActionFuture<>();
try (var bis = new BufferedInputStream(Files.newInputStream(watchedFile)); var parser = createParser(bis)) {
try (var bis = new BufferedInputStream(filesNewInputStream(watchedFile)); var parser = createParser(bis)) {
stateService.process(NAMESPACE, parser, versionCheck, (e) -> completeProcessing(e, completion));
}
completion.get();
Expand Down Expand Up @@ -343,4 +346,37 @@ public synchronized HealthIndicatorResult calculate(boolean verbose, int maxAffe
}
}
}

// the following methods are a workaround to ensure exclusive access for files
// required by child watchers; this is required because we only check the caller's module
// not the entire stack
@Override
protected boolean filesExists(Path path) {
return Files.exists(path);
}

@Override
protected boolean filesIsDirectory(Path path) {
return Files.isDirectory(path);
}

@Override
protected <A extends BasicFileAttributes> A filesReadAttributes(Path path, Class<A> clazz) throws IOException {
return Files.readAttributes(path, clazz);
}

@Override
protected Stream<Path> filesList(Path dir) throws IOException {
return Files.list(dir);
}

@Override
protected Path filesSetLastModifiedTime(Path path, FileTime time) throws IOException {
return Files.setLastModifiedTime(path, time);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how this one is working right now since we only have read access. IIRC this is an edge case for when we restore from a snapshot. We should (as a followup) re-consider how restoring from snapshots should interact with file settings because this breaks the rule that we want to use, only allowing read access to the config directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

}

@Override
protected InputStream filesNewInputStream(Path path) throws IOException {
return Files.newInputStream(path);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@
import org.junit.Before;

import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardWatchEventKinds;
import java.nio.file.WatchKey;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileTime;
import java.time.Instant;
import java.time.LocalDateTime;
Expand All @@ -40,6 +42,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.stream.Stream;

import static org.elasticsearch.node.Node.NODE_NAME_SETTING;
import static org.elasticsearch.test.hamcrest.OptionalMatchers.isEmpty;
Expand Down Expand Up @@ -81,6 +84,39 @@ protected void processInitialFilesMissing() {
called.accept(null);
}
}

// the following methods are a workaround to ensure exclusive access for files
// required by child watchers; this is required because we only check the caller's module
// not the entire stack
@Override
protected boolean filesExists(Path path) {
return Files.exists(path);
}

@Override
protected boolean filesIsDirectory(Path path) {
return Files.isDirectory(path);
}

@Override
protected <A extends BasicFileAttributes> A filesReadAttributes(Path path, Class<A> clazz) throws IOException {
return Files.readAttributes(path, clazz);
}

@Override
protected Stream<Path> filesList(Path dir) throws IOException {
return Files.list(dir);
}

@Override
protected Path filesSetLastModifiedTime(Path path, FileTime time) throws IOException {
return Files.setLastModifiedTime(path, time);
}

@Override
protected InputStream filesNewInputStream(Path path) throws IOException {
return Files.newInputStream(path);
}
}

private Path watchedFile;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,14 @@
import org.junit.Before;

import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.FileTime;
import java.util.concurrent.ExecutionException;
import java.util.function.Consumer;
import java.util.stream.Stream;

import static org.hamcrest.Matchers.is;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -62,6 +67,39 @@ protected void processFileChanges(Path file) throws InterruptedException, Execut
protected void processInitialFilesMissing() throws InterruptedException, ExecutionException, IOException {
// file always exists, but we don't care about the missing case for master node behavior
}

// the following methods are a workaround to ensure exclusive access for files
// required by child watchers; this is required because we only check the caller's module
// not the entire stack
@Override
protected boolean filesExists(Path path) {
return Files.exists(path);
}

@Override
protected boolean filesIsDirectory(Path path) {
return Files.isDirectory(path);
}

@Override
protected <A extends BasicFileAttributes> A filesReadAttributes(Path path, Class<A> clazz) throws IOException {
return Files.readAttributes(path, clazz);
}

@Override
protected Stream<Path> filesList(Path dir) throws IOException {
return Files.list(dir);
}

@Override
protected Path filesSetLastModifiedTime(Path path, FileTime time) throws IOException {
return Files.setLastModifiedTime(path, time);
}

@Override
protected InputStream filesNewInputStream(Path path) throws IOException {
return Files.newInputStream(path);
}
};
testService.start();
}
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugin/core/src/main/config/log4j2.properties
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,6 @@ logger.samlxml_decrypt.name = org.opensaml.xmlsec.encryption.support.Decrypter
logger.samlxml_decrypt.level = fatal
logger.saml2_decrypt.name = org.opensaml.saml.saml2.encryption.Decrypter
logger.saml2_decrypt.level = fatal

logger.entitlements_xpack_security.name = org.elasticsearch.entitlement.runtime.policy.PolicyManager.x-pack-security.org.elasticsearch.security
logger.entitlements_xpack_security.level = error