WIP: Refactor how cloud options are specified

[ChaosInTheCRD]

Hi There! 👋

I am new around here, so this is my first interaction with the project 😃

I fancied trying to address issue #1155 and submit a PR for it, and this one was of interest to many people so I decided to go for it.

Keeping both options certainly makes things a bit messy. Having options.cloud and options.ext.loadimpact accepted into the same function seemed to remove the ability to pass them into the GetConsolidatedConfig function as the same variable (one is a map and the other is just a JSON raw message). Instead I had to add an extra parameter and put the decision logic of whether it was cloud or loadimpact outside of the MergeFromExternal function. I am new around here, so I am unsure whether the way I have tried to add the warning to cloud.go makes sense. But I figured that would be easy for someone to recommend an alternative in the PR if it is wrong 😄 .

I have tried to mark TODO:'s wherever the temporary loadimpact'ness is still lying around, so it will be easy to remove it when the time comes.

The only other thing is that one unit test was failing on my machine. This is cert related, and I decided to wait before deciding to debug. I figured it might be something to do with my local machine. I can work on this a bit more to address if need be 😄:

--- FAIL: TestVUIntegrationClientCerts (0.01s)
    --- FAIL: TestVUIntegrationClientCerts/VerifyServerCert (0.00s)
        --- FAIL: TestVUIntegrationClientCerts/VerifyServerCert/Archive (0.01s)
            runner_test.go:1846:
                	Error Trace:	/Users/tom.meadows/Git/k6-fork/js/runner_test.go:1846
                	Error:      	Error "GoError: Get \"https://127.0.0.1:49852\": x509: “127.0.0.1:6969” certificate is not standards compliant\n\tat go.k6.io/k6/js/modules/k6/http.(*RootModule).NewModuleInstance.func2 (native)\n\tat file:///script.js:7:25(4)\n" does not contain "certificate signed by unknown authority"
                	Test:       	TestVUIntegrationClientCerts/VerifyServerCert/Archive
        --- FAIL: TestVUIntegrationClientCerts/VerifyServerCert/Source (0.01s)
            runner_test.go:1846:
                	Error Trace:	/Users/tom.meadows/Git/k6-fork/js/runner_test.go:1846
                	Error:      	Error "GoError: Get \"https://127.0.0.1:49852\": x509: “127.0.0.1:6969” certificate is not standards compliant\n\tat go.k6.io/k6/js/modules/k6/http.(*RootModule).NewModuleInstance.func2 (native)\n\tat file:///script.js:7:25(4)\n" does not contain "certificate signed by unknown authority"
                	Test:       	TestVUIntegrationClientCerts/VerifyServerCert/Source
2023/03/09 01:02:49 http: TLS handshake error from [::1]:50048: remote error: tls: bad certificate
2023/03/09 01:02:49 http: TLS handshake error from [::1]:50049: remote error: tls: bad certificate
2023/03/09 01:02:49 http: TLS handshake error from [::1]:50052: remote error: tls: bad certificate
2023/03/09 01:02:49 http: TLS handshake error from [::1]:50053: remote error: tls: bad certificate
2023/03/09 01:02:49 http: TLS handshake error from [::1]:50056: tls: no cipher suite supported by both client and server
2023/03/09 01:02:49 http: TLS handshake error from [::1]:50057: tls: no cipher suite supported by both client and server
2023/03/09 01:02:49 http: TLS handshake error from [::1]:50058: EOF
2023/03/09 01:02:49 http: TLS handshake error from [::1]:50059: EOF
FAIL

I hope this was in some way helpful 😄 I am going to try and manually test my changes out, as I still haven't had time to do that. If this doesn't solve the problem and is off the mark, please let me know and I will try and find some time to move it towards done.

[CLAassistant]

All committers have signed the CLA.

[codecov-commenter]

Codecov Report

Merging #2965 (eb580b5) into master (367163b) will decrease coverage by 0.07%.
The diff coverage is 72.41%.

❗ Current head eb580b5 differs from pull request most recent head 5bb0620. Consider uploading reports for the commit 5bb0620 to get more accurate results

@@            Coverage Diff             @@
##           master    #2965      +/-   ##
==========================================
- Coverage   76.84%   76.77%   -0.07%     
==========================================
  Files         229      227       -2     
  Lines       16924    16934      +10     
==========================================
- Hits        13005    13001       -4     
- Misses       3077     3085       +8     
- Partials      842      848       +6     

Flag	Coverage Δ
ubuntu	76.77% <72.41%> (+0.01%)	⬆️
windows	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cmd/login_cloud.go	20.22% <0.00%> (-0.47%)	⬇️
lib/options.go	89.62% <ø> (ø)
cloudapi/config.go	90.00% <73.33%> (+0.86%)	⬆️
cmd/cloud.go	71.89% <100.00%> (+0.30%)	⬆️
output/cloud/output.go	84.83% <100.00%> (+0.12%)	⬆️
loader/filesystems.go	60.00% <0.00%> (-40.00%)	⬇️
js/common/initenv.go	66.66% <0.00%> (-33.34%)	⬇️
api/v1/metric.go	54.54% <0.00%> (-13.64%)	⬇️
cmd/tests/test_state.go	87.75% <0.00%> (-4.09%)	⬇️
js/initcontext.go	80.00% <0.00%> (-4.00%)	⬇️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[codebien]

Hi @ChaosInTheCRD,
thanks for your contribution 🙇

Unfortunately, this is not as easy as we reported in the issue. This is not only around the k6 open source code but has several implications for the entire Grafana k6 platform. It requires some decisions for the product and the code architecture because it impacts the Cloud and how we must maintain them in the future.

I would suggest picking an issue where the scope and the requirements are better defined and it isn't so dependent on internal knowledge. In this way, you may experience better collaboration.

Remember that it generally helps if you ask for quick feedback on your solution's idea in the issue before starting work on it.

[ChaosInTheCRD]

Hi @codebien ☺️

That's absolutely no problem. Sorry for not voicing ideas first, I just fancied having a crack at writing some code for the project to see how I would get on. I then felt there wouldn't be any harm in putting the PR forward, even if it doesn't get used.

When I get some more time in the future I'll raise thoughts and ideas in issues before proceeding. In the meantime though, I guess the PR can be closed and the code can stay put for anyone looking to implement this when the time comes!

Thanks,