diff --git a/backend/app/cmd/cmd.go b/backend/app/cmd/cmd.go index 4d257192..534c1788 100644 --- a/backend/app/cmd/cmd.go +++ b/backend/app/cmd/cmd.go @@ -22,6 +22,7 @@ type CommonOptionsCommander interface { SetCommon(commonOpts CommonOpts) Execute(args []string) error HandleDeprecatedFlags() []DeprecatedFlag + FindDeprecatedFlagsCollisions() []DeprecatedFlag } // CommonOpts sets externally from main, shared across all commands @@ -49,6 +50,10 @@ func (c *CommonOpts) SetCommon(commonOpts CommonOpts) { // HandleDeprecatedFlags sets new flags from deprecated and returns their list func (c *CommonOpts) HandleDeprecatedFlags() []DeprecatedFlag { return nil } +// FindDeprecatedFlagsCollisions returns list of flags collisions, e.g. when both deprecated +// and new flags are set to non-default values, and different from each other. +func (c *CommonOpts) FindDeprecatedFlagsCollisions() []DeprecatedFlag { return nil } + // fileParser used to convert template strings like blah-{{.SITE}}-{{.YYYYMMDD}} the final format type fileParser struct { site string diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index 89b2b919..06ebcee1 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -321,7 +321,9 @@ func (s *ServerCommand) Execute(_ []string) error { return nil } -// HandleDeprecatedFlags sets new flags from deprecated returns their list +// HandleDeprecatedFlags sets new flags from deprecated returns their list. +// Returned list has DeprecatedFlag.Old and DeprecatedFlag.Version set, and DeprecatedFlag.New is optional +// (as some entries are removed without substitute). func (s *ServerCommand) HandleDeprecatedFlags() (result []DeprecatedFlag) { if s.Auth.Email.Host != "" && s.SMTP.Host == "" { s.SMTP.Host = s.Auth.Email.Host @@ -343,7 +345,8 @@ func (s *ServerCommand) HandleDeprecatedFlags() (result []DeprecatedFlag) { s.SMTP.Password = s.Auth.Email.SMTPPassword result = append(result, DeprecatedFlag{Old: "auth.email.passwd", New: "smtp.password", Version: "1.5"}) } - if s.Auth.Email.TimeOut != 10*time.Second && s.SMTP.TimeOut == 10*time.Second { + const emailDefaultTimout = 10 * time.Second + if s.Auth.Email.TimeOut != emailDefaultTimout && s.SMTP.TimeOut == emailDefaultTimout { s.SMTP.TimeOut = s.Auth.Email.TimeOut result = append(result, DeprecatedFlag{Old: "auth.email.timeout", New: "smtp.timeout", Version: "1.5"}) } @@ -368,8 +371,8 @@ func (s *ServerCommand) HandleDeprecatedFlags() (result []DeprecatedFlag) { s.Telegram.Token = s.Notify.Telegram.Token result = append(result, DeprecatedFlag{Old: "notify.telegram.token", New: "telegram.token", Version: "1.9"}) } - const telegramDefaultDuration = time.Second * 5 - if s.Notify.Telegram.Timeout != telegramDefaultDuration && s.Telegram.Timeout == telegramDefaultDuration { + const telegramDefaultTimeout = time.Second * 5 + if s.Notify.Telegram.Timeout != telegramDefaultTimeout && s.Telegram.Timeout == telegramDefaultTimeout { s.Telegram.Timeout = s.Notify.Telegram.Timeout result = append(result, DeprecatedFlag{Old: "notify.telegram.timeout", New: "telegram.timeout", Version: "1.9"}) } @@ -379,6 +382,40 @@ func (s *ServerCommand) HandleDeprecatedFlags() (result []DeprecatedFlag) { return result } +// FindDeprecatedFlagsCollisions returns flags which are set both old (deprecated) and new way, +// which means new ones are used and old ones are ignored by deprecated flag handler. +// It returns DeprecatedFlag list which always has only DeprecatedFlag.Old and DeprecatedFlag.New set. +func (s *ServerCommand) FindDeprecatedFlagsCollisions() (result []DeprecatedFlag) { + if stringsSetAndDifferent(s.Auth.Email.Host, s.SMTP.Host) { + result = append(result, DeprecatedFlag{Old: "auth.email.host", New: "smtp.host"}) + } + if s.Auth.Email.Port != 0 && s.SMTP.Port != 0 && s.Auth.Email.Port != s.SMTP.Port { + result = append(result, DeprecatedFlag{Old: "auth.email.port", New: "smtp.port"}) + } + if stringsSetAndDifferent(s.Auth.Email.SMTPUserName, s.SMTP.Username) { + result = append(result, DeprecatedFlag{Old: "auth.email.user", New: "smtp.username"}) + } + if stringsSetAndDifferent(s.Auth.Email.SMTPPassword, s.SMTP.Password) { + result = append(result, DeprecatedFlag{Old: "auth.email.passwd", New: "smtp.password"}) + } + const emailDefaultTimout = 10 * time.Second + if s.Auth.Email.TimeOut != emailDefaultTimout && s.SMTP.TimeOut != emailDefaultTimout && s.Auth.Email.TimeOut != s.SMTP.TimeOut { + result = append(result, DeprecatedFlag{Old: "auth.email.timeout", New: "smtp.timeout"}) + } + if !(len(s.Notify.Type) == 1 && contains("none", s.Notify.Type)) && // default, "none" notify type + (len(s.Notify.Users) != 0 || len(s.Notify.Admins) != 0) { // new notify param(s) are used, old ones will be ignored + result = append(result, DeprecatedFlag{Old: "notify.type", New: "notify.(users|admins)"}) + } + if stringsSetAndDifferent(s.Notify.Telegram.Token, s.Telegram.Token) { + result = append(result, DeprecatedFlag{Old: "notify.telegram.token", New: "telegram.token"}) + } + const telegramDefaultTimeout = time.Second * 5 + if s.Notify.Telegram.Timeout != telegramDefaultTimeout && s.Telegram.Timeout != telegramDefaultTimeout && s.Notify.Telegram.Timeout != s.Telegram.Timeout { + result = append(result, DeprecatedFlag{Old: "notify.telegram.timeout", New: "telegram.timeout"}) + } + return result +} + func (s *ServerCommand) handleDeprecatedNotifications() { for _, t := range s.Notify.Type { if t == "email" && !contains(t, s.Notify.Users) { @@ -390,6 +427,13 @@ func (s *ServerCommand) handleDeprecatedNotifications() { } } +func stringsSetAndDifferent(s1, s2 string) bool { + if s1 != "" && s2 != "" && s1 != s2 { + return true + } + return false +} + func contains(s string, a []string) bool { for _, t := range a { if t == s { diff --git a/backend/app/cmd/server_test.go b/backend/app/cmd/server_test.go index 074dce1b..6437263f 100644 --- a/backend/app/cmd/server_test.go +++ b/backend/app/cmd/server_test.go @@ -410,7 +410,10 @@ func TestServerApp_DeprecatedArgs(t *testing.T) { p := flags.NewParser(&s, flags.Default) args := []string{ "test", + "--notify.type=email", "--notify.type=telegram", + "--img-proxy", + "--notify.email.notify_admin", "--auth.email.host=smtp.example.org", "--auth.email.port=666", "--auth.email.tls", @@ -420,6 +423,7 @@ func TestServerApp_DeprecatedArgs(t *testing.T) { "--auth.email.template=file.tmpl", "--notify.telegram.token=abcd", "--notify.telegram.timeout=3m", + "--notify.telegram.api=http://example.org", } assert.Empty(t, s.SMTP.Host) assert.Empty(t, s.SMTP.Port) @@ -439,9 +443,12 @@ func TestServerApp_DeprecatedArgs(t *testing.T) { {Old: "auth.email.passwd", New: "smtp.password", Version: "1.5"}, {Old: "auth.email.timeout", New: "smtp.timeout", Version: "1.5"}, {Old: "auth.email.template", Version: "1.5"}, + {Old: "img-proxy", New: "image-proxy.http2https", Version: "1.5"}, + {Old: "notify.email.notify_admin", New: "notify.admins=email", Version: "1.9"}, {Old: "notify.type", New: "notify.(users|admins)", Version: "1.9"}, {Old: "notify.telegram.token", New: "telegram.token", Version: "1.9"}, {Old: "notify.telegram.timeout", New: "telegram.timeout", Version: "1.9"}, + {Old: "notify.telegram.api", Version: "1.9"}, }, deprecatedFlags) assert.Equal(t, "smtp.example.org", s.SMTP.Host) @@ -452,6 +459,61 @@ func TestServerApp_DeprecatedArgs(t *testing.T) { assert.Equal(t, 15*time.Second, s.SMTP.TimeOut) } +func TestServerApp_DeprecatedArgsCollisions(t *testing.T) { + s := ServerCommand{} + s.SetCommon(CommonOpts{RemarkURL: "https://demo.remark42.com", SharedSecret: "123456"}) + + p := flags.NewParser(&s, flags.Default) + args := []string{ + "test", + "--auth.email.host=smtp-old.example.org", + "--smtp.host=smtp-new.example.org", + "--auth.email.port=666", + "--smtp.port=999", + "--auth.email.user=test_user", + "--smtp.username=new_test_user", + "--auth.email.passwd=test_password", + "--smtp.password=new_test_password", + "--auth.email.timeout=15s", + "--smtp.timeout=20s", + "--notify.type=telegram", + "--notify.users=telegram", + "--notify.telegram.token=abcd", + "--telegram.token=dcba", + "--notify.telegram.timeout=3m", + "--telegram.timeout=5m", + } + _, err := p.ParseArgs(args) + require.NoError(t, err) + deprecatedFlagsCollisions := s.FindDeprecatedFlagsCollisions() + assert.ElementsMatch(t, + []DeprecatedFlag{ + {Old: "notify.type", New: "notify.(users|admins)"}, + {Old: "auth.email.host", New: "smtp.host"}, + {Old: "auth.email.port", New: "smtp.port"}, + {Old: "auth.email.user", New: "smtp.username"}, + {Old: "auth.email.passwd", New: "smtp.password"}, + {Old: "auth.email.timeout", New: "smtp.timeout"}, + {Old: "notify.telegram.token", New: "telegram.token"}, + {Old: "notify.telegram.timeout", New: "telegram.timeout"}, + }, + deprecatedFlagsCollisions) + + // case which should return nothing + s = ServerCommand{} + s.SetCommon(CommonOpts{RemarkURL: "https://demo.remark42.com", SharedSecret: "123456"}) + p = flags.NewParser(&s, flags.Default) + args = []string{ + "test", + "--auth.email.host=smtp-old.example.org", + "--smtp.host=''", + } + _, err = p.ParseArgs(args) + require.NoError(t, err) + deprecatedFlagsCollisions = s.FindDeprecatedFlagsCollisions() + assert.Empty(t, []DeprecatedFlag{}, deprecatedFlagsCollisions) +} + func Test_ACMEEmail(t *testing.T) { cmd := ServerCommand{} cmd.SetCommon(CommonOpts{RemarkURL: "https://remark.com:443", SharedSecret: "123456"}) diff --git a/backend/app/main.go b/backend/app/main.go index 2371349c..77eeac9f 100644 --- a/backend/app/main.go +++ b/backend/app/main.go @@ -45,6 +45,9 @@ func main() { SharedSecret: opts.SharedSecret, Revision: revision, }) + for _, entry := range c.FindDeprecatedFlagsCollisions() { + log.Print(fmt.Sprintf("[ERROR] deprecated --%s and new --%s options are set to different values, old one is ignored: please remove it", entry.Old, entry.New)) + } for _, entry := range c.HandleDeprecatedFlags() { deprecationNote := fmt.Sprintf("[WARN] --%s is deprecated since v%s and will be removed in the future", entry.Old, entry.Version) if entry.New != "" {