-
Notifications
You must be signed in to change notification settings - Fork 110
feat(api,sdk,cli): add unauthenticated version endpoint #1602
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,3 +14,9 @@ const ( | |
| ) | ||
|
|
||
| var NewID = trexapi.NewID | ||
|
|
||
| var ( | ||
| Version = "" | ||
| BuildTime = "" | ||
| GitTag = "" | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| package version | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "net/http" | ||
| "strings" | ||
|
|
||
| localapi "github.com/ambient-code/platform/components/ambient-api-server/pkg/api" | ||
| pkgserver "github.com/openshift-online/rh-trex-ai/pkg/server" | ||
| ) | ||
|
|
||
| const versionPath = "/api/ambient/v1/version" | ||
|
|
||
| var responseBytes []byte | ||
|
|
||
| func init() { | ||
| responseBytes, _ = json.Marshal(versionResponse{ | ||
| Version: localapi.Version, | ||
| BuildTime: localapi.BuildTime, | ||
| GitTag: localapi.GitTag, | ||
| }) | ||
|
|
||
| pkgserver.RegisterPreAuthMiddleware(func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| if r.Method == http.MethodGet && strings.TrimSuffix(r.URL.Path, "/") == versionPath { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _, _ = w.Write(responseBytes) | ||
| return | ||
| } | ||
| next.ServeHTTP(w, r) | ||
| }) | ||
| }) | ||
| } | ||
|
|
||
| type versionResponse struct { | ||
| Version string `json:"version"` | ||
| BuildTime string `json:"build_time"` | ||
| GitTag string `json:"git_tag"` | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,19 +1,42 @@ | ||||||||||||||||||||||||||||||||||
| // Package version implements the version subcommand displaying build metadata. | ||||||||||||||||||||||||||||||||||
| // Package version implements the acpctl version command. | ||||||||||||||||||||||||||||||||||
| package version | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import ( | ||||||||||||||||||||||||||||||||||
| "context" | ||||||||||||||||||||||||||||||||||
| "fmt" | ||||||||||||||||||||||||||||||||||
| "time" | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| "github.com/ambient-code/platform/components/ambient-cli/pkg/config" | ||||||||||||||||||||||||||||||||||
| "github.com/ambient-code/platform/components/ambient-cli/pkg/info" | ||||||||||||||||||||||||||||||||||
| sdkclient "github.com/ambient-code/platform/components/ambient-sdk/go-sdk/client" | ||||||||||||||||||||||||||||||||||
| "github.com/spf13/cobra" | ||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| var Cmd = &cobra.Command{ | ||||||||||||||||||||||||||||||||||
| Use: "version", | ||||||||||||||||||||||||||||||||||
| Short: "Print the version", | ||||||||||||||||||||||||||||||||||
| Short: "Print the client and server version", | ||||||||||||||||||||||||||||||||||
| Args: cobra.NoArgs, | ||||||||||||||||||||||||||||||||||
| Run: func(cmd *cobra.Command, _ []string) { | ||||||||||||||||||||||||||||||||||
| fmt.Fprintf(cmd.OutOrStdout(), "acpctl %s (commit: %s, built: %s)\n", | ||||||||||||||||||||||||||||||||||
| fmt.Fprintf(cmd.OutOrStdout(), "Client: %s (commit: %s, built: %s)\n", | ||||||||||||||||||||||||||||||||||
| info.Version, info.Commit, info.BuildDate) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| cfg, err := config.Load() | ||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| apiURL := cfg.GetAPIUrl() | ||||||||||||||||||||||||||||||||||
| if apiURL == "" { | ||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
+30
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not silently drop server-version failure states Line 24 and Line 28 return without surfacing why server version is missing. This hides partial failures and makes troubleshooting harder. Suggested fix cfg, err := config.Load()
if err != nil {
+ fmt.Fprintln(cmd.OutOrStdout(), "Server: unavailable (config not loaded)")
return
}
apiURL := cfg.GetAPIUrl()
if apiURL == "" {
+ fmt.Fprintln(cmd.OutOrStdout(), "Server: unavailable (API URL not configured)")
return
}As per coding guidelines, "**/*.go: Never silently swallow partial failures; every error path must propagate or be explicitly collected, never discarded". 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| ctx, cancel := context.WithTimeout(cmd.Context(), 5*time.Second) | ||||||||||||||||||||||||||||||||||
| defer cancel() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| sv, err := sdkclient.FetchServerVersion(ctx, apiURL, cfg.InsecureTLSVerify) | ||||||||||||||||||||||||||||||||||
| if err != nil { | ||||||||||||||||||||||||||||||||||
| fmt.Fprintf(cmd.OutOrStdout(), "Server: unavailable (%v)\n", err) | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid printing raw errors in user-facing version output Line 37 includes Suggested fix- fmt.Fprintf(cmd.OutOrStdout(), "Server: unavailable (%v)\n", err)
+ fmt.Fprintln(cmd.OutOrStdout(), "Server: unavailable")
returnAs per coding guidelines, "**/*.go: Never log, error, or return tokens directly in responses; use len(token) for logging and provide generic messages to users". 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| fmt.Fprintf(cmd.OutOrStdout(), "Server: %s (tag: %s, built: %s)\n", sv.Version, sv.GitTag, sv.BuildTime) | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package client | ||
|
|
||
| import ( | ||
| "context" | ||
| "crypto/tls" | ||
| "encoding/json" | ||
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "strings" | ||
| "time" | ||
| ) | ||
|
|
||
| type ServerVersion struct { | ||
| Version string `json:"version"` | ||
| BuildTime string `json:"build_time"` | ||
| GitTag string `json:"git_tag"` | ||
| } | ||
|
|
||
| func (c *Client) ServerVersion(ctx context.Context) (*ServerVersion, error) { | ||
| var result ServerVersion | ||
| if err := c.do(ctx, http.MethodGet, "/version", nil, http.StatusOK, &result); err != nil { | ||
| return nil, err | ||
| } | ||
| return &result, nil | ||
| } | ||
|
|
||
| func FetchServerVersion(ctx context.Context, baseURL string, insecureSkipVerify bool) (*ServerVersion, error) { | ||
| url := strings.TrimSuffix(baseURL, "/") + "/api/ambient/v1/version" | ||
|
|
||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("create request: %w", err) | ||
| } | ||
| req.Header.Set("Accept", "application/json") | ||
|
|
||
| httpClient := &http.Client{Timeout: 10 * time.Second} | ||
| if insecureSkipVerify { | ||
| t := http.DefaultTransport.(*http.Transport).Clone() | ||
| if t.TLSClientConfig == nil { | ||
| t.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} | ||
| } | ||
| t.TLSClientConfig.InsecureSkipVerify = true //nolint:gosec | ||
| httpClient.Transport = t | ||
| } | ||
|
|
||
| resp, err := httpClient.Do(req) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("HTTP request failed: %w", err) | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode != http.StatusOK { | ||
| return nil, fmt.Errorf("server returned HTTP %d", resp.StatusCode) | ||
| } | ||
|
|
||
| body, err := io.ReadAll(resp.Body) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("read response: %w", err) | ||
| } | ||
|
|
||
| var result ServerVersion | ||
| if err := json.Unmarshal(body, &result); err != nil { | ||
| return nil, fmt.Errorf("unmarshal response: %w", err) | ||
| } | ||
| return &result, nil | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from dataclasses import dataclass | ||
| from typing import Optional | ||
|
|
||
| import httpx | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class ServerVersion: | ||
| version: str = "" | ||
| build_time: str = "" | ||
| git_tag: str = "" | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, data: dict) -> ServerVersion: | ||
| return cls( | ||
| version=data.get("version", ""), | ||
| build_time=data.get("build_time", ""), | ||
| git_tag=data.get("git_tag", ""), | ||
| ) | ||
|
|
||
|
|
||
| def fetch_server_version( | ||
| base_url: str, | ||
| *, | ||
| timeout: float = 10.0, | ||
| verify_ssl: bool = True, | ||
| ) -> ServerVersion: | ||
| url = base_url.rstrip("/") + "/api/ambient/v1/version" | ||
| with httpx.Client(timeout=timeout, verify=verify_ssl) as client: | ||
| response = client.get(url, headers={"Accept": "application/json"}) | ||
| response.raise_for_status() | ||
| return ServerVersion.from_dict(response.json()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle the marshal/write error paths instead of discarding them.
Line 17 and Line 27 both ignore errors. Please collect/log these explicitly so failures are not silent in this unauthenticated path.
Suggested fix
As per coding guidelines, "Never silently swallow partial failures; every error path must propagate or be explicitly collected, never discarded".
📝 Committable suggestion
🤖 Prompt for AI Agents