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
22 changes: 16 additions & 6 deletions backend/plugins/q_dev/api/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,22 @@ func GetConnection(input *plugin.ApiResourceInput) (*plugin.ApiResourceOutput, e

// validateConnection validates connection parameters including Identity Store fields
func validateConnection(connection *models.QDevConnection) error {
// Validate AWS credentials
if connection.AccessKeyId == "" {
return errors.Default.New("AccessKeyId is required")
}
if connection.SecretAccessKey == "" {
return errors.Default.New("SecretAccessKey is required")
// Default to access_key auth type if not specified
if connection.AuthType == "" {
connection.AuthType = "access_key"
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 (medium): A validation function ideally shouldn't mutate its input. Setting the default AuthType here means validateConnection has a side effect that callers may not expect. Consider moving the defaulting logic to a dedicated normalizeConnection() step (or into the caller) and keeping validateConnection pure.

}
if connection.AuthType != "access_key" && connection.AuthType != "iam_role" {
return errors.Default.New("AuthType must be 'access_key' or 'iam_role'")
}

// 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 Down
61 changes: 61 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: "access_key",
AccessKeyId: "AKIAIOSFODNN7EXAMPLE",
SecretAccessKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
Region: "us-east-1",
Expand All @@ -43,9 +44,68 @@ func TestValidateConnection_Success(t *testing.T) {
assert.NoError(t, err)
}

func TestValidateConnection_IAMRoleSuccess(t *testing.T) {
connection := &models.QDevConnection{
QDevConn: models.QDevConn{
AuthType: "iam_role",
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: "iam_role",
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",
},
}

err := validateConnection(connection)
assert.NoError(t, err)
assert.Equal(t, "access_key", 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: "access_key",
AccessKeyId: "", // Missing
SecretAccessKey: "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
Region: "us-east-1",
Expand All @@ -63,6 +123,7 @@ func TestValidateConnection_MissingAccessKeyId(t *testing.T) {
func TestValidateConnection_MissingSecretAccessKey(t *testing.T) {
connection := &models.QDevConnection{
QDevConn: models.QDevConn{
AuthType: "access_key",
AccessKeyId: "AKIAIOSFODNN7EXAMPLE",
SecretAccessKey: "", // Missing
Region: "us-east-1",
Expand Down
11 changes: 9 additions & 2 deletions backend/plugins/q_dev/models/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ import (

// 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 +44,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 == "iam_role"
}

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,54 @@
/*
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"
)

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 = 'access_key' WHERE auth_type IS NULL OR auth_type = ''"); 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 @@ -36,5 +36,6 @@ func All() []plugin.MigrationScript {
new(fixDedupUserTables),
new(resetS3FileMetaProcessed),
new(addLoggingTables),
new(addAuthTypeToConnection),
}
}
14 changes: 9 additions & 5 deletions backend/plugins/q_dev/tasks/identity_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,18 @@ func NewQDevIdentityClient(connection *models.QDevConnection) (*QDevIdentityClie
}

// Create AWS session with Identity Store region and credentials
sess, err := session.NewSession(&aws.Config{
cfg := &aws.Config{
Region: aws.String(connection.IdentityStoreRegion),
Credentials: credentials.NewStaticCredentials(
}
// Only set static credentials for access_key auth; IAM role uses the default credential chain
if !connection.IsIAMRoleAuth() {
cfg.Credentials = credentials.NewStaticCredentials(
connection.AccessKeyId,
connection.SecretAccessKey,
"", // No session token
),
})
"",
)
}
sess, err := session.NewSession(cfg)
if err != nil {
return nil, err
}
Expand Down
12 changes: 8 additions & 4 deletions backend/plugins/q_dev/tasks/s3_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,14 @@ import (

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, ""),
})
cfg := &aws.Config{
Region: aws.String(connection.Region),
}
// Only set static credentials for access_key auth; IAM role uses the default credential chain
if !connection.IsIAMRoleAuth() {
cfg.Credentials = credentials.NewStaticCredentials(connection.AccessKeyId, connection.SecretAccessKey, "")
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.

Nit (low): This block is nearly identical to identity_client.go lines 51-59. A small helper like newAWSSession(conn *models.QDevConnection, region string) (*session.Session, error) would DRY this up. Not blocking — just a suggestion for a follow-up cleanup.

}
sess, err := session.NewSession(cfg)
if err != nil {
return nil, errors.Convert(err)
}
Expand Down
Loading