Skip to content
Open
Show file tree
Hide file tree
Changes from 13 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
98 changes: 98 additions & 0 deletions backend/helpers/snowflakehelper/db.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
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,
}

Copy link
Copy Markdown

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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":
if privateKeyPEM == "" {
return nil, errors.BadInput.New("privateKey is required for keypair auth")
}
privKey, err := ParseRSAPrivateKey(privateKeyPEM)
if err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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
}
90 changes: 90 additions & 0 deletions backend/helpers/snowflakehelper/db_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
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"
"fmt"
"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 TestOpen_EmptyPrivateKeyForKeypair(t *testing.T) {
for _, authType := range []string{"", "keypair"} {
t.Run(fmt.Sprintf("authType=%q", authType), func(t *testing.T) {
_, err := Open("acct", "user", authType, "", "db", "schema", "", "")
assert.NotNil(t, err)
assert.Contains(t, err.Error(), "privateKey is required for keypair auth")
})
}
}

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")
}
103 changes: 103 additions & 0 deletions backend/plugins/github_snowflake/AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# 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_*`. Sync tasks populate
`RawDataOrigin` (`_raw_data_table` / `_raw_data_params`) on tool-layer rows so
convertors can delete domain records by `_raw_data_params` on full sync

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] operational-constraint

Documented constraint that a repo must not be configured in both a GitHub API connection and a github_snowflake connection is not enforced at runtime. Domain ID duplication could result.

Suggested fix: Consider adding a runtime check in PrepareTaskData that queries existing GithubRepo scopes for the same githubId on a different connection type.

(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)
75 changes: 75 additions & 0 deletions backend/plugins/github_snowflake/api/connection_api.go
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{}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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{}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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)
}
Loading
Loading