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

(ecr): make validateRepositoryName errors human readable #26715

Closed
1 of 2 tasks
scanlonp opened this issue Aug 10, 2023 · 3 comments · Fixed by #27186
Closed
1 of 2 tasks

(ecr): make validateRepositoryName errors human readable #26715

scanlonp opened this issue Aug 10, 2023 · 3 comments · Fixed by #27186
Assignees
Labels
@aws-cdk/aws-ecr Related to Amazon Elastic Container Registry effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@scanlonp
Copy link
Contributor

Describe the feature

The current errors say repository names must match a long regex. Make the errors explain why the name was not valid in an understandable way.

Use Case

Understanding why a repository name is not valid without deciphering regex.

Proposed Solution

Translate the regex into words in the error messages.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.91.0

Environment details (OS name and version, etc.)

Mac

@scanlonp scanlonp added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 10, 2023
@scanlonp scanlonp self-assigned this Aug 10, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ecr Related to Amazon Elastic Container Registry label Aug 10, 2023
@scanlonp
Copy link
Contributor Author

For reference

private static validateRepositoryName(physicalName: string) {
const repositoryName = physicalName;
if (!repositoryName || Token.isUnresolved(repositoryName)) {
// the name is a late-bound value, not a defined string,
// so skip validation
return;
}
const errors: string[] = [];
// Rules codified from https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ecr-repository.html
if (repositoryName.length < 2 || repositoryName.length > 256) {
errors.push('Repository name must be at least 2 and no more than 256 characters');
}
const isPatternMatch = /^(?:[a-z0-9]+(?:[._-][a-z0-9]+)*\/)*[a-z0-9]+(?:[._-][a-z0-9]+)*$/.test(repositoryName);
if (!isPatternMatch) {
errors.push('Repository name must follow the specified pattern: (?:[a-z0-9]+(?:[._-][a-z0-9]+)*/)*[a-z0-9]+(?:[._-][a-z0-9]+)*');
}
if (errors.length > 0) {
throw new Error(`Invalid ECR repository name (value: ${repositoryName})${EOL}${errors.join(EOL)}`);
}
}

@pahud
Copy link
Contributor

pahud commented Aug 11, 2023

Yeah it makes sense to improve the error messages here.

@pahud pahud added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 11, 2023
@mergify mergify bot closed this as completed in #27186 Sep 19, 2023
mergify bot pushed a commit that referenced this issue Sep 19, 2023
When a user specifies an invalid repository name, the error message will now describe why the name is invalid instead of providing the regex used to validate the name. The message comes from the console when defining a new repository. The docstring of `repositoryName` now reflects this and mirrors the docstring of the underlying CFN resource.

Closes #26715.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

HBobertz pushed a commit that referenced this issue Sep 19, 2023
When a user specifies an invalid repository name, the error message will now describe why the name is invalid instead of providing the regex used to validate the name. The message comes from the console when defining a new repository. The docstring of `repositoryName` now reflects this and mirrors the docstring of the underlying CFN resource.

Closes #26715.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecr Related to Amazon Elastic Container Registry effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants