From d4eee339a9799a94eff049fa7641cf5806c9f3ea Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 12 May 2026 02:12:54 +0530 Subject: [PATCH 1/4] refactor: replace verbose dump helpers with loggingTransport The old approach used a config.Verbose global and scattered httputil.DumpRequestOut/DumpResponse calls across every method. Replace it with a small loggingTransport that wraps the HTTP client's RoundTripper once at construction time. - Logs >> METHOD URL and each request header to stderr - Redacts the Authorization header value - Logs << STATUS after each response - Removes the Verbose global and DumpRequestIfRequired / DumpResponseIfRequired helpers from pkg/config Signed-off-by: Aditya --- pkg/config/config.go | 31 ----------- pkg/connectors/keycloak_client.go | 9 +--- pkg/connectors/microcks_client.go | 74 +++++++++++--------------- pkg/connectors/microcks_client_test.go | 59 ++++++++++++++++++++ 4 files changed, 92 insertions(+), 81 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index e5ff9a9..d9f4dbb 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -19,8 +19,6 @@ import ( "crypto/tls" "crypto/x509" "fmt" - "net/http" - "net/http/httputil" "os" "path/filepath" strings "strings" @@ -31,8 +29,6 @@ var ( InsecureTLS bool = false // CaCertPaths defines extra paths (comma-separated) of CRT files to add to system CA Roots. CaCertPaths string - // Verbose represents a debug flag for HTTP Exchanges - Verbose bool = false ConfigPath = filepath.Join(os.Getenv("HOME"), ".microcks-cli", "config.yaml") ) @@ -67,30 +63,3 @@ func CreateTLSConfig() *tls.Config { } return tlsConfig } - -// DumpRequestIfRequired takes care of dumping request if configured that way -func DumpRequestIfRequired(name string, req *http.Request, body bool) { - if Verbose { - fmt.Printf("\nDumping request '%s':\n", name) - dump, err := httputil.DumpRequestOut(req, body) - if err != nil { - fmt.Println("Got error while dumping request out") - } - fmt.Printf("%s", dump) - } -} - -// DumpResponseIfRequired takes care of dumping request if configured that way -func DumpResponseIfRequired(name string, resp *http.Response, body bool) { - if Verbose { - fmt.Printf("\nDumping response '%s':\n", name) - dump, err := httputil.DumpResponse(resp, body) - if err != nil { - fmt.Println("Got error while dumping response") - } - fmt.Printf("%s", dump) - if body { - fmt.Println("") - } - } -} diff --git a/pkg/connectors/keycloak_client.go b/pkg/connectors/keycloak_client.go index 5d6aeed..4ee6af0 100644 --- a/pkg/connectors/keycloak_client.go +++ b/pkg/connectors/keycloak_client.go @@ -25,8 +25,9 @@ import ( "net/url" "strings" - "github.com/microcks/microcks-cli/pkg/config" "golang.org/x/oauth2" + + "github.com/microcks/microcks-cli/pkg/config" ) // KeycloakClient defines methods for cinteracting with Keycloak @@ -83,18 +84,12 @@ func (c *keycloakClient) ConnectAndGetToken() (string, error) { req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Basic "+credential) - // Dump request if verbose required. - config.DumpRequestIfRequired("Keycloak for getting token", req, false) - resp, err := c.httpClient.Do(req) if err != nil { return "", err } defer resp.Body.Close() - // Dump response if verbose required. - config.DumpResponseIfRequired("Keycloak for getting token", resp, true) - body, err := io.ReadAll(resp.Body) if err != nil { panic(err.Error()) diff --git a/pkg/connectors/microcks_client.go b/pkg/connectors/microcks_client.go index f76884b..c5502ce 100644 --- a/pkg/connectors/microcks_client.go +++ b/pkg/connectors/microcks_client.go @@ -43,6 +43,27 @@ var ( grantTypeChoices = map[string]bool{"PASSWORD": true, "CLIENT_CREDENTIALS": true, "REFRESH_TOKEN": true} ) +type loggingTransport struct { + transport http.RoundTripper +} + +func (l *loggingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + fmt.Fprintf(os.Stderr, ">> %s %s\n", req.Method, req.URL.String()) + for k, v := range req.Header { + if strings.EqualFold(k, "authorization") { + fmt.Fprintf(os.Stderr, ">> %s: [REDACTED]\n", k) + continue + } + fmt.Fprintf(os.Stderr, ">> %s: %s\n", k, strings.Join(v, ", ")) + } + res, err := l.transport.RoundTrip(req) + if err != nil { + return nil, err + } + fmt.Fprintf(os.Stderr, "<< %s\n", res.Status) + return res, nil +} + // MicrocksClient allows interacting with Microcks APIs type MicrocksClient interface { HttpClient() *http.Client @@ -148,18 +169,18 @@ func NewClient(opts ClientOptions) (MicrocksClient, error) { } if opts.Verbose { - c.Verbose = opts.Verbose + c.Verbose = true } + var transport http.RoundTripper = http.DefaultTransport if config.InsecureTLS || len(config.CaCertPaths) > 0 { tlsConfig := config.CreateTLSConfig() - tr := &http.Transport{ - TLSClientConfig: tlsConfig, - } - c.httpClient = &http.Client{Transport: tr} - } else { - c.httpClient = http.DefaultClient + transport = &http.Transport{TLSClientConfig: tlsConfig} + } + if c.Verbose { + transport = &loggingTransport{transport: transport} } + c.httpClient = &http.Client{Transport: transport} if localCfg != nil { err = c.refreshAuthToken(localCfg, ctxName, opts.ConfigPath) @@ -187,15 +208,12 @@ func NewMicrocksClient(apiURL string) MicrocksClient { } mc.APIURL = u + var transport http.RoundTripper = http.DefaultTransport if config.InsecureTLS || len(config.CaCertPaths) > 0 { tlsConfig := config.CreateTLSConfig() - tr := &http.Transport{ - TLSClientConfig: tlsConfig, - } - mc.httpClient = &http.Client{Transport: tr} - } else { - mc.httpClient = http.DefaultClient + transport = &http.Transport{TLSClientConfig: tlsConfig} } + mc.httpClient = &http.Client{Transport: transport} return &mc } func (c *microcksClient) HttpClient() *http.Client { @@ -214,18 +232,12 @@ func (c *microcksClient) GetKeycloakURL() (string, error) { req.Header.Set("Accept", "application/json") - // Dump request if verbose required. - config.DumpRequestIfRequired("Microcks for getting Keycloak config", req, true) - resp, err := c.httpClient.Do(req) if err != nil { return "", err } defer resp.Body.Close() - // Dump request if verbose required. - config.DumpResponseIfRequired("Microcks for getting Keycloak config", resp, true) - body, err := io.ReadAll(resp.Body) if err != nil { panic(err.Error()) @@ -353,18 +365,12 @@ func (c *microcksClient) CreateTestResult(serviceID string, testEndpoint string, req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Bearer "+c.AuthToken) - // Dump request if verbose required. - config.DumpRequestIfRequired("Microcks for creating test", req, true) - resp, err := c.httpClient.Do(req) if err != nil { return "", err } defer resp.Body.Close() - // Dump response if verbose required. - config.DumpResponseIfRequired("Microcks for creating test", resp, true) - body, err := io.ReadAll(resp.Body) if err != nil { panic(err.Error()) @@ -392,18 +398,12 @@ func (c *microcksClient) GetTestResult(testResultID string) (*TestResultSummary, req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Bearer "+c.AuthToken) - // Dump request if verbose required. - config.DumpRequestIfRequired("Microcks for getting status", req, false) - resp, err := c.httpClient.Do(req) if err != nil { return nil, err } defer resp.Body.Close() - // Dump response if verbose required. - config.DumpResponseIfRequired("Microcks for getting status test", resp, true) - body, err := io.ReadAll(resp.Body) if err != nil { panic(err.Error()) @@ -456,18 +456,12 @@ func (c *microcksClient) UploadArtifact(specificationFilePath string, mainArtifa req.Header.Set("Content-Type", writer.FormDataContentType()) req.Header.Set("Authorization", "Bearer "+c.AuthToken) - // Dump request if verbose required. - config.DumpRequestIfRequired("Microcks for uploading artifact", req, true) - resp, err := c.httpClient.Do(req) if err != nil { return "", err } defer resp.Body.Close() - // Dump response if verbose required. - config.DumpResponseIfRequired("Microcks for uploading artifact", resp, true) - respBody, err := io.ReadAll(resp.Body) if err != nil { panic(err.Error()) @@ -510,18 +504,12 @@ func (c *microcksClient) DownloadArtifact(artifactURL string, mainArtifact bool, req.Header.Set("Content-Type", writer.FormDataContentType()) req.Header.Set("Authorization", "Bearer "+c.AuthToken) - // Dump request if verbose required. - config.DumpRequestIfRequired("Microcks for uploading artifact", req, true) - resp, err := c.httpClient.Do(req) if err != nil { return "", err } defer resp.Body.Close() - // Dump response if verbose required. - config.DumpResponseIfRequired("Microcks for uploading artifact", resp, true) - respBody, err := io.ReadAll(resp.Body) if err != nil { panic(err.Error()) diff --git a/pkg/connectors/microcks_client_test.go b/pkg/connectors/microcks_client_test.go index 9e9b6c8..5928ec5 100644 --- a/pkg/connectors/microcks_client_test.go +++ b/pkg/connectors/microcks_client_test.go @@ -3,6 +3,7 @@ package connectors import ( "net/http" "net/http/httptest" + "os" "strings" "testing" ) @@ -41,3 +42,61 @@ func TestDownloadArtifactReturnsResponseBody(t *testing.T) { t.Fatalf("expected response body %q, got %q", expectedBody, msg) } } + +func TestLoggingTransportPassesThrough(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + req, _ := http.NewRequest("GET", server.URL, nil) + lt := &loggingTransport{transport: http.DefaultTransport} + + old := os.Stderr + pr, pw, _ := os.Pipe() + os.Stderr = pw + + resp, err := lt.RoundTrip(req) + + pw.Close() + os.Stderr = old + pr.Close() + + if err != nil { + t.Fatalf("RoundTrip returned error: %v", err) + } + if resp.StatusCode != http.StatusOK { + t.Fatalf("expected 200, got %d", resp.StatusCode) + } +} + +func TestLoggingTransportRedactsAuth(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + req, _ := http.NewRequest("GET", server.URL, nil) + req.Header.Set("Authorization", "Bearer supersecret") + + old := os.Stderr + pr, pw, _ := os.Pipe() + os.Stderr = pw + + lt := &loggingTransport{transport: http.DefaultTransport} + _, _ = lt.RoundTrip(req) + + pw.Close() + os.Stderr = old + + buf := make([]byte, 4096) + n, _ := pr.Read(buf) + out := string(buf[:n]) + + if strings.Contains(out, "supersecret") { + t.Fatal("loggingTransport leaked the Authorization token") + } + if !strings.Contains(out, "[REDACTED]") { + t.Fatal("loggingTransport did not redact the Authorization header") + } +} From 4920c44ae19ece477ce5d92c0a3735505dbcea5a Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 12 May 2026 02:13:12 +0530 Subject: [PATCH 2/4] feat: add -v shorthand for --verbose flag Also remove the now-dead config.Verbose assignments from all command Run functions - the flag value flows through ClientOptions.Verbose directly into the HTTP client transport. Signed-off-by: Aditya --- cmd/cmd.go | 2 +- cmd/import.go | 2 -- cmd/importDir.go | 1 - cmd/importURL.go | 1 - cmd/login.go | 1 - cmd/test.go | 1 - 6 files changed, 1 insertion(+), 7 deletions(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index 16df6b9..ee3fd40 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -53,7 +53,7 @@ func NewCommad() *cobra.Command { errors.CheckError(err) command.PersistentFlags().StringVar(&clientOpts.ConfigPath, "config", defaultLocalConfigPath, "Path to Microcks config") command.PersistentFlags().StringVar(&clientOpts.Context, "microcks-context", "", "Name of the Microcks context to use") - command.PersistentFlags().BoolVar(&clientOpts.Verbose, "verbose", false, "Produce dumps of HTTP exchanges") + command.PersistentFlags().BoolVarP(&clientOpts.Verbose, "verbose", "v", false, "print HTTP request and response details to stderr") command.PersistentFlags().BoolVar(&clientOpts.InsecureTLS, "insecure-tls", false, "Whether to accept insecure HTTPS connection") command.PersistentFlags().StringVar(&clientOpts.CaCertPaths, "caCerts", "", "Comma separated paths of CRT files to add to Root CAs") command.PersistentFlags().StringVar(&clientOpts.ClientId, "keycloakClientId", "", "Keycloak Realm Service Account ClientId") diff --git a/cmd/import.go b/cmd/import.go index c1a74c4..2e0bc89 100644 --- a/cmd/import.go +++ b/cmd/import.go @@ -46,10 +46,8 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command specificationFiles := args[0] - // Initialize config from command options. config.InsecureTLS = globalClientOpts.InsecureTLS config.CaCertPaths = globalClientOpts.CaCertPaths - config.Verbose = globalClientOpts.Verbose // Read local config file in case we need some context info. localConfig, err := config.ReadLocalConfig(globalClientOpts.ConfigPath) diff --git a/cmd/importDir.go b/cmd/importDir.go index 16a9157..34f3e4d 100644 --- a/cmd/importDir.go +++ b/cmd/importDir.go @@ -126,7 +126,6 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm config.InsecureTLS = globalClientOpts.InsecureTLS config.CaCertPaths = globalClientOpts.CaCertPaths - config.Verbose = globalClientOpts.Verbose localConfig, err := config.ReadLocalConfig(globalClientOpts.ConfigPath) if err != nil { diff --git a/cmd/importURL.go b/cmd/importURL.go index 69c32e8..c5c8dc5 100644 --- a/cmd/importURL.go +++ b/cmd/importURL.go @@ -43,7 +43,6 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm config.InsecureTLS = globalClientOpts.InsecureTLS config.CaCertPaths = globalClientOpts.CaCertPaths - config.Verbose = globalClientOpts.Verbose var mc connectors.MicrocksClient diff --git a/cmd/login.go b/cmd/login.go index bd24415..90bc17b 100644 --- a/cmd/login.go +++ b/cmd/login.go @@ -69,7 +69,6 @@ microcks login http://localhost:8080 --sso --sso-launch-browser=false config.InsecureTLS = globalClientOpts.InsecureTLS config.CaCertPaths = globalClientOpts.CaCertPaths - config.Verbose = globalClientOpts.Verbose server = args[0] mc := connectors.NewMicrocksClient(server) diff --git a/cmd/test.go b/cmd/test.go index 70ecb4f..f47f0fa 100644 --- a/cmd/test.go +++ b/cmd/test.go @@ -83,7 +83,6 @@ func NewTestCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command { // Collect optional HTTPS transport flags. config.InsecureTLS = globalClientOpts.InsecureTLS config.CaCertPaths = globalClientOpts.CaCertPaths - config.Verbose = globalClientOpts.Verbose // Compute time to wait in milliseconds. var waitForMilliseconds int64 From e97db23b8f9156b7773ba1830c94db5783fe0e99 Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 12 May 2026 02:36:25 +0530 Subject: [PATCH 3/4] fix: verbose guard in loggingTransport and wire flag through all paths - Add verbose bool field to loggingTransport; RoundTrip is a no-op when verbose is false so the transport is always safe to install - Add verbose param to NewMicrocksClient and NewKeycloakClient so the direct-auth code path (--microcksURL + --keycloakClientId) also logs when -v is passed - Pass c.Verbose into the internal keycloak client used for token refresh so logging is consistent across the whole session - Remove the local --verbose flag from import-dir (it conflicted with the root persistent flag); globalClientOpts.Verbose now drives both HTTP logging and the file-listing output - Fix TestLoggingTransportSilentWhenVerboseFalse: read stderr before closing the pipe and assert zero bytes written - Drop deprecated filepath.HasPrefix in importDir_test.go - Remove the benchmark that was noise for this PR Signed-off-by: Aditya --- cmd/import.go | 4 ++-- cmd/importDir.go | 6 ++--- cmd/importDir_test.go | 31 ++------------------------ cmd/importURL.go | 4 ++-- cmd/login.go | 10 ++++----- cmd/test.go | 4 ++-- pkg/connectors/keycloak_client.go | 14 ++++++------ pkg/connectors/microcks_client.go | 15 +++++++++---- pkg/connectors/microcks_client_test.go | 15 +++++++++---- pkg/watcher/executor.go | 2 +- 10 files changed, 45 insertions(+), 60 deletions(-) diff --git a/cmd/import.go b/cmd/import.go index 2e0bc89..3d58828 100644 --- a/cmd/import.go +++ b/cmd/import.go @@ -61,7 +61,7 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command if globalClientOpts.ServerAddr != "" && globalClientOpts.ClientId != "" && globalClientOpts.ClientSecret != "" { // Create client with server address. - mc = connectors.NewMicrocksClient(globalClientOpts.ServerAddr) + mc = connectors.NewMicrocksClient(globalClientOpts.ServerAddr, globalClientOpts.Verbose) keycloakURL, err := mc.GetKeycloakURL() if err != nil { @@ -72,7 +72,7 @@ func NewImportCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command var oauthToken string = "unauthenticated-token" if keycloakURL != "null" { // If Keycloak is enabled, retrieve an OAuth token using Keycloak Client. - kc := connectors.NewKeycloakClient(keycloakURL, globalClientOpts.ClientId, globalClientOpts.ClientSecret) + kc := connectors.NewKeycloakClient(keycloakURL, globalClientOpts.ClientId, globalClientOpts.ClientSecret, globalClientOpts.Verbose) oauthToken, err = kc.ConnectAndGetToken() if err != nil { diff --git a/cmd/importDir.go b/cmd/importDir.go index 34f3e4d..4f7b2b0 100644 --- a/cmd/importDir.go +++ b/cmd/importDir.go @@ -99,7 +99,6 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm var ( recursive bool pattern string - verbose bool ) var importDirCmd = &cobra.Command{ @@ -154,7 +153,7 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm importConfig := ImportConfig{ Recursive: recursive, Pattern: pattern, - Verbose: verbose, + Verbose: globalClientOpts.Verbose, } // Execute business logic @@ -169,7 +168,7 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm } // Display results - if verbose { + if globalClientOpts.Verbose { fmt.Printf("Found %d specification files to import...\n", result.TotalFiles) for i, file := range result.SuccessFiles { fmt.Printf("[%d/%d] ✓ Imported: %s\n", i+1, result.TotalFiles, file) @@ -201,7 +200,6 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm importDirCmd.Flags().BoolVar(&recursive, "recursive", false, "Scan subdirectories recursively") importDirCmd.Flags().StringVar(&pattern, "pattern", "", "File pattern to match (e.g., '*.yaml', 'openapi.*')") - importDirCmd.Flags().BoolVar(&verbose, "verbose", false, "Show detailed progress") return importDirCmd } diff --git a/cmd/importDir_test.go b/cmd/importDir_test.go index 5693880..e10670e 100644 --- a/cmd/importDir_test.go +++ b/cmd/importDir_test.go @@ -19,12 +19,12 @@ import ( "fmt" "os" "path/filepath" + "strings" "testing" "time" "github.com/microcks/microcks-cli/pkg/connectors" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) type MockMicrocksClient struct { @@ -68,7 +68,7 @@ func (m *MockFileSystem) Walk(root string, walkFn filepath.WalkFunc) error { } for path, isDir := range m.Files { - if filepath.HasPrefix(path, root) { + if strings.HasPrefix(path, root) { info := &MockFileInfo{name: filepath.Base(path), isDir: isDir} if err := walkFn(path, info, nil); err != nil { return err @@ -374,10 +374,6 @@ func TestNewImportDirCommand(t *testing.T) { patternFlag := cmd.Flags().Lookup("pattern") assert.NotNil(t, patternFlag) assert.Equal(t, "string", patternFlag.Value.Type()) - - verboseFlag := cmd.Flags().Lookup("verbose") - assert.NotNil(t, verboseFlag) - assert.Equal(t, "bool", verboseFlag.Value.Type()) } // TestImportResult tests the ImportResult struct @@ -398,26 +394,3 @@ func TestImportResult(t *testing.T) { assert.Len(t, result.FailedFiles, 2) assert.Len(t, result.Errors, 2) } - -// Benchmark tests for performance -func BenchmarkImportDirectory(b *testing.B) { - // Create a mock with many files - mockClient := &MockMicrocksClient{} - mockFS := &MockFileSystem{ - Files: make(map[string]bool), - } - - // Add 100 test files - for i := 0; i < 100; i++ { - path := fmt.Sprintf("/test/spec_%d.yaml", i) - mockFS.Files[path] = false - } - - config := ImportConfig{Recursive: false, Pattern: ""} - - b.ResetTimer() - for i := 0; i < b.N; i++ { - _, err := ImportDirectory(mockClient, mockFS, "/test", config) - require.NoError(b, err) - } -} diff --git a/cmd/importURL.go b/cmd/importURL.go index c5c8dc5..db772a6 100644 --- a/cmd/importURL.go +++ b/cmd/importURL.go @@ -48,7 +48,7 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm if globalClientOpts.ServerAddr != "" && globalClientOpts.ClientId != "" && globalClientOpts.ClientSecret != "" { // create client with server address - mc = connectors.NewMicrocksClient(globalClientOpts.ServerAddr) + mc = connectors.NewMicrocksClient(globalClientOpts.ServerAddr, globalClientOpts.Verbose) keycloakURL, err := mc.GetKeycloakURL() if err != nil { @@ -59,7 +59,7 @@ func NewImportURLCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm var oauthToken string = "unauthenticated-token" if keycloakURL != "null" { // If Keycloak is enabled, retrieve an OAuth token using Keycloak Client. - kc := connectors.NewKeycloakClient(keycloakURL, globalClientOpts.ClientId, globalClientOpts.ClientSecret) + kc := connectors.NewKeycloakClient(keycloakURL, globalClientOpts.ClientId, globalClientOpts.ClientSecret, globalClientOpts.Verbose) oauthToken, err = kc.ConnectAndGetToken() if err != nil { diff --git a/cmd/login.go b/cmd/login.go index 90bc17b..900dd81 100644 --- a/cmd/login.go +++ b/cmd/login.go @@ -71,7 +71,7 @@ microcks login http://localhost:8080 --sso --sso-launch-browser=false config.CaCertPaths = globalClientOpts.CaCertPaths server = args[0] - mc := connectors.NewMicrocksClient(server) + mc := connectors.NewMicrocksClient(server, globalClientOpts.Verbose) keycloakUrl, err := mc.GetKeycloakURL() if err != nil { @@ -119,13 +119,13 @@ microcks login http://localhost:8080 --sso --sso-launch-browser=false os.Exit(1) } //Perform login and retrive tokens - authToken, refreshToken = passwordLogin(keycloakUrl, clientID, clientSecret, username, password) + authToken, refreshToken = passwordLogin(keycloakUrl, clientID, clientSecret, username, password, globalClientOpts.Verbose) authCfg.ClientId = clientID authCfg.ClientSecret = clientSecret } else { httpClient := mc.HttpClient() ctx = oidc.ClientContext(ctx, httpClient) - kc := connectors.NewKeycloakClient(keycloakUrl, "", "") + kc := connectors.NewKeycloakClient(keycloakUrl, "", "", globalClientOpts.Verbose) oauth2conf, err := kc.GetOIDCConfig() errors.CheckError(err) authToken, refreshToken = oauth2login(ctx, ssoProt, oauth2conf, ssoLaunchBrowser) @@ -307,8 +307,8 @@ func ssoAuthFlow(url string, ssoLaunchBrowser bool) { } } -func passwordLogin(keycloakURL, clientId, clientSecret, Username, Password string) (string, string) { - kc := connectors.NewKeycloakClient(keycloakURL, clientId, clientSecret) +func passwordLogin(keycloakURL, clientId, clientSecret, Username, Password string, verbose bool) (string, string) { + kc := connectors.NewKeycloakClient(keycloakURL, clientId, clientSecret, verbose) username, password := promptCredentials(Username, Password) authToken, refreshToken, err := kc.ConnectAndGetTokenAndRefreshToken(username, password) diff --git a/cmd/test.go b/cmd/test.go index f47f0fa..92f1248 100644 --- a/cmd/test.go +++ b/cmd/test.go @@ -116,7 +116,7 @@ func NewTestCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command { // create client with server address serverAddr = globalClientOpts.ServerAddr - mc = connectors.NewMicrocksClient(serverAddr) + mc = connectors.NewMicrocksClient(serverAddr, globalClientOpts.Verbose) keycloakURL, err := mc.GetKeycloakURL() if err != nil { @@ -127,7 +127,7 @@ func NewTestCommand(globalClientOpts *connectors.ClientOptions) *cobra.Command { var oauthToken string = "unauthenticated-token" if keycloakURL != "null" { // If Keycloak is enabled, retrieve an OAuth token using Keycloak Client. - kc := connectors.NewKeycloakClient(keycloakURL, globalClientOpts.ClientId, globalClientOpts.ClientSecret) + kc := connectors.NewKeycloakClient(keycloakURL, globalClientOpts.ClientId, globalClientOpts.ClientSecret, globalClientOpts.Verbose) oauthToken, err = kc.ConnectAndGetToken() if err != nil { diff --git a/pkg/connectors/keycloak_client.go b/pkg/connectors/keycloak_client.go index 4ee6af0..f2b0b18 100644 --- a/pkg/connectors/keycloak_client.go +++ b/pkg/connectors/keycloak_client.go @@ -46,7 +46,7 @@ type keycloakClient struct { } // NewKeycloakClient build a new KeycloakClient implementation -func NewKeycloakClient(realmURL string, username string, password string) KeycloakClient { +func NewKeycloakClient(realmURL string, username string, password string, verbose bool) KeycloakClient { kc := keycloakClient{} u, err := url.Parse(realmURL) @@ -57,15 +57,15 @@ func NewKeycloakClient(realmURL string, username string, password string) Keyclo kc.Username = username kc.Password = password + var transport http.RoundTripper = http.DefaultTransport if config.InsecureTLS || len(config.CaCertPaths) > 0 { tlsConfig := config.CreateTLSConfig() - tr := &http.Transport{ - TLSClientConfig: tlsConfig, - } - kc.httpClient = &http.Client{Transport: tr} - } else { - kc.httpClient = http.DefaultClient + transport = &http.Transport{TLSClientConfig: tlsConfig} } + if verbose { + transport = &loggingTransport{transport: transport, verbose: true} + } + kc.httpClient = &http.Client{Transport: transport} return &kc } diff --git a/pkg/connectors/microcks_client.go b/pkg/connectors/microcks_client.go index c5502ce..9fe19c9 100644 --- a/pkg/connectors/microcks_client.go +++ b/pkg/connectors/microcks_client.go @@ -45,9 +45,13 @@ var ( type loggingTransport struct { transport http.RoundTripper + verbose bool } func (l *loggingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + if !l.verbose { + return l.transport.RoundTrip(req) + } fmt.Fprintf(os.Stderr, ">> %s %s\n", req.Method, req.URL.String()) for k, v := range req.Header { if strings.EqualFold(k, "authorization") { @@ -178,7 +182,7 @@ func NewClient(opts ClientOptions) (MicrocksClient, error) { transport = &http.Transport{TLSClientConfig: tlsConfig} } if c.Verbose { - transport = &loggingTransport{transport: transport} + transport = &loggingTransport{transport: transport, verbose: true} } c.httpClient = &http.Client{Transport: transport} @@ -192,8 +196,8 @@ func NewClient(opts ClientOptions) (MicrocksClient, error) { } // NewMicrocksClient builds a new headless MicrocksClient without any authtoken and all for general purposes -func NewMicrocksClient(apiURL string) MicrocksClient { - mc := microcksClient{} +func NewMicrocksClient(apiURL string, verbose bool) MicrocksClient { + mc := microcksClient{Verbose: verbose} if strings.HasSuffix(apiURL, "/api") { apiURL += "/" @@ -213,6 +217,9 @@ func NewMicrocksClient(apiURL string) MicrocksClient { tlsConfig := config.CreateTLSConfig() transport = &http.Transport{TLSClientConfig: tlsConfig} } + if verbose { + transport = &loggingTransport{transport: transport, verbose: true} + } mc.httpClient = &http.Client{Transport: transport} return &mc } @@ -306,7 +313,7 @@ func (c *microcksClient) refreshAuthToken(localCfg *config.LocalConfig, ctxName, func (c *microcksClient) redeemRefreshToken(auth config.Auth) (string, string, error) { keyCloakUrl, err := c.GetKeycloakURL() errors.CheckError(err) - kc := NewKeycloakClient(keyCloakUrl, "", "") + kc := NewKeycloakClient(keyCloakUrl, "", "", c.Verbose) oauth2Conf, err := kc.GetOIDCConfig() errors.CheckError(err) oauth2Conf.ClientID = auth.ClientId diff --git a/pkg/connectors/microcks_client_test.go b/pkg/connectors/microcks_client_test.go index 5928ec5..164cefa 100644 --- a/pkg/connectors/microcks_client_test.go +++ b/pkg/connectors/microcks_client_test.go @@ -32,7 +32,7 @@ func TestDownloadArtifactReturnsResponseBody(t *testing.T) { })) defer server.Close() - client := NewMicrocksClient(server.URL) + client := NewMicrocksClient(server.URL, false) msg, err := client.DownloadArtifact("https://example.com/openapi.yaml", true, "") if err != nil { @@ -43,14 +43,14 @@ func TestDownloadArtifactReturnsResponseBody(t *testing.T) { } } -func TestLoggingTransportPassesThrough(t *testing.T) { +func TestLoggingTransportSilentWhenVerboseFalse(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) defer server.Close() req, _ := http.NewRequest("GET", server.URL, nil) - lt := &loggingTransport{transport: http.DefaultTransport} + lt := &loggingTransport{transport: http.DefaultTransport, verbose: false} old := os.Stderr pr, pw, _ := os.Pipe() @@ -60,6 +60,9 @@ func TestLoggingTransportPassesThrough(t *testing.T) { pw.Close() os.Stderr = old + + buf := make([]byte, 4096) + n, _ := pr.Read(buf) pr.Close() if err != nil { @@ -68,6 +71,9 @@ func TestLoggingTransportPassesThrough(t *testing.T) { if resp.StatusCode != http.StatusOK { t.Fatalf("expected 200, got %d", resp.StatusCode) } + if n > 0 { + t.Fatalf("expected no stderr output when verbose=false, got: %q", string(buf[:n])) + } } func TestLoggingTransportRedactsAuth(t *testing.T) { @@ -83,7 +89,7 @@ func TestLoggingTransportRedactsAuth(t *testing.T) { pr, pw, _ := os.Pipe() os.Stderr = pw - lt := &loggingTransport{transport: http.DefaultTransport} + lt := &loggingTransport{transport: http.DefaultTransport, verbose: true} _, _ = lt.RoundTrip(req) pw.Close() @@ -91,6 +97,7 @@ func TestLoggingTransportRedactsAuth(t *testing.T) { buf := make([]byte, 4096) n, _ := pr.Read(buf) + pr.Close() out := string(buf[:n]) if strings.Contains(out, "supersecret") { diff --git a/pkg/watcher/executor.go b/pkg/watcher/executor.go index 3347fc6..42861ca 100644 --- a/pkg/watcher/executor.go +++ b/pkg/watcher/executor.go @@ -35,7 +35,7 @@ func TriggerImport(entry config.WatchEntry) { } } else { // We have no config file, so just create a client with context as server URL. - mc = connectors.NewMicrocksClient(context) + mc = connectors.NewMicrocksClient(context, false) } _, err = mc.UploadArtifact(entry.FilePath, entry.MainArtifact) From 775349bbe1e9e64b809e1d60239c7c27ca3098ce Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 12 May 2026 02:43:45 +0530 Subject: [PATCH 4/4] fix: polish import-dir and executor before PR - Remove dead ImportError type from importDir.go - Add direct-auth path (--microcksURL + keycloak flags) to import-dir so it matches the other commands (import, test, import-url) - Redirect verbose diagnostic output to stderr per issue spec - Fix silent fmt.Errorf in executor.go (was swallowing the error) replaced with fmt.Fprintf(os.Stderr, ...) Signed-off-by: Aditya --- cmd/importDir.go | 76 +++++++++++++++++++++++------------------ pkg/watcher/executor.go | 2 +- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/cmd/importDir.go b/cmd/importDir.go index 4f7b2b0..7bc1daf 100644 --- a/cmd/importDir.go +++ b/cmd/importDir.go @@ -78,19 +78,10 @@ var supportedExtensions = map[string]bool{ ".xml": true, } -type ImportError struct { - File string - Err error -} - type ValidationError struct { Message string } -func (e ImportError) Error() string { - return fmt.Sprintf("failed to import %s: %v", e.File, e.Err) -} - func (e ValidationError) Error() string { return e.Message } @@ -126,26 +117,45 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm config.InsecureTLS = globalClientOpts.InsecureTLS config.CaCertPaths = globalClientOpts.CaCertPaths - localConfig, err := config.ReadLocalConfig(globalClientOpts.ConfigPath) - if err != nil { - fmt.Println(err) - return - } + var mc connectors.MicrocksClient - if localConfig == nil { - fmt.Println("Please login to perform operation...") - return - } + if globalClientOpts.ServerAddr != "" && globalClientOpts.ClientId != "" && globalClientOpts.ClientSecret != "" { + mc = connectors.NewMicrocksClient(globalClientOpts.ServerAddr, globalClientOpts.Verbose) - if globalClientOpts.Context == "" { - globalClientOpts.Context = localConfig.CurrentContext - } + keycloakURL, err := mc.GetKeycloakURL() + if err != nil { + fmt.Printf("Got error when invoking Microcks client retrieving config: %s", err) + os.Exit(1) + } - // Create client - mc, err := connectors.NewClient(*globalClientOpts) - if err != nil { - fmt.Printf("error %v", err) - return + var oauthToken string = "unauthenticated-token" + if keycloakURL != "null" { + kc := connectors.NewKeycloakClient(keycloakURL, globalClientOpts.ClientId, globalClientOpts.ClientSecret, globalClientOpts.Verbose) + oauthToken, err = kc.ConnectAndGetToken() + if err != nil { + fmt.Printf("Got error when invoking Keycloak client: %s", err) + os.Exit(1) + } + } + mc.SetOAuthToken(oauthToken) + } else { + localConfig, err := config.ReadLocalConfig(globalClientOpts.ConfigPath) + if err != nil { + fmt.Println(err) + return + } + if localConfig == nil { + fmt.Println("Please login to perform operation...") + return + } + if globalClientOpts.Context == "" { + globalClientOpts.Context = localConfig.CurrentContext + } + mc, err = connectors.NewClient(*globalClientOpts) + if err != nil { + fmt.Printf("error %v", err) + return + } } // Set up business logic dependencies @@ -169,28 +179,28 @@ func NewImportDirCommand(globalClientOpts *connectors.ClientOptions) *cobra.Comm // Display results if globalClientOpts.Verbose { - fmt.Printf("Found %d specification files to import...\n", result.TotalFiles) + fmt.Fprintf(os.Stderr, "Found %d specification files to import...\n", result.TotalFiles) for i, file := range result.SuccessFiles { - fmt.Printf("[%d/%d] ✓ Imported: %s\n", i+1, result.TotalFiles, file) + fmt.Fprintf(os.Stderr, "[%d/%d] Imported: %s\n", i+1, result.TotalFiles, file) } for i, file := range result.FailedFiles { - errorMsg := "Unknown error" + errorMsg := "unknown error" if i < len(result.Errors) { errorMsg = result.Errors[i] } - fmt.Printf("✗ Failed: %s - %s\n", file, errorMsg) + fmt.Fprintf(os.Stderr, "Failed: %s - %s\n", file, errorMsg) } } else { fmt.Println("\nImport results:") for _, file := range result.SuccessFiles { - fmt.Printf("✓ Imported: %s\n", file) + fmt.Printf("Imported: %s\n", file) } for i, file := range result.FailedFiles { - errorMsg := "Unknown error" + errorMsg := "unknown error" if i < len(result.Errors) { errorMsg = result.Errors[i] } - fmt.Printf("✗ Failed: %s - %s\n", file, errorMsg) + fmt.Printf("Failed: %s - %s\n", file, errorMsg) } } diff --git a/pkg/watcher/executor.go b/pkg/watcher/executor.go index 42861ca..5ba1bd6 100644 --- a/pkg/watcher/executor.go +++ b/pkg/watcher/executor.go @@ -12,7 +12,7 @@ func TriggerImport(entry config.WatchEntry) { // Retrieve config to get client options. cfgPath, err := config.DefaultLocalConfigPath() if err != nil { - fmt.Errorf("Error while loading config: %s", err.Error()) + fmt.Fprintf(os.Stderr, "Error while loading config: %s\n", err.Error()) } fmt.Println("[INFO] Re-importing changed file: " + entry.FilePath)