| Summary: | [RFE] Inform user if there is a safemode rendering issue in provisioning template | ||
|---|---|---|---|
| Product: | Red Hat Satellite | Reporter: | sthirugn <sthirugn> |
| Component: | Provisioning Templates | Assignee: | satellite6-bugs <satellite6-bugs> |
| Status: | CLOSED UPSTREAM | QA Contact: | |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | 6.2.2 | CC: | andrew.schofield, bbuckingham, bkearney, jcallaha, lzap, mhulan, mruzicka, sthirugn |
| Target Milestone: | Unspecified | Keywords: | FutureFeature, Triaged, UserExperience |
| Target Release: | Unused | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2017-07-19 08:45:25 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
Sorry I missed one statement:
To sum it all, I would expect an error message something like:
"
Safemode doesn't allow to access 'iasgn' on $vauto = to_jail.Array("192.168.1"). More information on safemode methods can be found in http://projects.theforeman.org/projects/foreman/wiki/templatewriting#Ruby-methods. Although it is not recommended, if due to some reason, if your code cannot be rewritten using the safemode methods, consider turning off safemode rendering (Administer -> Settings -> Provisioning -> Safemode rendering -> Set to false).
Always click on Preview of the template to make sure the rendering works fine before provisioning.
"
While I agree it would be nice to have better validation checks for the template rendering, this always comes down to the following questions. When do we want to display the validation problems? If the answer is when the template is saved, then there's an issue with the context. The template is always rendered for some host. Without specified host it usually can't render because @host used in template source would be nil, hence we can't get proper validations. There was added a check on host save and if some error is detected it should not let the host save. Could you verify you were still able to save the host that is associated with affected template? We could possibly modify host build state when any template rendering fails for a given host, but since this action is not triggered by the user, we don't have a way to display notification with the error. The best workaround we have right now is to preview the template for the desired host. And yes, there we could better describe what safemode error means, would this be acceptable fix? I understand that you need a @host context to validate the template. The scenario here is: Navigate to Satellite6 Web UI -> Hosts -> Provisioning templates -> Select a template -> Edit the template -> Click Submit. To solve the host context: 1. Can we just use the first host in the dropdown (which was shown while manually clicking the Preview) 2. Can we always use the default capsule to validate the template - I assume default capsule will be present in all Satellite? Please correct me if I am wrong here. I think the template validation with atleast one host will be good enough to avoid safe mode rendering issues. Ofcourse we cannot guarantee that the template is fool proof for all hosts but the idea here is to avoid some user errors to improve usability So perhaps the proposed scenario will be: Navigate to Satellite6 Web UI -> Hosts -> Provisioning templates -> Select a template -> Edit the template -> Click Submit -> System automatically validates the template with one of the methods described above -> Show success or error message accordingly. Hello, I was wondering if there was any further feedback on Sureshkumar's more recent suggestion so I can provide our customer an update. Thank you. I don't think testing with first host would work well. Some hosts can be missing attributes required for such template. If this check should prevent saving then there would be no way to save the template if it's not rendering correctly for the first host. We could possibly build new empty host with minimal set of attributes. At least some safe mode errors might be caught with it. Building off an empty host seems like it would be better than current. Sureshkumar, any further input here since you understand this issue better than I do? Building an empty host option sounds good to me. Template errors were vastly improved recently and this will be part of upcoming 6.3 release. Also there is a new feature Template check, templates are verified when a host is entering build mode via "Review build status for xyz" modal dialog. I am closing this, will be in 6.3. |
Description of problem: User is not notified when there is a safemode rendering issue in provisioning template. So when the user tries to provision a client using this template, a generic error message 'Error downloading kickstart file.' is shown which is not helpful in troubleshooting. Version-Release number of selected component (if applicable): satellite-6.2.2-1.1.el7sat.noarch How reproducible: Always Steps to Reproduce: 1. Use an erb class (like Array) in the Provisioning template which is not approved for safemode rendering 2. Go to Hosts -> Provisioning Templates -> Clone a working provisioning template and add any erb code using Array class. Example: <% $var1=Array(['192.168.1',]) $var1.each do |network| if network.include? (@host.ip.split('.').first(3).join(".")) $vauto=true end end %> 3. Submit the template 4. Provision a new host with this new cloned template. 5. Satellite fails finding the kickstart file and provisioning failed without showing clear error message to the user Logs showed this: /var/log/foreman/production.log: 2016-09-29 08:05:02 [app] [W] There was an error rendering the rhel-provision template: | ActionView::Template::Error: Safemode doesn't allow to access 'iasgn' on $vautoNets = to_jail.Array("192.168.1") | /opt/theforeman/tfm/root/usr/share/gems/gems/safemode-1.2.3/lib/safemode/parser.rb:125:in `raise_security_error' | /opt/theforeman/tfm/root/usr/share/gems/gems/safemode-1.2.3/lib/safemode/parser.rb:66:in `process_iasgn' | /opt/theforeman/tfm/root/usr/share/gems/gems/ruby2ruby-2.1.3/lib/ruby2ruby.rb:472:in `process_gasgn' Actual results: As shown above. Expected results: Show descriptive error message to the user with all of the following information: 1. Show the exact error from the exception. In this case - `Safemode doesn't allow to access 'iasgn' on $vautoNets = to_jail.Array("192.168.1")` 2. Give alternate options to the user. Something like: `More information on safemode methods can be found in http://projects.theforeman.org/projects/foreman/wiki/templatewriting#Ruby-methods` 3. Alternatively, if need be, recommend turning off safemode rendering. Go to Administer -> Settings -> Provisioning -> Safemode rendering -> Set to false. 4. Always click on Preview of the template to make sure the rendering works fine before provisioning. To sum it all, I would expect an error message something like: " Safemode doesn't allow to access 'iasgn' on $vauto = to_jail.Array("192.168.1"). More information on safemode methods can be found in http://projects.theforeman.org/projects/foreman/wiki/templatewriting#Ruby-methods. Although it is not recommended, if due to some reason, if your code cannot be rewritten using the safemode methods, consider turning off safemode rendering (Administer -> Settings -> Provisioning -> Safemode rendering -> Set to false) " Additional info: