Skip to content
Open
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
1 change: 1 addition & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ Descriptions say **when to load** the doc, not what it contains.
| [E2E_TESTS.md](testing/E2E_TESTS.md) | Working on the Playwright suite in `core-web/apps/dotcms-ui-e2e` |
| [API_TESTING.md](testing/API_TESTING.md) | Writing API tests — Postman (`dotcms-postman/`) and Karate (`test-karate/`) |
| [PERFORMANCE_TESTS.md](testing/PERFORMANCE_TESTS.md) | Load testing with JMeter (`test-jmeter/`) or the Kubernetes analytics suite |
| [BINARY_S3_STORAGE.md](testing/BINARY_S3_STORAGE.md) | Working on S3 asset storage (`FEATURE_FLAG_S3_ASSET_STORAGE`): what changes with the flag on and off, and how to run its MinIO and integration checks |
| [test-cases/README.md](test-cases/README.md) | Looking for the manual/feature test-suite index |

## CLI (`tools/dotcms-cli`)
Expand Down
129 changes: 129 additions & 0 deletions docs/testing/BINARY_S3_STORAGE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
# S3 asset storage

S3 asset storage lets dotCMS keep binary assets and their metadata durably in S3, with the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could this page state the deployment this feature targets? Reading the parent issue, the goal is to drop the shared NFS volume: S3 holds the only durable copy and is what nodes share, and each node keeps a bounded, disposable local cache (#37868, "scale out without a shared NFS volume"). The page doesn't say that, and a reader can easily conclude the opposite: that the flag adds S3 on top of an existing NFS asset directory, which only adds a layer (and eviction on a shared NFS directory is called out as unsafe later in the stack).

Two things would help:

  • Say explicitly that the intended setup is a node-local asset directory per node, and whether running with the flag on over a shared NFS directory is supported, discouraged or untested.
  • Say whether serving directly from S3 (streaming, or range requests without a full download to the local cache) is planned, or out of scope. Today a cold read downloads the whole object before serving, including for range requests on large files.

local asset directory acting as a cache. It is opt-in and off by default. With the flag off,
the filesystem/NFS behavior is unchanged.

This page describes what is in the code today. It grows as the remaining parts land.

## Behavior with the flag off

With the flag off, the storage chain, its providers, metadata and static push publishing run
the same code paths as before. Two details are worth knowing:

- This feature packages the AWS STS module (`aws-java-sdk-sts`). Before, the web identity step
of the SDK's default credential chain always failed for lack of that module, and the chain
moved on to the profile files and then the container or EC2 instance role. With the flag off,
every place where dotCMS falls back to the default chain (static push publishing, endpoint
validation and the S3 metadata provider) uses `NoWebIdentityCredentialsProviderChain`, which
is that chain without the web identity step, so a pod with a web identity token keeps using
the identity it used before. Two differences remain because the module is on the classpath:
a profile in the AWS config files that assumes a role (`role_arn`) now resolves instead of
failing over to the instance role, and an OSGi plugin that builds its own
`DefaultAWSCredentialsProviderChain` now gets the web identity step.
- The flag is logged at INFO only when it is on. With it off, the first read logs at debug.

## Feature flag

Set `FEATURE_FLAG_S3_ASSET_STORAGE=true` (Docker: `DOT_FEATURE_FLAG_S3_ASSET_STORAGE=true`)
in the environment or `dotmarketing-config.properties`, then restart every node. The default
is `false`.

The flag is read once per process (`AssetStorageFeature.isEnabled()`) and then kept, because
several storage objects capture the mode when they are built. A value set only in the system
table may not be seen at first read, and a runtime change is ignored until restart. In-memory
overrides through `Config.setProperty` do re-read it, which is how tests switch modes. Tests
that mock `Config` statically must call `AssetStorageFeature.reset()` themselves.

## Storage layer behavior with the flag on

The storage chain (`ChainableStoragePersistenceAPI`) and its providers change as follows:

- Writes publish to every durable provider before the new local copy becomes visible, and a
failed remote write keeps the previous local contents.
- Reads restore a missing local copy from S3. A miss is not cached as a 404, because another
node can upload the same key at any time. Object reads restore under the same per-key lock
as writes and deletes, so a restore cannot overwrite a newer write on the same chain.
- A zero-length or truncated local metadata file is a read failure, not a miss. When S3 holds a
readable copy, the chain replaces the local file with it under the per-key lock. Otherwise the
read fails and the local file is kept, so the metadata is never treated as absent and
regenerated, which would drop custom attributes such as a focal point.
- Deletes keep the local copy until all durable providers accept the deletion, so an
in-flight restore on the same chain cannot bring the object back.
- Storage and database query failures propagate instead of being reported as a missing
object.
- Providers can list objects under a prefix, verify that a durable copy has the same
contents (`hasDurableCopy`), and copy an existing file or object to durable storage without
overwriting a conflicting one (`backfillFile`, `backfillObject`).
- S3 existence checks (`existsGroup`, `existsObject`, `hasDurableCopy`) list at most one key
per lookup. Only listings that need every key (`listObjectPaths`, `listObjectSnapshots`,
`deleteGroup` and static push) follow every page.

Two building blocks are included for the slices that follow. `S3ContentAddressedStorage`
stores one immutable copy of each set of bytes at
`asset-blobs/sha256/<chars-1-2>/<chars-3-4>/<chars-5-6>/<chars-7-8>/<sha256>`.
`SharedExtractedMetadata` caches byte-derived Tika extraction at
`extracted-metadata/<source-sha256>/<configuration-hash>.json`. Nothing calls either one yet.

## Credentials and region

With the flag on, leave both `storage.file-metadata.s3.access-key` and
`storage.file-metadata.s3.secret-access-key` unset to use the AWS SDK default credential
provider chain. The configured bucket region and endpoint are kept in that mode. A partial
key pair fails initialization. The AWS STS module is packaged so role-based providers,
including web identity, work with the flag on.

Without a custom endpoint, a configured region selects the regional AWS endpoint. With no
region, the client uses the global endpoint and the SDK looks up the bucket's region on first
use. Requests are signed with SigV4, the SDK's default S3 signer. A custom endpoint
(`storage.file-metadata.s3.endpoint`) must be an absolute HTTP(S) URL and requires
`storage.file-metadata.s3.bucket-region` for request signing. Invalid endpoint configuration
fails instead of silently selecting AWS. With the flag off, configuration behaves as before.

## Sharing one bucket across installations

With the flag on, `storage.file-metadata.s3.namespace` (Docker:
`DOT_STORAGE_FILE_METADATA_S3_NAMESPACE`) separates the keys of installations that share a
bucket. Owned keys become `asset-namespaces/<namespace>/<group>/<existing-key>`. Two prefixes
stay at the bucket root and are shared by every namespace: content-addressed blobs
(`asset-blobs/`), so identical bytes are stored once, and shared extracted metadata
(`extracted-metadata/`), so identical bytes are extracted once. Use the same
value for all nodes in one cluster and distinct values for independent installations. Values
are 1 to 64 letters, digits, underscores or hyphens, starting with a letter or digit. Empty is
the default and keeps the existing layout.

Set the namespace before the installation first stores assets. Changing it later does not
migrate existing records. It separates application keys only; it does not restrict what the
bucket credentials can access.

## Static push publishing

Enabling the flag also changes static push publishing to S3 (`AWSS3Storage`). Clients sign
with SigV4 and list every page of objects instead of only the first 1,000. Without a custom
endpoint, a configured region selects that region's endpoint; with no region, the client uses
the global endpoint and the SDK looks up the bucket's region, so a region is not required.
Endpoints that use the default credential chain are not pinned to a region either. Their
configured endpoint and region are still not passed to the client on that path, as before, so
an S3-compatible custom endpoint needs a key and secret.

## Running the checks

The S3 checks use the real filesystem provider, chain and AWS adapter against a disposable
MinIO bucket. `BinaryS3StorageTest` is skipped unless `s3.test.endpoint` is set; the other
tests always run. The credentials below are disposable local test values.

```sh
docker run -d --rm --name binary-s3-test \
-p 127.0.0.1:19002:9000 \
-e MINIO_ROOT_USER=binary-storage-test \
-e MINIO_ROOT_PASSWORD=binary-storage-test \
minio/minio:latest server /data

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I tried to follow this section to run the MinIO tests locally during QA, and couldn't, because the image can't be pulled:

  • docker pull minio/minio:latest fails with pull access denied for minio/minio, repository does not exist or may require 'docker login'
  • docker pull quay.io/minio/minio:latest (MinIO's own registry) fails with 401 UNAUTHORIZED

Other Docker Hub images pull fine on the same machine, so it's this image specifically, not a general Docker or network problem.

Why it matters: BinaryS3StorageTest is the only test in this PR that talks to a real S3 API, and CI already skips it because there's no endpoint there. If developers can't run it locally either, nobody exercises it: the conditional writes (If-None-Match / If-Match), the real 404/409/412 handling, pagination, and S3ContentAddressedStorage against a real server. As a result, my local run of the PR's unit tests had to skip both tests in that class.

What I'm asking for:

  • Point the doc at an image that can actually be pulled, ideally pinned to a specific tag or digest so it doesn't silently change. A dotCMS-mirrored image would be ideal.
  • Use that same image for the docker-maven-plugin entry Fabrizzio suggested in his BinaryS3StorageTest comment, so the tests run identically in CI and locally.
  • If MinIO is replaced with another S3-compatible server, it needs to support conditional PUTs (If-None-Match: * and If-Match), because these tests depend on them.

Once there's a working image I'll re-run the 2 skipped tests and report back on this PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For context: this isn't a transient registry issue. MinIO stopped publishing community images in October 2025 (source-only since), and quay.io/minio/minio is frozen at the September 2025 release with no later security fixes, so pinning that isn't a good option. Chainguard's free minio image is a ready alternative if building a dotCMS mirror takes time.


./mvnw test -pl :dotcms-core -Dmaven.build.cache.enabled=false \
-Dtest=AssetStorageFeatureTest,AssetStorageFeatureLatchTest,S3StorageConfigurationTest,NoWebIdentityCredentialsProviderChainTest,BinaryS3StorageTest \
-Ds3.test.endpoint=http://127.0.0.1:19002

docker stop binary-s3-test
```

CI does not yet provide the MinIO service, so `BinaryS3StorageTest` does not run there.
4 changes: 4 additions & 0 deletions dotCMS/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -763,6 +763,10 @@
<groupId>com.amazonaws</groupId>
<artifactId>aws-java-sdk-s3</artifactId>
</dependency>
<dependency>
<groupId>com.amazonaws</groupId>
<artifactId>aws-java-sdk-sts</artifactId>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I want to flag this one specifically, because it's the only change in the PR that reaches installations running with the flag off, and the goal of this review is that flag-off behaves exactly like main.

What I verified. Adding the STS module changes how the AWS SDK v1 resolves credentials when no access key and secret are configured. NoWebIdentityCredentialsProviderChain handles this well for dotCMS's own code: the three places where dotCMS falls back to the default chain (the S3 metadata provider, static push publishing, and the static push endpoint validation) use a chain without the web-identity step while the flag is off. Those three keep resolving credentials exactly as they did on main.

What still changes with the flag off (the PR description mentions both, but only briefly):

  1. AWS profiles that use role_arn. On main, when the SDK reaches a profile with role_arn, it tries to load STS by reflection, fails with "To use assume role profiles the aws-java-sdk-sts module must be on the class path" (I confirmed the message in the SDK jar), and quietly moves on to the next provider, usually the EC2/ECS instance role. With STS packaged, the role is now actually assumed. So the identity dotCMS uses for S3 can change after an upgrade, with no configuration change on the customer's side. If the assumed role and the instance role have different S3 permissions, static push publishing or the S3 metadata provider could start failing with AccessDenied, or start succeeding where they used to fail.
  2. Plugins that build their own DefaultAWSCredentialsProviderChain with the SDK classes dotCMS provides. In an EKS pod with a web identity token, those plugins now get the pod's role instead of the node's role.

What it doesn't affect: installations with static keys configured (static keys always win before these steps), SDK v2 clients such as the SQS code (v2 has its own separate STS module), and stored data. Rolling back just removes the jar and returns to the old resolution, so there's no rollback-safety concern.

What I'm asking for: the risk is low, and both setups are uncommon in our Docker/Kubernetes deployments. But it's the one flag-off difference, and if it ever bites someone it will look like an unrelated permissions problem right after an upgrade. Could you:

  • add a short note in BINARY_S3_STORAGE.md under "Behavior with the flag off", and
  • make sure it goes into the release notes for the release that ships this PR?

That way support has something to point to if a customer reports S3 AccessDenied after upgrading.

</dependency>
<dependency>
<groupId>software.amazon.awssdk</groupId>
<artifactId>sqs</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.dotcms.publisher.environment.business.EnvironmentAPI;
import com.dotcms.publisher.pusher.PushUtils;
import com.dotcms.publisher.util.PusheableAsset;
import com.dotcms.storage.NoWebIdentityCredentialsProviderChain;
import com.dotcms.publishing.*;
import com.dotcms.system.event.local.business.LocalSystemEventsAPI;
import com.dotcms.system.event.local.type.pushpublish.EndpointFailureDetail;
Expand Down Expand Up @@ -507,6 +508,8 @@ private String updateStatusFailedToSend(PublishAuditHistory currentStatusHistory
/**
* Creates and returns an AWSS3EndPointPublisher.
* Uses the default AWS credentials provider chain if the tokenProp and/or secretProp are not set.
* With S3 asset storage off, that chain leaves out the web identity step, as it did before the
* STS module was packaged (see {@link NoWebIdentityCredentialsProviderChain#defaultChain()}).
* Uses the provided tokenProp and secretProp if they are both set.
*
* @param tokenProp - the AWS credentials token key
Expand All @@ -519,7 +522,7 @@ private AWSS3EndPointPublisher getAWSS3EndPointPublisher(final String tokenProp,
final String secretProp, final String endPoint, final String region) {
AWSS3EndPointPublisher endPointPublisher;
if (!UtilMethods.isSet(tokenProp) || !UtilMethods.isSet(secretProp)) {
DefaultAWSCredentialsProviderChain creds = new DefaultAWSCredentialsProviderChain();
DefaultAWSCredentialsProviderChain creds = NoWebIdentityCredentialsProviderChain.defaultChain();
endPointPublisher = new AWSS3EndPointPublisher(creds);
} else {
AWSS3Configuration awss3Configuration =
Expand Down
Loading
Loading