refactor: remove previously deprecated options (#2629)

This removes the deprecated logging, host, port, and tls options per our deprecation policy.
This commit is contained in:
James Elliott 2021-12-02 00:01:32 +11:00 committed by GitHub
parent ad8e844af6
commit 8a12af97ab
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 26 additions and 211 deletions

View File

@ -21,15 +21,17 @@ server:
```
## Policy
Our deprecation policy for configuration keys is 3 minor versions. For example if a configuration option is deprecated
in version 4.30.0, it will remain as a warning for 4.30.x, 4.31.x, and 4.32.x; then it will become a fatal error in
4.33.0+.
## Migrations
### 4.30.0
### 4.33.0
The options deprecated in version [4.30.0](#4300) have been fully removed as per our deprecation policy and warnings
logged for users.
### 4.30.0
The following changes occurred in 4.30.0:
|Previous Key |New Key |

View File

@ -32,7 +32,7 @@ func TestShouldErrorSecretNotExist(t *testing.T) {
assert.NoError(t, os.Setenv(DefaultEnvPrefix+"SESSION_REDIS_HIGH_AVAILABILITY_SENTINEL_PASSWORD_FILE", filepath.Join(dir, "redis-sentinel")))
assert.NoError(t, os.Setenv(DefaultEnvPrefix+"STORAGE_MYSQL_PASSWORD_FILE", filepath.Join(dir, "mysql")))
assert.NoError(t, os.Setenv(DefaultEnvPrefix+"STORAGE_POSTGRES_PASSWORD_FILE", filepath.Join(dir, "postgres")))
assert.NoError(t, os.Setenv(DefaultEnvPrefix+"TLS_KEY_FILE", filepath.Join(dir, "tls")))
assert.NoError(t, os.Setenv(DefaultEnvPrefix+"SERVER_TLS_KEY_FILE", filepath.Join(dir, "tls")))
assert.NoError(t, os.Setenv(DefaultEnvPrefix+"IDENTITY_PROVIDERS_OIDC_ISSUER_PRIVATE_KEY_FILE", filepath.Join(dir, "oidc-key")))
assert.NoError(t, os.Setenv(DefaultEnvPrefix+"IDENTITY_PROVIDERS_OIDC_HMAC_SECRET_FILE", filepath.Join(dir, "oidc-hmac")))
@ -61,7 +61,7 @@ func TestShouldErrorSecretNotExist(t *testing.T) {
assert.EqualError(t, errs[8], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "redis"), "session.redis.password", fmt.Sprintf(errFmt, filepath.Join(dir, "redis"))))
assert.EqualError(t, errs[9], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "redis-sentinel"), "session.redis.high_availability.sentinel_password", fmt.Sprintf(errFmt, filepath.Join(dir, "redis-sentinel"))))
assert.EqualError(t, errs[10], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "session"), "session.secret", fmt.Sprintf(errFmt, filepath.Join(dir, "session"))))
assert.EqualError(t, errs[11], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "tls"), "tls_key", fmt.Sprintf(errFmt, filepath.Join(dir, "tls"))))
assert.EqualError(t, errs[11], fmt.Sprintf(errFmtSecretIOIssue, filepath.Join(dir, "tls"), "server.tls.key", fmt.Sprintf(errFmt, filepath.Join(dir, "tls"))))
}
func TestLoadShouldReturnErrWithoutValidator(t *testing.T) {
@ -308,8 +308,8 @@ func testReset() {
testUnsetEnvName("SESSION_REDIS_HIGH_AVAILABILITY_SENTINEL_PASSWORD")
testUnsetEnvName("STORAGE_MYSQL_PASSWORD")
testUnsetEnvName("STORAGE_POSTGRES_PASSWORD")
testUnsetEnvName("TLS_KEY")
testUnsetEnvName("PORT")
testUnsetEnvName("SERVER_TLS_KEY")
testUnsetEnvName("SERVER_PORT")
testUnsetEnvName("IDENTITY_PROVIDERS_OIDC_ISSUER_PRIVATE_KEY")
testUnsetEnvName("IDENTITY_PROVIDERS_OIDC_HMAC_SECRET")
testUnsetEnvName("STORAGE_ENCRYPTION_KEY")

View File

@ -7,14 +7,6 @@ type Configuration struct {
JWTSecret string `koanf:"jwt_secret"`
DefaultRedirectionURL string `koanf:"default_redirection_url"`
Host string `koanf:"host"` // Deprecated: To be Removed. TODO: Remove in 4.33.0.
Port int `koanf:"port"` // Deprecated: To be Removed. TODO: Remove in 4.33.0.
TLSCert string `koanf:"tls_cert"` // Deprecated: To be Removed. TODO: Remove in 4.33.0.
TLSKey string `koanf:"tls_key"` // Deprecated: To be Removed. TODO: Remove in 4.33.0.
LogLevel string `koanf:"log_level"` // Deprecated: To be Removed. TODO: Remove in 4.33.0.
LogFormat string `koanf:"log_format"` // Deprecated: To be Removed. TODO: Remove in 4.33.0.
LogFilePath string `koanf:"log_file_path"` // Deprecated: To be Removed. TODO: Remove in 4.33.0.
Log LogConfiguration `koanf:"log"`
IdentityProviders IdentityProvidersConfiguration `koanf:"identity_providers"`
AuthenticationBackend AuthenticationBackendConfiguration `koanf:"authentication_backend"`

View File

@ -94,8 +94,16 @@ const (
// Error constants.
const (
errFmtDeprecatedConfigurationKey = "the %s configuration option is deprecated and will be " +
"removed in %s, please use %s instead"
/*
errFmtDeprecatedConfigurationKey = "the %s configuration option is deprecated and will be " +
"removed in %s, please use %s instead"
Uncomment for use when deprecating keys.
TODO: Create a method from within Koanf to automatically remap deprecated keys and produce warnings.
TODO (cont): The main consideration is making sure we do not overwrite the destination key name if it already exists.
*/
errFmtReplacedConfigurationKey = "invalid configuration key '%s' was replaced by '%s'"
errFmtLoggingLevelInvalid = "the log level '%s' is invalid, must be one of: %s"
@ -139,16 +147,6 @@ var ValidKeys = []string{
"log.file_path",
"log.keep_stdout",
// TODO: DEPRECATED START. Remove in 4.33.0.
"host",
"port",
"tls_key",
"tls_cert",
"log_level",
"log_format",
"log_file_path",
// TODO: DEPRECATED END. Remove in 4.33.0.
// Server Keys.
"server.host",
"server.port",
@ -328,8 +326,15 @@ var replacedKeys = map[string]string{
"authentication_backend.ldap.skip_verify": "authentication_backend.ldap.tls.skip_verify",
"authentication_backend.ldap.minimum_tls_version": "authentication_backend.ldap.tls.minimum_version",
"notifier.smtp.disable_verify_cert": "notifier.smtp.tls.skip_verify",
"logs_file_path": "log.file_path",
"logs_level": "log.level",
"logs_file_path": "log.file_path",
"log_level": "log.level",
"log_file_path": "log.file_path",
"log_format": "log.format",
"host": "server.host",
"port": "server.port",
"tls_key": "server.tls.key",
"tls_cert": "server.tls.certificate",
}
var specificErrorKeys = map[string]string{

View File

@ -10,8 +10,6 @@ import (
// ValidateLogging validates the logging configuration.
func ValidateLogging(configuration *schema.Configuration, validator *schema.StructValidator) {
applyDeprecatedLoggingConfiguration(configuration, validator) // TODO: DEPRECATED LINE. Remove in 4.33.0.
if configuration.Log.Level == "" {
configuration.Log.Level = schema.DefaultLoggingConfiguration.Level
}
@ -24,30 +22,3 @@ func ValidateLogging(configuration *schema.Configuration, validator *schema.Stru
validator.Push(fmt.Errorf(errFmtLoggingLevelInvalid, configuration.Log.Level, strings.Join(validLoggingLevels, ", ")))
}
}
// TODO: DEPRECATED FUNCTION. Remove in 4.33.0.
func applyDeprecatedLoggingConfiguration(configuration *schema.Configuration, validator *schema.StructValidator) {
if configuration.LogLevel != "" {
validator.PushWarning(fmt.Errorf(errFmtDeprecatedConfigurationKey, "log_level", "4.33.0", "log.level"))
if configuration.Log.Level == "" {
configuration.Log.Level = configuration.LogLevel
}
}
if configuration.LogFormat != "" {
validator.PushWarning(fmt.Errorf(errFmtDeprecatedConfigurationKey, "log_format", "4.33.0", "log.format"))
if configuration.Log.Format == "" {
configuration.Log.Format = configuration.LogFormat
}
}
if configuration.LogFilePath != "" {
validator.PushWarning(fmt.Errorf(errFmtDeprecatedConfigurationKey, "log_file_path", "4.33.0", "log.file_path"))
if configuration.Log.FilePath == "" {
configuration.Log.FilePath = configuration.LogFilePath
}
}
}

View File

@ -1,7 +1,6 @@
package validator
import (
"fmt"
"testing"
"github.com/stretchr/testify/assert"
@ -22,10 +21,6 @@ func TestShouldSetDefaultLoggingValues(t *testing.T) {
require.NotNil(t, config.Log.KeepStdout)
assert.Equal(t, "", config.LogLevel) // TODO: DEPRECATED TEST. Remove in 4.33.0.
assert.Equal(t, "", config.LogFormat) // TODO: DEPRECATED TEST. Remove in 4.33.0.
assert.Equal(t, "", config.LogFilePath) // TODO: DEPRECATED TEST. Remove in 4.33.0.
assert.Equal(t, "info", config.Log.Level)
assert.Equal(t, "text", config.Log.Format)
assert.Equal(t, "", config.Log.FilePath)
@ -47,65 +42,3 @@ func TestShouldRaiseErrorOnInvalidLoggingLevel(t *testing.T) {
assert.EqualError(t, validator.Errors()[0], "the log level 'TRACE' is invalid, must be one of: trace, debug, info, warn, error")
}
// TODO: DEPRECATED TEST. Remove in 4.33.0.
func TestShouldMigrateDeprecatedLoggingConfig(t *testing.T) {
config := &schema.Configuration{
LogLevel: "trace",
LogFormat: "json",
LogFilePath: "/a/b/c",
}
validator := schema.NewStructValidator()
ValidateLogging(config, validator)
assert.Len(t, validator.Errors(), 0)
require.Len(t, validator.Warnings(), 3)
require.NotNil(t, config.Log.KeepStdout)
assert.Equal(t, "trace", config.LogLevel)
assert.Equal(t, "json", config.LogFormat)
assert.Equal(t, "/a/b/c", config.LogFilePath)
assert.Equal(t, "trace", config.Log.Level)
assert.Equal(t, "json", config.Log.Format)
assert.Equal(t, "/a/b/c", config.Log.FilePath)
assert.EqualError(t, validator.Warnings()[0], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "log_level", "4.33.0", "log.level"))
assert.EqualError(t, validator.Warnings()[1], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "log_format", "4.33.0", "log.format"))
assert.EqualError(t, validator.Warnings()[2], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "log_file_path", "4.33.0", "log.file_path"))
}
func TestShouldRaiseErrorsAndNotOverwriteConfigurationWhenUsingDeprecatedLoggingConfig(t *testing.T) {
config := &schema.Configuration{
Log: schema.LogConfiguration{
Level: "info",
Format: "text",
FilePath: "/x/y/z",
KeepStdout: true,
},
LogLevel: "debug",
LogFormat: "json",
LogFilePath: "/a/b/c",
}
validator := schema.NewStructValidator()
ValidateLogging(config, validator)
require.NotNil(t, config.Log.KeepStdout)
assert.Equal(t, "info", config.Log.Level)
assert.Equal(t, "text", config.Log.Format)
assert.True(t, config.Log.KeepStdout)
assert.Equal(t, "/x/y/z", config.Log.FilePath)
assert.Len(t, validator.Errors(), 0)
require.Len(t, validator.Warnings(), 3)
assert.EqualError(t, validator.Warnings()[0], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "log_level", "4.33.0", "log.level"))
assert.EqualError(t, validator.Warnings()[1], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "log_format", "4.33.0", "log.format"))
assert.EqualError(t, validator.Warnings()[2], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "log_file_path", "4.33.0", "log.file_path"))
}

View File

@ -11,8 +11,6 @@ import (
// ValidateServer checks a server configuration is correct.
func ValidateServer(configuration *schema.Configuration, validator *schema.StructValidator) {
applyDeprecatedServerConfiguration(configuration, validator)
if configuration.Server.Host == "" {
configuration.Server.Host = schema.DefaultServerConfiguration.Host
}
@ -49,37 +47,3 @@ func ValidateServer(configuration *schema.Configuration, validator *schema.Struc
validator.Push(fmt.Errorf("server write buffer size must be above 0"))
}
}
func applyDeprecatedServerConfiguration(configuration *schema.Configuration, validator *schema.StructValidator) {
if configuration.Host != "" {
validator.PushWarning(fmt.Errorf(errFmtDeprecatedConfigurationKey, "host", "4.33.0", "server.host"))
if configuration.Server.Host == "" {
configuration.Server.Host = configuration.Host
}
}
if configuration.Port != 0 {
validator.PushWarning(fmt.Errorf(errFmtDeprecatedConfigurationKey, "port", "4.33.0", "server.port"))
if configuration.Server.Port == 0 {
configuration.Server.Port = configuration.Port
}
}
if configuration.TLSCert != "" {
validator.PushWarning(fmt.Errorf(errFmtDeprecatedConfigurationKey, "tls_cert", "4.33.0", "server.tls_cert"))
if configuration.Server.TLS.Certificate == "" {
configuration.Server.TLS.Certificate = configuration.TLSCert
}
}
if configuration.TLSKey != "" {
validator.PushWarning(fmt.Errorf(errFmtDeprecatedConfigurationKey, "tls_key", "4.33.0", "server.tls_key"))
if configuration.Server.TLS.Key == "" {
configuration.Server.TLS.Key = configuration.TLSKey
}
}
}

View File

@ -1,7 +1,6 @@
package validator
import (
"fmt"
"testing"
"github.com/stretchr/testify/assert"
@ -30,57 +29,6 @@ func TestShouldSetDefaultServerValues(t *testing.T) {
assert.Equal(t, schema.DefaultServerConfiguration.EnablePprof, config.Server.EnablePprof)
}
// TODO: DEPRECATED TEST. Remove in 4.33.0.
func TestShouldNotOverrideNewValuesWithDeprecatedValues(t *testing.T) {
validator := schema.NewStructValidator()
config := &schema.Configuration{Host: "123.0.0.1", Port: 9101, TLSKey: "/tmp/key.pem", TLSCert: "/tmp/cert.pem"}
config.Server.Host = "192.168.0.2"
config.Server.Port = 80
config.Server.TLS.Key = "/tmp/new/key.pem"
config.Server.TLS.Certificate = "/tmp/new/cert.pem"
ValidateServer(config, validator)
require.Len(t, validator.Errors(), 0)
require.Len(t, validator.Warnings(), 4)
assert.EqualError(t, validator.Warnings()[0], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "host", "4.33.0", "server.host"))
assert.EqualError(t, validator.Warnings()[1], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "port", "4.33.0", "server.port"))
assert.EqualError(t, validator.Warnings()[2], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "tls_cert", "4.33.0", "server.tls_cert"))
assert.EqualError(t, validator.Warnings()[3], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "tls_key", "4.33.0", "server.tls_key"))
assert.Equal(t, "192.168.0.2", config.Server.Host)
assert.Equal(t, 80, config.Server.Port)
assert.Equal(t, "/tmp/new/key.pem", config.Server.TLS.Key)
assert.Equal(t, "/tmp/new/cert.pem", config.Server.TLS.Certificate)
}
// TODO: DEPRECATED TEST. Remove in 4.33.0.
func TestShouldSetDeprecatedValues(t *testing.T) {
validator := schema.NewStructValidator()
config := &schema.Configuration{}
config.Host = "192.168.0.1"
config.Port = 80
config.TLSCert = "/tmp/cert.pem"
config.TLSKey = "/tmp/key.pem"
ValidateServer(config, validator)
assert.Len(t, validator.Errors(), 0)
require.Len(t, validator.Warnings(), 4)
assert.Equal(t, "192.168.0.1", config.Server.Host)
assert.Equal(t, 80, config.Server.Port)
assert.Equal(t, "/tmp/cert.pem", config.Server.TLS.Certificate)
assert.Equal(t, "/tmp/key.pem", config.Server.TLS.Key)
assert.EqualError(t, validator.Warnings()[0], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "host", "4.33.0", "server.host"))
assert.EqualError(t, validator.Warnings()[1], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "port", "4.33.0", "server.port"))
assert.EqualError(t, validator.Warnings()[2], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "tls_cert", "4.33.0", "server.tls_cert"))
assert.EqualError(t, validator.Warnings()[3], fmt.Sprintf(errFmtDeprecatedConfigurationKey, "tls_key", "4.33.0", "server.tls_key"))
}
func TestShouldSetDefaultConfig(t *testing.T) {
validator := schema.NewStructValidator()
config := &schema.Configuration{}