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

fix(secret): add a secret ref column to source #17269

Merged
merged 9 commits into from
Jun 17, 2024
Merged

fix(secret): add a secret ref column to source #17269

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7ee917c
init
yuhao-su Jun 17, 2024
fef9e6b
more
yuhao-su Jun 17, 2024
abb8e91
format
yuhao-su Jun 17, 2024
baf290e
name
yuhao-su Jun 17, 2024
ce24a2e
fix proto
yuhao-su Jun 17, 2024
6935c96
improve
yuhao-su Jun 17, 2024
4865c83
resolve conflict
yuhao-su Jun 17, 2024
d13ed73
Merge branch 'main' of github.com:singularity-data/risingwave into yu…
yuhao-su Jun 17, 2024
7a9229d
comment
yuhao-su Jun 17, 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
26 changes: 25 additions & 1 deletion src/meta/model_v2/migration/src/m20240525_090457_secret.rs
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

you should not modify an existing migration, just create a new one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yezizp2012 said it's ok since it's not released

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are more to change. secret_ref should not be map<string, u32>. We need more than a secret id like if it's an as file

Copy link
Contributor

Choose a reason for hiding this comment

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

There are more to change. secret_ref should not be map<string, u32>.

Here is a little diff with my assumption, ref secret <secret-name> means the source/sink relies on the secret binary. Seeing it as a string, or a file, is something more related to the exec itself rather than spec in catalog.
Just share my thoughts, it is okay you impl in your way.

Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl MigrationTrait for Migration {
)
.await?;

// Add a new column to the table
// Add a new column to the `sink` table
manager
.alter_table(
MigrationTable::alter()
Expand All @@ -47,6 +47,16 @@ impl MigrationTrait for Migration {
)
.await?;

// Add a new column to the `source` table
manager
.alter_table(
MigrationTable::alter()
.table(Source::Table)
.add_column(ColumnDef::new(Source::SecretRef).json_binary())
.to_owned(),
)
.await?;

Ok(())
}

Expand All @@ -60,6 +70,14 @@ impl MigrationTrait for Migration {
.to_owned(),
)
.await?;
manager
.alter_table(
MigrationTable::alter()
.table(Source::Table)
.drop_column(Source::SecretRef)
.to_owned(),
)
.await?;
Ok(())
}
}
Expand All @@ -77,3 +95,9 @@ enum Sink {
Table,
SecretRef,
}

#[derive(DeriveIden)]
enum Source {
Table,
SecretRef,
}
7 changes: 5 additions & 2 deletions src/meta/model_v2/src/source.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ use sea_orm::ActiveValue::Set;
use serde::{Deserialize, Serialize};

use crate::{
ColumnCatalogArray, ConnectionId, I32Array, Property, SourceId, StreamSourceInfo, TableId,
WatermarkDescArray,
ColumnCatalogArray, ConnectionId, I32Array, Property, SecretRef, SourceId, StreamSourceInfo,
TableId, WatermarkDescArray,
};

#[derive(Clone, Debug, PartialEq, DeriveEntityModel, Eq, Serialize, Deserialize)]
Expand All @@ -39,6 +39,8 @@ pub struct Model {
pub optional_associated_table_id: Option<TableId>,
pub connection_id: Option<ConnectionId>,
pub version: i64,
// `secret_ref` stores a json string, mapping from property name to secret id.
yuhao-su marked this conversation as resolved.
Show resolved Hide resolved
pub secret_ref: Option<SecretRef>,
}

#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)]
Expand Down Expand Up @@ -101,6 +103,7 @@ impl From<PbSource> for ActiveModel {
optional_associated_table_id: Set(optional_associated_table_id),
connection_id: Set(source.connection_id.map(|id| id as _)),
version: Set(source.version as _),
secret_ref: Set(None),
}
}
}
Loading