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.

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

feat(wren-ui): Added FE Support for MySQL SSL Connection #1072

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
25 changes: 19 additions & 6 deletions wren-ui/src/apollo/server/adaptors/tests/ibisAdaptor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
IbisQueryResponse,
ValidationRules,
} from '../ibisAdaptor';
import { DataSourceName } from '../../types';
import { DataSourceName, SSLMode } from '../../types';
import { Manifest } from '../../mdl/type';
import {
BIG_QUERY_CONNECTION_INFO,
Expand Down Expand Up @@ -45,6 +45,8 @@ describe('IbisAdaptor', () => {
database: 'my-database',
user: 'my-user',
password: 'my-password',
sslMode: SSLMode.VERIFY_CA,
sslCA: 'encrypted-certificate-string',
};

const mockPostgresConnectionInfo: POSTGRES_CONNECTION_INFO = {
Expand Down Expand Up @@ -175,17 +177,28 @@ describe('IbisAdaptor', () => {
mockedAxios.post.mockResolvedValue(mockResponse);
// mock decrypt method in Encryptor to return the same password
mockedEncryptor.prototype.decrypt.mockReturnValue(
JSON.stringify({ password: mockMySQLConnectionInfo.password }),
JSON.stringify({
password: mockMySQLConnectionInfo.password,
...(mockMySQLConnectionInfo.sslCA && { sslCA: mockMySQLConnectionInfo.sslCA })
}),
);

const result = await ibisAdaptor.getConstraints(
DataSourceName.MYSQL,
mockMySQLConnectionInfo,
);
const expectConnectionInfo = Object.entries(mockMySQLConnectionInfo).reduce(
(acc, [key, value]) => ((acc[snakeCase(key)] = value), acc),
{},
);
const expectConnectionInfo = Object.entries(
mockMySQLConnectionInfo,
).reduce((acc, [key, value]) => {
if (key === 'sslCA') {
acc['sslCA'] = Buffer
.from(mockMySQLConnectionInfo.sslCA)
.toString('base64');
} else {
acc[key] = value;
}
return acc;
}, {});

expect(result).toEqual([]);
expect(mockedAxios.post).toHaveBeenCalledWith(
Expand Down
18 changes: 15 additions & 3 deletions wren-ui/src/apollo/server/dataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,27 @@ const dataSource = {

// mysql
[DataSourceName.MYSQL]: {
sensitiveProps: ['password'],
sensitiveProps: ['password', 'sslCA'],
toIbisConnectionInfo(connectionInfo) {
const decryptedConnectionInfo = decryptConnectionInfo(
DataSourceName.MYSQL,
connectionInfo,
);
const { host, port, database, user, password } =
const { host, port, database, user, password, ...ssl } =
decryptedConnectionInfo as MYSQL_CONNECTION_INFO;
return { host, port, database, user, password };
return {
host,
port,
database,
user,
password,
sslMode: ssl.sslMode,
...(ssl.sslCA && {
sslCA: Buffer.from(
ssl.sslCA,
).toString('base64')
}),
};
},
} as IDataSourceConnectionInfo<
MYSQL_CONNECTION_INFO,
Expand Down
4 changes: 3 additions & 1 deletion wren-ui/src/apollo/server/repositories/projectRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
snakeCase,
isEmpty,
} from 'lodash';
import { DataSourceName } from '@server/types';
import { DataSourceName, SSLMode } from '@server/types';

export interface BIG_QUERY_CONNECTION_INFO {
projectId: string;
Expand All @@ -30,6 +30,8 @@ export interface MYSQL_CONNECTION_INFO {
user: string;
password: string;
database: string;
sslMode: SSLMode;
sslCA?: string;
}

export interface MS_SQL_CONNECTION_INFO {
Expand Down
1 change: 1 addition & 0 deletions wren-ui/src/apollo/server/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ export * from './manifest';
export * from './diagram';
export * from './metric';
export * from './context';
export * from './sslMode';
5 changes: 5 additions & 0 deletions wren-ui/src/apollo/server/types/sslMode.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export enum SSLMode {
DISABLED = 'disabled',
ENABLED = 'enabled',
VERIFY_CA = 'verify_ca',
}
ongdisheng marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,15 +1,73 @@
import { Form, Input } from 'antd';
import { useEffect, useState } from 'react';
import { Form, Input, Select, Button, Upload } from 'antd';
import UploadOutlined from '@ant-design/icons/UploadOutlined';
import { UploadFile } from 'antd/lib/upload/interface';
import { ERROR_TEXTS } from '@/utils/error';
import { FORM_MODE } from '@/utils/enum';
import { hostValidator } from '@/utils/validator';
import { SSLMode } from '@/apollo/server/types';

interface Props {
mode?: FORM_MODE;
}

Comment on lines 10 to 13
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Update Props interface with SSL-related properties.

The Props interface should be extended to include SSL-related properties for better type safety.

 interface Props {
   mode?: FORM_MODE;
+  sslMode?: SSLMode;
+  onSSLModeChange?: (mode: SSLMode) => void;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
interface Props {
mode?: FORM_MODE;
}
interface Props {
mode?: FORM_MODE;
sslMode?: SSLMode;
onSSLModeChange?: (mode: SSLMode) => void;
}

const UploadSSL = (props) => {
const { onChange, value } = props;
Comment on lines +14 to +15
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add TypeScript interface for component props

The UploadSSL component lacks a proper TypeScript interface for its props. This could lead to type-related issues.

Add a props interface:

interface UploadSSLProps {
  onChange?: (value: string | undefined) => void;
  value?: string;
}

const UploadSSL = (props: UploadSSLProps) => {


const [fileList, setFileList] = useState<UploadFile[]>([]);

useEffect(() => {
if (!value) setFileList([]);
}, [value]);

const readFileContent = (file: any, callback: (value: string) => void) => {
const reader = new FileReader();
reader.onloadend = (_e) => {
const result = reader.result;

if (result) {
const fileContent = String(result);
callback(fileContent);
}
};

reader.readAsText(file);
};
Comment on lines +23 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve type safety in file handling.

The file parameter is typed as any which bypasses TypeScript's type checking. Additionally, consider validating the file type before reading.

-  const readFileContent = (file: any, callback: (value: string) => void) => {
+  const readFileContent = (file: File, callback: (value: string) => void) => {
+    const validFileTypes = ['.pem', '.crt', '.key'];
+    const fileExtension = file.name.toLowerCase().slice(file.name.lastIndexOf('.'));
+    
+    if (!validFileTypes.includes(fileExtension)) {
+      console.error('Invalid file type');
+      return;
+    }
+
     const reader = new FileReader();
     reader.onloadend = (_e) => {
       const result = reader.result;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const readFileContent = (file: any, callback: (value: string) => void) => {
const reader = new FileReader();
reader.onloadend = (_e) => {
const result = reader.result;
if (result) {
const fileContent = String(result);
callback(fileContent);
}
};
reader.readAsText(file);
};
const readFileContent = (file: File, callback: (value: string) => void) => {
const validFileTypes = ['.pem', '.crt', '.key'];
const fileExtension = file.name.toLowerCase().slice(file.name.lastIndexOf('.'));
if (!validFileTypes.includes(fileExtension)) {
console.error('Invalid file type');
return;
}
const reader = new FileReader();
reader.onloadend = (_e) => {
const result = reader.result;
if (result) {
const fileContent = String(result);
callback(fileContent);
}
};
reader.readAsText(file);
};


const onUploadChange = (info) => {
const { file, fileList } = info;
if (fileList.length) {
const uploadFile = fileList[0];
readFileContent(file.originFileObj, (fileContent: string) => {
onChange && onChange(fileContent);
});
setFileList([uploadFile]);
}
};
Comment on lines +37 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve error handling and type safety in upload handling.

The upload handler should include error handling and proper TypeScript types.

-  const onUploadChange = (info) => {
+  const onUploadChange = (info: { file: UploadFile; fileList: UploadFile[] }) => {
     const { file, fileList } = info;
+    
+    if (file.status === 'error') {
+      console.error('File upload failed:', file.error);
+      return;
+    }
+
     if (fileList.length) {
       const uploadFile = fileList[0];
-      readFileContent(file.originFileObj, (fileContent: string) => {
-        onChange && onChange(fileContent);
-      });
+      if (file.originFileObj) {
+        readFileContent(file.originFileObj, (fileContent: string) => {
+          onChange?.(fileContent);
+        });
+      }
       setFileList([uploadFile]);
     }
   };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const onUploadChange = (info) => {
const { file, fileList } = info;
if (fileList.length) {
const uploadFile = fileList[0];
readFileContent(file.originFileObj, (fileContent: string) => {
onChange && onChange(fileContent);
});
setFileList([uploadFile]);
}
};
const onUploadChange = (info: { file: UploadFile; fileList: UploadFile[] }) => {
const { file, fileList } = info;
if (file.status === 'error') {
console.error('File upload failed:', file.error);
return;
}
if (fileList.length) {
const uploadFile = fileList[0];
if (file.originFileObj) {
readFileContent(file.originFileObj, (fileContent: string) => {
onChange?.(fileContent);
});
}
setFileList([uploadFile]);
}
};
🧰 Tools
🪛 Biome (1.9.4)

[error] 42-42: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


const onRemove = () => {
setFileList([]);
onChange && onChange(undefined);
};

return (
<Upload
accept=".pem,.crt,.key"
fileList={fileList}
onChange={onUploadChange}
onRemove={onRemove}
maxCount={1}
>
<Button icon={<UploadOutlined />}>Click to upload SSL cert file</Button>
</Upload>
);
};

export default function MySQLProperties(props: Props) {
const { mode } = props;
const isEditMode = mode === FORM_MODE.EDIT;
const [sslMode, setSSLMode] = useState<string>(SSLMode.DISABLED);
const onSSLModeChange = (value: string) => setSSLMode(value)
return (
<>
<Form.Item
Expand Down Expand Up @@ -89,6 +147,34 @@ export default function MySQLProperties(props: Props) {
>
<Input placeholder="MySQL database name" disabled={isEditMode} />
</Form.Item>
<Form.Item label="SSL mode" name="sslMode" initialValue={SSLMode.DISABLED}>
<Select
style={{ width: 120 }}
onChange={onSSLModeChange}
disabled={isEditMode}
options={[
{ value: SSLMode.DISABLED, label: 'Disabled' },
{ value: SSLMode.ENABLED, label: 'Enabled' },
{ value: SSLMode.VERIFY_CA, label: 'Verify CA' },
]}
/>
</Form.Item>
{
sslMode === SSLMode.VERIFY_CA &&
<Form.Item
label="SSL CA file"
name="sslCA"
required
rules={[
{
required: true,
message: ERROR_TEXTS.CONNECTION.SSL_CERT.REQUIRED,
},
]}
>
<UploadSSL />
</Form.Item>
}
</>
);
}
3 changes: 3 additions & 0 deletions wren-ui/src/utils/error/dictionary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ export const ERROR_TEXTS = {
ACCOUNT: {
REQUIRED: 'Please input account.',
},
SSL_CERT: {
REQUIRED: 'Please upload SSL cert file.',
},
},
ADD_RELATION: {
FROM_FIELD: {
Expand Down