Skip to content

Commit 21d747c

Browse files
Fix API config structure and add comprehensive test coverage (minekube#563)
* fix: simplify API config to use bind string directly fixed api config from using wrong nested config to bind string like it was supposed to: old yaml: api: enabled: true config: bind: 0.0.0.0:8080 new yaml: api: enabled: true bind: 0.0.0.0:3000 * fix: use config struct * Add comprehensive test coverage for API config changes - Tests flattened YAML structure parsing (current format) - Tests nested YAML structure behavior (legacy format) - Tests validation for invalid/empty bind addresses - Tests disabled API configuration handling - Ensures the yaml:",inline" fix works correctly Co-authored-by: robinbraemer <robinbraemer@users.noreply.github.com> * SECURITY: Fix API default config and restore proper validation - Restore api.DefaultConfig usage (localhost:8080) instead of hardcoded 0.0.0.0:8080 - Security fix: Default API bind should use localhost, not 0.0.0.0 (all interfaces) - Restore proper c.API.Config.Validate() method instead of direct field validation - Add TestAPIConfigDefaultSecurity to ensure secure defaults - Update test assertions to match proper validation error format This prevents accidentally exposing the API to all network interfaces by default. --------- Co-authored-by: Dylan <dylan@dylanh.dev> Co-authored-by: robinbraemer <robinbraemer@users.noreply.github.com>
1 parent ea592b2 commit 21d747c

2 files changed

Lines changed: 129 additions & 1 deletion

File tree

pkg/gate/config/api_config_test.go

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
package config
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
"gopkg.in/yaml.v3"
9+
)
10+
11+
func TestAPIConfigFlattened(t *testing.T) {
12+
// Test YAML that matches our current config files (flattened structure)
13+
yamlConfig := `
14+
api:
15+
enabled: true
16+
bind: "0.0.0.0:3000"
17+
`
18+
19+
var cfg Config
20+
err := yaml.Unmarshal([]byte(yamlConfig), &cfg)
21+
require.NoError(t, err, "Should unmarshal flattened API config")
22+
23+
assert.True(t, cfg.API.Enabled, "API should be enabled")
24+
assert.Equal(t, "0.0.0.0:3000", cfg.API.Config.Bind, "API bind should be parsed correctly")
25+
26+
// Test validation
27+
warns, errs := cfg.Validate()
28+
assert.Empty(t, errs, "Should have no validation errors")
29+
assert.Empty(t, warns, "Should have no validation warnings")
30+
}
31+
32+
func TestAPIConfigNested(t *testing.T) {
33+
// Test YAML with old nested structure (should NOT work with yaml:",inline" - this is expected)
34+
yamlConfig := `
35+
api:
36+
enabled: true
37+
config:
38+
bind: "0.0.0.0:3000"
39+
`
40+
41+
var cfg Config
42+
err := yaml.Unmarshal([]byte(yamlConfig), &cfg)
43+
require.NoError(t, err, "Should unmarshal nested API config without errors")
44+
45+
assert.True(t, cfg.API.Enabled, "API should be enabled")
46+
assert.Equal(t, "", cfg.API.Config.Bind, "API bind should be empty (nested structure not supported with inline)")
47+
48+
// Test validation (should fail because bind is empty)
49+
_, errs := cfg.Validate()
50+
assert.NotEmpty(t, errs, "Should have validation errors because nested config is ignored")
51+
assert.Contains(t, errs[0].Error(), "api:", "Error should mention API validation")
52+
}
53+
54+
func TestAPIConfigDisabled(t *testing.T) {
55+
// Test disabled API
56+
yamlConfig := `
57+
api:
58+
enabled: false
59+
bind: "localhost:8080"
60+
`
61+
62+
var cfg Config
63+
err := yaml.Unmarshal([]byte(yamlConfig), &cfg)
64+
require.NoError(t, err, "Should unmarshal disabled API config")
65+
66+
assert.False(t, cfg.API.Enabled, "API should be disabled")
67+
assert.Equal(t, "localhost:8080", cfg.API.Config.Bind, "API bind should still be parsed")
68+
69+
// Test validation (should pass even when disabled)
70+
warns, errs := cfg.Validate()
71+
assert.Empty(t, errs, "Should have no validation errors for disabled API")
72+
assert.Empty(t, warns, "Should have no validation warnings for disabled API")
73+
}
74+
75+
func TestAPIConfigInvalidBind(t *testing.T) {
76+
// Test invalid bind address
77+
yamlConfig := `
78+
api:
79+
enabled: true
80+
bind: "invalid-address"
81+
`
82+
83+
var cfg Config
84+
err := yaml.Unmarshal([]byte(yamlConfig), &cfg)
85+
require.NoError(t, err, "Should unmarshal config with invalid bind")
86+
87+
assert.True(t, cfg.API.Enabled, "API should be enabled")
88+
assert.Equal(t, "invalid-address", cfg.API.Config.Bind, "Invalid bind should be parsed")
89+
90+
// Test validation (should fail)
91+
_, errs := cfg.Validate()
92+
assert.NotEmpty(t, errs, "Should have validation errors for invalid bind")
93+
assert.Contains(t, errs[0].Error(), "api:", "Error should mention API validation")
94+
}
95+
96+
func TestAPIConfigEmptyBind(t *testing.T) {
97+
// Test empty bind address
98+
yamlConfig := `
99+
api:
100+
enabled: true
101+
bind: ""
102+
`
103+
104+
var cfg Config
105+
err := yaml.Unmarshal([]byte(yamlConfig), &cfg)
106+
require.NoError(t, err, "Should unmarshal config with empty bind")
107+
108+
assert.True(t, cfg.API.Enabled, "API should be enabled")
109+
assert.Equal(t, "", cfg.API.Config.Bind, "Empty bind should be parsed")
110+
111+
// Test validation (should fail)
112+
_, errs := cfg.Validate()
113+
assert.NotEmpty(t, errs, "Should have validation errors for empty bind")
114+
assert.Contains(t, errs[0].Error(), "api:", "Error should mention API validation")
115+
}
116+
117+
func TestAPIConfigDefaultSecurity(t *testing.T) {
118+
// Test that default config uses localhost (not 0.0.0.0) for security
119+
cfg := DefaultConfig
120+
121+
assert.False(t, cfg.API.Enabled, "API should be disabled by default")
122+
assert.Equal(t, "localhost:8080", cfg.API.Config.Bind, "Default API bind should use localhost for security")
123+
124+
// Test that API config itself is valid (even though full config may have other warnings)
125+
apiWarns, apiErrs := cfg.API.Config.Validate()
126+
assert.Empty(t, apiErrs, "Default API config should have no validation errors")
127+
assert.Empty(t, apiWarns, "Default API config should have no validation warnings")
128+
}

pkg/gate/config/config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ type HealthService struct {
4545
// API is the configuration for the Gate API.
4646
type API struct {
4747
Enabled bool `json:"enabled,omitempty" yaml:"enabled,omitempty"`
48-
Config api.Config `json:"config,omitempty" yaml:"config,omitempty"`
48+
Config api.Config `yaml:",inline"`
4949
}
5050

5151
// Validate validates a Config and all enabled configs.

0 commit comments

Comments
 (0)