Sync Release 2.14.x #79
Conversation
Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
Signed-off-by: Daniel Jiang <[email protected]>
…#22331) Remove port 9443 from harbor IP for webhook event check for LDAP test, the default port changed from 443 to 9443, the original hostname contains IP:PORT, but the webhook event just contains the IP, need remove port before validation. fix issue related to CI pass checks Signed-off-by: stonezdj <[email protected]>
Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
…22332) Remove GCR replication because GCR account is removed Signed-off-by: stonezdj <[email protected]>
Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
fix robot issue-21406 fixes #21406 The changes are target to fix the system robot to create a project level robots. It should not to get the creator robot with the createe robot's project id, and updates the code get the creator robot from the security context. Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
…#22409) limit the proxy connection to upstream registry Signed-off-by: stonezdj <[email protected]>
Add UI for limit upstream registry connection Signed-off-by: stonezdj <[email protected]>
Allow disable audit log to DB when initialize Fixes #22257 Add support for SKIP_LOG_AUDIT_DATABASE env to control audit log database storage during Harbor initialization. Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
add migraiton sql for skip_audit_log_database give the value of skip_audit_log_database bases on the real usage. 1. If audit logs exist in the system (either in audit_log or audit_log_ext tables), set skip_audit_log_database to false 2. If no audit logs exist but the tables show evidence of previous usage set skip_audit_log_database to false Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
refactor the logic to init skip_audit_log_database simplify the sql to set the value of skip_audit_log_database, any evidence of previous usage of db to record the audit, set it to false. Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
Signed-off-by: Daniel Jiang <[email protected]>
Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
Pin to trivy adapter v0.34.1 Signed-off-by: Daniel Jiang <[email protected]>
… /src (#22518) chore(deps): bump github.com/golang-jwt/jwt/v5 in /src Bumps [github.com/golang-jwt/jwt/v5](https://github.com/golang-jwt/jwt) from 5.2.2 to 5.3.0. - [Release notes](https://github.com/golang-jwt/jwt/releases) - [Changelog](https://github.com/golang-jwt/jwt/blob/main/VERSION_HISTORY.md) - [Commits](golang-jwt/jwt@v5.2.2...v5.3.0) --- updated-dependencies: - dependency-name: github.com/golang-jwt/jwt/v5 dependency-version: 5.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
) Bumps [k8s.io/client-go](https://github.com/kubernetes/client-go) from 0.33.3 to 0.34.1. - [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md) - [Commits](kubernetes/client-go@v0.33.3...v0.34.1) --- updated-dependencies: - dependency-name: k8s.io/client-go dependency-version: 0.34.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Making sure it compiles under golang v1.24 Signed-off-by: Daniel Jiang <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds Redis-backed upstream connection limiting with project-scoped or artifact-scoped keys, exposes Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Proxy as RepoProxy
participant Project as ProjectMeta
participant Limiter as ConnLimiter
participant Redis
participant Upstream as UpstreamRegistry
Client->>Proxy: GET blob/manifest
Proxy->>Project: MaxUpstreamConnection()
alt limit > 0
Proxy->>Limiter: Acquire(ctx, key, limit)
Limiter->>Redis: EVAL increaseWithLimit (Lua)
alt acquired
Redis-->>Limiter: ok
Limiter-->>Proxy: true
Proxy->>Upstream: proxy request upstream
Upstream-->>Proxy: response
Proxy->>Limiter: Release(ctx, key)
Limiter->>Redis: EVAL decrease (Lua)
Redis-->>Limiter: ok
Proxy-->>Client: response
else exceeded
Redis-->>Limiter: deny
Limiter-->>Proxy: false
Proxy-->>Client: 429 Too Many Requests
end
else no limit
Proxy->>Upstream: proxy request upstream
Upstream-->>Proxy: response
Proxy-->>Client: response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This pull request adds support for configuring maximum upstream registry connections for proxy cache projects and introduces functionality to skip audit log database writes via environment variables. The PR also updates Go version to 1.24.9 and updates several dependencies.
- Adds a new
max_upstream_connmetadata field to control concurrent connections to upstream registries in proxy cache projects - Implements connection limiting using Redis-based semaphores
- Adds ability to initialize
skip_audit_log_databaseconfiguration from environment variables - Updates Go version from 1.24.6 to 1.24.9 and updates dependencies including k8s libraries
- Removes obsolete Google GCR replication test case
Reviewed Changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pkg/proxy/connection/limit.go | New connection limiter implementation using Redis for semaphore-based connection limiting |
| src/server/middleware/repoproxy/proxy.go | Integrates connection limiting into blob and manifest proxy handlers |
| src/pkg/project/models/project.go | Adds MaxUpstreamConnection method to read the new metadata field |
| src/server/v2.0/handler/robot.go | Refactors robot creation to validate permissions earlier in the security context |
| src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.ts | Adds UI validation for max upstream connection field |
| src/core/main.go | Adds initialization of skip_audit_log_database from environment variables |
| src/go.mod | Updates Go version to 1.24.9 and updates k8s, jwt, and other dependencies |
| tests/robot-cases/Group1-Nightly/Replication.robot | Removes obsolete Google GCR replication test |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| cnt, err := strconv.ParseInt(countVal, 10, 32) | ||
| if err != nil { | ||
| log.Warningf("failed th parse the max_upstream_conn, val:%s error %v", countVal, err) |
There was a problem hiding this comment.
Corrected spelling of 'th' to 'to' in the log message.
| log.Warningf("failed th parse the max_upstream_conn, val:%s error %v", countVal, err) | |
| log.Warningf("failed to parse the max_upstream_conn, val:%s error %v", countVal, err) |
| return err | ||
| } | ||
| } else { | ||
| log.Debugf("key SkipAuditLogDatabase aleady exist in the db with value %v", val) |
There was a problem hiding this comment.
Corrected spelling of 'aleady' to 'already' in the log message.
| log.Debugf("key SkipAuditLogDatabase aleady exist in the db with value %v", val) | |
| log.Debugf("key SkipAuditLogDatabase already exist in the db with value %v", val) |
| } | ||
|
|
||
| this.validateMaxUpstreamConnections(); | ||
| if (this.bandwidthError) { |
There was a problem hiding this comment.
The condition checks this.bandwidthError but displays this.maxUpstreamConnError. This should check this.maxUpstreamConnError instead to properly validate the max upstream connection field.
| if (this.bandwidthError) { | |
| if (this.maxUpstreamConnError) { |
| "NO_PROJECT": "未发现任何项目" | ||
| "NO_PROJECT": "未发现任何项目", | ||
| "CONNECTION_LIMIT": "Max connection to upstream registry", | ||
| "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if less than 0, then there is no limit", |
There was a problem hiding this comment.
The description is inconsistent with other language files and the validation logic. The English version states 'if -1, then there is no limit' while this states 'if less than 0'. The validation only accepts -1 or values greater than 0, so this should match the English version for consistency.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/v2.0/handler/robot.go (1)
446-483: Fix Effect field handling inisValidPermissionScopeto account for empty/unset values.The function directly uses the
Effectfield in map key generation (lines 457, 461), but test cases showEffectcan be empty. When empty, the comparison fails even for matching permissions. Use theGetEffect()method which provides the correct default value:key := fmt.Sprintf("%s:%s:%s", creatorP.Resource, creatorP.Action, creatorP.GetEffect())Apply the same fix to both locations where Effect is used in map key creation within
hasLessThanOrEqualAccess.
🧹 Nitpick comments (21)
.github/workflows/pass-CI.yml (1)
5-5: Good documentation addition with minor grammar note.The comments help maintainers keep the paths synchronized between pass-CI.yml and CI.yml.
Minor grammar suggestion (optional): "the paths is" → "the paths are"
Also applies to: 17-17
.github/workflows/CI.yml (1)
28-30: Consider removing redundant path pattern.The pattern
!tests/robot-cases/Group1-Nightly/**is redundant because!tests/robot-cases/**already includes all subdirectories undertests/robot-cases/, includingGroup1-Nightly/.While this doesn't break functionality, removing the redundant pattern would simplify maintenance.
Apply this diff to both the
pull_requestandpushsections:- '!tests/resources/**' - '!tests/robot-cases/**' - - '!tests/robot-cases/Group1-Nightly/**'Also applies to: 40-42
tests/resources/TestCaseBody.robot (1)
24-27: Consider making the port removal more flexible.The
Remove Portkeyword is hardcoded to remove:9443specifically. If the port number changes or varies across test environments, this implementation will fail silently or produce incorrect results.Consider making the port parameter configurable:
Remove Port - [Arguments] ${address} - ${result}= Replace String ${address} :9443 ${EMPTY} + [Arguments] ${address} ${port}=:9443 + ${result}= Replace String ${address} ${port} ${EMPTY} [Return] ${result}Alternatively, use a regex pattern to remove any port:
Remove Port [Arguments] ${address} ${result}= Evaluate re.sub(r':\d+$', '', '${address}') modules=re [Return] ${result}src/pkg/proxy/connection/limit.go (2)
37-48: Consider reducing the expiry time to prevent leaked connection slots.The 3600-second (1 hour) expiry is quite long. If a connection is not properly released due to a crash or bug, the slot remains occupied for an hour, potentially causing artificial connection exhaustion.
Consider a shorter TTL (e.g., 300-600 seconds) and implement a heartbeat/refresh mechanism in the proxy middleware to periodically extend the TTL for active connections:
local current = tonumber(redis.call('GET', KEYS[1]) or '0') local max = tonumber(ARGV[1]) if current + 1 <= max then redis.call('INCRBY', KEYS[1], 1) redis.call('EXPIRE', KEYS[1], 300) -- 5 minutes instead of 1 hour return 1 else return 0 endAlternatively, implement explicit cleanup logic in the middleware to ensure
Releaseis always called viadefer.
74-79: Consider logging release failures at Error level.The
Releasemethod logs failures atInfolevel, which might cause connection leaks to go unnoticed in production. Since failed releases can lead to artificial connection exhaustion, these should be more visible.func (c *ConnLimiter) Release(ctx context.Context, rdb *redis.Client, key string) { _, err := decreaseScript.Run(ctx, rdb, []string{key}).Int() if err != nil { - log.Infof("release connection failed:%v", err) + log.Errorf("failed to release connection for key %s: %v", key, err) } }src/pkg/proxy/connection/limit_test.go (1)
27-59: Add cleanup and consider additional test cases.The test validates the happy path effectively, but could be improved:
- Missing cleanup: The test key remains in Redis after the test completes
- No error case testing: Should test Redis connection failures, invalid parameters, etc.
- No concurrent access testing: The limiter will be used concurrently in production
Add cleanup:
func TestConnLimiter_Acquire_Release(t *testing.T) { redisAddress := os.Getenv("REDIS_HOST") redisHost := "localhost" if len(redisAddress) > 0 { redisHost = redisAddress } ctx := context.Background() rdb := redis.NewClient(&redis.Options{ Addr: fmt.Sprintf("%s:6379", redisHost), Password: "", DB: 0, }) + defer rdb.Close() + key := "test_max_connection_key" + defer rdb.Del(ctx, key) // Cleanup test key + maxConn := 10 // ... rest of test }Consider adding test cases for:
- Concurrent acquire/release operations
- Context cancellation during acquire
- Redis connection failures
- Edge cases (maxConn = 0, negative values)
api/v2.0/swagger.yaml (1)
7324-7328: Tighten schema + clarify semantics for max_upstream_conn.
- Constrain values to -1 (unlimited) or positive integers via pattern.
- Clarify wording (“connections”, “current”) and add an example.
- Document default behavior (what value is assumed when unset) to avoid client ambiguity.
Apply:
max_upstream_conn: type: string - description: 'The max connection per artifact to the upstream registry in current proxy cache project, if it is -1, no limit to upstream registry connections' + description: 'Maximum connections per artifact to the upstream registry in the current proxy cache project. Use "-1" for unlimited. Please document the default when unset.' x-nullable: true + pattern: '^(-1|[1-9][0-9]*)$' + example: "8"Please also confirm server-side validation mirrors this contract (reject 0/negatives other than -1; define default when missing).
src/portal/src/app/base/project/project-config/project-policy-config/project.ts (1)
39-40: Set a sensible default to match UI semantics (-1 = no limit).Mirror proxy_speed_kb’s default to keep behavior consistent.
constructor() { this.metadata.public = false; this.metadata.enable_content_trust_cosign = false; this.metadata.prevent_vul = false; this.metadata.severity = 'low'; this.metadata.auto_scan = false; this.metadata.auto_sbom_generation = false; this.metadata.proxy_speed_kb = -1; + // -1 means no limit (consistent with i18n tips) + this.metadata.max_upstream_conn = -1; }src/pkg/config/db/cache.go (1)
64-67: Passthrough looks fine; add a short doc/nil-guard for robustness.Document that this bypasses the cache. Optionally check d.driver != nil (mirrors store.GetFromDriver) and return a clear error.
src/portal/src/i18n/lang/en-us-lang.json (1)
275-278: Minor copy polish for clarity.Prefer plural/expanded wording.
- "CONNECTION_LIMIT": "Max connection to upstream registry", + "CONNECTION_LIMIT": "Maximum connections to upstream registry", - "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit", + "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "Maximum concurrent connections to the upstream registry for this proxy cache project. Use -1 for no limit.", - "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. " + "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Enter -1 or an integer greater than 0."src/portal/src/i18n/lang/de-de-lang.json (1)
274-278: Optional: localize or mirror polished English.Either translate to German or keep consistent with the en-us copy tweaks.
- "CONNECTION_LIMIT": "Max connection to upstream registry", + "CONNECTION_LIMIT": "Maximale Verbindungen zur Upstream‑Registry", - "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit", + "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "Maximale gleichzeitige Verbindungen zur Upstream‑Registry für dieses Proxy‑Cache‑Projekt. '-1' bedeutet keine Begrenzung.", - "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. " + "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Bitte '-1' oder eine ganze Zahl größer 0 eingeben."src/pkg/config/rest/rest.go (1)
66-69: Explicitly mark unsupported with context and verify callers handle itReturning ErrUnsupported is fine as a placeholder. Add context to aid debugging, and ensure higher layers treat this as a non-fatal "not implemented" path.
Apply:
+import ( + "context" + "errors" + "fmt" + // ... +) @@ func (h *Driver) Get(_ context.Context, _ string) (map[string]any, error) { - return nil, errors.ErrUnsupported + return nil, fmt.Errorf("rest.Driver.Get unsupported: %w", errors.ErrUnsupported) }Please confirm Config Manager’s GetItemFromDriver path degrades gracefully on errors.ErrUnsupported (e.g., falls back to DB driver or returns a clear 501/feature-gated error).
src/server/v2.0/handler/project.go (1)
824-828: Align validation message and allowed range for max_upstream_conn
- Message says “int” but parse uses 32 bits; prefer “int32” for clarity (like proxy_speed_kb).
- Semantics mismatch: UI tooltip says “-1 or integer > 0”, but tests allow 0. Decide intended meaning (e.g., -1 = unlimited, 0 = disabled/default?) and consistently enforce it here, in project_metadata.validate, and in i18n hints.
Minimal message tweak:
- return errors.BadRequestError(nil).WithMessagef("metadata.max_upstream_conn should be an int, but got '%s', err: %s", *cnt, err) + return errors.BadRequestError(nil).WithMessagef("metadata.max_upstream_conn should be an int32, but got '%s', err: %v", *cnt, err)Please confirm the intended behavior for value 0 and update validation + UI text accordingly.
src/server/v2.0/handler/project_metadata_test.go (1)
24-76: Good coverage; consider boundary and semantics casesSolid table-driven tests. Add cases for large positive (e.g., math.MaxInt32), negative other than -1, and document intended meaning of 0 vs -1 in a brief comment to avoid ambiguity.
src/core/main.go (1)
366-395: Guard against nil map and polish logsIf mgr.GetItemFromDriver ever returns a nil map, writing to it will panic. Also fix minor typos in logs.
Apply:
@@ - cfg, err := mgr.GetItemFromDriver(ctx, common.SkipAuditLogDatabase) + cfg, err := mgr.GetItemFromDriver(ctx, common.SkipAuditLogDatabase) if err != nil { return err } + if cfg == nil { + cfg = make(map[string]any) + } @@ - log.Debugf("key SkipAuditLogDatabase do not exist in the db, will initialize as %v", skipAuditEnv) + log.Debugf("key SkipAuditLogDatabase does not exist in the DB, initializing as %v", skipAuditEnv) @@ - log.Debugf("key SkipAuditLogDatabase aleady exist in the db with value %v", val) + log.Debugf("key SkipAuditLogDatabase already exists in the DB with value %v", val)src/portal/src/i18n/lang/zh-tw-lang.json (1)
272-276: Localize new keys and trim trailing spaceThe three added strings are English in a zh‑TW file and one ends with a trailing space. Please provide Traditional Chinese translations and remove trailing whitespace.
I can supply proposed zh‑TW translations if helpful.
src/lib/config/config.go (1)
45-57: Document new interface method and contractAdd a doc comment for GetItemFromDriver clarifying:
- Returns a map containing only the requested key (if present).
- May return ErrUnsupported from certain drivers; callers should handle gracefully.
type Manager interface { @@ - GetItemFromDriver(ctx context.Context, key string) (map[string]any, error) + // GetItemFromDriver fetches a single config item directly from the underlying driver. + // The returned map contains the requested key if present. Drivers that do not + // support item-level fetch should return an appropriate error (e.g., errors.ErrUnsupported). + GetItemFromDriver(ctx context.Context, key string) (map[string]any, error)Ensure all Manager implementations and mocks embed this new method to avoid interface breakage (run unit tests).
src/portal/src/i18n/lang/fr-fr-lang.json (1)
275-278: Translate new strings to French and trim trailing spacePlease localize the three added keys to French and remove the trailing space at the end of PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP.
Happy to propose French translations if desired.
src/portal/src/i18n/lang/pt-br-lang.json (1)
272-275: Localize new keys and trim trailing whitespace; keep -1 sentinel.These strings are still in English and one ends with a trailing space. Suggest localized PT-BR and consistent guidance.
Apply:
- "CONNECTION_LIMIT": "Max connection to upstream registry", - "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit", - "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. " + "CONNECTION_LIMIT": "Limite de conexões com o registro upstream", + "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "Número máximo de conexões simultâneas com o registro upstream para este projeto de proxy cache; se for -1, não há limite", + "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Informe -1 ou um inteiro maior que 0."src/portal/src/i18n/lang/tr-tr-lang.json (1)
275-279: Translate new keys to Turkish and remove trailing space; keep -1 sentinel.Provide TR equivalents and consistent guidance.
Apply:
- "CONNECTION_LIMIT": "Max connection to upstream registry", - "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit", - "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. " + "CONNECTION_LIMIT": "Yukarı akış kayıt defterine azami bağlantı sayısı", + "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "Bu proxy önbellek projesi için yukarı akış kayıt defterine izin verilen en fazla eşzamanlı bağlantı; değer -1 ise sınır yoktur", + "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Lütfen -1 ya da 0'dan büyük bir tam sayı girin."src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.ts (1)
445-452: Keep form validity in sync with upstream limit errors.
isValidignoresmaxUpstreamConnError, leaving the submit control enabled when that error is set. Fold the new flag into the predicate to keep the UI consistent with the validation.!this.isSubmitOnGoing && this.isNameValid && !this.checkOnGoing && - !this.bandwidthError + !this.bandwidthError && + !this.maxUpstreamConnError );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/go.sumis excluded by!**/*.sum
📒 Files selected for processing (49)
.buildbaselog(2 hunks).github/workflows/CI.yml(3 hunks).github/workflows/pass-CI.yml(2 hunks)CONTRIBUTING.md(1 hunks)Makefile(2 hunks)api/v2.0/swagger.yaml(1 hunks)make/migrations/postgresql/0171_2.14.1_schema.up.sql(1 hunks)src/core/main.go(3 hunks)src/go.mod(6 hunks)src/lib/config/config.go(1 hunks)src/pkg/config/db/cache.go(1 hunks)src/pkg/config/db/dao/dao.go(3 hunks)src/pkg/config/db/db.go(2 hunks)src/pkg/config/inmemory/manager.go(2 hunks)src/pkg/config/manager.go(1 hunks)src/pkg/config/manager_test.go(1 hunks)src/pkg/config/rest/rest.go(1 hunks)src/pkg/config/store/driver.go(1 hunks)src/pkg/config/store/store.go(1 hunks)src/pkg/config/store/store_test.go(1 hunks)src/pkg/project/models/pro_meta.go(1 hunks)src/pkg/project/models/project.go(2 hunks)src/pkg/proxy/connection/limit.go(1 hunks)src/pkg/proxy/connection/limit_test.go(1 hunks)src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.html(1 hunks)src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.ts(5 hunks)src/portal/src/app/base/project/project-config/project-policy-config/project-policy-config.component.html(1 hunks)src/portal/src/app/base/project/project-config/project-policy-config/project-policy-config.component.ts(5 hunks)src/portal/src/app/base/project/project-config/project-policy-config/project.ts(1 hunks)src/portal/src/app/base/project/project.ts(1 hunks)src/portal/src/i18n/lang/de-de-lang.json(1 hunks)src/portal/src/i18n/lang/en-us-lang.json(1 hunks)src/portal/src/i18n/lang/es-es-lang.json(1 hunks)src/portal/src/i18n/lang/fr-fr-lang.json(1 hunks)src/portal/src/i18n/lang/ko-kr-lang.json(1 hunks)src/portal/src/i18n/lang/pt-br-lang.json(1 hunks)src/portal/src/i18n/lang/ru-ru-lang.json(1 hunks)src/portal/src/i18n/lang/tr-tr-lang.json(1 hunks)src/portal/src/i18n/lang/zh-cn-lang.json(1 hunks)src/portal/src/i18n/lang/zh-tw-lang.json(1 hunks)src/server/middleware/repoproxy/proxy.go(5 hunks)src/server/v2.0/handler/project.go(3 hunks)src/server/v2.0/handler/project_metadata.go(1 hunks)src/server/v2.0/handler/project_metadata_test.go(1 hunks)src/server/v2.0/handler/robot.go(1 hunks)src/testing/lib/config/manager.go(1 hunks)tests/ci/distro_installer.sh(1 hunks)tests/resources/TestCaseBody.robot(2 hunks)tests/robot-cases/Group1-Nightly/Replication.robot(0 hunks)
💤 Files with no reviewable changes (1)
- tests/robot-cases/Group1-Nightly/Replication.robot
🧰 Additional context used
🧬 Code graph analysis (16)
src/pkg/config/db/db.go (1)
src/lib/q/query.go (1)
KeyWords(20-20)
src/lib/config/config.go (1)
src/jobservice/job/impl/context.go (1)
Context(42-55)
src/testing/lib/config/manager.go (1)
src/lib/config/config.go (1)
Manager(46-57)
src/pkg/proxy/connection/limit_test.go (1)
src/pkg/proxy/connection/limit.go (1)
Limiter(31-31)
src/pkg/config/rest/rest.go (2)
src/pkg/config/inmemory/manager.go (1)
Driver(34-37)src/pkg/config/store/driver.go (1)
Driver(21-28)
src/pkg/project/models/project.go (1)
src/pkg/project/models/pro_meta.go (1)
ProMetaMaxUpstreamConn(28-28)
src/server/middleware/repoproxy/proxy.go (4)
src/lib/redis/client.go (1)
GetHarborClient(64-89)src/lib/errors/errors.go (1)
NewErrs(130-132)src/lib/log/logger.go (2)
Debugf(303-305)Infof(313-315)src/pkg/proxy/connection/limit.go (1)
Limiter(31-31)
src/server/v2.0/handler/project_metadata_test.go (1)
src/pkg/project/models/pro_meta.go (1)
ProMetaMaxUpstreamConn(28-28)
src/pkg/config/db/dao/dao.go (3)
src/lib/q/query.go (2)
Query(23-34)MustClone(54-71)src/lib/config/models/model.go (1)
ConfigEntry(60-64)src/lib/orm/query.go (1)
QuerySetter(75-109)
src/pkg/config/inmemory/manager.go (1)
src/pkg/config/store/driver.go (1)
Driver(21-28)
src/pkg/config/store/store_test.go (1)
src/pkg/config/store/store.go (1)
ConfigStore(33-36)
src/server/v2.0/handler/robot.go (5)
src/controller/member/controller.go (1)
User(61-64)src/pkg/robot/model/model.go (1)
Robot(31-46)src/controller/robot/model.go (1)
Robot(40-47)src/server/v2.0/handler/model/robot.go (1)
Robot(27-29)src/lib/errors/const.go (1)
DENIED(39-39)
src/server/v2.0/handler/project.go (2)
src/pkg/project/models/project.go (1)
Project(43-57)src/lib/errors/const.go (1)
BadRequestError(75-77)
src/pkg/config/manager_test.go (2)
src/pkg/config/manager.go (1)
CfgManager(35-37)src/pkg/config/store/store.go (1)
NewConfigStore(39-41)
src/server/v2.0/handler/project_metadata.go (2)
src/pkg/project/models/pro_meta.go (1)
ProMetaMaxUpstreamConn(28-28)src/lib/errors/const.go (1)
BadRequestCode(25-25)
src/core/main.go (4)
src/lib/log/logger.go (3)
Errorf(333-335)Warningf(323-325)Debugf(303-305)src/lib/config/config.go (1)
GetCfgManager(116-118)src/common/const.go (1)
SkipAuditLogDatabase(219-219)src/lib/config/userconfig.go (1)
SkipAuditLogDatabase(252-254)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL analysis (go)
- GitHub Check: Analyze (go)
🔇 Additional comments (22)
.buildbaselog (1)
11-22: LGTM!The new changelog entries follow the established format and conventions. The dates are chronologically ordered and all entries properly document base image refresh activities.
src/server/v2.0/handler/robot.go (1)
88-93: Security improvement: Prevents robot privilege escalation.The added validations properly ensure that:
- The security context contains a valid robot user
- The new robot's permissions are equal to or more restrictive than the creator robot's permissions
This prevents a robot from creating another robot with elevated privileges. The permission scope validation using
isValidPermissionScopecorrectly enforces subset semantics.CONTRIBUTING.md (1)
171-171: LGTM!The Go version update for Harbor 2.14 is consistent with the build infrastructure changes in the Makefile.
Makefile (1)
114-115: LGTM!The version updates align with release 2.14.0:
- Trivy: v0.65.0 → v0.66.0
- Trivy Adapter: v0.34.0-rc.1 → v0.34.0 (RC to stable)
- Go build image: 1.24.6 → 1.24.9
All changes are consistent with the CONTRIBUTING.md documentation update.
Also applies to: 154-154
tests/ci/distro_installer.sh (1)
6-7: LGTM!The Go build image version update is consistent across both
package_onlineandpackage_offlinecommands, and aligns with the Makefile changes..github/workflows/CI.yml (1)
18-19: Good documentation with helpful context.The comments include a reference to GitHub's documentation on handling skipped-but-required checks, which is useful context for maintainers.
Also applies to: 32-32
make/migrations/postgresql/0171_2.14.1_schema.up.sql (1)
13-14: Verify the sequence threshold is intentional.The condition checks
last_value > 1, which means the migration will only setskip_audit_log_databasetofalseif there are 2 or more audit log entries.If exactly 1 audit log entry exists, the sequence
last_valuewould be 1, and the condition would not trigger. Is this intentional, or should the check be>= 1to detect any audit log usage?src/pkg/config/db/dao/dao.go (1)
34-35: LGTM!The new
GetConfigItemmethod provides query-based configuration retrieval and follows established patterns:
- Properly clones the query to prevent mutations
- Uses the ORM QuerySetter abstraction consistently
- Includes appropriate error handling
The implementation aligns well with the existing
GetConfigEntriesmethod and integrates cleanly with the broader config infrastructure expansion in this PR.Also applies to: 92-104
src/pkg/config/inmemory/manager.go (1)
58-61: The original review comment's concern is unfounded—no verification needed.The in-memory driver's
Getmethod returningerrors.ErrUnsupporteddoes not affect tests or production code that calls the normalGetflow.Tests invoke
inMemoryManager.Get(ctx, "ldap_url"), which delegates toConfigStore.Get(key). This method reads from the internalcfgValuescache (async.Map) and never callsdriver.Get. Thedriver.Getmethod is only invoked explicitly throughGetFromDriver(), a separate API used for direct driver access. No callers invokeGetFromDriverin the in-memory test suite, and production usage is limited to the db driver's cache layer.Likely an incorrect or invalid review comment.
src/pkg/proxy/connection/limit.go (2)
26-31: LGTM!The global
Limiterinstance is acceptable here sinceConnLimiteris stateless and only provides utility methods for Redis operations.
53-61: LGTM!The error handling is defensive—returning
falseon Redis errors prevents hanging the request and allows the system to continue operating (albeit without connection limiting). The logging provides visibility for operational debugging.src/pkg/config/manager.go (1)
183-186: LGTM!The method is a straightforward delegation to the underlying store layer, consistent with the existing architectural pattern in this manager.
src/pkg/config/store/driver.go (1)
26-27: LGTM!The
Getmethod addition to theDriverinterface is consistent with the existingLoadandSaveoperations and follows the expected pattern for config retrieval.src/go.mod (2)
34-34: Dependency updates verified as safe.All three dependency updates have been verified:
golang-jwt/jwt/v5v5.3.0 actually fixes a HIGH severity vulnerability in versions < 5.2.2 (excessive memory allocation during header parsing)k8s.io/client-gov0.34.1 is well beyond all reported vulnerabilitiessigs.k8s.io/yamlv1.6.0 contains only feature additions (kyaml support, yamlfmt) with no breaking API changesThe updates are safe to merge.
3-3: I need to verify that Go 1.24.9 specifically exists as a valid release.
Go 1.24.9 was released October 13, 2025 with fixes to the crypto/x509 package, confirming it is a valid and legitimate Go release. The version specified in the go.mod file is correct and current.
src/pkg/project/models/pro_meta.go (1)
28-28: LGTM!The new constant
ProMetaMaxUpstreamConnfollows the existing naming pattern and correctly defines the metadata key for the max upstream connection feature.src/portal/src/i18n/lang/ko-kr-lang.json (2)
274-274: LGTM!The trailing comma added to the
NO_PROJECTentry is correct JSON formatting to accommodate the new keys below.
275-277: Review comment is incorrect—issue affects all language files, not just Korean.The verification reveals that ALL language files (en-us, ru-ru, zh-cn, zh-tw, tr-tr, es-es, pt-br, ko-kr, fr-fr, de-de) contain only English text for the new
CONNECTION_LIMIT,PROXY_CACHE_MAX_UPSTREAM_CONN_TIP, andPROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIPkeys. The original review characterizes this as a Korean-specific localization bug, but the incomplete translations are systemic across all supported languages, not a Korean file issue. Either the new keys are pending translation globally, or English is intentionally used as a fallback across all locales.Likely an incorrect or invalid review comment.
src/pkg/config/store/store.go (1)
54-64: LGTM — clean delegate with nil-driver protection.No issues spotted.
src/testing/lib/config/manager.go (1)
79-108: LGTM — generated mock matches the interface and common mockery patterns.No action needed.
src/server/v2.0/handler/project.go (1)
166-170: LGTM: safely ignore proxy-only metadata for non‑proxy projectsClearing proxy-only fields on non‑proxy projects prevents stray values from persisting. This is consistent across create and update paths.
Also applies to: 570-574
src/core/main.go (1)
227-230: LGTM: initialize SkipAuditDB on startup when providedThe hook is placed after config load and overwrite; sequencing looks correct.
|
|
||
| * Jul 15 2021 <[email protected]> | ||
| - Create this file to trigger build base action in buld-package workflow | ||
| - Create this file to trigger build base action in buld-package workflow |
There was a problem hiding this comment.
Fix typo in the modified line.
The line contains a typo: "buld-package" should be "build-package". Since this line is being modified, please correct the typo.
Apply this diff to fix the typo:
-- Create this file to trigger build base action in buld-package workflow
+- Create this file to trigger build base action in build-package workflow 📝 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.
| - Create this file to trigger build base action in buld-package workflow | |
| - Create this file to trigger build base action in build-package workflow |
🤖 Prompt for AI Agents
In .buildbaselog around line 48, the modified line has a typo: "buld-package"
should be "build-package"; update the line to read "Create this file to trigger
build base action in build-package workflow".
| // Get - Get config item from db | ||
| func (d *Database) Get(ctx context.Context, key string) (map[string]any, error) { | ||
| resultMap := map[string]any{} | ||
| configEntries, err := d.cfgDAO.GetConfigItem(ctx, q.New(q.KeyWords{"k": key})) | ||
| if err != nil { | ||
| log.Debugf("get config db error: %v", err) | ||
| return resultMap, err | ||
| } | ||
| // convert to map if there's any record | ||
| for _, item := range configEntries { | ||
| resultMap[item.Key] = item.Value | ||
| } | ||
| return resultMap, nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Query keyword likely wrong; may return no rows or wrong rows and overwrite config
Using q.KeyWords{"k": key} looks incorrect. DAO filters typically use the field name "key". If this filter misses, Get returns empty and callers (e.g., initSkipAuditDBbyEnv) will think the key is absent and write defaults, overriding user settings.
Fix the selector and keep behavior consistent:
- configEntries, err := d.cfgDAO.GetConfigItem(ctx, q.New(q.KeyWords{"k": key}))
+ qry := q.New(q.KeyWords{"key": key})
+ configEntries, err := d.cfgDAO.GetConfigItem(ctx, qry)Optionally add password-type handling (parity with Load) so secrets are transparently decrypted on read:
@@
- for _, item := range configEntries {
- resultMap[item.Key] = item.Value
- }
+ for _, item := range configEntries {
+ if md, ok := metadata.Instance().GetByName(item.Key); ok {
+ if _, isPwd := md.ItemType.(*metadata.PasswordType); isPwd {
+ if dec, err := encrypt.Instance().Decrypt(item.Value); err == nil {
+ resultMap[item.Key] = dec
+ continue
+ } else {
+ log.Errorf("decrypt password failed, key: %s, error %v", item.Key, err)
+ }
+ }
+ }
+ resultMap[item.Key] = item.Value
+ }🏁 Script executed:
#!/bin/bash
# Find DAO expectations for the query keyword used by GetConfigItem
rg -nP -C3 'GetConfigItem\s*\(.*\*q\.Query' src
rg -nP -C2 '\bKeywords\b.*("key"|`key`|\'key\')' src/pkg/config
# Sanity: list all uses of q.KeyWords with "k:" vs "key:"
rg -nP 'q\.KeyWords\s*{\s*"k"\s*:|q\.KeyWords\s*{\s*"key"\s*:' -n src -C1Length of output: 1131
</script>
🏁 Script executed:
#!/bin/bash
# View the GetConfigItem implementation
sed -n '93,110p' src/pkg/config/db/dao/dao.go
# Find ConfigEntry model definition
find src -name "*.go" -exec grep -l "type ConfigEntry struct" {} \;
# Search for other GetConfigItem calls to see what keywords are used
rg -nP 'GetConfigItem\s*\(' src -A2 -B1
# Find orm.QuerySetter usage patterns
rg -nP 'orm\.QuerySetter' src -C3 -g '*.go' | head -50Length of output: 4123
🏁 Script executed:
#!/bin/bash
# Check the Load method mentioned in the review for comparison
rg -nP 'func \(d \*Database\) Load' src -A15 -C1Length of output: 265
🏁 Script executed:
#!/bin/bash
# Find ConfigEntry struct fields
rg -nP 'type ConfigEntry struct' src -A10Length of output: 619
</script>
🏁 Script executed:
#!/bin/bash
# View the full Load method to see if it uses the correct keyword pattern
sed -n '36,88p' src/pkg/config/db/db.go
# Check if there are any other calls to GetConfigItem
rg -nP 'GetConfigItem\s*\(' src --type=goLength of output: 2142
Fix query keyword from "k" to "Key"; using database column name breaks the filter and returns empty results
The issue is confirmed. ConfigEntry has a struct field named Key (not k), and orm.QuerySetter maps keywords to struct field names, not database columns. Using q.KeyWords{"k": key} results in an empty query that matches no records, causing Get() to return an empty map. Callers will incorrectly assume the key is absent and write defaults, overwriting user settings.
- configEntries, err := d.cfgDAO.GetConfigItem(ctx, q.New(q.KeyWords{"k": key}))
+ configEntries, err := d.cfgDAO.GetConfigItem(ctx, q.New(q.KeyWords{"Key": key}))Optional: For consistency with Load(), decrypt password-type values transparently:
- for _, item := range configEntries {
- resultMap[item.Key] = item.Value
- }
+ for _, item := range configEntries {
+ if md, ok := metadata.Instance().GetByName(item.Key); ok {
+ if _, isPwd := md.ItemType.(*metadata.PasswordType); isPwd {
+ if dec, err := encrypt.Instance().Decrypt(item.Value); err == nil {
+ resultMap[item.Key] = dec
+ continue
+ } else {
+ log.Errorf("decrypt password failed, key: %s, error %v", item.Key, err)
+ }
+ }
+ }
+ resultMap[item.Key] = item.Value
+ }📝 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.
| // Get - Get config item from db | |
| func (d *Database) Get(ctx context.Context, key string) (map[string]any, error) { | |
| resultMap := map[string]any{} | |
| configEntries, err := d.cfgDAO.GetConfigItem(ctx, q.New(q.KeyWords{"k": key})) | |
| if err != nil { | |
| log.Debugf("get config db error: %v", err) | |
| return resultMap, err | |
| } | |
| // convert to map if there's any record | |
| for _, item := range configEntries { | |
| resultMap[item.Key] = item.Value | |
| } | |
| return resultMap, nil | |
| } | |
| // Get - Get config item from db | |
| func (d *Database) Get(ctx context.Context, key string) (map[string]any, error) { | |
| resultMap := map[string]any{} | |
| configEntries, err := d.cfgDAO.GetConfigItem(ctx, q.New(q.KeyWords{"Key": key})) | |
| if err != nil { | |
| log.Debugf("get config db error: %v", err) | |
| return resultMap, err | |
| } | |
| // convert to map if there's any record | |
| for _, item := range configEntries { | |
| resultMap[item.Key] = item.Value | |
| } | |
| return resultMap, nil | |
| } |
🤖 Prompt for AI Agents
In src/pkg/config/db/db.go around lines 89 to 102, the query keyword is
incorrectly using "k" causing the ORM filter to match nothing; change the query
to use the struct field name key (e.g. q.KeyWords{"Key": key}) so the DAO
filters by the ConfigEntry.Key field and returns results; after fixing the
keyword, keep the existing loop that populates resultMap from configEntries and
(optionally) decrypt any values whose type indicates password before assigning
them to resultMap; add/update unit tests to assert that Get returns stored
entries for existing keys.
| // MaxUpstreamConnection ... | ||
| func (p *Project) MaxUpstreamConnection() int { | ||
| countVal, exist := p.GetMetadata(ProMetaMaxUpstreamConn) | ||
| if !exist { | ||
| return 0 | ||
| } | ||
| cnt, err := strconv.ParseInt(countVal, 10, 32) | ||
| if err != nil { | ||
| log.Warningf("failed th parse the max_upstream_conn, val:%s error %v", countVal, err) | ||
| return 0 | ||
| } | ||
| return int(cnt) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Fix log message typo; verify default semantics; consider type consistency.
- Typo in warning: “failed th parse”.
- Please confirm that a missing value (0) is treated as “unset/no limit” (UI says -1 is “no limit”) and won’t block all upstream connections.
- Optional: align return type with ProxyCacheSpeed (int32) for consistency.
- log.Warningf("failed th parse the max_upstream_conn, val:%s error %v", countVal, err)
+ log.Warningf("failed to parse max_upstream_conn: value=%s, error=%v", countVal, err)
---
🏁 Script executed:
```shell
#!/bin/bash
# Verify the typo in line 180 and find ProxyCacheSpeed method
cd /tmp/repo && cat src/pkg/project/models/project.go | sed -n '173,185p'
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Search for ProxyCacheSpeed method to check return type
rg -nA5 'func.*ProxyCacheSpeed' src/pkg/project/models/Length of output: 398
🏁 Script executed:
#!/bin/bash
# Search for MaxUpstreamConnection usage to understand semantics
rg -nB2 -A2 'MaxUpstreamConnection' src/ --type=goLength of output: 2097
🏁 Script executed:
#!/bin/bash
# Search for ProMetaMaxUpstreamConn to see how it's set and what values are expected
rg -nB2 -A2 'ProMetaMaxUpstreamConn' src/ --type=goLength of output: 3251
🏁 Script executed:
#!/bin/bash
# Search for "no limit" or -1 patterns in metadata handling to understand default semantics
rg -nB3 -A3 'no.limit|unlimited|-1' src/pkg/project/models/ --type=go -iLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check for other similar metadata getter methods to see return type patterns
rg -nA8 'func.*\*Project\).*Metadata' src/pkg/project/models/project.go --type=go | head -40Length of output: 430
---
🏁 Script executed:
```shell
#!/bin/bash
# Extract the exact MaxUpstreamConnection method to verify the typo
rg -nA12 'func \(p \*Project\) MaxUpstreamConnection' src/pkg/project/models/project.go
Length of output: 371
🏁 Script executed:
#!/bin/bash
# Search for the exact warning message to confirm typo exists in codebase
rg -n 'failed th parse' src/ --type=goLength of output: 125
Fix log message typo; consider type consistency with ProxyCacheSpeed.
The typo at line 181 is confirmed: "failed th parse" should be "failed to parse". The default behavior returning 0 for missing metadata is correct—usage checks > 0 before enforcing the limit, so 0 safely means "no limit applied."
The type inconsistency with ProxyCacheSpeed (which returns int32) is worth addressing for consistency across similar metadata getter methods.
- log.Warningf("failed th parse the max_upstream_conn, val:%s error %v", countVal, err)
+ log.Warningf("failed to parse max_upstream_conn: value=%s, error=%v", countVal, err)Optional: align return type from int to int32 to match ProxyCacheSpeed().
🤖 Prompt for AI Agents
In src/pkg/project/models/project.go around lines 173 to 185, fix the log
message typo and optionally align the return type with ProxyCacheSpeed: change
the log text from "failed th parse the max_upstream_conn" to "failed to parse
the max_upstream_conn"; if you want type consistency, change the method
signature to return int32, update strconv.ParseInt to parse with 32 bit
(strconv.ParseInt(countVal, 10, 32)) and return int32(cnt) (or cast as needed),
and update any callers expecting int to use int32 or cast at call sites.
| <div | ||
| class="clr-form-control mt-05" | ||
| [style.visibility]=" | ||
| isSystemAdmin && enableProxyCache ? 'visible' : 'hidden' | ||
| "> | ||
| <label for="maxUpStreamConn" class="clr-control-label required"> | ||
| {{ 'PROJECT.CONNECTION_LIMIT' | translate }} | ||
| <clr-tooltip> | ||
| <clr-icon | ||
| clrTooltipTrigger | ||
| shape="info-circle" | ||
| size="24"></clr-icon> | ||
| <clr-tooltip-content | ||
| clrPosition="bottom-left" | ||
| clrSize="lg" | ||
| *clrIfOpen> | ||
| <span>{{ | ||
| 'PROJECT.PROXY_CACHE_MAX_UPSTREAM_CONN_TIP' | ||
| | translate | ||
| }}</span> | ||
| </clr-tooltip-content> | ||
| </clr-tooltip> | ||
| </label> | ||
| <div | ||
| class="clr-control-container" | ||
| [class.clr-error]="maxUpstreamConnError"> | ||
| <input | ||
| type="number" | ||
| id="maxUpStreamConn" | ||
| [(ngModel)]="project.metadata.max_upstream_conn" | ||
| name="maxUpStreamConn" | ||
| class="clr-input width-164 mr-10" | ||
| placeholder="Enter upstream connection limit" | ||
| autocomplete="off" | ||
| (ngModelChange)="validateMaxUpstreamConnections()" /> | ||
| <clr-icon | ||
| *ngIf="maxUpstreamConnError" | ||
| class="clr-validate-icon" | ||
| shape="exclamation-circle"></clr-icon> | ||
| <clr-control-error | ||
| *ngIf="maxUpstreamConnError" | ||
| class="tooltip-content"> | ||
| {{ | ||
| 'PROJECT.PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP' | ||
| | translate | ||
| }} | ||
| </clr-control-error> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify duplication and fix naming inconsistency.
The AI summary mentions "two identical additions in the diff, resulting in duplicated input blocks gated by the same condition," but only one block is visible in the provided code. Please verify whether a duplicate block exists elsewhere in the file and remove it if present.
Additionally, there's a naming inconsistency: the element uses maxUpStreamConn (capital S in "Stream") while the model property is max_upstream_conn (lowercase, underscore-separated). For consistency with typical camelCase conventions, consider using maxUpstreamConn throughout.
🏁 Script executed:
#!/bin/bash
# Search for duplicate maxUpStreamConn or max_upstream_conn blocks in the template
rg -n -C5 'maxUpStreamConn|max_upstream_conn' --type=htmlLength of output: 5530
---
🏁 Script executed:
```shell
#!/bin/bash
# Check placeholder patterns in create-project.component.html
rg -n 'placeholder=' --type=html src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.html
Length of output: 199
🏁 Script executed:
#!/bin/bash
# Check for other conditional visibility blocks using proxy cache pattern
rg -n 'enableProxyCache' --type=html src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.html | head -20Length of output: 632
🏁 Script executed:
#!/bin/bash
# Search for form input naming conventions (id= patterns) to understand camelCase usage
rg -n 'id="[a-zA-Z]+' --type=html src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.html | head -15Length of output: 647
---
🏁 Script executed:
```shell
#!/bin/bash
# Check git history or context around line 258 (bandwidth placeholder)
# to confirm if this pattern pre-existed or is also a new addition
rg -n -B10 -A2 'placeholder="Enter bandwidth"' --type=html src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.html
Length of output: 617
Fix naming inconsistency to match project conventions.
The duplication claim is incorrect—only one maxUpStreamConn block exists in the file. However, the naming does need correction: maxUpStreamConn (capital S) violates the project's naming conventions (kebab-case like create-project-name and underscores like bandwidth_unit). Use max-upstream-conn or max_upstream_conn consistently.
Additionally, the placeholder "Enter upstream connection limit" is hard-coded without the translate pipe. While the bandwidth input on line 258 has a similar pattern, placeholders should be internationalized to match other labels in the form (e.g., "CONNECTION_LIMIT" translation key).
| [(ngModel)]="project.metadata.max_upstream_conn" | ||
| name="maxUpStreamConn" | ||
| class="clr-input width-164 mr-10" | ||
| placeholder="Enter upstream connection limit" |
There was a problem hiding this comment.
Internationalize the placeholder text.
The placeholder text "Enter upstream connection limit" is hardcoded and not internationalized, unlike other form fields in this template that use the translate pipe.
Apply this diff to internationalize the placeholder:
- placeholder="Enter upstream connection limit"
+ [placeholder]="'PROJECT.UPSTREAM_CONN_PLACEHOLDER' | translate"Then add the corresponding translation key to all language files.
📝 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.
| placeholder="Enter upstream connection limit" | |
| [placeholder]="'PROJECT.UPSTREAM_CONN_PLACEHOLDER' | translate" |
🤖 Prompt for AI Agents
In
src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.html
around line 316, the placeholder "Enter upstream connection limit" is hardcoded;
replace it with the translate pipe (e.g. placeholder="{{
'projects.create.upstreamConnectionLimitPlaceholder' | translate }}" or
equivalent binding used in this template) so it is internationalized, then add
the key projects.create.upstreamConnectionLimitPlaceholder with appropriate
translated strings to all language JSON files.
| max_upstream_conn: number; | ||
| }; | ||
| constructor() { | ||
| this.metadata = <any>{}; | ||
| this.metadata.public = false; | ||
| this.metadata.max_upstream_conn = -1; |
There was a problem hiding this comment.
🧩 Analysis chain
Clarify the semantics of the default value -1.
The max_upstream_conn field is initialized to -1, which typically indicates "unlimited" or "not set" in configuration contexts. Ensure this default value is:
- Documented in user-facing tooltips/help text
- Validated consistently on the backend (in
src/server/v2.0/handler/project_metadata.go) - Handled appropriately in the UI (negative values, zero, etc.)
Verify that the backend validation and UI validation are aligned on the semantics of -1:
🏁 Script executed:
#!/bin/bash
# Check validation logic for max_upstream_conn in backend
rg -n "max_upstream_conn|ProMetaMaxUpstreamConn" --type go -A 5 -B 2Length of output: 5891
🏁 Script executed:
#!/bin/bash
# Check frontend code for max_upstream_conn handling, validation, and documentation
rg -n "max_upstream_conn|maxUpstreamConn|MAX_UPSTREAM" --type ts --type tsx --type js -B 3 -A 3Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check for any help text, tooltips, or documentation related to upstream connection
rg -n "upstream|Upstream" src/portal/src/app/base/project/ -B 2 -A 2 --type ts --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Look for validation rules or constants defining valid ranges
rg -n "upstream.*-1|unlimited|default.*value" src/portal/ -i -B 2 -A 2 --type ts --type tsx --type jsonLength of output: 27
🏁 Script executed:
#!/bin/bash
# Check the specific project.ts file structure for metadata defaults
cat src/portal/src/app/base/project/project.ts | head -100Length of output: 1486
🏁 Script executed:
#!/bin/bash
# Look for any API models or interfaces that document max_upstream_conn
rg -n "max_upstream_conn" src/portal/ -B 5 -A 5 --type ts --type tsx --type jsonLength of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for max_upstream_conn in portal without type restrictions
rg -n "max_upstream_conn" src/portal/ -B 3 -A 3
Length of output: 8206
🏁 Script executed:
#!/bin/bash
# Search for upstream in portal without type restrictions
rg -n "upstream" src/portal/ -i -B 2 -A 2Length of output: 33757
🏁 Script executed:
#!/bin/bash
# Look for form validation or input constraints
rg -n "validator|validate|pattern|min|max" src/portal/src/app/base/project/ -i -B 1 -A 1Length of output: 132186
🏁 Script executed:
#!/bin/bash
# Check for swagger/openapi specifications documenting the API
find . -name "*.yaml" -o -name "*.yml" -o -name "*.json" | grep -i "swagger\|openapi\|spec" | head -20Length of output: 159
🏁 Script executed:
#!/bin/bash
# Search entire codebase for any documentation or comments about -1 semantics
rg -n "\-1.*unlimited|\-1.*default|unlimited.*\-1" -i -B 2 -A 2Length of output: 14259
Address validation mismatch between frontend and backend for max_upstream_conn.
The semantics of -1 (unlimited) are documented in multilingual tooltips (en-us, de-de, zh-cn, etc.), and the frontend correctly validates this with explicit special-case handling. However, the backend validation (project.go:824-828) accepts any valid int32 without enforcing semantic constraints. Specifically:
- Frontend rejects
0(requires-1or>0), but backend accepts0viastrconv.ParseInt - Backend lacks validation logic to enforce that only
-1or positive integers are valid - Backend ignores these fields for non-proxy-cache projects but doesn't validate the project type
Ensure backend validation (src/server/v2.0/handler/project.go) mirrors frontend constraints to prevent API bypass.
🤖 Prompt for AI Agents
In src/server/v2.0/handler/project.go around lines 824 to 828, the backend
currently parses max_upstream_conn as any int32 and thus accepts 0 while
frontend requires -1 (unlimited) or a positive integer and only for proxy-cache
projects; update the handler to (1) enforce that the field is only
accepted/processed when project.Type == "proxy-cache" (return a validation error
if the caller attempts to set it for other project types), (2) parse the value
and validate it is either -1 or >0 (reject 0 and negative values other than -1),
and (3) return a clear bad-request error when validation fails so backend
semantics mirror the frontend validation.
| "NO_PROJECT": "No pudimos encontrar ningún proyecto", | ||
| "CONNECTION_LIMIT": "Max connection to upstream registry", | ||
| "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit", | ||
| "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. " |
There was a problem hiding this comment.
Provide Spanish translations for new upstream-connection strings
These new keys surface in the es-ES locale but are left in English. Please translate them (e.g., «Máximo de conexiones al registro ascendente», etc.) so Spanish users see localized copy. citeturn0file4
🤖 Prompt for AI Agents
In src/portal/src/i18n/lang/es-es-lang.json around lines 275 to 278, three keys
are still in English; replace them with Spanish translations: set
"CONNECTION_LIMIT" to "Máximo de conexiones al registro ascendente",
"PROXY_CACHE_MAX_UPSTREAM_CONN_TIP" to "El número máximo de conexiones al
registro ascendente para este proyecto de proxy cache; si es -1, no hay límite",
and "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP" to "Por favor, introduzca -1 o un
número entero mayor que 0."
| "NO_PROJECT": "Мы не смогли найти никаких проектов", | ||
| "CONNECTION_LIMIT": "Max connection to upstream registry", | ||
| "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit", | ||
| "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. " |
There was a problem hiding this comment.
Localize new Russian strings for upstream connection limit
These entries are exposed in the Russian locale but remain in English. Please translate them so RU users see localized UI text (e.g., «Максимальное количество подключений к вышестоящему реестру», etc.). citeturn0file0
🤖 Prompt for AI Agents
In src/portal/src/i18n/lang/ru-ru-lang.json around lines 259 to 262 the three
keys ("CONNECTION_LIMIT", "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP",
"PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP") are still in English; replace them
with proper Russian translations so the UI is localized (suggested translations:
"CONNECTION_LIMIT": "Максимальное количество подключений к вышестоящему
реестру", "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "Максимальное количество
подключений к вышестоящему реестру для этого прокси‑кеша; если указано -1,
ограничения нет", "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Введите -1 или
целое число больше 0"). Ensure punctuation and spacing match existing JSON
style.
| "CONNECTION_LIMIT": "Max connection to upstream registry", | ||
| "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if less than 0, then there is no limit", | ||
| "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "请输入-1或者大于0的整数" |
There was a problem hiding this comment.
Fix sentinel description (-1 only) and localize strings.
The tip says “小于0则不限” but input tip allows only “-1 or >0”; also two strings remain in English. Align to -1 and localize.
Apply:
- "CONNECTION_LIMIT": "Max connection to upstream registry",
- "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if less than 0, then there is no limit",
- "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "请输入-1或者大于0的整数"
+ "CONNECTION_LIMIT": "上游仓库最大连接数",
+ "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "该代理缓存项目连接上游仓库的最大并发数，若为 -1，则不限制",
+ "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "请输入 -1 或者大于 0 的整数"📝 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.
| "CONNECTION_LIMIT": "Max connection to upstream registry", | |
| "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if less than 0, then there is no limit", | |
| "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "请输入-1或者大于0的整数" | |
| "CONNECTION_LIMIT": "上游仓库最大连接数", | |
| "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "该代理缓存项目连接上游仓库的最大并发数，若为 -1，则不限制", | |
| "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "请输入 -1 或者大于 0 的整数" |
🤖 Prompt for AI Agents
In src/portal/src/i18n/lang/zh-cn-lang.json around lines 274-276, the
description and localization are inconsistent: the tip text says “小于0则不限” but
the input tip only allows “-1 or >0”, and two entries are still in English.
Update the three values to Chinese and align semantics to use -1 as the sentinel
for unlimited. For example, change CONNECTION_LIMIT to a Chinese label like
"上游注册表最大连接数", change PROXY_CACHE_MAX_UPSTREAM_CONN_TIP to
"此代理缓存项目连接上游注册表的最大连接数；若为 -1 则表示不限", and change
PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP to "请输入 -1 或大于 0 的整数". Ensure
spacing/punctuation matches project i18n style.
| case proModels.ProMetaMaxUpstreamConn: | ||
| v, err := strconv.ParseInt(value, 10, 32) | ||
| if err != nil { | ||
| return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessagef("invalid value: %s", value) | ||
| } | ||
| metas[proModels.ProMetaMaxUpstreamConn] = strconv.FormatInt(v, 10) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add range validation for max_upstream_conn.
The current validation only checks if the value can be parsed as an integer but doesn't validate the acceptable range. Based on the TypeScript default of -1 (presumably meaning "unlimited"), the backend should enforce valid ranges.
Add range validation to ensure the value is semantically valid:
case proModels.ProMetaMaxUpstreamConn:
v, err := strconv.ParseInt(value, 10, 32)
if err != nil {
return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessagef("invalid value: %s", value)
}
+ // -1 means unlimited, 0 means disabled, positive means limit
+ if v < -1 {
+ return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessagef("max_upstream_conn must be >= -1, got: %d", v)
+ }
+ if v > 10000 {
+ return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessagef("max_upstream_conn exceeds reasonable limit (10000), got: %d", v)
+ }
metas[proModels.ProMetaMaxUpstreamConn] = strconv.FormatInt(v, 10)This ensures the value is within a reasonable range and prevents potential resource exhaustion or misconfigurations.
📝 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.
| case proModels.ProMetaMaxUpstreamConn: | |
| v, err := strconv.ParseInt(value, 10, 32) | |
| if err != nil { | |
| return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessagef("invalid value: %s", value) | |
| } | |
| metas[proModels.ProMetaMaxUpstreamConn] = strconv.FormatInt(v, 10) | |
| case proModels.ProMetaMaxUpstreamConn: | |
| v, err := strconv.ParseInt(value, 10, 32) | |
| if err != nil { | |
| return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessagef("invalid value: %s", value) | |
| } | |
| // -1 means unlimited, 0 means disabled, positive means limit | |
| if v < -1 { | |
| return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessagef("max_upstream_conn must be >= -1, got: %d", v) | |
| } | |
| if v > 10000 { | |
| return nil, errors.New(nil).WithCode(errors.BadRequestCode).WithMessagef("max_upstream_conn exceeds reasonable limit (10000), got: %d", v) | |
| } | |
| metas[proModels.ProMetaMaxUpstreamConn] = strconv.FormatInt(v, 10) |
🤖 Prompt for AI Agents
In src/server/v2.0/handler/project_metadata.go around lines 164 to 169, the code
parses max_upstream_conn but does not validate allowed ranges; update validation
to accept -1 (meaning unlimited) or a non-negative integer within a sane upper
bound (for example 0..10000), returning a BadRequest error with a clear message
if the value is out of range; keep parsing with strconv.ParseInt, then check if
v == -1 || (v >= 0 && v <= 10000) before assigning
metas[proModels.ProMetaMaxUpstreamConn] = strconv.FormatInt(v, 10), otherwise
return an errors.New(nil).WithCode(errors.BadRequestCode).WithMessagef("invalid
max_upstream_conn: %d", v).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
19 issues found across 50 files
Prompt for AI agents (all 19 issues)
Understand the root cause of the following 19 issues and fix them.
<file name="src/portal/src/i18n/lang/es-es-lang.json">
<violation number="1" location="src/portal/src/i18n/lang/es-es-lang.json:276">
This Spanish-locale string remains in English; please translate it so the es-ES locale stays consistent.</violation>
<violation number="2" location="src/portal/src/i18n/lang/es-es-lang.json:278">
This validation hint for the Spanish locale is left in English; translating it keeps the locale consistent.</violation>
</file>
<file name="src/server/middleware/repoproxy/proxy.go">
<violation number="1" location="src/server/middleware/repoproxy/proxy.go:114">
If connection.Limiter.Acquire fails because redis is unavailable, this line treats it as a rate-limit hit and returns HTTP 429, blocking all pulls even though the limit was never exceeded. Please surface redis errors differently so requests don’t get rejected on backend failure.</violation>
</file>
<file name="make/migrations/postgresql/0171_2.14.1_schema.up.sql">
<violation number="1" location="make/migrations/postgresql/0171_2.14.1_schema.up.sql:13">
The sequence check misses the case where exactly one audit-log row exists: `last_value` stays 1 after the first insert, so this condition never records the existing usage and the safeguard config is skipped.</violation>
</file>
<file name="src/portal/src/i18n/lang/ru-ru-lang.json">
<violation number="1" location="src/portal/src/i18n/lang/ru-ru-lang.json:260">
These newly added ru-RU locale strings are still in English, so Russian-language users will see untranslated UI text. Please provide Russian translations for these entries to keep the locale consistent.</violation>
</file>
<file name="src/portal/src/i18n/lang/fr-fr-lang.json">
<violation number="1" location="src/portal/src/i18n/lang/fr-fr-lang.json:275">
Le libellé reste en anglais dans la locale française, ce qui rompt la cohérence de la traduction. Merci de fournir une traduction française, par exemple "Nombre maximal de connexions vers le registre amont".</violation>
<violation number="2" location="src/portal/src/i18n/lang/fr-fr-lang.json:276">
Cette info-bulle est encore en anglais dans le fichier fr-FR. Veuillez la traduire pour les utilisateurs francophones.</violation>
</file>
<file name="src/portal/src/i18n/lang/de-de-lang.json">
<violation number="1" location="src/portal/src/i18n/lang/de-de-lang.json:275">
The new entries for the German locale are still English. Please provide German translations so localized users do not see untranslated strings.</violation>
</file>
<file name="src/portal/src/i18n/lang/ko-kr-lang.json">
<violation number="1" location="src/portal/src/i18n/lang/ko-kr-lang.json:275">
이번에 추가된 안내 문구가 모두 영어로 남아 있어 ko-KR 로케일 선택 시 UI가 혼합 언어로 표시됩니다. 동일 위치의 기존 항목들은 한국어 번역을 사용하고 있으니, 새 키들도 한국어로 번역해 주세요.</violation>
</file>
<file name="src/portal/src/i18n/lang/zh-cn-lang.json">
<violation number="1" location="src/portal/src/i18n/lang/zh-cn-lang.json:275">
This tooltip says "if less than 0" even though the allowed values are -1 or integers greater than 0, contradicting the nearby input hint and other locales. Please update the tooltip to say "if -1, then there is no limit" so users aren’t misled.</violation>
</file>
<file name="src/portal/src/i18n/lang/en-us-lang.json">
<violation number="1" location="src/portal/src/i18n/lang/en-us-lang.json:275">
The label uses the singular "connection" even though it refers to a count of connections, which reads as a grammatical mistake in the UI. Please update it to the plural form for clarity.</violation>
</file>
<file name="src/pkg/config/db/db.go">
<violation number="1" location="src/pkg/config/db/db.go:92">
Use the struct field name in the query filter; using "k" prevents matches and can cause defaults to overwrite existing configuration</violation>
</file>
<file name="src/portal/src/i18n/lang/pt-br-lang.json">
<violation number="1" location="src/portal/src/i18n/lang/pt-br-lang.json:273">
This pt-BR locale string is still in English, so the Portuguese UI will show untranslated text. Please localize it for Brazilian Portuguese users.</violation>
<violation number="2" location="src/portal/src/i18n/lang/pt-br-lang.json:274">
This tooltip string in the pt-BR locale remains in English, so Portuguese users will see untranslated UI text. Please provide the Brazilian Portuguese translation.</violation>
</file>
<file name="src/portal/src/i18n/lang/zh-tw-lang.json">
<violation number="1" location="src/portal/src/i18n/lang/zh-tw-lang.json:273">
這些新增的 zh-tw 語系字串仍為英文，會讓繁體中文介面出現未翻譯內容。建議提供對應的繁體中文翻譯以維持在地化品質。</violation>
</file>
<file name="src/portal/src/app/base/project/project-config/project-policy-config/project-policy-config.component.html">
<violation number="1" location="src/portal/src/app/base/project/project-config/project-policy-config/project-policy-config.component.html:107">
This input is always disabled because the disabled attribute is set as a literal string. Use property binding so the field enables when allowUpdateProxyCacheConfiguration is true.</violation>
</file>
<file name="src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.ts">
<violation number="1" location="src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.ts:369">
After validating max_upstream_conn you still guard on bandwidthError, so invalid upstream settings slip through and the error message never appears. Please gate on maxUpstreamConnError instead.</violation>
</file>
<file name="src/pkg/config/manager_test.go">
<violation number="1" location="src/pkg/config/manager_test.go:123">
TestGetItemFromDriverNilContext asserts that passing a nil context should succeed, but GetItemFromDriver simply forwards the context to the store driver, which requires a non-nil context and will panic when it reaches database helpers. Update the test to expect an error (or remove nil usage) and adjust the production code if nil contexts need to be supported.</violation>
<violation number="2" location="src/pkg/config/manager_test.go:138">
TestGetItemFromDriverEmptyKey assumes that an empty key should succeed and return an empty map. The production driver treats an empty key as invalid and returns an error from the database layer, so this test will always fail when run against the real implementation. Adjust the test to expect an error (or validate the input before calling GetItemFromDriver).</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| "NO_PROJECT": "No pudimos encontrar ningún proyecto", | ||
| "CONNECTION_LIMIT": "Max connection to upstream registry", | ||
| "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit", | ||
| "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. " |
There was a problem hiding this comment.
This validation hint for the Spanish locale is left in English; translating it keeps the locale consistent.
Prompt for AI agents
Address the following comment on src/portal/src/i18n/lang/es-es-lang.json at line 278:
<comment>This validation hint for the Spanish locale is left in English; translating it keeps the locale consistent.</comment>
<file context>
@@ -272,7 +272,10 @@
+ "NO_PROJECT": "No pudimos encontrar ningún proyecto",
+ "CONNECTION_LIMIT": "Max connection to upstream registry",
+ "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit",
+ "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. "
},
"PROJECT_DETAIL": {
</file context>
| "PROXY_CACHE_ENDPOINT": "Proxy Cache Endpoint", | ||
| "NO_PROJECT": "No pudimos encontrar ningún proyecto" | ||
| "NO_PROJECT": "No pudimos encontrar ningún proyecto", | ||
| "CONNECTION_LIMIT": "Max connection to upstream registry", |
There was a problem hiding this comment.
This Spanish-locale string remains in English; please translate it so the es-ES locale stays consistent.
Prompt for AI agents
Address the following comment on src/portal/src/i18n/lang/es-es-lang.json at line 276:
<comment>This Spanish-locale string remains in English; please translate it so the es-ES locale stays consistent.</comment>
<file context>
@@ -272,7 +272,10 @@
"PROXY_CACHE_ENDPOINT": "Proxy Cache Endpoint",
- "NO_PROJECT": "No pudimos encontrar ningún proyecto"
+ "NO_PROJECT": "No pudimos encontrar ningún proyecto",
+ "CONNECTION_LIMIT": "Max connection to upstream registry",
+ "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit",
+ "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. "
</file context>
| } | ||
| key := upstreamRegistryConnectionKey(art) | ||
| log.Debugf("handle blob, upstream registry connection limit key: %s", key) | ||
| if !connection.Limiter.Acquire(ctx, client, key, p.MaxUpstreamConnection()) { |
There was a problem hiding this comment.
If connection.Limiter.Acquire fails because redis is unavailable, this line treats it as a rate-limit hit and returns HTTP 429, blocking all pulls even though the limit was never exceeded. Please surface redis errors differently so requests don’t get rejected on backend failure.
Prompt for AI agents
Address the following comment on src/server/middleware/repoproxy/proxy.go at line 114:
<comment>If connection.Limiter.Acquire fails because redis is unavailable, this line treats it as a rate-limit hit and returns HTTP 429, blocking all pulls even though the limit was never exceeded. Please surface redis errors differently so requests don’t get rejected on backend failure.</comment>
<file context>
@@ -99,6 +103,22 @@ func handleBlob(w http.ResponseWriter, r *http.Request, next http.Handler) error
+ }
+ key := upstreamRegistryConnectionKey(art)
+ log.Debugf("handle blob, upstream registry connection limit key: %s", key)
+ if !connection.Limiter.Acquire(ctx, client, key, p.MaxUpstreamConnection()) {
+ log.Infof("current connection exceed max connections to upstream registry")
+ // send http code 429 to client
</file context>
| RETURN; | ||
| END IF; | ||
|
|
||
| IF (SELECT last_value FROM audit_log_id_seq) > 1 |
There was a problem hiding this comment.
The sequence check misses the case where exactly one audit-log row exists: last_value stays 1 after the first insert, so this condition never records the existing usage and the safeguard config is skipped.
Prompt for AI agents
Address the following comment on make/migrations/postgresql/0171_2.14.1_schema.up.sql at line 13:
<comment>The sequence check misses the case where exactly one audit-log row exists: `last_value` stays 1 after the first insert, so this condition never records the existing usage and the safeguard config is skipped.</comment>
<file context>
@@ -0,0 +1,17 @@
+ RETURN;
+ END IF;
+
+ IF (SELECT last_value FROM audit_log_id_seq) > 1
+ OR (SELECT last_value FROM audit_log_ext_id_seq) > 1 THEN
+ INSERT INTO properties (k, v) VALUES ('skip_audit_log_database', 'false');
</file context>
| "PROXY_CACHE_ENDPOINT": "Конечная точка прокси-кэша", | ||
| "NO_PROJECT": "Мы не смогли найти никаких проектов" | ||
| "NO_PROJECT": "Мы не смогли найти никаких проектов", | ||
| "CONNECTION_LIMIT": "Max connection to upstream registry", |
There was a problem hiding this comment.
These newly added ru-RU locale strings are still in English, so Russian-language users will see untranslated UI text. Please provide Russian translations for these entries to keep the locale consistent.
Prompt for AI agents
Address the following comment on src/portal/src/i18n/lang/ru-ru-lang.json at line 260:
<comment>These newly added ru-RU locale strings are still in English, so Russian-language users will see untranslated UI text. Please provide Russian translations for these entries to keep the locale consistent.</comment>
<file context>
@@ -256,7 +256,10 @@
"PROXY_CACHE_ENDPOINT": "Конечная точка прокси-кэша",
- "NO_PROJECT": "Мы не смогли найти никаких проектов"
+ "NO_PROJECT": "Мы не смогли найти никаких проектов",
+ "CONNECTION_LIMIT": "Max connection to upstream registry",
+ "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit",
+ "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. "
</file context>
| "PROXY_CACHE_ENDPOINT": "代理快取端點", | ||
| "NO_PROJECT": "找不到任何專案" | ||
| "NO_PROJECT": "找不到任何專案", | ||
| "CONNECTION_LIMIT": "Max connection to upstream registry", |
There was a problem hiding this comment.
這些新增的 zh-tw 語系字串仍為英文，會讓繁體中文介面出現未翻譯內容。建議提供對應的繁體中文翻譯以維持在地化品質。
Prompt for AI agents
Address the following comment on src/portal/src/i18n/lang/zh-tw-lang.json at line 273:
<comment>這些新增的 zh-tw 語系字串仍為英文，會讓繁體中文介面出現未翻譯內容。建議提供對應的繁體中文翻譯以維持在地化品質。</comment>
<file context>
@@ -269,7 +269,10 @@
"PROXY_CACHE_ENDPOINT": "代理快取端點",
- "NO_PROJECT": "找不到任何專案"
+ "NO_PROJECT": "找不到任何專案",
+ "CONNECTION_LIMIT": "Max connection to upstream registry",
+ "PROXY_CACHE_MAX_UPSTREAM_CONN_TIP": "The max connection to the upstream registry for this proxy cache project, if -1, then there is no limit",
+ "PROXY_CACHE_MAX_UPSTREAM_CONN_INPUT_TIP": "Please enter -1 or an integer greater than 0. "
</file context>
| id="max_upstream_conn" | ||
| [(ngModel)]="projectPolicy.MaxUpstreamConn" | ||
| name="max_upstream_conn" | ||
| disabled="!allowUpdateProxyCacheConfiguration" |
There was a problem hiding this comment.
This input is always disabled because the disabled attribute is set as a literal string. Use property binding so the field enables when allowUpdateProxyCacheConfiguration is true.
Prompt for AI agents
Address the following comment on src/portal/src/app/base/project/project-config/project-policy-config/project-policy-config.component.html at line 107:
<comment>This input is always disabled because the disabled attribute is set as a literal string. Use property binding so the field enables when allowUpdateProxyCacheConfiguration is true.</comment>
<file context>
@@ -92,6 +92,35 @@
+ id="max_upstream_conn"
+ [(ngModel)]="projectPolicy.MaxUpstreamConn"
+ name="max_upstream_conn"
+ disabled="!allowUpdateProxyCacheConfiguration"
+ class="clr-input width-164 mr-10 clr-input-underline"
+ autocomplete="off" />
</file context>
| disabled="!allowUpdateProxyCacheConfiguration" | |
| [disabled]="!allowUpdateProxyCacheConfiguration" |
| } | ||
|
|
||
| this.validateMaxUpstreamConnections(); | ||
| if (this.bandwidthError) { |
There was a problem hiding this comment.
After validating max_upstream_conn you still guard on bandwidthError, so invalid upstream settings slip through and the error message never appears. Please gate on maxUpstreamConnError instead.
Prompt for AI agents
Address the following comment on src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.ts at line 369:
<comment>After validating max_upstream_conn you still guard on bandwidthError, so invalid upstream settings slip through and the error message never appears. Please gate on maxUpstreamConnError instead.</comment>
<file context>
@@ -342,6 +365,12 @@ export class CreateProjectComponent
}
+ this.validateMaxUpstreamConnections();
+ if (this.bandwidthError) {
+ this.inlineAlert.showInlineError(this.maxUpstreamConnError);
+ return;
</file context>
| if (this.bandwidthError) { | |
| if (this.maxUpstreamConnError) { |
| suite.driver.AssertExpectations(suite.T()) | ||
| } | ||
|
|
||
| func (suite *GetItemFromDriverTestSuite) TestGetItemFromDriverEmptyKey() { |
There was a problem hiding this comment.
TestGetItemFromDriverEmptyKey assumes that an empty key should succeed and return an empty map. The production driver treats an empty key as invalid and returns an error from the database layer, so this test will always fail when run against the real implementation. Adjust the test to expect an error (or validate the input before calling GetItemFromDriver).
Prompt for AI agents
Address the following comment on src/pkg/config/manager_test.go at line 138:
<comment>TestGetItemFromDriverEmptyKey assumes that an empty key should succeed and return an empty map. The production driver treats an empty key as invalid and returns an error from the database layer, so this test will always fail when run against the real implementation. Adjust the test to expect an error (or validate the input before calling GetItemFromDriver).</comment>
<file context>
@@ -0,0 +1,153 @@
+ suite.driver.AssertExpectations(suite.T())
+}
+
+func (suite *GetItemFromDriverTestSuite) TestGetItemFromDriverEmptyKey() {
+ key := ""
+ expectedResult := map[string]any{}
</file context>
| suite.driver.AssertExpectations(suite.T()) | ||
| } | ||
|
|
||
| func (suite *GetItemFromDriverTestSuite) TestGetItemFromDriverNilContext() { |
There was a problem hiding this comment.
TestGetItemFromDriverNilContext asserts that passing a nil context should succeed, but GetItemFromDriver simply forwards the context to the store driver, which requires a non-nil context and will panic when it reaches database helpers. Update the test to expect an error (or remove nil usage) and adjust the production code if nil contexts need to be supported.
Prompt for AI agents
Address the following comment on src/pkg/config/manager_test.go at line 123:
<comment>TestGetItemFromDriverNilContext asserts that passing a nil context should succeed, but GetItemFromDriver simply forwards the context to the store driver, which requires a non-nil context and will panic when it reaches database helpers. Update the test to expect an error (or remove nil usage) and adjust the production code if nil contexts need to be supported.</comment>
<file context>
@@ -0,0 +1,153 @@
+ suite.driver.AssertExpectations(suite.T())
+}
+
+func (suite *GetItemFromDriverTestSuite) TestGetItemFromDriverNilContext() {
+ key := common.SkipAuditLogDatabase
+ expectedResult := map[string]any{
</file context>
1, bump base photon 2, bump go for v1.24.10 Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.buildbaselog (1)
51-51: Fix the typo flagged in previous review.The typo "buld-package" should be "build-package". This was flagged in a previous review and the line is being modified, so please correct it.
Apply this diff to fix the typo:
-- Create this file to trigger build base action in buld-package workflow +- Create this file to trigger build base action in build-package workflow
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.buildbaselog(2 hunks)CONTRIBUTING.md(1 hunks)Makefile(2 hunks)src/go.mod(6 hunks)tests/ci/distro_installer.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- CONTRIBUTING.md
- Makefile
🔇 Additional comments (4)
tests/ci/distro_installer.sh (1)
6-7: LGTM - Consistent with Go version update.The GOBUILDIMAGE update to golang:1.24.10 is consistent with the Go toolchain update in src/go.mod. Ensure the Go version verification from src/go.mod passes before merging.
.buildbaselog (1)
11-25: LGTM - Changelog entries are consistent.The new base image refresh entries follow the established format and are appropriately dated.
src/go.mod (2)
78-81: Version verified as latest stable release.The Kubernetes client-go library version v0.34.1 is the latest stable version in the v0.34 series, released Sep 9, 2025. The go.mod file uses the correct and current version. No action required.
34-34: No action required — v5.3.0 is safe.The version github.com/golang-jwt/jwt/v5 v5.3.0 contains the fix for the HIGH severity vulnerability in header parsing (CVE fixed in v5.2.2). No known vulnerabilities affect this version.
src/go.mod
Outdated
| module github.com/goharbor/harbor/src | ||
|
|
||
| go 1.24.6 | ||
| go 1.24.10 |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Go 1.24.10 exists and is the recommended version.
Go 1.24.10 may not exist yet or may not be the latest patch version for the 1.24 series. Please confirm this version is valid and stable.
🏁 Script executed:
#!/bin/bash
# Check if golang:1.24.10 Docker image exists
docker manifest inspect golang:1.24.10 >/dev/null 2>&1 && echo "Image exists" || echo "Image does not exist"Length of output: 20
🌐 Web query:
What is the latest stable version of Go 1.24?
💡 Result:
The latest stable Go 1.24 patch is go1.24.9 — released October 13, 2025. [1][2]
Sources:
[1] Go release history (go.dev).
[2] go1.24.9 release details.
Update Go version to 1.24.9 (latest stable for 1.24 series).
Go 1.24.10 does not exist. The latest stable Go 1.24 patch is go1.24.9 — released October 13, 2025. Update src/go.mod line 3 from go 1.24.10 to go 1.24.9.
🤖 Prompt for AI Agents
In src/go.mod at line 3, the Go version is set to the non-existent patch "go
1.24.10"; change that line to the correct latest patch for the 1.24 series by
replacing it with "go 1.24.9" so the module declares a valid Go version.
Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
Signed-off-by: wang yan <[email protected]> Co-authored-by: wang yan <[email protected]>
|
4 participants
This syncs release 2.14 to harbor next
Summary by cubic
Syncs 2.14.x changes into the release branch. Adds upstream connection limits for proxy cache projects, allows disabling audit log DB writes at init, and updates Go/dependencies; also fixes robot permissions and CI tests.
New Features
Bug Fixes
Written for commit f1393ed. Summary will update automatically on new commits.
Summary by CodeRabbit
New Features
Improvements
Tests
Chores