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.

Sign up for GitHub

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

Jump to bottom

appservice: Fix containerized function app name validation to exclude uppercase letters #1754

Merged
merged 7 commits into from
Jul 9, 2024
Merged

appservice: Fix containerized function app name validation to exclude uppercase letters #1754

Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ba69bab
fix name validation
motm32 Jul 8, 2024
f3f6bdb
more changes
motm32 Jul 8, 2024
36217a3
switc conditional
motm32 Jul 8, 2024
288c52a
bump version
motm32 Jul 8, 2024
52806ff
change regex
motm32 Jul 9, 2024
415f552
fix merge conflicts
motm32 Jul 9, 2024
882fdb8
bump version
motm32 Jul 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions appservice/src/createAppService/SiteNameStep.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ export class SiteNameStep extends AzureNameStep<SiteNameStepWizardContext> {

if (name.length < siteNamingRules.minLength || name.length > siteNamingRules.maxLength) {
return vscode.l10n.t('The name must be between {0} and {1} characters.', siteNamingRules.minLength, siteNamingRules.maxLength);
} else if (this._siteFor === "containerizedFunctionApp" && (!/^[a-z]([-a-z0-9]*[a-z0-9])?$/.test(name))) {
Copy link
Contributor

@MicroFish91 MicroFish91 Jul 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think the * here might be in the wrong spot, otherwise you can still have multiple -'s together. Here is what I think you had for ContainerAppNameStep!

!/^[a-z][a-z0-9]*(-[a-z0-9]+)*$/.test(name)

Copy link
Contributor Author

@motm32 motm32 Jul 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Oh lol yes I copied this from ManagedEnvironmentNameStep since I figured it was the same since the error message was the same but I can change it.

On another note shouldn't the regex be the same between ContainerAppNameStep and ManagedEnvironmentNameStep since the error message is the same between the two?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah good catch, if the requirements are the same I think we should update that check logic

return vscode.l10n.t("A name must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character and cannot have '--'.");
} else if (siteNamingRules.invalidCharsRegExp.test(name)) {
return vscode.l10n.t("The name can only contain letters, numbers, or hyphens.");
}
Expand Down
Loading