YAML config: Supports parsing array format

[spairy]

Which issue(s) this PR fixes:
None.

What this PR does / why we need it:
Supports parsing array of basic data types when analyzing YAML configuration files.

The current behavior of Fluentd:

• Support YAML Array format for $args only.
• Other Array options must be specified in a comma-separated format.
  • retryable_response_codes: 503, 504
  • If there’s only a single value, it must be specified as String or with the trailing comma.
    • retryable_response_codes: "503"
    • retryable_response_codes: 503,

This supports YAML Array format for all options.

retryable_response_codes:
  - 503

retryable_response_codes: [503]

Note: This PR doesn't address the issue where setting an Int directly to an Array option causes ConfigError: retryable_response_codes: 503 (The C case in #5126 (comment))
It could be a different issue. => #5149

Docs Changes: Not Need

Release Note: Same as the title.

[daipom]

Thanks for this improvement!
Could you add DCO?
Could you explain this feature in more detail?

[spairy]

Gett error when config array of basic data types with fluentd.yaml, eg:

retryable_response_codes:
  - 503  

or
retryable_response_codes: [503]

This feature is to address this issue.

[daipom]

I see. There seems to be a bug.
Fluentd should be able to parse YAML Array format: https://docs.fluentd.org/configuration/config-file-yaml#multiline-support-for-quoted-string-array-and-hash-values

I have confirmed the following behaviors.

• A: Can parse Array format for $tag.
• B: Cannot parse Array format for normal Array options.
• C: Cannot parse a single Int for normal Array options.
  • It appears that the workaround is to specify a single String. (See A example below)

A: Can parse Array format for $tag

config:
  - source:
      $type: http
  - match:
      $type: stdout
      $tag: server.**
      format:
        $type: csv
        fields: message
      buffer:
        $arg:
          - tag
          - time
        $type: memory
        timekey: 1h
        flush_mode: immediate
  - source:
      $type: sample
      tag: client
  - match:
      $type: http
      $tag: client.**
      endpoint: http://localhost:9880/server
      retryable_response_codes: "503"
      buffer:
        flush_mode: immediate

B: Cannot parse Array format for normal Array options.

fluentd/lib/fluent/config/yaml_parser/parser.rb:145:in `section_build': undefined method `each' for "message":String (NoMethodError)

config:
  - source:
      $type: http
  - match:
      $type: stdout
      $tag: server.**
      format:
        $type: csv
        fields:
          - message
      buffer:
        $arg:
          - tag
          - time
        $type: memory
        timekey: 1h
        flush_mode: immediate
  - source:
      $type: sample
      tag: client
  - match:
      $type: http
      $tag: client.**
      endpoint: http://localhost:9880/server
      retryable_response_codes:
        - 503
      buffer:
        flush_mode: immediate

C: Cannot parse a single Int for normal Array options.

[error]: config error file="..." error_class=Fluent::ConfigError error="array required but got 503"

config:
  - source:
      $type: http
  - match:
      $type: stdout
      $tag: server.**
      format:
        $type: csv
        fields: message
      buffer:
        $arg:
          - tag
          - time
        $type: memory
        timekey: 1h
        flush_mode: immediate
  - source:
      $type: sample
      tag: client
  - match:
      $type: http
      $tag: client.**
      endpoint: http://localhost:9880/server
      retryable_response_codes: 503
      buffer:
        flush_mode: immediate

[daipom]

Hmm, it looks like the current implementation intentionally treats only $args as an Array.
Is this a documentation mistake, or was something missed in the implementation?

fluentd/lib/fluent/config/yaml_parser/parser.rb

Lines 145 to 163 in 3c85506

	config.each do |key, val|
	if val.is_a?(Array)
	val.each do |v|
	sb.add_section(section_build(key, v, indent: indent + @base_indent))
	end
	elsif val.is_a?(Hash)
	harg = val.delete('$arg')
	if harg.is_a?(Array)
	# To prevent to generate invalid configuration for arg.
	# "arg" should be String object and concatenated by ","
	# when two or more objects are specified there.
	sb.add_section(section_build(key, val, indent: indent + @base_indent, arg: harg&.join(',')))
	else
	sb.add_section(section_build(key, val, indent: indent + @base_indent, arg: harg))
	end
	else
	sb.add_line(key, val)
	end
	end

[daipom]

It’s still unclear whether the current implementation is intentional or an oversight (possibly a bug).
Here’s the current behavior of Fluentd.

• Support YAML Array format for $args only.
• Other Array options must be specified in a comma-separated format.
  • retryable_response_codes: 503, 504
  • If there’s only a single value, it must be specified as String or with the trailing comma.
    • retryable_response_codes: "503"
    • retryable_response_codes: 503,

[daipom]

We supported YAML in the following:

• Handle YAML configuration format on configuration file #3712

There hasn’t been any particular discussion about handling Arrays. There are no test cases for it, but it’s clearly documented.

[daipom]

• Multiple store sections cannot be described in yaml format #3925

The following code is to parse the multiple sections.

fluentd/lib/fluent/config/yaml_parser/parser.rb

Lines 146 to 149 in 3c85506

	if val.is_a?(Array)
	val.each do |v|
	sb.add_section(section_build(key, v, indent: indent + @base_indent))
	end

[daipom]

There are two cases for Arrays: one where they should be recognized as sections, and another where they should be treated as single options.
Currently, the internal logic forces them to be recognized as sections.
To support Array options, we need to add cases like in this PR.

[daipom]

Thanks!
The direction of this fix looks good!

Since the current implementation assumes either an Array or a Hash as a section, how about checking it like this?

[daipom]

I’d appreciate it if you could add some test cases.
Please let me know if that’s difficult.

fluentd/test/test_config.rb

Line 161 in dc21d83

sub_test_case "yaml config" do

[daipom]

And could you add DCO?

https://github.com/fluent/fluentd/pull/5126/checks?check_run_id=53146586771

[daipom]

Whether to backport this needs to be discussed.

[spairy]

Thanks! Test code is difficult for me. I have add DCO. I have no other question. Please help to close this Commit if necessary.

[daipom]

Thanks!
OK! We will add some test codes to this PR.

[daipom]

Thanks! LGTM.

[Watson1978]

Thanks 👍🏻