fix(config): 让环境变量能真正配置 image_storage 等凭证
viper.Unmarshal 只解码 AllKeys() 返回的键，而 AllKeys() 只汇总 SetDefault、 配置文件和显式 BindEnv 三个来源。AutomaticEnv 仅能覆盖已在其中的键，无法引入 新键；能兜底的 viper_bind_struct 又被 build tag 排除（我们只用 -tags embed）。 因此任何「没有注册默认值、且不在 config.yaml 里」的配置项，其环境变量会被静默 丢弃。image_storage 的 endpoint/bucket/access_key_id/secret_access_key/ public_base_url 正属此列，于是纯环境变量部署落到最坏组合：IMAGE_STORAGE_ENABLED 生效使 Enabled=true，四个凭证却为空 → Active()=false → 异步生图接口整体 404， 运维看到的却是"凭证不完整"。deploy/docker-compose.yml 默认就是纯环境变量驱动， 且自动生成的 config.yaml 从不写 image_storage 段，必然踩中（见 #4458、#4542）。 同类缺口不止于此：github_oauth、google_oauth、dingtalk_connect 三组第三方登录 配置（含 client_secret）同样完全无法用环境变量设置。 - 为这些键注册零值默认，使其进入 AllKeys() 而可被环境变量覆盖。零值与"键缺失" 时的解码结果一致，故行为不变。 - sticky_escape_enabled 例外：它的实际默认是 true（靠 IsSet 守卫在解码后补上）， 注册 false 会让 IsSet 恒真而永久关闭该特性，故直接注册 true。 - 启动告警补上 missing_keys 字段，指明到底哪个凭证为空。 - 新增反射守卫测试：Config 结构体上每个可由环境变量表达的字段都必须已注册默认值。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VHreE5pzCkSYz7J45fmd2Y
This commit is contained in:
haruka
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
b1a6b80267
commit
37db8d031b
@@ -255,6 +255,22 @@ func (c *ImageStorageConfig) Active() bool {
|
||||
return c.Enabled && c.IsConfigured()
|
||||
}
|
||||
|
||||
// MissingCredentialKeys 返回 IsConfigured 所缺的配置键名。
|
||||
// 用于启动日志：只说"凭证不完整"会让运维以为自己漏填了，而实际可能是值填了却没被读到。
|
||||
func (c *ImageStorageConfig) MissingCredentialKeys() []string {
|
||||
var missing []string
|
||||
if c.Bucket == "" {
|
||||
missing = append(missing, "image_storage.bucket")
|
||||
}
|
||||
if c.AccessKeyID == "" {
|
||||
missing = append(missing, "image_storage.access_key_id")
|
||||
}
|
||||
if c.SecretAccessKey == "" {
|
||||
missing = append(missing, "image_storage.secret_access_key")
|
||||
}
|
||||
return missing
|
||||
}
|
||||
|
||||
type LinuxDoConnectConfig struct {
|
||||
Enabled bool `mapstructure:"enabled"`
|
||||
ClientID string `mapstructure:"client_id"`
|
||||
@@ -1562,6 +1578,9 @@ func load(allowMissingJWTSecret bool) (*Config, error) {
|
||||
if cfg.Gateway.OpenAIScheduler.StickyEscapeErrorRate == 0 {
|
||||
cfg.Gateway.OpenAIScheduler.StickyEscapeErrorRate = 0.5
|
||||
}
|
||||
// Kept as a backstop: setEnvReachableDefaults now registers this key with its
|
||||
// effective default (true), so IsSet always reports true and this branch no
|
||||
// longer fires. It still guards the default if that registration is dropped.
|
||||
if !cfg.Gateway.OpenAIScheduler.StickyEscapeEnabled && !viper.IsSet("gateway.openai_scheduler.sticky_escape_enabled") {
|
||||
cfg.Gateway.OpenAIScheduler.StickyEscapeEnabled = true
|
||||
}
|
||||
@@ -1926,6 +1945,15 @@ func setDefaults() {
|
||||
viper.SetDefault("image_storage.force_path_style", false)
|
||||
viper.SetDefault("image_storage.presign_expiry_hours", 24)
|
||||
viper.SetDefault("image_storage.max_download_bytes", 33554432)
|
||||
// Registered with empty defaults so AutomaticEnv can reach them: viper only
|
||||
// decodes keys present in AllKeys(), so a credential that is supplied purely
|
||||
// via IMAGE_STORAGE_* and never appears in config.yaml would be dropped and
|
||||
// silently disable the whole async image feature.
|
||||
viper.SetDefault("image_storage.endpoint", "")
|
||||
viper.SetDefault("image_storage.bucket", "")
|
||||
viper.SetDefault("image_storage.access_key_id", "")
|
||||
viper.SetDefault("image_storage.secret_access_key", "")
|
||||
viper.SetDefault("image_storage.public_base_url", "")
|
||||
|
||||
// Ops (vNext)
|
||||
viper.SetDefault("ops.enabled", true)
|
||||
@@ -2205,6 +2233,76 @@ func setDefaults() {
|
||||
viper.SetDefault("subscription_maintenance.worker_count", 2)
|
||||
viper.SetDefault("subscription_maintenance.queue_size", 1024)
|
||||
|
||||
setEnvReachableDefaults()
|
||||
}
|
||||
|
||||
// setEnvReachableDefaults registers zero-valued defaults for keys that are
|
||||
// documented in deploy/config.example.yaml but had no default of their own.
|
||||
//
|
||||
// viper.Unmarshal only decodes the keys returned by AllKeys(), which unions
|
||||
// SetDefault keys, config-file keys and explicitly bound BindEnv keys.
|
||||
// AutomaticEnv can override a key already in that union, but it never adds one,
|
||||
// and the viper_bind_struct escape hatch is compiled out (we build with
|
||||
// -tags embed). So a key that lives only in the example file was unreachable by
|
||||
// environment variable: the value was read from the process environment and
|
||||
// then silently dropped. Deployments driven purely by env — which is what
|
||||
// deploy/docker-compose.yml does — got the zero value with no warning.
|
||||
//
|
||||
// The values below are deliberately zero rather than the documented example
|
||||
// values: an absent key already unmarshalled to the zero value, so registering
|
||||
// zero keeps behavior identical while making the key addressable from the
|
||||
// environment. Any subsystem that wants a richer default still applies it after
|
||||
// unmarshal, exactly as before.
|
||||
func setEnvReachableDefaults() {
|
||||
viper.SetDefault("gateway.forced_codex_instructions_template_file", "")
|
||||
viper.SetDefault("gateway.session_idle_timeout_minutes", 0)
|
||||
viper.SetDefault("gateway.user_message_queue.mode", "")
|
||||
viper.SetDefault("update.proxy_url", "")
|
||||
|
||||
// sticky_escape_enabled is the one exception to the zero-value rule: its
|
||||
// effective default is true, applied post-unmarshal via a viper.IsSet guard.
|
||||
// Registering false would make IsSet always report true and permanently
|
||||
// disable sticky escape, so register the effective default instead. An
|
||||
// explicit false in config or env still wins.
|
||||
viper.SetDefault("gateway.openai_scheduler.sticky_escape_enabled", true)
|
||||
viper.SetDefault("gateway.openai_scheduler.sticky_escape_error_rate", 0.0)
|
||||
viper.SetDefault("gateway.openai_scheduler.sticky_escape_ttft_ms", 0)
|
||||
|
||||
// Third-party login providers. These carry client secrets and are exactly
|
||||
// the settings an operator expects to inject via the environment, but every
|
||||
// key here was previously unreachable that way.
|
||||
for _, provider := range []string{"github_oauth", "google_oauth"} {
|
||||
viper.SetDefault(provider+".enabled", false)
|
||||
viper.SetDefault(provider+".client_id", "")
|
||||
viper.SetDefault(provider+".client_secret", "")
|
||||
viper.SetDefault(provider+".authorize_url", "")
|
||||
viper.SetDefault(provider+".token_url", "")
|
||||
viper.SetDefault(provider+".userinfo_url", "")
|
||||
viper.SetDefault(provider+".emails_url", "")
|
||||
viper.SetDefault(provider+".scopes", "")
|
||||
viper.SetDefault(provider+".redirect_url", "")
|
||||
viper.SetDefault(provider+".frontend_redirect_url", "")
|
||||
}
|
||||
|
||||
viper.SetDefault("dingtalk_connect.client_id", "")
|
||||
viper.SetDefault("dingtalk_connect.client_secret", "")
|
||||
viper.SetDefault("dingtalk_connect.internal_corp_id", "")
|
||||
viper.SetDefault("dingtalk_connect.redirect_url", "")
|
||||
viper.SetDefault("dingtalk_connect.bypass_registration", false)
|
||||
viper.SetDefault("dingtalk_connect.username_attribute_key", "")
|
||||
viper.SetDefault("dingtalk_connect.enable_attribute_matching", false)
|
||||
viper.SetDefault("dingtalk_connect.enable_attribute_sync", false)
|
||||
viper.SetDefault("dingtalk_connect.attribute_sync_fields", []string{})
|
||||
viper.SetDefault("dingtalk_connect.attribute_sync_overwrite_policy", "")
|
||||
viper.SetDefault("dingtalk_connect.sync_display_name", false)
|
||||
viper.SetDefault("dingtalk_connect.sync_display_name_attr_key", "")
|
||||
viper.SetDefault("dingtalk_connect.sync_display_name_attr_name", "")
|
||||
viper.SetDefault("dingtalk_connect.sync_dept", false)
|
||||
viper.SetDefault("dingtalk_connect.sync_dept_attr_key", "")
|
||||
viper.SetDefault("dingtalk_connect.sync_dept_attr_name", "")
|
||||
viper.SetDefault("dingtalk_connect.sync_corp_email", false)
|
||||
viper.SetDefault("dingtalk_connect.sync_corp_email_attr_key", "")
|
||||
viper.SetDefault("dingtalk_connect.sync_corp_email_attr_name", "")
|
||||
}
|
||||
|
||||
func (c *Config) Validate() error {
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
//go:build unit
|
||||
|
||||
package config
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"sort"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/spf13/viper"
|
||||
)
|
||||
|
||||
// collectMapstructureKeys walks a config struct and returns every dotted key
|
||||
// viper would need in order to populate it.
|
||||
func collectMapstructureKeys(t reflect.Type, prefix string, out map[string]string) {
|
||||
for i := 0; i < t.NumField(); i++ {
|
||||
field := t.Field(i)
|
||||
if field.PkgPath != "" {
|
||||
continue // unexported
|
||||
}
|
||||
tag := field.Tag.Get("mapstructure")
|
||||
name, _, _ := strings.Cut(tag, ",")
|
||||
if name == "-" {
|
||||
continue
|
||||
}
|
||||
if name == "" {
|
||||
name = strings.ToLower(field.Name)
|
||||
}
|
||||
key := name
|
||||
if prefix != "" {
|
||||
key = prefix + "." + name
|
||||
}
|
||||
|
||||
ft := field.Type
|
||||
for ft.Kind() == reflect.Ptr {
|
||||
ft = ft.Elem()
|
||||
}
|
||||
if ft.Kind() == reflect.Struct {
|
||||
collectMapstructureKeys(ft, key, out)
|
||||
continue
|
||||
}
|
||||
if ft.Kind() == reflect.Map {
|
||||
// A map cannot be expressed in a single environment variable, so it
|
||||
// is out of scope here — such settings need a config file either way.
|
||||
continue
|
||||
}
|
||||
out[strings.ToLower(key)] = ft.String()
|
||||
}
|
||||
}
|
||||
|
||||
// TestConfigKeysAreEnvReachable is the systemic guard behind the image_storage
|
||||
// bug: viper.Unmarshal only decodes keys returned by AllKeys(), which unions
|
||||
// SetDefault keys, config-file keys and explicit BindEnv keys. AutomaticEnv can
|
||||
// override a key already in that union but never introduces one, and the
|
||||
// viper_bind_struct escape hatch is compiled out (we build with -tags embed).
|
||||
//
|
||||
// So a Config field with no registered default is unreachable by environment
|
||||
// variable whenever the deployment has no config.yaml containing it — the
|
||||
// operator sets the variable, the loader discards it, and the feature behaves
|
||||
// as if it were never configured. That is exactly how image_storage credentials
|
||||
// were lost, silently disabling async image tasks for env-driven deployments.
|
||||
//
|
||||
// When this fails, register a zero-valued default in setEnvReachableDefaults
|
||||
// for each reported key.
|
||||
func TestConfigKeysAreEnvReachable(t *testing.T) {
|
||||
bound := map[string]string{}
|
||||
collectMapstructureKeys(reflect.TypeOf(Config{}), "", bound)
|
||||
|
||||
viper.Reset()
|
||||
t.Cleanup(viper.Reset)
|
||||
setDefaults()
|
||||
registered := map[string]struct{}{}
|
||||
for _, key := range viper.AllKeys() {
|
||||
registered[key] = struct{}{}
|
||||
}
|
||||
|
||||
var unreachable []string
|
||||
for key, kind := range bound {
|
||||
if _, ok := registered[key]; !ok {
|
||||
unreachable = append(unreachable, key+" ("+kind+")")
|
||||
}
|
||||
}
|
||||
sort.Strings(unreachable)
|
||||
|
||||
if len(unreachable) > 0 {
|
||||
t.Fatalf("%d config keys have no default registered, so their environment variables are silently ignored:\n %s",
|
||||
len(unreachable), strings.Join(unreachable, "\n "))
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,41 @@
|
||||
//go:build unit
|
||||
|
||||
package config
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestLoadImageStorageFromEnv guards against a viper trap that silently disabled
|
||||
// asynchronous image tasks for every environment-variable-only deployment.
|
||||
//
|
||||
// viper only decodes keys returned by AllKeys(), which unions SetDefault keys,
|
||||
// config-file keys and explicit BindEnv keys. AutomaticEnv can override a key
|
||||
// that is already in that list, but it never introduces a new one. Credentials
|
||||
// such as image_storage.bucket therefore need an (empty) default registered, or
|
||||
// IMAGE_STORAGE_BUCKET is dropped on the floor and Active() stays false while
|
||||
// image_storage.enabled reads true — the endpoints 404 with no useful signal.
|
||||
func TestLoadImageStorageFromEnv(t *testing.T) {
|
||||
resetViperWithJWTSecret(t)
|
||||
t.Setenv("IMAGE_STORAGE_ENABLED", "true")
|
||||
t.Setenv("IMAGE_STORAGE_ENDPOINT", "https://acct.r2.cloudflarestorage.com")
|
||||
t.Setenv("IMAGE_STORAGE_BUCKET", "my-images")
|
||||
t.Setenv("IMAGE_STORAGE_ACCESS_KEY_ID", "ak")
|
||||
t.Setenv("IMAGE_STORAGE_SECRET_ACCESS_KEY", "sk")
|
||||
t.Setenv("IMAGE_STORAGE_PUBLIC_BASE_URL", "https://cdn.example.com")
|
||||
|
||||
cfg, err := Load()
|
||||
require.NoError(t, err)
|
||||
|
||||
require.True(t, cfg.ImageStorage.Enabled)
|
||||
require.Equal(t, "https://acct.r2.cloudflarestorage.com", cfg.ImageStorage.Endpoint)
|
||||
require.Equal(t, "my-images", cfg.ImageStorage.Bucket)
|
||||
require.Equal(t, "ak", cfg.ImageStorage.AccessKeyID)
|
||||
require.Equal(t, "sk", cfg.ImageStorage.SecretAccessKey)
|
||||
require.Equal(t, "https://cdn.example.com", cfg.ImageStorage.PublicBaseURL)
|
||||
|
||||
require.True(t, cfg.ImageStorage.IsConfigured())
|
||||
require.True(t, cfg.ImageStorage.Active(), "async image tasks must be active when every credential is supplied via env")
|
||||
}
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"github.com/Wei-Shaw/sub2api/internal/pkg/logger"
|
||||
"github.com/google/wire"
|
||||
"github.com/redis/go-redis/v9"
|
||||
"go.uber.org/zap"
|
||||
)
|
||||
|
||||
// BuildInfo contains build information
|
||||
@@ -529,7 +530,10 @@ func ProvideAPIKeyAuthCacheInvalidator(apiKeyService *APIKeyService) APIKeyAuthC
|
||||
func ProvideImageTaskService(store ImageTaskStore, storage ImageStorage, cfg *config.Config) *ImageTaskService {
|
||||
if !cfg.ImageStorage.Active() {
|
||||
if cfg.ImageStorage.Enabled {
|
||||
logger.L().Warn("image_storage.enabled is true but object storage is not fully configured; async image tasks are disabled")
|
||||
// 列出具体缺失的键。若这些键其实已在环境变量里设过，说明它们没被读进来，
|
||||
// 请确认 setDefaults 中已为其注册默认值（见 config.setEnvReachableDefaults）。
|
||||
logger.L().Warn("image_storage.enabled is true but object storage is not fully configured; async image tasks are disabled",
|
||||
zap.Strings("missing_keys", cfg.ImageStorage.MissingCredentialKeys()))
|
||||
}
|
||||
return NewImageTaskService(store)
|
||||
}
|
||||
|
||||
@@ -41,6 +41,22 @@ When a task completes, each generated image is uploaded to the bucket and the re
|
||||
|
||||
To support a different vendor beyond the S3-compatible client, implement the `service.ImageStorage` interface (`Save(ctx, key, contentType, data) (url, error)`) and provide it in place of the S3 implementation.
|
||||
|
||||
### Troubleshooting: the endpoints return 404 after enabling
|
||||
|
||||
`404 async image tasks are not enabled` means `image_storage` did not resolve to a complete configuration, so the feature stayed off. The route exists either way — the 404 comes from the handler, not from an unregistered path, which makes it easy to mistake for a missing build.
|
||||
|
||||
Check the startup log for:
|
||||
|
||||
```text
|
||||
WARN image_storage.enabled is true but object storage is not fully configured; async image tasks are disabled missing_keys=[...]
|
||||
```
|
||||
|
||||
`missing_keys` names exactly which credentials were empty when the config was loaded.
|
||||
|
||||
Note that releases **before v0.1.161 silently dropped `IMAGE_STORAGE_ENDPOINT`, `_BUCKET`, `_ACCESS_KEY_ID`, `_SECRET_ACCESS_KEY` and `_PUBLIC_BASE_URL`** when they were supplied only through the environment: those keys had no registered default, and viper cannot see an environment variable for a key it does not already know about. Deployments driven purely by `environment:` — which is what `deploy/docker-compose.yml` does by default — therefore reported `enabled: true` with empty credentials and 404'd on every async call. On an affected release the workaround is to also place the `image_storage` block in `/app/data/config.yaml` (copy it from `deploy/config.example.yaml`); once the keys exist in the file, the environment overrides apply normally.
|
||||
|
||||
Two further causes of a 404 that are unrelated to storage: the API key's group must be on the **OpenAI or Grok** platform (any other platform, or a key with no group at all, yields `Images API is not supported for this platform`), and a task may only be polled with the **same API key that submitted it** — polling with a different key of the same user returns `image task not found` by design.
|
||||
|
||||
## Submit a task
|
||||
|
||||
```bash
|
||||
|
||||
Reference in New Issue
Block a user