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

Set limits for VM and Template names and descriptions #16736

Merged
merged 1 commit into from
Jan 4, 2018

Conversation

masayag
Copy link
Contributor

@masayag masayag commented Jan 3, 2018

RHV enforce the following limits:

  • VM name to 255 characters
  • VM description to 255 characters
  • Template name to 40 characters (it will be extended to 255 in ovirt-engine-4.2)
  • Template description to 255 characters

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1529052

The UI enforces the limits on insertion for the name and description fields:
provision_vm_name_limit

publish_vm_template_name_limit

@masayag
Copy link
Contributor Author

masayag commented Jan 3, 2018

@miq-bot assign @gmcculloug

@masayag
Copy link
Contributor Author

masayag commented Jan 3, 2018

@miq-bot add_labels gaprindashvili/yes, bug

@gmcculloug
Copy link
Member

What is the timeframe for the ovirt-engine-4.2 changes? Should we just use the higher value now? Are we going to remember to increase it once it is updated?

@Fryguy
Copy link
Member

Fryguy commented Jan 3, 2018

Do these changes apply to all supported versions of RHV?

@masayag
Copy link
Contributor Author

masayag commented Jan 3, 2018

@gmcculloug ovirt-engine-4.2 is already available upstream. However, if we increase the limit, the 'publish vm as template' might fail for ovirt-engine-4.1 (if the user will set a name longer than 40 chars).

That's why I thought to set the shorter limit of 4.1 which will work for both versions.

@Fryguy the limits which are specified in this PR are supported by all ovirt-engine versions (they were defined in the early days of ovirt-engine-3.x), except of the template name limit that was recently increased (in 4.2).

@gmcculloug
Copy link
Member

@masayag I'm always a little concerned about enforcing these types of limits in the provisioning dialogs as admins can make modifications to the name value from automate. Like adding enumeration to the name value, but I do realized we need to start somewhere.

Two thoughts:

  1. If you want to enforce this limit based on the provider level of the targeted environment you could create an validation method in the backend. For example: https://github.com/ManageIQ/manageiq/blob/master/app/models/miq_provision_virt_workflow/dialog_field_validation.rb. Users would still be able to disable this by modifying the dialog if they wanted/needed to.
  2. This bug is internally reported and I would probably opt for using the larger template length value. If a user needed to smaller limit they could reduce the value in the dialog.

RHV enforce the following limits:
* VM name to 255 characters
* VM description to 255 characters
* Template name to 40 characters (it will be extended to 255 in
ovirt-engine-4.2)
* Template description to 255 characters

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1529052
@masayag masayag force-pushed the vm_template_limits branch from 6403513 to 1f2a123 Compare January 4, 2018 13:30
@masayag
Copy link
Contributor Author

masayag commented Jan 4, 2018

@gmcculloug I'd prefer the second option. I've updated the PR accordingly.

@miq-bot
Copy link
Member

miq-bot commented Jan 4, 2018

Checked commit masayag@1f2a123 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. ⭐

@gmcculloug gmcculloug merged commit bf689eb into ManageIQ:master Jan 4, 2018
@gmcculloug gmcculloug added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 4, 2018
simaishi pushed a commit that referenced this pull request Jan 4, 2018
Set limits for VM and Template names and descriptions
(cherry picked from commit bf689eb)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1531292
@simaishi
Copy link
Contributor

simaishi commented Jan 4, 2018

Gaprindashvili backport details:

$ git log -1
commit 0ae390bc3049e2121a125b8b48dc49eb659a514f
Author: Greg McCullough <[email protected]>
Date:   Thu Jan 4 09:40:55 2018 -0500

    Merge pull request #16736 from masayag/vm_template_limits
    
    Set limits for VM and Template names and descriptions
    (cherry picked from commit bf689eba47174db57344d593e53bb3cf28677c95)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1531292

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.

5 participants