Support providing ZDM configuration as YAML file #71
Conversation
| # Defines how configuration shall be passed to ZDM proxy. | ||
| # Allowed values are "env_vars" and "conf_file". | ||
| zdm_proxy_config_mode: env_vars |
There was a problem hiding this comment.
Is there some pre-check we can do on this value before running a runbook? I don't know much about ansible to know how feasible this would be. Maybe a task that runs before Create ZDM proxy container ... ?
There was a problem hiding this comment.
We could do below before creating configuration files, but in the end we do not validate other parameters (e.g. contact points):
- name: Verify proxy container configuration
assert:
that: zdm_proxy_config_mode == "env_vars" or zdm_proxy_config_mode == "conf_file"
success_msg: "ZDM proxy container correctly configured"
fail_msg: "Invalid value of 'zdm_proxy_config_mode' parameter"
There was a problem hiding this comment.
Contact points will be validated by the proxy, automation just passes them through. This setting is an automation specific setting.
There was a problem hiding this comment.
The general guideline with Ansible is typically to rely on the playbook letting you know that it failed and why. Ansible is pretty good at that, it usually returns meaningful errors and it is idempotent, so if you fix the issue and re-run a partially executed playbook it will skip the steps that it has already done and pick up any change, re-executing only whatever is affected by the change and then anything that was not executed at all of course.
However we have to see how this works for us in this particular case.
I would say that there are two things to check:
- if the parameter is not set at all, does the execution default to
env_vars? which i think is what we wanted in order to have backward compatibility - if the parameter is explicitly set to an incorrect value (e.g.
env_varorconfig_file), does the playbook fail in a visible and clear way?
If we see that it doesn't explicitly fail but silently executes incorrectly, or if the failure is not clear, validating this particular parameter may be appropriate.
There was a problem hiding this comment.
I have added application of default value and explicit value check. In other cases, expressions at various places are getting more and more complex (we always need to check if value is undefined).
| origin_username: {{ origin_username | default('') }} | ||
| origin_password: {{ origin_password | default('') }} | ||
| target_username: {{ target_username | default('') }} | ||
| target_password: {{ target_password | default('') }} |
There was a problem hiding this comment.
where are these settings in the env vars mode?
There was a problem hiding this comment.
oh they are included as env vars in the actual runbook yml, in that case we need to make sure these env vars are not set if the config mode is file config
There was a problem hiding this comment.
That is already taken care of.
There was a problem hiding this comment.
Nevermind, I went back and read it again and I understand now.
ansible/deploy_zdm_proxy.yml
Outdated
| assert: | ||
| that: zdm_proxy_config_mode == "env_vars" or zdm_proxy_config_mode == "conf_file" | ||
| success_msg: "ZDM proxy container correctly configured" | ||
| fail_msg: "Invalid value of 'zdm_proxy_config_mode' parameter" |
There was a problem hiding this comment.
looks good and should be clearer for users. i would suggest two very small changes to improve clarity.
(1) make naming more consistent. the parameter is called zdm_proxy_config_mode but on of the possible values is conf_file. this small inconsistency (conf vs config) could be confusing. i would stick to config for both the parameter name and its potential value, as we already use config in other variable names.
(2) explicitly list the two supported modes in the error message, so the user can easily see if they accidentally passed an invalid value (env-vars, env_var, conf-file, or similar things). for example, if you opted for the config naming, you could have something like: Invalid value for the 'zdm_proxy_config_mode' parameter: accepted values are 'env_vars' or 'config_file'
No description provided.