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

Inconsistent Profile Name restrictions #5576

Closed
ocket8888 opened this issue Feb 25, 2021 · 4 comments · Fixed by #6018
Closed

Inconsistent Profile Name restrictions #5576

ocket8888 opened this issue Feb 25, 2021 · 4 comments · Fixed by #6018
Labels
bug something isn't working as intended low impact affects only a small portion of a CDN, and cannot itself break one Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1

Comments

@ocket8888
Copy link
Contributor

I'm submitting a ...

  • bug report

Traffic Control components affected ...

  • Traffic Ops
  • Traffic Portal

Current behavior:

When cloning a Profile in Traffic Portal, the Name of the new Profile is allowed to contain spaces. On the Profile's "details" page, it is not. The Traffic Ops API doesn't care if a Profile Name contains spaces.

Expected behavior:

The restrictions placed on Profile Name should be consistent throughout the Traffic Portal UI, and should match what is required by the Traffic Ops API. This is a bug with Traffic Portal not accepting valid Profile Names, if the intention is to allow Profile Names to contain spaces. If the intention is for Profile Names to not contain spaces, then this a bug with both Traffic Portal and Traffic Ops.

Minimal reproduction of the problem with instructions:

Clone a Profile in Traffic Portal, put a space or two in the Name, then observe that the field is highlighted as invalid after a successful cloning.

@ocket8888 ocket8888 added bug something isn't working as intended Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1 low impact affects only a small portion of a CDN, and cannot itself break one labels Feb 25, 2021
@rawlinp
Copy link
Contributor

rawlinp commented Feb 25, 2021

We have profile names as path parameters, so I imagine we should prohibit spaces in the names. I'm actually surprised we don't already restrict to alphanumeric + _ + - + ..

@ocket8888
Copy link
Contributor Author

ocket8888 commented Feb 25, 2021

That'll require a migration to make existing Profiles compliant. Probably by replacing all whitespace with _? Though also path parameters can contain spaces, if properly encoded as %20. I don't particularly care one way or the other.

@rawlinp
Copy link
Contributor

rawlinp commented Feb 25, 2021

We don't necessarily have to migrate existing names. It can be a new validation on profile creates/updates. If you want to update a profile, first you'd have to fix the name. Plus, the TP validation already (mostly) prohibits it.

@ocket8888
Copy link
Contributor Author

Yeah, that's true. We wouldn't have to use a migration. "Fix it on update" is fine as far as I'm concerned, as long as operators would prefer that to a migration. Though they may not care one way or the other, because since - as you said - TP validation already mostly covers it so there are probably a max of like 5 Profiles in the world with this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working as intended low impact affects only a small portion of a CDN, and cannot itself break one Traffic Ops related to Traffic Ops Traffic Portal v1 related to Traffic Portal version 1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants