Before this PR, withOpts checked if the IP denylist at opts.BlacklistedIP was empty, and if it was, it didn't override the default value (10.0.0.0/8). Unfortunately, this effectively prevented from clearning that default, as specifying --blocked-nets="" would cause opts.BlacklistedIP to become the empty string, which will not be applied over the default.
To prevent having two defaults (cli flag and in-constructor), with the constructor one being always ignored, this PR gets rid of the constructor default.
Additionally, this PR fixes a bug that made this change ineffective when WithLogger was called on the local runner, as doing so would not copy the IP denylist.
Finally, having a mixture of pointer and non-pointer returns for LocalRunner was making testing hard. This PR also uniforms to non-pointer runners everywhere.
Before this PR,
withOptschecked if the IP denylist atopts.BlacklistedIPwas empty, and if it was, it didn't override the default value (10.0.0.0/8). Unfortunately, this effectively prevented from clearning that default, as specifying--blocked-nets=""would causeopts.BlacklistedIPto become the empty string, which will not be applied over the default.To prevent having two defaults (cli flag and in-constructor), with the constructor one being always ignored, this PR gets rid of the constructor default.
The changes here should not cause any functional change, as
blocked-netsdefaults to10.0.0.0/8either way in the CLI code: https://github.com/grafana/synthetic-monitoring-agent/blob/7123e22162573d5ff1b83649aca571dd7f0479b9/cmd/synthetic-monitoring-agent/main.go#L80Additionally, this PR fixes a bug that made this change ineffective when
WithLoggerwas called on the local runner, as doing so would not copy the IP denylist.Finally, having a mixture of pointer and non-pointer returns for LocalRunner was making testing hard. This PR also uniforms to non-pointer runners everywhere.
Related to: https://github.com/grafana/synthetic-monitoring-agent/issues/730