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

Add validate_blacklist method for VM pre-provisioning #15513

Merged
merged 1 commit into from
Jul 10, 2017
Merged

Add validate_blacklist method for VM pre-provisioning #15513

merged 1 commit into from
Jul 10, 2017

Conversation

djberg96
Copy link
Contributor

@djberg96 djberg96 commented Jul 5, 2017

This modifies the MiqRequestWorkflow::DialogFieldValidation module by adding a validate_blacklist method that provisioning templates can then hook into.

Specifically, this is needed for Azure (though possibly other providers) which have special blacklists for VM usernames and passwords that would pass a regex check but are not considered valid by Microsoft. For example, you cannot use "abc@123" as a password for Azure VM's, even though that would normally pass a validity check.

These lists are too long to be put into a regex validation directly. It simply isn't practical, so I believe a separate method is necessary.

For more information please see https://docs.microsoft.com/en-us/azure/virtual-machines/windows/faq

Partially solves https://bugzilla.redhat.com/show_bug.cgi?id=1454829. The other half will happen on the Azure provider side.

@miq-bot
Copy link
Member

miq-bot commented Jul 5, 2017

Checked commit https://github.com/djberg96/manageiq/commit/b9c9a7bdd8e2d8e7e8e1decfaba058cd88004b77 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 1 offense detected

spec/models/miq_request_workflow_spec.rb

@djberg96
Copy link
Contributor Author

djberg96 commented Jul 5, 2017

@gmcculloug @blomquisg @h-kataria Look ok?

Copy link
Contributor

@syncrou syncrou left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@gmcculloug
Copy link
Member

Looks good.

The one thing I wanted to mention is that we should always make these types of validations optional so the user can modify the dialog to allow special characters since the dialog is just collecting the initial name to use and it can be modified before passing to the provider.

For example, adding the sequence $n{04} into the dialog name would cause the code to substitute an enumerated leading-zero value. The characters $ and {} may not be valid for a VM name but may mean something to the back-end code or could be substituted in custom automation code.

We are going with the 80/20 rule here as most users would not be entering special characters at dialog ordering time. The one exception would likely be an admin setting up a Catalog Item in which case they would have more knowledge of the back-end customizations.

@gmcculloug gmcculloug merged commit 34cab60 into ManageIQ:master Jul 10, 2017
@gmcculloug gmcculloug added this to the Sprint 64 Ending Jul 10, 2017 milestone Jul 10, 2017
@djberg96
Copy link
Contributor Author

@miq-bot add_label fine/yes

@djberg96 djberg96 deleted the validate_blacklist branch July 17, 2017 18:29
simaishi pushed a commit that referenced this pull request Aug 4, 2017
Add validate_blacklist method for VM pre-provisioning
(cherry picked from commit 34cab60)

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

simaishi commented Aug 4, 2017

Fine backport details:

$ git log -1
commit 0c39c467217b47095ac4a8c8bd47fa87c02241a7
Author: Greg McCullough <[email protected]>
Date:   Mon Jul 10 15:55:19 2017 -0400

    Merge pull request #15513 from djberg96/validate_blacklist
    
    Add validate_blacklist method for VM pre-provisioning
    (cherry picked from commit 34cab60b852f37c0c5145a07b68cd07f45cc6e8c)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1478563

d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
Add validate_blacklist method for VM pre-provisioning
(cherry picked from commit 34cab60)

https://bugzilla.redhat.com/show_bug.cgi?id=1478563
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.

6 participants