Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

codeintel: first implementation of auto-indexing secrets #45580

Merged
merged 15 commits into from
Dec 15, 2022

Conversation

Strum355
Copy link
Contributor

@Strum355 Strum355 commented Dec 12, 2022

Adds executor secrets support to auto-indexing, allowing for new capabilities including, and most notably, authentication with private registries. This is an early-phase (but functional) PR with first-step support for private Go modules.

Test plan

Ran a local E2E test-run (not an actual E2E automated test), back-compat tests keeping the DB stuff relatively sane, but also stressed importance of reviewing that as well as running it through a few scenarios

main dry-run https://buildkite.com/sourcegraph/sourcegraph/builds/189673

@cla-bot cla-bot bot added the cla-signed label Dec 12, 2022
@sg-e2e-regression-test-bob
Copy link

sg-e2e-regression-test-bob commented Dec 12, 2022

Bundle size report 📦

Initial size Total size Async size Modules
0.00% (+0.05 kb) 0.00% (+0.16 kb) 0.00% (+0.12 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits 52df392 and 3cb4a50 or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

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

nice, good stuff!

ALTER TABLE executor_secret_access_logs
DROP COLUMN IF EXISTS machine_user;

DELETE FROM executor_secret_access_logs WHERE user_id IS NULL;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@evict Im not sure how desirable this would be. Any opinions?

Copy link
Contributor

@evict evict Dec 14, 2022

Choose a reason for hiding this comment

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

Hm, I think we should keep existing access logs even though user_id is not set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bobheadxi is there an approach we should take in the down migration here? Im not sure how we can preserve it without losing the machine_user info

Copy link
Member

Choose a reason for hiding this comment

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

let's move it to a backup table for worst-case scenarios.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep thats the approach im going to go with, will have an impl for review today

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Strum355 Strum355 marked this pull request as ready for review December 14, 2022 16:29
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Dec 14, 2022

Codenotify: Notifying subscribers in CODENOTIFY files for diff 3774fbd...52df392.

Notify File(s)
@efritz client/web/src/enterprise/executors/secrets/ExecutorSecretsListPage.tsx
client/web/src/enterprise/executors/secrets/SecretAccessLogsModal.tsx
enterprise/cmd/frontend/internal/executorqueue/queues/codeintel/queue.go
enterprise/cmd/frontend/internal/executorqueue/queues/codeintel/transform.go
enterprise/cmd/frontend/internal/executorqueue/queues/codeintel/transform_test.go
enterprise/internal/codeintel/autoindexing/internal/background/utils.go
enterprise/internal/codeintel/autoindexing/internal/inference/lang_go_test.go
enterprise/internal/codeintel/autoindexing/internal/inference/lang_typescript_test.go
enterprise/internal/codeintel/autoindexing/internal/inference/lua/go.lua
enterprise/internal/codeintel/autoindexing/internal/inference/lua/typescript.lua
enterprise/internal/codeintel/autoindexing/internal/inference/luatypes/index_job.go
enterprise/internal/codeintel/autoindexing/internal/jobselector/job_selector.go
enterprise/internal/codeintel/autoindexing/internal/store/scan.go
enterprise/internal/codeintel/autoindexing/internal/store/store_indexes.go
enterprise/internal/codeintel/shared/types/index.go
lib/codeintel/autoindex/config/types.go
@eseliger client/web/src/enterprise/executors/secrets/ExecutorSecretsListPage.tsx
client/web/src/enterprise/executors/secrets/SecretAccessLogsModal.tsx
enterprise/cmd/frontend/internal/executorqueue/queues/codeintel/queue.go
enterprise/cmd/frontend/internal/executorqueue/queues/codeintel/transform.go
enterprise/cmd/frontend/internal/executorqueue/queues/codeintel/transform_test.go
@sourcegraph/code-insights-backend enterprise/internal/insights/store/store.go

Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

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

nice SQL trickery!

@Strum355 Strum355 enabled auto-merge (squash) December 15, 2022 22:07
@Strum355 Strum355 merged commit 8c996a7 into main Dec 15, 2022
@Strum355 Strum355 deleted the nsc/autoindexing-secrets branch December 15, 2022 22:32
@varungandhi-src varungandhi-src added team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) and removed team/language-platform labels Feb 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-index cla-signed executors team/graph Graph Team (previously Code Intel/Language Tools/Language Platform)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants