Skip to content
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

feat: add analytics event processing [IDE-736] #712

Merged
merged 32 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
10a093b
chore: separate lint from unit tests
bastiandoetsch Oct 28, 2024
aca0e71
fix: syntax error in build.yaml
bastiandoetsch Oct 28, 2024
47f338e
fix: another syntax error
bastiandoetsch Oct 28, 2024
d5daf71
chore: make all build jobs depend on lint job
bastiandoetsch Oct 28, 2024
48684b4
feat: enable sending of event data with helper struct
bastiandoetsch Oct 28, 2024
a48a29b
chore: remove automatic release on commit
bastiandoetsch Oct 28, 2024
639133f
feat: add auth analytics
bastiandoetsch Oct 28, 2024
d7d34bc
fix: add pact test for analytics event flow
bastiandoetsch Oct 29, 2024
357252f
fix: add error to authentication event reporting
bastiandoetsch Oct 29, 2024
fa2588c
fix: add early returns
bastiandoetsch Oct 29, 2024
d765649
feat: add auth event
bastiandoetsch Oct 29, 2024
9525306
chore: remove warning
bastiandoetsch Oct 29, 2024
fca7e30
Merge branch 'main' into feat/IDE-736_add-analytics-event
bastiandoetsch Oct 29, 2024
0cee479
refactor: remove duplication
bastiandoetsch Oct 29, 2024
c0e0e4a
fix: temporary fix for failing test
bastiandoetsch Oct 29, 2024
ca52030
fix: tests
bastiandoetsch Oct 29, 2024
bff57b0
fix: update github release workflow
bastiandoetsch Oct 29, 2024
fa61de7
fix: parallelization test
bastiandoetsch Oct 29, 2024
3f29c86
fix: test
bastiandoetsch Oct 29, 2024
1eb564a
fix: cli installation in tests
bastiandoetsch Oct 29, 2024
fd54e35
fix: make cli installation independent between tests
bastiandoetsch Oct 29, 2024
1fe6fbc
fix: cli path issues for concurrent tests
bastiandoetsch Oct 29, 2024
cdd697e
fix: linting problem
bastiandoetsch Oct 29, 2024
ff1c8d6
fix: use lock file relative to cli path
bastiandoetsch Oct 29, 2024
8838d4c
fix: lock file for download - use parent dir, not cli file
bastiandoetsch Oct 29, 2024
3157acf
fix: lock file creation
bastiandoetsch Oct 29, 2024
69ae2d3
fix: linter
bastiandoetsch Oct 29, 2024
4b6e1b0
fix: race condition in tests
bastiandoetsch Oct 30, 2024
ba44271
fix: tests
bastiandoetsch Oct 30, 2024
6d625c0
chore: enable trace logging for failing test
bastiandoetsch Oct 30, 2024
e720e1f
chore: add more debug information for failing test
bastiandoetsch Oct 30, 2024
5e2ebc4
chore: disable Test_SmokeIssueCaching on windows
bastiandoetsch Oct 30, 2024
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
33 changes: 28 additions & 5 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,28 @@ on:
pull_request:

jobs:
lint:
ShawkyZ marked this conversation as resolved.
Show resolved Hide resolved
name: lint
runs-on: ubuntu-latest
steps:
- name: Prepare git
run: git config --global core.autocrlf false

- uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v5
with:
go-version-file: "./go.mod"
cache: "true"

- name: Lint source code
run: |
make tools lint

unit-tests:
name: unit tests
needs: [lint]
runs-on: ubuntu-latest
steps:
- name: Prepare git
Expand Down Expand Up @@ -49,10 +69,6 @@ jobs:
run: |
make tools

- name: Lint source code
run: |
make tools lint

- name: Run tests
env:
DEEPROXY_API_URL: ${{secrets.DEEPROXY_API_URL}}
Expand All @@ -70,6 +86,7 @@ jobs:

integration-tests:
name: integration-tests
needs: [lint]
runs-on: ${{ matrix.os }}
strategy:
matrix:
Expand Down Expand Up @@ -99,12 +116,13 @@ jobs:
run: |
make tools

