The backend S3 client now resolves credentials from user-configurable environment variables
This commit is contained in:
@@ -71,7 +71,7 @@ Narratio now includes configuration and path-model foundations for archive suppo
|
||||
|
||||
Implemented foundations:
|
||||
|
||||
- `pipeline.storage.s3` config shape (`bucket`, `root_prefix`, `region`, `endpoint`, `force_path_style`)
|
||||
- `pipeline.storage.s3` config shape (`bucket`, `root_prefix`, `region`, `endpoint`, `force_path_style`, `access_key_id_env`, `secret_access_key_env`)
|
||||
- `pipeline.spool` config shape (`root`, `delete_audio_after_archive`)
|
||||
- `pipeline.archive` config shape (`enabled`, `upload_run`, `promote_artifacts`)
|
||||
- promotion-rule validation (`from`/`to` required, relative-only paths, traversal rejected)
|
||||
@@ -83,6 +83,8 @@ Implemented foundations:
|
||||
Current defaults:
|
||||
|
||||
- `pipeline.storage.s3.root_prefix`: `dnd`
|
||||
- `pipeline.storage.s3.access_key_id_env`: `OBJECT_STORAGE_KEY_ID`
|
||||
- `pipeline.storage.s3.secret_access_key_env`: `OBJECT_STORAGE_KEY`
|
||||
- `pipeline.workspace.cleanup_after_archive`: `false`
|
||||
- `pipeline.spool.root`: `/var/spool/narratio`
|
||||
- `pipeline.spool.delete_audio_after_archive`: `false`
|
||||
@@ -110,7 +112,8 @@ Current boundaries:
|
||||
- `pipeline.workspace.cleanup_after_archive: true` removes only the run-scoped local workdir after successful archive commit
|
||||
- cleanup executes only after all selected stages for the command invocation succeed
|
||||
- cleanup does not run for failed, incomplete, skipped, or unarchived runs
|
||||
- local development `audio_dir`/`audio_files` source inputs are never deleted by spool cleanup
|
||||
- local development `audio_dir`/`audio_files` source inputs are never deleted by spool cleanup
|
||||
- S3 credentials are resolved from configured env-var names when both are present; if either is missing, Narratio falls back to the AWS SDK default credential chain
|
||||
|
||||
S3 input details and current boundaries are documented in [docs/s3-audio-input.md](docs/s3-audio-input.md).
|
||||
|
||||
|
||||
@@ -151,6 +151,8 @@ Storage and archive foundations:
|
||||
- `region`
|
||||
- `endpoint`
|
||||
- `force_path_style` (default `false`)
|
||||
- `access_key_id_env` (default `OBJECT_STORAGE_KEY_ID`)
|
||||
- `secret_access_key_env` (default `OBJECT_STORAGE_KEY`)
|
||||
- `pipeline.spool.root` defaults to `/var/spool/narratio`
|
||||
- `pipeline.workspace.cleanup_after_archive` defaults to `false`
|
||||
- `pipeline.spool.delete_audio_after_archive` defaults to `false`
|
||||
@@ -176,7 +178,9 @@ Session input foundations:
|
||||
Cross-config validation scope:
|
||||
|
||||
- `pipeline.storage.s3.bucket` is required only when an S3-dependent feature is explicitly configured (for current foundations, that includes `session.inputs.audio_s3`, and archive upload intent when using `storage.backend: s3`)
|
||||
- no AWS credentials are stored in Narratio config; credential resolution remains an external runtime concern
|
||||
- no AWS credential values are stored in Narratio config; only env-var names are configured
|
||||
- when both configured credential env vars resolve to non-empty values, the S3 backend uses them as static credentials
|
||||
- when either configured credential value is missing, the S3 backend falls back to the AWS SDK default credential chain
|
||||
|
||||
Remote object-store backend scope:
|
||||
|
||||
|
||||
@@ -20,6 +20,8 @@ Not implemented:
|
||||
|
||||
- `storage.s3.bucket` must be set when S3 audio input is used.
|
||||
- `storage.s3.root_prefix` defaults to `dnd`.
|
||||
- `storage.s3.access_key_id_env` defaults to `OBJECT_STORAGE_KEY_ID`.
|
||||
- `storage.s3.secret_access_key_env` defaults to `OBJECT_STORAGE_KEY`.
|
||||
- `spool.root` defaults to `/var/spool/narratio`.
|
||||
|
||||
`session.yml`:
|
||||
|
||||
@@ -30,6 +30,8 @@ Construction:
|
||||
- region
|
||||
- endpoint
|
||||
- force_path_style
|
||||
- access_key_id_env
|
||||
- secret_access_key_env
|
||||
|
||||
## Key Invariant
|
||||
|
||||
@@ -42,7 +44,9 @@ S3 session/run key builders remain separate and continue to live outside backend
|
||||
## Security Boundary
|
||||
|
||||
- do not store AWS credentials in Narratio config
|
||||
- AWS credentials are resolved through standard AWS SDK credential chains
|
||||
- Narratio first checks configured env-var names (`access_key_id_env`, `secret_access_key_env`);
|
||||
when both are present and non-empty, it uses static credentials from those values
|
||||
- when either configured credential value is missing, Narratio falls back to the standard AWS SDK credential chain
|
||||
- AWS SDK-specific types remain isolated to the storage adapter package
|
||||
|
||||
## Testing
|
||||
|
||||
@@ -8,6 +8,9 @@ storage:
|
||||
bucket: "my-dnd-archive"
|
||||
root_prefix: "dnd"
|
||||
region: "us-east-1"
|
||||
# Optional credential env-var names (defaulted when omitted):
|
||||
# access_key_id_env: "OBJECT_STORAGE_KEY_ID"
|
||||
# secret_access_key_env: "OBJECT_STORAGE_KEY"
|
||||
|
||||
spool:
|
||||
root: "/var/spool/narratio"
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
"time"
|
||||
|
||||
awsconfig "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"
|
||||
"github.com/aws/aws-sdk-go-v2/service/s3/types"
|
||||
"github.com/aws/smithy-go"
|
||||
@@ -35,6 +36,8 @@ type s3ClientOptions struct {
|
||||
Region string
|
||||
Endpoint string
|
||||
ForcePathStyle bool
|
||||
AccessKeyID string
|
||||
SecretKey string
|
||||
}
|
||||
|
||||
var newS3Client = func(ctx context.Context, opts s3ClientOptions) (s3API, error) {
|
||||
@@ -42,6 +45,15 @@ var newS3Client = func(ctx context.Context, opts s3ClientOptions) (s3API, error)
|
||||
if strings.TrimSpace(opts.Region) != "" {
|
||||
loadOpts = append(loadOpts, awsconfig.WithRegion(strings.TrimSpace(opts.Region)))
|
||||
}
|
||||
if strings.TrimSpace(opts.AccessKeyID) != "" && strings.TrimSpace(opts.SecretKey) != "" {
|
||||
loadOpts = append(loadOpts, awsconfig.WithCredentialsProvider(
|
||||
credentials.NewStaticCredentialsProvider(
|
||||
strings.TrimSpace(opts.AccessKeyID),
|
||||
strings.TrimSpace(opts.SecretKey),
|
||||
"",
|
||||
),
|
||||
))
|
||||
}
|
||||
awsCfg, err := awsconfig.LoadDefaultConfig(ctx, loadOpts...)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("load aws config: %w", err)
|
||||
@@ -67,6 +79,8 @@ func NewS3BackendFromConfig(ctx context.Context, cfg config.StorageS3Config) (*S
|
||||
Region: cfg.Region,
|
||||
Endpoint: cfg.Endpoint,
|
||||
ForcePathStyle: cfg.ForcePathStyle,
|
||||
AccessKeyID: s3CredentialFromEnv(orDefaultEnvName(cfg.AccessKeyIDEnv, config.DefaultS3AccessKeyIDEnv)),
|
||||
SecretKey: s3CredentialFromEnv(orDefaultEnvName(cfg.SecretKeyEnv, config.DefaultS3SecretAccessKeyEnv)),
|
||||
})
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("build s3 client: %w", err)
|
||||
@@ -78,6 +92,26 @@ func NewS3BackendFromConfig(ctx context.Context, cfg config.StorageS3Config) (*S
|
||||
}, nil
|
||||
}
|
||||
|
||||
func s3CredentialFromEnv(envVarName string) string {
|
||||
name := strings.TrimSpace(envVarName)
|
||||
if name == "" {
|
||||
return ""
|
||||
}
|
||||
value, ok := os.LookupEnv(name)
|
||||
if !ok {
|
||||
return ""
|
||||
}
|
||||
return strings.TrimSpace(value)
|
||||
}
|
||||
|
||||
func orDefaultEnvName(name, fallback string) string {
|
||||
trimmed := strings.TrimSpace(name)
|
||||
if trimmed == "" {
|
||||
return fallback
|
||||
}
|
||||
return trimmed
|
||||
}
|
||||
|
||||
// List returns objects under prefix.
|
||||
func (b *S3Backend) List(ctx context.Context, prefix string) ([]ObjectInfo, error) {
|
||||
normalizedPrefix := normalizeObjectKey(prefix)
|
||||
|
||||
@@ -187,6 +187,8 @@ func TestS3BackendExistsNotFound(t *testing.T) {
|
||||
func TestNewS3BackendFromConfigUsesClientOptions(t *testing.T) {
|
||||
original := newS3Client
|
||||
t.Cleanup(func() { newS3Client = original })
|
||||
t.Setenv("OBJECT_STORAGE_KEY_ID", "id-123")
|
||||
t.Setenv("OBJECT_STORAGE_KEY", "secret-abc")
|
||||
|
||||
var got s3ClientOptions
|
||||
newS3Client = func(_ context.Context, opts s3ClientOptions) (s3API, error) {
|
||||
@@ -209,6 +211,9 @@ func TestNewS3BackendFromConfigUsesClientOptions(t *testing.T) {
|
||||
if got.Region != "us-east-1" || got.Endpoint != "http://localhost:9000" || !got.ForcePathStyle {
|
||||
t.Fatalf("client options = %#v, want region/endpoint/path-style values", got)
|
||||
}
|
||||
if got.AccessKeyID != "id-123" || got.SecretKey != "secret-abc" {
|
||||
t.Fatalf("client options credentials = %#v, want env-resolved static credentials", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewS3BackendFromConfigRequiresBucket(t *testing.T) {
|
||||
@@ -218,6 +223,30 @@ func TestNewS3BackendFromConfigRequiresBucket(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestNewS3BackendFromConfigFallsBackWhenCredentialEnvMissing(t *testing.T) {
|
||||
original := newS3Client
|
||||
t.Cleanup(func() { newS3Client = original })
|
||||
|
||||
var got s3ClientOptions
|
||||
newS3Client = func(_ context.Context, opts s3ClientOptions) (s3API, error) {
|
||||
got = opts
|
||||
return &fakeS3API{}, nil
|
||||
}
|
||||
|
||||
_, err := NewS3BackendFromConfig(context.Background(), config.StorageS3Config{
|
||||
Bucket: "my-archive",
|
||||
Region: "us-east-1",
|
||||
AccessKeyIDEnv: "MISSING_ACCESS_KEY_ID",
|
||||
SecretKeyEnv: "MISSING_SECRET_KEY",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("NewS3BackendFromConfig() error = %v", err)
|
||||
}
|
||||
if got.AccessKeyID != "" || got.SecretKey != "" {
|
||||
t.Fatalf("client options credentials = %#v, want empty fallback values", got)
|
||||
}
|
||||
}
|
||||
|
||||
func strPtr(v string) *string { return &v }
|
||||
func int64Ptr(v int64) *int64 { return &v }
|
||||
|
||||
|
||||
@@ -60,6 +60,8 @@ type StorageS3Config struct {
|
||||
Region string `yaml:"region"`
|
||||
Endpoint string `yaml:"endpoint"`
|
||||
ForcePathStyle bool `yaml:"force_path_style"`
|
||||
AccessKeyIDEnv string `yaml:"access_key_id_env"`
|
||||
SecretKeyEnv string `yaml:"secret_access_key_env"`
|
||||
}
|
||||
|
||||
// SpoolConfig configures local spool storage for staged data.
|
||||
|
||||
@@ -8,6 +8,8 @@ const (
|
||||
DefaultSessionConfigPathLocal = "./session.yml"
|
||||
DefaultSessionConfigPathUsrLocal = "/usr/local/etc/narratio/session.yml"
|
||||
DefaultSessionConfigPathEtc = "/etc/narratio/session.yml"
|
||||
DefaultS3AccessKeyIDEnv = "OBJECT_STORAGE_KEY_ID"
|
||||
DefaultS3SecretAccessKeyEnv = "OBJECT_STORAGE_KEY"
|
||||
)
|
||||
|
||||
// DefaultPipelineConfigSearchPaths defines the default search order for
|
||||
|
||||
@@ -176,6 +176,12 @@ func applyStorageDefaults(cfg *StorageConfig) {
|
||||
if cfg.S3.RootPrefix == "" {
|
||||
cfg.S3.RootPrefix = "dnd"
|
||||
}
|
||||
if cfg.S3.AccessKeyIDEnv == "" {
|
||||
cfg.S3.AccessKeyIDEnv = DefaultS3AccessKeyIDEnv
|
||||
}
|
||||
if cfg.S3.SecretKeyEnv == "" {
|
||||
cfg.S3.SecretKeyEnv = DefaultS3SecretAccessKeyEnv
|
||||
}
|
||||
}
|
||||
|
||||
func applySpoolDefaults(cfg *SpoolConfig) {
|
||||
|
||||
@@ -24,6 +24,12 @@ storage:
|
||||
if cfg.Pipeline.Storage.S3.RootPrefix != "dnd" {
|
||||
t.Fatalf("storage.s3.root_prefix = %q, want dnd", cfg.Pipeline.Storage.S3.RootPrefix)
|
||||
}
|
||||
if cfg.Pipeline.Storage.S3.AccessKeyIDEnv != DefaultS3AccessKeyIDEnv {
|
||||
t.Fatalf("storage.s3.access_key_id_env = %q, want %q", cfg.Pipeline.Storage.S3.AccessKeyIDEnv, DefaultS3AccessKeyIDEnv)
|
||||
}
|
||||
if cfg.Pipeline.Storage.S3.SecretKeyEnv != DefaultS3SecretAccessKeyEnv {
|
||||
t.Fatalf("storage.s3.secret_access_key_env = %q, want %q", cfg.Pipeline.Storage.S3.SecretKeyEnv, DefaultS3SecretAccessKeyEnv)
|
||||
}
|
||||
if cfg.Pipeline.Storage.S3.ForcePathStyle {
|
||||
t.Fatalf("storage.s3.force_path_style = true, want false default")
|
||||
}
|
||||
@@ -33,6 +39,77 @@ storage:
|
||||
}
|
||||
}
|
||||
|
||||
func TestStorageS3CredentialEnvNamesLoadAndValidate(t *testing.T) {
|
||||
pipelineYAML := testPipelineBaseYAML + `
|
||||
storage:
|
||||
backend: s3
|
||||
s3:
|
||||
bucket: my-dnd-archive
|
||||
access_key_id_env: CUSTOM_KEY_ID
|
||||
secret_access_key_env: CUSTOM_SECRET
|
||||
`
|
||||
pipelinePath, sessionPath := writeConfigFiles(t, pipelineYAML, testSessionBaseYAML)
|
||||
|
||||
cfg, err := Load(pipelinePath, sessionPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Load() error = %v", err)
|
||||
}
|
||||
if cfg.Pipeline.Storage.S3.AccessKeyIDEnv != "CUSTOM_KEY_ID" {
|
||||
t.Fatalf("storage.s3.access_key_id_env = %q, want CUSTOM_KEY_ID", cfg.Pipeline.Storage.S3.AccessKeyIDEnv)
|
||||
}
|
||||
if cfg.Pipeline.Storage.S3.SecretKeyEnv != "CUSTOM_SECRET" {
|
||||
t.Fatalf("storage.s3.secret_access_key_env = %q, want CUSTOM_SECRET", cfg.Pipeline.Storage.S3.SecretKeyEnv)
|
||||
}
|
||||
if err := Validate(cfg); err != nil {
|
||||
t.Fatalf("Validate() error = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStorageS3CredentialEnvValidation(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
pipelineYML string
|
||||
wantErr string
|
||||
}{
|
||||
{
|
||||
name: "invalid access key env name",
|
||||
pipelineYML: testPipelineBaseYAML + `
|
||||
storage:
|
||||
backend: s3
|
||||
s3:
|
||||
bucket: my-dnd-archive
|
||||
access_key_id_env: "123BAD"
|
||||
`,
|
||||
wantErr: "pipeline.storage.s3.access_key_id_env must be a valid environment variable name",
|
||||
},
|
||||
{
|
||||
name: "invalid secret key env name",
|
||||
pipelineYML: testPipelineBaseYAML + `
|
||||
storage:
|
||||
backend: s3
|
||||
s3:
|
||||
bucket: my-dnd-archive
|
||||
secret_access_key_env: "bad-name"
|
||||
`,
|
||||
wantErr: "pipeline.storage.s3.secret_access_key_env must be a valid environment variable name",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
pipelinePath, sessionPath := writeConfigFiles(t, tt.pipelineYML, testSessionBaseYAML)
|
||||
cfg, err := Load(pipelinePath, sessionPath)
|
||||
if err != nil {
|
||||
t.Fatalf("Load() error = %v", err)
|
||||
}
|
||||
err = Validate(cfg)
|
||||
if err == nil || !strings.Contains(err.Error(), tt.wantErr) {
|
||||
t.Fatalf("Validate() error = %v, want to contain %q", err, tt.wantErr)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestSpoolAndArchiveDefaults(t *testing.T) {
|
||||
pipelinePath, sessionPath := writeConfigFiles(t, testPipelineBaseYAML, testSessionBaseYAML)
|
||||
|
||||
|
||||
@@ -91,6 +91,12 @@ func validateStorage(cfg StorageConfig) error {
|
||||
if cfg.S3.Endpoint != "" && strings.TrimSpace(cfg.S3.Endpoint) == "" {
|
||||
return fmt.Errorf("pipeline.storage.s3.endpoint must be non-empty when provided")
|
||||
}
|
||||
if err := validateEnvVarNameField("pipeline.storage.s3.access_key_id_env", cfg.S3.AccessKeyIDEnv); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := validateEnvVarNameField("pipeline.storage.s3.secret_access_key_env", cfg.S3.SecretKeyEnv); err != nil {
|
||||
return err
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -430,6 +436,18 @@ func archiveUploadConfiguredForS3(pipeline *PipelineConfig) bool {
|
||||
}
|
||||
|
||||
var windowsAbsPathRE = regexp.MustCompile(`^[A-Za-z]:[\\/].*`)
|
||||
var envVarNameRE = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`)
|
||||
|
||||
func validateEnvVarNameField(fieldName, value string) error {
|
||||
trimmed := strings.TrimSpace(value)
|
||||
if trimmed == "" {
|
||||
return fmt.Errorf("%s must be non-empty", fieldName)
|
||||
}
|
||||
if !envVarNameRE.MatchString(trimmed) {
|
||||
return fmt.Errorf("%s must be a valid environment variable name", fieldName)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func validateRelativeSafePath(fieldName, value string) error {
|
||||
trimmed := strings.TrimSpace(value)
|
||||
|
||||
Reference in New Issue
Block a user