Skip to content

Commit 6b73f7e

Browse files
[cmd/opampsupervisor] Validate that the HUP config reload cannot be used on Windows (#41077)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description The changes in this PR add a validation in the Supervisor's agent configuration that prevents the usage of the SIGHUP configuration reload feature in Windows, as this OS doesn't support such signal. This PR is a follow up to #40522. Thank you @nenadnoveljic for point it out and notifying us of the Windows build failure. <!--Describe what testing was performed and which tests were added.--> #### Testing Automated test added. --------- Signed-off-by: Douglas Camata <[email protected]>
1 parent f4d39f9 commit 6b73f7e

File tree

2 files changed

+59
-16
lines changed

2 files changed

+59
-16
lines changed

cmd/opampsupervisor/supervisor/config/config.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,10 @@ func (a Agent) Validate() error {
219219
return errors.New("agent::config_apply_timeout must be valid duration")
220220
}
221221

222+
if runtime.GOOS == "windows" && a.UseHUPConfigReload {
223+
return errors.New("agent::use_hup_config_reload is not supported on Windows")
224+
}
225+
222226
return nil
223227
}
224228

cmd/opampsupervisor/supervisor/config/config_test.go

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/http"
1010
"os"
1111
"path/filepath"
12+
"runtime"
1213
"testing"
1314
"time"
1415

@@ -18,14 +19,20 @@ import (
1819
"go.uber.org/zap/zapcore"
1920
)
2021

22+
func simpleError(err string) func() string {
23+
return func() string {
24+
return err
25+
}
26+
}
27+
2128
func TestValidate(t *testing.T) {
2229
tlsConfig := configtls.NewDefaultClientConfig()
2330
tlsConfig.InsecureSkipVerify = true
2431

2532
testCases := []struct {
26-
name string
27-
config Supervisor
28-
expectedError string
33+
name string
34+
config Supervisor
35+
expectedErrorFunc func() string
2936
}{
3037
{
3138
name: "Valid filled out config",
@@ -42,6 +49,7 @@ func TestValidate(t *testing.T) {
4249
OrphanDetectionInterval: 5 * time.Second,
4350
ConfigApplyTimeout: 2 * time.Second,
4451
BootstrapTimeout: 5 * time.Second,
52+
UseHUPConfigReload: false,
4553
},
4654
Capabilities: Capabilities{
4755
AcceptsRemoteConfig: true,
@@ -72,7 +80,7 @@ func TestValidate(t *testing.T) {
7280
Directory: "/etc/opamp-supervisor/storage",
7381
},
7482
},
75-
expectedError: "server::endpoint must be specified",
83+
expectedErrorFunc: simpleError("server::endpoint must be specified"),
7684
},
7785
{
7886
name: "Invalid URL",
@@ -96,7 +104,7 @@ func TestValidate(t *testing.T) {
96104
Directory: "/etc/opamp-supervisor/storage",
97105
},
98106
},
99-
expectedError: "invalid URL for server::endpoint:",
107+
expectedErrorFunc: simpleError("invalid URL for server::endpoint:"),
100108
},
101109
{
102110
name: "Invalid endpoint scheme",
@@ -120,7 +128,7 @@ func TestValidate(t *testing.T) {
120128
Directory: "/etc/opamp-supervisor/storage",
121129
},
122130
},
123-
expectedError: `invalid scheme "tcp" for server::endpoint, must be one of "http", "https", "ws", or "wss"`,
131+
expectedErrorFunc: simpleError(`invalid scheme "tcp" for server::endpoint, must be one of "http", "https", "ws", or "wss"`),
124132
},
125133
{
126134
name: "Invalid tls settings",
@@ -150,7 +158,7 @@ func TestValidate(t *testing.T) {
150158
Directory: "/etc/opamp-supervisor/storage",
151159
},
152160
},
153-
expectedError: "invalid server::tls settings:",
161+
expectedErrorFunc: simpleError("invalid server::tls settings:"),
154162
},
155163
{
156164
name: "Empty agent executable path",
@@ -175,7 +183,7 @@ func TestValidate(t *testing.T) {
175183
Directory: "/etc/opamp-supervisor/storage",
176184
},
177185
},
178-
expectedError: "agent::executable must be specified",
186+
expectedErrorFunc: simpleError("agent::executable must be specified"),
179187
},
180188
{
181189
name: "agent executable does not exist",
@@ -200,7 +208,7 @@ func TestValidate(t *testing.T) {
200208
Directory: "/etc/opamp-supervisor/storage",
201209
},
202210
},
203-
expectedError: "could not stat agent::executable path:",
211+
expectedErrorFunc: simpleError("could not stat agent::executable path:"),
204212
},
205213
{
206214
name: "Invalid orphan detection interval",
@@ -224,7 +232,7 @@ func TestValidate(t *testing.T) {
224232
Directory: "/etc/opamp-supervisor/storage",
225233
},
226234
},
227-
expectedError: "agent::orphan_detection_interval must be positive",
235+
expectedErrorFunc: simpleError("agent::orphan_detection_interval must be positive"),
228236
},
229237
{
230238
name: "Zero value health check port number",
@@ -297,7 +305,7 @@ func TestValidate(t *testing.T) {
297305
Directory: "/etc/opamp-supervisor/storage",
298306
},
299307
},
300-
expectedError: "agent::bootstrap_timeout must be positive",
308+
expectedErrorFunc: simpleError("agent::bootstrap_timeout must be positive"),
301309
},
302310
{
303311
name: "Invalid opamp server port number",
@@ -322,7 +330,7 @@ func TestValidate(t *testing.T) {
322330
Directory: "/etc/opamp-supervisor/storage",
323331
},
324332
},
325-
expectedError: "agent::opamp_server_port must be a valid port number",
333+
expectedErrorFunc: simpleError("agent::opamp_server_port must be a valid port number"),
326334
},
327335
{
328336
name: "Zero value opamp server port number",
@@ -371,7 +379,38 @@ func TestValidate(t *testing.T) {
371379
Directory: "/etc/opamp-supervisor/storage",
372380
},
373381
},
374-
expectedError: "agent::config_apply_timeout must be valid duration",
382+
expectedErrorFunc: simpleError("agent::config_apply_timeout must be valid duration"),
383+
},
384+
{
385+
name: "HUP config reload not supported on Windows",
386+
config: Supervisor{
387+
Server: OpAMPServer{
388+
Endpoint: "wss://localhost:9090/opamp",
389+
Headers: http.Header{
390+
"Header1": []string{"HeaderValue"},
391+
},
392+
TLS: tlsConfig,
393+
},
394+
Agent: Agent{
395+
Executable: "${file_path}",
396+
OrphanDetectionInterval: 5 * time.Second,
397+
ConfigApplyTimeout: 2 * time.Second,
398+
BootstrapTimeout: 5 * time.Second,
399+
UseHUPConfigReload: true,
400+
},
401+
Capabilities: Capabilities{
402+
AcceptsRemoteConfig: true,
403+
},
404+
Storage: Storage{
405+
Directory: "/etc/opamp-supervisor/storage",
406+
},
407+
},
408+
expectedErrorFunc: func() string {
409+
if runtime.GOOS != "windows" {
410+
return ""
411+
}
412+
return "agent::use_hup_config_reload is not supported on Windows"
413+
},
375414
},
376415
}
377416

@@ -394,10 +433,10 @@ func TestValidate(t *testing.T) {
394433

395434
err := tc.config.Validate()
396435

397-
if tc.expectedError == "" {
398-
require.NoError(t, err)
436+
if tc.expectedErrorFunc != nil && tc.expectedErrorFunc() != "" {
437+
require.ErrorContains(t, err, tc.expectedErrorFunc())
399438
} else {
400-
require.ErrorContains(t, err, tc.expectedError)
439+
require.NoError(t, err)
401440
}
402441
})
403442
}

0 commit comments

Comments
 (0)