- name: Run integration tests with Pact
- name: Run integration & smoke tests with Pact
if: matrix.os == 'ubuntu-latest'
env:
DEEPROXY_API_URL: ${{secrets.DEEPROXY_API_URL}}
SNYK_TOKEN: ${{secrets.SNYK_TOKEN }}
INTEG_TESTS: "true"
SMOKE_TESTS: "true"
bastiandoetsch marked this conversation as resolved.
Show resolved Hide resolved
run: |
export PATH=$PATH:~/pact/bin

Expand All @@ -121,6 +139,7 @@ jobs:
DEEPROXY_API_URL: ${{secrets.DEEPROXY_API_URL}}
SNYK_TOKEN: ${{secrets.SNYK_TOKEN }}
INTEG_TESTS: "true"
SMOKE_TESTS: "true"
run: |
export PATH=$PATH:~/pact/bin

Expand All @@ -136,12 +155,14 @@ jobs:
DEEPROXY_API_URL: ${{secrets.DEEPROXY_API_URL}}
SNYK_TOKEN: ${{secrets.SNYK_TOKEN }}
INTEG_TESTS: "true"
SMOKE_TESTS: "true"
run: |
make clean test


proxy-test:
name: proxy-test
needs: [lint]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand All @@ -154,6 +175,7 @@ jobs:

race-tests:
name: race-test
needs: [lint]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -216,6 +238,7 @@ jobs:
push: true
test-release:
name: test-release
needs: [lint, unit-tests]
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -133,24 +133,30 @@ jobs:
chmod +x build/snyk-ls_linux_amd64_v1/snyk-ls
build/snyk-ls_linux_amd64_v1/snyk-ls -licenses

