Skip to content

Commit

Permalink
Refactor XMLFileResourceProvider to support read-only configuration d…
Browse files Browse the repository at this point in the history
…irectories

Enhance `XMLFileResourceProvider` to allow GeoWebCache to start with a
read-only configuration directory and `geowebcache.xml` file. Previously, the provider
assumed that the configuration directory was always writable, which caused startup
failures when running in environments with restricted file permissions.

- **Support read-only configuration directories**:
  - `hasInput()` now correctly returns `true` if the configuration file
     exists or if a template is available and the directory is writable.
  - `hasOutput()` ensures that the configuration file can be written before
     allowing output operations.
  - Improved error handling when checking directory and file writability.

- **Refactored file existence checks**:
  - Consolidated logic for checking configuration file existence and writability
    into `findConfigFile()`.
  - Moved the directory writability check from `findConfigFile()` to
    `findOrCreateConfFile()` to prevent premature failures.

- **Improved logging and exception handling**:
  - `hasInput()` and `hasOutput()` now log warnings when encountering errors
     instead of failing silently.
  - `findOrCreateConfFile()` now explicitly throws an exception when it cannot
     create a new configuration file due to permission issues.

- GeoWebCache can now start successfully with a pre-existing, read-only configuration directory.
- Configurations stored in a read-only filesystem (e.g., mounted from a
  read-only volume) can still be used without requiring write access.
- The provider only attempts to create a configuration file when explicitly
  required, eliminating side effects from query methods `hasInput()` and
  `hasOutput()`.

This change improves the resilience of GeoWebCache in constrained environments
while maintaining backward compatibility for writable configurations.
  • Loading branch information
