Skip to content

Commit

Permalink
dcache-qos(verifier,engine): fix incompatibility issues with Subject …
Browse files Browse the repository at this point in the history
…and attributes

Motivation:

See RT 10469 QoS change is not working.

This brought to my attention two incompatibilities which
require a bit more defensive code.

1.  https://github.com/dCache/dcache-security-fixes/tree/fix/master/qos-propagate-subject-to-adjuster
    introduced a subject field/column in the `qos-operation` table to enforce certain kinds of
    restrictions.   However, it neglected to consider the case where a pre-existing table
    would be modified by Liquibase and the update would fill that field with `null` values,
    causing a deserialization error when refetching the metadata.

2.  The `ALRPStorageUnitQoSProvider` does not know how to handle undefined AL or RP values (it expects
    the file attributes always to be defined).

Modification:

1.  Just return a null for the Subject and let the operation fail if it is a QoSModify.  The operation
    should be removed from the database, and a subsequent retry should work.

2.  Inject the default values into the class and use those when the FileAttributes do not contain them.

Result:

Two small incompatibilities fixed.

Target: master
Request: 9.1
Request: 9.0
Request: 8.2
Patch: https://rb.dcache.org/r/13997/
Bug: #10469 (RT)
Requires-notes: yes
Requires-book: no
Acked-by: Marina
  • Loading branch information
alrossi authored and lemora committed Jun 30, 2023
1 parent f15ba04 commit 68327fd
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 2 deletions.
Expand Up @@ -159,8 +159,8 @@ public FileQoSRequirements fetchRequirements(FileQoSUpdate update) throws QoSExc
}

FileAttributes attributes = descriptor.getAttributes();
AccessLatency accessLatency = attributes.getAccessLatency();
RetentionPolicy retentionPolicy = attributes.getRetentionPolicy();
AccessLatency accessLatency = attributes.getAccessLatencyIfPresent().orElse(null);
RetentionPolicy retentionPolicy = attributes.getRetentionPolicyIfPresent().orElse(null);

String unitKey = attributes.getStorageClass() + "@" + attributes.getHsm();
StorageUnit storageUnit = poolSelectionUnit().getStorageUnit(unitKey);
Expand Down
Expand Up @@ -118,6 +118,9 @@ private static VerifyOperation toOperation(ResultSet rs, int row) throws SQLExce
}

private static String serialize(Subject subject) throws QoSException {
if (subject == null) {
return null;
}
ByteArrayOutputStream baos = new ByteArrayOutputStream();
try (ObjectOutputStream ostream = new ObjectOutputStream(baos)) {
ostream.writeObject(subject);
Expand All @@ -128,6 +131,9 @@ private static String serialize(Subject subject) throws QoSException {
}

private static Object deserialize(String base64) throws SQLException {
if (base64 == null) {
return null;
}
byte[] array = Base64.getDecoder().decode(base64);
ByteArrayInputStream bais = new ByteArrayInputStream(array);
try (ObjectInputStream istream = new ObjectInputStream(bais)) {
Expand Down
Expand Up @@ -74,6 +74,18 @@ public class QoSPermissionUtils {
* @param attributes
*/
public static boolean canModifyQos(Subject subject, FileAttributes attributes) {
if (subject == null) {
/*
* This is a workaround for legacy database entries before
* https://github.com/dCache/dcache-security-fixes/tree/fix/master/qos-propagate-subject-to-adjuster
* was introduced. An incompatibility was overlooked whereby
* the database could contain entries when updated by liquibase,
* thus making the new subject field null.
*
* In this case we just return false.
*/
return false;
}
return Subjects.isRoot(subject) || Subjects.getUid(subject) == attributes.getOwner();
}

Expand Down

0 comments on commit 68327fd

Please sign in to comment.