Add check for invalid roles in server_role_names=

[Fryguy]

This fixes an issue where invalid roles names are silently ignored. I think it was expected that assign_role call a little below would call ServerRole.to_role and raise this exception, however the ServerRole.where just before it will ignore the invalid roles on querying, and thus assign_role can never be called with a bad role name.

I was thinking of fixing this to call .to_role, but I realize that method is a wasted abstraction and I plan to remove it in a follow up PR.

@jrafanie Please review

[Fryguy]

FWIW, I noticed this because I was trying to run_single_worker locally passing --roles ui, and it turns out the actual role is called user_interface. It just kept telling me I had insufficient roles, but I didn't know it was because I typoed the role name.

[Fryguy]

I also found that miq_alert_spec was actually silently not setting the notifier role as it was specified. Since ServerRole was not seeded, then calling role= was ignoring the notifier role and just not setting it and yet the specs passed. We will probably want to revisit that but for now I've updated the local_miq_server method to accept role and do the right thing. The only caller to local_miq_server with a role was miq_alert_spec, so I could also just update that one to not set the roles in that way.

[Fryguy]

Updated to just fix the only caller that used :role = to local_miq_server

[Fryguy]

@miq-bot cross_repo_tests manageiq-api, manageiq-ui-classic, manageiq-providers-amazon

[miq-bot]

Checked commit Fryguy@1024c1a with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint
4 files checked, 2 offenses detected

spec/models/miq_server/role_management_spec.rb

• ❗ - Line 66, Col 9 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.
• ❗ - Line 97, Col 9 - Style/MethodCallWithArgsParentheses - Use parentheses for method calls with arguments.

[jrafanie]

LGTM, I only have concern about the change in behavior and if we accidentally tried to assigned undefined roles in the past, it would silently be ignored. Now, it raises. It makes sense but I'm not sure if this is completely covered by tests.

[Fryguy]

So, the reason it ignored it previously is because in tests things were not seeded. So when it tried to set the role, it just ignored the "missing" roles. In a real production run, the server roles would be seeded. The only way it could fail now is if there is a typo, which seems unlikely because then it never would have worked.

[jrafanie]

Merging, I agree, production code shouldn't accept undefined roles and it's more likely it's test code with not fully seeded roles that was hitting this so it's very unlikely to cause a production problem.

[jrafanie]

@Fryguy is this kasparov/yes?