# we only want to upload when we consciously release, not on merge
- name: Login to AWS
if: {{ github.event_name == 'workflow_dispatch }}
run: |
.github/setup_aws_credentials.py \
--role-arn "arn:aws:iam::198361731867:role/Snyk-Assets-WriteOnly" \
--region "${{ secrets.AWS_REGION }}"

- name: Upload binaries to static.snyk.io
if: {{ github.event_name == 'workflow_dispatch }}
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
AWS_S3_BUCKET_NAME: ${{ secrets.AWS_S3_BUCKET_NAME }}
run: |
.github/upload-to-s3.sh

# creating PR in cli repository needs ssh agent
- uses: webfactory/[email protected]
if: {{ github.event_name != 'workflow_dispatch }}
with:
ssh-private-key: ${{ secrets.TEAM_IDE_USER_SSH }}

- name: Create PR in CLI to integrate LS
if: {{ github.event_name != 'workflow_dispatch }}
env:
GH_TOKEN: ${{ secrets.HAMMERHEAD_GITHUB_PAT_SNYKLS }}
GITHUB_TOKEN: ${{ secrets.HAMMERHEAD_GITHUB_PAT_SNYKLS }}
Expand Down
16 changes: 14 additions & 2 deletions application/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,9 +358,21 @@ func (c *Config) Format() string {
defer c.m.Unlock()
return c.format
}
func (c *Config) CLIDownloadLockFileName() string {
return filepath.Join(c.cliSettings.DefaultBinaryInstallPath(), "snyk-cli-download.lock")
func (c *Config) CLIDownloadLockFileName() (string, error) {
c.cliSettings.cliPathAccessMutex.Lock()
defer c.cliSettings.cliPathAccessMutex.Unlock()
var path string
if c.cliSettings.cliPath == "" {
c.cliSettings.cliPath = c.cliSettings.DefaultBinaryInstallPath()
}
path = filepath.Dir(c.cliSettings.cliPath)
err := os.MkdirAll(path, 0755)
if err != nil {
return "", err
}
return filepath.Join(path, "snyk-cli-download.lock"), nil
}

func (c *Config) IsErrorReportingEnabled() bool { return c.isErrorReportingEnabled.Get() }
func (c *Config) IsSnykOssEnabled() bool { return c.isSnykOssEnabled.Get() }
func (c *Config) IsSnykCodeEnabled() bool { return c.isSnykCodeEnabled.Get() }
Expand Down
4 changes: 1 addition & 3 deletions application/server/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package server

import (
"context"
"reflect"
"time"

"github.com/rs/zerolog"
Expand All @@ -32,7 +31,6 @@ import (
)

func notifier(c *config.Config, srv types.Server, method string, params any) {
c.Logger().Debug().Str("method", "notifier").Str("type", reflect.TypeOf(params).String()).Msgf("Notifying")
err := srv.Notify(context.Background(), method, params)
logError(c.Logger(), err, "notifier")
}
Expand Down Expand Up @@ -99,7 +97,7 @@ func registerNotifier(c *config.Config, srv types.Server) {
logger.Debug().Msg("sending cli path")
case sglsp.ShowMessageParams:
notifier(c, srv, "window/showMessage", params)
logger.Debug().Interface("message", params).Msg("showing message")
logger.Debug().Interface("message", params.Message).Msg("showing message")
acke marked this conversation as resolved.
Show resolved Hide resolved
case types.SnykTrustedFoldersParams:
notifier(c, srv, "$/snyk.addTrustedFolders", params)
logger.Info().
Expand Down
4 changes: 4 additions & 0 deletions application/server/parallelization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ func Test_Concurrent_CLI_Runs(t *testing.T) {
}
wg.Wait()

setUniqueCliPath(t, c)

clientParams := types.InitializeParams{
WorkspaceFolders: workspaceFolders,
InitializationOptions: types.Settings{
Expand All @@ -79,6 +81,8 @@ func Test_Concurrent_CLI_Runs(t *testing.T) {
FilterSeverity: types.DefaultSeverityFilter(),
AuthenticationMethod: types.TokenAuthentication,
AutomaticAuthentication: "false",
ManageBinariesAutomatically: "true",
CliPath: c.CliSettings().Path(),
},
}

Expand Down
55 changes: 46 additions & 9 deletions application/server/server_smoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ import (
"os"
"path"
"path/filepath"
"runtime"
"strconv"
"strings"
"testing"
"time"

"github.com/creachadair/jrpc2"
"github.com/creachadair/jrpc2/server"
"github.com/rs/zerolog"
"github.com/snyk/go-application-framework/pkg/configuration"
sglsp "github.com/sourcegraph/go-lsp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -37,6 +40,8 @@ import (
"github.com/snyk/snyk-ls/application/di"
"github.com/snyk/snyk-ls/domain/ide/hover"
"github.com/snyk/snyk-ls/domain/ide/workspace"
"github.com/snyk/snyk-ls/domain/snyk"
"github.com/snyk/snyk-ls/infrastructure/cli/install"
"github.com/snyk/snyk-ls/infrastructure/code"
"github.com/snyk/snyk-ls/internal/product"
"github.com/snyk/snyk-ls/internal/testutil"
Expand Down Expand Up @@ -141,6 +146,7 @@ func Test_SmokeWorkspaceScan(t *testing.T) {
}

func Test_SmokeIssueCaching(t *testing.T) {
testutil.NotOnWindows(t, "git clone fails on juiceshop ") // TODO remove & fix
t.Run("adds issues to cache correctly", func(t *testing.T) {
loc, jsonRPCRecorder := setupServer(t)
c := testutil.SmokeTest(t, false)
Expand All @@ -161,15 +167,25 @@ func Test_SmokeIssueCaching(t *testing.T) {
ossIssuesForFile := folderGoof.IssuesForFile(filepath.Join(cloneTargetDirGoof, "package.json"))
require.Greater(t, len(ossIssuesForFile), 1) // 108 is the number of issues in the package.json file as of now

codeIssuesForFile := folderGoof.IssuesForFile(filepath.Join(cloneTargetDirGoof, "app.js"))
require.Greater(t, len(codeIssuesForFile), 1) // 5 is the number of issues in the app.js file as of now
var codeIssuesForFile []snyk.Issue

require.Eventually(t, func() bool {
codeIssuesForFile = folderGoof.IssuesForFile(filepath.Join(cloneTargetDirGoof, "app.js"))
return len(codeIssuesForFile) > 1
}, time.Second*5, time.Second)

checkDiagnosticPublishingForCachingSmokeTest(t, jsonRPCRecorder, 1, 1, c)

jsonRPCRecorder.ClearNotifications()
jsonRPCRecorder.ClearCallbacks()

// now we add juice shop as second folder/repo
if runtime.GOOS == "windows" {
t.Setenv("SNYK_LOG_LEVEL", "trace")
c.ConfigureLogging(nil)
c.SetLogLevel(zerolog.TraceLevel.String())
}

folderJuice := addJuiceShopAsWorkspaceFolder(t, loc, c)

// scan both created folders
Expand Down Expand Up @@ -434,7 +450,7 @@ func checkDiagnosticPublishingForCachingSmokeTest(
packageJsonCount == expectedOSS

return result
}, time.Second*5, time.Second)
}, time.Second*600, time.Second)
acke marked this conversation as resolved.
Show resolved Hide resolved
}

func runSmokeTest(t *testing.T, repo string, commit string, file1 string, file2 string, useConsistentIgnores bool,
Expand Down Expand Up @@ -657,7 +673,6 @@ func getIssueListFromPublishDiagnosticsNotification(t *testing.T, jsonRPCRecorde
issueList = append(issueList, diagnostic.Data)
}
}

return issueList
}

Expand Down Expand Up @@ -712,6 +727,8 @@ func prepareInitParams(t *testing.T, cloneTargetDir string, c *config.Config) ty
Uri: uri.PathToUri(cloneTargetDir),
}

setUniqueCliPath(t, c)

clientParams := types.InitializeParams{
WorkspaceFolders: []types.WorkspaceFolder{folder},
InitializationOptions: types.Settings{
Expand All @@ -724,11 +741,20 @@ func prepareInitParams(t *testing.T, cloneTargetDir string, c *config.Config) ty
ActivateSnykCode: strconv.FormatBool(c.IsSnykCodeEnabled()),
ActivateSnykIac: strconv.FormatBool(c.IsSnykIacEnabled()),
ActivateSnykOpenSource: strconv.FormatBool(c.IsSnykOssEnabled()),
ActivateSnykCodeQuality: strconv.FormatBool(c.IsSnykCodeQualityEnabled()),
acke marked this conversation as resolved.
Show resolved Hide resolved
ActivateSnykCodeSecurity: strconv.FormatBool(c.IsSnykCodeSecurityEnabled()),
CliPath: c.CliSettings().Path(),
},
}
return clientParams
}

func setUniqueCliPath(t *testing.T, c *config.Config) {
t.Helper()
discovery := install.Discovery{}
c.CliSettings().SetPath(filepath.Join(t.TempDir(), discovery.ExecutableName(false)))
}

func checkFeatureFlagStatus(t *testing.T, c *config.Config, loc *server.Local) {
t.Helper()
// only check on mt-us
Expand Down Expand Up @@ -795,7 +821,7 @@ func Test_SmokeSnykCodeFileScan(t *testing.T) {

_ = textDocumentDidSave(t, &loc, testPath)

assert.Eventually(t, checkForPublishedDiagnostics(t, testPath, 6, jsonRPCRecorder), maxIntegTestDuration, 10*time.Millisecond)
assert.Eventually(t, checkForPublishedDiagnostics(t, testPath, -1, jsonRPCRecorder), 2*time.Minute, 10*time.Millisecond)
}

func Test_SmokeUncFilePath(t *testing.T) {
Expand Down Expand Up @@ -825,7 +851,7 @@ func Test_SmokeUncFilePath(t *testing.T) {
assert.Eventually(t, checkForPublishedDiagnostics(t, testPath, -1, jsonRPCRecorder), maxIntegTestDuration, 10*time.Millisecond)
}

func Test_SmokeSnykCodeDelta_OneNewVuln(t *testing.T) {
func Test_SmokeSnykCodeDelta_NewVulns(t *testing.T) {
loc, jsonRPCRecorder := setupServer(t)
c := testutil.SmokeTest(t, false)
c.SetSnykCodeEnabled(true)
Expand All @@ -834,11 +860,18 @@ func Test_SmokeSnykCodeDelta_OneNewVuln(t *testing.T) {
di.Init()

fileWithNewVulns := "vulns.js"
var cloneTargetDir, err = testutil.SetupCustomTestRepo(t, t.TempDir(), "https://github.com/snyk-labs/nodejs-goof", "0336589", c.Logger())
var cloneTargetDir, err = testutil.SetupCustomTestRepo(t, t.TempDir(), nodejsGoof, "0336589", c.Logger())
assert.NoError(t, err)

newFileInCurrentDir(t, cloneTargetDir, fileWithNewVulns, "var token = 'SECRET_TOKEN_f8ed84e8f41e4146403dd4a6bbcea5e418d23a9';")
sourceContent, err := os.ReadFile(filepath.Join(cloneTargetDir, "app.js"))
require.NoError(t, err)

newFileInCurrentDir(t, cloneTargetDir, fileWithNewVulns, string(sourceContent))

c.SetSnykOssEnabled(false)
c.SetSnykIacEnabled(false)
c.EnableSnykCodeQuality(false)
c.SetManageBinariesAutomatically(false)
initParams := prepareInitParams(t, cloneTargetDir, c)

ensureInitialized(t, c, loc, initParams)
Expand All @@ -848,7 +881,7 @@ func Test_SmokeSnykCodeDelta_OneNewVuln(t *testing.T) {
checkForScanParams(t, jsonRPCRecorder, cloneTargetDir, product.ProductCode)
issueList := getIssueListFromPublishDiagnosticsNotification(t, jsonRPCRecorder, product.ProductCode, cloneTargetDir)

assert.Equal(t, len(issueList), 1)
assert.Greater(t, len(issueList), 0)
assert.Contains(t, issueList[0].FilePath, fileWithNewVulns)
}

Expand Down Expand Up @@ -903,6 +936,10 @@ func Test_SmokeSnykCodeDelta_NoNewIssuesFound(t *testing.T) {

func ensureInitialized(t *testing.T, c *config.Config, loc server.Local, initParams types.InitializeParams) {
t.Helper()
// temporary until policy engine doesn't output to stdout anymore
t.Setenv("SNYK_LOG_LEVEL", "info")
c.ConfigureLogging(nil)
c.Engine().GetConfiguration().Set(configuration.DEBUG, false)

_, err := loc.Client.Call(ctx, "initialize", initParams)
assert.NoError(t, err)
Expand Down
6 changes: 3 additions & 3 deletions domain/ide/command/get_active_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func Test_getActiveUser_Execute_User_found(t *testing.T) {

expectedUser, expectedUserData := whoamiWorkflowResponse(t)

mockEngine, engineConfig := setUpEngineMock(t, c)
mockEngine, engineConfig := testutil.SetUpEngineMock(t, c)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick

Suggested change
mockEngine, engineConfig := testutil.SetUpEngineMock(t, c)
mockEngine, engineConfig := testutil.SetupEngineMock(t, c)

mockEngine.EXPECT().GetConfiguration().Return(engineConfig).AnyTimes()
mockEngine.EXPECT().InvokeWithConfig(localworkflows.WORKFLOWID_WHOAMI, gomock.Any()).Return(expectedUserData, nil)

Expand Down Expand Up @@ -76,7 +76,7 @@ func Test_getActiveUser_Execute_Result_Empty(t *testing.T) {
c := testutil.UnitTest(t)
cmd := setupCommandWithAuthService(t, c)

mockEngine, engineConfig := setUpEngineMock(t, c)
mockEngine, engineConfig := testutil.SetUpEngineMock(t, c)
mockEngine.EXPECT().GetConfiguration().Return(engineConfig).AnyTimes()
mockEngine.EXPECT().InvokeWithConfig(localworkflows.WORKFLOWID_WHOAMI, gomock.Any()).Return([]workflow.Data{}, nil)

Expand All @@ -90,7 +90,7 @@ func Test_getActiveUser_Execute_Error_Result(t *testing.T) {
c := testutil.UnitTest(t)
cmd := setupCommandWithAuthService(t, c)

mockEngine, engineConfig := setUpEngineMock(t, c)
mockEngine, engineConfig := testutil.SetUpEngineMock(t, c)
mockEngine.EXPECT().GetConfiguration().Return(engineConfig).AnyTimes()
testError := errors.New("test error")
mockEngine.EXPECT().InvokeWithConfig(localworkflows.WORKFLOWID_WHOAMI, gomock.Any()).Return([]workflow.Data{}, testError)
Expand Down
Loading