From df3362aef8172df22f0fa47445c553a234918383 Mon Sep 17 00:00:00 2001 From: Alessandro Ros Date: Wed, 25 Dec 2024 19:28:54 +0100 Subject: [PATCH] warn users if deprecated parameters are being used (#4080) --- .github/DISCUSSION_TEMPLATE/questions.yml | 4 ++ internal/api/api.go | 12 ++--- internal/api/api_test.go | 2 +- internal/conf/conf.go | 54 +++++++++++++++++++++-- internal/conf/conf_test.go | 28 ++++++------ internal/conf/path.go | 10 +++++ internal/core/core.go | 6 ++- 7 files changed, 89 insertions(+), 27 deletions(-) diff --git a/.github/DISCUSSION_TEMPLATE/questions.yml b/.github/DISCUSSION_TEMPLATE/questions.yml index 9576b013f89..efddde7abdc 100644 --- a/.github/DISCUSSION_TEMPLATE/questions.yml +++ b/.github/DISCUSSION_TEMPLATE/questions.yml @@ -1,4 +1,8 @@ body: + - type: markdown + attributes: + value: | + Please create a discussion FOR EACH question. Do not ask for multiple questions all at once, otherwise they'll probably never get all answered. - type: textarea attributes: diff --git a/internal/api/api.go b/internal/api/api.go index 5398642a976..062d5621993 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -329,7 +329,7 @@ func (a *API) onConfigGlobalPatch(ctx *gin.Context) { newConf.PatchGlobal(&c) - err = newConf.Validate() + err = newConf.Validate(nil) if err != nil { a.writeError(ctx, http.StatusBadRequest, err) return @@ -367,7 +367,7 @@ func (a *API) onConfigPathDefaultsPatch(ctx *gin.Context) { newConf.PatchPathDefaults(&p) - err = newConf.Validate() + err = newConf.Validate(nil) if err != nil { a.writeError(ctx, http.StatusBadRequest, err) return @@ -448,7 +448,7 @@ func (a *API) onConfigPathsAdd(ctx *gin.Context) { //nolint:dupl return } - err = newConf.Validate() + err = newConf.Validate(nil) if err != nil { a.writeError(ctx, http.StatusBadRequest, err) return @@ -489,7 +489,7 @@ func (a *API) onConfigPathsPatch(ctx *gin.Context) { //nolint:dupl return } - err = newConf.Validate() + err = newConf.Validate(nil) if err != nil { a.writeError(ctx, http.StatusBadRequest, err) return @@ -530,7 +530,7 @@ func (a *API) onConfigPathsReplace(ctx *gin.Context) { //nolint:dupl return } - err = newConf.Validate() + err = newConf.Validate(nil) if err != nil { a.writeError(ctx, http.StatusBadRequest, err) return @@ -564,7 +564,7 @@ func (a *API) onConfigPathsDelete(ctx *gin.Context) { return } - err = newConf.Validate() + err = newConf.Validate(nil) if err != nil { a.writeError(ctx, http.StatusBadRequest, err) return diff --git a/internal/api/api_test.go b/internal/api/api_test.go index 85ec632c965..481a2662b92 100644 --- a/internal/api/api_test.go +++ b/internal/api/api_test.go @@ -29,7 +29,7 @@ func tempConf(t *testing.T, cnt string) *conf.Conf { require.NoError(t, err) defer os.Remove(fi) - cnf, _, err := conf.Load(fi, nil) + cnf, _, err := conf.Load(fi, nil, nil) require.NoError(t, err) return cnf diff --git a/internal/conf/conf.go b/internal/conf/conf.go index b7b8f1b80db..f2eca4b14f1 100644 --- a/internal/conf/conf.go +++ b/internal/conf/conf.go @@ -414,7 +414,7 @@ func (conf *Conf) setDefaults() { } // Load loads a Conf. -func Load(fpath string, defaultConfPaths []string) (*Conf, string, error) { +func Load(fpath string, defaultConfPaths []string, l logger.Writer) (*Conf, string, error) { conf := &Conf{} fpath, err := conf.loadFromFile(fpath, defaultConfPaths) @@ -432,7 +432,7 @@ func Load(fpath string, defaultConfPaths []string) (*Conf, string, error) { return nil, "", err } - err = conf.Validate() + err = conf.Validate(l) if err != nil { return nil, "", err } @@ -495,8 +495,17 @@ func (conf Conf) Clone() *Conf { return &dest } +type nilLogger struct{} + +func (nilLogger) Log(_ logger.Level, _ string, _ ...interface{}) { +} + // Validate checks the configuration for errors. -func (conf *Conf) Validate() error { +func (conf *Conf) Validate(l logger.Writer) error { + if l == nil { + l = &nilLogger{} + } + // General if conf.ReadTimeout <= 0 { @@ -506,6 +515,7 @@ func (conf *Conf) Validate() error { return fmt.Errorf("'writeTimeout' must be greater than zero") } if conf.ReadBufferCount != nil { + l.Log(logger.Warn, "parameter 'readBufferCount' is deprecated and has been replaced with 'writeQueueSize'") conf.WriteQueueSize = *conf.ReadBufferCount } if (conf.WriteQueueSize & (conf.WriteQueueSize - 1)) != 0 { @@ -518,6 +528,8 @@ func (conf *Conf) Validate() error { // Authentication if conf.ExternalAuthenticationURL != nil { + l.Log(logger.Warn, "parameter 'externalAuthenticationURL' is deprecated "+ + "and has been replaced with 'authMethod' and 'authHTTPAddress'") conf.AuthMethod = AuthMethodHTTP conf.AuthHTTPAddress = *conf.ExternalAuthenticationURL } @@ -533,6 +545,10 @@ func (conf *Conf) Validate() error { } deprecatedCredentialsMode := false if anyPathHasDeprecatedCredentials(conf.PathDefaults, conf.OptionalPaths) { + l.Log(logger.Warn, "you are using one or more authentication-related deprecated parameters "+ + "(publishUser, publishPass, publishIPs, readUser, readPass, readIPs). "+ + "These have been replaced by 'authInternalUsers'") + if conf.AuthInternalUsers != nil && !reflect.DeepEqual(conf.AuthInternalUsers, defaultAuthInternalUsers) { return fmt.Errorf("authInternalUsers and legacy credentials " + "(publishUser, publishPass, publishIPs, readUser, readPass, readIPs) cannot be used together") @@ -583,12 +599,15 @@ func (conf *Conf) Validate() error { // RTSP if conf.RTSPDisable != nil { + l.Log(logger.Warn, "parameter 'rtspDisabled' is deprecated and has been replaced with 'rtsp'") conf.RTSP = !*conf.RTSPDisable } if conf.Protocols != nil { + l.Log(logger.Warn, "parameter 'protocols' is deprecated and has been replaced with 'rtspTransports'") conf.RTSPTransports = *conf.Protocols } if conf.Encryption != nil { + l.Log(logger.Warn, "parameter 'encryption' is deprecated and has been replaced with 'rtspEncryption'") conf.RTSPEncryption = *conf.Encryption } if conf.RTSPEncryption == EncryptionStrict { @@ -600,6 +619,7 @@ func (conf *Conf) Validate() error { } } if conf.AuthMethods != nil { + l.Log(logger.Warn, "parameter 'authMethods' is deprecated and has been replaced with 'rtspAuthMethods'") conf.RTSPAuthMethods = *conf.AuthMethods } if contains(conf.RTSPAuthMethods, auth.ValidateMethodDigestMD5) { @@ -613,39 +633,53 @@ func (conf *Conf) Validate() error { } } if conf.ServerCert != nil { + l.Log(logger.Warn, "parameter 'serverCert' is deprecated and has been replaced with 'rtspServerCert'") conf.RTSPServerCert = *conf.ServerCert } if conf.ServerKey != nil { + l.Log(logger.Warn, "parameter 'serverKey' is deprecated and has been replaced with 'rtspServerKey'") conf.RTSPServerKey = *conf.ServerKey } // RTMP if conf.RTMPDisable != nil { + l.Log(logger.Warn, "parameter 'rtmpDisabled' is deprecated and has been replaced with 'rtmp'") conf.RTMP = !*conf.RTMPDisable } // HLS if conf.HLSDisable != nil { + l.Log(logger.Warn, "parameter 'hlsDisable' is deprecated and has been replaced with 'hls'") conf.HLS = !*conf.HLSDisable } // WebRTC if conf.WebRTCDisable != nil { + l.Log(logger.Warn, "parameter 'webrtcDisable' is deprecated and has been replaced with 'webrtc'") conf.WebRTC = !*conf.WebRTCDisable } if conf.WebRTCICEUDPMuxAddress != nil { + l.Log(logger.Warn, "parameter 'webrtcICEUDPMuxAdderss' is deprecated "+ + "and has been replaced with 'webrtcLocalUDPAddress'") conf.WebRTCLocalUDPAddress = *conf.WebRTCICEUDPMuxAddress } if conf.WebRTCICETCPMuxAddress != nil { + l.Log(logger.Warn, "parameter 'webrtcICETCPMuxAddress' is deprecated "+ + "and has been replaced with 'webrtcLocalTCPAddress'") conf.WebRTCLocalTCPAddress = *conf.WebRTCICETCPMuxAddress } if conf.WebRTCICEHostNAT1To1IPs != nil { + l.Log(logger.Warn, "parameter 'webrtcICEHostNAT1To1IPs' is deprecated "+ + "and has been replaced with 'webrtcAdditionalHosts'") conf.WebRTCAdditionalHosts = *conf.WebRTCICEHostNAT1To1IPs } if conf.WebRTCICEServers != nil { + l.Log(logger.Warn, "parameter 'webrtcICEServers' is deprecated "+ + "and has been replaced with 'webrtcICEServers2'") + for _, server := range *conf.WebRTCICEServers { parts := strings.Split(server, ":") if len(parts) == 5 { @@ -683,21 +717,33 @@ func (conf *Conf) Validate() error { // Record (deprecated) if conf.Record != nil { + l.Log(logger.Warn, "parameter 'record' is deprecated "+ + "and has been replaced with 'pathDefaults.record'") conf.PathDefaults.Record = *conf.Record } if conf.RecordPath != nil { + l.Log(logger.Warn, "parameter 'recordPath' is deprecated "+ + "and has been replaced with 'pathDefaults.recordPath'") conf.PathDefaults.RecordPath = *conf.RecordPath } if conf.RecordFormat != nil { + l.Log(logger.Warn, "parameter 'recordFormat' is deprecated "+ + "and has been replaced with 'pathDefaults.recordFormat'") conf.PathDefaults.RecordFormat = *conf.RecordFormat } if conf.RecordPartDuration != nil { + l.Log(logger.Warn, "parameter 'recordPartDuration' is deprecated "+ + "and has been replaced with 'pathDefaults.recordPartDuration'") conf.PathDefaults.RecordPartDuration = *conf.RecordPartDuration } if conf.RecordSegmentDuration != nil { + l.Log(logger.Warn, "parameter 'recordSegmentDuration' is deprecated "+ + "and has been replaced with 'pathDefaults.recordSegmentDuration'") conf.PathDefaults.RecordSegmentDuration = *conf.RecordSegmentDuration } if conf.RecordDeleteAfter != nil { + l.Log(logger.Warn, "parameter 'recordDeleteAfter' is deprecated "+ + "and has been replaced with 'pathDefaults.recordDeleteAfter'") conf.PathDefaults.RecordDeleteAfter = *conf.RecordDeleteAfter } @@ -727,7 +773,7 @@ func (conf *Conf) Validate() error { } for _, name := range sortedKeys(conf.OptionalPaths) { - err := conf.Paths[name].validate(conf, name, deprecatedCredentialsMode) + err := conf.Paths[name].validate(conf, name, deprecatedCredentialsMode, l) if err != nil { return err } diff --git a/internal/conf/conf_test.go b/internal/conf/conf_test.go index 91eb4222f7d..1d71a827720 100644 --- a/internal/conf/conf_test.go +++ b/internal/conf/conf_test.go @@ -39,7 +39,7 @@ func TestConfFromFile(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpf) - conf, confPath, err := Load(tmpf, nil) + conf, confPath, err := Load(tmpf, nil, nil) require.NoError(t, err) require.Equal(t, tmpf, confPath) @@ -88,7 +88,7 @@ func TestConfFromFile(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpf) - _, _, err = Load(tmpf, nil) + _, _, err = Load(tmpf, nil, nil) require.NoError(t, err) }() @@ -97,7 +97,7 @@ func TestConfFromFile(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpf) - _, _, err = Load(tmpf, nil) + _, _, err = Load(tmpf, nil, nil) require.NoError(t, err) }() @@ -108,7 +108,7 @@ func TestConfFromFile(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpf) - _, _, err = Load(tmpf, nil) + _, _, err = Load(tmpf, nil, nil) require.NoError(t, err) }() } @@ -130,7 +130,7 @@ func TestConfFromFileAndEnv(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpf) - conf, confPath, err := Load(tmpf, nil) + conf, confPath, err := Load(tmpf, nil, nil) require.NoError(t, err) require.Equal(t, tmpf, confPath) @@ -149,7 +149,7 @@ func TestConfFromFileAndEnv(t *testing.T) { func TestConfFromEnvOnly(t *testing.T) { t.Setenv("MTX_PATHS_CAM1_SOURCE", "rtsp://testing") - conf, confPath, err := Load("", nil) + conf, confPath, err := Load("", nil, nil) require.NoError(t, err) require.Equal(t, "", confPath) @@ -182,7 +182,7 @@ func TestConfEncryption(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpf) - conf, confPath, err := Load(tmpf, nil) + conf, confPath, err := Load(tmpf, nil, nil) require.NoError(t, err) require.Equal(t, tmpf, confPath) @@ -202,7 +202,7 @@ func TestConfDeprecatedAuth(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpf) - conf, _, err := Load(tmpf, nil) + conf, _, err := Load(tmpf, nil, nil) require.NoError(t, err) require.Equal(t, AuthInternalUsers{ @@ -380,7 +380,7 @@ func TestConfErrors(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpf) - _, _, err = Load(tmpf, nil) + _, _, err = Load(tmpf, nil, nil) require.EqualError(t, err, ca.err) }) } @@ -388,13 +388,13 @@ func TestConfErrors(t *testing.T) { func TestSampleConfFile(t *testing.T) { func() { - conf1, confPath1, err := Load("../../mediamtx.yml", nil) + conf1, confPath1, err := Load("../../mediamtx.yml", nil, nil) require.NoError(t, err) require.Equal(t, "../../mediamtx.yml", confPath1) conf1.Paths = make(map[string]*Path) conf1.OptionalPaths = nil - conf2, confPath2, err := Load("", nil) + conf2, confPath2, err := Load("", nil, nil) require.NoError(t, err) require.Equal(t, "", confPath2) @@ -402,7 +402,7 @@ func TestSampleConfFile(t *testing.T) { }() func() { - conf1, confPath1, err := Load("../../mediamtx.yml", nil) + conf1, confPath1, err := Load("../../mediamtx.yml", nil, nil) require.NoError(t, err) require.Equal(t, "../../mediamtx.yml", confPath1) @@ -410,7 +410,7 @@ func TestSampleConfFile(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpf) - conf2, confPath2, err := Load(tmpf, nil) + conf2, confPath2, err := Load(tmpf, nil, nil) require.NoError(t, err) require.Equal(t, tmpf, confPath2) @@ -429,7 +429,7 @@ func TestConfOverrideDefaultSlices(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpf) - conf, _, err := Load(tmpf, nil) + conf, _, err := Load(tmpf, nil, nil) require.NoError(t, err) require.Equal(t, AuthInternalUsers{ diff --git a/internal/conf/path.go b/internal/conf/path.go index 0e1c6162f0d..36d4ec3a80c 100644 --- a/internal/conf/path.go +++ b/internal/conf/path.go @@ -11,6 +11,7 @@ import ( "time" "github.com/bluenviron/gortsplib/v4/pkg/base" + "github.com/bluenviron/mediamtx/internal/logger" ) var rePathName = regexp.MustCompile(`^[0-9a-zA-Z_\-/\.~]+$`) @@ -256,6 +257,7 @@ func (pconf *Path) validate( conf *Conf, name string, deprecatedCredentialsMode bool, + l logger.Writer, ) error { pconf.Name = name @@ -296,6 +298,8 @@ func (pconf *Path) validate( switch { case pconf.Source == "publisher": if pconf.DisablePublisherOverride != nil { + l.Log(logger.Warn, "parameter 'disablePublisherOverride' is deprecated "+ + "and has been replaced with 'overridePublisher'") pconf.OverridePublisher = !*pconf.DisablePublisherOverride } @@ -314,9 +318,11 @@ func (pconf *Path) validate( } if pconf.SourceProtocol != nil { + l.Log(logger.Warn, "parameter 'sourceProtocol' is deprecated and has been replaced with 'rtspTransport'") pconf.RTSPTransport = *pconf.SourceProtocol } if pconf.SourceAnyPortEnable != nil { + l.Log(logger.Warn, "parameter 'sourceAnyPortEnable' is deprecated and has been replaced with 'rtspAnyPort'") pconf.RTSPAnyPort = *pconf.SourceAnyPortEnable } @@ -464,6 +470,10 @@ func (pconf *Path) validate( // Record + if pconf.Playback != nil { + l.Log(logger.Warn, "parameter 'playback' is deprecated and has no effect") + } + if conf.Playback { if !strings.Contains(pconf.RecordPath, "%Y") || !strings.Contains(pconf.RecordPath, "%m") || diff --git a/internal/core/core.go b/internal/core/core.go index b7389450d7e..976173e4095 100644 --- a/internal/core/core.go +++ b/internal/core/core.go @@ -118,7 +118,9 @@ func New(args []string) (*Core, bool) { done: make(chan struct{}), } - p.conf, p.confPath, err = conf.Load(cli.Confpath, defaultConfPaths) + tempLogger, _ := logger.New(logger.Warn, []logger.Destination{logger.DestinationStdout}, "") + + p.conf, p.confPath, err = conf.Load(cli.Confpath, defaultConfPaths, tempLogger) if err != nil { fmt.Printf("ERR: %s\n", err) return nil, false @@ -175,7 +177,7 @@ outer: case <-confChanged: p.Log(logger.Info, "reloading configuration (file changed)") - newConf, _, err := conf.Load(p.confPath, nil) + newConf, _, err := conf.Load(p.confPath, nil, p.logger) if err != nil { p.Log(logger.Error, "%s", err) break outer