otk: add mtls into resolve ostree command

[lzap]

SSIA

this allows resolution of pulp hosted repos

[lzap]

Sure amended a test.

[mvo5]

Thank you for working on this! Small ideas/suggestion inline, otherwise this looks very nice

[mvo5]

(nitpick) I'm not a fan of bools in APIs like this, it makes it non-obvious what this "false" means. Which means one has to go back into the function do see what it means which will break the reading flow. the "low-tech" fix would be to just define a const above like "useMtls := false" and pass that. But looking at the code maybe a small refactor could also be an option, makes it slightly more indirect though:

diff --git a/cmd/otk/osbuild-resolve-ostree-commit/main_test.go b/cmd/otk/osbuild-resolve-ostree-commit/main_test.go
index 11395309f..f3ff7695b 100644
--- a/cmd/otk/osbuild-resolve-ostree-commit/main_test.go
+++ b/cmd/otk/osbuild-resolve-ostree-commit/main_test.go
@@ -27,7 +27,7 @@ const TestCertDir = "../../../pkg/ostree/test_mtls_server"
 
 // Create a test server that responds with the commit ID that corresponds to
 // the ref.
-func createTestServer(refIDs map[string]string, mtls bool) *httptest.Server {
+func createTestHandler(refIDs map[string]string) *http.ServeMux {
        handler := http.NewServeMux()
        handler.HandleFunc("/refs/heads/", func(w http.ResponseWriter, r *http.Request) {
                reqRef := strings.TrimPrefix(r.URL.Path, "/refs/heads/")
@@ -38,26 +38,14 @@ func createTestServer(refIDs map[string]string, mtls bool) *httptest.Server {
                }
                fmt.Fprint(w, id)
        })
-
-       var server *httptest.Server
-       if mtls {
-               mtlss, err := test_mtls_server.NewMTLSServerInPath(handler, TestCertDir)
-               if err != nil {
-                       panic(err)
-               }
-
-               server = mtlss.Server
-       } else {
-               server = httptest.NewServer(handler)
-       }
-       return server
+       return handler
 }
 
 func TestResolver(t *testing.T) {
        require := require.New(t)
        assert := assert.New(t)
 
-       repoServer := createTestServer(commitMap, false)
+       repoServer := httptest.NewServer(createTestHandler(commitMap))
        defer repoServer.Close()
 
        url := repoServer.URL
@@ -95,7 +83,9 @@ func TestResolverMTLS(t *testing.T) {
        require := require.New(t)
        assert := assert.New(t)
 
-       repoServer := createTestServer(commitMap, true)
+       mtlsServer, err := test_mtls_server.NewMTLSServerInPath(createTestHandler(commitMap), TestCertDir)
+       require.NoError(err)
+       repoServer := mtlsServer.Server
        defer repoServer.Close()
 
        url := repoServer.URL
@@ -206,7 +196,7 @@ func TestResolverIDwithURL(t *testing.T) {
 
 func TestResolverErrors(t *testing.T) {
 
-       repoServer := createTestServer(commitMap, false)
+       repoServer := httptest.NewServer(createTestHandler(commitMap))
        defer repoServer.Close()
 
        type testCase struct {

[lzap]

Took a different approach, I think creating MTLS server explicitly can be copied later into more and more test cases. I agree with booleans, I do not like them for public APIs, here I did not care because it is single package/file. But point taken, introduced two separately named functions.

[lzap]

Removed the boolean, rebased, fixed tests.

[mvo5]

Thank you!

[lzap]

No other reviewers, adding to the merge queue. Thank you for your patience with me :)