Skip to content

fix: WIP stop using LS's API for feature flag checks [IDE-1106] #824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions domain/ide/command/command_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ func CreateFromCommandData(c *config.Config, commandData types.CommandData, srv
apiClient := snyk_api.NewSnykApiClient(c, httpClient)
return &sastEnabled{command: commandData, apiClient: apiClient, logger: c.Logger(), authenticationService: authService}, nil
case types.GetFeatureFlagStatus:
apiClient := snyk_api.NewSnykApiClient(c, httpClient)
return &featureFlagStatus{command: commandData, apiClient: apiClient, authenticationService: authService}, nil
return &featureFlagStatus{command: commandData, authenticationService: authService}, nil
case types.GetActiveUserCommand:
return &getActiveUser{command: commandData, authenticationService: authService, notifier: notifier}, nil
case types.ReportAnalyticsCommand:
Expand Down
14 changes: 5 additions & 9 deletions domain/ide/command/get_feature_flag_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
import (
"context"
"errors"
"fmt"

"github.com/snyk/go-application-framework/pkg/local_workflows/config_utils"

"github.com/snyk/snyk-ls/application/config"
"github.com/snyk/snyk-ls/infrastructure/authentication"
Expand All @@ -29,7 +30,6 @@

type featureFlagStatus struct {
command types.CommandData
apiClient snyk_api.SnykApiClient
authenticationService authentication.AuthenticationService
}

Expand Down Expand Up @@ -57,16 +57,12 @@
return nil, errors.New("invalid feature flag name argument")
}

ff := snyk_api.FeatureFlagType(ffStr)
ffResponse, err := cmd.apiClient.FeatureFlagStatus(ff)

message := fmt.Sprintf("Feature flag status for '%s': %v", ffStr, ffResponse.Ok)
logger.Debug().Msg(message)

// No need for language server to enforce which feature flags exist or not.
enabled, err := config_utils.CurrentFeatureFlagChecker().GetFeatureFlag(config.CurrentConfig().Engine(), ffStr)

Check failure on line 61 in domain/ide/command/get_feature_flag_status.go

View workflow job for this annotation

GitHub Actions / lint

undefined: config_utils.CurrentFeatureFlagChecker) (typecheck)

Check failure on line 61 in domain/ide/command/get_feature_flag_status.go

View workflow job for this annotation

GitHub Actions / lint

undefined: config_utils.CurrentFeatureFlagChecker) (typecheck)
if err != nil {
logger.Warn().Err(err).Msg("Failed to get feature flag: " + ffStr)
return snyk_api.FFResponse{Ok: false, UserMessage: err.Error()}, nil
}

