kellyjonbrazil / jc

CLI tool and python library that converts the output of popular command-line tools, file-types, and common strings to JSON, YAML, or Dictionaries. This allows piping of output to tools like jq and simplifying automation scripts.
MIT License
7.91k stars 210 forks source link

Ansible community.general.jc error with JC 1.25.0 and ini parser #539

Closed chriscroome closed 9 months ago

chriscroome commented 9 months ago

I'm not sure if this is a JC or an Ansible issue but older versions of JC with older version of Ansible are fine:

- name: Slurp the MariaDB configuration file when it exists for /etc/mysql/mariadb.conf.d/50-server.cnf
  ansible.builtin.slurp:
    path: /etc/mysql/mariadb.conf.d/50-server.cnf
  register: mariadb_cnf_file_b64encoded

- name: Set a fact for the existing MariaDB configuration file variables for /etc/mysql/mariadb.conf.d/50-server.cnf
  ansible.builtin.set_fact:
    mariadb_cnf_file_existing_vars: "{{ mariadb_cnf_file_b64encoded['content'] | ansible.builtin.b64decode | community.general.jc('ini') }}"

Fail with:

TASK [mariadb : Set a fact for the existing MariaDB configuration file variables for /etc/mysql/mariadb.conf.d/50-server.cnf] *********
task path: /home/chris/webarch/mariadb/tasks/conf.yml:71
fatal: [wsh.webarchitects.org.uk]: FAILED! => 
  msg: 'Error in jc filter plugin:  ''str'' object has no attribute ''append'''

Could this have been caused by recent security fixes in Ansible?

Address issues where internal templating can cause unsafe variables to lose their unsafe designation (CVE-2023-5764)

:shrug:

kellyjonbrazil commented 9 months ago

Hi! Could you send the debug.msg to show what the decoded string looks like?

chriscroome commented 9 months ago

I'm not sure what you mean by the debug.msg? The result are the same as above when run with -vvvvvvv.

kellyjonbrazil commented 9 months ago

Could you set up your yaml file to send this output to debug.msg?

