refactor: remove private link related connection #18975
Conversation
Signed-off-by: tabVersion <[email protected]>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
license-eye has totally checked 5395 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 2289 | 1 | 3105 | 0 |
Click to see the invalid file list
- src/meta/service/src/cloud_service.rs
Signed-off-by: tabVersion <[email protected]>
|
|
||
| #[async_trait] | ||
| impl CloudService for CloudServiceImpl { | ||
| async fn rw_cloud_validate_source( |
There was a problem hiding this comment.
This rpc interface is provided to the cloud portal previously, I am not sure whether it is still valid. Please confirm with @cloudcarver @neverchanje
#10139
There was a problem hiding this comment.
@Gogomoe worked on this previously
There was a problem hiding this comment.
The cloud service is call via cloudagent rpc, according to https://risingwave-labs.slack.com/archives/C03DFGV6L2W/p1729495045551239?thread_ts=1729479931.025979&cid=C03DFGV6L2W.
PingSource func is used in https://github.com/risingwavelabs/risingwave-cloud/blob/main/mgmt/internal/apiservice/controllers/controller_v1/source.go#L124 and https://github.com/risingwavelabs/risingwave-cloud/blob/main/mgmt/internal/apiservice/controllers/controller_v2/sources.go#L160.
But upper level func PostTenantsNsIdSourcesPing and PostTenantTenantIdSourcesPing are not called in the codebase. Seeing them as deprecated.
Will proceed on removing the service.
Will revert the Cloud Service, PingSource is still in use.
Signed-off-by: tabVersion <[email protected]>
There was a problem hiding this comment.
license-eye has totally checked 5395 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 2288 | 2 | 3105 | 0 |
Click to see the invalid file list
- src/meta/service/src/cloud_service.rs
- src/meta/src/rpc/ddl_controller_v2.rs
| @@ -26,6 +26,8 @@ pub struct Model { | |||
| #[sea_orm(primary_key, auto_increment = false)] | |||
| pub connection_id: ConnectionId, | |||
| pub name: String, | |||
|
|
|||
| // todo: Private link service has been deprecated, consider using a new field for the connection info | |||
There was a problem hiding this comment.
I think we should deprecate the unnecessary fields in message PrivateLinkService, only keeping the necessary fields for create source.
There was a problem hiding this comment.
In long term, the create connection statement should be kept, but the semantic of it is simply storing the user-specific endpoint and targets, instead of calling AWS API to create a private link. This aligns with the style of the connection RFC.
There was a problem hiding this comment.
I am not sure if it is safe to remove the field. The exisiting connection still need this.
After the cloud portal supports building subnets for private link (long ago), we no longer encourage using create connection for this case.
but the semantic of it is simply storing the user-specific endpoint and targets, instead of calling AWS API to create a private link
IIRC, we store the two fields together with other connector props, rather than in the connection catalog.
There was a problem hiding this comment.
IIRC, we store the two fields together with other connector props, rather than in the connection catalog.
I see. Then we may deprecate the entire message PrivateLinkService. See also https://stackoverflow.com/a/52787662/5739882.
There was a problem hiding this comment.
Does the change involve a new migration, cc @yezizp2012 ?
src/meta/service/src/ddl_service.rs
Outdated
| ))); | ||
| } | ||
| } | ||
| if let Some(connection::Info::PrivateLinkService(_)) = &connection.info { |
There was a problem hiding this comment.
Here is in validate_connection. Will it be called during startup or recovery? If possible, we should allow an existing PrivateLink connection to work after upgrading RW.
There was a problem hiding this comment.
Will it be called during startup or recovery
Nope. We only call validate_connection when create_source and create_sink, not blocking recovery.
Signed-off-by: tabVersion <[email protected]>
There was a problem hiding this comment.
license-eye has totally checked 5397 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 2289 | 2 | 3106 | 0 |
Click to see the invalid file list
- src/meta/service/src/cloud_service.rs
- src/meta/src/rpc/ddl_controller_v2.rs
Signed-off-by: tabVersion <[email protected]>
There was a problem hiding this comment.
license-eye has totally checked 5397 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 2290 | 1 | 3106 | 0 |
Click to see the invalid file list
- src/meta/src/rpc/ddl_controller_v2.rs
There was a problem hiding this comment.
license-eye has totally checked 5397 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 2290 | 1 | 3106 | 0 |
Click to see the invalid file list
- src/meta/src/rpc/ddl_controller_v2.rs
Signed-off-by: tabVersion <[email protected]>
Signed-off-by: tabVersion <[email protected]>
Signed-off-by: tabVersion <[email protected]>
Signed-off-by: tabVersion <[email protected]>
Signed-off-by: tabVersion <[email protected]>
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 9425213 | Triggered | Generic Password | 635ca5e | e2e_test/source_inline/tvf/postgres_query.slt | View secret |
| 9425213 | Triggered | Generic Password | 635ca5e | e2e_test/source_inline/tvf/postgres_query.slt | View secret |
| 9425213 | Triggered | Generic Password | 635ca5e | risedev.yml | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
I think yes, this is documented https://docs.risingwave.com/docs/current/sql-create-connection/#create-an-aws-privatelink-connection |
yes, config aws with kernel code is not recommended long ago. |
|
tested via RisingWave Cloud, PingSource func works as expected. 
|
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
as title. Deprecate change cloud vendor stuff from kernel
Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.