Skip to content

Commit a59aa7d

Browse files
bsalomon-snykbsalomon-snyk
authored
Merge pull request #27 from snyk/feat/unified-test-by-default
feat: DGP-691 - defaults OS test to use the unified Test API
2 parents 98c22a2 + 21881c4 commit a59aa7d

File tree

3 files changed

+77
-105
lines changed

3 files changed

+77
-105
lines changed

‎internal/commands/ostest/ostest.go

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ const FeatureFlagRiskScore = "feature_flag_experimental_risk_score"
4545
// FeatureFlagRiskScoreInCLI is used to gate the risk score feature in the CLI.
4646
const FeatureFlagRiskScoreInCLI = "feature_flag_experimental_risk_score_in_cli"
4747

48+
// ForceLegacyCLIEnvVar is an internal environment variable to force the legacy CLI flow.
49+
const ForceLegacyCLIEnvVar = "SNYK_FORCE_LEGACY_CLI"
50+
4851
// ApplicationJSONContentType matches the content type for legacy JSON findings records.
4952
const ApplicationJSONContentType = "application/json"
5053

@@ -107,23 +110,33 @@ func OSWorkflow(
107110
return runReachabilityFlow(ctx, config, errFactory, ictx, logger, sbom, sourceDir)
108111

109112
default:
110-
riskScoreThreshold := config.GetInt(flags.FlagRiskScoreThreshold)
111-
if !config.GetBool(flags.FlagUnifiedTestAPI) && riskScoreThreshold == -1 {
112-
logger.Debug().Msg("Using legacy flow since risk score threshold and unified test flags are disabled")
113+
if config.GetBool(ForceLegacyCLIEnvVar) {
114+
logger.Debug().Msgf("Using legacy flow due to %s environment variable.", ForceLegacyCLIEnvVar)
113115
return code_workflow.EntryPointLegacy(ictx)
114116
}
115117

116-
// Unified test flow (with risk score threshold or unified-test flag)
118+
ffRiskScore := config.GetBool(FeatureFlagRiskScore)
119+
ffRiskScoreInCLI := config.GetBool(FeatureFlagRiskScoreInCLI)
120+
useUnifiedFlow := ffRiskScore && ffRiskScoreInCLI
117121

118-
if riskScoreThreshold >= 0 {
119-
if !config.GetBool(FeatureFlagRiskScore) {
122+
// The unified test flow is only used if both risk score feature flags are enabled.
123+
riskScoreThreshold := config.GetInt(flags.FlagRiskScoreThreshold)
124+
if riskScoreThreshold != -1 && !useUnifiedFlow {
125+
// The user tried to use a risk score threshold without the required feature flags.
126+
// Return a specific error for the first missing flag found.
127+
if !ffRiskScore {
120128
return nil, errFactory.NewFeatureNotPermittedError(FeatureFlagRiskScore)
121129
}
122-
if !config.GetBool(FeatureFlagRiskScoreInCLI) {
123-
return nil, errFactory.NewFeatureNotPermittedError(FeatureFlagRiskScoreInCLI)
124-
}
130+
return nil, errFactory.NewFeatureNotPermittedError(FeatureFlagRiskScoreInCLI)
125131
}
126132

133+
if !useUnifiedFlow {
134+
logger.Debug().Msg("Using legacy flow since one or both experimental risk score feature flags are not enabled.")
135+
return code_workflow.EntryPointLegacy(ictx)
136+
}
137+
138+
// Unified test flow
139+
127140
filename := config.GetString(flags.FlagFile)
128141
if filename == "" {
129142
logger.Error().Msg("No file specified for testing")

‎internal/commands/ostest/ostest_test.go

Lines changed: 55 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -39,85 +39,37 @@ func TestOSWorkflow_LegacyFlow(t *testing.T) {
3939
assert.NoError(t, err)
4040
}
4141

42-
// TestOSWorkflow_UnifiedTestFlag tests the workflow when run with the unified test flag.
43-
func TestOSWorkflow_UnifiedTestFlag(t *testing.T) {
44-
// Setup - Unified test flag set to true
45-
ctrl := gomock.NewController(t)
46-
defer ctrl.Finish()
47-
48-
mockEngine := mocks.NewMockEngine(ctrl)
49-
mockInvocationCtx := createMockInvocationCtxWithURL(t, ctrl, mockEngine, mockServerURL)
50-
51-
// Set the unified test flag and required risk score feature flags
52-
mockInvocationCtx.GetConfiguration().Set(flags.FlagUnifiedTestAPI, true)
53-
mockInvocationCtx.GetConfiguration().Set(ostest.FeatureFlagRiskScore, true)
54-
mockInvocationCtx.GetConfiguration().Set(ostest.FeatureFlagRiskScoreInCLI, true)
55-
56-
// Mock the depgraph workflow
57-
mockEngine.EXPECT().
58-
InvokeWithConfig(gomock.Any(), gomock.Any()).
59-
Return(nil, assert.AnError).
60-
Times(1)
61-
62-
// Execute
63-
_, err := ostest.OSWorkflow(mockInvocationCtx, []workflow.Data{})
64-
65-
// Verify - Should return error from depgraph workflow
66-
assert.Error(t, err)
67-
assert.Contains(t, err.Error(), "failed to create depgraph")
68-
}
69-
70-
// TestOSWorkflow_RiskScoreThreshold tests the workflow when run with a risk score threshold.
71-
func TestOSWorkflow_RiskScoreThreshold(t *testing.T) {
72-
// Setup - Risk score threshold set
73-
ctrl := gomock.NewController(t)
74-
defer ctrl.Finish()
75-
76-
mockEngine := mocks.NewMockEngine(ctrl)
77-
mockInvocationCtx := createMockInvocationCtxWithURL(t, ctrl, mockEngine, mockServerURL)
78-
79-
// Set a risk score threshold
80-
mockInvocationCtx.GetConfiguration().Set(flags.FlagRiskScoreThreshold, 700)
81-
// Enable Risk Score feature flags for this test case
82-
mockInvocationCtx.GetConfiguration().Set(ostest.FeatureFlagRiskScore, true)
83-
mockInvocationCtx.GetConfiguration().Set(ostest.FeatureFlagRiskScoreInCLI, true)
84-
85-
// Mock the depgraph workflow
86-
mockEngine.EXPECT().
87-
InvokeWithConfig(gomock.Any(), gomock.Any()).
88-
Return(nil, assert.AnError).
89-
Times(1)
90-
91-
// Execute
92-
_, err := ostest.OSWorkflow(mockInvocationCtx, []workflow.Data{})
93-
94-
// Verify - Should return error from depgraph workflow
95-
assert.Error(t, err)
96-
assert.Contains(t, err.Error(), "failed to create depgraph")
97-
}
98-
99-
// TestOSWorkflow_SBOMReachabilityFlag_MissingFF tests requirement of the SBOM reachability feature flag.
100-
func TestOSWorkflow_SBOMReachabilityFlag_MissingFF(t *testing.T) {
101-
// Setup - Unified test flag set to true
102-
ctrl := gomock.NewController(t)
103-
defer ctrl.Finish()
104-
105-
mockEngine := mocks.NewMockEngine(ctrl)
106-
mockInvocationCtx := createMockInvocationCtxWithURL(t, ctrl, mockEngine, mockServerURL)
107-
108-
// Set the sbom reachability flags
109-
mockInvocationCtx.GetConfiguration().Set(flags.FlagReachability, true)
110-
mockInvocationCtx.GetConfiguration().Set(flags.FlagSBOM, "bom.json")
111-
112-
// Execute
113-
_, err := ostest.OSWorkflow(mockInvocationCtx, []workflow.Data{})
114-
115-
// Verify - Should return feature not permitted error
116-
assert.Error(t, err)
117-
assert.Contains(t, err.Error(), "The feature you are trying to use is not available for your organization")
42+
func TestOSWorkflow_ForceLegacyFlowWithEnvVar(t *testing.T) {
43+
t.Run("forces legacy flow when env var is set, even with unified flow flags", func(t *testing.T) {
44+
ctrl := gomock.NewController(t)
45+
defer ctrl.Finish()
46+
47+
mockEngine := mocks.NewMockEngine(ctrl)
48+
mockInvocationCtx := createMockInvocationCtxWithURL(t, ctrl, mockEngine, mockServerURL)
49+
50+
// Setup: set the env var and all flags that would normally trigger the unified flow
51+
config := mockInvocationCtx.GetConfiguration()
52+
config.Set(ostest.ForceLegacyCLIEnvVar, true)
53+
config.Set(ostest.FeatureFlagRiskScore, true)
54+
config.Set(ostest.FeatureFlagRiskScoreInCLI, true)
55+
config.Set(flags.FlagRiskScoreThreshold, 500)
56+
57+
// Mock the legacy flow, which should be called despite the unified flow flags
58+
mockEngine.EXPECT().
59+
InvokeWithConfig(gomock.Any(), gomock.Any()).
60+
Return([]workflow.Data{}, nil).
61+
Times(1)
62+
63+
// Execute
64+
_, err := ostest.OSWorkflow(mockInvocationCtx, []workflow.Data{})
65+
66+
// Verify
67+
assert.NoError(t, err)
68+
})
11869
}
11970

120-
// TODO: Test combinations with sbom and reachability flags.
71+
// TestOSWorkflow_FlagCombinations tests various flag combinations to ensure correct routing
72+
// between the legacy, unified, and reachability test flows.
12173
func TestOSWorkflow_FlagCombinations(t *testing.T) {
12274
tests := []struct {
12375
name string
@@ -127,7 +79,6 @@ func TestOSWorkflow_FlagCombinations(t *testing.T) {
12779
{
12880
name: "Unified test API flag set to true, expects depgraph error",
12981
setup: func(config configuration.Configuration, mockEngine *mocks.MockEngine) {
130-
config.Set(flags.FlagUnifiedTestAPI, true)
13182
config.Set(ostest.FeatureFlagRiskScore, true)
13283
config.Set(ostest.FeatureFlagRiskScoreInCLI, true)
13384
mockEngine.EXPECT().
@@ -167,20 +118,6 @@ func TestOSWorkflow_FlagCombinations(t *testing.T) {
167118
},
168119
expectedError: "failed to create depgraph",
169120
},
170-
{
171-
name: "Both unified test and risk score set",
172-
setup: func(config configuration.Configuration, mockEngine *mocks.MockEngine) {
173-
config.Set(flags.FlagUnifiedTestAPI, true)
174-
config.Set(flags.FlagRiskScoreThreshold, 700)
175-
config.Set(ostest.FeatureFlagRiskScore, true)
176-
config.Set(ostest.FeatureFlagRiskScoreInCLI, true)
177-
mockEngine.EXPECT().
178-
InvokeWithConfig(gomock.Any(), gomock.Any()).
179-
Return(nil, assert.AnError).
180-
Times(1)
181-
},
182-
expectedError: "failed to create depgraph",
183-
},
184121
{
185122
name: "SBOM reachability without feature flag",
186123
setup: func(config configuration.Configuration, _ *mocks.MockEngine) {
@@ -193,7 +130,8 @@ func TestOSWorkflow_FlagCombinations(t *testing.T) {
193130
{
194131
name: "Severity threshold set with unified test flag, expects depgraph error",
195132
setup: func(config configuration.Configuration, mockEngine *mocks.MockEngine) {
196-
config.Set(flags.FlagUnifiedTestAPI, true)
133+
config.Set(ostest.FeatureFlagRiskScore, true)
134+
config.Set(ostest.FeatureFlagRiskScoreInCLI, true)
197135
config.Set(flags.FlagSeverityThreshold, "high")
198136
mockEngine.EXPECT().
199137
InvokeWithConfig(gomock.Any(), gomock.Any()).
@@ -227,6 +165,30 @@ func TestOSWorkflow_FlagCombinations(t *testing.T) {
227165
},
228166
expectedError: "", // No error, should succeed via legacy path
229167
},
168+
{
169+
name: "Only one risk score FF enabled, uses legacy flow",
170+
setup: func(config configuration.Configuration, mockEngine *mocks.MockEngine) {
171+
config.Set(ostest.FeatureFlagRiskScore, true)
172+
// ffRiskScoreInCLI is false by default
173+
mockEngine.EXPECT().
174+
InvokeWithConfig(gomock.Any(), gomock.Any()).
175+
Return([]workflow.Data{}, nil).
176+
Times(1)
177+
},
178+
expectedError: "", // No error, should succeed via legacy path
179+
},
180+
{
181+
name: "Only CLI risk score FF enabled, uses legacy flow",
182+
setup: func(config configuration.Configuration, mockEngine *mocks.MockEngine) {
183+
config.Set(ostest.FeatureFlagRiskScoreInCLI, true)
184+
// ffRiskScore is false by default
185+
mockEngine.EXPECT().
186+
InvokeWithConfig(gomock.Any(), gomock.Any()).
187+
Return([]workflow.Data{}, nil).
188+
Times(1)
189+
},
190+
expectedError: "", // No error, should succeed via legacy path
191+
},
230192
}
231193

232194
for _, test := range tests {
@@ -266,7 +228,6 @@ func createMockInvocationCtxWithURL(t *testing.T, ctrl *gomock.Controller, engin
266228
mockConfig.Set(configuration.API_URL, sbomServiceURL)
267229

268230
// Initialize with default values for our flags
269-
mockConfig.Set(flags.FlagUnifiedTestAPI, false)
270231
mockConfig.Set(flags.FlagRiskScoreThreshold, -1)
271232
mockConfig.Set(flags.FlagFile, "test-file.txt") // Add default test file
272233

‎internal/flags/flags.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ const (
99
FlagProjectName = "project-name"
1010
FlagRiskScoreThreshold = "risk-score-threshold"
1111
FlagSeverityThreshold = "severity-threshold"
12-
FlagUnifiedTestAPI = "unified-test"
1312

1413
// SBOM reachability.
1514
FlagReachability = "reachability"
@@ -67,7 +66,6 @@ func OSTestFlagSet() *pflag.FlagSet {
6766
flagSet.String(FlagFile, "", "Specify a package file.")
6867
flagSet.String(FlagProjectName, "", "Specify a name for the project.")
6968

70-
flagSet.Bool(FlagUnifiedTestAPI, false, "Use the unified test API workflow.")
7169
flagSet.Int(FlagRiskScoreThreshold, -1, "Include findings at or over this risk score threshold.")
7270
flagSet.String(FlagSeverityThreshold, "", "Report only findings at the specified level or higher.")
7371

0 commit comments

Comments
 (0)