Skip to content

feat: support random suffix#4

Merged
yuzichen12123 merged 2 commits into
AlaudaDevops:mainfrom
yuzichen12123:feat/support-random-suffix
Apr 30, 2026
Merged

feat: support random suffix#4
yuzichen12123 merged 2 commits into
AlaudaDevops:mainfrom
yuzichen12123:feat/support-random-suffix

Conversation

@yuzichen12123

@yuzichen12123 yuzichen12123 commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator

将 neuxs-cli 生成的后缀由秒级时间戳改为 毫秒级时间戳 与 4位随机后缀,改动后的用户名长度不会超出 nexus 限制。

已运行单元测试,全部通过。并在 tektoncd-operator 中执行过 make prepare-nexus-data,成功。

@codecov-commenter

codecov-commenter commented Apr 30, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 87.64045% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/config/name_resolver.go 87.64% 7 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

@alaudabot

alaudabot commented Apr 30, 2026

Copy link
Copy Markdown

🤖 AI Code Review

Property Value
Model opencode/minimax-m2.5-free
Style strict
Issues Found 1
Config Source centralized
Profile ❌ Not Found
Personalized Prompt ❌ No
Prompt Path .github/review/profiles/alaudadevops/nexus-cli/pr-review.md
Alauda Skills ✅ base-acp-operator-list, base-acp-operator-release, base-authoring, base-m365, base-ocp-operator-list, base-skill-setup, builders-alauda-pipeline, builders-component-knowledge, builders-confluence, builders-jira, builders-notify-wecom, builders-prd-to-testcase, builders-publish-errata, builders-roadmap-studio, builders-story-split, cross-repo-add-mirror, cross-repo-publish, devops-autodns, devops-candidate-version-supervisor, devops-connectors-acceptance-test, devops-connectors-explore, devops-connectors-poc-case, devops-connectors-review, devops-connectors-unit-test, devops-connectors-upgrade-test, devops-connectors-write-user-docs, devops-creating-tekton-pipelines, devops-fix-go-vulns, devops-fork-alauda-binary-release, devops-gen-advanced-form-descriptors, devops-knowledge-adoption, devops-refresh-containerfile-digests, devops-refresh-containerfile-tags, devops-replace-strings, devops-scan-docker-keywords, devops-sync-alauda-github-releases, devops-tekton-dynamic-form-optimizer, devops-tekton-operator-task-e2e, devops-tekton-pipeline-delivery, devops-tekton-task-generator, devops-tekton-task-overview-template, devops-tekton-task-version-upgrade, devops-tekton-upgrade-notes, devops-tool-report-troubleshoot, devops-ui-e2e-code-audit, devops-ui-e2e-fix-base-on-report, devops-ui-e2e-regression-and-fix, devops-ui-generate-e2e-from-feature, devops-ui-pre-setup, devops-upgrade-go, devops-upstream-backport-cve, devops-upstream-upgrade
Reviewed at 2026-04-30 07:40:28 UTC

Summary

This PR changes the suffix generation from second-level timestamp to millisecond-level timestamp with 4 random alphanumeric characters. The implementation includes proper error handling with crypto/rand and fallback to time-based generation. Significant code refactoring improves maintainability. One critical issue found in the fallback logic that could produce shorter-than-expected suffixes.

Review Statistics

Category Count
Critical Issues 1
Warnings 0
Suggestions 2
Files Reviewed 4

Critical Issues

  • [pkg/config/name_resolver.go:~54] (bug/function-contract): The fallback logic in generateShortRandomSuffix() may return a string shorter than the requested length. When randomRead fails, the code checks if len(fallbackFromTime) > length and returns fallbackFromTime[len(fallbackFromTime)-length:]. However, this condition will be FALSE when length is > the string length, causing the function to return an incomplete string (less than length characters). This violates the function contract that promises to return exactly length characters.

Warnings

No warnings found.

Suggestions

  • [pkg/config/name_resolver.go:~54] (style/robustness): Consider adding input validation for length parameter. While negative/zero values are handled, consider returning an error for clearly invalid values (e.g., length > 100) to fail fast and make bugs more obvious.

  • [pkg/config/name_resolver.go:~44] (refactor/clarity): The fallbackFromTime computation uses time.Now().UnixNano() which is 19+ digits. A shorter source like time.Now().Unix() (seconds since epoch) produces more predictable output. Consider using a circular buffer approach to ensure exactly length characters even in fallback.

