-
Notifications
You must be signed in to change notification settings - Fork 21
feat: add github_snowflake plugin for Snowflake-backed GitHub ingestion #139
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
base: main
Are you sure you want to change the base?
Changes from 7 commits
414e473
dc66560
7b025bf
171f31b
64fdf81
9967fe2
cf37ef8
0a255dd
51fd7ec
cf67860
0f1041e
7574268
fda3108
c05c649
990eb5c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| /* | ||
| Licensed to the Apache Software Foundation (ASF) under one or more | ||
| contributor license agreements. See the NOTICE file distributed with | ||
| this work for additional information regarding copyright ownership. | ||
| The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| (the "License"); you may not use this file except in compliance with | ||
| the License. You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| // Package snowflakehelper provides shared Snowflake connectivity helpers for | ||
| // Konflux-owned snowflake-backed DevLake plugins (github_snowflake, jira_snowflake). | ||
| package snowflakehelper | ||
|
|
||
| import ( | ||
| "crypto/rsa" | ||
| "crypto/x509" | ||
| "database/sql" | ||
| "encoding/pem" | ||
| "fmt" | ||
|
|
||
| "github.com/apache/incubator-devlake/core/errors" | ||
| sf "github.com/snowflakedb/gosnowflake" | ||
| ) | ||
|
|
||
| // Open opens a database/sql connection to Snowflake. | ||
| // | ||
| // authType controls authentication: | ||
| // - "keypair" (default): JWT key-pair auth using privateKeyPEM. Works in containers and CI. | ||
| // - "externalbrowser": SSO via browser pop-up. Only works when DevLake runs on a desktop host | ||
| // (i.e. via `make run`, not inside a Docker container). | ||
| func Open(account, user, authType, privateKeyPEM, database, schema, warehouse, role string) (*sql.DB, errors.Error) { | ||
| cfg := &sf.Config{ | ||
| Account: account, | ||
| User: user, | ||
| Database: database, | ||
| Schema: schema, | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] missing-validation Open does not validate that account, user, database, or schema are non-empty. The connection model has validate:required tags at the API layer, but direct calls (CLI, tests) bypass that validation. Suggested fix: Add explicit empty-string checks for required fields at the top of Open. |
||
| if warehouse != "" { | ||
| cfg.Warehouse = warehouse | ||
| } | ||
| if role != "" { | ||
| cfg.Role = role | ||
| } | ||
|
|
||
| switch authType { | ||
| case "", "keypair": | ||
| privKey, err := ParseRSAPrivateKey(privateKeyPEM) | ||
| if err != nil { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] missing-validation When authType is keypair with an empty privateKeyPEM, the error is failed to decode PEM block rather than a clear private key required for keypair auth message. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] behavioral change in refactored code The old jira_snowflake OpenSnowflakeDB treated ALL authType values that were not 'externalbrowser' as keypair auth (silent fallthrough). The new shared Open function rejects unknown authType values with an error. If any existing jira_snowflake connection row in production has a non-canonical auth_type value, that connection will stop working after this upgrade. Suggested fix: Verify no production _tool_jira_snowflake_connections rows have auth_type values other than '', 'keypair', or 'externalbrowser'. Alternatively, treat unknown authType as 'keypair' with a warning log. |
||
| return nil, errors.Default.Wrap(err, "failed to parse Snowflake private key") | ||
| } | ||
| cfg.Authenticator = sf.AuthTypeJwt | ||
| cfg.PrivateKey = privKey | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] error-handling-gap The Open function treats an empty authType the same as 'keypair', calling ParseRSAPrivateKey(privateKeyPEM) even when privateKeyPEM is empty. This produces a generic 'failed to decode PEM block' error instead of a clear validation message. The SnowflakeGithubConnection model also lacks a validate:'required' tag on PrivateKey. Suggested fix: Add an early check in the keypair case: if privateKeyPEM == '' { return nil, errors.BadInput.New('privateKey is required for keypair auth') } |
||
| case "externalbrowser": | ||
| cfg.Authenticator = sf.AuthTypeExternalBrowser | ||
| default: | ||
| return nil, errors.BadInput.New(fmt.Sprintf( | ||
| `unsupported authType %q; must be "keypair" or "externalbrowser"`, authType, | ||
| )) | ||
| } | ||
|
|
||
| dsn, goErr := sf.DSN(cfg) | ||
| if goErr != nil { | ||
| return nil, errors.Default.Wrap(goErr, "failed to build Snowflake DSN") | ||
| } | ||
| db, goErr := sql.Open("snowflake", dsn) | ||
| if goErr != nil { | ||
| return nil, errors.Default.Wrap(goErr, "failed to open Snowflake connection") | ||
| } | ||
| return db, nil | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] error-handling sql.Open only validates the driver name and DSN syntax; it does not establish a TCP connection. Misconfigured accounts or unreachable Snowflake will only surface when the first sync subtask runs. For externalbrowser auth, the SSO browser pop-up appears mid-subtask instead of at connection time. Suggested fix: After sql.Open, add db.PingContext(context.Background()) and close/return error on failure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] error handling gap sql.Open does not actually establish a connection. A misconfigured DSN will not be caught until the first query. Suggested fix: Consider adding db.PingContext(ctx) after sql.Open to verify connectivity. |
||
| // ParseRSAPrivateKey parses a PKCS#8 PEM-encoded RSA private key. | ||
| func ParseRSAPrivateKey(pemStr string) (*rsa.PrivateKey, error) { | ||
| block, _ := pem.Decode([]byte(pemStr)) | ||
| if block == nil { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] error handling gap sql.Open returns a lazy connection pool without dialing the server. Credential or connectivity errors surface only during the first query. This is pre-existing behavior from jira_snowflake preserved in the shared helper. Suggested fix: Consider adding db.PingContext() after sql.Open for eager validation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] error-handling-gap Open returns *sql.DB without calling db.Ping() to verify the connection is reachable. sql.Open only validates DSN syntax; bad credentials or unreachable Snowflake host surface as confusing errors on the first sync query. Matches existing jira_snowflake pattern and is standard Go database/sql behavior. Suggested fix: Consider adding db.PingContext(ctx) after sql.Open to fail fast on configuration errors. This requires accepting a context.Context parameter. |
||
| return nil, fmt.Errorf("failed to decode PEM block from private key") | ||
| } | ||
| key, err := x509.ParsePKCS8PrivateKey(block.Bytes) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse PKCS8 private key: %w", err) | ||
| } | ||
| rsaKey, ok := key.(*rsa.PrivateKey) | ||
| if !ok { | ||
| return nil, fmt.Errorf("private key is not an RSA key") | ||
| } | ||
| return rsaKey, nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| /* | ||
| Licensed to the Apache Software Foundation (ASF) under one or more | ||
| contributor license agreements. See the NOTICE file distributed with | ||
| this work for additional information regarding copyright ownership. | ||
| The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| (the "License"); you may not use this file except in compliance with | ||
| the License. You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package snowflakehelper | ||
|
|
||
| import ( | ||
| "crypto/rand" | ||
| "crypto/rsa" | ||
| "crypto/x509" | ||
| "encoding/pem" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func generateTestPKCS8PEM(t *testing.T) string { | ||
| t.Helper() | ||
| key, err := rsa.GenerateKey(rand.Reader, 2048) | ||
| require.NoError(t, err) | ||
| der, err := x509.MarshalPKCS8PrivateKey(key) | ||
| require.NoError(t, err) | ||
| block := &pem.Block{Type: "PRIVATE KEY", Bytes: der} | ||
| return string(pem.EncodeToMemory(block)) | ||
| } | ||
|
|
||
| func TestOpen_InvalidAuthType(t *testing.T) { | ||
| _, err := Open("acct", "user", "keypai", "", "db", "schema", "", "") | ||
| assert.NotNil(t, err) | ||
| assert.Contains(t, err.Error(), "unsupported authType") | ||
| } | ||
|
|
||
| func TestParseRSAPrivateKey_ValidPKCS8(t *testing.T) { | ||
| pemStr := generateTestPKCS8PEM(t) | ||
| key, err := ParseRSAPrivateKey(pemStr) | ||
| require.NoError(t, err) | ||
| assert.NotNil(t, key) | ||
| assert.Equal(t, 2048, key.N.BitLen()) | ||
| } | ||
|
|
||
| func TestParseRSAPrivateKey_EmptyString(t *testing.T) { | ||
| _, err := ParseRSAPrivateKey("") | ||
| assert.Error(t, err) | ||
| assert.Contains(t, err.Error(), "PEM") | ||
| } | ||
|
|
||
| func TestParseRSAPrivateKey_InvalidPEM(t *testing.T) { | ||
| _, err := ParseRSAPrivateKey("not a pem block") | ||
| assert.Error(t, err) | ||
| assert.Contains(t, err.Error(), "PEM") | ||
| } | ||
|
|
||
| func TestParseRSAPrivateKey_WrongKeyType(t *testing.T) { | ||
| // PKCS#1 format (BEGIN RSA PRIVATE KEY) is not PKCS#8, should fail ParsePKCS8PrivateKey | ||
| key, genErr := rsa.GenerateKey(rand.Reader, 2048) | ||
| require.NoError(t, genErr) | ||
| block := &pem.Block{ | ||
| Type: "RSA PRIVATE KEY", | ||
| Bytes: x509.MarshalPKCS1PrivateKey(key), | ||
| } | ||
| pemStr := string(pem.EncodeToMemory(block)) | ||
|
|
||
| _, err := ParseRSAPrivateKey(pemStr) | ||
| assert.Error(t, err, "PKCS#1 key should be rejected; expected PKCS#8") | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| # github_snowflake Plugin — Agent Context | ||
|
|
||
| Drop-in replacement for the GitHub API plugin per repository. Reads GitHub data | ||
| from a Snowflake replica (Fivetran, `GITHUB_DB.MARTS`) and writes into the | ||
| existing `_tool_github_*` tool-layer tables, then runs domain-layer convertors to | ||
| produce `code.*` domain records. | ||
|
|
||
| ## Build & Test | ||
|
|
||
| ```bash | ||
| cd backend | ||
| go build ./plugins/github_snowflake/... | ||
| go test ./plugins/github_snowflake/... -v | ||
| golangci-lint run ./plugins/github_snowflake/... | ||
| ``` | ||
|
|
||
| Unit tests do **not** need MySQL. A real pipeline run does. | ||
|
|
||
| ### Minimal real-run setup | ||
|
|
||
| 1. Start MySQL: `podman compose -f docker-compose-dev.yml up -d mysql` | ||
| 2. Ensure `.env` has `DB_URL`, `ENCRYPTION_SECRET`, and (for local API) `AUTH_ENABLED=false`. | ||
| For native `make run`, use `localhost` (or `127.0.0.1`) in `DB_URL` — `env.example`'s | ||
| `@mysql:3306` host only resolves inside the compose network. | ||
| 3. Run the server (native desktop required for `externalbrowser` SSO): | ||
|
|
||
| ```bash | ||
| cd backend | ||
| # Include github so its migrations create shared _tool_github_* tables (needed on a fresh DB). | ||
| DEVLAKE_PLUGINS=github,github_snowflake DISABLED_REMOTE_PLUGINS=true ENV_FILE=../.env make build-plugin run | ||
| ``` | ||
|
|
||
| 4. Create a connection (`authType: externalbrowser` for local desktop) + run a pipeline for one repo (`githubId` + `fullName`) | ||
|
|
||
| Full step-by-step (Snowflake check, migrations, curl examples, result queries): | ||
| [docs/github-snowflake-local-testing.md](../../../docs/github-snowflake-local-testing.md) | ||
|
|
||
| ## Layout | ||
|
|
||
| ``` | ||
| impl/impl.go — plugin interfaces, SubTaskMetas, PrepareTaskData | ||
| api/connection_api.go — connection CRUD (POST/GET/PATCH/DELETE) | ||
| models/connection.go — SnowflakeGithubConnection (table: _tool_github_snowflake_connections) | ||
| models/migrationscripts/ — DB migrations | ||
| tasks/task_data.go — GithubSnowflakeOptions, GithubSnowflakeTaskData | ||
| tasks/shared.go — URL helpers | ||
| tasks/sync_*.go — Snowflake SQL queries → _tool_github_* tool-layer tables | ||
| tasks/convert_*.go — domain-layer convertors (adapted copies of github/tasks/*) | ||
| ``` | ||
|
|
||
| Shared Snowflake connectivity lives in `helpers/snowflakehelper` (`Open`, `ParseRSAPrivateKey`). | ||
|
|
||
| ## Subtask pipeline order | ||
|
|
||
| 1. `syncRepos` — REPOSITORY → `_tool_github_repos` | ||
| 2. `syncPullRequests` — PULL_REQUEST ⨝ ISSUE ⨝ ISSUE_MERGED ⨝ USER → `_tool_github_pull_requests` | ||
| 3. `syncPrCommits` — COMMIT_PULL_REQUEST ⨝ COMMIT → `_tool_github_pull_request_commits` | ||
| 4. `syncPrReviews` — PULL_REQUEST_REVIEW ⨝ USER → `_tool_github_pull_request_reviews` | ||
| 5. `syncReviewers` — REQUESTED_REVIEWER_HISTORY → `_tool_github_reviewers` | ||
| 6. `syncAccounts` — USER (+ USER_EMAIL) → `_tool_github_accounts` + `_tool_github_repo_accounts` | ||
| 7. `convertRepo` / `convertPullRequests` / `convertPrCommits` / `convertPrReviews` / `convertReviews` / `convertAccounts` | ||
|
|
||
| ## Key conventions | ||
|
|
||
| - **Scope unit is `GithubRepo`** (numeric `githubId` + `fullName` owner/repo). | ||
| - **No raw-table layer**: writes directly to `_tool_github_*`. Convertors use | ||
| `_raw_data_params`-scoped deletion for full sync (same pattern as jira_snowflake). | ||
| - **AuthType**: `"keypair"` (default, JWT) or `"externalbrowser"` (SSO, desktop only). | ||
| - **Connection defaults**: Database=`GITHUB_DB`, Schema=`MARTS`, Warehouse=`DEFAULT`. | ||
| - **PR line/comment counts** are unavailable in Snowflake — leave 0. | ||
| - **Actions jobs** table does not exist — never enable job sync/convertors. | ||
| - Do not log commit author emails in debug output. | ||
|
|
||
| ## Snowflake schema notes (GITHUB_DB.MARTS — verified 2026-07-21) | ||
|
|
||
| | Table | Notes | | ||
| |---|---| | ||
| | `REPOSITORY` | No HTML/Clone URL — derive from FULL_NAME | | ||
| | `PULL_REQUEST` + `ISSUE` + `ISSUE_MERGED` | Fivetran splits PR fields across three tables | | ||
| | `COMMIT_PULL_REQUEST` | Orphan PR links exist — always INNER JOIN PULL_REQUEST | | ||
| | `PULL_REQUEST_REVIEW` | States: APPROVED, COMMENTED, DISMISSED, CHANGES_REQUESTED | | ||
| | `REQUESTED_REVIEWER_HISTORY` | Filter `REQUESTED_REVIEWER_TYPE = 'user'`; take latest non-removed | | ||
| | `"USER"` / `USER_EMAIL` | Identity present (not PII-stripped). Quote `"USER"` — reserved keyword in Snowflake. | | ||
|
|
||
| Pilot coverage today: **konflux-ci** org only in MARTS. | ||
|
|
||
| ## GitHub plugin models dependency | ||
|
|
||
| Imports `plugins/github/models` for tool-layer structs. This is a shared schema | ||
| dependency, not a business-logic cross-import. Runtime API/business logic does | ||
| not require the github plugin, but a fresh DB still needs github loaded once so | ||
| its migrations create `_tool_github_*`. `impl.Init` registers a minimal | ||
| `githubPluginStub` for didgen. | ||
|
|
||
| ## Don'ts | ||
|
|
||
| - Don't add models without a migration in `migrationscripts/register.go` | ||
| - Don't skip the Apache 2.0 license header on new `.go` files | ||
| - Don't configure the same repo in both a GitHub API connection and a | ||
| github_snowflake connection simultaneously — this causes domain ID duplication | ||
| - Don't enable Actions job convertors (table missing in Snowflake) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| /* | ||
| Licensed to the Apache Software Foundation (ASF) under one or more | ||
| contributor license agreements. See the NOTICE file distributed with | ||
| this work for additional information regarding copyright ownership. | ||
| The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| (the "License"); you may not use this file except in compliance with | ||
| the License. You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package api | ||
|
|
||
| import ( | ||
| "github.com/apache/incubator-devlake/core/context" | ||
| "github.com/apache/incubator-devlake/core/errors" | ||
| "github.com/apache/incubator-devlake/core/plugin" | ||
| helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api" | ||
| "github.com/apache/incubator-devlake/plugins/github_snowflake/models" | ||
| ) | ||
|
|
||
| var connHelper *helper.ConnectionApiHelper | ||
|
|
||
| // Init initialises the API layer. Called from impl.Init. | ||
| func Init(br context.BasicRes, p plugin.PluginMeta) { | ||
| connHelper = helper.NewConnectionHelper(br, nil, p.Name()) | ||
| } | ||
|
|
||
| // PostConnections creates a new SnowflakeGithubConnection. | ||
| func PostConnections(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) { | ||
| connection := &models.SnowflakeGithubConnection{} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] api-contract PostConnections returns HTTP 200 instead of 201 on creation. Consistent with jira_snowflake but diverges from REST conventions. |
||
| if err := connHelper.Create(connection, input); err != nil { | ||
| return nil, err | ||
| } | ||
| return &plugin.ApiResourceOutput{Body: connection, Status: 200}, nil | ||
| } | ||
|
|
||
| // GetConnections returns all SnowflakeGithubConnections. | ||
| func GetConnections(_ *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) { | ||
| var connections []models.SnowflakeGithubConnection | ||
| if err := connHelper.List(&connections); err != nil { | ||
| return nil, err | ||
| } | ||
| return &plugin.ApiResourceOutput{Body: connections, Status: 200}, nil | ||
| } | ||
|
|
||
| // GetConnection returns a single connection by connectionId path param. | ||
| func GetConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) { | ||
| connection := &models.SnowflakeGithubConnection{} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] data-exposure GetConnections/GetConnection endpoints return the full connection struct including the PrivateKey field via json:privateKey. This follows the framework pattern but the RSA private key is more sensitive than a rotatable API token. Suggested fix: Consider adding json:- to the PrivateKey field to suppress it from API responses. |
||
| if err := connHelper.First(connection, input.Params); err != nil { | ||
| return nil, err | ||
| } | ||
| return &plugin.ApiResourceOutput{Body: connection, Status: 200}, nil | ||
| } | ||
|
|
||
| // PatchConnection updates fields on an existing connection. | ||
| func PatchConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) { | ||
| connection := &models.SnowflakeGithubConnection{} | ||
| if err := connHelper.Patch(connection, input); err != nil { | ||
| return nil, err | ||
| } | ||
| return &plugin.ApiResourceOutput{Body: connection, Status: 200}, nil | ||
| } | ||
|
|
||
| // DeleteConnection removes a connection. | ||
| func DeleteConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, errors.Error) { | ||
| connection := &models.SnowflakeGithubConnection{} | ||
| return connHelper.Delete(connection, input) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[low] secrets handling
The decrypted private key PEM is passed as a Go string (immutable, cannot be zeroed in memory). This is standard Go behavior consistent with other DevLake plugins.