Skip to content

Commit 196c85c

Browse files
authored
fix: [sc-86007] Add config validation in config mode (#41)
* fix: add validate configuration step in config mode * ci: stagger auto updates and reduce backoff retry time * refactor: default constants and validation * fix: use github token for update api calls * ci: match build version
1 parent 659b3ed commit 196c85c

File tree

7 files changed

+192
-28
lines changed

7 files changed

+192
-28
lines changed

.github/workflows/integration-test.yml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ jobs:
177177
shell: bash
178178
run: |
179179
sudo ./dist/rewst_agent_config.linux.it.bin --update --org-id ${{ vars.IT_ORG_ID }}
180-
echo "Installed integration test binary (version 0.0.0-it)"
180+
echo "Installed integration test binary (version v0.0.0-it)"
181181
182182
- name: Wait for auto updater to trigger and update
183183
shell: bash
@@ -208,9 +208,9 @@ jobs:
208208
fi
209209
done
210210
211-
if echo "$logContent" | grep -qF "version=0.0.0-it"; then
211+
if echo "$logContent" | grep -qF "version=v0.0.0-it"; then
212212
lastVersion=$(echo "$logContent" | grep "Agent Smith started" | tail -1)
213-
if echo "$lastVersion" | grep -qF "version=0.0.0-it"; then
213+
if echo "$lastVersion" | grep -qF "version=v0.0.0-it"; then
214214
echo "ERROR: Agent is still running the integration test version after update"
215215
exit 1
216216
fi
@@ -454,7 +454,7 @@ jobs:
454454
shell: pwsh
455455
run: |
456456
./dist/rewst_agent_config.win.it.exe --update --org-id ${{ vars.IT_ORG_ID }}
457-
Write-Output "Installed integration test binary (version 0.0.0-it)"
457+
Write-Output "Installed integration test binary (version v0.0.0-it)"
458458
459459
- name: Wait for auto updater to trigger and update
460460
shell: pwsh
@@ -486,7 +486,7 @@ jobs:
486486
}
487487
488488
$lastStartLine = ($logContent -split "`n" | Where-Object { $_ -match "Agent Smith started" } | Select-Object -Last 1)
489-
if ($lastStartLine -match [regex]::Escape("version=0.0.0-it")) {
489+
if ($lastStartLine -match [regex]::Escape("version=v0.0.0-it")) {
490490
Write-Error "Agent is still running the integration test version after update"
491491
exit 1
492492
}
@@ -758,7 +758,7 @@ jobs:
758758
shell: bash
759759
run: |
760760
sudo ./dist/rewst_agent_config.mac-os.it.bin --update --org-id ${{ vars.IT_ORG_ID }}
761-
echo "Installed integration test binary (version 0.0.0-it)"
761+
echo "Installed integration test binary (version v0.0.0-it)"
762762
763763
- name: Wait for auto updater to trigger and update
764764
shell: bash
@@ -790,7 +790,7 @@ jobs:
790790
done
791791
792792
lastVersion=$(echo "$logContent" | grep "Agent Smith started" | tail -1)
793-
if echo "$lastVersion" | grep -qF "version=0.0.0-it"; then
793+
if echo "$lastVersion" | grep -qF "version=v0.0.0-it"; then
794794
echo "ERROR: Agent is still running the integration test version after update"
795795
exit 1
796796
fi

cmd/agent_smith/config.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,22 @@ type fetchConfigurationResponse struct {
2121
Configuration agent.Device `json:"configuration"`
2222
}
2323

24+
func validateConfiguration(device agent.Device) error {
25+
if device.DeviceId == "" {
26+
return fmt.Errorf("missing required field: device_id")
27+
}
28+
if device.RewstEngineHost == "" {
29+
return fmt.Errorf("missing required field: rewst_engine_host")
30+
}
31+
if device.SharedAccessKey == "" {
32+
return fmt.Errorf("missing required field: shared_access_key")
33+
}
34+
if device.AzureIotHubHost == "" {
35+
return fmt.Errorf("missing required field: azure_iot_hub_host")
36+
}
37+
return nil
38+
}
39+
2440
func runConfig(params *configContext) error {
2541
logger := utils.ConfigureLogger("agent_smith", os.Stdout, utils.Default)
2642

@@ -81,6 +97,11 @@ func runConfig(params *configContext) error {
8197
if err != nil {
8298
return fmt.Errorf("failed to parse response: %w", err)
8399
}
100+
101+
if err := validateConfiguration(response.Configuration); err != nil {
102+
return fmt.Errorf("invalid configuration: %w", err)
103+
}
104+
84105
response.Configuration.LoggingLevel = utils.LoggingLevel(params.LoggingLevel)
85106
response.Configuration.UseSyslog = params.UseSyslog
86107
response.Configuration.DisableAgentPostback = params.DisableAgentPostback

cmd/agent_smith/config_test.go

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,91 @@ func newBaseConfigParams(configURL string) *configContext {
7676
}
7777
}
7878

79+
// ── validateConfiguration tests ──────────────────────────────────────────────
80+
81+
func TestValidateConfiguration_Valid(t *testing.T) {
82+
device := agent.Device{
83+
DeviceId: "device-123",
84+
RewstEngineHost: "engine.example.com",
85+
SharedAccessKey: "key123",
86+
AzureIotHubHost: "hub.example.com",
87+
}
88+
if err := validateConfiguration(device); err != nil {
89+
t.Errorf("expected no error, got %v", err)
90+
}
91+
}
92+
93+
func TestValidateConfiguration_MissingFields(t *testing.T) {
94+
tests := []struct {
95+
name string
96+
device agent.Device
97+
field string
98+
}{
99+
{
100+
name: "missing device_id",
101+
device: agent.Device{
102+
RewstEngineHost: "engine.example.com",
103+
SharedAccessKey: "key123",
104+
AzureIotHubHost: "hub.example.com",
105+
},
106+
field: "device_id",
107+
},
108+
{
109+
name: "missing rewst_engine_host",
110+
device: agent.Device{
111+
DeviceId: "device-123",
112+
SharedAccessKey: "key123",
113+
AzureIotHubHost: "hub.example.com",
114+
},
115+
field: "rewst_engine_host",
116+
},
117+
{
118+
name: "missing shared_access_key",
119+
device: agent.Device{
120+
DeviceId: "device-123",
121+
RewstEngineHost: "engine.example.com",
122+
AzureIotHubHost: "hub.example.com",
123+
},
124+
field: "shared_access_key",
125+
},
126+
{
127+
name: "missing azure_iot_hub_host",
128+
device: agent.Device{
129+
DeviceId: "device-123",
130+
RewstEngineHost: "engine.example.com",
131+
SharedAccessKey: "key123",
132+
},
133+
field: "azure_iot_hub_host",
134+
},
135+
}
136+
137+
for _, tt := range tests {
138+
t.Run(tt.name, func(t *testing.T) {
139+
err := validateConfiguration(tt.device)
140+
if err == nil {
141+
t.Errorf("expected error for missing %s, got nil", tt.field)
142+
return
143+
}
144+
if !strings.Contains(err.Error(), tt.field) {
145+
t.Errorf("expected error to mention %q, got %v", tt.field, err)
146+
}
147+
})
148+
}
149+
}
150+
151+
func TestRunConfig_InvalidConfiguration(t *testing.T) {
152+
// Response missing required fields (no device_id, engine host, etc.)
153+
body := `{"configuration": {}}`
154+
srv := newConfigServer(t, http.StatusOK, body)
155+
defer srv.Close()
156+
157+
err := runConfig(newBaseConfigParams(srv.URL))
158+
159+
if err == nil || !strings.Contains(err.Error(), "invalid configuration") {
160+
t.Errorf("expected 'invalid configuration' error, got %v", err)
161+
}
162+
}
163+
79164
// ── tests ─────────────────────────────────────────────────────────────────────
80165

81166
func TestRunConfig_Success(t *testing.T) {

cmd/agent_smith/service.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,17 @@ func (svc *serviceContext) Execute(
107107
logger,
108108
&device,
109109
"https://api.github.com/repos/rewstapp/agent-smith-go/releases/latest",
110+
os.Getenv("GITHUB_TOKEN"),
110111
func(path string, args []string) error {
111112
return detachedCommand(path, args, logFile, logFile).Start()
112113
},
113114
)
114115
runner := agent.NewAutoUpdateRunner(
115-
logger, updater, agent.DefaultUpdateInterval(), 5, 5*time.Minute,
116+
logger,
117+
updater,
118+
agent.DefaultUpdateInterval(),
119+
agent.DefaultMaxRetries(),
120+
agent.DefaultBaseBackoff(),
116121
)
117122
runner.Start()
118123
defer runner.Stop()

internal/agent/updater.go

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,20 @@ type Updater interface {
3636
// Example: -ldflags "-X github.com/RewstApp/agent-smith-go/internal/agent.updateIntervalStr=30s"
3737
var updateIntervalStr = ""
3838

39+
// baseBackoffStr is overridable via -ldflags for integration testing.
40+
// Example: -ldflags "-X github.com/RewstApp/agent-smith-go/internal/agent.baseBackoffStr=5s"
41+
var baseBackoffStr = ""
42+
43+
// maxRetriesStr is overridable via -ldflags for integration testing.
44+
// Example: -ldflags "-X github.com/RewstApp/agent-smith-go/internal/agent.maxRetriesStr=3"
45+
var maxRetriesStr = ""
46+
47+
const (
48+
defaultUpdateInterval = 48 * time.Hour
49+
defaultBaseBackoff = 5 * time.Minute
50+
defaultMaxRetries = 5
51+
)
52+
3953
// DefaultUpdateInterval returns the auto-update check interval.
4054
// Uses updateIntervalStr if set via ldflags, otherwise defaults to 48 hours.
4155
func DefaultUpdateInterval() time.Duration {
@@ -44,7 +58,30 @@ func DefaultUpdateInterval() time.Duration {
4458
return d
4559
}
4660
}
47-
return 48 * time.Hour
61+
return defaultUpdateInterval
62+
}
63+
64+
// DefaultBaseBackoff returns the base backoff duration for update retries.
65+
// Uses baseBackoffStr if set via ldflags, otherwise defaults to 5 minutes.
66+
func DefaultBaseBackoff() time.Duration {
67+
if baseBackoffStr != "" {
68+
if d, err := time.ParseDuration(baseBackoffStr); err == nil {
69+
return d
70+
}
71+
}
72+
return defaultBaseBackoff
73+
}
74+
75+
// DefaultMaxRetries returns the maximum number of update retry attempts.
76+
// Uses maxRetriesStr if set via ldflags, otherwise defaults to 5.
77+
func DefaultMaxRetries() int {
78+
if maxRetriesStr != "" {
79+
var n int
80+
if _, err := fmt.Sscanf(maxRetriesStr, "%d", &n); err == nil && n > 0 {
81+
return n
82+
}
83+
}
84+
return defaultMaxRetries
4885
}
4986

5087
type RunCommandFunc = func(path string, args []string) error
@@ -53,19 +90,22 @@ type defaultUpdater struct {
5390
logger hclog.Logger
5491
device *Device
5592
latestReleaseUrl string
93+
githubToken string
5694
runCommand RunCommandFunc
5795
}
5896

5997
func NewUpdater(
6098
logger hclog.Logger,
6199
device *Device,
62100
latestReleaseUrl string,
101+
githubToken string,
63102
runCommand RunCommandFunc,
64103
) Updater {
65104
return &defaultUpdater{
66105
logger: logger,
67106
device: device,
68107
latestReleaseUrl: latestReleaseUrl,
108+
githubToken: githubToken,
69109
runCommand: runCommand,
70110
}
71111
}
@@ -74,7 +114,15 @@ func (u *defaultUpdater) Check() (Release, error) {
74114
release := Release{}
75115
u.logger.Info("Checking for updates")
76116

77-
resp, err := http.Get(u.latestReleaseUrl)
117+
req, err := http.NewRequest(http.MethodGet, u.latestReleaseUrl, nil)
118+
if err != nil {
119+
return release, err
120+
}
121+
if u.githubToken != "" {
122+
req.Header.Set("Authorization", "Bearer "+u.githubToken)
123+
}
124+
125+
resp, err := http.DefaultClient.Do(req)
78126
if err != nil {
79127
u.logger.Error("Failed to fetch latest release", "url", u.latestReleaseUrl, "error", err)
80128
return release, err
@@ -138,6 +186,9 @@ func (u *defaultUpdater) Download(asset Asset) (string, error) {
138186
}
139187

140188
req.Header.Add("Accept", "application/octet-stream")
189+
if u.githubToken != "" {
190+
req.Header.Set("Authorization", "Bearer "+u.githubToken)
191+
}
141192

142193
resp, err := http.DefaultClient.Do(req)
143194
if err != nil {

0 commit comments

Comments
 (0)