From f94214e52db768ce63b20e744a0df6e8c147d16e Mon Sep 17 00:00:00 2001 From: ka3de Date: Mon, 2 Sep 2024 11:37:19 +0200 Subject: [PATCH] feat: Validate browser capability (#809) * Validate browser capability k6 functionality is required in the agent by default, and is only NOT required in case both, scripted and browser checks, have been disabled for the probe from the API. * Run test in parallel --- internal/checks/checks.go | 7 ++- internal/checks/checks_test.go | 106 ++++++++++++++++++++++++++++----- 2 files changed, 96 insertions(+), 17 deletions(-) diff --git a/internal/checks/checks.go b/internal/checks/checks.go index d5c9fdfaf..1b9232708 100644 --- a/internal/checks/checks.go +++ b/internal/checks/checks.go @@ -470,8 +470,11 @@ func (c *Updater) loop(ctx context.Context) (bool, error) { } func (c *Updater) validateProbeCapabilities(capabilities *sm.Probe_Capabilities) error { - // k6 is only not required on this probe if explicitly disabled - requireK6 := capabilities == nil || !capabilities.DisableScriptedChecks + // k6 is required by default unless it has been explicitly disabled from + // the API side by forbidding scripted and browser checks execution. + requireK6 := capabilities == nil || + !capabilities.DisableScriptedChecks || + !capabilities.DisableBrowserChecks if requireK6 && c.k6Runner == nil { return errCapabilityK6Missing diff --git a/internal/checks/checks_test.go b/internal/checks/checks_test.go index d2957d6d8..816b13b38 100644 --- a/internal/checks/checks_test.go +++ b/internal/checks/checks_test.go @@ -269,12 +269,72 @@ func TestHandleCheckOp(t *testing.T) { } func TestCheckHandlerProbeValidation(t *testing.T) { + t.Parallel() + testcases := map[string]struct { opts UpdaterOptions probe sm.Probe expectedError error }{ - "has K6 when required": { + "has K6 when required for scripted checks": { + expectedError: nil, + opts: UpdaterOptions{ + Conn: new(grpc.ClientConn), + PromRegisterer: prometheus.NewPedanticRegistry(), + Publisher: channelPublisher(make(chan pusher.Payload)), + TenantCh: make(chan<- sm.Tenant), + Logger: zerolog.Nop(), + K6Runner: noopRunner{}, + }, + probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{ + DisableScriptedChecks: false, + DisableBrowserChecks: true, + }}, + }, + "missing K6 when required for scripted checks": { + expectedError: errCapabilityK6Missing, + opts: UpdaterOptions{ + Conn: new(grpc.ClientConn), + PromRegisterer: prometheus.NewPedanticRegistry(), + Publisher: channelPublisher(make(chan pusher.Payload)), + TenantCh: make(chan<- sm.Tenant), + Logger: zerolog.Nop(), + }, + probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{ + DisableScriptedChecks: false, + DisableBrowserChecks: true, + }}, + }, + "has K6 when required for browser checks": { + expectedError: nil, + opts: UpdaterOptions{ + Conn: new(grpc.ClientConn), + PromRegisterer: prometheus.NewPedanticRegistry(), + Publisher: channelPublisher(make(chan pusher.Payload)), + TenantCh: make(chan<- sm.Tenant), + Logger: zerolog.Nop(), + K6Runner: noopRunner{}, + }, + probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{ + DisableScriptedChecks: true, + DisableBrowserChecks: false, + }}, + }, + "missing K6 when required for browser checks": { + expectedError: errCapabilityK6Missing, + opts: UpdaterOptions{ + Conn: new(grpc.ClientConn), + PromRegisterer: prometheus.NewPedanticRegistry(), + Publisher: channelPublisher(make(chan pusher.Payload)), + TenantCh: make(chan<- sm.Tenant), + Logger: zerolog.Nop(), + }, + probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{ + DisableScriptedChecks: true, + DisableBrowserChecks: false, + }}, + }, + "has K6 when required for scripted and browser checks": { expectedError: nil, opts: UpdaterOptions{ Conn: new(grpc.ClientConn), @@ -284,9 +344,12 @@ func TestCheckHandlerProbeValidation(t *testing.T) { Logger: zerolog.Nop(), K6Runner: noopRunner{}, }, - probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{DisableScriptedChecks: false}}, + probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{ + DisableScriptedChecks: false, + DisableBrowserChecks: false, + }}, }, - "missing K6 when required": { + "missing K6 when required for scripted and browser checks": { expectedError: errCapabilityK6Missing, opts: UpdaterOptions{ Conn: new(grpc.ClientConn), @@ -295,7 +358,10 @@ func TestCheckHandlerProbeValidation(t *testing.T) { TenantCh: make(chan<- sm.Tenant), Logger: zerolog.Nop(), }, - probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{DisableScriptedChecks: false}}, + probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{ + DisableScriptedChecks: false, + DisableBrowserChecks: false, + }}, }, "has K6 but not required": { expectedError: nil, @@ -307,7 +373,10 @@ func TestCheckHandlerProbeValidation(t *testing.T) { Logger: zerolog.Nop(), K6Runner: noopRunner{}, }, - probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{DisableScriptedChecks: true}}, + probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{ + DisableScriptedChecks: true, + DisableBrowserChecks: true, + }}, }, "missing K6 but not required": { expectedError: nil, @@ -318,7 +387,10 @@ func TestCheckHandlerProbeValidation(t *testing.T) { TenantCh: make(chan<- sm.Tenant), Logger: zerolog.Nop(), }, - probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{DisableScriptedChecks: true}}, + probe: sm.Probe{Id: 100, Name: "test-probe", Capabilities: &sm.Probe_Capabilities{ + DisableScriptedChecks: true, + DisableBrowserChecks: true, + }}, }, "missing K6 when required by default": { expectedError: errCapabilityK6Missing, @@ -345,17 +417,21 @@ func TestCheckHandlerProbeValidation(t *testing.T) { }, } - for _, tc := range testcases { - u, err := NewUpdater(tc.opts) - require.NoError(t, err) - - err = u.validateProbeCapabilities(tc.probe.Capabilities) + for testName, tc := range testcases { + t.Run(testName, func(t *testing.T) { + t.Parallel() - if tc.expectedError != nil { - require.Error(t, err, tc.expectedError) - } else { + u, err := NewUpdater(tc.opts) require.NoError(t, err) - } + + err = u.validateProbeCapabilities(tc.probe.Capabilities) + + if tc.expectedError != nil { + require.Error(t, err, tc.expectedError) + } else { + require.NoError(t, err) + } + }) } }