Skip to content

Commit fc02b24

Browse files
committed
refactor: remove unused SetSortByFieldPrefix and SetKeyFieldPrefix methods from Options
refactor(dialect): enhance identifier quoting to escape embedded quotes for MySQL, PostgreSQL, and SQLite refactor: rename qualifyIdentifier to QualifyIdentifier and update references feat: remove PostgreSQL-MinIO test configuration chore: update Go version to 1.24 in integration test workflows refactor: streamline PostgreSQL integration by updating kustomization files and removing unused network policies refactor: update PostgreSQL network policy references and remove obsolete file Signed-off-by: kaikaila <[email protected]>
1 parent 12a3ddc commit fc02b24

35 files changed

+318
-1722
lines changed

.github/resources/manifests/multiuser/postgresql-minio/apiserver-env.yaml

Lines changed: 0 additions & 16 deletions
This file was deleted.

.github/resources/manifests/multiuser/postgresql-minio/kustomization.yaml

Lines changed: 0 additions & 83 deletions
This file was deleted.

.github/resources/manifests/standalone/postgresql-minio/apiserver-env.yaml

Lines changed: 0 additions & 16 deletions
This file was deleted.

.github/resources/manifests/standalone/postgresql-minio/kustomization.yaml

Lines changed: 0 additions & 88 deletions
This file was deleted.

.github/resources/scripts/deploy-kfp.sh

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -142,38 +142,35 @@ if [ "${MULTI_USER}" == "true" ]; then
142142
fi
143143

144144
# Manifests will be deployed according to the flag provided
145+
# Priority 1: Multi-user false + standalone (not kubernetes-native)
145146
if [ "${MULTI_USER}" == "false" ] && [ "${PIPELINES_STORE}" != "kubernetes" ]; then
146147
TEST_MANIFESTS="${TEST_MANIFESTS}/standalone"
147148

148-
# Priority 1: TLS-enabled (mutually exclusive with other options)
149+
# Priority 1.1: TLS-enabled (mutually exclusive with other options)
149150
if $POD_TO_POD_TLS_ENABLED; then
150151
TEST_MANIFESTS="${TEST_MANIFESTS}/tls-enabled"
151152

152-
# Priority 2: PostgreSQL + MinIO combination
153-
elif [ "${DB_TYPE}" == "pgx" ] && [ "${STORAGE_BACKEND}" == "minio" ]; then
154-
TEST_MANIFESTS="${TEST_MANIFESTS}/postgresql-minio"
155-
156-
# Priority 3: PostgreSQL (with default SeaweedFS storage)
153+
# Priority 1.2: PostgreSQL (with default SeaweedFS storage)
157154
elif [ "${DB_TYPE}" == "pgx" ]; then
158155
TEST_MANIFESTS="${TEST_MANIFESTS}/postgresql"
159156

160-
# Priority 4: Check for cache-disabled + proxy + minio combination
157+
# Priority 1.3: Check for cache-disabled + proxy + minio combination
161158
elif $CACHE_DISABLED && $USE_PROXY && [ "${STORAGE_BACKEND}" == "minio" ]; then
162159
TEST_MANIFESTS="${TEST_MANIFESTS}/cache-disabled-proxy-minio"
163160

164-
# Priority 5: Check for cache-disabled + proxy combination
161+
# Priority 1.4: Check for cache-disabled + proxy combination
165162
elif $CACHE_DISABLED && $USE_PROXY; then
166163
TEST_MANIFESTS="${TEST_MANIFESTS}/cache-disabled-proxy"
167164

168-
# Priority 6: Check for cache-disabled + minio combination
165+
# Priority 1.5: Check for cache-disabled + minio combination
169166
elif $CACHE_DISABLED && [ "${STORAGE_BACKEND}" == "minio" ]; then
170167
TEST_MANIFESTS="${TEST_MANIFESTS}/cache-disabled-minio"
171168

