Skip to content
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

Jump to bottom

Add address checks for mysql and postgres #1959

Merged
merged 3 commits into from
Oct 27, 2023
Merged

Add address checks for mysql and postgres #1959

merged 3 commits into from
Oct 27, 2023

Conversation

rylev
Copy link
Collaborator

@rylev rylev commented Oct 26, 2023

Continuation of #1933 but for MySQL and PostgreSQL.

Users of the v2 mysql and postgres interfaces must now add the hosts they are allowed to call to the manifest.

@rylev rylev requested a review from lann October 26, 2023 13:18
@rylev rylev force-pushed the address-checks-mysql-postgres branch 4 times, most recently from 98c945d to 86f1cf6 Compare October 27, 2023 07:54
@radu-matei
Copy link
Member

radu-matei commented Oct 27, 2023 •
edited
Loading

The end-to-end failures are perfectly catching the missing outbound hosts:

Failures:
    spinup_tests::component_outbound_http_works
    spinup_tests::http_rust_outbound_mysql_works
    spinup_tests::http_rust_outbound_pg_works

The MySQL test error:

 right: `500`: Expected status 200 but got 500 Internal Server Error. Response body: 'Error::ConnectionFailed("address ***mysql/spin_dev is not permitted")'', tests/testcases/mod.rs:982:30

radu-matei
radu-matei reviewed Oct 27, 2023
View reviewed changes
crates/outbound-networking/src/lib.rs Outdated
if !is_allowed {
terminal::warn!("A component tried to make a request to non-allowed address '{address}'.");
let (host, port) = (address.host(), address.port());
eprintln!("To allow requests, add 'allowed_outbound_hosts = '[\"{host}:{port}\"]' to the manifest component section.");
Copy link
Member

Choose a reason for hiding this comment

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

❤️

radu-matei
radu-matei approved these changes Oct 27, 2023
View reviewed changes
Copy link
Member

@radu-matei radu-matei left a comment

Choose a reason for hiding this comment

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

LGTM once the e2e tests pass.

Thanks!

@rylev
Add address check for mysql
36d04ec 
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
@rylev
Add address check for postgres
bc80401 
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
@rylev
Fix e2e tests
2692d57 
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
@rylev rylev force-pushed the address-checks-mysql-postgres branch from 31e0bae to 2692d57 Compare October 27, 2023 16:36
@rylev rylev enabled auto-merge October 27, 2023 16:37
@rylev rylev merged commit 47e40b9 into main Oct 27, 2023
9 checks passed
@rylev rylev deleted the address-checks-mysql-postgres branch October 27, 2023 17:35
This was referenced Oct 27, 2023
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lann lann lann approved these changes

@radu-matei radu-matei radu-matei approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Spin 2.0
Status: ✅ Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rylev @radu-matei @lann