search

voxpupuli / puppet-gitlab_ci_runner

Module to mange gitlab CI runners. Extracted from https://github.com/voxpupuli/puppet-gitlab
Apache License 2.0
14 stars 53 forks source link

register_to_file: Support Sensitive `regtoken` #164

Closed arusso closed 3 months ago

arusso commented 1 year ago

Pull Request (PR) description

When retrieving a runner registration token from a lookup plugin such as vault_lookup the token will be marked as Sensitive. This change allows for an object of Sensitive[String] type to be passed in for regtoken and ensures it is properly unwrapped before use.

alexjfisher commented 1 year ago

Looks correct. Could you regenerate the REFERENCE.md (bundle exec rake strings:generate:reference)? I think that's what the test failure is complaining about.

smortex commented 1 year ago

Hum, maybe your bundle is outdated?

bundle update and then bundle exec rake strings:generate:reference?

arusso commented 1 year ago

No change. It seems like it's complaining about the @return missing from the unregister_from_file.rb. Am I reading that right?

It's not a file I touched, but maybe we just need to add an @return Any or modify the dispatch to be clear that a String is always returned?

arusso commented 1 year ago

Looks-like CI is happier 👍

Just a minor thing, and I think we are good to go!

Ended up rebuilding my ruby environment and that seemed to do the trick.

arusso commented 1 year ago

I took a stab at the spec test for this change and it passed when run locally. Not sure what to make of the automated tests, seems like they're broken (at least) due to Puppet 8.0.1 needing a newer version of Ruby than whats provided?

arusso commented 10 months ago

Anything I can/need do to get this merged, or is this still broken due to the Puppet 8 release?

smortex commented 10 months ago

I think the error was fixed. Let's close & re-open to trigger a build.

traylenator commented 5 months ago

Just ran into this - patch works great thanks.

traylenator commented 5 months ago

Approved but given the age here a rebase makes sense I would say.

traylenator commented 5 months ago

191 should fix tests.