From 0d1ae91278afea5985d8072beaad7bf71780124e Mon Sep 17 00:00:00 2001 From: Abdelrahman Shawki Hassan Date: Mon, 4 Nov 2024 15:16:46 +0100 Subject: [PATCH] fix: delete lsconfig content if invalid json --- .github/upload-to-s3.sh | 8 ++-- .github/workflows/build.yaml | 34 ++++++++++++--- .github/workflows/release.yaml | 6 +++ application/server/server_smoke_test.go | 4 +- infrastructure/oss/oss_integration_test.go | 6 +++ internal/storage/storage.go | 48 +++++++++++++++++++++- internal/storage/storage_test.go | 24 +++++++++++ 7 files changed, 116 insertions(+), 14 deletions(-) diff --git a/.github/upload-to-s3.sh b/.github/upload-to-s3.sh index 2890d501a..e5d5eff4d 100755 --- a/.github/upload-to-s3.sh +++ b/.github/upload-to-s3.sh @@ -68,7 +68,7 @@ function uploadFile() { uploadFile $FILENAME_SRC $FILENAME_DST $DRY_RUN copyOrDownloadToTemp $FILENAME_SRC "$FILENAME_DST" $DRY_RUN - FILENAME_SRC="$SCRIPT_DIR/../build/snyk-ls_darwin_arm64/snyk-ls" + FILENAME_SRC="$SCRIPT_DIR/../build/snyk-ls_darwin_arm64_v8.0/snyk-ls" FILENAME_DST="snyk-ls_${VERSION}_darwin_arm64" # shellcheck disable=SC2086 uploadFile $FILENAME_SRC $FILENAME_DST $DRY_RUN @@ -80,19 +80,19 @@ function uploadFile() { uploadFile $FILENAME_SRC $FILENAME_DST $DRY_RUN copyOrDownloadToTemp $FILENAME_SRC "$FILENAME_DST" $DRY_RUN - FILENAME_SRC="$SCRIPT_DIR/../build/snyk-ls_linux_386/snyk-ls" + FILENAME_SRC="$SCRIPT_DIR/../build/snyk-ls_linux_386_sse2/snyk-ls" FILENAME_DST="snyk-ls_${VERSION}_linux_386" # shellcheck disable=SC2086 uploadFile $FILENAME_SRC $FILENAME_DST $DRY_RUN copyOrDownloadToTemp $FILENAME_SRC "$FILENAME_DST" $DRY_RUN - FILENAME_SRC="$SCRIPT_DIR/../build/snyk-ls_linux_arm64/snyk-ls" + FILENAME_SRC="$SCRIPT_DIR/../build/snyk-ls_linux_arm64_v8.0/snyk-ls" FILENAME_DST="snyk-ls_${VERSION}_linux_arm64" # shellcheck disable=SC2086 uploadFile $FILENAME_SRC $FILENAME_DST $DRY_RUN copyOrDownloadToTemp $FILENAME_SRC "$FILENAME_DST" $DRY_RUN - FILENAME_SRC="$SCRIPT_DIR/../build/snyk-ls_windows_386/snyk-ls.exe" + FILENAME_SRC="$SCRIPT_DIR/../build/snyk-ls_windows_386_sse2/snyk-ls.exe" FILENAME_DST="snyk-ls_${VERSION}_windows_386.exe" # shellcheck disable=SC2086 uploadFile $FILENAME_SRC $FILENAME_DST $DRY_RUN diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 5eff19681..ae19aa617 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -18,8 +18,28 @@ on: pull_request: jobs: + lint: + 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 @@ -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}} @@ -70,6 +86,7 @@ jobs: integration-tests: name: integration-tests + needs: [lint] runs-on: ${{ matrix.os }} strategy: matrix: @@ -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" run: | export PATH=$PATH:~/pact/bin @@ -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 @@ -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 @@ -154,6 +175,7 @@ jobs: race-tests: name: race-test + needs: [lint] runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -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 @@ -259,4 +282,3 @@ jobs: AWS_S3_BUCKET_NAME: ${{ secrets.AWS_S3_BUCKET_NAME }} run: | .github/upload-to-s3.sh --dryrun - diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 9723a7d1e..a4db01773 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -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/ssh-agent@v0.9.0 + 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 }} diff --git a/application/server/server_smoke_test.go b/application/server/server_smoke_test.go index e2794592a..50af43471 100644 --- a/application/server/server_smoke_test.go +++ b/application/server/server_smoke_test.go @@ -550,7 +550,7 @@ func checkOnlyOneQuickFixCodeAction(t *testing.T, jsonRPCRecorder *testutil.Json // "tap": "^11.1.3", 12 fixable, 11 unfixable if issue.Range.Start.Line == 46 && isQuickfixAction { - assert.Contains(t, action.Title, "and fix 23 issues") + assert.Contains(t, action.Title, "and fix 24 issues") } } // no issues should have more than one quickfix @@ -602,7 +602,7 @@ func checkOnlyOneCodeLens(t *testing.T, jsonRPCRecorder *testutil.JsonRPCRecorde // "tap": "^11.1.3", 12 fixable, 11 unfixable if lens.Range.Start.Line == 46 { - assert.Contains(t, lens.Command.Title, "and fix 23 issues") + assert.Contains(t, lens.Command.Title, "and fix 24 issues") } } } diff --git a/infrastructure/oss/oss_integration_test.go b/infrastructure/oss/oss_integration_test.go index 4128fe8f7..5ec703a90 100644 --- a/infrastructure/oss/oss_integration_test.go +++ b/infrastructure/oss/oss_integration_test.go @@ -23,6 +23,7 @@ import ( "strings" "testing" + "github.com/snyk/go-application-framework/pkg/configuration" "github.com/stretchr/testify/assert" "github.com/snyk/snyk-ls/application/config" @@ -69,6 +70,11 @@ func Test_Scan(t *testing.T) { workingDir, _ := os.Getwd() path, _ := filepath.Abs(filepath.Join(workingDir, "testdata", "package.json")) + // 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) + issues, _ := scanner.Scan(ctx, path, workingDir) assert.NotEqual(t, 0, len(issues)) diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 6f2af9fd4..92ee27a75 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -18,6 +18,10 @@ package storage import ( "context" + "encoding/json" + "errors" + "os" + "sync" "time" "github.com/adrg/xdg" @@ -36,23 +40,50 @@ type StorageWithCallbacks interface { type storage struct { callbacks map[string]StorageCallbackFunc jsonStorage *configuration.JsonStorage + storageFile string + mutex sync.RWMutex } func (s *storage) Refresh(config configuration.Configuration, key string) error { - return s.jsonStorage.Refresh(config, key) + s.mutex.Lock() + + contents, err := os.ReadFile(s.storageFile) + if err != nil { + s.mutex.Unlock() + return err + } + doc := map[string]interface{}{} + err = json.Unmarshal(contents, &doc) + if err != nil { + s.mutex.Unlock() + return err + } + + s.mutex.Unlock() + if value, ok := doc[key]; ok { + config.Set(key, value) + } + return nil } func (s *storage) Lock(ctx context.Context, retryDelay time.Duration) error { + s.mutex.Lock() + defer s.mutex.Unlock() return s.jsonStorage.Lock(ctx, retryDelay) } func (s *storage) Unlock() error { + s.mutex.Lock() + defer s.mutex.Unlock() return s.jsonStorage.Unlock() } type storageOption func(*storage) func (s *storage) Set(key string, value any) error { + s.mutex.Lock() + defer s.mutex.Unlock() + callback := s.callbacks[key] if callback != nil { @@ -60,9 +91,21 @@ func (s *storage) Set(key string, value any) error { } err := s.jsonStorage.Set(key, value) - if err != nil { + + var syntaxError *json.SyntaxError + if errors.As(err, &syntaxError) { + err = os.WriteFile(s.storageFile, []byte("{}"), 0666) + if err != nil { + return err + } + err = s.jsonStorage.Set(key, value) + if err != nil { + return err + } + } else if err != nil { return err } + return nil } @@ -101,5 +144,6 @@ func WithCallbacks(callbacks map[string]StorageCallbackFunc) func(*storage) { func WithStorageFile(file string) func(*storage) { return func(s *storage) { s.jsonStorage = configuration.NewJsonStorage(file) + s.storageFile = file } } diff --git a/internal/storage/storage_test.go b/internal/storage/storage_test.go index 713894d95..03f341c07 100644 --- a/internal/storage/storage_test.go +++ b/internal/storage/storage_test.go @@ -49,6 +49,30 @@ func Test_StorageCallsRegisterCallbacksForKeys(t *testing.T) { }, 5*time.Second, time.Millisecond, "callback was not called") } +func Test_StorageCallsRegisterCallbacks_InvalidJsonContent_ShouldClean(t *testing.T) { + called := make(chan bool, 1) + callbacks := map[string]StorageCallbackFunc{} + file := filepath.Join(t.TempDir(), t.Name()) + err := os.WriteFile(file, []byte("{\"INTERNAL_OAUTH_TOKEN_STORAGE\":\"{\\\"access_token\\\":\\\"mytoken\\\",\\\"token_type\\\":\\\"bearer\\\",\\\"refresh_token\\\":\\\"myrefreshtoken\\\",\\\"expiry\\\":\\\"2024-10-01T21:43:26.209852+02:00\\\"}\"}\"snyk_token\":\"\"}"), 0644) + assert.NoError(t, err) + myCallback := func(_ string, _ any) { called <- true } + + key := "test" + value := "test" + callbacks[key] = myCallback + s, err := NewStorageWithCallbacks(WithCallbacks(callbacks), WithStorageFile(file)) + require.NoError(t, err) + + err = s.Set(key, value) + require.NoError(t, err) + content, err := os.ReadFile(file) + assert.NoError(t, err) + assert.Equal(t, "{\"test\":\"test\"}", string(content)) + require.Eventuallyf(t, func() bool { + return <-called + }, 5*time.Second, time.Millisecond, "callback was not called") +} + func Test_ParallelFileLocking(t *testing.T) { t.Run("should respect locking order", func(t *testing.T) { file := filepath.Join(t.TempDir(), t.Name())