-
Notifications
You must be signed in to change notification settings - Fork 876
feat: add awsiamrds db auth driver #12566
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
Changes from all commits
d23ad15
f2426d1
03eb9c1
1ff0291
269097c
eba4c6b
83d7101
f25ae4e
cb887c0
c0bd787
891a7c0
7ab2e95
b44becf
66c686d
a425f5b
de7e7d9
c256df3
c0a0515
6c3d73d
c56fddc
293140b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
package awsiamrds | ||
|
||
import ( | ||
"context" | ||
"database/sql" | ||
"database/sql/driver" | ||
"fmt" | ||
"net/url" | ||
|
||
"github.com/aws/aws-sdk-go-v2/aws" | ||
"github.com/aws/aws-sdk-go-v2/config" | ||
"github.com/aws/aws-sdk-go-v2/feature/rds/auth" | ||
"golang.org/x/xerrors" | ||
) | ||
|
||
type awsIamRdsDriver struct { | ||
parent driver.Driver | ||
cfg aws.Config | ||
} | ||
|
||
var _ driver.Driver = &awsIamRdsDriver{} | ||
|
||
// Register initializes and registers our aws iam rds wrapped database driver. | ||
func Register(ctx context.Context, parentName string) (string, error) { | ||
cfg, err := config.LoadDefaultConfig(ctx) | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
db, err := sql.Open(parentName, "") | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
// create a new aws iam rds driver | ||
d := newDriver(db.Driver(), cfg) | ||
name := fmt.Sprintf("%s-awsiamrds", parentName) | ||
sql.Register(fmt.Sprintf("%s-awsiamrds", parentName), d) | ||
|
||
return name, nil | ||
} | ||
|
||
// newDriver will create a new *AwsIamRdsDriver using the environment aws session. | ||
func newDriver(parentDriver driver.Driver, cfg aws.Config) *awsIamRdsDriver { | ||
return &awsIamRdsDriver{ | ||
parent: parentDriver, | ||
cfg: cfg, | ||
} | ||
} | ||
|
||
// Open creates a new connection to the database using the provided name. | ||
func (d *awsIamRdsDriver) Open(name string) (driver.Conn, error) { | ||
// set password with signed aws authentication token for the rds instance | ||
nURL, err := getAuthenticatedURL(d.cfg, name) | ||
if err != nil { | ||
return nil, xerrors.Errorf("assigning authentication token to url: %w", err) | ||
} | ||
|
||
// make connection | ||
conn, err := d.parent.Open(nURL) | ||
if err != nil { | ||
return nil, xerrors.Errorf("opening connection with %s: %w", nURL, err) | ||
} | ||
|
||
return conn, nil | ||
} | ||
|
||
func getAuthenticatedURL(cfg aws.Config, dbURL string) (string, error) { | ||
nURL, err := url.Parse(dbURL) | ||
if err != nil { | ||
return "", xerrors.Errorf("parsing dbURL: %w", err) | ||
} | ||
|
||
// generate a new rds session auth tokenized URL | ||
rdsEndpoint := fmt.Sprintf("%s:%s", nURL.Hostname(), nURL.Port()) | ||
token, err := auth.BuildAuthToken(context.Background(), rdsEndpoint, cfg.Region, nURL.User.Username(), cfg.Credentials) | ||
if err != nil { | ||
return "", xerrors.Errorf("building rds auth token: %w", err) | ||
} | ||
// set token as user password | ||
nURL.User = url.UserPassword(nURL.User.Username(), token) | ||
|
||
return nURL.String(), nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package awsiamrds_test | ||
|
||
import ( | ||
"context" | ||
"os" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
"cdr.dev/slog/sloggers/slogtest" | ||
|
||
"github.com/coder/coder/v2/cli" | ||
awsrdsiam "github.com/coder/coder/v2/coderd/database/awsiamrds" | ||
"github.com/coder/coder/v2/testutil" | ||
) | ||
|
||
func TestDriver(t *testing.T) { | ||
t.Parallel() | ||
// Be sure to set AWS_DEFAULT_REGION to the database region as well. | ||
// Example: | ||
// export AWS_DEFAULT_REGION=us-east-2; | ||
// export DBAWSIAMRDS_TEST_URL="postgres://user@host:5432/dbname"; | ||
url := os.Getenv("DBAWSIAMRDS_TEST_URL") | ||
if url == "" { | ||
t.Skip() | ||
} | ||
|
||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) | ||
defer cancel() | ||
|
||
sqlDriver, err := awsrdsiam.Register(ctx, "postgres") | ||
require.NoError(t, err) | ||
|
||
db, err := cli.ConnectToPostgres(ctx, slogtest.Make(t, nil), sqlDriver, url) | ||
require.NoError(t, err) | ||
defer func() { | ||
_ = db.Close() | ||
}() | ||
|
||
i, err := db.QueryContext(ctx, "select 1;") | ||
require.NoError(t, err) | ||
defer func() { | ||
_ = i.Close() | ||
}() | ||
|
||
require.True(t, i.Next()) | ||
var one int | ||
require.NoError(t, i.Scan(&one)) | ||
require.Equal(t, 1, one) | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
Is there a specific string format the connection has when connecting via AWS IAM? I think it'd be preferred to check for that over adding a new server option.
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.
Just a missing password field, maybe an AWS domain in the URL, but I don't think there's anything that's enough to make an inference.
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.
One thing that might be possible is if we detected the URL scheme and had people set
awsiamrds://...
instead ofpostgres://
, or add a query param? It is unforunate we need a new flag just for aws auth, but it's probable we would exapand this in the future. My only worry is that just detecting it in the connection string would be non-obvious, since I haven't really seen it anywhere else, but to be fair I haven't really done much research.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.
Yeah I wouldn't wanna change the protocol because that kinda implies it's not speaking
postgres
protocol. I think given we may have other auth schemes in the future we need to support, a flag seems fine to me and better than any parsing options I can think of.