Refactor key/value parsing

[aledbf]

What this PR does / why we need it:

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Which issue/s this PR fixes

fixes #6202

How Has This Been Tested?

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I've read the CONTRIBUTION guide
• I have added tests to cover my changes.
• All new and existing tests passed.

[ChiefAlexander]

Have flags without values been removed? I am wondering how this will handle flags that don't have values?

[aledbf]

Have flags without values been removed? I am wondering how this will handle flags that don't have values?

What do you mean? All flags have values

[ChiefAlexander]

Have flags without values been removed? I am wondering how this will handle flags that don't have values?

What do you mean? All flags have values

I wonder if something like --alsologtostderr from the docs is the reason this was made the way it was. I assume under the covers it is a true/false flag.

[ChiefAlexander]

Also if it is supported to just pass the flag without any value behind it wouldn't this be a breaking change as we might have people using that functionality?

[aledbf]

Have flags without values been removed? I am wondering how this will handle flags that don't have values?

What do you mean? All flags have values

I wonder if something like --alsologtostderr from the docs is the reason this was made the way it was. I assume under the covers it is a true/false flag.

Right, now if I define

extraArgs:
  alsologtostderr:

is valid?

[aledbf]

/hold

[ChiefAlexander]

extraArgs:
alsologtostderr

controller:
  extraArgs:
    alsologtostderr: ""

Generates today:

          args:
            - /nginx-ingress-controller
            - --publish-service=$(POD_NAMESPACE)/RELEASE-NAME-ingress-nginx-controller
            - --election-id=ingress-controller-leader
            - --ingress-class=nginx
            - --configmap=$(POD_NAMESPACE)/RELEASE-NAME-ingress-nginx-controller
            - --validating-webhook=:8443
            - --validating-webhook-certificate=/usr/local/certificates/cert
            - --validating-webhook-key=/usr/local/certificates/key
            - --alsologtostderr

[ChiefAlexander]

Definitely not perfect...

[aledbf]

Definitely not perfect...

ok, maybe check for empty value instead of a boolean check? (to not break what exists now)

[ChiefAlexander]

I was thinking we should probably just have the value be stringified via the quote method

[aledbf]

/hold cancel

[aledbf]

@ChiefAlexander ok, this works now without breaking existing definitions.

[ChiefAlexander]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aledbf, ChiefAlexander

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• charts/ingress-nginx/OWNERS [ChiefAlexander,aledbf]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mKaloer]

Thanks guys