172-
# Priority 7: Check for proxy + minio combination
169+
# Priority 1.6: Check for proxy + minio combination
173170
elif $USE_PROXY && [ "${STORAGE_BACKEND}" == "minio" ]; then
174171
TEST_MANIFESTS="${TEST_MANIFESTS}/proxy-minio"
175172

176-
# Priority 8: Check for single flags (cache-disabled, proxy, or minio)
173+
# Priority 1.7: Check for single flags (cache-disabled, proxy, or minio)
177174
elif $CACHE_DISABLED; then
178175
TEST_MANIFESTS="${TEST_MANIFESTS}/cache-disabled"
179176
elif $USE_PROXY; then
@@ -185,37 +182,35 @@ if [ "${MULTI_USER}" == "false" ] && [ "${PIPELINES_STORE}" != "kubernetes" ]; t
185182
else
186183
TEST_MANIFESTS="${TEST_MANIFESTS}/default"
187184
fi
185+
# Priority 2: Multi-user false + kubernetes-native
188186
elif [ "${MULTI_USER}" == "false" ] && [ "${PIPELINES_STORE}" == "kubernetes" ]; then
189187
TEST_MANIFESTS="${TEST_MANIFESTS}/kubernetes-native"
190188
if $CACHE_DISABLED; then
191189
TEST_MANIFESTS="${TEST_MANIFESTS}/cache-disabled"
192190
else
193191
TEST_MANIFESTS="${TEST_MANIFESTS}/default"
194192
fi
193+
# Priority 3: Multi-user true
195194
elif [ "${MULTI_USER}" == "true" ]; then
196195
TEST_MANIFESTS="${TEST_MANIFESTS}/multiuser"
197196

198-
# Priority 1: PostgreSQL + MinIO
199-
if [ "${DB_TYPE}" == "pgx" ] && [ "${STORAGE_BACKEND}" == "minio" ]; then
200-
TEST_MANIFESTS="${TEST_MANIFESTS}/postgresql-minio"
201-
202-
# Priority 2: PostgreSQL (with default SeaweedFS)
203-
elif [ "${DB_TYPE}" == "pgx" ]; then
197+
# Priority 3.1: PostgreSQL (with default SeaweedFS)
198+
if [ "${DB_TYPE}" == "pgx" ]; then
204199
TEST_MANIFESTS="${TEST_MANIFESTS}/postgresql"
205200

206-
# Priority 3: Artifact proxy (MySQL + SeaweedFS)
201+
# Priority 3.2: Artifact proxy (MySQL + SeaweedFS)
207202
elif $ARTIFACT_PROXY_ENABLED && [ "${STORAGE_BACKEND}" == "seaweedfs" ]; then
208203
TEST_MANIFESTS="${TEST_MANIFESTS}/artifact-proxy"
209204

210-
# Priority 4: Cache disabled + MinIO (MySQL + MinIO)
205+
# Priority 3.3: Cache disabled + MinIO (MySQL + MinIO)
211206
elif $CACHE_DISABLED && [ "${STORAGE_BACKEND}" == "minio" ]; then
212207
TEST_MANIFESTS="${TEST_MANIFESTS}/cache-disabled-minio"
213208

214-
# Priority 5: MinIO (MySQL + MinIO)
209+
# Priority 3.4: MinIO (MySQL + MinIO)
215210
elif [ "${STORAGE_BACKEND}" == "minio" ]; then
216211
TEST_MANIFESTS="${TEST_MANIFESTS}/minio"
217212

218-
# Priority 6: Cache disabled (MySQL + SeaweedFS)
213+
# Priority 3.5: Cache disabled (MySQL + SeaweedFS)
219214
elif $CACHE_DISABLED; then
220215
TEST_MANIFESTS="${TEST_MANIFESTS}/cache-disabled"
221216