groldan authored and github-actions[bot] committed Feb 24, 2025
1 parent 69b9d82 commit 4f995ba
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -788,14 +788,15 @@ private static Node applyTransform(Node oldRootNode, String xslFilename) {

@Override
public void afterPropertiesSet() throws GeoWebCacheException {

if (!resourceProvider.hasInput()) {
throw new ConfigurationException(
String.format("The configuration resource provider is unable to provide a configuration file"));
}
if (gridSetBroker == null) {
throw new IllegalStateException("GridSetBroker has not been set");
}

if (resourceProvider.hasInput()) {
this.setGwcConfig(loadConfiguration());
}
this.setGwcConfig(loadConfiguration());

log.config("Initializing GridSets from " + getIdentifier());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.geowebcache.util.ApplicationContextProvider;
import org.geowebcache.util.GWCVars;
import org.springframework.context.ApplicationContext;
import org.springframework.lang.NonNull;
import org.springframework.web.context.WebApplicationContext;

/** Default implementation of ConfigurationResourceProvider that uses the file system. */
Expand All @@ -49,9 +50,11 @@ public class XMLFileResourceProvider implements ConfigurationResourceProvider {
private final WebApplicationContext context;

/** Location of the configuration file */
@NonNull
private final File configDirectory;

/** Name of the configuration file */
@NonNull
private final String configFileName;

private String templateLocation;
Expand Down Expand Up @@ -135,11 +138,27 @@ public XMLFileResourceProvider(
this(configFileName, appCtx, getConfigDirVar(appCtx), storageDirFinder);
}

/**
* {@inheritDoc}
*
* <p>If the configuration file doesn't exist and {@link #hasInput() == true}, the file will be first created from
* the {@link #setTemplate(String) template}
*
* @throws IOException if the file can't be created or copied from the template
*/
@Override
public InputStream in() throws IOException {
return new FileInputStream(findOrCreateConfFile());
}

/**
* {@inheritDoc}
*
* <p>If the configuration file doesn't exist and {@link #hasOutput() == true}, the file will be first created from
* the {@link #setTemplate(String) template}
*
* @throws IOException if the file can't be created or copied from the template
*/
@Override
public OutputStream out() throws IOException {
return new FileOutputStream(findOrCreateConfFile());
Expand All @@ -165,22 +184,13 @@ public void setTemplate(final String templateLocation) {
}

private File findConfigFile() throws IOException {
if (null == configDirectory) {
throw new IllegalStateException();
}

if (!configDirectory.exists() && !configDirectory.mkdirs()) {
throw new IOException("TileLayerConfiguration directory does not exist and cannot be created: '"
+ configDirectory.getAbsolutePath()
+ "'");
}
if (!configDirectory.canWrite()) {
throw new IOException(
"TileLayerConfiguration directory is not writable: '" + configDirectory.getAbsolutePath() + "'");
}

File xmlFile = new File(configDirectory, configFileName);
return xmlFile;
return new File(configDirectory, configFileName);
}

@Override
Expand All @@ -200,6 +210,11 @@ private File findOrCreateConfFile() throws IOException {
if (xmlFile.exists()) {
log.config("Found configuration file in " + configDirectory.getAbsolutePath());
} else if (templateLocation != null) {
if (!configDirectory.canWrite()) {
throw new IOException("TileLayerConfiguration directory is not writable: '"
+ configDirectory.getAbsolutePath() + "'");
}

log.warning("Found no configuration file in config directory, will create one at '"
+ xmlFile.getAbsolutePath()
+ "' from template "
Expand Down Expand Up @@ -249,17 +264,40 @@ private void backUpConfig(final File xmlFile) throws IOException {
log.fine("Config backup done");
}

/**
* Determines if the config file exists and is readable, or doesn't exist but can be created.
*
* <p>Calling this method has no side effects. The target file either exists, or can be created throught the
* {@link #setTemplate(String) template}, if a template has been set. In such case, it'll be created by either
* {@link #in()} or {@link #out()}.
*/
@Override
public boolean hasInput() {
try {
return findOrCreateConfFile().exists();
File file = findConfigFile();
return file.exists() || (templateLocation != null && configDirectory.canWrite());
} catch (IOException e) {
log.log(Level.WARNING, "Error obtaining config file", e);
return false;
}
}

/**
* Determines if the configuration can be {@link #out() written} to the {@link #getLocation() output file}.
*
* <p>Calling this method has no side effects. The target file may or may not exist. The target directory must be
* writable, and so must the target file in case it does exist.
*
* @return {@code true} if the {@link #getLocation() configuration file} can be written to.
*/
@Override
public boolean hasOutput() {
return true;
try {
File file = findConfigFile();
return configDirectory.canWrite() && (!file.exists() || file.canWrite());
} catch (IOException e) {
log.log(Level.WARNING, "Error obtaining config file", e);
return false;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static org.easymock.EasyMock.replay;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertArrayEquals;
Expand All @@ -27,13 +28,15 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -47,6 +50,7 @@
import java.util.logging.Logger;
import org.apache.commons.io.FileUtils;
import org.geotools.util.logging.Logging;
import org.geowebcache.GeoWebCacheException;
import org.geowebcache.GeoWebCacheExtensions;
import org.geowebcache.MockWepAppContextRule;
import org.geowebcache.config.legends.LegendRawInfo;
Expand All @@ -64,6 +68,7 @@
import org.geowebcache.layer.TileLayer;
import org.geowebcache.layer.wms.WMSLayer;
import org.geowebcache.util.TestUtils;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -557,4 +562,34 @@ public void testWmtsCiteStrictComplianceIsActivated() throws Exception {
// CITE strict compliance should be activated for WMTS
assertThat(config.isWmtsCiteCompliant(), is(true));
}

@Test
public void loadFromReadOnlyDirectory() throws GeoWebCacheException {
Assume.assumeTrue(
"Ignore if setWritable(false) does not succeed, may happen on Windows", configDir.setWritable(false));
assertThat(configFile.exists(), is(true));
try {
config = new XMLConfiguration(null, configDir.getAbsolutePath());
config.setGridSetBroker(gridSetBroker);
config.afterPropertiesSet();
assertThat(config.getLayerCount(), is(greaterThan(0)));
} finally {
configDir.setWritable(true);
}
}

@Test
public void loadFromEmptyReadOnlyDirectoryFails() throws GeoWebCacheException, IOException {
File roEmptyDir = this.temp.newFolder();
Assume.assumeTrue(
"Ignore if setWritable(false) does not succeed, may happen on Windows", roEmptyDir.setWritable(false));
try {
config = new XMLConfiguration(null, roEmptyDir.getAbsolutePath());
config.setGridSetBroker(gridSetBroker);

assertThrows(ConfigurationException.class, () -> config.afterPropertiesSet());
} finally {
roEmptyDir.setWritable(true);
}
}
}

0 comments on commit 4f995ba

Please sign in to comment.