Skip to content

Commit

Permalink
dcache-core,dcache-xroot,dcache-bulk,dcache-frontend: resolve path pr…
Browse files Browse the repository at this point in the history
…efixes and paths for symlinks

Motivation:

https://rb.dcache.org/r/13937/
master@51594993ee1ac88d8b8ad350788f5153e0665b98

and

https://rb.dcache.org/r/13923/
master@c491f2d312abcbadefcf1df2af85c5620e1e1867

introduced support for relative paths in
the Frontend/Bulk requests and in the
xroot door, respectively.

It did not consider, however, the case when
path prefixes may themselves involve
symlinks.  This could happen either
by a symlink in the configured prefix
from the door or user root, on in the
prefix part of the actual target.

Modification:

Since https://rb.dcache.org/r/13908/
we have the stored procedure and PnfsManager
support for resolving symlinks.  This
patch adds a PnfsMessage and uses the
PnfsHandler to ask for resolution of the
symlinks in the Frontend and xroot doors;
in addition, symlink resolution is done
on the paths of the initial targets
given to the Bulk service when they
are processed.

Result:

The support for both absolute and relative
paths does not fail if there are symlinks
affecting the path prefix.

Target: master
Request 9.0 (partial -- for frontend)
Patch: https://rb.dcache.org/r/13971/
Requires-notes: yes (for 9.0)
Acked-by: Tigran
  • Loading branch information
