Skip to content

Commit

Permalink
warn users if deprecated parameters are being used
Browse files Browse the repository at this point in the history
  • Loading branch information
aler9 committed Dec 25, 2024
1 parent dfb792e commit a3c8cf5
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 28 deletions.
4 changes: 4 additions & 0 deletions .github/DISCUSSION_TEMPLATE/questions.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
12 changes: 6 additions & 6 deletions internal/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/certloader/certloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestCertReload(t *testing.T) {
require.NoError(t, err)
defer os.Remove(serverKeyPath)

loader, err := New(serverCertPath, serverKeyPath, test.NilLogger)
loader, err := New(serverCertPath, serverKeyPath, nil)
require.NoError(t, err)
defer loader.Close()

Expand Down
54 changes: 50 additions & 4 deletions internal/conf/conf.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
}
Expand All @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down
28 changes: 14 additions & 14 deletions internal/conf/conf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
}()

Expand All @@ -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)
}()

Expand All @@ -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)
}()
}
Expand All @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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)

Expand All @@ -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{
Expand Down Expand Up @@ -380,37 +380,37 @@ 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)
})
}
}

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)

require.Equal(t, conf1, conf2)
}()

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)

tmpf, err := createTempFile([]byte("paths:\n all_others:"))
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)

Expand All @@ -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{
Expand Down
Loading

0 comments on commit a3c8cf5

Please sign in to comment.