From 1d420d2e060366ecb0779d22ccbf146a7b5c000f Mon Sep 17 00:00:00 2001 From: Fabien FLEUREAU Date: Wed, 11 Mar 2026 11:32:46 +0100 Subject: [PATCH 1/3] feat(api): add qovery api command for authenticated HTTP requests Implements the `qovery api ` command with --method, --input, --field, --header, and --include flags. Adds validateAPIArgs() and writeResponse() helpers for testable pure-logic validation and response routing. Fixes code review findings: removes file-path --input variant (stdin-only), adds duplicate --field key detection, restores conditional server config in GetQoveryClient, and rewrites tests to exercise real code paths. --- cmd/api.go | 285 +++++++++++++++++++++++++++++ cmd/api_test.go | 475 ++++++++++++++++++++++++++++++++++++++++++++++++ utils/qovery.go | 14 +- 3 files changed, 768 insertions(+), 6 deletions(-) create mode 100644 cmd/api.go create mode 100644 cmd/api_test.go diff --git a/cmd/api.go b/cmd/api.go new file mode 100644 index 00000000..91c9ec30 --- /dev/null +++ b/cmd/api.go @@ -0,0 +1,285 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "os" + "sort" + "strconv" + "strings" + "time" + + "github.com/spf13/cobra" + + "github.com/qovery/qovery-cli/utils" +) + +var apiMethod string +var apiInput string +var apiFields []string +var apiHeaders []string +var apiInclude bool + +var apiCmd = &cobra.Command{ + Use: "api ", + Short: "Make an authenticated request to the Qovery API", + Long: `Make an authenticated HTTP request to the Qovery API. + +EXAMPLES + + # List organizations + $ qovery api organization + + # Get a specific organization + $ qovery api organization/ + + # List projects in current organization (from context) + $ qovery api organization/{organizationId}/project + + # Get current environment's services (fully from context) + $ qovery api organization/{organizationId}/project/{projectId}/environment/{environmentId}/service + + # Create an organization using --field + $ qovery api organization --field name=my-org --field plan=FREE + + # Pipe body from stdin + $ echo '{"name":"my-org","plan":"FREE"}' | qovery api organization --input - + + # Send a JSON file as body + $ qovery api organization//project --input - < body.json + + # Delete a resource + $ qovery api organization/ --method DELETE + + # Show response headers + $ qovery api organization --include + + # Add a custom header + $ qovery api organization -H "X-Request-Id: abc123" + + # Use a staging environment + $ QOVERY_API_URL=https://staging.api.qovery.com qovery api organization`, + Args: cobra.ExactArgs(1), + Run: runAPI, +} + +func init() { + rootCmd.AddCommand(apiCmd) + apiCmd.Flags().StringVarP(&apiMethod, "method", "X", "", "HTTP method (GET, POST, PUT, PATCH, DELETE)") + apiCmd.Flags().StringVar(&apiInput, "input", "", "Body: '-' for stdin (pipe JSON to command)") + apiCmd.Flags().StringArrayVarP(&apiFields, "field", "f", []string{}, "Add a key=value pair to the JSON body (repeatable, smart type coercion)") + apiCmd.Flags().StringArrayVarP(&apiHeaders, "header", "H", []string{}, "Additional request headers in 'Key: Value' format (repeatable)") + apiCmd.Flags().BoolVarP(&apiInclude, "include", "i", false, "Print HTTP response status and headers before body") +} + +// validateAPIArgs validates all arguments and flag values before any I/O. +// It returns an error describing the first problem found. +func validateAPIArgs(endpoint, method, input string, fields, headers []string) error { + if strings.HasPrefix(endpoint, "http://") || strings.HasPrefix(endpoint, "https://") { + return errors.New("endpoint must be a path (e.g. /organization), not a full URL") + } + if input != "" && input != "-" { + return errors.New(`--input only accepts '-' (stdin); to send a file: qovery api --input - < file.json`) + } + if len(fields) > 0 && input != "" { + return errors.New("--field and --input are mutually exclusive") + } + allowed := map[string]bool{"GET": true, "POST": true, "PUT": true, "PATCH": true, "DELETE": true} + if method != "" && !allowed[method] { + return fmt.Errorf("invalid HTTP method %q: must be one of GET, POST, PUT, PATCH, DELETE", method) + } + for _, h := range headers { + if strings.Index(h, ": ") == -1 { + return fmt.Errorf("invalid header %q: must be in 'Key: Value' format", h) + } + } + seen := make(map[string]bool) + for _, f := range fields { + idx := strings.Index(f, "=") + if idx == -1 { + return fmt.Errorf("invalid field %q: must be in 'key=value' format", f) + } + key := f[:idx] + if seen[key] { + return fmt.Errorf("duplicate field key %q: each key may only appear once", key) + } + seen[key] = true + } + return nil +} + +// writeResponse writes the response body to stdout (2xx) or stderr (non-2xx). +// Returns true on success (2xx), false on error response. +func writeResponse(resp *http.Response, include bool, stdout, stderr io.Writer) (bool, error) { + if include { + fmt.Fprintf(stdout, "HTTP/%d.%d %s\n", resp.ProtoMajor, resp.ProtoMinor, resp.Status) + headerKeys := make([]string, 0, len(resp.Header)) + for k := range resp.Header { + headerKeys = append(headerKeys, k) + } + sort.Strings(headerKeys) + for _, k := range headerKeys { + for _, v := range resp.Header[k] { + fmt.Fprintf(stdout, "%s: %s\n", k, v) + } + } + fmt.Fprintln(stdout) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + return false, err + } + if resp.StatusCode >= 200 && resp.StatusCode < 300 { + _, _ = stdout.Write(body) + return true, nil + } + _, _ = stderr.Write(body) + return false, nil +} + +// substitutePathPlaceholders replaces {organizationId}, {projectId}, {environmentId}, {serviceId} +// in the path with values from the current Qovery context (best-effort — errors silently ignored). +// Empty context values leave the literal placeholder unchanged. +func substitutePathPlaceholders(path string) string { + ctx, _ := utils.GetCurrentContext() + pairs := []struct { + placeholder string + value string + }{ + {"{organizationId}", string(ctx.OrganizationId)}, + {"{projectId}", string(ctx.ProjectId)}, + {"{environmentId}", string(ctx.EnvironmentId)}, + {"{serviceId}", string(ctx.ServiceId)}, + } + for _, p := range pairs { + if p.value != "" { + path = strings.ReplaceAll(path, p.placeholder, p.value) + } + } + return path +} + +// coerceFieldValue applies smart type coercion for --field values. +// Order: bool → int64 → float64 → string. +func coerceFieldValue(v string) any { + if v == "true" { + return true + } + if v == "false" { + return false + } + if i, err := strconv.ParseInt(v, 10, 64); err == nil { + return i + } + if f, err := strconv.ParseFloat(v, 64); err == nil { + return f + } + return v +} + +func runAPI(cmd *cobra.Command, args []string) { + endpoint := args[0] + + if err := validateAPIArgs(endpoint, apiMethod, apiInput, apiFields, apiHeaders); err != nil { + utils.PrintlnError(err) + os.Exit(1) + } + + // Parse headers (format already validated) + parsedHeaders := make(map[string]string) + for _, h := range apiHeaders { + idx := strings.Index(h, ": ") + parsedHeaders[h[:idx]] = h[idx+2:] + } + + // Parse fields (format already validated) + parsedFields := make(map[string]string) + for _, f := range apiFields { + idx := strings.Index(f, "=") + parsedFields[f[:idx]] = f[idx+1:] + } + + // Determine effective HTTP method + method := apiMethod + if method == "" { + if apiInput != "" || len(apiFields) > 0 { + method = "POST" + } else { + method = "GET" + } + } + + // Build the full URL + path := strings.TrimLeft(endpoint, "/") + path = substitutePathPlaceholders(path) + fullURL := utils.GetAPIBaseURL() + "/" + path + + // Build request body + var body io.Reader + hasBody := apiInput != "" || len(apiFields) > 0 + + switch { + case apiInput == "-": + body = os.Stdin + case len(apiFields) > 0: + fieldMap := make(map[string]any, len(parsedFields)) + for k, v := range parsedFields { + fieldMap[k] = coerceFieldValue(v) + } + jsonBytes, err := json.Marshal(fieldMap) + if err != nil { + utils.PrintlnError(err) + os.Exit(1) + } + body = bytes.NewReader(jsonBytes) + } + + // Create HTTP request + req, err := http.NewRequest(method, fullURL, body) + if err != nil { + utils.PrintlnError(err) + os.Exit(1) + } + + // Get auth token + tokenType, token, err := utils.GetAccessToken() + if err != nil { + utils.PrintlnError(err) + os.Exit(1) + } + + // Set Authorization header + req.Header.Set("Authorization", utils.GetAuthorizationHeaderValue(tokenType, token)) + + // Set default Content-Type when body is expected (flag presence check, not body-nil check) + if hasBody { + req.Header.Set("Content-Type", "application/json") + } + + // Apply user headers (always wins — applied after defaults) + for k, v := range parsedHeaders { + req.Header.Set(k, v) + } + + // Execute request with 60s timeout + client := &http.Client{Timeout: 60 * time.Second} + resp, err := client.Do(req) + if err != nil { + utils.PrintlnError(err) + os.Exit(1) + } + defer resp.Body.Close() + + ok, err := writeResponse(resp, apiInclude, os.Stdout, os.Stderr) + if err != nil { + utils.PrintlnError(err) + os.Exit(1) + } + if !ok { + os.Exit(1) + } +} diff --git a/cmd/api_test.go b/cmd/api_test.go new file mode 100644 index 00000000..c876ab75 --- /dev/null +++ b/cmd/api_test.go @@ -0,0 +1,475 @@ +package cmd + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/jarcoal/httpmock" + "github.com/stretchr/testify/assert" + + "github.com/qovery/qovery-cli/utils" +) + +// captureOutput temporarily replaces os.Stdout and os.Stderr and returns the +// data written to each after the function returns. +func captureOutput(fn func()) (stdout string, stderr string) { + oldOut := os.Stdout + oldErr := os.Stderr + defer func() { + os.Stdout = oldOut + os.Stderr = oldErr + }() + + rOut, wOut, _ := os.Pipe() + rErr, wErr, _ := os.Pipe() + os.Stdout = wOut + os.Stderr = wErr + + fn() + + wOut.Close() + wErr.Close() + + outBuf, _ := io.ReadAll(rOut) + errBuf, _ := io.ReadAll(rErr) + return string(outBuf), string(errBuf) +} + +// writeContextFile creates a minimal ~/.qovery/context.json in a temp HOME dir, +// sets HOME to that dir, and returns a cleanup func. +func writeContextFile(t *testing.T, orgID, projectID, envID, serviceID string) { + t.Helper() + tmpHome := t.TempDir() + qoveryDir := filepath.Join(tmpHome, ".qovery") + if err := os.MkdirAll(qoveryDir, 0700); err != nil { + t.Fatal(err) + } + contextData := fmt.Sprintf(`{ + "access_token": "fake", + "access_token_expiration": "2099-01-01T00:00:00Z", + "refresh_token": "fake", + "organization_id": %q, + "project_id": %q, + "environment_id": %q, + "service_id": %q + }`, orgID, projectID, envID, serviceID) + if err := os.WriteFile(filepath.Join(qoveryDir, "context.json"), []byte(contextData), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("HOME", tmpHome) +} + +// --- Scenario 1: GET 200 — body written to stdout --- +func TestAPIGet200(t *testing.T) { + t.Setenv("QOVERY_CLI_ACCESS_TOKEN", "fake-token") + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + expected := `{"results":[]}` + httpmock.RegisterResponder("GET", "https://api.qovery.com/organization", + httpmock.NewStringResponder(200, expected)) + + // Reset flag state + apiMethod = "" + apiInput = "" + apiFields = []string{} + apiHeaders = []string{} + apiInclude = false + + stdout, _ := captureOutput(func() { + runAPI(apiCmd, []string{"organization"}) + }) + + assert.Equal(t, expected, stdout) +} + +// --- Scenario 2: POST with stdin body --- +func TestAPIPostStdin(t *testing.T) { + t.Setenv("QOVERY_CLI_ACCESS_TOKEN", "fake-token") + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + requestBody := `{"name":"my-org","plan":"FREE"}` + var capturedBody string + httpmock.RegisterResponder("POST", "https://api.qovery.com/organization", + func(req *http.Request) (*http.Response, error) { + b, _ := io.ReadAll(req.Body) + capturedBody = string(b) + return httpmock.NewStringResponse(200, `{"id":"123"}`), nil + }) + + // Replace stdin + r, w, _ := os.Pipe() + _, _ = w.WriteString(requestBody) + w.Close() + oldStdin := os.Stdin + os.Stdin = r + defer func() { os.Stdin = oldStdin }() + + apiMethod = "POST" + apiInput = "-" + apiFields = []string{} + apiHeaders = []string{} + apiInclude = false + + captureOutput(func() { + runAPI(apiCmd, []string{"organization"}) + }) + + assert.Equal(t, requestBody, capturedBody) +} + +// --- Scenario 3: --input with file path is rejected --- +func TestAPIInputFilePathRejected(t *testing.T) { + err := validateAPIArgs("organization", "", "body.json", nil, nil) + assert.ErrorContains(t, err, "--input only accepts") +} + +// --- Scenario 4: DELETE method --- +func TestAPIDelete(t *testing.T) { + t.Setenv("QOVERY_CLI_ACCESS_TOKEN", "fake-token") + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + var capturedMethod string + httpmock.RegisterResponder("DELETE", "https://api.qovery.com/organization/abc", + func(req *http.Request) (*http.Response, error) { + capturedMethod = req.Method + return httpmock.NewStringResponse(200, ``), nil + }) + + apiMethod = "DELETE" + apiInput = "" + apiFields = []string{} + apiHeaders = []string{} + apiInclude = false + + stdout, _ := captureOutput(func() { + runAPI(apiCmd, []string{"organization/abc"}) + }) + + assert.Equal(t, "DELETE", capturedMethod) + assert.Equal(t, "", stdout) // DELETE 200 with empty body → nothing on stdout +} + +// --- Scenario 5: Invalid method (pure unit test via validateAPIArgs) --- +func TestAPIInvalidMethod(t *testing.T) { + assert.ErrorContains(t, validateAPIArgs("organization", "BREW", "", nil, nil), "invalid HTTP method") +} + +// --- Scenario 6: Non-2xx → body to stderr, nothing to stdout --- +func TestAPINon2xx(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + errorBody := `{"status":404,"message":"not found"}` + httpmock.RegisterResponder("GET", "https://api.qovery.com/missing-resource", + httpmock.NewStringResponder(404, errorBody)) + + client := &http.Client{} + req, _ := http.NewRequest("GET", "https://api.qovery.com/missing-resource", nil) + resp, err := client.Do(req) + assert.Nil(t, err) + defer resp.Body.Close() + + var outBuf, errBuf bytes.Buffer + ok, writeErr := writeResponse(resp, false, &outBuf, &errBuf) + assert.Nil(t, writeErr) + assert.False(t, ok) + assert.Equal(t, "", outBuf.String()) // nothing on stdout + assert.Equal(t, errorBody, errBuf.String()) // body on stderr +} + +// --- Scenario 7: --include flag output format --- +func TestAPIIncludeFlag(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + responseBody := `{"results":[]}` + httpmock.RegisterResponder("GET", "https://api.qovery.com/organization", + func(req *http.Request) (*http.Response, error) { + resp := httpmock.NewStringResponse(200, responseBody) + resp.Header.Set("Content-Type", "application/json") + return resp, nil + }) + + client := &http.Client{} + req, _ := http.NewRequest("GET", "https://api.qovery.com/organization", nil) + resp, err := client.Do(req) + assert.Nil(t, err) + defer resp.Body.Close() + + var outBuf, errBuf bytes.Buffer + ok, writeErr := writeResponse(resp, true, &outBuf, &errBuf) + assert.Nil(t, writeErr) + assert.True(t, ok) + + stdout := outBuf.String() + // Must start with HTTP status line + assert.True(t, strings.HasPrefix(stdout, "HTTP/"), "stdout must start with HTTP/ status line, got: %q", stdout) + // Must contain Content-Type header + assert.Contains(t, stdout, "Content-Type: application/json") + // Must contain blank line before body + assert.Contains(t, stdout, "\n\n") + // Must contain body + assert.Contains(t, stdout, responseBody) +} + +// --- Scenario 8: Custom -H header sent in request --- +func TestAPICustomHeader(t *testing.T) { + t.Setenv("QOVERY_CLI_ACCESS_TOKEN", "fake-token") + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + var capturedHeader string + httpmock.RegisterResponder("GET", "https://api.qovery.com/organization", + func(req *http.Request) (*http.Response, error) { + capturedHeader = req.Header.Get("X-Request-Id") + return httpmock.NewStringResponse(200, `{}`), nil + }) + + apiMethod = "" + apiInput = "" + apiFields = []string{} + apiHeaders = []string{"X-Request-Id: abc123"} + apiInclude = false + + captureOutput(func() { + runAPI(apiCmd, []string{"organization"}) + }) + + assert.Equal(t, "abc123", capturedHeader) +} + +// --- Scenario 9: Malformed -H header (pure unit test via validateAPIArgs) --- +func TestAPIMalformedHeader(t *testing.T) { + assert.ErrorContains(t, validateAPIArgs("organization", "", "", nil, []string{"Badheader"}), "invalid header") +} + +// --- Scenario 10: Full URL rejected (pure unit test via validateAPIArgs) --- +func TestAPIFullURLRejected(t *testing.T) { + assert.ErrorContains(t, validateAPIArgs("https://api.qovery.com/organization", "", "", nil, nil), "not a full URL") +} + +// --- Scenario 11: Path normalisation (pure unit test) --- +func TestAPIPathNormalisation(t *testing.T) { + cases := []struct { + input string + expected string + }{ + {"/organization", "https://api.qovery.com/organization"}, + {"organization", "https://api.qovery.com/organization"}, + } + for _, tc := range cases { + path := strings.TrimLeft(tc.input, "/") + result := "https://api.qovery.com" + "/" + path + assert.Equal(t, tc.expected, result) + } +} + +// --- Scenario 12: --field string value --- +func TestAPIFieldString(t *testing.T) { + t.Setenv("QOVERY_CLI_ACCESS_TOKEN", "fake-token") + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + var capturedBody string + httpmock.RegisterResponder("POST", "https://api.qovery.com/organization", + func(req *http.Request) (*http.Response, error) { + b, _ := io.ReadAll(req.Body) + capturedBody = string(b) + return httpmock.NewStringResponse(200, `{}`), nil + }) + + apiMethod = "" + apiInput = "" + apiFields = []string{"name=myorg"} + apiHeaders = []string{} + apiInclude = false + + captureOutput(func() { + runAPI(apiCmd, []string{"organization"}) + }) + + var result map[string]any + _ = json.Unmarshal([]byte(capturedBody), &result) + assert.Equal(t, "myorg", result["name"]) +} + +// --- Scenario 13: --field bool coercion --- +func TestAPIFieldBoolCoercion(t *testing.T) { + t.Setenv("QOVERY_CLI_ACCESS_TOKEN", "fake-token") + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + var capturedBody string + httpmock.RegisterResponder("POST", "https://api.qovery.com/organization", + func(req *http.Request) (*http.Response, error) { + b, _ := io.ReadAll(req.Body) + capturedBody = string(b) + return httpmock.NewStringResponse(200, `{}`), nil + }) + + apiMethod = "" + apiInput = "" + apiFields = []string{"enabled=true"} + apiHeaders = []string{} + apiInclude = false + + captureOutput(func() { + runAPI(apiCmd, []string{"organization"}) + }) + + var result map[string]any + _ = json.Unmarshal([]byte(capturedBody), &result) + assert.Equal(t, true, result["enabled"]) +} + +// --- Scenario 14: --field int coercion --- +func TestAPIFieldIntCoercion(t *testing.T) { + t.Setenv("QOVERY_CLI_ACCESS_TOKEN", "fake-token") + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + var capturedBody string + httpmock.RegisterResponder("POST", "https://api.qovery.com/organization", + func(req *http.Request) (*http.Response, error) { + b, _ := io.ReadAll(req.Body) + capturedBody = string(b) + return httpmock.NewStringResponse(200, `{}`), nil + }) + + apiMethod = "" + apiInput = "" + apiFields = []string{"count=42"} + apiHeaders = []string{} + apiInclude = false + + captureOutput(func() { + runAPI(apiCmd, []string{"organization"}) + }) + + var result map[string]any + _ = json.Unmarshal([]byte(capturedBody), &result) + // After json.Unmarshal into map[string]any, all numbers become float64 + assert.Equal(t, float64(42), result["count"]) +} + +// --- Scenario 15: --field multiple fields --- +func TestAPIFieldMultipleFields(t *testing.T) { + t.Setenv("QOVERY_CLI_ACCESS_TOKEN", "fake-token") + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + var capturedBody string + httpmock.RegisterResponder("POST", "https://api.qovery.com/organization", + func(req *http.Request) (*http.Response, error) { + b, _ := io.ReadAll(req.Body) + capturedBody = string(b) + return httpmock.NewStringResponse(200, `{}`), nil + }) + + apiMethod = "" + apiInput = "" + apiFields = []string{"name=x", "count=1"} + apiHeaders = []string{} + apiInclude = false + + captureOutput(func() { + runAPI(apiCmd, []string{"organization"}) + }) + + var result map[string]any + _ = json.Unmarshal([]byte(capturedBody), &result) + assert.Equal(t, "x", result["name"]) + assert.Equal(t, float64(1), result["count"]) +} + +// --- Scenario 16: --field + --input together (pure unit test via validateAPIArgs) --- +func TestAPIFieldAndInputMutuallyExclusive(t *testing.T) { + assert.ErrorContains(t, validateAPIArgs("organization", "", "-", []string{"name=x"}, nil), "mutually exclusive") +} + +// --- Scenario 17: Malformed --field entry (pure unit test via validateAPIArgs) --- +func TestAPIMalformedField(t *testing.T) { + assert.ErrorContains(t, validateAPIArgs("organization", "", "", []string{"badfield"}, nil), "invalid field") +} + +// --- Scenario 18: Placeholder substitution with org context --- +func TestAPIPlaceholderSubstitution(t *testing.T) { + writeContextFile(t, "org-123", "proj-456", "env-789", "svc-abc") + + result := substitutePathPlaceholders("organization/{organizationId}/project") + assert.Equal(t, "organization/org-123/project", result) +} + +// --- Scenario 19: Missing/empty placeholder left as literal --- +func TestAPIPlaceholderEmptyValue(t *testing.T) { + writeContextFile(t, "org-123", "", "env-789", "svc-abc") + + result := substitutePathPlaceholders("project/{projectId}/env") + assert.Equal(t, "project/{projectId}/env", result) +} + +// --- Scenario 20: Context unavailable — literal preserved --- +func TestAPIPlaceholderContextUnavailable(t *testing.T) { + // Point HOME to a temp dir with no .qovery/context.json + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + + result := substitutePathPlaceholders("organization/{organizationId}/project") + // GetCurrentContext() will error → zero-value context → empty string → literal preserved + assert.Equal(t, "organization/{organizationId}/project", result) +} + +// --- Unit tests for coerceFieldValue --- +func TestCoerceFieldValue(t *testing.T) { + tests := []struct { + input string + expected any + }{ + {"true", true}, + {"false", false}, + {"42", int64(42)}, + {"3.14", float64(3.14)}, + {"42.0", float64(42.0)}, + {"hello", "hello"}, + {"123abc", "123abc"}, + } + for _, tc := range tests { + t.Run(tc.input, func(t *testing.T) { + assert.Equal(t, tc.expected, coerceFieldValue(tc.input)) + }) + } +} + +// --- Unit tests for GetAPIBaseURL --- +func TestGetAPIBaseURL(t *testing.T) { + t.Run("default URL when env var not set", func(t *testing.T) { + t.Setenv("QOVERY_API_URL", "") + assert.Equal(t, "https://api.qovery.com", utils.GetAPIBaseURL()) + }) + + t.Run("env var URL used when set", func(t *testing.T) { + t.Setenv("QOVERY_API_URL", "https://staging.api.qovery.com") + assert.Equal(t, "https://staging.api.qovery.com", utils.GetAPIBaseURL()) + }) + + t.Run("trailing slash stripped from env var", func(t *testing.T) { + t.Setenv("QOVERY_API_URL", "https://staging.api.qovery.com/") + assert.Equal(t, "https://staging.api.qovery.com", utils.GetAPIBaseURL()) + }) +} + +// --- M4: Duplicate --field key rejected --- +func TestAPIFieldDuplicateKey(t *testing.T) { + err := validateAPIArgs("organization", "", "", []string{"name=a", "name=b"}, nil) + assert.ErrorContains(t, err, "duplicate field key") +} diff --git a/utils/qovery.go b/utils/qovery.go index f3068aef..7daa0c25 100644 --- a/utils/qovery.go +++ b/utils/qovery.go @@ -63,16 +63,18 @@ func CheckError(err error) { } } +func GetAPIBaseURL() string { + if url := os.Getenv("QOVERY_API_URL"); url != "" { + return strings.TrimRight(url, "/") + } + return "https://api.qovery.com" +} + func GetQoveryClient(tokenType AccessTokenType, token AccessToken) *qovery.APIClient { conf := qovery.NewConfiguration() conf.UserAgent = "CLI " + Version if url := os.Getenv("QOVERY_API_URL"); url != "" { - conf.Servers = qovery.ServerConfigurations{ - { - URL: url, - Description: "No description provided", - }, - } + conf.Servers = qovery.ServerConfigurations{{URL: GetAPIBaseURL(), Description: "No description provided"}} } conf.DefaultHeader["Authorization"] = GetAuthorizationHeaderValue(tokenType, token) conf.Debug = variable.Verbose From f22e7037f99e9a65e7660ec9fd2049e11b9ed672 Mon Sep 17 00:00:00 2001 From: Fabien FLEUREAU Date: Wed, 11 Mar 2026 11:44:53 +0100 Subject: [PATCH 2/3] fix(api): resolve golangci-lint errcheck and staticcheck findings Suppress unchecked return values on fmt.Fprintf/Fprintln (writeResponse), defer resp.Body.Close, and pipe Close calls in tests. Replace strings.Index(...) == -1 with !strings.Contains per staticcheck S1003. --- cmd/api.go | 10 +++++----- cmd/api_test.go | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/api.go b/cmd/api.go index 91c9ec30..22ea495e 100644 --- a/cmd/api.go +++ b/cmd/api.go @@ -93,7 +93,7 @@ func validateAPIArgs(endpoint, method, input string, fields, headers []string) e return fmt.Errorf("invalid HTTP method %q: must be one of GET, POST, PUT, PATCH, DELETE", method) } for _, h := range headers { - if strings.Index(h, ": ") == -1 { + if !strings.Contains(h, ": ") { return fmt.Errorf("invalid header %q: must be in 'Key: Value' format", h) } } @@ -116,7 +116,7 @@ func validateAPIArgs(endpoint, method, input string, fields, headers []string) e // Returns true on success (2xx), false on error response. func writeResponse(resp *http.Response, include bool, stdout, stderr io.Writer) (bool, error) { if include { - fmt.Fprintf(stdout, "HTTP/%d.%d %s\n", resp.ProtoMajor, resp.ProtoMinor, resp.Status) + _, _ = fmt.Fprintf(stdout, "HTTP/%d.%d %s\n", resp.ProtoMajor, resp.ProtoMinor, resp.Status) headerKeys := make([]string, 0, len(resp.Header)) for k := range resp.Header { headerKeys = append(headerKeys, k) @@ -124,10 +124,10 @@ func writeResponse(resp *http.Response, include bool, stdout, stderr io.Writer) sort.Strings(headerKeys) for _, k := range headerKeys { for _, v := range resp.Header[k] { - fmt.Fprintf(stdout, "%s: %s\n", k, v) + _, _ = fmt.Fprintf(stdout, "%s: %s\n", k, v) } } - fmt.Fprintln(stdout) + _, _ = fmt.Fprintln(stdout) } body, err := io.ReadAll(resp.Body) if err != nil { @@ -272,7 +272,7 @@ func runAPI(cmd *cobra.Command, args []string) { utils.PrintlnError(err) os.Exit(1) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() ok, err := writeResponse(resp, apiInclude, os.Stdout, os.Stderr) if err != nil { diff --git a/cmd/api_test.go b/cmd/api_test.go index c876ab75..0cf75eba 100644 --- a/cmd/api_test.go +++ b/cmd/api_test.go @@ -34,8 +34,8 @@ func captureOutput(fn func()) (stdout string, stderr string) { fn() - wOut.Close() - wErr.Close() + _ = wOut.Close() + _ = wErr.Close() outBuf, _ := io.ReadAll(rOut) errBuf, _ := io.ReadAll(rErr) @@ -108,7 +108,7 @@ func TestAPIPostStdin(t *testing.T) { // Replace stdin r, w, _ := os.Pipe() _, _ = w.WriteString(requestBody) - w.Close() + _ = w.Close() oldStdin := os.Stdin os.Stdin = r defer func() { os.Stdin = oldStdin }() @@ -177,7 +177,7 @@ func TestAPINon2xx(t *testing.T) { req, _ := http.NewRequest("GET", "https://api.qovery.com/missing-resource", nil) resp, err := client.Do(req) assert.Nil(t, err) - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() var outBuf, errBuf bytes.Buffer ok, writeErr := writeResponse(resp, false, &outBuf, &errBuf) @@ -204,7 +204,7 @@ func TestAPIIncludeFlag(t *testing.T) { req, _ := http.NewRequest("GET", "https://api.qovery.com/organization", nil) resp, err := client.Do(req) assert.Nil(t, err) - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() var outBuf, errBuf bytes.Buffer ok, writeErr := writeResponse(resp, true, &outBuf, &errBuf) From ffcb0f9f150fcf11f782e8203212e2f1061d669a Mon Sep 17 00:00:00 2001 From: Fabien FLEUREAU Date: Wed, 11 Mar 2026 12:26:29 +0100 Subject: [PATCH 3/3] fix(api): address Copilot PR review comments - Fix stale writeContextFile doc comment (no return value) - Validate HTTP header name is a non-empty RFC 7230 token (rejects ': value') - Reject empty field keys ('=value') in --field validation - writeResponse routes both headers and body to the same stream (stdout for 2xx, stderr for non-2xx) so --include is consistent on error responses - Replace TestAPIPathNormalisation inline logic with integration test via runAPI --- cmd/api.go | 57 ++++++++++++++++++++++++++++++++++++++----------- cmd/api_test.go | 45 +++++++++++++++++++++++++++++++------- 2 files changed, 81 insertions(+), 21 deletions(-) diff --git a/cmd/api.go b/cmd/api.go index 22ea495e..f67903f1 100644 --- a/cmd/api.go +++ b/cmd/api.go @@ -76,6 +76,26 @@ func init() { apiCmd.Flags().BoolVarP(&apiInclude, "include", "i", false, "Print HTTP response status and headers before body") } +// isValidHTTPHeaderName reports whether name is a valid HTTP token per RFC 7230. +func isValidHTTPHeaderName(name string) bool { + if name == "" { + return false + } + for i := 0; i < len(name); i++ { + ch := name[i] + if (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') { + continue + } + switch ch { + case '!', '#', '$', '%', '&', '\'', '*', '+', '-', '.', '^', '_', '`', '|', '~': + continue + default: + return false + } + } + return true +} + // validateAPIArgs validates all arguments and flag values before any I/O. // It returns an error describing the first problem found. func validateAPIArgs(endpoint, method, input string, fields, headers []string) error { @@ -93,9 +113,14 @@ func validateAPIArgs(endpoint, method, input string, fields, headers []string) e return fmt.Errorf("invalid HTTP method %q: must be one of GET, POST, PUT, PATCH, DELETE", method) } for _, h := range headers { - if !strings.Contains(h, ": ") { + idx := strings.Index(h, ":") + if idx <= 0 { return fmt.Errorf("invalid header %q: must be in 'Key: Value' format", h) } + name := h[:idx] + if !isValidHTTPHeaderName(name) { + return fmt.Errorf("invalid header name %q: must be a non-empty HTTP token", name) + } } seen := make(map[string]bool) for _, f := range fields { @@ -104,6 +129,9 @@ func validateAPIArgs(endpoint, method, input string, fields, headers []string) e return fmt.Errorf("invalid field %q: must be in 'key=value' format", f) } key := f[:idx] + if key == "" { + return fmt.Errorf("invalid field %q: key must not be empty", f) + } if seen[key] { return fmt.Errorf("duplicate field key %q: each key may only appear once", key) } @@ -112,11 +140,18 @@ func validateAPIArgs(endpoint, method, input string, fields, headers []string) e return nil } -// writeResponse writes the response body to stdout (2xx) or stderr (non-2xx). +// writeResponse writes the response status line, headers (if include), and body +// to a single stream: stdout for 2xx responses, stderr for non-2xx. // Returns true on success (2xx), false on error response. func writeResponse(resp *http.Response, include bool, stdout, stderr io.Writer) (bool, error) { + is2xx := resp.StatusCode >= 200 && resp.StatusCode < 300 + out := stdout + if !is2xx { + out = stderr + } + if include { - _, _ = fmt.Fprintf(stdout, "HTTP/%d.%d %s\n", resp.ProtoMajor, resp.ProtoMinor, resp.Status) + _, _ = fmt.Fprintf(out, "HTTP/%d.%d %s\n", resp.ProtoMajor, resp.ProtoMinor, resp.Status) headerKeys := make([]string, 0, len(resp.Header)) for k := range resp.Header { headerKeys = append(headerKeys, k) @@ -124,21 +159,17 @@ func writeResponse(resp *http.Response, include bool, stdout, stderr io.Writer) sort.Strings(headerKeys) for _, k := range headerKeys { for _, v := range resp.Header[k] { - _, _ = fmt.Fprintf(stdout, "%s: %s\n", k, v) + _, _ = fmt.Fprintf(out, "%s: %s\n", k, v) } } - _, _ = fmt.Fprintln(stdout) + _, _ = fmt.Fprintln(out) } body, err := io.ReadAll(resp.Body) if err != nil { return false, err } - if resp.StatusCode >= 200 && resp.StatusCode < 300 { - _, _ = stdout.Write(body) - return true, nil - } - _, _ = stderr.Write(body) - return false, nil + _, _ = out.Write(body) + return is2xx, nil } // substitutePathPlaceholders replaces {organizationId}, {projectId}, {environmentId}, {serviceId} @@ -192,8 +223,8 @@ func runAPI(cmd *cobra.Command, args []string) { // Parse headers (format already validated) parsedHeaders := make(map[string]string) for _, h := range apiHeaders { - idx := strings.Index(h, ": ") - parsedHeaders[h[:idx]] = h[idx+2:] + idx := strings.Index(h, ":") + parsedHeaders[h[:idx]] = strings.TrimPrefix(h[idx+1:], " ") } // Parse fields (format already validated) diff --git a/cmd/api_test.go b/cmd/api_test.go index 0cf75eba..feef4563 100644 --- a/cmd/api_test.go +++ b/cmd/api_test.go @@ -42,8 +42,8 @@ func captureOutput(fn func()) (stdout string, stderr string) { return string(outBuf), string(errBuf) } -// writeContextFile creates a minimal ~/.qovery/context.json in a temp HOME dir, -// sets HOME to that dir, and returns a cleanup func. +// writeContextFile creates a minimal ~/.qovery/context.json in a temp HOME dir +// and sets HOME to that dir. func writeContextFile(t *testing.T, orgID, projectID, envID, serviceID string) { t.Helper() tmpHome := t.TempDir() @@ -251,6 +251,8 @@ func TestAPICustomHeader(t *testing.T) { // --- Scenario 9: Malformed -H header (pure unit test via validateAPIArgs) --- func TestAPIMalformedHeader(t *testing.T) { assert.ErrorContains(t, validateAPIArgs("organization", "", "", nil, []string{"Badheader"}), "invalid header") + // Empty header name (": value") must also be rejected + assert.ErrorContains(t, validateAPIArgs("organization", "", "", nil, []string{": value"}), "invalid header") } // --- Scenario 10: Full URL rejected (pure unit test via validateAPIArgs) --- @@ -258,19 +260,44 @@ func TestAPIFullURLRejected(t *testing.T) { assert.ErrorContains(t, validateAPIArgs("https://api.qovery.com/organization", "", "", nil, nil), "not a full URL") } -// --- Scenario 11: Path normalisation (pure unit test) --- +// --- Scenario 11: Path normalisation (integration-style via runAPI) --- func TestAPIPathNormalisation(t *testing.T) { + t.Setenv("QOVERY_CLI_ACCESS_TOKEN", "fake-token") + cases := []struct { + name string input string expected string }{ - {"/organization", "https://api.qovery.com/organization"}, - {"organization", "https://api.qovery.com/organization"}, + {"leading-slash", "/organization", "https://api.qovery.com/organization"}, + {"no-leading-slash", "organization", "https://api.qovery.com/organization"}, } + for _, tc := range cases { - path := strings.TrimLeft(tc.input, "/") - result := "https://api.qovery.com" + "/" + path - assert.Equal(t, tc.expected, result) + tc := tc // capture range variable + t.Run(tc.name, func(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + var requestedURL string + httpmock.RegisterResponder("GET", "https://api.qovery.com/organization", + func(req *http.Request) (*http.Response, error) { + requestedURL = req.URL.String() + return httpmock.NewStringResponse(200, `{}`), nil + }) + + apiMethod = "" + apiInput = "" + apiFields = []string{} + apiHeaders = []string{} + apiInclude = false + + captureOutput(func() { + runAPI(apiCmd, []string{tc.input}) + }) + + assert.Equal(t, tc.expected, requestedURL) + }) } } @@ -400,6 +427,8 @@ func TestAPIFieldAndInputMutuallyExclusive(t *testing.T) { // --- Scenario 17: Malformed --field entry (pure unit test via validateAPIArgs) --- func TestAPIMalformedField(t *testing.T) { assert.ErrorContains(t, validateAPIArgs("organization", "", "", []string{"badfield"}, nil), "invalid field") + // Empty key ("=value") must also be rejected + assert.ErrorContains(t, validateAPIArgs("organization", "", "", []string{"=value"}, nil), "key must not be empty") } // --- Scenario 18: Placeholder substitution with org context ---