return snyk_api.FFResponse{Ok: ffResponse.Ok}, nil
return snyk_api.FFResponse{Ok: enabled}, nil
}
23 changes: 14 additions & 9 deletions domain/ide/command/get_feature_flag_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,12 @@ import (
"context"
"testing"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"

"github.com/snyk/go-application-framework/pkg/local_workflows/config_utils"
"github.com/snyk/go-application-framework/pkg/mocks"

"github.com/snyk/snyk-ls/application/config"
"github.com/snyk/snyk-ls/infrastructure/authentication"
"github.com/snyk/snyk-ls/infrastructure/snyk_api"
Expand All @@ -31,16 +35,18 @@ import (
"github.com/snyk/snyk-ls/internal/types"
)

func Test_ApiClient_FeatureFlagIsEnabled(t *testing.T) {
func Test_FeatureFlagIsEnabled(t *testing.T) {
c := testutil.UnitTest(t)

// Arrange
expectedResponse := snyk_api.FFResponse{Ok: true}
featureFlag := "aFakeTestFeatureFlag"

fakeApiClient := &snyk_api.FakeApiClient{}
fakeApiClient.SetResponse("FeatureFlagStatus", expectedResponse)
ctrl := gomock.NewController(t)
mockFFC := mocks.NewMockFeatureFlagChecker(ctrl)
config_utils.SetCurrentFeatureFlagChecker(mockFFC)
mockFFC.EXPECT().GetFeatureFlag(config.CurrentConfig().Engine(), featureFlag).Return(true, nil)

featureFlagStatusCmd := setupFeatureFlagCommand(t, c, fakeApiClient)
featureFlagStatusCmd := setupFeatureFlagCommand(t, c, featureFlag)

// Execute the command
result, err := featureFlagStatusCmd.Execute(context.Background())
Expand All @@ -52,15 +58,14 @@ func Test_ApiClient_FeatureFlagIsEnabled(t *testing.T) {
assert.True(t, ffResponse.Ok)
}

func setupFeatureFlagCommand(t *testing.T, c *config.Config, fakeApiClient *snyk_api.FakeApiClient) featureFlagStatus {
func setupFeatureFlagCommand(t *testing.T, c *config.Config, featureFlag string) featureFlagStatus {
t.Helper()
provider := authentication.NewFakeCliAuthenticationProvider(c)
provider.IsAuthenticated = true

// Pass the featureFlagType to the command
// Pass the featureFlag to the command
featureFlagStatusCmd := featureFlagStatus{
apiClient: fakeApiClient,
command: types.CommandData{Arguments: []interface{}{"snykCodeConsistentIgnores"}},
command: types.CommandData{Arguments: []interface{}{featureFlag}},
authenticationService: authentication.NewAuthenticationService(
c,
provider,
Expand Down
26 changes: 7 additions & 19 deletions infrastructure/code/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,28 +24,27 @@ import (
"time"
"unicode/utf8"

"github.com/rs/zerolog"

"github.com/erni27/imcache"
"github.com/pkg/errors"
"github.com/puzpuzpuz/xsync"
"github.com/rs/zerolog"

codeClient "github.com/snyk/code-client-go"
codeClientObservability "github.com/snyk/code-client-go/observability"
"github.com/snyk/code-client-go/scan"
"github.com/snyk/go-application-framework/pkg/configuration"
"github.com/snyk/go-application-framework/pkg/utils"

"github.com/snyk/snyk-ls/internal/types"
"github.com/snyk/snyk-ls/internal/util"

"github.com/snyk/snyk-ls/application/config"
"github.com/snyk/snyk-ls/domain/snyk"
"github.com/snyk/snyk-ls/infrastructure/learn"
"github.com/snyk/snyk-ls/infrastructure/snyk_api"
"github.com/snyk/snyk-ls/internal/notification"
"github.com/snyk/snyk-ls/internal/product"
"github.com/snyk/snyk-ls/internal/progress"
"github.com/snyk/snyk-ls/internal/types"
"github.com/snyk/snyk-ls/internal/uri"
"github.com/snyk/snyk-ls/internal/util"
)

type ScanStatus struct {
Expand Down Expand Up @@ -263,7 +262,9 @@ func internalScan(ctx context.Context, sc *Scanner, folderPath types.FilePath, l
return results, err
}

if sc.useIgnoresFlow() {
codeConsistentIgnoresEnabled := sc.C.Engine().GetConfiguration().GetBool(configuration.FF_CODE_CONSISTENT_IGNORES)

if codeConsistentIgnoresEnabled {
results, err = sc.UploadAndAnalyzeWithIgnores(ctx, folderPath, files, filesToBeScanned, t)
} else {
results, err = sc.UploadAndAnalyze(ctx, files, folderPath, filesToBeScanned, t)
Expand Down Expand Up @@ -599,16 +600,3 @@ type UploadStatus struct {
UploadedFiles int
TotalFiles int
}

func (sc *Scanner) useIgnoresFlow() bool {
logger := config.CurrentConfig().Logger().With().Str("method", "code.useIgnoresFlow").Logger()
response, err := sc.SnykApiClient.FeatureFlagStatus(snyk_api.FeatureFlagSnykCodeConsistentIgnores)
if err != nil {
logger.Debug().Msg("Failed to check if the ignores experience is enabled")
return false
}
if !response.Ok && response.UserMessage != "" {
logger.Info().Msg(response.UserMessage)
}
return response.Ok
}
20 changes: 11 additions & 9 deletions infrastructure/code/code_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,27 +26,28 @@ import (
"testing"
"time"

"github.com/snyk/snyk-ls/internal/product"

"github.com/snyk/snyk-ls/internal/progress"
"github.com/snyk/snyk-ls/internal/types"
"github.com/snyk/snyk-ls/internal/vcs"

"github.com/erni27/imcache"
"github.com/golang/mock/gomock"
"github.com/sourcegraph/go-lsp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/snyk/go-application-framework/pkg/configuration"
"github.com/snyk/go-application-framework/pkg/mocks"

"github.com/snyk/snyk-ls/application/config"
"github.com/snyk/snyk-ls/domain/snyk"
"github.com/snyk/snyk-ls/infrastructure/learn"
"github.com/snyk/snyk-ls/infrastructure/learn/mock_learn"
"github.com/snyk/snyk-ls/infrastructure/snyk_api"
"github.com/snyk/snyk-ls/internal/notification"
"github.com/snyk/snyk-ls/internal/product"
"github.com/snyk/snyk-ls/internal/progress"
"github.com/snyk/snyk-ls/internal/testutil"
"github.com/snyk/snyk-ls/internal/types"
"github.com/snyk/snyk-ls/internal/uri"
"github.com/snyk/snyk-ls/internal/util"
"github.com/snyk/snyk-ls/internal/vcs"
)

// can we replace them with more succinct higher level integration tests?[keeping them for sanity for the time being]
Expand Down Expand Up @@ -522,7 +523,6 @@ func Test_Scan(t *testing.T) {
assert.Nil(t, params)
})

//nolint:dupl // test cases differ by a boolean
t.Run("Should run existing flow if feature flag is disabled", func(t *testing.T) {
c := testutil.UnitTest(t)
snykCodeMock := &FakeSnykCodeClient{C: c}
Expand All @@ -543,12 +543,14 @@ func Test_Scan(t *testing.T) {
assert.NotNil(t, params)
})

//nolint:dupl // test cases differ by a boolean
t.Run("Should run new flow if feature flag is enabled", func(t *testing.T) {
c := testutil.UnitTest(t)
snykCodeMock := &FakeSnykCodeClient{C: c}
snykApiMock := &snyk_api.FakeApiClient{CodeEnabled: true}
snykApiMock.SetResponse("FeatureFlagStatus", snyk_api.FFResponse{Ok: true})
ctrl := gomock.NewController(t)
mockConfiguration := mocks.NewMockConfiguration(ctrl)
c.Engine().SetConfiguration(mockConfiguration)
mockConfiguration.EXPECT().GetBool(configuration.FF_CODE_CONSISTENT_IGNORES).Return(true)
learnMock := mock_learn.NewMockService(gomock.NewController(t))
learnMock.
EXPECT().
Expand Down
21 changes: 0 additions & 21 deletions infrastructure/snyk_api/fake_api_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,6 @@ func (f *FakeApiClient) SetResponse(method string, response any) {
f.Responses[method] = response
}

func (f *FakeApiClient) addCallForMethod(method string, args []any) {
mutex.Lock()
defer mutex.Unlock()

if f.Calls == nil {
f.Calls = make(map[string][][]any)
}
f.Calls[method] = append(f.Calls[method], args)
}

func (f *FakeApiClient) addCall(params []any, op string) {
mutex.Lock()
defer mutex.Unlock()
Expand Down Expand Up @@ -113,14 +103,3 @@ func (f *FakeApiClient) SastSettings() (SastResponse, error) {
AutofixEnabled: f.AutofixEnabled,
}, nil
}

func (f *FakeApiClient) FeatureFlagStatus(featureFlagType FeatureFlagType) (FFResponse, error) {
f.addCallForMethod("FeatureFlagStatus", []any{featureFlagType})

if resp, ok := f.Responses["FeatureFlagStatus"]; ok {
if ffResp, ok := resp.(FFResponse); ok {
return ffResp, nil
}
}
return FFResponse{}, nil
}
33 changes: 0 additions & 33 deletions infrastructure/snyk_api/snyk_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ type FFResponse struct {

type SnykApiClient interface {
SastSettings() (SastResponse, error)
FeatureFlagStatus(featureFlagType FeatureFlagType) (FFResponse, error)
}

type SnykApiError struct {
Expand Down Expand Up @@ -134,38 +133,6 @@ func (s *SnykApiClientImpl) normalizeAPIPathForV1(c *config.Config, path string)
return path
}

func (s *SnykApiClientImpl) FeatureFlagStatus(featureFlagType FeatureFlagType) (FFResponse, error) {
if s.c.Offline() {
return FFResponse{}, nil
}
method := "snyk_api.FeatureFlagStatus"
c := config.CurrentConfig()
logger := c.Logger().With().Str("method", method).Logger()

var response FFResponse
logger.Debug().Msgf("API: Getting %s", featureFlagType)
path := s.normalizeAPIPathForV1(c, fmt.Sprintf("/cli-config/feature-flags/%s", string(featureFlagType)))
u, err := url.Parse(path)
if err != nil {
return FFResponse{}, err
}
u = s.addOrgToQuery(c, u)
logger.Debug().Str("path", path).Msg("API: Getting feature flag status")

err = s.getApiResponse(method, u.String(), &response)
if err != nil {
if strings.Contains(err.Error(), "403 Forbidden") {
logger.Debug().Msgf("Feature flag '%s' is disabled", featureFlagType)
return FFResponse{Ok: false}, nil
}
logger.Err(err).Msg("Error when calling featureFlagSettings endpoint")
return FFResponse{}, err
}

logger.Debug().Msgf("Feature flag '%s' is enabled", featureFlagType)
return response, nil
}

func (s *SnykApiClientImpl) doCall(method string, endpointPath string, requestBody []byte) ([]byte, error) {
host := s.c.SnykApi()

Expand Down
85 changes: 0 additions & 85 deletions infrastructure/snyk_api/snyk_api_pact_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,91 +152,6 @@ func TestSnykApiPact(t *testing.T) {

assert.NoError(t, err)
})

t.Run("Get feature flag status", func(t *testing.T) {
organization := orgUUID
config.CurrentConfig().SetOrganization(organization)
var featureFlagType FeatureFlagType = "snykCodeConsistentIgnores"

expectedResponse := FFResponse{
Ok: true,
}

matcher := dsl.MapMatcher{}
matcher["org"] = dsl.String(organization)

interaction := pact.AddInteraction().
WithRequest(dsl.Request{
Method: "GET",
Path: dsl.String("/v1/cli-config/feature-flags/" + featureFlagType),
Query: matcher,
Headers: dsl.MapMatcher{
"Content-Type": dsl.String("application/json"),
"Authorization": dsl.Regex("token fc763eba-0905-41c5-a27f-3934ab26786c", `^token [0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}`),
},
}).WillRespondWith(dsl.Response{
Status: 200,
Headers: dsl.MapMatcher{
"Content-Type": dsl.String("application/json"),
},
Body: dsl.Match(expectedResponse),
})
interaction.Description = "feature flag with org as query param"

test := func() error {
_, err := client.FeatureFlagStatus("snykCodeConsistentIgnores")
if err != nil {
return err
}
return nil
}

err := pact.Verify(test)

assert.NoError(t, err)
})

t.Run("Get feature flag status when disabled for a ORG", func(t *testing.T) {
organization := "00000000-0000-0000-0000-000000000099"
config.CurrentConfig().SetOrganization(organization)
featureFlagType := FeatureFlagType("snykCodeConsistentIgnores")

message := "Org " + organization + " doesn't have '" + string(featureFlagType) + "' feature enabled"
disabledResponse := FFResponse{
Ok: false,
UserMessage: message,
}

matcher := dsl.MapMatcher{}
matcher["org"] = dsl.String(organization)

interaction := pact.AddInteraction().
WithRequest(dsl.Request{
Method: "GET",
Path: dsl.String("/v1/cli-config/feature-flags/" + featureFlagType),
Query: matcher,
Headers: dsl.MapMatcher{
"Content-Type": dsl.String("application/json"),
"Authorization": dsl.Regex("token fc763eba-0905-41c5-a27f-3934ab26786c", `^token [0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}`),
},
}).WillRespondWith(dsl.Response{
Status: 200,
Headers: dsl.MapMatcher{
"Content-Type": dsl.String("application/json"),
},
Body: dsl.Match(disabledResponse),
})
interaction.Description = fmt.Sprintf("feature flag '%s' disabled for org", featureFlagType)

test := func() error {
_, err := client.FeatureFlagStatus(featureFlagType)
t.Logf("err: %+v\n", err)
return err
}

err := pact.Verify(test)
assert.NoError(t, err)
})
}

func setupPact(c *config.Config) {
Expand Down
Loading