mirrors/remark42
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

log when deprecated and new args are set at the same time

Browse Source 
For example, when notify.telegram.token and telegram.token
are both set but to different values, user might see
"access denied" error in log on attempt to send telegram
notification, thinking that notify.telegram.token value
is used, when in fact it is ignored and only telegram.token
is used.

New behavior is the same, ignoring the old param when new
one is set, but issuing the error log message which
explicitly tells the user about that.

Resolves #1218.
This commit is contained in:
Dmitry Verkhoturov
2022-01-31 00:40:03 +03:00
parent ffa0d34ed1
commit 8689b11e7c
4 changed files with 118 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/app/cmd/cmd.go
+5
View File
@@ -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
backend/app/cmd/server.go
+48 -4
View File
@@ -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 {
backend/app/cmd/server_test.go
+62
View File
@@ -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"})
backend/app/main.go
+3
View File
@@ -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 != "" {