diff --git a/client-lib/go/client/jvs_client_test.go b/client-lib/go/client/jvs_client_test.go index 2c47ec38..4f736031 100644 --- a/client-lib/go/client/jvs_client_test.go +++ b/client-lib/go/client/jvs_client_test.go @@ -91,7 +91,7 @@ func TestValidateJWT(t *testing.T) { }) client, err := NewJVSClient(ctx, &JVSConfig{ - Version: 1, + Version: "1", JVSEndpoint: svr.URL + path, CacheTimeout: 5 * time.Minute, }) diff --git a/client-lib/go/client/jvs_config.go b/client-lib/go/client/jvs_config.go index 68d6ca27..d686b36b 100644 --- a/client-lib/go/client/jvs_config.go +++ b/client-lib/go/client/jvs_config.go @@ -19,56 +19,42 @@ import ( "fmt" "time" + "github.com/abcxyz/jvs/pkg/config" "github.com/hashicorp/go-multierror" "github.com/sethvargo/go-envconfig" "gopkg.in/yaml.v2" ) -const ( - // Version default for config. - Version = 1 - CacheTimeoutDefault = 5 * time.Minute -) +var versions = config.NewVersionList("1") // JVSConfig is the jvs client configuration. type JVSConfig struct { // Version is the version of the config. - Version uint8 `yaml:"version,omitempty" env:"VERSION,overwrite"` + Version string `yaml:"version,omitempty" env:"VERSION,overwrite,default=1"` // JVS Endpoint. Expected to be fully qualified, including port. ex. http://127.0.0.1:8080 JVSEndpoint string `yaml:"endpoint,omitempty" env:"ENDPOINT,overwrite"` // CacheTimeout is the duration that keys stay in cache before being revoked. - CacheTimeout time.Duration `yaml:"cache_timeout" env:"CACHE_TIMEOUT,overwrite"` + CacheTimeout time.Duration `yaml:"cache_timeout" env:"CACHE_TIMEOUT,overwrite,default=5m"` } // Validate checks if the config is valid. func (cfg *JVSConfig) Validate() error { - cfg.SetDefault() var err *multierror.Error - if cfg.Version != Version { - err = multierror.Append(err, fmt.Errorf("unexpected Version %d want %d", cfg.Version, Version)) + if !versions.Contains(cfg.Version) { + err = multierror.Append(err, fmt.Errorf("version %q is invalid, valid versions are: %q", + cfg.Version, versions.List())) } if cfg.JVSEndpoint == "" { err = multierror.Append(err, fmt.Errorf("endpoint must be set")) } if cfg.CacheTimeout <= 0 { - err = multierror.Append(err, fmt.Errorf("cache timeout invalid: %d", cfg.CacheTimeout)) + err = multierror.Append(err, fmt.Errorf("cache timeout must be a positive duration, got %q", cfg.CacheTimeout)) } return err.ErrorOrNil() } -// SetDefault sets defaults for the config. -func (cfg *JVSConfig) SetDefault() { - if cfg.Version == 0 { - cfg.Version = Version - } - if cfg.CacheTimeout == 0 { - // env config lib doesn't gracefully handle env overrides with defaults, have to set manually. - cfg.CacheTimeout = CacheTimeoutDefault - } -} - // LoadJVSConfig calls the necessary methods to load in config using the OsLookuper which finds env variables specified on the host. func LoadJVSConfig(ctx context.Context, b []byte) (*JVSConfig, error) { return loadJVSConfigFromLookuper(ctx, b, envconfig.OsLookuper()) diff --git a/client-lib/go/client/jvs_config_test.go b/client-lib/go/client/jvs_config_test.go index 9bc717f2..46934434 100644 --- a/client-lib/go/client/jvs_config_test.go +++ b/client-lib/go/client/jvs_config_test.go @@ -44,7 +44,7 @@ endpoint: example.com:8080 cache_timeout: 1m `, wantConfig: &JVSConfig{ - Version: 1, + Version: "1", JVSEndpoint: "example.com:8080", CacheTimeout: time.Minute, }, @@ -55,7 +55,7 @@ cache_timeout: 1m endpoint: example.com:8080 `, wantConfig: &JVSConfig{ - Version: 1, + Version: "1", JVSEndpoint: "example.com:8080", CacheTimeout: 5 * time.Minute, }, @@ -68,7 +68,7 @@ endpoint: example.com:8080 cache_timeout: 1m `, wantConfig: nil, - wantErr: "failed validating config: 1 error occurred:\n\t* unexpected Version 255 want 1\n\n", + wantErr: `version "255" is invalid, valid versions are:`, }, { name: "test_invalid_timeout", @@ -78,7 +78,7 @@ endpoint: example.com:8080 cache_timeout: -1m `, wantConfig: nil, - wantErr: "failed validating config: 1 error occurred:\n\t* cache timeout invalid: -60000000000\n\n", + wantErr: `cache timeout must be a positive duration, got "-1m0s"`, }, { name: "all_values_specified_env_override", @@ -93,7 +93,7 @@ cache_timeout: 1m "JVS_CACHE_TIMEOUT": "2m", }, wantConfig: &JVSConfig{ - Version: 1, + Version: "1", JVSEndpoint: "other.net:443", CacheTimeout: 2 * time.Minute, }, diff --git a/client-lib/go/go.mod b/client-lib/go/go.mod index e476b4e7..451265c0 100644 --- a/client-lib/go/go.mod +++ b/client-lib/go/go.mod @@ -3,12 +3,12 @@ module github.com/abcxyz/jvs/client-lib/go go 1.18 require ( - github.com/abcxyz/jvs v0.0.0-20220520175656-c98ac822412a + github.com/abcxyz/jvs v0.0.0-00010101000000-000000000000 github.com/abcxyz/pkg v0.0.0-20220627210229-493bf5fec862 github.com/google/go-cmp v0.5.8 github.com/hashicorp/go-multierror v1.1.1 github.com/lestrrat-go/jwx/v2 v2.0.3 - github.com/sethvargo/go-envconfig v0.6.2 + github.com/sethvargo/go-envconfig v0.8.0 gopkg.in/yaml.v2 v2.4.0 ) diff --git a/client-lib/go/go.sum b/client-lib/go/go.sum index 130478f2..58f116ad 100644 --- a/client-lib/go/go.sum +++ b/client-lib/go/go.sum @@ -226,8 +226,8 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= -github.com/sethvargo/go-envconfig v0.6.2 h1:982sW5Fc4zj7GWCGbt+tfjvl5fOUm/zMZXLT/hBBfr0= -github.com/sethvargo/go-envconfig v0.6.2/go.mod h1:00S1FAhRUuTNJazWBWcJGvEHOM+NO6DhoRMAOX7FY5o= +github.com/sethvargo/go-envconfig v0.8.0 h1:AcmdAewSFAc7pQ1Ghz+vhZkilUtxX559QlDuLLiSkdI= +github.com/sethvargo/go-envconfig v0.8.0/go.mod h1:Iz1Gy1Sf3T64TQlJSvee81qDhf7YIlt8GMUX6yyNFs0= github.com/sethvargo/go-gcpkms v0.1.0 h1:pyjDLqLwpk9pMjDSTilPpaUjgP1AfSjX9WGzitZwGUY= github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/go.mod b/go.mod index bffe29df..be47f4de 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/google/uuid v1.3.0 github.com/hashicorp/go-multierror v1.1.1 github.com/lestrrat-go/jwx/v2 v2.0.3 - github.com/sethvargo/go-envconfig v0.6.2 + github.com/sethvargo/go-envconfig v0.8.0 github.com/sethvargo/go-gcpkms v0.1.0 github.com/sethvargo/go-retry v0.2.3 github.com/spf13/cobra v1.4.0 diff --git a/go.sum b/go.sum index fc9f8a07..1a03757d 100644 --- a/go.sum +++ b/go.sum @@ -260,8 +260,8 @@ github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6L github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/sethvargo/go-envconfig v0.6.2 h1:982sW5Fc4zj7GWCGbt+tfjvl5fOUm/zMZXLT/hBBfr0= -github.com/sethvargo/go-envconfig v0.6.2/go.mod h1:00S1FAhRUuTNJazWBWcJGvEHOM+NO6DhoRMAOX7FY5o= +github.com/sethvargo/go-envconfig v0.8.0 h1:AcmdAewSFAc7pQ1Ghz+vhZkilUtxX559QlDuLLiSkdI= +github.com/sethvargo/go-envconfig v0.8.0/go.mod h1:Iz1Gy1Sf3T64TQlJSvee81qDhf7YIlt8GMUX6yyNFs0= github.com/sethvargo/go-gcpkms v0.1.0 h1:pyjDLqLwpk9pMjDSTilPpaUjgP1AfSjX9WGzitZwGUY= github.com/sethvargo/go-gcpkms v0.1.0/go.mod h1:33BuvqUjsYk0bpMgn+WCclCYtMLOyaqtn5j0fCo4vvk= github.com/sethvargo/go-retry v0.2.3 h1:oYlgvIvsju3jNbottWABtbnoLC+GDtLdBHxKWxQm/iU= diff --git a/pkg/cli/root_test.go b/pkg/cli/root_test.go index ab1e5c3f..9e0c4aa5 100644 --- a/pkg/cli/root_test.go +++ b/pkg/cli/root_test.go @@ -40,7 +40,7 @@ func TestInitCfg(t *testing.T) { initCfg() wantCfg := &config.CLIConfig{ - Version: 1, + Version: "1", Server: "https://example.com", Authentication: &config.CLIAuthentication{}, } diff --git a/pkg/config/cli_config.go b/pkg/config/cli_config.go index 58772b32..b8d6ba5d 100644 --- a/pkg/config/cli_config.go +++ b/pkg/config/cli_config.go @@ -20,17 +20,15 @@ import ( "github.com/hashicorp/go-multierror" ) -const ( - // DefaultCLIConfigVersion is the default CLI config version. - DefaultCLIConfigVersion = 1 -) +// CLIConfigVersions is the list of allowed versions for the CLIConfig. +var CLIConfigVersions = NewVersionList("1") type CLIConfig struct { // Version is the version of the config. - Version uint8 `yaml:"version,omitempty" env:"VERSION,overwrite"` + Version string `yaml:"version,omitempty"` // Server is the JVS server address. - Server string `yaml:"server,omitempty" env:"SERVER,overwrite"` + Server string `yaml:"server,omitempty"` // Authentication is the authentication config. Authentication *CLIAuthentication `yaml:"authentication,omitempty"` @@ -47,6 +45,12 @@ func (cfg *CLIConfig) Validate() error { cfg.SetDefault() var err *multierror.Error + + if !CLIConfigVersions.Contains(cfg.Version) { + err = multierror.Append(err, fmt.Errorf("version %q is invalid, valid versions are: %q", + cfg.Version, CLIConfigVersions.List())) + } + if cfg.Server == "" { err = multierror.Append(err, fmt.Errorf("missing JVS server address")) } @@ -56,8 +60,8 @@ func (cfg *CLIConfig) Validate() error { // SetDefault sets default for the config. func (cfg *CLIConfig) SetDefault() { - if cfg.Version == 0 { - cfg.Version = DefaultCLIConfigVersion + if cfg.Version == "" { + cfg.Version = "1" } if cfg.Authentication == nil { cfg.Authentication = &CLIAuthentication{} diff --git a/pkg/config/cli_config_test.go b/pkg/config/cli_config_test.go index d6368e94..8e414deb 100644 --- a/pkg/config/cli_config_test.go +++ b/pkg/config/cli_config_test.go @@ -21,21 +21,31 @@ import ( "github.com/google/go-cmp/cmp" ) -func TestValidateCLIConfig(t *testing.T) { +func TestCLIConfig_Validate(t *testing.T) { t.Parallel() tests := []struct { name string cfg *CLIConfig wantErr string - }{{ - name: "no_error", - cfg: &CLIConfig{Server: "example.com"}, - }, { - name: "missing_server_error", - cfg: &CLIConfig{}, - wantErr: "missing JVS server address", - }} + }{ + { + name: "no_error", + cfg: &CLIConfig{Server: "example.com"}, + }, + { + name: "bad_version", + cfg: &CLIConfig{ + Version: "255", + }, + wantErr: "missing JVS server address", + }, + { + name: "missing_server_error", + cfg: &CLIConfig{}, + wantErr: "missing JVS server address", + }, + } for _, tc := range tests { tc := tc @@ -49,7 +59,7 @@ func TestValidateCLIConfig(t *testing.T) { } } -func TestSetDefault(t *testing.T) { +func TestCLIConfig_SetDefault(t *testing.T) { t.Parallel() tests := []struct { @@ -60,15 +70,17 @@ func TestSetDefault(t *testing.T) { name: "default_empty_authentication", cfg: &CLIConfig{}, wantCfg: &CLIConfig{ - Version: 1, + Version: "1", Authentication: &CLIAuthentication{}, }, }} for _, tc := range tests { tc := tc + t.Run(tc.name, func(t *testing.T) { t.Parallel() + tc.cfg.SetDefault() if diff := cmp.Diff(tc.wantCfg, tc.cfg); diff != "" { t.Errorf("config with defaults (-want,+got):\n%s", diff) diff --git a/pkg/config/crypto_config.go b/pkg/config/crypto_config.go index d151ca57..35a02e43 100644 --- a/pkg/config/crypto_config.go +++ b/pkg/config/crypto_config.go @@ -25,15 +25,13 @@ import ( "gopkg.in/yaml.v2" ) -const ( - // Version default for config. - Version = 1 -) +// CryptoConfigVersions is the list of allowed versions for the CryptoConfig. +var CryptoConfigVersions = NewVersionList("1") // CryptoConfig is the full jvs config. type CryptoConfig struct { // Version is the version of the config. - Version uint8 `yaml:"version,omitempty" env:"VERSION,overwrite"` + Version string `yaml:"version,omitempty" env:"VERSION,overwrite,default=1"` // -- Crypto variables -- // KeyTTL is the length of time that we expect a key to be valid for. @@ -54,41 +52,37 @@ type CryptoConfig struct { // Validate checks if the config is valid. func (cfg *CryptoConfig) Validate() error { - cfg.SetDefault() - var err *multierror.Error - if cfg.Version != Version { - err = multierror.Append(err, fmt.Errorf("unexpected Version %d want %d", cfg.Version, Version)) + + if !CryptoConfigVersions.Contains(cfg.Version) { + err = multierror.Append(err, fmt.Errorf("version %q is invalid, valid versions are: %q", + cfg.Version, CryptoConfigVersions.List())) } if cfg.KeyTTL <= 0 { - err = multierror.Append(err, fmt.Errorf("key ttl is invalid: %v", cfg.KeyTTL)) + err = multierror.Append(err, fmt.Errorf("key ttl must be a positive duration, got %q", cfg.KeyTTL)) } if cfg.GracePeriod <= 0 { - err = multierror.Append(err, fmt.Errorf("grace period is invalid: %v", cfg.GracePeriod)) + err = multierror.Append(err, fmt.Errorf("grace period must be a positive duration, got %q", cfg.GracePeriod)) } if cfg.DisabledPeriod <= 0 { - err = multierror.Append(err, fmt.Errorf("disabled period is invalid: %v", cfg.DisabledPeriod)) + err = multierror.Append(err, fmt.Errorf("disabled period must be a positive duration, got %q", cfg.DisabledPeriod)) } - // Propagation delay must be lower than grace period. - if cfg.PropagationDelay <= 0 || cfg.PropagationDelay > cfg.GracePeriod { - err = multierror.Append(err, fmt.Errorf("propagation delay is invalid: %v", cfg.PropagationDelay)) + // Propagation delay must be positive but less than than grace period. + if cfg.PropagationDelay <= 0 { + err = multierror.Append(err, fmt.Errorf("propagation delay must be a positive duration, got %q", cfg.PropagationDelay)) + } + if cfg.PropagationDelay > cfg.GracePeriod { + err = multierror.Append(err, fmt.Errorf("propagation delay %q must be less than grace period %q", + cfg.PropagationDelay, cfg.GracePeriod)) } return err.ErrorOrNil() } -// SetDefault sets default for the config. -func (cfg *CryptoConfig) SetDefault() { - // TODO: set defaults for other fields if necessary. - if cfg.Version == 0 { - cfg.Version = Version - } -} - // GetRotationAge gets the duration after a key has been created that a new key should be created. func (cfg *CryptoConfig) RotationAge() time.Duration { return cfg.KeyTTL - cfg.GracePeriod diff --git a/pkg/config/crypto_config_test.go b/pkg/config/crypto_config_test.go index 4506db9f..e2f2c109 100644 --- a/pkg/config/crypto_config_test.go +++ b/pkg/config/crypto_config_test.go @@ -25,6 +25,25 @@ import ( "github.com/sethvargo/go-envconfig" ) +func TestCryptoConfig_Defaults(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + var cryptoConfig CryptoConfig + if err := envconfig.ProcessWith(ctx, &cryptoConfig, envconfig.MapLookuper(nil)); err != nil { + t.Fatal(err) + } + + want := &CryptoConfig{ + Version: "1", + } + + if diff := cmp.Diff(want, &cryptoConfig); diff != "" { + t.Errorf("config with defaults (-want, +got):\n%s", diff) + } +} + func TestLoadCryptoConfig(t *testing.T) { t.Parallel() ctx := context.Background() @@ -46,7 +65,7 @@ disabled_period: 720h # 30 days propagation_delay: 1h `, wantConfig: &CryptoConfig{ - Version: 1, + Version: "1", KeyTTL: 720 * time.Hour, // 30 days GracePeriod: 2 * time.Hour, // 2 hours DisabledPeriod: 720 * time.Hour, // 30 days @@ -63,7 +82,7 @@ disabled_period: 720h # 30 days propagation_delay: 1h `, wantConfig: &CryptoConfig{ - Version: 1, + Version: "1", KeyTTL: 720 * time.Hour, // 30 days GracePeriod: 2 * time.Hour, // 2 hours DisabledPeriod: 720 * time.Hour, // 30 days @@ -74,54 +93,75 @@ propagation_delay: 1h name: "test_wrong_version", cfg: ` version: 255 -key_ttl: 720h # 30 days -grace_period: 2h -disabled_period: 720h # 30 days -propagation_delay: 1h `, wantConfig: nil, - wantErr: "failed validating config: 1 error occurred:\n\t* unexpected Version 255 want 1\n\n", + wantErr: `version "255" is invalid, valid versions are:`, + }, + { + name: "test_empty_key_ttl", + cfg: ``, + wantConfig: nil, + wantErr: `key ttl must be a positive duration, got "0s"`, + }, + { + name: "test_empty_grace_period", + cfg: ``, + wantConfig: nil, + wantErr: `grace period must be a positive duration, got "0s"`, + }, + { + name: "test_empty_disabled_period", + cfg: ``, + wantConfig: nil, + wantErr: `disabled period must be a positive duration, got "0s"`, + }, + { + name: "test_empty_propagation_delay", + cfg: ``, + wantConfig: nil, + wantErr: `propagation delay must be a positive duration, got "0s"`, }, + { - name: "test_invalid_propagation_delay", + name: "test_negative_key_ttl", cfg: ` -version: 1 -key_ttl: 720h # 30 days -grace_period: 2h -disabled_period: 720h # 30 days -propagation_delay: 3h +key_ttl: -720h `, wantConfig: nil, - wantErr: "failed validating config: 1 error occurred:\n\t* propagation delay is invalid: 3h0m0s\n\n", + wantErr: `key ttl must be a positive duration, got "-720h0m0s"`, }, { - name: "test_empty_ttl", + name: "test_negative_grace_period", cfg: ` -version: 1 -grace_period: 2h -disabled_period: 720h # 30 days -propagation_delay: 1h +grace_period: -2h `, wantConfig: nil, - wantErr: "failed validating config: 1 error occurred:\n\t* key ttl is invalid: 0s\n\n", + wantErr: `grace period must be a positive duration, got "-2h0m0s"`, }, { - name: "test_empty", - cfg: "", + name: "test_negative_disabled_period", + cfg: ` +disabled_period: -720h +`, wantConfig: nil, - wantErr: "failed validating config: 4 errors occurred:\n\t* key ttl is invalid: 0s\n\t* grace period is invalid: 0s\n\t* disabled period is invalid: 0s\n\t* propagation delay is invalid: 0s\n\n", + wantErr: `disabled period must be a positive duration, got "-720h0m0s"`, }, { - name: "test_negative", + name: "test_negative_propagation_delay", cfg: ` -version: 1 -key_ttl: -720h -grace_period: -2h -disabled_period: -720h propagation_delay: -1h `, wantConfig: nil, - wantErr: "failed validating config: 4 errors occurred:\n\t* key ttl is invalid: -720h0m0s\n\t* grace period is invalid: -2h0m0s\n\t* disabled period is invalid: -720h0m0s\n\t* propagation delay is invalid: -1h0m0s\n\n", + wantErr: `propagation delay must be a positive duration, got "-1h0m0s"`, + }, + { + name: "test_propagation_delay_greater_than_grace_period", + cfg: ` +grace_period: 1h +propagation_delay: 2h +`, + wantConfig: nil, + wantErr: `propagation delay "2h0m0s" must be less than grace period "1h0m0s"`, }, { name: "all_values_specified_env_override", @@ -137,7 +177,7 @@ propagation_delay: 1h "JVS_GRACE_PERIOD": "4h", }, wantConfig: &CryptoConfig{ - Version: 1, + Version: "1", KeyTTL: 1080 * time.Hour, // 45 days GracePeriod: 4 * time.Hour, // 4 hours DisabledPeriod: 720 * time.Hour, // 30 days @@ -154,7 +194,7 @@ propagation_delay: 1h "JVS_PROPAGATION_DELAY": "1h", }, wantConfig: &CryptoConfig{ - Version: 1, + Version: "1", KeyTTL: 1080 * time.Hour, // 45 days GracePeriod: 4 * time.Hour, // 4 hours DisabledPeriod: 1080 * time.Hour, // 45 days @@ -165,8 +205,10 @@ propagation_delay: 1h for _, tc := range tests { tc := tc + t.Run(tc.name, func(t *testing.T) { t.Parallel() + lookuper := envconfig.MapLookuper(tc.envs) content := bytes.NewBufferString(tc.cfg).Bytes() gotConfig, err := loadCryptoConfigFromLookuper(ctx, content, lookuper) @@ -182,8 +224,9 @@ propagation_delay: 1h func TestRotationAge(t *testing.T) { t.Parallel() + cfg := &CryptoConfig{ - Version: 1, + Version: "1", KeyTTL: 720 * time.Hour, // 30 days GracePeriod: 2 * time.Hour, // 2 hours DisabledPeriod: 720 * time.Hour, // 30 days @@ -199,8 +242,9 @@ func TestRotationAge(t *testing.T) { func TestDestroyAge(t *testing.T) { t.Parallel() + cfg := &CryptoConfig{ - Version: 1, + Version: "1", KeyTTL: 720 * time.Hour, // 30 days GracePeriod: 2 * time.Hour, // 2 hours DisabledPeriod: 720 * time.Hour, // 30 days diff --git a/pkg/config/justification_config.go b/pkg/config/justification_config.go index 25648da5..79e72c1f 100644 --- a/pkg/config/justification_config.go +++ b/pkg/config/justification_config.go @@ -25,62 +25,45 @@ import ( "gopkg.in/yaml.v2" ) -const ( - // Version default for config. - CurrentVersion = 1 - SignerCacheTimeoutDefault = 5 * time.Minute - IssuerDefault = "jvs.abcxyz.dev" -) +// JustificationConfigVersions is the list of allowed versions for the +// JustificationConfig. +var JustificationConfigVersions = NewVersionList("1") // JustificationConfig is the full jvs config. type JustificationConfig struct { // Version is the version of the config. - Version uint8 `yaml:"version,omitempty" env:"VERSION,overwrite"` + Version string `yaml:"version,omitempty" env:"VERSION,overwrite,default=1"` // Service configuration. - Port string `yaml:"port,omitempty" env:"PORT,overwrite"` + Port string `yaml:"port,omitempty" env:"PORT,overwrite,default=8080"` // KeyName format: `projects/*/locations/*/keyRings/*/cryptoKeys/*` // https://pkg.go.dev/google.golang.org/genproto/googleapis/cloud/kms/v1#CryptoKey KeyName string `yaml:"key,omitempty" env:"KEY,overwrite"` // SignerCacheTimeout is the duration that keys stay in cache before being revoked. - SignerCacheTimeout time.Duration `yaml:"signer_cache_timeout" env:"SIGNER_CACHE_TIMEOUT,overwrite"` + SignerCacheTimeout time.Duration `yaml:"signer_cache_timeout" env:"SIGNER_CACHE_TIMEOUT,overwrite,default=5m"` // Issuer will be used to set the issuer field when signing JWTs - Issuer string `yaml:"issuer" env:"ISSUER,overwrite"` + Issuer string `yaml:"issuer" env:"ISSUER,overwrite,default=jvs.abcxyz.dev"` } // Validate checks if the config is valid. func (cfg *JustificationConfig) Validate() error { - cfg.SetDefault() var err *multierror.Error - if cfg.Version != CurrentVersion { - err = multierror.Append(err, fmt.Errorf("unexpected Version %d want %d", cfg.Version, CurrentVersion)) + + if !JustificationConfigVersions.Contains(cfg.Version) { + err = multierror.Append(err, fmt.Errorf("version %q is invalid, valid versions are: %q", + cfg.Version, JustificationConfigVersions.List())) } + if cfg.SignerCacheTimeout <= 0 { - err = multierror.Append(err, fmt.Errorf("cache timeout invalid: %d", cfg.SignerCacheTimeout)) + err = multierror.Append(err, fmt.Errorf("cache timeout must be a positive duration, got %s", + cfg.SignerCacheTimeout)) } return err.ErrorOrNil() } -// SetDefault sets default for the config. -func (cfg *JustificationConfig) SetDefault() { - if cfg.Port == "" { - cfg.Port = "8080" - } - if cfg.Version == 0 { - cfg.Version = Version - } - if cfg.SignerCacheTimeout == 0 { - // env config lib doesn't gracefully handle env overrides with defaults, have to set manually. - cfg.SignerCacheTimeout = SignerCacheTimeoutDefault - } - if cfg.Issuer == "" { - cfg.Issuer = IssuerDefault - } -} - // LoadJustificationConfig calls the necessary methods to load in config using the OsLookuper which finds env variables specified on the host. func LoadJustificationConfig(ctx context.Context, b []byte) (*JustificationConfig, error) { return loadJustificationConfigFromLookuper(ctx, b, envconfig.OsLookuper()) diff --git a/pkg/config/justification_config_test.go b/pkg/config/justification_config_test.go index b6380204..aaaa020a 100644 --- a/pkg/config/justification_config_test.go +++ b/pkg/config/justification_config_test.go @@ -25,6 +25,28 @@ import ( "github.com/sethvargo/go-envconfig" ) +func TestJustificationConfig_Defaults(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + var justificationConfig JustificationConfig + if err := envconfig.ProcessWith(ctx, &justificationConfig, envconfig.MapLookuper(nil)); err != nil { + t.Fatal(err) + } + + want := &JustificationConfig{ + Version: "1", + Port: "8080", + SignerCacheTimeout: 5 * time.Minute, + Issuer: "jvs.abcxyz.dev", + } + + if diff := cmp.Diff(want, &justificationConfig); diff != "" { + t.Errorf("config with defaults (-want, +got):\n%s", diff) + } +} + func TestLoadJustificationConfig(t *testing.T) { t.Parallel() ctx := context.Background() @@ -46,7 +68,7 @@ issuer: jvs `, wantConfig: &JustificationConfig{ Port: "123", - Version: 1, + Version: "1", SignerCacheTimeout: 1 * time.Minute, Issuer: "jvs", }, @@ -56,7 +78,7 @@ issuer: jvs cfg: ``, wantConfig: &JustificationConfig{ Port: "8080", - Version: 1, + Version: "1", SignerCacheTimeout: 5 * time.Minute, Issuer: "jvs.abcxyz.dev", }, @@ -67,7 +89,7 @@ issuer: jvs version: 255 `, wantConfig: nil, - wantErr: "failed validating config: 1 error occurred:\n\t* unexpected Version 255 want 1\n\n", + wantErr: `version "255" is invalid, valid versions are:`, }, { name: "test_invalid_signer_cache_timeout", @@ -75,7 +97,7 @@ version: 255 signer_cache_timeout: -1m `, wantConfig: nil, - wantErr: "failed validating config: 1 error occurred:\n\t* cache timeout invalid: -60000000000\n\n", + wantErr: `cache timeout must be a positive duration, got -1m0s`, }, { name: "all_values_specified_env_override", @@ -92,7 +114,7 @@ issuer: jvs "JVS_ISSUER": "other", }, wantConfig: &JustificationConfig{ - Version: 1, + Version: "1", Port: "tcp", SignerCacheTimeout: 2 * time.Minute, Issuer: "other", @@ -102,8 +124,10 @@ issuer: jvs for _, tc := range tests { tc := tc + t.Run(tc.name, func(t *testing.T) { t.Parallel() + lookuper := envconfig.MapLookuper(tc.envs) content := bytes.NewBufferString(tc.cfg).Bytes() gotConfig, err := loadJustificationConfigFromLookuper(ctx, content, lookuper) diff --git a/pkg/config/versions.go b/pkg/config/versions.go new file mode 100644 index 00000000..9f87e392 --- /dev/null +++ b/pkg/config/versions.go @@ -0,0 +1,61 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import "sort" + +// VersionList is a set of allowed versions. Create with NewVersionList. +type VersionList struct { + m map[string]struct{} +} + +// NewVersionList creates an efficient list of allowed version strings and +// exposes functions for efficiently querying membership. +func NewVersionList(versions ...string) *VersionList { + m := make(map[string]struct{}, len(versions)) + for _, v := range versions { + m[v] = struct{}{} + } + + return &VersionList{ + m: m, + } +} + +// Contains returns true if the given version string is an allowed version in +// the list, or false otherwise. +func (vl *VersionList) Contains(version string) bool { + if vl == nil || vl.m == nil { + return false + } + + _, ok := vl.m[version] + return ok +} + +// List returns a copy of the list of allowed versions, usually for displaying +// in an error message. +func (vl *VersionList) List() []string { + if vl == nil || vl.m == nil { + return []string{} + } + + l := make([]string, 0, len(vl.m)) + for key := range vl.m { + l = append(l, key) + } + sort.Strings(l) + return l +} diff --git a/pkg/config/versions_test.go b/pkg/config/versions_test.go new file mode 100644 index 00000000..94a8c0f6 --- /dev/null +++ b/pkg/config/versions_test.go @@ -0,0 +1,193 @@ +// Copyright 2022 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package config + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestNewVersionList(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + input []string + exp *VersionList + }{ + { + name: "nil", + input: nil, + exp: &VersionList{ + m: map[string]struct{}{}, + }, + }, + { + name: "empty", + input: []string{""}, + exp: &VersionList{ + m: map[string]struct{}{ + "": {}, + }, + }, + }, + { + name: "single", + input: []string{"v1alpha"}, + exp: &VersionList{ + m: map[string]struct{}{ + "v1alpha": {}, + }, + }, + }, + { + name: "multiple", + input: []string{"v1alpha", "v1"}, + exp: &VersionList{ + m: map[string]struct{}{ + "v1alpha": {}, + "v1": {}, + }, + }, + }, + } + + for _, tc := range cases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := NewVersionList(tc.input...) + if diff := cmp.Diff(got, tc.exp, cmp.AllowUnexported(VersionList{})); diff != "" { + t.Errorf("Config unexpected diff (-want, +got):\n%s", diff) + } + }) + } +} + +func TestVersionList_Contains(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + versionList *VersionList + input string + exp bool + }{ + { + name: "nil", + versionList: nil, + input: "v1", + exp: false, + }, + { + name: "nil_map", + versionList: &VersionList{ + m: nil, + }, + input: "v1", + exp: false, + }, + { + name: "exists", + versionList: &VersionList{ + m: map[string]struct{}{ + "v1alpha": {}, + "v1": {}, + }, + }, + input: "v1", + exp: true, + }, + { + name: "not_exists", + versionList: &VersionList{ + m: map[string]struct{}{ + "v1alpha": {}, + "v1": {}, + }, + }, + input: "v2", + exp: false, + }, + } + + for _, tc := range cases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + if got, want := tc.versionList.Contains(tc.input), tc.exp; got != want { + t.Errorf("expected %t to be %t", got, want) + } + }) + } +} + +func TestVersionList_List(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + versionList *VersionList + exp []string + }{ + { + name: "nil", + versionList: nil, + exp: []string{}, + }, + { + name: "nil_map", + versionList: &VersionList{ + m: nil, + }, + exp: []string{}, + }, + { + name: "sorts", + versionList: &VersionList{ + m: map[string]struct{}{ + "v1alpha": {}, + "v1": {}, + "v2": {}, + "_v1": {}, + }, + }, + exp: []string{ + "_v1", + "v1", + "v1alpha", + "v2", + }, + }, + } + + for _, tc := range cases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := tc.versionList.List() + if diff := cmp.Diff(got, tc.exp); diff != "" { + t.Errorf("Config unexpected diff (-want, +got):\n%s", diff) + } + }) + } +} diff --git a/pkg/justification/processor_test.go b/pkg/justification/processor_test.go index 3731686e..107c98d6 100644 --- a/pkg/justification/processor_test.go +++ b/pkg/justification/processor_test.go @@ -135,10 +135,10 @@ func TestCreateToken(t *testing.T) { } processor := NewProcessor(c, &config.JustificationConfig{ - Version: 1, + Version: "1", KeyName: key, SignerCacheTimeout: 5 * time.Minute, - Issuer: config.IssuerDefault, + Issuer: "test-iss", }, authHandler) hour, err := time.ParseDuration("3600s") @@ -226,11 +226,11 @@ func validateClaims(tb testing.TB, provided *jvspb.JVSClaims, expectedJustificat tb.Helper() // test the standard claims filled by processor - if provided.StandardClaims.Issuer != config.IssuerDefault { - tb.Errorf("audience value %s incorrect, expected %s", provided.StandardClaims.Issuer, config.IssuerDefault) + if got, want := provided.Issuer, "test-iss"; got != want { + tb.Errorf("audience value %s incorrect, expected %s", got, want) } - if provided.StandardClaims.Subject != "user@example.com" { - tb.Errorf("subject value %s incorrect, expected %s", provided.StandardClaims.Subject, "user@example.com") + if got, want := provided.Subject, "user@example.com"; got != want { + tb.Errorf("subject value %s incorrect, expected %s", got, want) } // TODO: as we add more standard claims, add more validations. diff --git a/test/integ/main_test.go b/test/integ/main_test.go index 6ea5bea5..4d9b6f60 100644 --- a/test/integ/main_test.go +++ b/test/integ/main_test.go @@ -79,9 +79,10 @@ func TestJVS(t *testing.T) { }) cfg := &config.JustificationConfig{ - Version: 1, - KeyName: keyName, - Issuer: "ci-test", + Version: "1", + KeyName: keyName, + Issuer: "ci-test", + SignerCacheTimeout: 1 * time.Nanosecond, // no caching } if err := cfg.Validate(); err != nil { t.Fatal(err) @@ -255,7 +256,7 @@ func TestRotator(t *testing.T) { kmsClient, keyName := testSetupRotator(ctx, t) cfg := &config.CryptoConfig{ - Version: 1, + Version: "1", KeyTTL: 7 * time.Second, GracePeriod: 2 * time.Second, // rotate after 5 seconds PropagationDelay: time.Second, @@ -348,7 +349,7 @@ func TestRotator_EdgeCases(t *testing.T) { kmsClient, keyName := testSetupRotator(ctx, t) cfg := &config.CryptoConfig{ - Version: 1, + Version: "1", KeyTTL: 99 * time.Hour, GracePeriod: time.Second, PropagationDelay: time.Second, @@ -472,7 +473,7 @@ func TestCertActions(t *testing.T) { kmsClient, keyName := testSetupRotator(ctx, t) cfg := &config.CryptoConfig{ - Version: 1, + Version: "1", KeyTTL: 7 * time.Second, GracePeriod: 2 * time.Second, // rotate after 5 seconds PropagationDelay: time.Second,