Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc1a661a

Browse files
committed
refactor: make SAST flags use config SAST settings
This allows the config cache to prevent making multiple network requests when we already have one place that contains all the information.
1 parentb10f2bc commitc1a661a

File tree

5 files changed

+158
-40
lines changed

5 files changed

+158
-40
lines changed

‎pkg/local_workflows/code_workflow.go‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,44 +75,57 @@ func getSastSettingsConfig(engine workflow.Engine) configuration.DefaultValueFun
7575
returncallback
7676
}
7777

78+
funcgetSastResponseFromConfig(config configuration.Configuration) (*sast_contract.SastResponse,error) {
79+
sastResponseGeneric,err:=config.GetWithError(code_workflow.ConfigurationSastSettings)
80+
iferr!=nil {
81+
returnnil,err
82+
}
83+
ifsastResponseGeneric==nil {
84+
returnnil,fmt.Errorf("SAST settings are nil")
85+
}
86+
sastResponse,ok:=sastResponseGeneric.(*sast_contract.SastResponse)
87+
if!ok {
88+
returnnil,fmt.Errorf("SAST settings are not a SastResponse")
89+
}
90+
returnsastResponse,nil
91+
}
92+
7893
funcgetSastEnabled(engine workflow.Engine) configuration.DefaultValueFunction {
79-
err:=engine.GetConfiguration().AddKeyDependency(code_workflow.ConfigurationSastEnabled,configuration.ORGANIZATION)
94+
err:=engine.GetConfiguration().AddKeyDependency(code_workflow.ConfigurationSastEnabled,code_workflow.ConfigurationSastSettings)
8095
iferr!=nil {
8196
engine.GetLogger().Err(err).Msg("Failed to add dependency for SAST settings.")
8297
}
83-
callback:=func(_ configuration.Configuration,existingValueany) (any,error) {
98+
callback:=func(config configuration.Configuration,existingValueany) (any,error) {
8499
ifexistingValue!=nil {
85100
returnexistingValue,nil
86101
}
87-
88-
response,err:=getSastSettings(engine)
102+
sastResponse,err:=getSastResponseFromConfig(config)
89103
iferr!=nil {
90104
engine.GetLogger().Err(err).Msg("Failed to access settings.")
91105
returnfalse,err
92106
}
93107

94-
returnresponse.SastEnabled,nil
108+
returnsastResponse.SastEnabled,nil
95109
}
96110
returncallback
97111
}
98112

99113
funcgetSlceEnabled(engine workflow.Engine) configuration.DefaultValueFunction {
100-
err:=engine.GetConfiguration().AddKeyDependency(code_workflow.ConfigurarionSlceEnabled,configuration.ORGANIZATION)
114+
err:=engine.GetConfiguration().AddKeyDependency(code_workflow.ConfigurarionSlceEnabled,code_workflow.ConfigurationSastSettings)
101115
iferr!=nil {
102116
engine.GetLogger().Err(err).Msg("Failed to add dependency for SAST settings.")
103117
}
104-
callback:=func(_ configuration.Configuration,existingValueany) (any,error) {
118+
callback:=func(config configuration.Configuration,existingValueany) (any,error) {
105119
ifexistingValue!=nil {
106120
returnexistingValue,nil
107121
}
108-
109-
response,err:=getSastSettings(engine)
122+
sastResponse,err:=getSastResponseFromConfig(config)
110123
iferr!=nil {
111124
engine.GetLogger().Err(err).Msg("Failed to access settings.")
112125
returnfalse,err
113126
}
114127

115-
returnresponse.LocalCodeEngine.Enabled,nil
128+
returnsastResponse.LocalCodeEngine.Enabled,nil
116129
}
117130
returncallback
118131
}

‎pkg/local_workflows/code_workflow_test.go‎

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ func Test_getSastSettingsConfig(t *testing.T) {
565565
LocalCodeEngine: sast_contract.LocalCodeEngine{Enabled:isFirstCall},
566566
}
567567
},
568-
getSastSettingsConfig,
568+
InitCodeWorkflow,
569569
&sast_contract.SastResponse{
570570
SastEnabled:true,
571571
LocalCodeEngine: sast_contract.LocalCodeEngine{Enabled:true},
@@ -599,20 +599,44 @@ func Test_getSastEnabled(t *testing.T) {
599599
assert.True(t,boolResult,"Should return existing value when provided")
600600
})
601601

602-
t.Run("callbackfetches settings when existing value is nil",func(t*testing.T) {
603-
server:=setupMockServerForSastSettings(t,true,false)
604-
deferserver.Close()
602+
t.Run("callbackreads from ConfigurationSastSettings (pre-cached) when existing value is nil",func(t*testing.T) {
603+
mockEngine:=setupMockEngine(t)
604+
config:=mockEngine.GetConfiguration()
605605

606-
mockEngine,config:=setupMockEngineWithServer(t,server)
606+
// Set ConfigurationSastSettings in config
607+
sastSettings:=&sast_contract.SastResponse{
608+
SastEnabled:true,
609+
LocalCodeEngine: sast_contract.LocalCodeEngine{Enabled:false},
610+
}
611+
config.Set(code_workflow.ConfigurationSastSettings,sastSettings)
607612

608613
result,err:=getSastEnabled(mockEngine)(config,nil)
609614
assert.NoError(t,err)
610615
boolResult,ok:=result.(bool)
611616
assert.True(t,ok,"result should be of type bool")
612-
assert.True(t,boolResult,"Should return SastEnabled fromAPI response")
617+
assert.True(t,boolResult,"Should return SastEnabled fromConfigurationSastSettings")
613618
})
614619

615-
t.Run("adds organization dependency and clears cache on org change",func(t*testing.T) {
620+
t.Run("depends on ConfigurationSastSettings",func(t*testing.T) {
621+
testutils.CheckConfigCachesDependency(
622+
t,
623+
code_workflow.ConfigurationSastEnabled,
624+
code_workflow.ConfigurationSastSettings,
625+
getSastEnabled,
626+
&sast_contract.SastResponse{
627+
SastEnabled:true,
628+
LocalCodeEngine: sast_contract.LocalCodeEngine{Enabled:false},
629+
},
630+
&sast_contract.SastResponse{
631+
SastEnabled:false,
632+
LocalCodeEngine: sast_contract.LocalCodeEngine{Enabled:false},
633+
},
634+
true,
635+
false,
636+
)
637+
})
638+
639+
t.Run("respects organization changes (full chain)",func(t*testing.T) {
616640
testutils.CheckCacheRespectOrgDependency(
617641
t,
618642
code_workflow.ConfigurationSastEnabled,
@@ -622,7 +646,7 @@ func Test_getSastEnabled(t *testing.T) {
622646
LocalCodeEngine: sast_contract.LocalCodeEngine{Enabled:false},
623647
}
624648
},
625-
getSastEnabled,
649+
InitCodeWorkflow,
626650
true,
627651
false,
628652
)
@@ -652,20 +676,44 @@ func Test_getSlceEnabled(t *testing.T) {
652676
assert.True(t,boolResult,"Should return existing value when provided")
653677
})
654678

655-
t.Run("callbackfetches settings when existing value is nil",func(t*testing.T) {
656-
server:=setupMockServerForSastSettings(t,false,true)
657-
deferserver.Close()
679+
t.Run("callbackreads from ConfigurationSastSettings (pre-cached) when existing value is nil",func(t*testing.T) {
680+
mockEngine:=setupMockEngine(t)
681+
config:=mockEngine.GetConfiguration()
658682

659-
mockEngine,config:=setupMockEngineWithServer(t,server)
683+
// Set ConfigurationSastSettings in config
684+
sastSettings:=&sast_contract.SastResponse{
685+
SastEnabled:false,
686+
LocalCodeEngine: sast_contract.LocalCodeEngine{Enabled:true},
687+
}
688+
config.Set(code_workflow.ConfigurationSastSettings,sastSettings)
660689

661690
result,err:=getSlceEnabled(mockEngine)(config,nil)
662691
assert.NoError(t,err)
663692
boolResult,ok:=result.(bool)
664693
assert.True(t,ok,"result should be of type bool")
665-
assert.True(t,boolResult,"Should return LocalCodeEngine.Enabled fromAPI response")
694+
assert.True(t,boolResult,"Should return LocalCodeEngine.Enabled fromConfigurationSastSettings")
666695
})
667696

668-
t.Run("adds organization dependency and clears cache on org change",func(t*testing.T) {
697+
t.Run("depends on ConfigurationSastSettings",func(t*testing.T) {
698+
testutils.CheckConfigCachesDependency(
699+
t,
700+
code_workflow.ConfigurarionSlceEnabled,
701+
code_workflow.ConfigurationSastSettings,
702+
getSlceEnabled,
703+
&sast_contract.SastResponse{
704+
SastEnabled:false,
705+
LocalCodeEngine: sast_contract.LocalCodeEngine{Enabled:true},
706+
},
707+
&sast_contract.SastResponse{
708+
SastEnabled:false,
709+
LocalCodeEngine: sast_contract.LocalCodeEngine{Enabled:false},
710+
},
711+
true,
712+
false,
713+
)
714+
})
715+
716+
t.Run("respects organization changes (full chain)",func(t*testing.T) {
669717
testutils.CheckCacheRespectOrgDependency(
670718
t,
671719
code_workflow.ConfigurarionSlceEnabled,
@@ -675,7 +723,7 @@ func Test_getSlceEnabled(t *testing.T) {
675723
LocalCodeEngine: sast_contract.LocalCodeEngine{Enabled:isFirstCall},
676724
}
677725
},
678-
getSlceEnabled,
726+
InitCodeWorkflow,
679727
true,
680728
false,
681729
)

‎pkg/local_workflows/config_utils/feature_flag_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package config_utils
33
import (
44
"testing"
55

6-
"github.com/snyk/go-application-framework/pkg/configuration"
76
testutils"github.com/snyk/go-application-framework/pkg/local_workflows/test_utils"
87
"github.com/snyk/go-application-framework/pkg/workflow"
98
)
@@ -18,7 +17,7 @@ func Test_AddFeatureFlagToConfig_CacheDependentOnOrg(t *testing.T) {
1817
"ok":isFirstCall,
1918
}
2019
},
21-
func(engine workflow.Engine)configuration.DefaultValueFunction {
20+
func(engine workflow.Engine)error {
2221
AddFeatureFlagToConfig(engine,testConfigKey,"testFeatureFlag")
2322
returnnil
2423
},

‎pkg/local_workflows/ignore_workflow/ignore_workflow_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,7 @@ func Test_getOrgIgnoreApprovalEnabled_CacheDependentOnOrg(t *testing.T) {
846846
},
847847
}
848848
},
849-
getOrgIgnoreApprovalEnabled,
849+
InitIgnoreWorkflows,
850850
true,
851851
false,
852852
)

‎pkg/local_workflows/test_utils/test_utils.go‎

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,75 @@ func NewErrorProducingTestClient(fn roundTripErrorFn) *http.Client {
4444
}
4545
}
4646

47+
// CheckConfigCachesDependency tests that a config value properly invalidates its cache when a dependency changes.
48+
// This is for testing config-to-config dependencies (e.g., ConfigurationSastEnabled depends on ConfigurationSastSettings).
49+
funcCheckConfigCachesDependency(
50+
t*testing.T,
51+
configKeystring,
52+
dependencyKeystring,
53+
defaultValueFuncFactoryfunc(engine workflow.Engine) configuration.DefaultValueFunction,
54+
dependencyValueBeforeany,
55+
dependencyValueAfterany,
56+
expectedValueBeforeany,
57+
expectedValueAfterany,
58+
) {
59+
t.Helper()
60+
61+
require.NotEqual(t,expectedValueBefore,expectedValueAfter,"expected values before and after key dependency change should be different")
62+
63+
// Setup config with caching enabled
64+
config:=configuration.NewWithOpts(configuration.WithCachingEnabled(10*time.Minute))
65+
66+
ctrl:=gomock.NewController(t)
67+
deferctrl.Finish()
68+
69+
mockEngine:=mocks.NewMockEngine(ctrl)
70+
logger:= zerolog.Logger{}
71+
72+
mockEngine.EXPECT().GetConfiguration().Return(config).AnyTimes()
73+
mockEngine.EXPECT().GetLogger().Return(&logger).AnyTimes()
74+
75+
// Create the default value function (which also registers the dependency)
76+
defaultValueFunc:=defaultValueFuncFactory(mockEngine)
77+
// Register the default value function, if it is not already registered
78+
// (the factory returns nil if it is already registered)
79+
ifdefaultValueFunc!=nil {
80+
wrappedDefaultValueFunc:=func(config configuration.Configuration,existingValueany) (any,error) {
81+
t.Logf("defaultValueFunc called")
82+
returndefaultValueFunc(config,existingValue)
83+
}
84+
config.AddDefaultValue(configKey,wrappedDefaultValueFunc)
85+
}
86+
87+
// Set initial dependency value
88+
config.Set(dependencyKey,dependencyValueBefore)
89+
90+
// First call - should compute from dependency
91+
result1,err:=config.GetWithError(configKey)
92+
require.NoError(t,err)
93+
assert.Equal(t,expectedValueBefore,result1,"First call should return value based on initial dependency")
94+
95+
// Second call with same dependency - should use cache
96+
result2,err:=config.GetWithError(configKey)
97+
require.NoError(t,err)
98+
assert.Equal(t,expectedValueBefore,result2,"Second call should return cached value")
99+
100+
// Change the dependency value
101+
config.Set(dependencyKey,dependencyValueAfter)
102+
103+
// Third call - cache should be invalidated, should compute from new dependency
104+
result3,err:=config.GetWithError(configKey)
105+
require.NoError(t,err)
106+
assert.Equal(t,expectedValueAfter,result3,"Third call should return value based on new dependency (cache invalidated)")
107+
}
108+
109+
// CheckCacheRespectOrgDependency tests ORGANIZATION-based cache invalidation with HTTP API call mocking.
110+
// Verifies cache is cleared when ORGANIZATION changes and API is called again with new org.
47111
funcCheckCacheRespectOrgDependency(
48112
t*testing.T,
49113
configKeystring,
50114
httpResponseFnfunc(isFirstCallbool)any,
51-
defaultValueFuncFactoryfunc(engine workflow.Engine)configuration.DefaultValueFunction,
115+
initFuncfunc(engine workflow.Engine)error,
52116
expectedValueBeforeOrgChangeany,
53117
expectedValueAfterOrgChangeany,
54118
) {
@@ -88,20 +152,14 @@ func CheckCacheRespectOrgDependency(
88152

89153
mockEngine.EXPECT().GetConfiguration().Return(config).AnyTimes()
90154
mockEngine.EXPECT().GetLogger().Return(&logger).AnyTimes()
155+
// Pretend to support workflow registration (init functions may register workflows, but we don't use them)
156+
mockEngine.EXPECT().Register(gomock.Any(),gomock.Any(),gomock.Any()).Return(nil,nil).AnyTimes()
91157
mockEngine.EXPECT().GetNetworkAccess().Return(mockNetworkAccess).AnyTimes()
92158
mockNetworkAccess.EXPECT().GetHttpClient().Return(httpClient).AnyTimes()
93159

94-
// Create the default value function (which also registers the dependency)
95-
defaultValueFunc:=defaultValueFuncFactory(mockEngine)
96-
// Register the default value function, if it is not already registered
97-
// (the factory returns nil if it is already registered)
98-
ifdefaultValueFunc!=nil {
99-
wrappedDefaultValueFunc:=func(config configuration.Configuration,existingValueany) (any,error) {
100-
t.Logf("defaultValueFunc called")
101-
returndefaultValueFunc(config,existingValue)
102-
}
103-
config.AddDefaultValue(configKey,wrappedDefaultValueFunc)
104-
}
160+
// Call the init function (which registers config defaults)
161+
err:=initFunc(mockEngine)
162+
require.NoError(t,err,"initFunc should not return an error")
105163

106164
// First call - should invoke API
107165
result1,err:=config.GetWithError(configKey)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp