Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Update Application Name Restrictions #7706

Merged
merged 1 commit into from
Jan 13, 2025
Merged

Conversation

zhengkunwang223
Copy link
Member

No description provided.

Copy link

f2c-ci-robot bot commented Jan 13, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@@ -222,7 +222,8 @@ const message = {
paramUrlAndPort:
'このフィールドは、「http(s)://(domain name/ip):(ポート)」の形式でなければなりません。',
nginxDoc: 'このフィールドは、英語、数字、「」で構成されている必要があります。文字。',
appName: `このフィールドは、「 - 」と「_」文字で開始および終了してはなりません。英語、数字、 "、および「_」文字で2〜30の文字で構成されている必要があります。`,
appName:
'小文字の英字、数字、-および_をサポートし、長さは2〜30で、-または_で始まったり終わったりすることはできません',
containerName: '文字、数字、 - 、_および。;- _または。で始めることはできません。長さ:2-128',
mirror: 'ミラーアクセラレーションアドレスは、http(s)://、英語の文字(大文字と小文字の両方)、数字をサポートする必要があります。/および - 、そして空白の行を含めてはなりません。',
disableFunction: 'サポートレター、アンダースコア、および',
Copy link
Member

Choose a reason for hiding this comment

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

The changes you've made appear to be correct. Here's the revised version:

const message = {
  // ... other properties ...
  appName:
    "小写字母的英文字、数字、-或_组成,长度为2至30,且不能以-或_开头或结尾",
  containerName:
    '字符、数字或 -、_、以及。;- 或者。开头和结尾都不是允许的,总长为2到128',
  mirror:
    '镜像地址必须是http(s)://的形式,并支持英文字母(大小写都可以)、数字、 / 和 -,但不能包括空格行。',
  disableFunction: '支持字母、下划线和其他',
};

These changes ensure that the appName format is adhered to strictly according to your requirements, preventing it from starting or ending with - or _.

@@ -289,7 +289,7 @@ const checkAppName = (rule: any, value: any, callback: any) => {
if (value === '' || typeof value === 'undefined' || value == null) {
callback(new Error(i18n.global.t('commons.rule.appName')));
} else {
const reg = /^(?![_-])[a-zA-Z0-9_-]{1,29}[a-zA-Z0-9]$/;
const reg = /^(?![_-])[a-z0-9_-]{1,29}[a-zA-Z0-9]$/;
if (!reg.test(value) && value !== '') {
callback(new Error(i18n.global.t('commons.rule.appName')));
} else {
Copy link
Member

Choose a reason for hiding this comment

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

The proposed changes make the regular expression more consistent with typical app name conventions by requiring at least one lowercase letter ([a-z]) before allowing uppercase letters and numbers to follow, ensuring a broader variety of names while maintaining simplicity. The regex now matches strings like "app1", which were previously unvalidated, improving validation for certain edge cases. No significant issues exist; this change only optimizes input validation consistency.

@@ -223,7 +223,7 @@ const message = {
paramComplexity: `This field mustn't start and end with special characters and must consist of English, numbers, "{0}" characters with a length of 6-128.`,
paramUrlAndPort: 'This field must be in the format of "http(s)://(domain name/ip):(port)".',
nginxDoc: 'This field must consist of English, numbers and "." characters.',
appName: `This field mustn't start and end with "-" and "_" characters and must consist of English, numbers, "-", and "_" characters with a length of 2-30.`,
appName: 'Supports lowercase letters, numbers, -, and _, length 2-30, and cannot start or end with - or _',
containerName: 'Supports letters, numbers, -, _ and .; cannot start with - _ or .; length: 2-128',
mirror: 'The mirror acceleration address should start with http(s)://, support English letters (both uppercase and lowercase), numbers, . / and -, and should not contain blank lines.',
disableFunction: 'Only support letters ,underscores,and,',
Copy link
Member

Choose a reason for hiding this comment

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

It seems like there is some ambiguity in the original text when it comes to the requirements of appName. The code snippet provided uses different styles for the rules:

  1. paramAppName: It specifies that the application name should start and end with "-" or "_", which contradicts the second part where lowercase letters, numbers, -, _ are mentioned.

  2. containerName, mirror: These fields have more explicit restrictions compared to appName.

To simplify and clarify these rules without introducing ambiguities:

@@ -223,7 +223,7 @@ const message = {
             paramComplexity: `This field mustn't start and end with special characters and must consist of English, numbers, "{0}" characters with a length of 6-128.`,
             paramUrlAndPort: 'This field must be in the format of "http(s)://(domain name/ip):(port)".',
             nginxDoc: 'This field must consist of English, numbers and "." characters.',
-            appName: `This field mustn't start and end with "-" and "_" characters and must consist of English, numbers, "-", and "_" characters with a length of 2-30.`,
+            appName: 'Supports only lowercase letters, numbers, -, and _, length 2-30, and cannot start or end with - or _. ',

Suggestions:

  • Ensure consistency in rules within each group (appName, etc.).
  • Avoid contradicting conditions in multiple places if possible.
  • Make sure all characters allowed in certain positions follow a clear pattern to prevent confusion.

Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud left a comment

Choose a reason for hiding this comment

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

/lgtm

@wanghe-fit2cloud
Copy link
Member

/approve

Copy link

f2c-ci-robot bot commented Jan 13, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wanghe-fit2cloud

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@f2c-ci-robot f2c-ci-robot bot merged commit 4dee4e1 into dev Jan 13, 2025
6 checks passed
@f2c-ci-robot f2c-ci-robot bot deleted the pr@dev@website branch January 13, 2025 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants