Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
35 changes: 26 additions & 9 deletions backend/plugins/q_dev/api/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func PostConnections(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput,
}

// 验证连接参数 (enhanced validation)
normalizeConnection(connection)
if err := validateConnection(connection); err != nil {
return nil, errors.BadInput.Wrap(err, "connection validation failed")
}
Expand All @@ -61,6 +62,7 @@ func PatchConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput,
}

// 验证更新后的连接参数 (enhanced validation)
normalizeConnection(connection)
if err := validateConnection(connection); err != nil {
return nil, errors.BadInput.Wrap(err, "connection validation failed")
}
Expand Down Expand Up @@ -106,14 +108,32 @@ func GetConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, e
return &plugin.ApiResourceOutput{Body: connection.Sanitize()}, err
}

// validateConnection validates connection parameters including Identity Store fields
// normalizeConnection applies defaults to connection fields.
// Call this before validateConnection to set sensible defaults.
func normalizeConnection(connection *models.QDevConnection) {
if connection.AuthType == "" {
connection.AuthType = models.AuthTypeAccessKey
}
if connection.RateLimitPerHour == 0 {
connection.RateLimitPerHour = 20000
}
}

// validateConnection validates connection parameters including Identity Store fields.
// This function is pure — it does not mutate the connection.
func validateConnection(connection *models.QDevConnection) error {
// Validate AWS credentials
if connection.AccessKeyId == "" {
return errors.Default.New("AccessKeyId is required")
if connection.AuthType != models.AuthTypeAccessKey && connection.AuthType != models.AuthTypeIAMRole {
return errors.Default.New("AuthType must be 'access_key' or 'iam_role'")
}
if connection.SecretAccessKey == "" {
return errors.Default.New("SecretAccessKey is required")

// Validate AWS credentials only for access_key auth type
if !connection.IsIAMRoleAuth() {
if connection.AccessKeyId == "" {
return errors.Default.New("AccessKeyId is required")
}
if connection.SecretAccessKey == "" {
return errors.Default.New("SecretAccessKey is required")
}
}
if connection.Region == "" {
return errors.Default.New("Region is required")
Expand All @@ -134,9 +154,6 @@ func validateConnection(connection *models.QDevConnection) error {
if connection.RateLimitPerHour < 0 {
return errors.Default.New("RateLimitPerHour must be positive")
}
if connection.RateLimitPerHour == 0 {
connection.RateLimitPerHour = 20000 // Set default value
}

return nil
}
69 changes: 69 additions & 0 deletions backend/plugins/q_dev/api/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
func TestValidateConnection_Success(t *testing.T) {
connection := &models.QDevConnection{
QDevConn: models.QDevConn{
AuthType: models.AuthTypeAccessKey,
AccessKeyId: "AKIAIOSFODNN7EXAMPLE",
SecretAccessKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
Region: "us-east-1",
Expand All @@ -43,9 +44,69 @@ func TestValidateConnection_Success(t *testing.T) {
assert.NoError(t, err)
}

func TestValidateConnection_IAMRoleSuccess(t *testing.T) {
connection := &models.QDevConnection{
QDevConn: models.QDevConn{
AuthType: models.AuthTypeIAMRole,
Region: "us-east-1",
Bucket: "my-q-dev-bucket",
},
}

err := validateConnection(connection)
assert.NoError(t, err)
}

func TestValidateConnection_IAMRoleNoCredentialsRequired(t *testing.T) {
connection := &models.QDevConnection{
QDevConn: models.QDevConn{
AuthType: models.AuthTypeIAMRole,
AccessKeyId: "", // Should not be required
SecretAccessKey: "", // Should not be required
Region: "us-east-1",
Bucket: "my-q-dev-bucket",
},
}

err := validateConnection(connection)
assert.NoError(t, err)
}

func TestValidateConnection_DefaultsToAccessKey(t *testing.T) {
connection := &models.QDevConnection{
QDevConn: models.QDevConn{
AuthType: "", // Should default to access_key
AccessKeyId: "AKIAIOSFODNN7EXAMPLE",
SecretAccessKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
Region: "us-east-1",
Bucket: "my-q-dev-bucket",
},
}

normalizeConnection(connection)
err := validateConnection(connection)
assert.NoError(t, err)
assert.Equal(t, models.AuthTypeAccessKey, connection.AuthType)
}

func TestValidateConnection_InvalidAuthType(t *testing.T) {
connection := &models.QDevConnection{
QDevConn: models.QDevConn{
AuthType: "invalid",
Region: "us-east-1",
Bucket: "my-q-dev-bucket",
},
}

err := validateConnection(connection)
assert.Error(t, err)
assert.Contains(t, err.Error(), "AuthType must be")
}

func TestValidateConnection_MissingAccessKeyId(t *testing.T) {
connection := &models.QDevConnection{
QDevConn: models.QDevConn{
AuthType: models.AuthTypeAccessKey,
AccessKeyId: "", // Missing
SecretAccessKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
Region: "us-east-1",
Expand All @@ -63,6 +124,7 @@ func TestValidateConnection_MissingAccessKeyId(t *testing.T) {
func TestValidateConnection_MissingSecretAccessKey(t *testing.T) {
connection := &models.QDevConnection{
QDevConn: models.QDevConn{
AuthType: models.AuthTypeAccessKey,
AccessKeyId: "AKIAIOSFODNN7EXAMPLE",
SecretAccessKey: "", // Missing
Region: "us-east-1",
Expand All @@ -89,6 +151,7 @@ func TestValidateConnection_MissingRegion(t *testing.T) {
},
}

normalizeConnection(connection)
err := validateConnection(connection)
assert.Error(t, err)
assert.Contains(t, err.Error(), "Region is required")
Expand All @@ -106,6 +169,7 @@ func TestValidateConnection_MissingBucket(t *testing.T) {
},
}

normalizeConnection(connection)
err := validateConnection(connection)
assert.Error(t, err)
assert.Contains(t, err.Error(), "Bucket is required")
Expand All @@ -123,6 +187,7 @@ func TestValidateConnection_EmptyIdentityStoreOk(t *testing.T) {
},
}

normalizeConnection(connection)
err := validateConnection(connection)
assert.NoError(t, err)
}
Expand All @@ -139,6 +204,7 @@ func TestValidateConnection_IdentityStoreRegionWithoutId(t *testing.T) {
},
}

normalizeConnection(connection)
err := validateConnection(connection)
assert.Error(t, err)
assert.Contains(t, err.Error(), "IdentityStoreRegion")
Expand All @@ -156,6 +222,7 @@ func TestValidateConnection_IdentityStoreIdWithoutRegion(t *testing.T) {
},
}

normalizeConnection(connection)
err := validateConnection(connection)
assert.Error(t, err)
assert.Contains(t, err.Error(), "IdentityStoreId provided but IdentityStoreRegion is empty")
Expand All @@ -174,6 +241,7 @@ func TestValidateConnection_InvalidRateLimit(t *testing.T) {
},
}

normalizeConnection(connection)
err := validateConnection(connection)
assert.Error(t, err)
assert.Contains(t, err.Error(), "RateLimitPerHour must be positive")
Expand All @@ -192,6 +260,7 @@ func TestValidateConnection_DefaultRateLimit(t *testing.T) {
},
}

normalizeConnection(connection)
err := validateConnection(connection)
assert.NoError(t, err)
assert.Equal(t, 20000, connection.RateLimitPerHour) // Should be set to default
Expand Down
17 changes: 15 additions & 2 deletions backend/plugins/q_dev/models/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,19 @@ import (
helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api"
)

// Auth type constants for AWS authentication
const (
AuthTypeAccessKey = "access_key"
AuthTypeIAMRole = "iam_role"
)

// QDevConn holds the essential information to connect to AWS S3
type QDevConn struct {
// AccessKeyId for AWS
// AuthType determines how to authenticate with AWS: "access_key" or "iam_role"
AuthType string `mapstructure:"authType" json:"authType"`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion (low): "access_key" and "iam_role" are repeated across validation, tests, migration, and both client files. Consider defining constants:

const (
    AuthTypeAccessKey = "access_key"
    AuthTypeIAMRole   = "iam_role"
)

This prevents typo-driven bugs and makes future additions easier to grep for.

// AccessKeyId for AWS (required when AuthType is "access_key")
AccessKeyId string `mapstructure:"accessKeyId" json:"accessKeyId"`
// SecretAccessKey for AWS
// SecretAccessKey for AWS (required when AuthType is "access_key")
SecretAccessKey string `mapstructure:"secretAccessKey" json:"secretAccessKey"`
// Region for AWS S3
Region string `mapstructure:"region" json:"region"`
Expand All @@ -42,6 +50,11 @@ type QDevConn struct {
IdentityStoreRegion string `mapstructure:"identityStoreRegion" json:"identityStoreRegion"`
}

// IsIAMRoleAuth returns true if the connection uses IAM role authentication
func (conn *QDevConn) IsIAMRoleAuth() bool {
return conn.AuthType == AuthTypeIAMRole
}

func (conn *QDevConn) Sanitize() QDevConn {
conn.SecretAccessKey = utils.SanitizeString(conn.SecretAccessKey)
return *conn
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
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 migrationscripts

import (
"github.com/apache/incubator-devlake/core/context"
"github.com/apache/incubator-devlake/core/dal"
"github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/plugins/q_dev/models"
)

var _ plugin.MigrationScript = (*addAuthTypeToConnection)(nil)

type addAuthTypeToConnection struct{}

func (*addAuthTypeToConnection) Up(basicRes context.BasicRes) errors.Error {
db := basicRes.GetDal()

if !db.HasColumn("_tool_q_dev_connections", "auth_type") {
if err := db.AddColumn("_tool_q_dev_connections", "auth_type", dal.Varchar); err != nil {
return errors.Default.Wrap(err, "failed to add auth_type to _tool_q_dev_connections")
}
}

// Default existing rows to "access_key" since they were created before IAM role support
if err := db.Exec("UPDATE _tool_q_dev_connections SET auth_type = ? WHERE auth_type IS NULL OR auth_type = ''", models.AuthTypeAccessKey); err != nil {
return errors.Default.Wrap(err, "failed to set default auth_type for existing connections")
}

return nil
}

func (*addAuthTypeToConnection) Version() uint64 {
return 20260320000001
}

func (*addAuthTypeToConnection) Name() string {
return "add auth_type column to _tool_q_dev_connections for IAM role support"
}
1 change: 1 addition & 0 deletions backend/plugins/q_dev/models/migrationscripts/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,6 @@ func All() []plugin.MigrationScript {
new(resetS3FileMetaProcessed),
new(addLoggingTables),
new(addLoggingFields),
new(addAuthTypeToConnection),
}
}
42 changes: 42 additions & 0 deletions backend/plugins/q_dev/tasks/aws_session.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
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 tasks

import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/aws/session"

"github.com/apache/incubator-devlake/plugins/q_dev/models"
)

// newAWSSession creates an AWS session for the given connection and region.
// For access_key auth, static credentials are used; for iam_role, the default credential chain is used.
func newAWSSession(conn *models.QDevConnection, region string) (*session.Session, error) {
cfg := &aws.Config{
Region: aws.String(region),
}
if !conn.IsIAMRoleAuth() {
cfg.Credentials = credentials.NewStaticCredentials(
conn.AccessKeyId,
conn.SecretAccessKey,
"",
)
}
return session.NewSession(cfg)
}
11 changes: 1 addition & 10 deletions backend/plugins/q_dev/tasks/identity_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ package tasks

import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/identitystore"

"github.com/apache/incubator-devlake/plugins/q_dev/models"
Expand Down Expand Up @@ -48,14 +46,7 @@ func NewQDevIdentityClient(connection *models.QDevConnection) (*QDevIdentityClie
}

// Create AWS session with Identity Store region and credentials
sess, err := session.NewSession(&aws.Config{
Region: aws.String(connection.IdentityStoreRegion),
Credentials: credentials.NewStaticCredentials(
connection.AccessKeyId,
connection.SecretAccessKey,
"", // No session token
),
})
sess, err := newAWSSession(connection, connection.IdentityStoreRegion)
if err != nil {
return nil, err
}
Expand Down
10 changes: 2 additions & 8 deletions backend/plugins/q_dev/tasks/s3_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,12 @@ import (
"github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/core/plugin"
"github.com/apache/incubator-devlake/plugins/q_dev/models"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/credentials"
"github.com/aws/aws-sdk-go/aws/session"
"github.com/aws/aws-sdk-go/service/s3"
)

func NewQDevS3Client(taskCtx plugin.TaskContext, connection *models.QDevConnection) (*QDevS3Client, errors.Error) {
// 创建AWS session
sess, err := session.NewSession(&aws.Config{
Region: aws.String(connection.Region),
Credentials: credentials.NewStaticCredentials(connection.AccessKeyId, connection.SecretAccessKey, ""),
})
// Create AWS session
sess, err := newAWSSession(connection, connection.Region)
if err != nil {
return nil, errors.Convert(err)
}
Expand Down