Skip to content

fix: use authenticated urls for pubsub #14261

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.

Sign up for GitHub

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

Merged
merged 25 commits into from
Aug 26, 2024
Merged

fix: use authenticated urls for pubsub #14261

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b89ff23
fix: use authenticated urls for pubsub
f0ssel Aug 13, 2024
88d50b8
comments
f0ssel Aug 13, 2024
8be0524
flake
f0ssel Aug 13, 2024
3be05c2
fmt
f0ssel Aug 13, 2024
c7964e4
temp add personal fork
f0ssel Aug 13, 2024
6ff2ec7
update coder fork
f0ssel Aug 14, 2024
02258ee
test pubsub
f0ssel Aug 14, 2024
9d8681b
test pubsub
f0ssel Aug 14, 2024
85f6cfe
t log
f0ssel Aug 14, 2024
dae69c7
add customer driver tests
f0ssel Aug 21, 2024
55df464
merge main
f0ssel Aug 21, 2024
2019fec
update flake
f0ssel Aug 21, 2024
ff2784c
lint
f0ssel Aug 21, 2024
c18963b
move to defer
f0ssel Aug 21, 2024
1600abf
fix tests
f0ssel Aug 22, 2024
839a52e
remove log
f0ssel Aug 22, 2024
428f3f5
merge main
f0ssel Aug 22, 2024
cd61cc7
update flake
f0ssel Aug 22, 2024
c946cdd
fmt
f0ssel Aug 22, 2024
1b139e4
close chan
f0ssel Aug 22, 2024
8c96f6e
use pq
f0ssel Aug 23, 2024
a645a76
merge main
f0ssel Aug 23, 2024
ed009f7
update flake
f0ssel Aug 23, 2024
c8a7c9a
fix flake
f0ssel Aug 23, 2024
05cbc3c
sleep to prevent flake
f0ssel Aug 26, 2024
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
Prev Previous commit
Next Next commit
comments
  • Loading branch information
@f0ssel
f0ssel committed Aug 14, 2024
commit 88d50b85e937bbfa777b5660cddd00b6dd87d0b8
3 changes: 2 additions & 1 deletion coderd/database/awsiamrds/awsiamrds.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func (d *awsIamRdsDriver) Open(name string) (driver.Conn, error) {
return conn, nil
}

// Connector returns a driver.Connector that fetches a new authentication token for each connection.
func (d *awsIamRdsDriver) Connector(name string) (driver.Connector, error) {
connector := &connector{
url: name,
Expand Down Expand Up @@ -122,7 +123,7 @@ func (c *connector) Connect(ctx context.Context) (driver.Conn, error) {
return nc.Connect(ctx)
}

func (c *connector) Driver() driver.Driver {
func (*connector) Driver() driver.Driver {
return &pq.Driver{}
}

Expand Down
2 changes: 2 additions & 0 deletions coderd/database/connector.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import (
"github.com/lib/pq"
)

// ConnectorCreator can create a driver.Connector.
type ConnectorCreator interface {
Connector(name string) (driver.Connector, error)
}

// DialerConnector can create a driver.Connector and set a pq.Dialer.
type DialerConnector interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Connect(context.Context) (driver.Conn, error)
Dialer(dialer pq.Dialer)
Expand Down
11 changes: 6 additions & 5 deletions coderd/database/pubsub/pubsub.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,15 +441,16 @@ func (p *PGPubsub) startListener(ctx context.Context, connectURL string) error {

// Create a custom connector if the database driver supports it.
connectorCreator, ok := p.db.Driver().(database.ConnectorCreator)
if !ok {
connector, err = pq.NewConnector(connectURL)
if ok {
connector, err = connectorCreator.Connector(connectURL)
Copy link
Contributor

Choose a reason for hiding this comment

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

This code doesn't get hit in the package unit tests. A good way to test it would be to create a pq Driver wrapper that we can control.

I'd like to see a test where we start pubsub with the wrapped driver, do some pub'ing and sub'ing, then kill the connection and verify that the pubsub / pq.Listener reconnects automatically. That would give a nice test of the pq changes you made as well.

if err != nil {
return xerrors.Errorf("create pq connector: %w", err)
return xerrors.Errorf("create custom connector: %w", err)
}
} else {
connector, err = connectorCreator.Connector(connectURL)
// use the default pq connector otherwise
connector, err = pq.NewConnector(connectURL)
if err != nil {
return xerrors.Errorf("create custom connector: %w", err)
return xerrors.Errorf("create pq connector: %w", err)
}
}

Expand Down