Positive Feedback

  • Excellent refactoring: Splitting the monolithic resolveNamesWithSuffix() into focused functions (resolveUsers, resolveRepositories, resolvePrivileges, resolveRoles, rewriteResolvedReferences) greatly improves maintainability
  • Good test coverage: Added comprehensive tests for edge cases (empty inputs, random failure fallback)
  • Proper error handling: Using crypto/rand with fallback to time-based generation is a solid pattern
  • Smart dependency injection: Using randomRead var allows tests to exercise fallback branches
  • Good use of nameResolutionState struct to organize state instead of multiple map variables


ℹ️ About this review

This review was automatically generated using the run-actions workflow.

  • Shared prompt: .github/prompts/code-review.md
  • Config source: centralized
  • Profile path: Not Found
  • Profile ref: ebb6c9593926ecbd0b2b1d3ebde0c09c862ff8bf
  • No repository-specific prompt configured
  • Alauda skills: base-acp-operator-list, base-acp-operator-release, base-authoring, base-m365, base-ocp-operator-list, base-skill-setup, builders-alauda-pipeline, builders-component-knowledge, builders-confluence, builders-jira, builders-notify-wecom, builders-prd-to-testcase, builders-publish-errata, builders-roadmap-studio, builders-story-split, cross-repo-add-mirror, cross-repo-publish, devops-autodns, devops-candidate-version-supervisor, devops-connectors-acceptance-test, devops-connectors-explore, devops-connectors-poc-case, devops-connectors-review, devops-connectors-unit-test, devops-connectors-upgrade-test, devops-connectors-write-user-docs, devops-creating-tekton-pipelines, devops-fix-go-vulns, devops-fork-alauda-binary-release, devops-gen-advanced-form-descriptors, devops-knowledge-adoption, devops-refresh-containerfile-digests, devops-refresh-containerfile-tags, devops-replace-strings, devops-scan-docker-keywords, devops-sync-alauda-github-releases, devops-tekton-dynamic-form-optimizer, devops-tekton-operator-task-e2e, devops-tekton-pipeline-delivery, devops-tekton-task-generator, devops-tekton-task-overview-template, devops-tekton-task-version-upgrade, devops-tekton-upgrade-notes, devops-tool-report-troubleshoot, devops-ui-e2e-code-audit, devops-ui-e2e-fix-base-on-report, devops-ui-e2e-regression-and-fix, devops-ui-generate-e2e-from-feature, devops-ui-pre-setup, devops-upgrade-go, devops-upstream-backport-cve, devops-upstream-upgrade

@yuzichen12123 yuzichen12123 force-pushed the feat/support-random-suffix branch from c375a94 to 8fa6534 Compare April 30, 2026 06:52

@alaudabot alaudabot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Found 1 critical issue that must be fixed: the fallback logic in generateShortRandomSuffix() may return a string shorter than requested length.

privilegeNameMap := make(map[string]string, len(resolved.Privileges))
if err := resolveUsers(resolved.Users, defaultMode, suffix, state); err != nil {
return nil, false, err
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Critical Issue (bug/function-contract): The fallback logic here may return a string shorter than length. When len(fallbackFromTime) <= length, the function returns the entire fallbackFromTime (which is ~19 characters for UnixNano()), violating the function contract that promises exactly length characters.

Suggestion: Always ensure exactly length characters in fallback:

Suggested change
}
if _, err := randomRead(rawRandomBytes); err != nil {
fallbackFromTime := fmt.Sprintf("%d", time.Now().UnixNano())
// Pad or truncate to ensure exactly `length` characters
for len(fallbackFromTime) < length {
fallbackFromTime += fallbackFromTime
}
return fallbackFromTime[:length]
}

@@ -54,120 +66,168 @@ func (c *Config) resolveNamesWithSuffix(suffix string) (*Config, bool, error) {
if err != nil {
return nil, false, fmt.Errorf("invalid config.nameMode: %w", err)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion (style/robustness): Consider validating length early for invalid values (e.g., > 100) to fail fast.

@yuzichen12123 yuzichen12123 merged commit 7079cd8 into AlaudaDevops:main Apr 30, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants