(PUP-3020) Add more config manipulation options

[binford2k]

• puppet config add [name] [value] will add to a comma separated list.
  • puppet config add reports store (adds store to the reports list)
• puppet config del [name] [value] will remove from a comma separated list
  • puppet config del reports store (deletes store from the list)
• puppet config del [name] will remove a setting from puppet.conf
  • puppet config del reporturl (removes reporturl from puppet.conf)

This obeys the standard options, like --section and shouldn't change
existing behaviour.

I tried writing spec tests, but all of the tests, not just the ones I
wrote, seemed to blow up. This is what I tried:

  it "adds a setting to a list" do
    Puppet[:reports] = 'https'
    subject.add('reports', 'store')

    expect { subject.print("reports") }.to have_printed('https,store')
  end

[puppetcla]

CLA signed by all contributors.

[adrienthebo]

@binford2k thanks for this contribution! First off, is there a JIRA issue associated with this change? Secondly it looks like there are spec failures that may have been due to the change in this PR, could you see if they were preexisting or if they're due to this change?

[adrienthebo]

@binford2k it looks like this hasn't seen an update in about a month, is this something you're still interested in working on?

[binford2k]

I will poke at it again this weekend :)

[adrienthebo]

@binford2k it's been about five days since we've seen an update on this issue, did you get a chance to work on this last weekend?

[adrienthebo]

And if you're not able to get to fixing this pull request up, would you be able to create an issue in JIRA so that we can track this work and possibly finish it ourselves?

[jpartlow]

@binford2k I added PUP-3020 for this; would you update the commits to reflect this ticket (and update the ticket as needed). Thanks!

[jpartlow]

@binford2k I've been testing out the PR locally, and I'm wondering about the add to list, del from list functionality. You mentioned the reports setting, but an (incomplete) look through our 200+ defaults is only turning up 7 that seem to be in the comma delimited list category. (The ones I found were reports, dns_alt_names, dynamic_facts, transaction, ldapclassattrs, ldapstackedattrs, and the ever popular ldapattrs.)

Can you give more information about your use cases here?

The puppet config del foo use is pretty straight forward and is a good addition.

There is a change to puppet config set foo '' though in that this used to end up with:

  foo=

and now deletes foo. I think it's counter intuitive that a set action would end up deleting the setting in puppet.conf, and this doesn't allow you to override a default with a blank setting. Though perhaps there is no use case for this.

[Iristyle]

@binford2k do you have any additional follow up on this? Thanks!

[Iristyle]

@binford2k We will close this at next weeks community PR triage on 9/17 if there is no follow up on this.

Thanks!

[binford2k]

@Iristyle I'm going to go ahead and close this. I'll revisit it after PuppetConf and submit an updated PR

[joshcooper]

ping @binford2k