Update and finalize the next stages of the implementation roadmap
This commit is contained in:
@@ -158,6 +158,18 @@ Test close to the behavior being changed:
|
||||
- Use `internal/testutil` for shared valid fixtures only; keep edge cases near the package under test.
|
||||
- Run `go test ./...` after cross-package changes or documentation/example changes tied to tests.
|
||||
|
||||
Live integration tests must be opt-in and skipped during normal `go test ./...`
|
||||
unless their required environment variables are set. Test-only environment
|
||||
variables must use this prefix shape:
|
||||
|
||||
```text
|
||||
DISTRIBUTOR_TEST_<BACKEND>_*
|
||||
```
|
||||
|
||||
Examples include `DISTRIBUTOR_TEST_SSH_HOST` and
|
||||
`DISTRIBUTOR_TEST_S3_ENDPOINT`. Do not use production credential variable names
|
||||
for test-only controls.
|
||||
|
||||
## Examples
|
||||
|
||||
Examples under `examples/` must be valid, maintained, and free of secrets.
|
||||
|
||||
@@ -60,23 +60,52 @@ The backend must implement the current `internal/storage.Backend` contract:
|
||||
|
||||
Use native SFTP operations rather than shelling out to `ssh`, `scp`, or `rsync`.
|
||||
|
||||
Use Go SSH/SFTP libraries behind the adapter boundary:
|
||||
|
||||
- `golang.org/x/crypto/ssh`;
|
||||
- `golang.org/x/crypto/ssh/agent`;
|
||||
- `golang.org/x/crypto/ssh/knownhosts`;
|
||||
- `github.com/pkg/sftp`.
|
||||
|
||||
Authentication behavior:
|
||||
|
||||
- prefer SSH agent by default;
|
||||
- use `known_hosts` validation by default where practical;
|
||||
- support optional key-file configuration only if it can be added cleanly;
|
||||
- do not support passwords in YAML in this stage.
|
||||
- support `ssh_key_file` from the start and use it after SSH agent auth;
|
||||
- do not support password authentication in YAML in this stage;
|
||||
- support host key policies `strict`, `true`, `accept-new`, `off`, and `false`;
|
||||
- accept both YAML booleans and strings for `host_key_policy`;
|
||||
- normalize `true`, `"true"`, and `"strict"` to `strict`;
|
||||
- normalize `false`, `"false"`, and `"off"` to `off`;
|
||||
- normalize `"accept-new"` to `accept-new`;
|
||||
- default host key policy to `accept-new`;
|
||||
- support an optional `known_hosts` path, defaulting to the service user's OpenSSH known-hosts file where practical;
|
||||
- treat changed host keys as fatal for both `strict` and `accept-new`;
|
||||
- allow `off` and `false` only as explicit insecure modes, and surface that insecurity in docs and operation output where practical;
|
||||
- persist newly accepted host keys for `accept-new` when a writable known-hosts path is available;
|
||||
- fail clearly if `accept-new` needs to persist a new host key but no writable known-hosts path is available;
|
||||
- do not create a missing parent `.ssh` directory automatically in this stage.
|
||||
|
||||
Config execution behavior:
|
||||
|
||||
- use the existing accepted config shape:
|
||||
- switch SSH execution to structured config fields instead of URI-based config:
|
||||
|
||||
```yaml
|
||||
backend: ssh
|
||||
uri: ssh://user@example.com:22
|
||||
host: example.com
|
||||
user: distributor
|
||||
port: 2222
|
||||
path: /remote/root
|
||||
ssh_key_file: /home/distributor/.ssh/id_ed25519
|
||||
known_hosts: /home/distributor/.ssh/known_hosts
|
||||
host_key_policy: accept-new
|
||||
```
|
||||
|
||||
- require `host` and `path`;
|
||||
- make `user` optional, defaulting to the current OS user where available;
|
||||
- fail clearly at validation or backend-open time if `user` is omitted and the current OS user cannot be determined;
|
||||
- make `port` optional, defaulting to `22`;
|
||||
- make `ssh_key_file`, `known_hosts`, and `host_key_policy` optional;
|
||||
- do not keep `uri` as an SSH execution field unless a later compatibility stage explicitly reintroduces it;
|
||||
- keep secrets out of config files;
|
||||
- keep config loading and validation centralized in `internal/config`;
|
||||
- wire runtime construction through app-level backend factory and storage registry patterns.
|
||||
@@ -114,14 +143,26 @@ Do not document S3 or force overwrite as implemented in this stage.
|
||||
Add unit tests for:
|
||||
|
||||
- SSH config execution wiring;
|
||||
- URI and path handling;
|
||||
- structured SSH config validation and defaulting;
|
||||
- host, user, port, remote root path, key file, known-hosts path, and host key policy handling;
|
||||
- logical path validation;
|
||||
- auth fallback order;
|
||||
- host key policy behavior through test doubles or isolated known-hosts files;
|
||||
- storage error translation where practical;
|
||||
- `Walk` and `HasAny` behavior through test doubles or controlled fixtures;
|
||||
- managed deletion boundaries;
|
||||
- app-level local-to-SSH and SSH-to-local planning or wiring using fakes/mocks where possible.
|
||||
|
||||
Add integration tests only if they are skipped unless explicit SSH test endpoint environment variables are configured. Normal `go test ./...` must not require a live SSH server.
|
||||
Add integration tests only if they are skipped unless explicit SSH test endpoint environment variables are configured. Use these environment variable names:
|
||||
|
||||
- `DISTRIBUTOR_TEST_SSH_HOST`;
|
||||
- `DISTRIBUTOR_TEST_SSH_USER`;
|
||||
- `DISTRIBUTOR_TEST_SSH_PORT`;
|
||||
- `DISTRIBUTOR_TEST_SSH_PATH`;
|
||||
- `DISTRIBUTOR_TEST_SSH_KEY_FILE`;
|
||||
- `DISTRIBUTOR_TEST_SSH_KNOWN_HOSTS`.
|
||||
|
||||
Normal `go test ./...` must not require a live SSH server.
|
||||
|
||||
### Completion Criteria
|
||||
|
||||
@@ -131,7 +172,106 @@ Add integration tests only if they are skipped unless explicit SSH test endpoint
|
||||
- Normal test runs do not require a live SSH server.
|
||||
- User docs accurately describe implemented SSH behavior and boundaries.
|
||||
|
||||
## Stage 2: S3-Compatible Backend
|
||||
## Stage 2: Secrets Directory and Credential Environment Resolution
|
||||
|
||||
### Goal
|
||||
|
||||
Add a top-level secrets directory feature that lets deployments provide credential values as files while keeping config files free of literal secrets and without mutating the process environment.
|
||||
|
||||
This stage exists before S3 because S3 credentials are the first planned backend credentials that need environment-variable resolution at runtime.
|
||||
|
||||
### Implementation Scope
|
||||
|
||||
Add top-level config for a secrets directory:
|
||||
|
||||
```yaml
|
||||
secrets:
|
||||
directory: /run/secrets/distributor
|
||||
```
|
||||
|
||||
If `secrets.directory` is omitted, behavior must remain unchanged.
|
||||
|
||||
If `secrets.directory` is configured, load it before backend construction and credential resolution. If the configured directory is missing or unreadable, fail clearly before opening source or destination backends.
|
||||
|
||||
Secrets directory behavior:
|
||||
|
||||
- each valid filename becomes an internal environment key;
|
||||
- file contents become the corresponding internal environment value;
|
||||
- trim exactly one trailing `\n` or `\r\n`;
|
||||
- preserve all other whitespace;
|
||||
- valid filenames must match `[A-Za-z_][A-Za-z0-9_]*`;
|
||||
- ignore subdirectories;
|
||||
- follow symlinks to regular files;
|
||||
- ignore real directories and symlinks that resolve to directories;
|
||||
- do not enforce file owner, group, or mode permission policy;
|
||||
- never log or print secret values.
|
||||
|
||||
Conflict behavior:
|
||||
|
||||
- existing process environment values take precedence over secrets-directory values;
|
||||
- if the process environment value equals the loaded secret file value, emit no warning;
|
||||
- if the values differ, emit a clear warning during `run` output that the secret file was ignored because the real environment already has that variable;
|
||||
- the warning must include the variable name but neither value.
|
||||
|
||||
Resolver behavior:
|
||||
|
||||
- do not mutate `os.Environ`;
|
||||
- implement an internal resolver that checks the real process environment first and then secrets-directory values;
|
||||
- use the resolver for explicit credential environment variable references;
|
||||
- SDK default credential chains continue to see only the real process environment unless the variable exists there independently.
|
||||
|
||||
Per-pipeline and per-destination credential behavior should continue to come from backend config. No separate `pipelines[].env` map is needed in this stage because backend credential environment variable names are already configured per source or destination backend.
|
||||
|
||||
### Documentation Updates
|
||||
|
||||
After implementation, update only current-behavior docs:
|
||||
|
||||
- `docs/config.md`: document `secrets.directory` and credential environment resolution.
|
||||
- `docs/operations.md`: document deployment patterns for secrets directories and conflict warnings.
|
||||
- `docs/troubleshooting.md`: add missing directory, unreadable directory, invalid filename, missing referenced credential variable, and conflict-warning entries.
|
||||
- `docs/internal/config.md`: document config loading, secrets loading, and resolver behavior.
|
||||
- `docs/policy/development.md`: document how future credential-consuming features should use the resolver instead of reading `os.Getenv` directly.
|
||||
|
||||
User docs should describe `secrets.directory` as credential support, not as a general templating or shell environment feature.
|
||||
|
||||
### Tests
|
||||
|
||||
Add config tests for:
|
||||
|
||||
- omitted `secrets`;
|
||||
- valid `secrets.directory`;
|
||||
- unknown nested secrets fields.
|
||||
|
||||
Add resolver tests for:
|
||||
|
||||
- loading valid files;
|
||||
- trimming exactly one trailing newline or CRLF;
|
||||
- preserving other whitespace;
|
||||
- rejecting invalid filenames;
|
||||
- ignoring directories;
|
||||
- following symlinks;
|
||||
- missing configured directory failure;
|
||||
- existing process environment values taking precedence;
|
||||
- same process-environment and secret value producing no warning;
|
||||
- different process-environment and secret value producing a warning without either value;
|
||||
- secret values not appearing in errors, warnings, or logs.
|
||||
|
||||
Add integration-style credential resolution tests using fake credentials:
|
||||
|
||||
- explicit credential env names can be satisfied by secrets-directory files;
|
||||
- real process environment values take precedence over secrets-directory values;
|
||||
- default SDK credential chains are not fed by secrets-directory values unless those variables exist in the real process environment.
|
||||
|
||||
### Completion Criteria
|
||||
|
||||
- `secrets.directory` config is accepted and validated.
|
||||
- Runtime `run` loads configured secrets before backend construction.
|
||||
- Explicit credential env references can resolve through the secrets-aware resolver.
|
||||
- Process environment is not mutated.
|
||||
- Warnings are emitted only for differing process-environment/secret conflicts.
|
||||
- Local MVP behavior remains unchanged when `secrets.directory` is omitted.
|
||||
|
||||
## Stage 3: S3-Compatible Backend
|
||||
|
||||
### Goal
|
||||
|
||||
@@ -141,6 +281,17 @@ Implement S3-compatible object storage backend support for sources and destinati
|
||||
|
||||
Add an S3-compatible adapter package under `internal/adapters/s3`.
|
||||
|
||||
This stage depends on Stage 2. Explicit S3 credential environment variable references must resolve through the secrets-aware resolver introduced there.
|
||||
|
||||
Use the AWS SDK for Go v2 behind the adapter boundary rather than hand-rolling S3 requests or signing:
|
||||
|
||||
- `github.com/aws/aws-sdk-go-v2/config`;
|
||||
- `github.com/aws/aws-sdk-go-v2/credentials`;
|
||||
- `github.com/aws/aws-sdk-go-v2/service/s3`;
|
||||
- add `github.com/aws/aws-sdk-go-v2/feature/s3/manager` only if multipart upload or download becomes necessary.
|
||||
|
||||
Do not implement a custom standard-library-only S3 client in this stage. The adapter should rely on the SDK for Signature V4, canonical request signing, endpoint behavior, retries where configured by the SDK, error decoding, pagination primitives, and credential-chain integration.
|
||||
|
||||
The backend must implement the current `internal/storage.Backend` contract:
|
||||
|
||||
- `ReadFile` and `OpenReader`;
|
||||
@@ -164,10 +315,27 @@ credentials:
|
||||
secret_access_key_env: DISTRIBUTOR_S3_SECRET_ACCESS_KEY
|
||||
```
|
||||
|
||||
Config semantics:
|
||||
|
||||
- require `endpoint` and `bucket`;
|
||||
- make `prefix` optional;
|
||||
- make `region` optional, defaulting to `us-east-1`;
|
||||
- make `force_path_style` optional, defaulting to `true` for S3-compatible storage;
|
||||
- distinguish omitted `force_path_style` from explicit `false` so users can disable path-style addressing;
|
||||
- normalize `prefix` as a slash-separated backend-root prefix;
|
||||
- allow an empty `prefix`;
|
||||
- trim leading and trailing slashes from `prefix`, then reject `.` or `..` segments, backslashes, and traversal;
|
||||
- do not add insecure TLS or TLS verification bypass config in this stage;
|
||||
- allow `http://` endpoints for local development or local S3-compatible test services when explicitly configured.
|
||||
|
||||
Credential behavior:
|
||||
|
||||
- read access key and secret key from the named environment variables when configured;
|
||||
- support standard SDK credential discovery only if it does not weaken explicit config behavior;
|
||||
- if either `credentials.access_key_id_env` or `credentials.secret_access_key_env` is configured, require both fields;
|
||||
- when credential environment variable names are configured, require both referenced environment variables to be present and non-empty;
|
||||
- resolve explicit credential environment variable names through the secrets-aware resolver;
|
||||
- explicit environment-variable credentials take precedence over SDK credential discovery;
|
||||
- when explicit credential environment variable names are omitted, use the AWS SDK default credential chain;
|
||||
- do not feed secrets-directory values into the AWS SDK default credential chain unless those variables exist in the real process environment;
|
||||
- do not put literal secrets in YAML.
|
||||
|
||||
Object semantics:
|
||||
@@ -177,13 +345,18 @@ Object semantics:
|
||||
- `Stat` must not synthesize directory metadata only because objects exist below a prefix;
|
||||
- `Walk` should use object-list pagination and should not load an entire prefix into memory;
|
||||
- `HasAny` should stop after the first matching object;
|
||||
- `DeleteManagedBundle` must delete only listed managed output objects plus `.distributor.json`.
|
||||
- `DeleteManagedBundle` must delete only listed managed output objects plus `.distributor.json`;
|
||||
- ignore bucket versioning in this stage; normal deletes are sufficient and the adapter should not manage object versions or delete markers.
|
||||
|
||||
Write behavior:
|
||||
|
||||
- treat successful object PUT as publish-on-success;
|
||||
- set content type from `storage.WriteOptions` where available;
|
||||
- spool or buffer `WriteFrom` only when required by the SDK or backend;
|
||||
- for `Overwrite: false`, perform `HeadObject` before `PutObject` and fail with already-exists if the object exists;
|
||||
- accept the small race between `HeadObject` and `PutObject` in this stage because `distributor` is not a multi-writer synchronization tool;
|
||||
- if `WriteFrom` has `SizeKnown`, stream with the known content length where the SDK allows;
|
||||
- if `WriteFrom` size is unknown, buffer or spool as needed; initial buffering is acceptable because current artifacts are expected to be small;
|
||||
- defer multipart upload unless large artifacts become a real requirement;
|
||||
- preserve overwrite checks and conservative conflict behavior.
|
||||
|
||||
Content type behavior should cover at least:
|
||||
@@ -191,7 +364,8 @@ Content type behavior should cover at least:
|
||||
- `.md`: `text/markdown; charset=utf-8`;
|
||||
- `.html`: `text/html; charset=utf-8`;
|
||||
- `.json`: `application/json`;
|
||||
- `.txt`: `text/plain; charset=utf-8`.
|
||||
- `.txt`: `text/plain; charset=utf-8`;
|
||||
- unknown extensions: `application/octet-stream`.
|
||||
|
||||
Supported pipeline combinations:
|
||||
|
||||
@@ -217,17 +391,35 @@ Do not document force overwrite or notification adapters as implemented in this
|
||||
Add unit tests for:
|
||||
|
||||
- S3 config execution wiring;
|
||||
- default region and default path-style behavior;
|
||||
- explicit `force_path_style: false` behavior;
|
||||
- S3 prefix trimming and validation;
|
||||
- credential environment variable handling;
|
||||
- secrets-aware explicit credential resolution;
|
||||
- SDK credential-chain fallback when explicit credential environment variable names are omitted;
|
||||
- key and prefix normalization;
|
||||
- exact prefix boundary behavior;
|
||||
- exact-object `Stat` behavior that does not synthesize prefix directories;
|
||||
- path traversal rejection;
|
||||
- content type selection;
|
||||
- `Overwrite: false` `HeadObject` behavior;
|
||||
- paginated `Walk` behavior through mocks/fakes;
|
||||
- early-stop `HasAny`;
|
||||
- normal managed deletes that ignore bucket versioning;
|
||||
- managed deletion boundaries;
|
||||
- publish planning with S3 destination state fixtures.
|
||||
|
||||
Add integration tests only if they are skipped unless explicit S3-compatible endpoint credentials are configured. Normal `go test ./...` must not require live S3 credentials.
|
||||
Add integration tests only if they are skipped unless explicit S3-compatible endpoint credentials are configured. Use these environment variable names:
|
||||
|
||||
- `DISTRIBUTOR_TEST_S3_ENDPOINT`;
|
||||
- `DISTRIBUTOR_TEST_S3_BUCKET`;
|
||||
- `DISTRIBUTOR_TEST_S3_PREFIX`;
|
||||
- `DISTRIBUTOR_TEST_S3_REGION`;
|
||||
- `DISTRIBUTOR_TEST_S3_FORCE_PATH_STYLE`;
|
||||
- `DISTRIBUTOR_TEST_S3_ACCESS_KEY_ID`;
|
||||
- `DISTRIBUTOR_TEST_S3_SECRET_ACCESS_KEY`.
|
||||
|
||||
Normal `go test ./...` must not require live S3 credentials.
|
||||
|
||||
### Completion Criteria
|
||||
|
||||
@@ -237,7 +429,7 @@ Add integration tests only if they are skipped unless explicit S3-compatible end
|
||||
- Normal test runs do not require live S3.
|
||||
- User docs accurately describe implemented S3 behavior and boundaries.
|
||||
|
||||
## Stage 3: Cross-Backend Hardening and Documentation
|
||||
## Stage 4: Cross-Backend Hardening and Documentation
|
||||
|
||||
### Goal
|
||||
|
||||
@@ -305,7 +497,7 @@ Integration tests for SSH or S3 must remain opt-in through environment variables
|
||||
- Operator-facing errors are actionable.
|
||||
- Current-behavior docs and examples match implemented backend support.
|
||||
|
||||
## Stage 4: Explicit Force Overwrite
|
||||
## Stage 5: Explicit Force Overwrite
|
||||
|
||||
### Goal
|
||||
|
||||
@@ -376,7 +568,7 @@ Add tests for:
|
||||
- Default non-force behavior remains unchanged.
|
||||
- User docs clearly describe force risks and safe workflow.
|
||||
|
||||
## Stage 5: Release Readiness
|
||||
## Stage 6: Release Readiness
|
||||
|
||||
### Goal
|
||||
|
||||
@@ -432,6 +624,7 @@ The following work remains intentionally deferred unless a future roadmap promot
|
||||
- static site index pages beyond sidecar HTML output;
|
||||
- destination path remapping rules;
|
||||
- HTML themes beyond the minimal deterministic template;
|
||||
- optional HTML sanitization with `bluemonday` or equivalent, implemented as an HTML post-processing step in the transform layer if richer or less-trusted HTML output is later supported;
|
||||
- full plugin architecture;
|
||||
- web UI;
|
||||
- report editing;
|
||||
|
||||
Reference in New Issue
Block a user