{{ mariadb_cnf_file_b64encoded['content'] | ansible.builtin.b64decode |

I just want to see what the string looks like before it gets to jc. Not sure if Ansible is changing anything with it before it gets to jc.

chriscroome commented 9 months ago

Adding this:

- name: Debug the base64 decoded the existing MariaDB configuration file variables for /etc/mysql/mariadb.conf.d/50-server.cnf
  ansible.builtin.debug:
    msg: "{{ mariadb_cnf_file_b64encoded['content'] | ansible.builtin.b64decode }}"
    verbosity: "{% if ansible_check_mode | bool %}2{% else %}3{% endif %}"

Returns:

TASK [mariadb : Debug the base64 decoded the existing MariaDB configuration file variables for /etc/mysql/mariadb.conf.d/50-server.cnf] ***
task path: /home/chris/webarch/mariadb/tasks/conf.yml:71
ok: [wsh.webarchitects.org.uk] => 
  msg: |-
    # Ansible managed
    # These groups are read by MariaDB server.
    # Use it for options that only the server (but not clients) should see
    #
    # See the examples of server my.cnf files in /usr/share/mysql/
    #

    # this is read by the standalone daemon and embedded servers
    [server]

    # this is only for the mysqld standalone daemon
    [mysqld]

    #
    # * Basic Settings
    #
    user            = mysql
    pid_file        = /var/run/mysqld/mysqld.pid
    port            = 3306
    basedir         = /usr
    datadir         = /var/lib/mysql
    tmpdir          = /tmp
    lc_messages_dir = /usr/share/mysql
    skip_external_locking

    # Instead of skip-networking the default is now to listen only on
    # localhost which is more compatible and is not less secure.
    bind_address            = 127.0.0.1

    #
    # * Fine Tuning
    #
    key_buffer_size         = 16M
    max_allowed_packet      = 64M
    thread_stack            = 192K
    thread_cache_size       = 8
    # This replaces the startup script and checks MyISAM tables if needed
    # the first time they are touched
    myisam_recover_options  = BACKUP
    max_connections         = 80
    max_user_connections    = 0
    table_cache             = 64
    thread_concurrency      = 10
    open_files_limit        = 122880
    table_open_cache        = 6000
    tmp_table_size          = 32M
    join_buffer_size        = 8M
    max_heap_table_size     = 32M

    #
    # * Query Cache Configuration
    #
    # Disabled by default in MariaDB >= 10.1.7 see:
    # https://mariadb.com/kb/en/query-cache/
    query_cache_type        = 0
    query_cache_limit       = 0
    query_cache_size        = 0

    #
    # * Logging and Replication
    #
    # Both location gets rotated by the cronjob.
    # Be aware that this log type is a performance killer.
    # As of 5.1 you can enable the log at runtime!
    #general_log_file        = /var/log/mysql/mysql.log
    #general_log             = 1
    #
    # Error log - should be very few entries.
    #
    log_error = /var/log/mysql/error.log
    #
    # Enable the slow query log to see queries with especially long duration
    #slow_query_log_file    = /var/log/mysql/mariadb-slow.log
    #long_query_time = 10
    #log_slow_rate_limit    = 1000
    #log_slow_verbosity     = query_plan
    #log-queries-not-using-indexes
    #
    # The following can be used as easy to replay backup logs or for replication.
    # note: if you are setting up a replication slave, see README.Debian about
    #       other settings you may need to change.
    #server-id              = 1
    #log_bin                        = /var/log/mysql/mysql-bin.log
    expire_logs_days        = 10
    max_binlog_size   = 100M
    #binlog_do_db           = include_database_name
    #binlog_ignore_db       = exclude_database_name

    #
    # * InnoDB
    #
    # InnoDB is enabled by default with a 10MB datafile in /var/lib/mysql/.
    # Read the manual for more InnoDB related options. There are many!
    innodb_buffer_pool_size         = 1G
    innodb_log_file_size            = 256M

    # * Security Features
    #
    # Read the manual, too, if you want chroot!
    # chroot = /var/lib/mysql/
    #
    # For generating SSL certificates you can use for example the GUI tool "tinyca".
    #
    # ssl-ca=/etc/mysql/cacert.pem
    # ssl-cert=/etc/mysql/server-cert.pem
    # ssl-key=/etc/mysql/server-key.pem
    #
    # Accept only connections using the latest and most secure TLS protocol version.
    # ..when MariaDB is compiled with OpenSSL:
    # ssl-cipher=TLSv1.2
    # ..when MariaDB is compiled with YaSSL (default in Debian):
    # ssl=on

    #
    # * Character sets
    #
    # MySQL/MariaDB default is Latin1, but in Debian we rather default to the full
    # utf8 4-byte character set. See also client.cnf
    #
    character_set_server            = utf8mb4
    collation_server                = utf8mb4_general_ci
    ignore_db_dir = lost+found

    #
    # * Unix socket authentication plugin is built-in since 10.0.22-6
    #
    # Needed so the root database user can authenticate without a password but
    # only when running as the unix root user.
    #
    # Also available for other users if required.
    # See https://mariadb.com/kb/en/unix_socket-authentication-plugin/

    # this is only for embedded server
    [embedded]

    # This group is only read by MariaDB servers, not by MySQL.
    # If you use the same .cnf file for MySQL and MariaDB,
    # you can put MariaDB-only options here
    [mariadb]
    # https://mariadb.com/kb/en/library/performance-schema-overview/
    performance_schema=ON
    performance_schema_instrument='stage/%=ON'
    performance_schema_consumer_events_stages_current=ON
    performance_schema_consumer_events_stages_history=ON
    performance_schema_consumer_events_stages_history_long=ON

    # This group is only read by MariaDB-10.1 servers.
    # If you use the same .cnf file for MariaDB of different versions,
    # use this group for options that older servers don't understand
    [mariadb-10.1]
    # vim: syntax=dosini

Which is the contents of /etc/mysql/mariadb.conf.d/50-server.cnf.

I also tried commenting this line:

skip_external_locking

But that made no difference.

kellyjonbrazil commented 9 months ago

Interesting, it's dying at this line in the parser:

157             ini_parser.read_string(data)

Something within the python ConfigParser library doesn't like it. I'll have to take a closer look.

chriscroome commented 9 months ago

BTW there is a new Ansible from_ini filter but it so far lacks support for sections and duplicates so isn't a replacement for your parsers.

chriscroome commented 9 months ago

Switching back to JC version 1.24.0 appears to solve the issue.

kellyjonbrazil commented 9 months ago

Strange - does that also change the python version? On my laptop if I run the following I get the same error but if I slightly change the input it works fine:

% echo '[data]
novalue
' | jc --ini
jc:  Error - ini parser could not parse the input data.
             If this is the correct parser, try setting the locale to C (LC_ALL=C).
             For details use the -d or -dd option. Use "jc -h --ini" for help.

This is with python v3.11.1

chriscroome commented 9 months ago

This is with the Debian Bookworm python3:

apt show python3 | jc --ini | jp Version
"3.11.2-1+b1"
which jc
/home/chris/.local/bin/jc

jc --version
jc version:  1.24.0
python interpreter version:  3.11.2
python path:  /home/chris/.local/pipx/venvs/ansible/bin/python

https://github.com/kellyjonbrazil/jc
© 2019-2023 Kelly Brazil
echo '[data]
novalue
' | jc --ini -p
{
  "data": {
    "novalue": ""
  }
}
kellyjonbrazil commented 9 months ago

I'm trying to see if there were any changes to the ini parser that could have caused this. I wonder if this is relevant?

https://github.com/python/cpython/issues/107625

kellyjonbrazil commented 9 months ago

Ah, I think I see something that might be affecting this. I'm using a custom dictionary type to change None values to empty strings (``). This seems to break the ConfigParser module in these strange cases.

class MyDict(dict):
    def __setitem__(self, key, value):
        # convert None values to empty string
        if value is None:
            self[key] = ''
        ini_parser = configparser.ConfigParser(
            dict_type = MyDict,
            allow_no_value=True,
            interpolation=None,
            default_section=None,

It seems to work fine with the ini-dup parser because it wraps the items in a list, which can be appended to. I may need to rethink this method of converting None to a blank string.

I might even just move that type of conversion to the _process function, so it is outside the ConfigParser responsibility.

kellyjonbrazil commented 9 months ago

I have a fix in dev:

https://github.com/kellyjonbrazil/jc/blob/dev/jc/parsers/ini.py

kellyjonbrazil commented 9 months ago

fix in v1.25.1