.github/workflows/e2e-test.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ jobs:
105105
argo_version: "v3.6.7"
106106
- db_type: "pgx"
107107
argo_version: "v3.5.14"
108+
- db_type: "pgx"
109+
storage: "minio"
108110
fail-fast: false
109111
name: End to End ${{ matrix.test_label}} Tests - K8s ${{ matrix.k8s_version }} cacheEnabled=${{ matrix.cache_enabled }} argoVersion=${{ matrix.argo_version}} proxy=${{ matrix.proxy}} storage=${{ matrix.storage }} pod_to_pod_tls_enabled=${{ matrix.pod_to_pod_tls_enabled }} dbType=${{ matrix.db_type }}
110112
steps:
@@ -201,6 +203,9 @@ jobs:
201203
cache_enabled: "true"
202204
db_type: "mysql"
203205
test_label: "E2ECritical"
206+
exclude:
207+
- db_type: "pgx"
208+
storage: "minio"
204209
fail-fast: false
205210
name: End to End Critical Scenario Multi User Tests - K8s ${{ matrix.k8s_version }} cacheEnabled=${{ matrix.cache_enabled }} multiUser=${{ matrix.multi_user }} storage=${{ matrix.storage }} artifactProxy=${{ matrix.artifact_proxy }} dbType=${{ matrix.db_type }}
206211
steps:

.github/workflows/integration-tests-v1.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ jobs:
5252
if: ${{ matrix.db_type == 'pgx' }}
5353
uses: actions/setup-go@v5
5454
with:
55-
go-version: "1.22"
55+
go-version: "1.24"
5656
cache: true
5757

5858
- name: Create KFP cluster

.github/workflows/legacy-v2-api-integration-tests-postgres.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ jobs:
5151
- name: Set up Go
5252
uses: actions/setup-go@v5
5353
with:
54-
go-version: "1.22"
54+
go-version: "1.24"
5555
cache: true
5656

5757
- name: Create KFP cluster

backend/src/apiserver/common/sql/dialect/dialect.go

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,27 +45,36 @@ func NewDBDialect(name string) DBDialect {
4545
switch name {
4646
case "mysql":
4747
return DBDialect{
48-
name: "mysql",
49-
quoteIdentifier: func(id string) string { return fmt.Sprintf("`%s`", id) },
50-
lengthFunc: "CHAR_LENGTH",
48+
name: "mysql",
49+
quoteIdentifier: func(id string) string {
50+
escaped := strings.ReplaceAll(id, "`", "``")
51+
return fmt.Sprintf("`%s`", escaped)
52+
},
53+
lengthFunc: "CHAR_LENGTH",
5154
statementBuilder: sq.StatementBuilder.
5255
PlaceholderFormat(sq.Question),
5356
existDatabaseErrHint: "database exists",
5457
}
5558
case "pgx":
5659
return DBDialect{
57-
name: "pgx",
58-
quoteIdentifier: func(id string) string { return fmt.Sprintf(`"%s"`, id) },
59-
lengthFunc: "CHAR_LENGTH",
60+
name: "pgx",
61+
quoteIdentifier: func(id string) string {
62+
escaped := strings.ReplaceAll(id, `"`, `""`)
63+
return fmt.Sprintf(`"%s"`, escaped)
64+
},
65+
lengthFunc: "CHAR_LENGTH",
6066
statementBuilder: sq.StatementBuilder.
6167
PlaceholderFormat(sq.Dollar),
6268
existDatabaseErrHint: "already exists",
6369
}
6470
case "sqlite": // only for tests
6571
return DBDialect{
66-
name: "sqlite",
67-
quoteIdentifier: func(id string) string { return fmt.Sprintf(`"%s"`, id) },
68-
lengthFunc: "LENGTH",
72+
name: "sqlite",
73+
quoteIdentifier: func(id string) string {
74+
escaped := strings.ReplaceAll(id, `"`, `""`)
75+
return fmt.Sprintf(`"%s"`, escaped)
76+
},
77+
lengthFunc: "LENGTH",
6978
statementBuilder: sq.StatementBuilder.
7079
PlaceholderFormat(sq.Question),
7180
existDatabaseErrHint: "",

0 commit comments

Comments
 (0)