-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TUF] Pull autoupdate config values from config file OR command-line args #1512
[TUF] Pull autoupdate config values from config file OR command-line args #1512
Conversation
@@ -38,7 +39,7 @@ var channelsUsingNewAutoupdater = map[string]bool{ | |||
// For now, it is only available when launcher is on the nightly update channel. | |||
func CheckOutLatestWithoutConfig(binary autoupdatableBinary, logger log.Logger) (*BinaryUpdateInfo, error) { | |||
logger = log.With(logger, "component", "tuf_library_lookup") | |||
cfg, err := getAutoupdateConfig() | |||
cfg, err := getAutoupdateConfig(os.Args[1:]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You think os.Args, or should we be passing it in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly just moved os.Args[1:]
up from where it previously was in getAutoupdateConfig()
so that I could unit test it. I don't have a strong opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@directionless I'm going to merge this as-is to move this forward, but I'm going to be doing another PR in this same area and I'm can update this there if you'd like
Currently, the new autoupdater only pulls autoupdate config values (most notably the root dir and the update channel) from the config flag file, either pulling this from the command-line args or the well-known location. However, we should support setting those config values via command-line args as well; this PR implements that change.