alrossi committed May 3, 2023
1 parent 3657a1f commit 7817174
Show file tree
Hide file tree
Showing 14 changed files with 188 additions and 30 deletions.
Expand Up @@ -36,6 +36,7 @@ public class PrefixRestriction implements Restriction {
private transient Function<FsPath, FsPath> resolver;

public PrefixRestriction(FsPath... prefixes) {

this.prefixes = ImmutableSet.copyOf(prefixes);
}

Expand Down
Expand Up @@ -104,6 +104,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import org.dcache.util.list.DirectoryStream;
import org.dcache.util.list.ListDirectoryHandler;
import org.dcache.vehicles.FileAttributes;
import org.dcache.vehicles.PnfsResolveSymlinksMessage;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -445,7 +446,7 @@ protected void expandDepthFirst(Long id, PID pid, FsPath path, FileAttributes di
}

protected List<BulkRequestTarget> getInitialTargets() {
return targetStore.getInitialTargets(rid, true);
return targetStore.getInitialTargets(rid, true);
}

protected boolean hasBeenCancelled(Long id, PID pid, FsPath path, FileAttributes attributes) {
Expand Down Expand Up @@ -483,6 +484,12 @@ protected void removeTarget(BulkRequestTarget target) {
checkTransitionToDirs();
}

protected FsPath resolvePath(String targetPath) throws CacheException {
PnfsResolveSymlinksMessage message = new PnfsResolveSymlinksMessage(targetPath, null);
message = pnfsHandler.request(message);
return FsPath.create(message.getResolvedPath());
}

private void checkTransitionToDirs() {
if (semaphore.availablePermits() == activity.getMaxPermits()) {
synchronized (this) {
Expand Down
Expand Up @@ -226,22 +226,22 @@ protected void retryFailed(BatchedResult result, FileAttributes attributes)

private void addInfo(BulkRequestTarget target) {
Long id = target.getId();
FsPath path = target.getPath();
if (targetPrefix != null && !path.contains(targetPrefix)) {
path = computeFsPath(targetPrefix, target.getPath().toString());
}
try {
FsPath path = resolvePath(target.getPath().toString());
if (targetPrefix != null && !path.contains(targetPrefix)) {
path = computeFsPath(targetPrefix, target.getPath().toString());
}
targetInfo.add(new TargetInfo(id, path, pnfsHandler.getFileAttributes(path,
MINIMALLY_REQUIRED_ATTRIBUTES)));
} catch (CacheException e) {
LOGGER.error("addInfo {}, path {}, error {}.", ruid, path, e.getMessage());
LOGGER.error("addInfo {}, path {}, error {}.", ruid, target.getPath(), e.getMessage());
target.setState(FAILED);
target.setErrorObject(e);
statistics.increment(FAILED.name());
try {
targetStore.storeOrUpdate(target);
} catch (BulkStorageException ex) {
LOGGER.error("addInfo {}, path {}, could not store, error {}.", ruid, path,
LOGGER.error("addInfo {}, path {}, could not store, error {}.", ruid, target.getPath(),
ex.getMessage());
}
} finally {
Expand Down
Expand Up @@ -129,17 +129,31 @@ protected void processFileTargets() throws InterruptedException {
for (BulkRequestTarget tgt : requestTargets) {
checkForRequestCancellation();
Long id = tgt.getId();
FsPath path = tgt.getPath();
if (targetPrefix != null && !path.contains(targetPrefix)) {
path = computeFsPath(targetPrefix, tgt.getPath().toString());
}

switch (depth) {
case NONE:
perform(id, PID.INITIAL, path, null);
break;
default:
handleTarget(id, PID.INITIAL, path);
try {
FsPath path = resolvePath(tgt.getPath().toString());
if (targetPrefix != null && !path.contains(targetPrefix)) {
path = computeFsPath(targetPrefix, tgt.getPath().toString());
}

switch (depth) {
case NONE:
perform(id, PID.INITIAL, path, null);
break;
default:
handleTarget(id, PID.INITIAL, path);
}
} catch (CacheException e) {
LOGGER.error("problem handling target {}: {}.", tgt, e.toString());
tgt.setState(FAILED);
tgt.setErrorObject(e);
statistics.increment(FAILED.name());
try {
targetStore.storeOrUpdate(tgt);
} catch (BulkStorageException ex) {
LOGGER.error("processFileTargets {}, path {}, could not store, error {}.", ruid,
tgt.getPath(),
ex.getMessage());
}
}
}
}
Expand Down
Expand Up @@ -67,6 +67,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import com.google.common.base.Strings;
import com.google.gson.Gson;
import com.google.gson.JsonParseException;
import diskCacheV111.util.PnfsHandler;
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
Expand All @@ -84,6 +85,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import java.util.Set;
import java.util.stream.Collectors;
import javax.inject.Inject;
import javax.inject.Named;
import javax.security.auth.Subject;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.BadRequestException;
Expand All @@ -104,6 +106,8 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import org.dcache.auth.Subjects;
import org.dcache.auth.attributes.Restriction;
import org.dcache.auth.attributes.Restrictions;
import org.dcache.cells.CellStub;
import org.dcache.restful.util.HandlerBuilders;
import org.dcache.restful.util.RequestUser;
import org.dcache.restful.util.bulk.BulkServiceCommunicator;
import org.dcache.services.bulk.BulkRequest;
Expand Down Expand Up @@ -136,6 +140,10 @@ public final class BulkResources {
@Inject
private BulkServiceCommunicator service;

@Inject
@Named("pnfs-stub")
private CellStub pnfsmanager;

/**
* @return List of bulk request summaries that have not been cleared.
* If the client includes no query string then the response contains all bulk requests
Expand Down Expand Up @@ -224,8 +232,8 @@ public Response submit(
String requestPayload) {
Subject subject = getSubject();
Restriction restriction = getRestriction();

BulkRequest request = toBulkRequest(requestPayload, this.request);
PnfsHandler handler = HandlerBuilders.unrestrictedPnfsHandler(pnfsmanager);
BulkRequest request = toBulkRequest(requestPayload, this.request, handler);

/*
* Frontend sets the URL. The backend service provides the UUID.
Expand Down Expand Up @@ -414,7 +422,7 @@ public static Restriction getRestriction() {
* they are defined in the Bulk service as well.
*/
@VisibleForTesting
static BulkRequest toBulkRequest(String requestPayload, HttpServletRequest httpServletRequest) {
static BulkRequest toBulkRequest(String requestPayload, HttpServletRequest httpServletRequest, PnfsHandler handler) {
if (Strings.emptyToNull(requestPayload) == null) {
throw new BadRequestException("empty request payload.");
}
Expand Down Expand Up @@ -448,7 +456,7 @@ static BulkRequest toBulkRequest(String requestPayload, HttpServletRequest httpS
string = removeEntry(map, String.class, "target_prefix", "target-prefix",
"targetPrefix");
if (httpServletRequest != null) {
request.setTargetPrefix(getUserRootAwareTargetPrefix(httpServletRequest, string));
request.setTargetPrefix(getUserRootAwareTargetPrefix(httpServletRequest, string, handler));
} else {
request.setTargetPrefix(string);
}
Expand Down
Expand Up @@ -64,6 +64,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import static org.dcache.restful.util.RequestUser.getRestriction;
import static org.dcache.restful.util.RequestUser.getSubject;

import diskCacheV111.util.PnfsHandler;
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
Expand All @@ -75,6 +76,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import java.util.List;
import java.util.Map;
import javax.inject.Inject;
import javax.inject.Named;
import javax.security.auth.Subject;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.BadRequestException;
Expand All @@ -87,6 +89,8 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import org.dcache.auth.attributes.Restriction;
import org.dcache.cells.CellStub;
import org.dcache.restful.util.HandlerBuilders;
import org.dcache.restful.util.bulk.BulkServiceCommunicator;
import org.dcache.services.bulk.BulkRequest;
import org.dcache.services.bulk.BulkRequest.Depth;
Expand All @@ -112,6 +116,10 @@ public final class ReleaseResources {
@Inject
private BulkServiceCommunicator service;

@Inject
@Named("pnfs-stub")
private CellStub pnfsmanager;

/**
* Release files belonging to a bulk STAGE request.
* <p>
Expand Down Expand Up @@ -173,7 +181,10 @@ public Response release(
* Frontend sets the URL. The backend service provides the UUID.
*/
request.setUrlPrefix(this.request.getRequestURL().toString());
request.setTargetPrefix(getUserRootAwareTargetPrefix(this.request, null));

PnfsHandler handler = HandlerBuilders.unrestrictedPnfsHandler(pnfsmanager);

request.setTargetPrefix(getUserRootAwareTargetPrefix(this.request, null, handler));

BulkRequestMessage message = new BulkRequestMessage(request, restriction);
message.setSubject(subject);
Expand Down
Expand Up @@ -65,6 +65,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import static org.dcache.restful.util.JSONUtils.newBadRequestException;

import com.google.common.base.Strings;
import diskCacheV111.util.PnfsHandler;
import io.swagger.annotations.Api;
import io.swagger.annotations.ApiOperation;
import io.swagger.annotations.ApiParam;
Expand All @@ -77,6 +78,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import java.util.Map;
import java.util.Optional;
import javax.inject.Inject;
import javax.inject.Named;
import javax.security.auth.Subject;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.BadRequestException;
Expand All @@ -92,7 +94,9 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
import org.dcache.auth.attributes.Restriction;
import org.dcache.cells.CellStub;
import org.dcache.restful.providers.tape.StageRequestInfo;
import org.dcache.restful.util.HandlerBuilders;
import org.dcache.restful.util.bulk.BulkServiceCommunicator;
import org.dcache.services.bulk.BulkRequest;
import org.dcache.services.bulk.BulkRequest.Depth;
Expand Down Expand Up @@ -125,6 +129,10 @@ public final class StageResources {
@Inject
private BulkServiceCommunicator service;

@Inject
@Named("pnfs-stub")
private CellStub pnfsmanager;

private String[] supportedSitenames;

/**
Expand Down Expand Up @@ -341,7 +349,9 @@ private BulkRequest toBulkRequest(String requestPayload) {
request.setClearOnFailure(false);
request.setClearOnSuccess(false);
request.setActivity("STAGE");
request.setTargetPrefix(getUserRootAwareTargetPrefix(this.request, null));

PnfsHandler handler = HandlerBuilders.unrestrictedPnfsHandler(pnfsmanager);
request.setTargetPrefix(getUserRootAwareTargetPrefix(this.request, null, handler));

try {
JSONObject reqPayload = new JSONObject(requestPayload);
Expand Down
Expand Up @@ -45,4 +45,11 @@ public static PnfsHandler roleAwarePnfsHandler(CellStub pnfsManager) {

return handler;
}

public static PnfsHandler unrestrictedPnfsHandler(CellStub pnfsManager) {
PnfsHandler handler = pnfsHandler(pnfsManager);
handler.setSubject(Subjects.ROOT);
handler.setRestriction(Restrictions.none());
return handler;
}
}
Expand Up @@ -20,17 +20,21 @@

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import diskCacheV111.util.CacheException;
import diskCacheV111.util.FsPath;
import diskCacheV111.util.PnfsHandler;
import java.util.Set;
import javax.security.auth.Subject;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.BadRequestException;
import org.dcache.auth.Subjects;
import org.dcache.auth.attributes.LoginAttribute;
import org.dcache.auth.attributes.LoginAttributes;
import org.dcache.auth.attributes.Restriction;
import org.dcache.auth.attributes.Restrictions;
import org.dcache.auth.attributes.RootDirectory;
import org.dcache.http.AuthenticationHandler;
import org.dcache.vehicles.PnfsResolveSymlinksMessage;

/**
* Utility class for methods that operate on an HttpServletRequest object.
Expand Down Expand Up @@ -63,15 +67,23 @@ public static Restriction roleAwareRestriction(HttpServletRequest request) {
}

public static String getUserRootAwareTargetPrefix(HttpServletRequest request,
String includedPrefix) {
String includedPrefix, PnfsHandler handler) throws BadRequestException {
FsPath userRootPath = getLoginAttributes(request).stream()
.filter(RootDirectory.class::isInstance)
.findFirst()
.map(RootDirectory.class::cast)
.map(RootDirectory::getRoot)
.map(FsPath::create)
.orElse(FsPath.ROOT);
return getTargetPrefixFromUserRoot(userRootPath, includedPrefix);
PnfsResolveSymlinksMessage message = new PnfsResolveSymlinksMessage(userRootPath.toString(),
includedPrefix);
try {
message = handler.request(message);
} catch (CacheException e) {
throw new BadRequestException(e);
}
return getTargetPrefixFromUserRoot(FsPath.create(message.getResolvedPath()),
message.getPrefix());
}

@VisibleForTesting
Expand Down
Expand Up @@ -62,11 +62,16 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import static org.dcache.restful.resources.bulk.BulkResources.toBulkRequest;
import static org.junit.Assert.assertEquals;

import diskCacheV111.util.CacheException;
import diskCacheV111.util.PnfsHandler;
import diskCacheV111.vehicles.PnfsMessage;
import dmg.cells.nucleus.CellPath;
import java.util.List;
import java.util.concurrent.TimeUnit;
import javax.ws.rs.BadRequestException;
import org.dcache.services.bulk.BulkRequest;
import org.dcache.services.bulk.BulkRequest.Depth;
import org.dcache.vehicles.PnfsResolveSymlinksMessage;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -91,6 +96,10 @@ public class BulkRequestJsonParseTest {
Boolean clearOnFailure;
Depth expandDirectories;

PnfsHandler handler;

PnfsResolveSymlinksMessage message;

@Before
public void setUp() {
activity = "PIN";
Expand All @@ -101,6 +110,12 @@ public void setUp() {
clearOnSuccess = true;
clearOnFailure = true;
expandDirectories = Depth.ALL;
handler = new PnfsHandler(new CellPath()) {
public <T extends PnfsMessage> T request(T msg)
throws CacheException {
return msg;
}
};
}

@Test
Expand Down Expand Up @@ -167,6 +182,6 @@ private void givenJsonWithArrayTargetAttribute() {
}

private void whenParsed() {
bulkRequest = toBulkRequest(requestJson, null);
bulkRequest = toBulkRequest(requestJson, null, handler);
}
}

0 comments on commit 7817174

Please sign in to comment.