dokku_config: allow list type #55

Open decentral1se opened 4 years ago

TASK [Configure the environment] ***** fatal: [gitea]: FAILED! => {"changed": false, "meta": {"error": "All values must be keys, found invalid types for WHITELISTED_URIS", "present": false}, "msg": "All values must be keys, found invalid types for WHITELISTED_URIS"}

https://docs.gitea.io/en-us/config-cheat-sheet/

WHITELISTED_URIS: : If non-empty, list of POSIX regex patterns matching OpenID URI’s to permit.

So, I need to add the following config:

    - name: config env
      dokku_config:
        app: gitea
        config:
          WHITELISTED_URIS: ["foo.com"]

Shall we widen up the types that are accepted?

Actually, also "Foo: true" fails as well, which is pretty typical in an Ansible play. Looks like https://github.com/dokku/ansible-dokku/blob/master/library/dokku_config.py#L93 is the issue here. I think a bool should be easy to allow also. I imagine "FOO: 12" will also break then.

The error should say All values must be strings.

Description here as to my reasoning.

Hmmm, Ansible does the following regarding trying to convert bools:

def to_bool(a):
    ''' return a bool for the arg '''
    if a is None or isinstance(a, bool):
        return a
    if isinstance(a, string_types):
        a = a.lower()
    if a in ('yes', 'on', '1', 'true', 1):
        return True
    return False

So, we could do that to address you concern in https://github.com/dokku/ansible-dokku/issues/6#issuecomment-484308360. We can extend the "in" check to cover other values that you might use / expect for truthiness.

I understand why it works the way it works now but it unfortunately breaks a very common expectation when writing Ansible plays that you can use integers, bools and not quote booleans. That is confusing coming from a strictly Ansible side of things.

The problem here is that folks won't be able to specify the difference between True (string) and True (bool) values, so trying to set those as values may result in issues at the application level. For instance, you might set true expecting that as the literal string, but then it gets set as the literal True string when we call config:set. The underlying Dokku command does not distinguish between value types, so attempting to support custom value types doesn't make sense insofar as I don't want to publish rules on how we serialize things.

The rule that you can use different types within Ansible only applies to when the input expects that. We do not, and the module expects a string value. Trying to coerce the value here doesn't make sense at all since you're attempting to set a literal value in your app's environment.

Makes sense! I'll get a docs patch in to avoid this being raised up again :sun_behind_large_cloud:

@decentral1se Is the docs patch in? Can we close this?

dokku / ansible-dokku

dokku_config: allow list type #55