Jenkins CLI auth info in run_state is no longer being used as of 8.1.0

[nuclearsandwich]

Jenkins CLI auth info in run_state is no longer being picked up.

🥞 Cookbook version

8.1.0 is the first release to include the change made in #717.

👩‍🍳 Chef-Infra Version

cinc-client 15.14.0, testing with 15 and 16.

🎩 Platform details

Ubuntu 20.04

Steps To Reproduce

Steps to reproduce the behavior:

Prior to 8.1.0 setting node.run_state[:jenkins_username] and node.run_state[:jenkins_password] was a functional way to set authentication information without putting a password in a chef attribute.

Here's an example where my cookbook does this by fetching it from a data bag: https://github.com/ros-infrastructure/cookbook-ros-buildfarm/blob/40ea9247f9154a8b08d61efa36616d96ac1d3d83/recipes/jenkins.rb#L10-L14

✒️ Expected behavior

My understanding of current recommended practice is that we should prefer run_state to attributes for sensitive items like auth info. I think one way out would be to prefer the run_state and fall back to these attributes with a warning if nothing is found.

➕ Additional context

I'd like to come up with a solution that fixes the regression while still addressing the reason #717 was introduced. I'm working on the test issues in #757 and I'm pretty sure that they'll be affected by this change since the smoke tests also use run_state for CLI auth.

[djessich]

I have the same problem. How's the new way to properly authenticate with local cli using this cookbook?

The following workaround works as with version < 8.1.0, but I believe this way is not a good option:

module Jenkins
  module Helper
    def cli_username
      node.run_state[:jenkins_username]
    end

    def cli_password
      node.run_state[:jenkins_password]
    end
  end
end

[davidsainty]

The "new" way seems to be to put the credentials into attributes, but that's usually a bad idea. I'm not sure what #717 was trying to fix, but it might be worth getting the author to weigh in, or consider reverting #717 to repair the regression...

[djessich]

@davidsainty Yes, you are correct, this is usually a bad idea. I have no idea, why #717 was merged by the maintainers... However, it should somehow again use node.run_state as previously.

[djessich]

Any progress of this? The bug still persists and it is not possible to authenticate :(

[ramereth]

Unfortunately I haven't made any progress but hopefully I'll get back to this soon.

[djessich]

Whats the current status?