From f520d5ba6e51dc5151f678717345cb108aefff71 Mon Sep 17 00:00:00 2001 From: Andreas Grub Date: Fri, 23 Oct 2026 12:15:21 +0200 Subject: [PATCH 1/3] refactor(client): export the HTTP client as client/http so client.New is callable client.New and NewMeshInfoClient took an internal/http.Client, which no other module can build, and RawClient.DoRequest took its internal request options. internal/http moves to client/http with a smaller surface, internal/version to client/types/version, and ListOptions and Page to client/types, so no exported name of client uses an internal type. TestPublicApiNamesNoInternalType keeps it that way. NewClient takes WithoutRedirects and WithRetry, whose RetryPolicy picks per request which of RetryOnBusy, RetryOnGatewayError and RetryOnBrokenConnection apply; without options it returns the shared client as before. WithHeader replaces WithAccept and WithHeaders, WithBody takes the content type, Send returns the raw response, and an Error names the method and URL. The client.Authorization alias goes; the API docs download moves to internal/apidocs. Co-Authored-By: Claude Opus 5.5 --- .claude/skills/development/SKILL.md | 17 +-- .golangci.yml | 28 ++--- AGENTS.md | 2 +- client/api_surface_test.go | 105 ++++++++++++++++ client/building_block_definition.go | 2 +- client/building_block_definition_version.go | 2 +- client/building_block_run_action.go | 7 +- client/building_block_v2.go | 2 +- client/client.go | 10 +- {internal => client}/http/auth.go | 2 +- client/http/export_test.go | 6 + {internal => client}/http/http_client.go | 94 ++++++++++---- {internal => client}/http/http_client_test.go | 58 +++++++-- {internal => client}/http/http_error.go | 7 +- {internal => client}/http/logging.go | 0 .../http/logging_internal_test.go | 0 {internal => client}/http/logging_test.go | 2 +- {internal => client}/http/method.go | 0 {internal => client}/http/options.go | 42 +++---- {internal => client}/http/retry.go | 115 ++++++++++-------- {internal => client}/http/retry_test.go | 27 ++-- {internal => client}/http/stall.go | 0 {internal => client}/http/stall_test.go | 0 {internal => client}/http/user_agent.go | 0 client/internal/mesh_object.go | 31 ++--- client/landingzone.go | 2 +- client/mesh_info.go | 6 +- client/platform.go | 2 +- client/platform_type.go | 2 +- client/project.go | 2 +- client/project_group_binding.go | 2 +- client/project_user_binding.go | 2 +- client/raw.go | 15 +-- client/raw_test.go | 13 +- client/service_instance.go | 2 +- client/tenant_v4.go | 2 +- client/types/page.go | 13 ++ {internal => client/types}/version/version.go | 0 .../types}/version/version_test.go | 0 client/uuid_filter_test.go | 3 +- client/workspace_group_binding.go | 2 +- client/workspace_user_binding.go | 2 +- cmd/api/api.go | 9 +- cmd/buildingblock/tfstate/tfstate_test.go | 4 +- cmd/buildingblockrun/list.go | 7 +- cmd/buildingblockrun/logs.go | 2 +- cmd/internal/list.go | 9 +- cmd/internal/testacc/api_get_test.go | 4 +- cmd/internal/testacc/api_test.go | 3 +- cmd/meshstack/meshstack.go | 2 +- go.mod | 2 +- internal/apidocs/apidocs.go | 6 +- internal/{http => apidocs}/download.go | 52 +++----- internal/{http => apidocs}/download_test.go | 39 ++++-- internal/auth/auth.go | 2 +- internal/auth/credential/apikey.go | 2 +- internal/auth/credential/credential.go | 2 +- internal/auth/credential/manual.go | 2 +- internal/auth/credential/oidclogin.go | 2 +- internal/auth/login.go | 2 +- internal/auth/session.go | 2 +- internal/auth/session_test.go | 2 +- internal/auth/status.go | 2 +- internal/auth/version_check.go | 6 +- internal/auth/workspace_test.go | 2 +- internal/oidc/client.go | 2 +- internal/oidc/client_test.go | 2 +- .../testutil/fakemeshstack/fakemeshstack.go | 2 +- internal/tfstate/apiproxy/apiproxy.go | 4 +- internal/tfstate/lock.go | 2 +- internal/tfstate/proxy.go | 2 +- internal/tfstate/proxy_test.go | 7 +- internal/tfstate/store.go | 11 +- 73 files changed, 504 insertions(+), 322 deletions(-) create mode 100644 client/api_surface_test.go rename {internal => client}/http/auth.go (98%) create mode 100644 client/http/export_test.go rename {internal => client}/http/http_client.go (65%) rename {internal => client}/http/http_client_test.go (89%) rename {internal => client}/http/http_error.go (77%) rename {internal => client}/http/logging.go (100%) rename {internal => client}/http/logging_internal_test.go (100%) rename {internal => client}/http/logging_test.go (96%) rename {internal => client}/http/method.go (100%) rename {internal => client}/http/options.go (83%) rename {internal => client}/http/retry.go (67%) rename {internal => client}/http/retry_test.go (85%) rename {internal => client}/http/stall.go (100%) rename {internal => client}/http/stall_test.go (100%) rename {internal => client}/http/user_agent.go (100%) create mode 100644 client/types/page.go rename {internal => client/types}/version/version.go (100%) rename {internal => client/types}/version/version_test.go (100%) rename internal/{http => apidocs}/download.go (78%) rename internal/{http => apidocs}/download_test.go (79%) diff --git a/.claude/skills/development/SKILL.md b/.claude/skills/development/SKILL.md index b871b1d3..1b9ee46a 100644 --- a/.claude/skills/development/SKILL.md +++ b/.claude/skills/development/SKILL.md @@ -1,6 +1,6 @@ --- name: development -description: How the meshStack CLI's Go code is laid out and built - where a command, its logic and an HTTP call go, and what the client and settings must keep. Use when writing, changing or reviewing Go code here - a new command or package, a change to client/, pkg/ or internal/http, an HTTP request, a new setting or dependency, or a depguard or forbidigo finding. +description: How the meshStack CLI's Go code is laid out and built - where a command, its logic and an HTTP call go, and what the client and settings must keep. Use when writing, changing or reviewing Go code here - a new command or package, a change to client/, client/http or pkg/, an HTTP request, a new setting or dependency, or a depguard or forbidigo finding. --- # Developing the meshStack CLI @@ -87,32 +87,33 @@ git log -- client/client.go client.go # a path-limited log from client/ alone git blame client/client.go # traverses the merge on its own ``` -**`client/` does not log in.** `client.Authorization` produces a bearer token and replaces one that +**`client/` does not log in.** `http.Authorization` produces a bearer token and replaces one that came back 401; resolving a credential, minting a token, caching it and refreshing it is `pkg/auth`. Both front ends build their client through `auth.ResolveClient`, so the endpoint and the authorization always agree with what was resolved. Do **not** add a login exchange anywhere else: a second one gets a static token and starts returning 401 once it expires, and for a browser login it would end the user's session. -**`client/` does not own HTTP.** The client, the request options and the retry policy are -`internal/http`, because `internal/oidc` and `pkg/auth` need them and Go's internal rule closes -`client/internal` to both. Its names carry no `Http` prefix: `http.Client`, `http.Error`. +**HTTP is `client/http`.** The client, the request options and the retry policy are there, because +`internal/oidc` and `pkg/auth` need them as well, and another module calls `client.New` with them. +No exported name of `client/` may use an `internal/` type, which `TestPublicApiNamesNoInternalType` +checks. Its names carry no `Http` prefix: `http.Client`, `http.Error`. **Every request names the front end and version that sent it.** `http.NewClient` takes an `http.UserAgent`, which `auth.ResolveSessionOptions` embeds: the front end sets its `GitHubRepo` and `Version` once, the CLI in `cmd/internal.ResolveClientOptions`, and the release check reads the same two. So code below `cmd/` takes its client from its caller, and a command that needs one for anything but a meshStack client calls `internal.ResolveClientOptions().HttpClient()`. `forbidigo` -keeps `http.UserAgent` to those options, and the transport to `internal/http`. +keeps `http.UserAgent` to those options, and the transport to `client/http`. **`net/http` is always imported as `gohttp`**, which `importas` settles. That leaves the plain name -to `internal/http`, and `net/http` to the status and method constants and to the loopback server. +to `client/http`, and `net/http` to the status and method constants and to the loopback server. The `forbidigo` rule matches on the type rather than the written name, so it catches `gohttp.Client` and leaves `http.Client` alone. **Logging goes through `slog`'s default logger**, on which each front end installs its own handler: `cmd/meshstack` a `charmbracelet/log` one, the Terraform provider a `tflog` bridge. A handler -installed that late constrains every log call, and `internal/http/logging.go` states how. +installed that late constrains every log call, and `client/http/logging.go` states how. ## Settings diff --git a/.golangci.yml b/.golangci.yml index 4ecd93c9..e597a82d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -87,12 +87,12 @@ linters: - gosec text: G101 path: _test\.go - - path: internal/http/ + - path: client/http/ linters: - forbidigo text: one HTTP client # The options of a session carry the user agent the front end sets. - - path: (internal/http/|internal/auth/session\.go|internal/testutil/|_test\.go) + - path: (client/http/|internal/auth/session\.go|internal/testutil/|_test\.go) linters: - forbidigo text: user agent of the front end @@ -125,17 +125,17 @@ linters: - gosec settings: - # The process has one HTTP client, built in internal/http. + # The process has one HTTP client, built in client/http. forbidigo: analyze-types: true forbid: # Anchored to the package, as net/http has more methods named Get, such as Header.Get. - pattern: ^http\.(Client|DefaultClient|Transport|DefaultTransport|Get|Head|Post|PostForm)$ pkg: ^net/http$ - msg: the process has one HTTP client, built in internal/http; take it from there rather than making another + msg: the process has one HTTP client, built in client/http; take it from there rather than making another # A user agent made anywhere else drifts from the one the front end names itself by. - pattern: ^http\.UserAgent$ - pkg: ^github\.com/meshcloud/meshstack-cli/internal/http$ + pkg: ^github\.com/meshcloud/meshstack-cli/client/http$ msg: a client carries the user agent of the front end; take the client from your caller, or from auth.ResolveSessionOptions.HttpClient # A test that sleeps for real is slow, and flaky once the machine is slower than the # sleep assumed. synctest.Sleep panics outside a bubble, so it cannot be used the same way. @@ -290,9 +290,6 @@ linters: - encoding/json/v2 - github.com/meshcloud/meshstack-cli/cmd/internal - github.com/meshcloud/meshstack-cli/client - # the request options client.Raw.DoRequest takes, which pkg/ does not expose: they are for the - # CLI alone - - github.com/meshcloud/meshstack-cli/internal/http # the API docs choose the version of a request - github.com/meshcloud/meshstack-cli/internal/apidocs # a full URL picks the stored profile whose endpoint holds it, and has to fit an endpoint @@ -318,8 +315,7 @@ linters: allow: - $gostd - log/slog - # the proxy that serves the state to tofu. It needs the request options of - # client.Raw.DoRequest, and pkg/ does not expose them, because they are for the CLI alone. + # the proxy that serves the state to tofu - github.com/meshcloud/meshstack-cli/internal/tfstate - github.com/meshcloud/meshstack-cli/cmd/internal - github.com/meshcloud/meshstack-cli/pkg/setting @@ -423,7 +419,7 @@ linters: - github.com/charmbracelet/colorprofile # The package the Terraform provider consumes most directly, so it reaches nothing - # beyond the standard library and the three packages below. + # beyond the standard library and the packages below. client: files: # Both patterns are needed: '**/dir/**/*.go' only matches files in @@ -443,9 +439,7 @@ linters: - encoding/json/jsontext - log/slog - github.com/meshcloud/meshstack-cli/client - - github.com/meshcloud/meshstack-cli/internal/http - github.com/meshcloud/meshstack-cli/internal/json - - github.com/meshcloud/meshstack-cli/internal/version pkg: files: @@ -465,9 +459,9 @@ linters: - github.com/meshcloud/meshstack-cli/internal # client reaches these two, so whatever they may import, client gets. - internal-http: + client-http: files: - - "**/internal/http/*.go" + - "**/client/http/*.go" - "**/internal/json/*.go" - "!$test" list-mode: strict @@ -480,9 +474,9 @@ linters: - encoding/json/jsontext - github.com/meshcloud/meshstack-cli/internal/json - internal-version: + client-types-version: files: - - "**/internal/version/*.go" + - "**/client/types/version/*.go" - "!$test" list-mode: strict deny: diff --git a/AGENTS.md b/AGENTS.md index cc9db20f..5dbf401a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,7 +47,7 @@ The binary gets its name from its directory, `cmd/meshstack`, which is what `tas **Load the `development` skill before you write or review Go code here.** It holds the package layout, the split of a command into a front end in `cmd/` and its logic in `internal/`, the command -tree, the dependency policy, and what `client/`, `internal/http` and the settings must keep. +tree, the dependency policy, and what `client/`, `client/http` and the settings must keep. [`skills/meshstack-cli`](skills/meshstack-cli/SKILL.md) is the skill for those who **use** the CLI; other repositories link to it. A change to what a command does, prints or needs updates it in the diff --git a/client/api_surface_test.go b/client/api_surface_test.go new file mode 100644 index 00000000..02af0447 --- /dev/null +++ b/client/api_surface_test.go @@ -0,0 +1,105 @@ +package client + +import ( + "go/importer" + "go/token" + "go/types" + "io/fs" + "path" + "path/filepath" + "strings" + "testing" +) + +// TestPublicApiNamesNoInternalType keeps client callable from another module, which Go's internal +// rule forbids to name an internal type. +func TestPublicApiNamesNoInternalType(t *testing.T) { + imp := importer.ForCompiler(token.NewFileSet(), "source", nil).(types.ImporterFrom) //nolint:forcetypeassert // the source importer is one + err := filepath.WalkDir(".", func(dir string, d fs.DirEntry, err error) error { + if err != nil || !d.IsDir() { + return err + } + if d.Name() == "internal" || d.Name() == "testdata" { + return filepath.SkipDir + } + pkg, err := imp.ImportFrom(path.Join("github.com/meshcloud/meshstack-cli/client", filepath.ToSlash(dir)), ".", 0) + if err != nil { + return err + } + for _, name := range pkg.Scope().Names() { + obj := pkg.Scope().Lookup(name) + if !obj.Exported() { + continue + } + check := surfaceCheck{t: t, seen: map[types.Type]bool{}} + check.visit(obj.Pkg().Path()+"."+name, obj.Type()) + if named, ok := obj.Type().(*types.Named); ok { + for method := range named.Methods() { + if method.Exported() { + check.visit(obj.Pkg().Path()+"."+name+"."+method.Name(), method.Type()) + } + } + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } +} + +type surfaceCheck struct { + t *testing.T + seen map[types.Type]bool +} + +func (c surfaceCheck) visit(where string, typ types.Type) { + if c.seen[typ] { + return + } + c.seen[typ] = true + switch typ := typ.(type) { + case *types.Alias: + c.visit(where, types.Unalias(typ)) + case *types.Named: + if pkg := typ.Obj().Pkg(); pkg != nil && strings.Contains(pkg.Path()+"/", "/internal/") { + c.t.Errorf("%s uses %s", where, typ) + } + for arg := range typ.TypeArgs().Types() { + c.visit(where, arg) + } + if pkg := typ.Obj().Pkg(); pkg != nil && strings.HasPrefix(pkg.Path(), "github.com/meshcloud/meshstack-cli/client") { + c.visit(where, typ.Underlying()) + } + case *types.Pointer: + c.visit(where, typ.Elem()) + case *types.Slice: + c.visit(where, typ.Elem()) + case *types.Array: + c.visit(where, typ.Elem()) + case *types.Chan: + c.visit(where, typ.Elem()) + case *types.Map: + c.visit(where, typ.Key()) + c.visit(where, typ.Elem()) + case *types.Signature: + for param := range typ.Params().Variables() { + c.visit(where, param.Type()) + } + for result := range typ.Results().Variables() { + c.visit(where, result.Type()) + } + case *types.Struct: + for field := range typ.Fields() { + if field.Exported() { + c.visit(where+"."+field.Name(), field.Type()) + } + } + case *types.Interface: + for method := range typ.Methods() { + if method.Exported() { + c.visit(where+"."+method.Name(), method.Type()) + } + } + } +} diff --git a/client/building_block_definition.go b/client/building_block_definition.go index 3c062e02..8913ccb8 100644 --- a/client/building_block_definition.go +++ b/client/building_block_definition.go @@ -3,10 +3,10 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" "github.com/meshcloud/meshstack-cli/client/types" "github.com/meshcloud/meshstack-cli/client/types/enum" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshBuildingBlockType string diff --git a/client/building_block_definition_version.go b/client/building_block_definition_version.go index df33344c..4b40062c 100644 --- a/client/building_block_definition_version.go +++ b/client/building_block_definition_version.go @@ -7,10 +7,10 @@ import ( "fmt" "uuid" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" "github.com/meshcloud/meshstack-cli/client/types" "github.com/meshcloud/meshstack-cli/client/types/enum" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshBuildingBlockDefinitionVersionState string diff --git a/client/building_block_run_action.go b/client/building_block_run_action.go index 4d76e788..ed3ed2ed 100644 --- a/client/building_block_run_action.go +++ b/client/building_block_run_action.go @@ -3,11 +3,10 @@ package client import ( "context" "fmt" - gohttp "net/http" "net/url" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" - "github.com/meshcloud/meshstack-cli/internal/http" ) // Link is a HAL link. meshStack sends the link of an action only while the caller may take it. @@ -57,7 +56,7 @@ func (c *MeshBuildingBlockRunActionClient) ApproveRun(ctx context.Context, appro func (c *MeshBuildingBlockRunActionClient) AbortRun(ctx context.Context, abort Link) error { _, err := c.doAtLink[[]byte](ctx, http.MethodPost, abort, - http.WithHeaders(gohttp.Header{"Content-Type": {c.run.MeshObjectMimeType()}})) + http.WithHeader("Content-Type", c.run.MeshObjectMimeType())) return err } @@ -76,5 +75,5 @@ func (c *MeshBuildingBlockRunActionClient) doAtLink[R any](ctx context.Context, } below := c.httpClient.EndpointUrl.JoinPath(path) below.RawQuery = target.RawQuery - return c.httpClient.DoRequest[R](ctx, method, below, append(options, http.WithAccept(c.run.MeshObjectMimeType()))...) + return c.httpClient.DoRequest[R](ctx, method, below, append(options, http.WithHeader("Accept", c.run.MeshObjectMimeType()))...) } diff --git a/client/building_block_v2.go b/client/building_block_v2.go index ae493fee..2596014c 100644 --- a/client/building_block_v2.go +++ b/client/building_block_v2.go @@ -8,10 +8,10 @@ import ( "fmt" "slices" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" "github.com/meshcloud/meshstack-cli/client/types" "github.com/meshcloud/meshstack-cli/client/types/enum" - "github.com/meshcloud/meshstack-cli/internal/http" ) type BuildingBlockLifecycleState string diff --git a/client/client.go b/client/client.go index ea7e0983..717cba40 100644 --- a/client/client.go +++ b/client/client.go @@ -3,22 +3,18 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" + "github.com/meshcloud/meshstack-cli/client/types/version" "github.com/meshcloud/meshstack-cli/client/types/xurl" - "github.com/meshcloud/meshstack-cli/internal/http" - "github.com/meshcloud/meshstack-cli/internal/version" ) var MinMeshStackVersion = version.MustParse("2026.36.0") -// Version is re-exported because MinMeshStackVersion is one and internal/version is closed to -// another module. type Version = version.Version type HttpError = http.Error -type Authorization = http.Authorization - type Client struct { ApiKey MeshApiKeyClient BuildingBlock MeshBuildingBlockClient @@ -50,7 +46,7 @@ type Client struct { Endpoint xurl.URL } -func New(ctx context.Context, endpoint xurl.URL, client http.Client, auth Authorization) Client { +func New(ctx context.Context, endpoint xurl.URL, client http.Client, auth http.Authorization) Client { authorizedClient := internal.HttpClient{ AuthorizedClient: client.WithAuthorization(auth), EndpointUrl: endpoint, diff --git a/internal/http/auth.go b/client/http/auth.go similarity index 98% rename from internal/http/auth.go rename to client/http/auth.go index d0a6cea8..584b833d 100644 --- a/internal/http/auth.go +++ b/client/http/auth.go @@ -94,5 +94,5 @@ func (token BearerToken) Scope() string { } func (token BearerToken) asRequestOption() RequestOption { - return withHeader("Authorization", fmt.Sprintf("Bearer %s", token)) + return WithHeader("Authorization", "Bearer "+string(token)) } diff --git a/client/http/export_test.go b/client/http/export_test.go new file mode 100644 index 00000000..7013ff89 --- /dev/null +++ b/client/http/export_test.go @@ -0,0 +1,6 @@ +package http + +func WithTestRetry(c Client, maxRetries int, backoff retryBackoff) Client { + c.Transport = retryOptions{policy: defaultRetryPolicy, maxRetries: maxRetries, backoff: backoff}.wrap(c.Transport) + return c +} diff --git a/internal/http/http_client.go b/client/http/http_client.go similarity index 65% rename from internal/http/http_client.go rename to client/http/http_client.go index 4a893532..97b34f61 100644 --- a/internal/http/http_client.go +++ b/client/http/http_client.go @@ -15,17 +15,48 @@ import ( "github.com/meshcloud/meshstack-cli/internal/json" ) -var sharedClient = func() (client *gohttp.Client) { - client = &gohttp.Client{Transport: newTransport(1 * time.Minute)} - RetryOptions{ - // Sized to ride out a full meshStack backend restart, which can leave the gateway - // returning 503 for two to three minutes. This backoff sequence sums to about four - // minutes: 1+2+4+8+16+30*7 seconds. - MaxRetries: 12, - Backoff: ExponentialBackoff{MinWait: 1 * time.Second, MaxWait: 30 * time.Second}, - }.ApplyTo(client) - return -}() +// Sized to ride out a full meshStack backend restart, which can leave the gateway returning 503 +// for two to three minutes. This backoff sequence sums to about four minutes: 1+2+4+8+16+30*7 +// seconds. +var defaultRetry = retryOptions{ + policy: defaultRetryPolicy, + maxRetries: 12, + backoff: exponentialBackoff{MinWait: 1 * time.Second, MaxWait: 30 * time.Second}, +} + +var sharedClient = newHttpClient(clientOptions{retry: defaultRetry}) + +type ( + ClientOption func(opts *clientOptions) + + clientOptions struct { + retry retryOptions + noRedirects bool + } +) + +// WithoutRedirects returns a 3xx as it came. +func WithoutRedirects() ClientOption { + return func(opts *clientOptions) { + opts.noRedirects = true + } +} + +// WithRetry replaces the default policy, which retries a GET or a [Retryable] request on +// [RetryOnTransient] and nothing else. +func WithRetry(policy RetryPolicy) ClientOption { + return func(opts *clientOptions) { + opts.retry.policy = policy + } +} + +func newHttpClient(opts clientOptions) *gohttp.Client { + client := &gohttp.Client{Transport: opts.retry.wrap(newTransport(1 * time.Minute))} + if opts.noRedirects { + client.CheckRedirect = func(*gohttp.Request, []*gohttp.Request) error { return gohttp.ErrUseLastResponse } + } + return client +} func newTransport(silenceTimeout time.Duration) gohttp.RoundTripper { transport := gohttp.DefaultTransport.(*gohttp.Transport).Clone() //nolint:forcetypeassert // net/http declares it a *Transport @@ -40,11 +71,19 @@ type Client struct { } // NewClient panics on a UserAgent that does not validate, as a request has to name its front end. -func NewClient(userAgent UserAgent) Client { +// Without options, every client shares one connection pool. +func NewClient(userAgent UserAgent, options ...ClientOption) Client { if err := userAgent.Validate(); err != nil { panic(err) } - return Client{sharedClient, userAgent} + if len(options) == 0 { + return Client{sharedClient, userAgent} + } + opts := clientOptions{retry: defaultRetry} + for _, option := range options { + option(&opts) + } + return Client{newHttpClient(opts), userAgent} } // DoRequest sends one request and parses the answer as JSON, or returns it as it came, empty @@ -76,19 +115,9 @@ func (c Client) DoRequest[R any](ctx context.Context, method string, url *url.UR return } -func (c Client) doRequest(ctx context.Context, method string, url *url.URL, options []RequestOption) ([]byte, error) { - res, err := c.send(ctx, method, url, options) - if err != nil { - return nil, err - } - defer func() { - _ = res.Body.Close() - }() - return c.readBodyAndCheckSuccess(ctx, res) -} - -func (c Client) send(ctx context.Context, method string, url *url.URL, options []RequestOption) (*gohttp.Response, error) { - options = slices.Insert(options, 0, withHeader("User-Agent", c.UserAgent.String())) +// Send returns the response whatever its status, and the caller closes its body. +func (c Client) Send(ctx context.Context, method string, url *url.URL, options ...RequestOption) (*gohttp.Response, error) { + options = slices.Insert(options, 0, WithHeader("User-Agent", c.UserAgent.String())) opts := requestOptions{} for _, option := range options { option(&opts) @@ -100,6 +129,17 @@ func (c Client) send(ctx context.Context, method string, url *url.URL, options [ return c.Do(req) } +func (c Client) doRequest(ctx context.Context, method string, url *url.URL, options []RequestOption) ([]byte, error) { + res, err := c.Send(ctx, method, url, options...) + if err != nil { + return nil, err + } + defer func() { + _ = res.Body.Close() + }() + return c.readBodyAndCheckSuccess(ctx, res) +} + func isSuccess(statusCode int) bool { return statusCode >= 200 && statusCode <= 299 } @@ -116,6 +156,8 @@ func (c Client) readBodyAndCheckSuccess(ctx context.Context, res *gohttp.Respons } return responseBody, Error{ + Method: res.Request.Method, + Url: res.Request.URL.Redacted(), StatusCode: res.StatusCode, ResponseBody: responseBody, } diff --git a/internal/http/http_client_test.go b/client/http/http_client_test.go similarity index 89% rename from internal/http/http_client_test.go rename to client/http/http_client_test.go index ddd57ca1..316cf32d 100644 --- a/internal/http/http_client_test.go +++ b/client/http/http_client_test.go @@ -1,5 +1,5 @@ // Package http_test drives the client from the outside, so that it can parse answers into the -// types real callers declare. internal/http may not import any of them. +// types real callers declare. client/http may not import any of them. package http_test import ( @@ -19,8 +19,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/xurl" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/logs" "github.com/meshcloud/meshstack-cli/internal/oidc/jwt" "github.com/meshcloud/meshstack-cli/internal/testutil/fakemeshstack" @@ -83,7 +83,7 @@ func TestHttpClient(t *testing.T) { } resp.WriteHeader(gohttp.StatusOK) _, _ = resp.Write([]byte(`{}`)) - }), http.RetryOptions{MaxRetries: 3, Backoff: &retryTestBackoff}) + }), 3, &retryTestBackoff) start := time.Now() _, err := client.DoRequest[any](t.Context(), gohttp.MethodGet, client.ServerUrl.JoinPath("get")) @@ -108,7 +108,7 @@ func TestHttpClient(t *testing.T) { backoff := retryTestBackoff{} client := withTestRetry(newTestClientWithServer(t, func(resp gohttp.ResponseWriter, _ *gohttp.Request) { resp.WriteHeader(gohttp.StatusBadGateway) - }), http.RetryOptions{MaxRetries: 2, Backoff: &backoff}) + }), 2, &backoff) _, err := client.DoRequest[any](t.Context(), gohttp.MethodGet, client.ServerUrl.JoinPath("get")) var httpErr http.Error require.ErrorAs(t, err, &httpErr) @@ -128,7 +128,7 @@ func TestHttpClient(t *testing.T) { backoff := retryTestBackoff{WaitTime: 10 * time.Second} client := withTestRetry(newTestClientWithServer(t, func(resp gohttp.ResponseWriter, _ *gohttp.Request) { resp.WriteHeader(gohttp.StatusBadGateway) - }), http.RetryOptions{MaxRetries: 3, Backoff: &backoff}) + }), 3, &backoff) // The bubble's clock only moves once the 502 has arrived and the backoff waits, so this // cancels inside the wait. Cancelling from the handler raced the 502 back to the client. ctx, cancel := context.WithCancel(t.Context()) @@ -155,7 +155,7 @@ func TestHttpClient(t *testing.T) { return } resp.WriteHeader(gohttp.StatusNoContent) - }), http.RetryOptions{MaxRetries: 3, Backoff: &retryTestBackoff{}}) + }), 3, &retryTestBackoff{}) payload := http.WithJsonPayload(map[string]string{"key": "value"}, "application/json") for _, method := range []string{gohttp.MethodPatch, gohttp.MethodPost, gohttp.MethodPut, gohttp.MethodDelete} { @@ -254,6 +254,46 @@ func TestHttpClient(t *testing.T) { }) } +func TestClientOptions(t *testing.T) { + serve := func(t *testing.T, handler gohttp.HandlerFunc) *url.URL { + t.Helper() + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + serverUrl, err := url.Parse(server.URL) + require.NoError(t, err) + return serverUrl + } + + t.Run("WithoutRedirects answers with the 3xx", func(t *testing.T) { + serverUrl := serve(t, gohttp.RedirectHandler("/elsewhere", gohttp.StatusFound).ServeHTTP) + client := http.NewClient(fakemeshstack.UserAgent, http.WithoutRedirects()) + + _, err := client.DoRequest[[]byte](t.Context(), http.MethodGet, serverUrl) + + httpErr, ok := errors.AsType[http.Error](err) + require.True(t, ok, "%v", err) + assert.Equal(t, gohttp.StatusFound, httpErr.StatusCode) + }) + + t.Run("WithRetry retries a POST on 503 and stops at 502", func(t *testing.T) { + var answered []int + serverUrl := serve(t, func(resp gohttp.ResponseWriter, _ *gohttp.Request) { + status := []int{gohttp.StatusServiceUnavailable, gohttp.StatusBadGateway}[len(answered)] + answered = append(answered, status) + resp.Header().Set("Retry-After", "0") + resp.WriteHeader(status) + }) + client := http.NewClient(fakemeshstack.UserAgent, http.WithRetry(func(string, bool) http.RetryOn { return http.RetryOnBusy })) + + _, err := client.DoRequest[any](t.Context(), http.MethodPost, serverUrl) + + httpErr, ok := errors.AsType[http.Error](err) + require.True(t, ok, "%v", err) + assert.Equal(t, []int{gohttp.StatusServiceUnavailable, gohttp.StatusBadGateway}, answered) + assert.Equal(t, http.Error{Method: http.MethodPost, Url: serverUrl.String(), StatusCode: gohttp.StatusBadGateway, ResponseBody: []byte{}}, httpErr) + }) +} + type refreshableAuthorization struct { token http.BearerToken keepToken bool @@ -418,10 +458,8 @@ func newTestClientWithServer(t *testing.T, handlerFunc gohttp.HandlerFunc) TestC return TestClient{http.Client{Client: client, UserAgent: fakemeshstack.UserAgent}, serverUrl} } -// withTestRetry gives one test client its own retry policy. The shipped client is the one shared -// one and takes no configuration, so a test that needs a backoff it can count builds its own. -func withTestRetry(c TestClient, options http.RetryOptions) TestClient { - options.ApplyTo(c.Client.Client) +func withTestRetry(c TestClient, maxRetries int, backoff *retryTestBackoff) TestClient { + c.Client = http.WithTestRetry(c.Client, maxRetries, backoff) return c } diff --git a/internal/http/http_error.go b/client/http/http_error.go similarity index 77% rename from internal/http/http_error.go rename to client/http/http_error.go index 57914475..0005e8e1 100644 --- a/internal/http/http_error.go +++ b/client/http/http_error.go @@ -6,12 +6,17 @@ import ( ) type Error struct { + Method, Url string StatusCode int ResponseBody []byte } func (e Error) Error() string { - return fmt.Sprintf("http error %d, response '%s'", e.StatusCode, string(e.ResponseBody)) + message := fmt.Sprintf("http error %d, response '%s'", e.StatusCode, string(e.ResponseBody)) + if e.Method == "" { + return message + } + return e.Method + " " + e.Url + ": " + message } func (e Error) IsClientError() bool { diff --git a/internal/http/logging.go b/client/http/logging.go similarity index 100% rename from internal/http/logging.go rename to client/http/logging.go diff --git a/internal/http/logging_internal_test.go b/client/http/logging_internal_test.go similarity index 100% rename from internal/http/logging_internal_test.go rename to client/http/logging_internal_test.go diff --git a/internal/http/logging_test.go b/client/http/logging_test.go similarity index 96% rename from internal/http/logging_test.go rename to client/http/logging_test.go index 8bcbf189..b9533d66 100644 --- a/internal/http/logging_test.go +++ b/client/http/logging_test.go @@ -9,7 +9,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/meshcloud/meshstack-cli/internal/http" + "github.com/meshcloud/meshstack-cli/client/http" ) // TestJsonLogRedactsTheToken uses a handler that encodes attributes as JSON rather than diff --git a/internal/http/method.go b/client/http/method.go similarity index 100% rename from internal/http/method.go rename to client/http/method.go diff --git a/internal/http/options.go b/client/http/options.go similarity index 83% rename from internal/http/options.go rename to client/http/options.go index d53b09a3..9ecb8f3f 100644 --- a/internal/http/options.go +++ b/client/http/options.go @@ -1,7 +1,6 @@ package http import ( - "context" "encoding/json/jsontext" "fmt" "maps" @@ -23,8 +22,8 @@ type ( requestModifier func(req *gohttp.Request) error ) -// Retryable declares that replaying this request cannot do harm, which is the only way a method -// other than GET is ever retried. meshStack's /api/login is the case it exists for: it mints a +// Retryable declares that replaying this request cannot do harm, which the default [RetryPolicy] +// requires of any method other than GET. meshStack's /api/login is the case it exists for: it mints a // token and invalidates nothing, so a replay after a gateway 503 costs one token. // // Never put it on an OIDC refresh grant: that grant rotates the refresh token, and keycloak ends @@ -40,8 +39,8 @@ func Retryable() RequestOption { // because gohttp.Client passes the context on to the request it issues for a redirect. type retryableKey struct{} -func isRetryable(ctx context.Context) bool { - retryable, _ := ctx.Value(retryableKey{}).(bool) +func isRetryable(req *gohttp.Request) bool { + retryable, _ := req.Context().Value(retryableKey{}).(bool) return retryable } @@ -111,41 +110,28 @@ func appendRequestModifier(modifier requestModifier) RequestOption { } } -func WithAccept(accept string) RequestOption { - return withHeader("Accept", accept) -} - -func WithHeaders(header gohttp.Header) RequestOption { +func WithHeader(key string, values ...string) RequestOption { return appendRequestModifier(func(req *gohttp.Request) error { - for key, values := range header { - req.Header.Del(key) - for _, value := range values { - req.Header.Add(key, value) - } + req.Header.Del(key) + for _, value := range values { + req.Header.Add(key, value) } return nil }) } -// WithBody sends the bytes as they are, so the caller names their type in a Content-Type header. -func WithBody(body []byte) RequestOption { +func WithBody(contentType string, body []byte) RequestOption { return func(opts *requestOptions) { if body == nil { return } + WithHeader("Content-Type", contentType)(opts) opts.requestPayload = func() ([]byte, error) { return body, nil } } } -func withHeader(key, value string) RequestOption { - return appendRequestModifier(func(req *gohttp.Request) error { - req.Header.Set(key, value) - return nil - }) -} - // WithJsonPayload sends a value as a JSON body, and both sends and asks for the given content // type. The type is a parameter because meshStack names a meshObject's kind and version in it. func WithJsonPayload(payload any, contentType string) RequestOption { @@ -153,8 +139,8 @@ func WithJsonPayload(payload any, contentType string) RequestOption { if payload == nil { return } - WithAccept(contentType)(opts) - withHeader("Content-Type", contentType)(opts) + WithHeader("Accept", contentType)(opts) + WithHeader("Content-Type", contentType)(opts) opts.requestPayload = func() ([]byte, error) { return json.Marshal(payload) } @@ -169,8 +155,8 @@ func WithFormPayload(payload any) RequestOption { if payload == nil { return } - WithAccept("application/json")(opts) - withHeader("Content-Type", "application/x-www-form-urlencoded")(opts) + WithHeader("Accept", "application/json")(opts) + WithHeader("Content-Type", "application/x-www-form-urlencoded")(opts) opts.requestPayload = func() ([]byte, error) { values, err := convertStructOrMapToUrlValues(payload) if err != nil { diff --git a/internal/http/retry.go b/client/http/retry.go similarity index 67% rename from internal/http/retry.go rename to client/http/retry.go index 8befe34f..dd8d3de3 100644 --- a/internal/http/retry.go +++ b/client/http/retry.go @@ -14,47 +14,58 @@ import ( "time" ) -type RetryOptions struct { - MaxRetries int - Backoff RetryBackoff -} +type RetryOn uint8 + +const ( + // RetryOnBusy waits as long as the Retry-After of a 429 or 503 asks. + RetryOnBusy RetryOn = 1 << iota + // RetryOnGatewayError covers 502 and 504, after which the server may have acted on the request. + RetryOnGatewayError + // RetryOnBrokenConnection covers a connection that broke once it was up, as one an ingress drops + // during a rolling deploy does. + RetryOnBrokenConnection + + RetryOnTransient = RetryOnBusy | RetryOnGatewayError | RetryOnBrokenConnection +) + +// RetryPolicy gets retryable true when the caller marked the request with [Retryable]. +type RetryPolicy func(method string, retryable bool) RetryOn -// ApplyTo makes the given client retry a GET on its own, and any other method only where the -// caller marked the request with [Retryable]. -func (options RetryOptions) ApplyTo(c *gohttp.Client) { - next := gohttp.DefaultTransport - if c.Transport != nil { - next = c.Transport +func defaultRetryPolicy(method string, retryable bool) RetryOn { + if method == MethodGet || retryable { + return RetryOnTransient } - c.Transport = &retryRoundTripper{ - Next: next, - MaxRetries: options.MaxRetries, - ShouldRetryRequest: func(req *gohttp.Request) bool { - if options.Backoff == nil { - return false - } - return req.Method == MethodGet || isRetryable(req.Context()) - }, - ShouldRetryResponse: func(resp *gohttp.Response, err error) RetryBackoff { - if err != nil { - // Only a connection that broke once it was up clears on a retry, as one an ingress drops - // during a rolling deploy does. An unknown host, a refused connection or a TLS error - // repeats on every attempt, and a timeout would multiply by the retries. - if isBrokenConnection(err) { - return options.Backoff - } - return nil - } - switch resp.StatusCode { - case gohttp.StatusTooManyRequests, gohttp.StatusServiceUnavailable: - return retryAfterBackoff{Response: resp, Fallback: options.Backoff} - case gohttp.StatusBadGateway, gohttp.StatusGatewayTimeout: - return options.Backoff - default: - return nil - } - }, + return 0 +} + +type retryOptions struct { + policy RetryPolicy + maxRetries int + backoff retryBackoff +} + +func (options retryOptions) wrap(next gohttp.RoundTripper) gohttp.RoundTripper { + return &retryRoundTripper{next: next, retryOptions: options} +} + +func (on RetryOn) backoff(resp *gohttp.Response, err error, fallback retryBackoff) retryBackoff { + switch { + case err != nil: + // An unknown host, a refused connection or a TLS error repeats on every attempt, and a + // timeout would multiply by the retries. + if on&RetryOnBrokenConnection != 0 && isBrokenConnection(err) { + return fallback + } + case resp.StatusCode == gohttp.StatusTooManyRequests || resp.StatusCode == gohttp.StatusServiceUnavailable: + if on&RetryOnBusy != 0 { + return retryAfterBackoff{Response: resp, Fallback: fallback} + } + case resp.StatusCode == gohttp.StatusBadGateway || resp.StatusCode == gohttp.StatusGatewayTimeout: + if on&RetryOnGatewayError != 0 { + return fallback + } } + return nil } func isBrokenConnection(err error) bool { @@ -62,15 +73,15 @@ func isBrokenConnection(err error) bool { errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) } -type RetryBackoff interface { +type retryBackoff interface { Calculate(attempt int) time.Duration } -type ExponentialBackoff struct { +type exponentialBackoff struct { MinWait, MaxWait time.Duration } -func (b ExponentialBackoff) Calculate(attempt int) time.Duration { +func (b exponentialBackoff) Calculate(attempt int) time.Duration { nextWait := time.Duration(math.Pow(2, float64(attempt-1))) * b.MinWait if b.MaxWait > 0 && nextWait > b.MaxWait { return b.MaxWait @@ -82,7 +93,7 @@ var timeNow = time.Now type retryAfterBackoff struct { Response *gohttp.Response - Fallback RetryBackoff + Fallback retryBackoff } func (b retryAfterBackoff) Calculate(attempt int) (waitTime time.Duration) { @@ -112,27 +123,27 @@ func (b retryAfterBackoff) Calculate(attempt int) (waitTime time.Duration) { } type retryRoundTripper struct { - Next gohttp.RoundTripper - MaxRetries int - ShouldRetryRequest func(req *gohttp.Request) bool - ShouldRetryResponse func(resp *gohttp.Response, err error) RetryBackoff + retryOptions + + next gohttp.RoundTripper } func (r *retryRoundTripper) RoundTrip(req *gohttp.Request) (*gohttp.Response, error) { - if !r.ShouldRetryRequest(req) { - return r.Next.RoundTrip(req) + on := r.policy(req.Method, isRetryable(req)) + if on == 0 { + return r.next.RoundTrip(req) } req = makeRequestBodyRetryable(req) for attempt := 1; ; attempt++ { - resp, err := r.Next.RoundTrip(req) + resp, err := r.next.RoundTrip(req) // A request the context ended is not retried. The context is checked rather than the error, // since the transport returns the context's cause, which for Ctrl-C is a signal error that // wraps no context.Canceled. if errors.Is(err, errRetryableBodyClose) || err != nil && req.Context().Err() != nil { return resp, err } - backoff := r.ShouldRetryResponse(resp, err) - if backoff == nil || attempt > r.MaxRetries { + backoff := on.backoff(resp, err, r.backoff) + if backoff == nil || attempt > r.maxRetries { return resp, err } drainAndCloseResponseBody(req.Context(), resp) @@ -153,7 +164,7 @@ func (r *retryRoundTripper) RoundTrip(req *gohttp.Request) (*gohttp.Response, er }(), "method", req.Method, "path", req.URL.Path, - "attempt", fmt.Sprintf("%d/%d", attempt, r.MaxRetries), + "attempt", fmt.Sprintf("%d/%d", attempt, r.maxRetries), "waitTime", waitTime, )...) timer := time.NewTimer(waitTime) diff --git a/internal/http/retry_test.go b/client/http/retry_test.go similarity index 85% rename from internal/http/retry_test.go rename to client/http/retry_test.go index fa172761..cb3a0184 100644 --- a/internal/http/retry_test.go +++ b/client/http/retry_test.go @@ -30,7 +30,7 @@ func TestExponentialBackoff_Calculate(t *testing.T) { } for _, tt := range tests { t.Run(fmt.Sprintf("attempt %d", tt.attempt), func(t *testing.T) { - b := ExponentialBackoff{ + b := exponentialBackoff{ MinWait: 1 * time.Second, MaxWait: 5 * time.Second, } @@ -41,7 +41,7 @@ func TestExponentialBackoff_Calculate(t *testing.T) { func TestRetryAfterBackoff(t *testing.T) { bubbleStart := time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC) - fallback := ExponentialBackoff{MinWait: 1 * time.Second, MaxWait: 10 * time.Second} + fallback := exponentialBackoff{MinWait: 1 * time.Second, MaxWait: 10 * time.Second} tests := []struct { name string @@ -83,15 +83,14 @@ func TestRetryStopsOnceTheContextIsDone(t *testing.T) { require.NoError(t, err) calls := 0 - retrying := &retryRoundTripper{ - Next: roundTripperFunc(func(req *gohttp.Request) (*gohttp.Response, error) { - calls++ - return nil, context.Cause(req.Context()) - }), - MaxRetries: 3, - ShouldRetryRequest: func(*gohttp.Request) bool { return true }, - ShouldRetryResponse: func(*gohttp.Response, error) RetryBackoff { return ExponentialBackoff{} }, - } + retrying := retryOptions{ + policy: func(string, bool) RetryOn { return RetryOnTransient }, + maxRetries: 3, + backoff: exponentialBackoff{}, + }.wrap(roundTripperFunc(func(req *gohttp.Request) (*gohttp.Response, error) { + calls++ + return nil, context.Cause(req.Context()) + })) _, err = retrying.RoundTrip(req) //nolint:bodyclose // no response comes back require.ErrorIs(t, err, interrupted) @@ -106,7 +105,7 @@ func TestRetryGivesUpOnAServerThatDoesNotAnswer(t *testing.T) { })) defer server.Close() client := &gohttp.Client{Transport: newTransport(10 * time.Millisecond)} - RetryOptions{MaxRetries: 3, Backoff: ExponentialBackoff{}}.ApplyTo(client) + client.Transport = retryOptions{policy: defaultRetryPolicy, maxRetries: 3, backoff: exponentialBackoff{}}.wrap(client.Transport) _, err := client.Get(server.URL) //nolint:bodyclose,noctx // no response comes back @@ -122,7 +121,7 @@ func TestRetryGivesUpOnARefusedConnection(t *testing.T) { calls.Add(1) return gohttp.DefaultTransport.RoundTrip(req) })} - RetryOptions{MaxRetries: 3, Backoff: ExponentialBackoff{}}.ApplyTo(client) + client.Transport = retryOptions{policy: defaultRetryPolicy, maxRetries: 3, backoff: exponentialBackoff{}}.wrap(client.Transport) _, err := client.Get(server.URL) //nolint:bodyclose,noctx // no response comes back @@ -150,7 +149,7 @@ func TestRetryRecoversFromABrokenConnection(t *testing.T) { })) defer server.Close() client := &gohttp.Client{Transport: newTransport(time.Minute)} - RetryOptions{MaxRetries: 3, Backoff: ExponentialBackoff{}}.ApplyTo(client) + client.Transport = retryOptions{policy: defaultRetryPolicy, maxRetries: 3, backoff: exponentialBackoff{}}.wrap(client.Transport) resp, err := client.Get(server.URL) //nolint:noctx // the test server answers at once diff --git a/internal/http/stall.go b/client/http/stall.go similarity index 100% rename from internal/http/stall.go rename to client/http/stall.go diff --git a/internal/http/stall_test.go b/client/http/stall_test.go similarity index 100% rename from internal/http/stall_test.go rename to client/http/stall_test.go diff --git a/internal/http/user_agent.go b/client/http/user_agent.go similarity index 100% rename from internal/http/user_agent.go rename to client/http/user_agent.go diff --git a/client/internal/mesh_object.go b/client/internal/mesh_object.go index 9147f6f0..9519ab61 100644 --- a/client/internal/mesh_object.go +++ b/client/internal/mesh_object.go @@ -13,8 +13,9 @@ import ( "strings" "unicode" + "github.com/meshcloud/meshstack-cli/client/http" + "github.com/meshcloud/meshstack-cli/client/types" "github.com/meshcloud/meshstack-cli/client/types/xurl" - "github.com/meshcloud/meshstack-cli/internal/http" ) type HttpClient struct { @@ -90,7 +91,7 @@ func (c MeshObjectClient[M]) Get(ctx context.Context, id string) (resp *M, err e } func (c MeshObjectApi) GetAtPath[R any](ctx context.Context, id string, extraPath ...string) (R, error) { - return c.DoRequest[R](ctx, http.MethodGet, c.ApiUrl.JoinPath(id).JoinPath(extraPath...), http.WithAccept(c.MeshObjectMimeType())) + return c.DoRequest[R](ctx, http.MethodGet, c.ApiUrl.JoinPath(id).JoinPath(extraPath...), http.WithHeader("Accept", c.MeshObjectMimeType())) } func (c MeshObjectClient[M]) Post[P any](ctx context.Context, payload P) (*M, error) { @@ -99,7 +100,7 @@ func (c MeshObjectClient[M]) Post[P any](ctx context.Context, payload P) (*M, er func (c MeshObjectApi) PostAtPath[R, P any](ctx context.Context, payload P, extraPath ...string) (R, error) { return c.DoRequest[R](ctx, http.MethodPost, c.ApiUrl.JoinPath(extraPath...), - http.WithAccept(c.MeshObjectMimeType()), c.withMeshObjectPayload(payload)) + http.WithHeader("Accept", c.MeshObjectMimeType()), c.withMeshObjectPayload(payload)) } func (c MeshObjectClient[M]) Put[P any](ctx context.Context, id string, payload P) (*M, error) { @@ -111,15 +112,15 @@ func (c MeshObjectClient[M]) Delete(ctx context.Context, id string) (err error) } func (c MeshObjectApi) DeleteAtPath(ctx context.Context, id string, extraPath ...string) (err error) { - _, err = c.DoRequest[any](ctx, http.MethodDelete, c.ApiUrl.JoinPath(id).JoinPath(extraPath...), http.Retryable(), http.WithAccept(c.MeshObjectMimeType())) + _, err = c.DoRequest[any](ctx, http.MethodDelete, c.ApiUrl.JoinPath(id).JoinPath(extraPath...), http.Retryable(), http.WithHeader("Accept", c.MeshObjectMimeType())) return } func (c MeshObjectClient[M]) ListSeq(ctx context.Context, options ...http.RequestOption) iter.Seq2[M, error] { - return c.ListSeqAs[M](ctx, ListOptions{}, options...) + return c.ListSeqAs[M](ctx, types.ListOptions{}, options...) } -func (c MeshObjectApi) ListSeqAs[T any](ctx context.Context, listOptions ListOptions, options ...http.RequestOption) iter.Seq2[T, error] { +func (c MeshObjectApi) ListSeqAs[T any](ctx context.Context, listOptions types.ListOptions, options ...http.RequestOption) iter.Seq2[T, error] { return func(yield func(T, error) bool) { var noItem T embeddedKey := pluralizeKind(c.Kind) @@ -150,16 +151,9 @@ func (c MeshObjectApi) ListSeqAs[T any](ctx context.Context, listOptions ListOpt } } -type Page struct { - Size int `json:"size"` - TotalElements int `json:"totalElements"` - TotalPages int `json:"totalPages"` - Number int `json:"number"` -} - type paginatedResponse[T any] struct { Embedded map[string][]T `json:"_embedded"` - Page Page `json:"page"` + Page types.Page `json:"page"` } // withMeshObjectPayload takes P rather than an any because the `,embed` tag takes a struct, a @@ -172,13 +166,13 @@ func (c MeshObjectApi) withMeshObjectPayload[P any](payload P) http.RequestOptio }{c.ApiVersion, c.Kind, payload}, c.MeshObjectMimeType()) } -func (c MeshObjectApi) getPage[T any](ctx context.Context, listOptions ListOptions, pageNumber int, options []http.RequestOption) (paginatedResponse[T], error) { +func (c MeshObjectApi) getPage[T any](ctx context.Context, listOptions types.ListOptions, pageNumber int, options []http.RequestOption) (paginatedResponse[T], error) { query := map[string]any{"page": pageNumber} if listOptions.PageSize > 0 { query["size"] = listOptions.PageSize } response, err := c.DoRequest[paginatedResponse[T]](ctx, http.MethodGet, c.ApiUrl, append(options, - http.WithAccept(c.MeshObjectMimeType()), + http.WithHeader("Accept", c.MeshObjectMimeType()), http.WithUrlQuery(query), )...) if err != nil && pageNumber > 0 { @@ -192,11 +186,6 @@ func (c MeshObjectApi) getPage[T any](ctx context.Context, listOptions ListOptio return response, nil } -type ListOptions struct { - PageSize int - OnPage func(Page) -} - func (c MeshObjectClient[M]) List(ctx context.Context, options ...http.RequestOption) ([]M, error) { var result []M for item, err := range c.ListSeq(ctx, options...) { diff --git a/client/landingzone.go b/client/landingzone.go index 80fd4cc3..1f3b3f3b 100644 --- a/client/landingzone.go +++ b/client/landingzone.go @@ -3,8 +3,8 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshLandingZone struct { diff --git a/client/mesh_info.go b/client/mesh_info.go index 6c7d8306..2c728983 100644 --- a/client/mesh_info.go +++ b/client/mesh_info.go @@ -4,10 +4,10 @@ import ( "context" "fmt" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/enum" + "github.com/meshcloud/meshstack-cli/client/types/version" "github.com/meshcloud/meshstack-cli/client/types/xurl" - "github.com/meshcloud/meshstack-cli/internal/http" - "github.com/meshcloud/meshstack-cli/internal/version" ) // MeshFeatureFlag names an optional meshStack capability. /mesh/info reports each one as a bool of @@ -45,7 +45,7 @@ func NewMeshInfoClient(client http.Client, endpoint xurl.URL) MeshInfoClient { } func (c meshInfoClient) Read(ctx context.Context) (MeshInfo, error) { - return c.DoRequest[MeshInfo](ctx, "GET", c.Endpoint.JoinPath("/mesh/info"), http.WithAccept("application/json")) + return c.DoRequest[MeshInfo](ctx, "GET", c.Endpoint.JoinPath("/mesh/info"), http.WithHeader("Accept", "application/json")) } func (info MeshInfo) CheckVersion() error { diff --git a/client/platform.go b/client/platform.go index 3cfd38c5..0144c6b6 100644 --- a/client/platform.go +++ b/client/platform.go @@ -3,9 +3,9 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" "github.com/meshcloud/meshstack-cli/client/types" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshPlatform struct { diff --git a/client/platform_type.go b/client/platform_type.go index 69cb3bfe..37ea81f4 100644 --- a/client/platform_type.go +++ b/client/platform_type.go @@ -3,8 +3,8 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshPlatformType struct { diff --git a/client/project.go b/client/project.go index 5da6d19b..c8e06165 100644 --- a/client/project.go +++ b/client/project.go @@ -3,8 +3,8 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshProject struct { diff --git a/client/project_group_binding.go b/client/project_group_binding.go index f0f2d274..d81883ea 100644 --- a/client/project_group_binding.go +++ b/client/project_group_binding.go @@ -3,8 +3,8 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshProjectGroupBinding struct { diff --git a/client/project_user_binding.go b/client/project_user_binding.go index 90aaf14d..04b7d270 100644 --- a/client/project_user_binding.go +++ b/client/project_user_binding.go @@ -3,8 +3,8 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshProjectUserBinding struct { diff --git a/client/raw.go b/client/raw.go index ddc2e65f..e79ab3f0 100644 --- a/client/raw.go +++ b/client/raw.go @@ -10,20 +10,15 @@ import ( "time" "uuid" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" - "github.com/meshcloud/meshstack-cli/internal/http" + "github.com/meshcloud/meshstack-cli/client/types" ) -type ListOptions = internal.ListOptions - -type Page = internal.Page - // SortCriteria holds one `property,(asc|desc)` criterion per element, such as createdAt,desc. type SortCriteria []string -// RawClient answers with the JSON meshStack sent, for the CLI, which writes it out as it came. It -// is the CLI's alone: DoRequest takes the options of internal/http, which Go's internal rule closes -// to the Terraform provider. +// RawClient answers with the JSON meshStack sent, for the CLI, which writes it out as it came. type RawClient struct { httpClient internal.HttpClient // apis is keyed by the Go type rather than the kind, because MeshBuildingBlock and @@ -43,7 +38,7 @@ func newRawClient(httpClient internal.HttpClient) *RawClient { // List pages through the objects of M, narrowed by filter, a struct whose fields are the query // parameters of the listing. -func (r *RawClient) List[M any](ctx context.Context, filter any, options ListOptions) iter.Seq2[jsontext.Value, error] { +func (r *RawClient) List[M any](ctx context.Context, filter any, options types.ListOptions) iter.Seq2[jsontext.Value, error] { api, err := r.api[M]() if err != nil { return func(yield func(jsontext.Value, error) bool) { @@ -67,7 +62,7 @@ func (r *RawClient) Get[M any](ctx context.Context, uuid uuid.UUID, pathElems .. return nil, err } return api.DoRequest[jsontext.Value](ctx, http.MethodGet, api.ApiUrl.JoinPath(uuid.String()).JoinPath(pathElems...), - http.WithAccept(api.MeshObjectMimeType())) + http.WithHeader("Accept", api.MeshObjectMimeType())) } // DoRequest joins path onto the endpoint, even one that names a host, so that the bearer token diff --git a/client/raw_test.go b/client/raw_test.go index 8f8f4a4c..ebe0af7b 100644 --- a/client/raw_test.go +++ b/client/raw_test.go @@ -13,9 +13,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" + "github.com/meshcloud/meshstack-cli/client/types" "github.com/meshcloud/meshstack-cli/client/types/xurl" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/testutil/fakemeshstack" ) @@ -41,7 +42,7 @@ func TestRawClient(t *testing.T) { httpClient := newTestHttpClient(server) raw := newRawClient(httpClient).with(newWorkspaceClient(t.Context(), httpClient).meshObject) var requests []fakemeshstack.Request - list := func(t *testing.T, filter MeshWorkspaceListFilter, options ListOptions) (listed []string) { + list := func(t *testing.T, filter MeshWorkspaceListFilter, options types.ListOptions) (listed []string) { t.Helper() server.TakeRequests() for item, err := range raw.List[MeshWorkspace](t.Context(), filter, options) { @@ -63,7 +64,7 @@ func TestRawClient(t *testing.T) { // kind, which a POST of the edited object needs, and _links, which shows an LLM agent how // meshObjects relate. t.Run("a list yields every item of every page as meshStack sent it", func(t *testing.T) { - assert.Equal(t, workspaces, list(t, MeshWorkspaceListFilter{}, ListOptions{})) + assert.Equal(t, workspaces, list(t, MeshWorkspaceListFilter{}, types.ListOptions{})) }) t.Run("the list asks for newest first, and every page ends at the instant the list started", func(t *testing.T) { @@ -81,19 +82,19 @@ func TestRawClient(t *testing.T) { }) t.Run("a page size asks for pages of that size, and takes the smaller ones meshStack caps it to", func(t *testing.T) { - assert.Equal(t, workspaces, list(t, MeshWorkspaceListFilter{}, ListOptions{PageSize: 3})) + assert.Equal(t, workspaces, list(t, MeshWorkspaceListFilter{}, types.ListOptions{PageSize: 3})) assert.Equal(t, []string{"3", "3", "3"}, asked("size")) }) t.Run("the sort of a filter replaces newest first", func(t *testing.T) { - list(t, MeshWorkspaceListFilter{Sort: []string{"createdAt,desc", "name,asc"}}, ListOptions{}) + list(t, MeshWorkspaceListFilter{Sort: []string{"createdAt,desc", "name,asc"}}, types.ListOptions{}) assert.Equal(t, slices.Repeat([]string{"createdAt,desc name,asc"}, 3), asked("sort")) }) t.Run("a kind without a typed client fails before it asks meshStack", func(t *testing.T) { server.TakeRequests() var failed error - for _, err := range raw.List[MeshTenant](t.Context(), nil, ListOptions{}) { + for _, err := range raw.List[MeshTenant](t.Context(), nil, types.ListOptions{}) { failed = err } require.ErrorContains(t, failed, "client.MeshTenant") diff --git a/client/service_instance.go b/client/service_instance.go index 3b111413..f62ab334 100644 --- a/client/service_instance.go +++ b/client/service_instance.go @@ -3,9 +3,9 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" "github.com/meshcloud/meshstack-cli/client/types" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshServiceInstance struct { diff --git a/client/tenant_v4.go b/client/tenant_v4.go index 696723c4..43a2427f 100644 --- a/client/tenant_v4.go +++ b/client/tenant_v4.go @@ -5,9 +5,9 @@ import ( "errors" "fmt" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" "github.com/meshcloud/meshstack-cli/client/types/enum" - "github.com/meshcloud/meshstack-cli/internal/http" ) type TenantLifecycleState string diff --git a/client/types/page.go b/client/types/page.go new file mode 100644 index 00000000..70233977 --- /dev/null +++ b/client/types/page.go @@ -0,0 +1,13 @@ +package types + +type ListOptions struct { + PageSize int + OnPage func(Page) +} + +type Page struct { + Size int `json:"size"` + TotalElements int `json:"totalElements"` + TotalPages int `json:"totalPages"` + Number int `json:"number"` +} diff --git a/internal/version/version.go b/client/types/version/version.go similarity index 100% rename from internal/version/version.go rename to client/types/version/version.go diff --git a/internal/version/version_test.go b/client/types/version/version_test.go similarity index 100% rename from internal/version/version_test.go rename to client/types/version/version_test.go diff --git a/client/uuid_filter_test.go b/client/uuid_filter_test.go index 806c7680..9a0a8a2c 100644 --- a/client/uuid_filter_test.go +++ b/client/uuid_filter_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/meshcloud/meshstack-cli/client/types" "github.com/meshcloud/meshstack-cli/internal/testutil/fakemeshstack" ) @@ -19,7 +20,7 @@ func TestAUuidOfAFilterGoesAsItsTextAndAZeroOneNotAtAll(t *testing.T) { {BuildingBlockUuid: uuid.MustParse("0b5c1d3e-5f1a-4c2b-9d7e-2a6f8e4b1c90")}, {}, } { - for _, err := range raw.List[MeshBuildingBlockRun](t.Context(), filter, ListOptions{}) { + for _, err := range raw.List[MeshBuildingBlockRun](t.Context(), filter, types.ListOptions{}) { require.NoError(t, err) } } diff --git a/client/workspace_group_binding.go b/client/workspace_group_binding.go index 4c4da9c3..10f0e1b3 100644 --- a/client/workspace_group_binding.go +++ b/client/workspace_group_binding.go @@ -3,8 +3,8 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshWorkspaceGroupBinding struct { diff --git a/client/workspace_user_binding.go b/client/workspace_user_binding.go index 0c0a3179..126273ca 100644 --- a/client/workspace_user_binding.go +++ b/client/workspace_user_binding.go @@ -3,8 +3,8 @@ package client import ( "context" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/internal" - "github.com/meshcloud/meshstack-cli/internal/http" ) type MeshWorkspaceUserBinding struct { diff --git a/cmd/api/api.go b/cmd/api/api.go index b9424618..db2a3b3e 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -17,9 +17,9 @@ import ( "github.com/spf13/cobra" "github.com/meshcloud/meshstack-cli/client" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/openapi" "github.com/meshcloud/meshstack-cli/cmd/internal" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/setting" ) @@ -88,8 +88,11 @@ Replace the same command line with 'api-docs' in place of 'api' to describe the r.header.Set("Content-Type", "application/json") } - answer, err := meshStack.Raw.DoRequest(ctx, r.method, r.target.Path, - http.WithUrlQuery(r.target.Query()), http.WithHeaders(r.header), http.WithBody(r.body)) + options := []http.RequestOption{http.WithUrlQuery(r.target.Query()), http.WithBody(r.header.Get("Content-Type"), r.body)} + for key, values := range r.header { + options = append(options, http.WithHeader(key, values...)) + } + answer, err := meshStack.Raw.DoRequest(ctx, r.method, r.target.Path, options...) if httpErr, ok := errors.AsType[client.HttpError](err); ok { if writeErr := writeAnswer(cmd.OutOrStdout(), httpErr.ResponseBody); writeErr != nil { return writeErr diff --git a/cmd/buildingblock/tfstate/tfstate_test.go b/cmd/buildingblock/tfstate/tfstate_test.go index dcc124ad..24c1bb9a 100644 --- a/cmd/buildingblock/tfstate/tfstate_test.go +++ b/cmd/buildingblock/tfstate/tfstate_test.go @@ -17,9 +17,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/cmd/buildingblock/tfstate" "github.com/meshcloud/meshstack-cli/cmd/internal" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/meshstack" "github.com/meshcloud/meshstack-cli/internal/testutil/fakemeshstack" "github.com/meshcloud/meshstack-cli/internal/testutil/testlogin" @@ -75,7 +75,7 @@ func runChild(steps string) int { address, _ := url.Parse(os.Getenv("TF_HTTP_ADDRESS")) r := httptest.NewRequestWithContext(context.Background(), step, "/", nil) r.SetBasicAuth(os.Getenv("TF_HTTP_USERNAME"), os.Getenv("TF_HTTP_PASSWORD")) - _, _ = http.NewClient(fakemeshstack.UserAgent).DoRequest[[]byte](context.Background(), step, address, http.WithHeaders(r.Header), http.WithBody([]byte(state))) + _, _ = http.NewClient(fakemeshstack.UserAgent).DoRequest[[]byte](context.Background(), step, address, http.WithHeader("Authorization", r.Header.Get("Authorization")), http.WithBody("application/json", []byte(state))) } return 0 } diff --git a/cmd/buildingblockrun/list.go b/cmd/buildingblockrun/list.go index cc4071cd..48759081 100644 --- a/cmd/buildingblockrun/list.go +++ b/cmd/buildingblockrun/list.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/cobra" "github.com/meshcloud/meshstack-cli/client" + "github.com/meshcloud/meshstack-cli/client/types" "github.com/meshcloud/meshstack-cli/cmd/internal" ) @@ -45,7 +46,7 @@ credential can see.`, if blockFilter.WorkspaceIdentifier, err = internal.ListWorkspace(cmd.Context()); err != nil { return err } - return flags.RunSeq(cmd, func(ctx context.Context, meshStack client.Client, options client.ListOptions) iter.Seq2[jsontext.Value, error] { + return flags.RunSeq(cmd, func(ctx context.Context, meshStack client.Client, options types.ListOptions) iter.Seq2[jsontext.Value, error] { return allRuns(ctx, meshStack, blockFilter, options.PageSize) }) }, @@ -73,7 +74,7 @@ func allRuns(ctx context.Context, meshStack client.Client, blockFilter client.Me var runsOfEachBlock []iter.Seq2[jsontext.Value, error] // The blocks are read raw because only their uuid is needed, and a block the client cannot // fully decode still has runs to list. - for rawBlock, err := range meshStack.Raw.List[client.MeshBuildingBlockV2](ctx, blockFilter, client.ListOptions{}) { + for rawBlock, err := range meshStack.Raw.List[client.MeshBuildingBlockV2](ctx, blockFilter, types.ListOptions{}) { if err != nil { yield(nil, err) return @@ -100,7 +101,7 @@ func allRuns(ctx context.Context, meshStack client.Client, blockFilter client.Me func runsOf(ctx context.Context, meshStack client.Client, buildingBlockUuid uuid.UUID, pageSize int) iter.Seq2[jsontext.Value, error] { return func(yield func(jsontext.Value, error) bool) { filter := client.MeshBuildingBlockRunListFilter{BuildingBlockUuid: buildingBlockUuid} - for run, err := range meshStack.Raw.List[client.MeshBuildingBlockRun](ctx, filter, client.ListOptions{PageSize: pageSize}) { + for run, err := range meshStack.Raw.List[client.MeshBuildingBlockRun](ctx, filter, types.ListOptions{PageSize: pageSize}) { if err != nil { err = fmt.Errorf("listing the runs of building block %s: %w", buildingBlockUuid, err) } diff --git a/cmd/buildingblockrun/logs.go b/cmd/buildingblockrun/logs.go index bd788e6d..9e352f32 100644 --- a/cmd/buildingblockrun/logs.go +++ b/cmd/buildingblockrun/logs.go @@ -78,7 +78,7 @@ const ( const ( pollInterval = 2 * time.Second maxPollWait = time.Minute - // The HTTP client has retried each failed poll for minutes already, see internal/http. + // The HTTP client has retried each failed poll for minutes already, see client/http. maxFailedPollsInARow = 5 ) diff --git a/cmd/internal/list.go b/cmd/internal/list.go index ff040d6b..113918de 100644 --- a/cmd/internal/list.go +++ b/cmd/internal/list.go @@ -12,6 +12,7 @@ import ( "github.com/spf13/pflag" "github.com/meshcloud/meshstack-cli/client" + "github.com/meshcloud/meshstack-cli/client/types" "github.com/meshcloud/meshstack-cli/pkg/auth" ) @@ -41,22 +42,22 @@ func ListWorkspace(ctx context.Context) (*string, error) { // Run lists the objects of M that filter narrows. func (f *ListFlags) Run[M any](cmd *cobra.Command, filter any) error { - return f.RunSeq(cmd, func(ctx context.Context, meshStack client.Client, options client.ListOptions) iter.Seq2[jsontext.Value, error] { + return f.RunSeq(cmd, func(ctx context.Context, meshStack client.Client, options types.ListOptions) iter.Seq2[jsontext.Value, error] { return meshStack.Raw.List[M](ctx, filter, options) }) } // RunSeq is Run for a listing that is not one kind's, such as the runs of every building block. -func (f *ListFlags) RunSeq(cmd *cobra.Command, list func(ctx context.Context, meshStack client.Client, options client.ListOptions) iter.Seq2[jsontext.Value, error]) error { +func (f *ListFlags) RunSeq(cmd *cobra.Command, list func(ctx context.Context, meshStack client.Client, options types.ListOptions) iter.Seq2[jsontext.Value, error]) error { ctx := cmd.Context() meshStack, err := ResolveClient(ctx) if err != nil { return err } result := ListResult{Limit: int(f.limit), Defaulted: !cmd.Flags().Changed(limitFlagName)} - listOptions := client.ListOptions{ + listOptions := types.ListOptions{ PageSize: result.Limit, - OnPage: func(page client.Page) { + OnPage: func(page types.Page) { if result.Total == nil { result.Total = &page.TotalElements } diff --git a/cmd/internal/testacc/api_get_test.go b/cmd/internal/testacc/api_get_test.go index cb5b5232..6a5b54cb 100644 --- a/cmd/internal/testacc/api_get_test.go +++ b/cmd/internal/testacc/api_get_test.go @@ -18,10 +18,10 @@ import ( "github.com/stretchr/testify/require" "github.com/meshcloud/meshstack-cli/client" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/openapi" "github.com/meshcloud/meshstack-cli/cmd/internal" "github.com/meshcloud/meshstack-cli/internal/apidocs" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/pkg/auth" ) @@ -174,7 +174,7 @@ type listed struct { func (o *firstObjects) get(ctx context.Context, r getRequest, operation openapi.Operation) (int, string, error) { options := []http.RequestOption{http.WithUrlQuery(r.query)} if mediaType, ok := operation.LatestMediaType(); ok { - options = append(options, http.WithAccept(mediaType.Name)) + options = append(options, http.WithHeader("Accept", mediaType.Name)) } body, err := o.meshStack.Raw.DoRequest(ctx, gohttp.MethodGet, r.path, options...) if httpErr, ok := errors.AsType[client.HttpError](err); ok { diff --git a/cmd/internal/testacc/api_test.go b/cmd/internal/testacc/api_test.go index f58beb68..a982933a 100644 --- a/cmd/internal/testacc/api_test.go +++ b/cmd/internal/testacc/api_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "github.com/meshcloud/meshstack-cli/client" + "github.com/meshcloud/meshstack-cli/client/types" ) // apiAsksForJson checks the answers of meshStack that request.negotiate in cmd/api relies on where @@ -28,7 +29,7 @@ func apiAsksForJson(c *cli) func(*testing.T) { func apiFollowsASelfLink(c *cli) func(*testing.T) { return func(t *testing.T) { var href string - for item, err := range c.client(t).Raw.List[client.MeshWorkspace](t.Context(), nil, client.ListOptions{PageSize: 1}) { + for item, err := range c.client(t).Raw.List[client.MeshWorkspace](t.Context(), nil, types.ListOptions{PageSize: 1}) { require.NoError(t, err) var links struct { Links struct { diff --git a/cmd/meshstack/meshstack.go b/cmd/meshstack/meshstack.go index 0c85dd37..0cd98878 100644 --- a/cmd/meshstack/meshstack.go +++ b/cmd/meshstack/meshstack.go @@ -28,7 +28,7 @@ import ( func main() { // No deadline bounds a command as a whole, since a listing takes as long as it is long. The - // HTTP client gives up on a server that stopped answering, see internal/http. + // HTTP client gives up on a server that stopped answering, see client/http. ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) err := newRootCommand().ExecuteContext(ctx) stop() diff --git a/go.mod b/go.mod index 23a8a78a..5629aeab 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/meshcloud/meshstack-cli -// 1.27 is the floor because internal/http declares generic methods, which no earlier release +// 1.27 is the floor because client/http declares generic methods, which no earlier release // compiles. Keep the pins in flake.nix (go_1_27 and GOROOT) and in the // Dockerfile in lock-step. Raising it also breaks the nixpkgs package // (buildGo127Module in pkgs/by-name/me/meshstack-cli/package.nix of NixOS/nixpkgs): diff --git a/internal/apidocs/apidocs.go b/internal/apidocs/apidocs.go index 45bd30ba..ebe2ee76 100644 --- a/internal/apidocs/apidocs.go +++ b/internal/apidocs/apidocs.go @@ -10,12 +10,12 @@ import ( "fmt" "os" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/openapi" + "github.com/meshcloud/meshstack-cli/client/types/version" "github.com/meshcloud/meshstack-cli/client/types/xurl" "github.com/meshcloud/meshstack-cli/internal/config" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/setting" - "github.com/meshcloud/meshstack-cli/internal/version" ) var ( @@ -68,7 +68,7 @@ func Load(ctx context.Context, opts Options) (spec openapi.Spec, wait func(), er url, path, conditionalGet = custom, dir.Join(fmt.Sprintf("api-docs-custom-%x.json", urlHash[:6])), true } - wait, err = opts.Client.Download(ctx, url.URL, path, http.WithConditionalGet(conditionalGet)) + wait, err = download(ctx, opts.Client, url.URL, path, conditionalGet) if err != nil { return spec, wait, fmt.Errorf("cannot download the API docs from %s: %w", url, err) } diff --git a/internal/http/download.go b/internal/apidocs/download.go similarity index 78% rename from internal/http/download.go rename to internal/apidocs/download.go index b403a0ae..bed60c20 100644 --- a/internal/http/download.go +++ b/internal/apidocs/download.go @@ -1,4 +1,4 @@ -package http +package apidocs import ( "context" @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/internal/json" ) @@ -25,36 +26,18 @@ const ( downloadCheckInterval = 24 * time.Hour ) -type ( - DownloadOption func(opts *downloadOptions) - - downloadOptions struct { - conditionalGet bool - } -) - -// WithConditionalGet asks for a newer document on every download. Without it, Download asks -// once per downloadCheckInterval, to save the round trip. -func WithConditionalGet(conditionalGet bool) DownloadOption { - return func(opts *downloadOptions) { - opts.conditionalGet = conditionalGet - } -} - -// Download waits at most waitForNewerDownload for a newer document where path holds one already. +// download waits at most waitForNewerDownload for a newer document where path holds one already. // After that, the caller reads the stored document, and wait lets the download finish for the next -// command. -func (c Client) Download(ctx context.Context, url *url.URL, path string, options ...DownloadOption) (wait func(), err error) { - var opts downloadOptions - for _, option := range options { - option(&opts) - } +// command. Without conditionalGet, it asks for a newer document once per downloadCheckInterval, to +// save the round trip. +func download(ctx context.Context, client http.Client, url *url.URL, path string, conditionalGet bool) (wait func(), err error) { + opts := downloadOptions{conditionalGet: conditionalGet} noWait := func() {} stored, statErr := os.Stat(path) if statErr != nil { downloadCtx, cancel := context.WithTimeout(ctx, downloadTimeout) defer cancel() - if err := c.download(downloadCtx, url, path, nil); err != nil { + if err := downloadTo(downloadCtx, client, url, path); err != nil { return noWait, err } return noWait, opts.recordCheck(ctx, path) @@ -69,9 +52,8 @@ func (c Client) Download(ctx context.Context, url *url.URL, path string, options // signal.NotifyContext swallows every further Ctrl-C. downloadCtx, cancel := context.WithTimeout(ctx, downloadTimeout) defer cancel() - err := c.download(downloadCtx, url, path, []RequestOption{ - withHeader("If-Modified-Since", stored.ModTime().UTC().Format(gohttp.TimeFormat)), - }) + err := downloadTo(downloadCtx, client, url, path, + http.WithHeader("If-Modified-Since", stored.ModTime().UTC().Format(gohttp.TimeFormat))) // A cancelled download leaves the check due, so that the next command tries again. if ctx.Err() != nil { done <- nil @@ -95,6 +77,10 @@ func (c Client) Download(ctx context.Context, url *url.URL, path string, options } } +type downloadOptions struct { + conditionalGet bool +} + type downloadCheck struct { LastCheck time.Time `json:"lastCheck"` } @@ -127,11 +113,11 @@ func (opts downloadOptions) recordCheck(ctx context.Context, path string) error return json.MarshalTo(ctx, lastCheckJson(path), downloadCheck{time.Now()}) } -func (c Client) download(ctx context.Context, url *url.URL, path string, options []RequestOption) (err error) { +func downloadTo(ctx context.Context, client http.Client, url *url.URL, path string, options ...http.RequestOption) (err error) { // No option sets an Accept-Encoding, so that net/http asks for gzip and decompresses the answer: // the API docs then come as 0.5MB rather than 4.7MB. CloudFront serves brotli as well, but it // saves only 4 to 8% over gzip, too little for a dependency. - res, err := c.send(ctx, gohttp.MethodGet, url, options) + res, err := client.Send(ctx, http.MethodGet, url, options...) if err != nil { return err } @@ -142,9 +128,9 @@ func (c Client) download(ctx context.Context, url *url.URL, path string, options slog.DebugContext(ctx, path+" is up to date") return nil } - if !isSuccess(res.StatusCode) { - _, err = c.readBodyAndCheckSuccess(ctx, res) - return err + if res.StatusCode/100 != 2 { + body, _ := io.ReadAll(res.Body) + return http.Error{Method: http.MethodGet, Url: url.String(), StatusCode: res.StatusCode, ResponseBody: body} } // The document is renamed into place, so that no reader sees half of it. diff --git a/internal/http/download_test.go b/internal/apidocs/download_test.go similarity index 79% rename from internal/http/download_test.go rename to internal/apidocs/download_test.go index 45e73223..18fa9ede 100644 --- a/internal/http/download_test.go +++ b/internal/apidocs/download_test.go @@ -1,9 +1,11 @@ -package http_test +package apidocs import ( "compress/gzip" "log/slog" gohttp "net/http" + "net/http/httptest" + "net/url" "os" "path/filepath" "strings" @@ -14,8 +16,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/meshcloud/meshstack-cli/internal/http" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/internal/logs" + "github.com/meshcloud/meshstack-cli/internal/testutil/fakemeshstack" ) type fakeDocument struct { @@ -59,10 +62,10 @@ func (d *fakeDocument) serve(resp gohttp.ResponseWriter, req *gohttp.Request) { func TestDownload(t *testing.T) { path := filepath.Join(t.TempDir(), "doc.json") document := &fakeDocument{body: `{"version": 1}`, modified: time.Date(2026, 9, 30, 13, 49, 36, 0, time.UTC)} - download := func(t *testing.T, options ...http.DownloadOption) (wait func()) { + get := func(t *testing.T, conditionalGet bool) (wait func()) { t.Helper() - client := newTestClientWithServer(t, document.serve) - wait, err := client.Download(t.Context(), client.ServerUrl, path, options...) + client, serverUrl := newTestClient(t, document.serve) + wait, err := download(t.Context(), client, serverUrl, path, conditionalGet) require.NoError(t, err) return wait } @@ -71,7 +74,7 @@ func TestDownload(t *testing.T) { synctest.Test(t, func(t *testing.T) { captured := logs.Capture(t) - download(t)() + get(t, false)() assert.Equal(t, []string{"gzip"}, document.contentEncodings, "the 4.7MB of the API docs come as 0.5MB") assert.JSONEq(t, `{"version": 1}`, readDocument(t, path), "the stored document is decompressed") stat, err := os.Stat(path) @@ -79,18 +82,18 @@ func TestDownload(t *testing.T) { assert.True(t, document.modified.Equal(stat.ModTime()), "the mtime is the Last-Modified") synctest.Sleep(23 * time.Hour) - download(t)() + get(t, false)() assert.Equal(t, []int{gohttp.StatusOK}, document.answers, "a check within a day is not due") synctest.Sleep(time.Hour) - download(t)() + get(t, false)() assert.Equal(t, []int{gohttp.StatusOK, gohttp.StatusNotModified}, document.answers) assert.Empty(t, captured.Records(slog.LevelWarn)) synctest.Sleep(24 * time.Hour) document.missing = true - download(t)() - download(t)() + get(t, false)() + get(t, false)() assert.Equal(t, []int{gohttp.StatusOK, gohttp.StatusNotModified, gohttp.StatusNotFound}, document.answers) require.Len(t, captured.Records(slog.LevelWarn), 1) assert.Contains(t, captured.Lines(slog.LevelWarn)[0], "Cannot update "+path) @@ -105,7 +108,7 @@ func TestDownload(t *testing.T) { document.body, document.modified = `{"version": 2}`, document.modified.Add(time.Hour) document.release = make(chan struct{}) - wait := download(t, http.WithConditionalGet(true)) + wait := get(t, true) assert.JSONEq(t, `{"version": 1}`, readDocument(t, path), "the caller reads the stored document") require.Len(t, captured.Records(slog.LevelWarn), 1) assert.Contains(t, captured.Lines(slog.LevelWarn)[0], "The command ends once the download has finished.") @@ -117,10 +120,10 @@ func TestDownload(t *testing.T) { }) t.Run("fails without a stored document to fall back to, and leaves nothing behind", func(t *testing.T) { - client := newTestClientWithServer(t, (&fakeDocument{missing: true}).serve) + client, serverUrl := newTestClient(t, (&fakeDocument{missing: true}).serve) dir := t.TempDir() - _, err := client.Download(t.Context(), client.ServerUrl, filepath.Join(dir, "doc.json")) + _, err := download(t.Context(), client, serverUrl, filepath.Join(dir, "doc.json"), false) require.ErrorContains(t, err, "404") entries, err := os.ReadDir(dir) @@ -129,6 +132,16 @@ func TestDownload(t *testing.T) { }) } +func newTestClient(t *testing.T, handlerFunc gohttp.HandlerFunc) (http.Client, *url.URL) { + t.Helper() + // In memory, so that a test can run inside a synctest bubble. + server := httptest.NewTestServer(t, handlerFunc) + client := http.Client{Client: server.Client(), UserAgent: fakemeshstack.UserAgent} + serverUrl, err := url.Parse(server.URL) + require.NoError(t, err) + return client, serverUrl +} + func readDocument(t *testing.T, path string) string { t.Helper() content, err := os.ReadFile(filepath.Clean(path)) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 13bf060c..541617d0 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -8,7 +8,7 @@ import ( "time" "github.com/meshcloud/meshstack-cli/client" - "github.com/meshcloud/meshstack-cli/internal/http" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/internal/meshstack" "github.com/meshcloud/meshstack-cli/internal/oidc/jwt" ) diff --git a/internal/auth/credential/apikey.go b/internal/auth/credential/apikey.go index bb61217a..cfc69195 100644 --- a/internal/auth/credential/apikey.go +++ b/internal/auth/credential/apikey.go @@ -6,8 +6,8 @@ import ( "fmt" "uuid" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/xurl" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/oidc/jwt" ) diff --git a/internal/auth/credential/credential.go b/internal/auth/credential/credential.go index 1b5e214d..f0b1bcfb 100644 --- a/internal/auth/credential/credential.go +++ b/internal/auth/credential/credential.go @@ -7,7 +7,7 @@ import ( "reflect" "strings" - "github.com/meshcloud/meshstack-cli/internal/http" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/internal/meshstack" "github.com/meshcloud/meshstack-cli/internal/oidc/jwt" ) diff --git a/internal/auth/credential/manual.go b/internal/auth/credential/manual.go index 55a27dbc..66a7e6cd 100644 --- a/internal/auth/credential/manual.go +++ b/internal/auth/credential/manual.go @@ -4,8 +4,8 @@ import ( "context" "fmt" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/xurl" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/oidc/jwt" ) diff --git a/internal/auth/credential/oidclogin.go b/internal/auth/credential/oidclogin.go index ccf2173c..44bfc7cd 100644 --- a/internal/auth/credential/oidclogin.go +++ b/internal/auth/credential/oidclogin.go @@ -8,8 +8,8 @@ import ( gohttp "net/http" "time" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/xurl" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/meshstack" "github.com/meshcloud/meshstack-cli/internal/oidc" "github.com/meshcloud/meshstack-cli/internal/oidc/jwt" diff --git a/internal/auth/login.go b/internal/auth/login.go index ccb87644..41cce365 100644 --- a/internal/auth/login.go +++ b/internal/auth/login.go @@ -8,8 +8,8 @@ import ( "sync" "github.com/meshcloud/meshstack-cli/client" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/internal/auth/credential" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/meshstack" "github.com/meshcloud/meshstack-cli/internal/profile" "github.com/meshcloud/meshstack-cli/internal/setting" diff --git a/internal/auth/session.go b/internal/auth/session.go index e61108c2..f76e1b8b 100644 --- a/internal/auth/session.go +++ b/internal/auth/session.go @@ -8,9 +8,9 @@ import ( "sync" "github.com/meshcloud/meshstack-cli/client" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/xurl" "github.com/meshcloud/meshstack-cli/internal/auth/credential" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/meshstack" "github.com/meshcloud/meshstack-cli/internal/profile" "github.com/meshcloud/meshstack-cli/internal/setting" diff --git a/internal/auth/session_test.go b/internal/auth/session_test.go index 5144f760..48aa210f 100644 --- a/internal/auth/session_test.go +++ b/internal/auth/session_test.go @@ -13,11 +13,11 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/xurl" "github.com/meshcloud/meshstack-cli/internal/auth" "github.com/meshcloud/meshstack-cli/internal/auth/credential" "github.com/meshcloud/meshstack-cli/internal/config" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/logs" "github.com/meshcloud/meshstack-cli/internal/meshstack" "github.com/meshcloud/meshstack-cli/internal/profile" diff --git a/internal/auth/status.go b/internal/auth/status.go index 539a3568..4b2f5290 100644 --- a/internal/auth/status.go +++ b/internal/auth/status.go @@ -12,9 +12,9 @@ import ( "uuid" "github.com/meshcloud/meshstack-cli/client" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/xurl" "github.com/meshcloud/meshstack-cli/internal/auth/credential" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/meshstack" "github.com/meshcloud/meshstack-cli/internal/oidc/jwt" "github.com/meshcloud/meshstack-cli/internal/profile" diff --git a/internal/auth/version_check.go b/internal/auth/version_check.go index 7ceb31ed..91ce02f0 100644 --- a/internal/auth/version_check.go +++ b/internal/auth/version_check.go @@ -8,11 +8,11 @@ import ( "log/slog" "time" + "github.com/meshcloud/meshstack-cli/client/http" + "github.com/meshcloud/meshstack-cli/client/types/version" "github.com/meshcloud/meshstack-cli/client/types/xurl" "github.com/meshcloud/meshstack-cli/internal/config" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/json" - "github.com/meshcloud/meshstack-cli/internal/version" ) const ( @@ -20,7 +20,7 @@ const ( releaseCheckTimeout = 10 * time.Second ) -// warnIfNewerReleasePresent lives here rather than in internal/version, which the depguard rule +// warnIfNewerReleasePresent lives here rather than in client/types/version, which the depguard rule // in .golangci.yml keeps closed to everything but the standard library. func warnIfNewerReleasePresent(ctx context.Context, configDir config.Directory, client http.Client, opts ResolveSessionOptions) (err error) { currentVersion, err := version.Parse(opts.Version) diff --git a/internal/auth/workspace_test.go b/internal/auth/workspace_test.go index c72234d3..4f5e8f1b 100644 --- a/internal/auth/workspace_test.go +++ b/internal/auth/workspace_test.go @@ -7,10 +7,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/xurl" "github.com/meshcloud/meshstack-cli/internal/auth" "github.com/meshcloud/meshstack-cli/internal/auth/credential" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/meshstack" "github.com/meshcloud/meshstack-cli/internal/oidc" "github.com/meshcloud/meshstack-cli/internal/oidc/jwt" diff --git a/internal/oidc/client.go b/internal/oidc/client.go index f5482ac6..9b669660 100644 --- a/internal/oidc/client.go +++ b/internal/oidc/client.go @@ -8,8 +8,8 @@ import ( "net/url" "time" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/xurl" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/json" "github.com/meshcloud/meshstack-cli/internal/oidc/jwt" "github.com/meshcloud/meshstack-cli/internal/oidc/scope" diff --git a/internal/oidc/client_test.go b/internal/oidc/client_test.go index 53981140..de6cb87f 100644 --- a/internal/oidc/client_test.go +++ b/internal/oidc/client_test.go @@ -11,8 +11,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/client/types/xurl" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/oidc" "github.com/meshcloud/meshstack-cli/internal/oidc/scope" "github.com/meshcloud/meshstack-cli/internal/testutil/fakemeshstack" diff --git a/internal/testutil/fakemeshstack/fakemeshstack.go b/internal/testutil/fakemeshstack/fakemeshstack.go index 11b60027..c8e45624 100644 --- a/internal/testutil/fakemeshstack/fakemeshstack.go +++ b/internal/testutil/fakemeshstack/fakemeshstack.go @@ -24,7 +24,7 @@ import ( "testing" "time" - "github.com/meshcloud/meshstack-cli/internal/http" + "github.com/meshcloud/meshstack-cli/client/http" ) // DefaultVersion is at least client.MinMeshStackVersion, or the CLI refuses the backend. diff --git a/internal/tfstate/apiproxy/apiproxy.go b/internal/tfstate/apiproxy/apiproxy.go index b2944a78..1bbc56ef 100644 --- a/internal/tfstate/apiproxy/apiproxy.go +++ b/internal/tfstate/apiproxy/apiproxy.go @@ -1,6 +1,6 @@ // Package apiproxy passes requests on to the meshStack API. It stands apart from internal/tfstate // because httputil.ReverseProxy takes a transport rather than a client, and forbidigo keeps the -// transport to internal/http and this package. +// transport to client/http and this package. package apiproxy import ( @@ -9,7 +9,7 @@ import ( "net/url" "strings" - "github.com/meshcloud/meshstack-cli/internal/http" + "github.com/meshcloud/meshstack-cli/client/http" ) // New replaces the Authorization a request came with by that of client, and passes a request diff --git a/internal/tfstate/lock.go b/internal/tfstate/lock.go index 3701a580..03a2dbdb 100644 --- a/internal/tfstate/lock.go +++ b/internal/tfstate/lock.go @@ -9,7 +9,7 @@ import ( "uuid" "github.com/meshcloud/meshstack-cli/client" - "github.com/meshcloud/meshstack-cli/internal/http" + "github.com/meshcloud/meshstack-cli/client/http" ) type Lock struct { diff --git a/internal/tfstate/proxy.go b/internal/tfstate/proxy.go index 9673ff8b..17b7cc58 100644 --- a/internal/tfstate/proxy.go +++ b/internal/tfstate/proxy.go @@ -17,8 +17,8 @@ import ( "sync" "time" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/internal/auth" - "github.com/meshcloud/meshstack-cli/internal/http" "github.com/meshcloud/meshstack-cli/internal/meshstack" "github.com/meshcloud/meshstack-cli/internal/profile" "github.com/meshcloud/meshstack-cli/internal/tfstate/apiproxy" diff --git a/internal/tfstate/proxy_test.go b/internal/tfstate/proxy_test.go index 3a52a03b..fdd7caa4 100644 --- a/internal/tfstate/proxy_test.go +++ b/internal/tfstate/proxy_test.go @@ -20,7 +20,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/meshcloud/meshstack-cli/internal/http" + "github.com/meshcloud/meshstack-cli/client/http" "github.com/meshcloud/meshstack-cli/internal/oidc/jwt" "github.com/meshcloud/meshstack-cli/internal/testutil/fakemeshstack" ) @@ -347,9 +347,8 @@ func TestServeCountsTheRequestsUntilTheCommandEnds(t *testing.T) { var apiToken jwt.JWT require.NoError(t, apiToken.UnmarshalText([]byte(vars["MESHSTACK_API_TOKEN"])), "the provider since v0.26.0 takes only a JWT") assert.False(t, apiToken.GetClaim(jwt.ExpiryClaim).Expired(time.Hour)) - authorization := gohttp.Header{} - authorization.Set("Authorization", "Basic "+basicAuth(vars["TF_HTTP_USERNAME"], vars["TF_HTTP_PASSWORD"])) - served, err = http.NewClient(fakemeshstack.UserAgent).DoRequest[[]byte](t.Context(), http.MethodGet, address, http.WithHeaders(authorization)) + authorization := http.WithHeader("Authorization", "Basic "+basicAuth(vars["TF_HTTP_USERNAME"], vars["TF_HTTP_PASSWORD"])) + served, err = http.NewClient(fakemeshstack.UserAgent).DoRequest[[]byte](t.Context(), http.MethodGet, address, authorization) require.NoError(t, err) }) diff --git a/internal/tfstate/store.go b/internal/tfstate/store.go index 46872449..3476b4c3 100644 --- a/internal/tfstate/store.go +++ b/internal/tfstate/store.go @@ -7,10 +7,9 @@ import ( "encoding/json/v2" "errors" "fmt" - gohttp "net/http" "uuid" - "github.com/meshcloud/meshstack-cli/internal/http" + "github.com/meshcloud/meshstack-cli/client/http" ) // ErrNoState is meshStack's 404, which tofu's http backend takes for a state not written yet. @@ -45,7 +44,7 @@ func (s Store) Get(ctx context.Context) ([]byte, error) { // Put and Delete send the ID of tofu's lock as tofu's http backend does, because meshStack refuses // a write without the ID of the lock it holds. func (s Store) Put(ctx context.Context, state []byte, lockId string) error { - _, err := s.raw.DoRequest(ctx, http.MethodPost, s.path(), withLockId(lockId, http.WithBody(state), jsonContent)...) + _, err := s.raw.DoRequest(ctx, http.MethodPost, s.path(), withLockId(lockId, http.WithBody("application/json", state))...) return s.wrap("store", err) } @@ -61,19 +60,17 @@ func withLockId(lockId string, opts ...http.RequestOption) []http.RequestOption return append(opts, http.WithUrlQuery(map[string]string{"ID": lockId})) } -var jsonContent = http.WithHeaders(gohttp.Header{"Content-Type": {"application/json"}}) - // Lock and Unlock take tofu's lock info as tofu sends it. A Lock that meshStack refuses with 423 // is an [http.Error] that carries the lock info of the holder. func (s Store) Lock(ctx context.Context, info []byte) error { - _, err := s.raw.DoRequest(ctx, http.MethodPost, s.lockPath(), http.WithBody(info), jsonContent) + _, err := s.raw.DoRequest(ctx, http.MethodPost, s.lockPath(), http.WithBody("application/json", info)) return s.wrap("lock", err) } // Unlock fails with a 423 of meshStack where meshStack holds a lock of another ID than the one in // info. func (s Store) Unlock(ctx context.Context, info []byte) error { - _, err := s.raw.DoRequest(ctx, http.MethodDelete, s.lockPath(), http.WithBody(info), jsonContent) + _, err := s.raw.DoRequest(ctx, http.MethodDelete, s.lockPath(), http.WithBody("application/json", info)) return s.wrap("unlock", err) } From f0a46b689596c18bf8c0b62f4456c46e9ce6ab7b Mon Sep 17 00:00:00 2001 From: Andreas Grub Date: Tue, 13 Oct 2026 19:36:35 +0200 Subject: [PATCH 2/3] feat(client): add the runner API as client.Runner A building block runner claims runs, registers as a status source, reports status and moves the plan artifact through client.Runner, with the run token for every request about a run. Co-Authored-By: Claude Opus 5.5 --- client/client.go | 2 + client/http/auth.go | 8 + client/http/http_client.go | 44 ++- client/http/http_client_test.go | 21 ++ client/http/logging.go | 3 + client/http/method.go | 1 + client/http/options.go | 8 + client/runner.go | 470 ++++++++++++++++++++++++++++++++ client/runner_test.go | 167 ++++++++++++ client/testdata/runner_run.json | 56 ++++ 10 files changed, 779 insertions(+), 1 deletion(-) create mode 100644 client/runner.go create mode 100644 client/runner_test.go create mode 100644 client/testdata/runner_run.json diff --git a/client/client.go b/client/client.go index 717cba40..348a2de6 100644 --- a/client/client.go +++ b/client/client.go @@ -41,6 +41,7 @@ type Client struct { WorkspaceGroupBinding MeshWorkspaceGroupBindingClient WorkspaceUserBinding MeshWorkspaceUserBindingClient Raw *RawClient + Runner MeshRunnerClient // Endpoint is read by the Terraform provider's meshstack_instance data source. Endpoint xurl.URL @@ -88,6 +89,7 @@ func New(ctx context.Context, endpoint xurl.URL, client http.Client, auth http.A with(buildingBlockDefinition.meshObject). withOwnOrder(buildingBlockDefinitionVersion.meshObject). with(newEventLogMeshObject(ctx, authorizedClient)), + Runner: newRunnerClient(authorizedClient), Endpoint: endpoint, } diff --git a/client/http/auth.go b/client/http/auth.go index 584b833d..470cdc28 100644 --- a/client/http/auth.go +++ b/client/http/auth.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io" "log/slog" gohttp "net/http" "net/url" @@ -43,6 +44,13 @@ func (c AuthorizedClient) DoRequest[R any](ctx context.Context, method string, u return result, err } +func (c AuthorizedClient) DoStreamingRequest(ctx context.Context, method string, url *url.URL, w io.Writer, options ...RequestOption) error { + _, err := withBearerToken(ctx, c.Authorization, method, url, func(token RequestOption) (any, error) { + return nil, c.Client.DoStreamingRequest(ctx, method, url, w, append(options, token)...) + }) + return err +} + // Authorize is for a request that this client does not send itself, such as one a reverse proxy // passes on. Such a request gets no second try after a 401. func (c AuthorizedClient) Authorize(r *gohttp.Request) error { diff --git a/client/http/http_client.go b/client/http/http_client.go index 97b34f61..93da11ee 100644 --- a/client/http/http_client.go +++ b/client/http/http_client.go @@ -8,6 +8,7 @@ import ( "log/slog" gohttp "net/http" "net/url" + "os" "reflect" "slices" "time" @@ -115,6 +116,24 @@ func (c Client) DoRequest[R any](ctx context.Context, method string, url *url.UR return } +// DoStreamingRequest copies a 2xx body to w as it arrives. +func (c Client) DoStreamingRequest(ctx context.Context, method string, url *url.URL, w io.Writer, options ...RequestOption) error { + res, err := c.Send(ctx, method, url, options...) + if err != nil { + return err + } + defer func() { + _ = res.Body.Close() + }() + if !isSuccess(res.StatusCode) { + _, err = c.readBodyAndCheckSuccess(ctx, res) + return err + } + written, err := io.Copy(w, res.Body) + slog.DebugContext(ctx, "response", "status", res.StatusCode, "body", fmt.Sprintf("", written)) + return err +} + // Send returns the response whatever its status, and the caller closes its body. func (c Client) Send(ctx context.Context, method string, url *url.URL, options ...RequestOption) (*gohttp.Response, error) { options = slices.Insert(options, 0, WithHeader("User-Agent", c.UserAgent.String())) @@ -164,7 +183,7 @@ func (c Client) readBodyAndCheckSuccess(ctx context.Context, res *gohttp.Respons } func (c Client) buildRequest(ctx context.Context, method string, url *url.URL, opts requestOptions) (*gohttp.Request, error) { - var requestBody io.ReadWriter + var requestBody io.Reader if opts.requestPayload != nil { requestBodyData, err := opts.requestPayload() if err != nil { @@ -181,6 +200,12 @@ func (c Client) buildRequest(ctx context.Context, method string, url *url.URL, o if err != nil { return nil, fmt.Errorf("failed to create request: %w", err) } + if opts.bodyFile != "" { + if err := setFileBody(req, opts.bodyFile); err != nil { + return nil, err + } + requestBody = req.Body + } for _, modifier := range opts.requestModifiers { if err := modifier(req); err != nil { return nil, err @@ -189,3 +214,20 @@ func (c Client) buildRequest(ctx context.Context, method string, url *url.URL, o slog.DebugContext(ctx, "request", "url", req.URL.String(), "method", req.Method, "headers", loggedHeaders(req.Header), "body", loggedBody{requestBody}) return req, nil } + +// setFileBody streams the file, and reopens it when a retry replays the request. +func setFileBody(req *gohttp.Request, path string) error { + info, err := os.Stat(path) + if err != nil { + return err + } + open := func() (io.ReadCloser, error) { + return os.Open(path) //nolint:gosec // the caller names the file it uploads + } + if req.Body, err = open(); err != nil { + return err + } + req.GetBody = open + req.ContentLength = info.Size() + return nil +} diff --git a/client/http/http_client_test.go b/client/http/http_client_test.go index 316cf32d..8d79106e 100644 --- a/client/http/http_client_test.go +++ b/client/http/http_client_test.go @@ -12,6 +12,8 @@ import ( gohttp "net/http" "net/http/httptest" "net/url" + "os" + "path/filepath" "testing" "testing/synctest" "time" @@ -171,6 +173,25 @@ func TestHttpClient(t *testing.T) { } }) + t.Run("DoRequest reopens a body file to replay it", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "plan") + require.NoError(t, os.WriteFile(path, []byte("a plan"), 0o600)) + var bodies []string + client := withTestRetry(newTestClientWithServer(t, func(resp gohttp.ResponseWriter, req *gohttp.Request) { + body, _ := io.ReadAll(req.Body) + bodies = append(bodies, string(body)) + if len(bodies) == 1 { + resp.WriteHeader(gohttp.StatusServiceUnavailable) + return + } + resp.WriteHeader(gohttp.StatusNoContent) + }), 3, &retryTestBackoff{}) + + _, err := client.DoRequest[any](t.Context(), gohttp.MethodPut, client.ServerUrl, http.WithBodyFile("application/octet-stream", path), http.Retryable()) + require.NoError(t, err) + assert.Equal(t, []string{"a plan", "a plan"}, bodies) + }) + t.Run("DoRequest re-mints once on 401", func(t *testing.T) { t.Run("retries with the freshly minted token", func(t *testing.T) { auth := &refreshableAuthorization{token: "stale"} diff --git a/client/http/logging.go b/client/http/logging.go index d6bfadb4..6b6cea78 100644 --- a/client/http/logging.go +++ b/client/http/logging.go @@ -9,6 +9,7 @@ import ( "io" "maps" gohttp "net/http" + "os" "regexp" "slices" "strings" @@ -74,6 +75,8 @@ func (l loggedBody) String() string { return "" case *bytes.Buffer: return bytesToPrettyJson(body.Bytes()) + case *os.File: + return "" default: return fmt.Sprintf(" %v", body) } diff --git a/client/http/method.go b/client/http/method.go index e98d85f2..d2c2fefd 100644 --- a/client/http/method.go +++ b/client/http/method.go @@ -8,5 +8,6 @@ const ( MethodGet = gohttp.MethodGet MethodPost = gohttp.MethodPost MethodPut = gohttp.MethodPut + MethodPatch = gohttp.MethodPatch MethodDelete = gohttp.MethodDelete ) diff --git a/client/http/options.go b/client/http/options.go index 9ecb8f3f..396f454c 100644 --- a/client/http/options.go +++ b/client/http/options.go @@ -17,6 +17,7 @@ type ( requestOptions struct { retryable bool requestPayload func() ([]byte, error) + bodyFile string requestModifiers []requestModifier } requestModifier func(req *gohttp.Request) error @@ -132,6 +133,13 @@ func WithBody(contentType string, body []byte) RequestOption { } } +func WithBodyFile(contentType, path string) RequestOption { + return func(opts *requestOptions) { + WithHeader("Content-Type", contentType)(opts) + opts.bodyFile = path + } +} + // WithJsonPayload sends a value as a JSON body, and both sends and asks for the given content // type. The type is a parameter because meshStack names a meshObject's kind and version in it. func WithJsonPayload(payload any, contentType string) RequestOption { diff --git a/client/runner.go b/client/runner.go new file mode 100644 index 00000000..83a7bacc --- /dev/null +++ b/client/runner.go @@ -0,0 +1,470 @@ +package client + +import ( + "context" + "encoding/json/jsontext" + "errors" + "fmt" + "io" + "math/big" + "net/url" + "strings" + "time" + "uuid" + + "github.com/meshcloud/meshstack-cli/client/http" + "github.com/meshcloud/meshstack-cli/client/internal" + "github.com/meshcloud/meshstack-cli/client/types/enum" + "github.com/meshcloud/meshstack-cli/client/types/variant" + "github.com/meshcloud/meshstack-cli/client/types/xurl" + "github.com/meshcloud/meshstack-cli/internal/json" +) + +type MeshRunnerRun struct { + ApiVersion string `json:"apiVersion"` + Kind string `json:"kind"` + Metadata MeshRunnerRunMetadata `json:"metadata"` + Spec MeshRunnerRunSpec `json:"spec"` + Status string `json:"status"` + Links MeshRunnerRunLinks `json:"_links"` +} + +type MeshRunnerRunMetadata struct { + Uuid uuid.UUID `json:"uuid"` +} + +type MeshRunnerRunSpec struct { + RunNumber int `json:"runNumber"` + Behavior string `json:"behavior"` + BuildingBlock MeshRunnerRunBuildingBlock `json:"buildingBlock"` + Definition MeshRunnerRunDefinition `json:"buildingBlockDefinition"` + RunToken string `json:"runToken"` + Trigger *MeshRunnerRunTrigger `json:"trigger,omitzero"` +} + +type MeshRunnerRunBuildingBlock struct { + Uuid string `json:"uuid"` + Spec MeshRunnerRunBuildingBlockSpec `json:"spec"` +} + +type MeshRunnerRunBuildingBlockSpec struct { + DisplayName string `json:"displayName"` + WorkspaceIdentifier string `json:"workspaceIdentifier"` + ProjectIdentifier string `json:"projectIdentifier,omitempty"` + FullPlatformIdentifier string `json:"fullPlatformIdentifier,omitempty"` + Inputs []MeshRunnerRunInput `json:"inputs"` + ParentBuildingBlocks []MeshRunnerRunParentBlock `json:"parentBuildingBlocks"` +} + +type MeshRunnerRunParentBlock struct { + BuildingBlockUuid string `json:"buildingBlockUuid"` + DefinitionUuid string `json:"definitionUuid"` +} + +type MeshRunnerRunInput struct { + Key string `json:"key"` + // Value holds a *big.Int for an INTEGER, so that it keeps every digit. + Value any `json:"value"` + Type enum.Entry[MeshBuildingBlockIOType] `json:"type"` + IsSensitive bool `json:"isSensitive"` + Env bool `json:"isEnvironment"` +} + +func (i *MeshRunnerRunInput) UnmarshalJSON(data []byte) error { + type wire MeshRunnerRunInput + var target struct { + wire + + Value jsontext.Value `json:"value"` + } + if err := json.Unmarshal(data, &target); err != nil { + return err + } + *i = MeshRunnerRunInput(target.wire) + // An encrypted INTEGER is a string until it is decrypted. + if i.Type == MeshBuildingBlockIOTypeInteger && target.Value.Kind() == '0' { + i.Value = new(big.Int) + } + if err := json.Unmarshal(target.Value, &i.Value); err != nil { + return fmt.Errorf("input %s: %w", i.Key, err) + } + return nil +} + +type MeshRunnerRunDefinition struct { + Uuid string `json:"uuid"` + Spec MeshRunnerRunDefinitionSpec `json:"spec"` +} + +type MeshRunnerRunDefinitionSpec struct { + WorkspaceIdentifier string `json:"workspaceIdentifier"` + Version int `json:"version"` + Implementation MeshRunnerRunImplementation `json:"implementation"` +} + +type MeshRunnerRunTrigger struct { + Reason string `json:"reason"` + Author MeshRunnerRunAuthor `json:"author"` +} + +type MeshRunnerRunAuthor struct { + Identifier string `json:"identifier"` + Type string `json:"type"` + DisplayName *string `json:"displayName"` + Username *string `json:"username"` + Email *string `json:"email"` + Euid *string `json:"euid"` +} + +type MeshRunnerRunLinks struct { + Self MeshRunnerRunLink `json:"self"` + RegisterSource MeshRunnerRunLink `json:"registerSource"` + UpdateSource MeshRunnerRunLink `json:"updateSource"` + MeshstackBaseUrl MeshRunnerRunLink `json:"meshstackBaseUrl"` + // Artifact is set only when an APPLY run applies the plan of its DETECT run. + Artifact MeshRunnerRunLink `json:"artifact,omitzero"` + ArtifactUpload MeshRunnerRunLink `json:"artifactUpload,omitzero"` + // TfStateLock is empty on a meshStack that cannot lock the state. + TfStateLock MeshRunnerRunLink `json:"tfStateLock,omitzero"` +} + +type MeshRunnerRunLink struct { + Href string `json:"href"` + Templated bool `json:"templated,omitzero"` +} + +func (l MeshRunnerRunLink) parse(name string) (*url.URL, error) { + if l.Href == "" { + return nil, fmt.Errorf("the run has no %s link", name) + } + return url.Parse(l.Href) +} + +type MeshRunnerImplementationType string + +var ( + MeshRunnerImplementationTypes = enum.Enum[MeshRunnerImplementationType]{} + MeshRunnerImplementationTypeManual = MeshRunnerImplementationTypes.Entry("MANUAL") + MeshRunnerImplementationTypeTerraform = MeshRunnerImplementationTypes.Entry("TERRAFORM") + MeshRunnerImplementationTypeGithubWorkflow = MeshRunnerImplementationTypes.Entry("GITHUB_WORKFLOW") + MeshRunnerImplementationTypeGitlabCiCd = MeshRunnerImplementationTypes.Entry("GITLAB_CICD") + MeshRunnerImplementationTypeAzureDevops = MeshRunnerImplementationTypes.Entry("AZURE_DEVOPS") +) + +// MeshRunnerRunImplementation sets no variant for a type this client does not know. +type MeshRunnerRunImplementation struct { + Type enum.Entry[MeshRunnerImplementationType] + Manual *MeshRunnerManualImplementation + Terraform *MeshRunnerTerraformImplementation + GithubWorkflow *MeshRunnerGithubImplementation + GitlabCiCd *MeshRunnerGitlabImplementation + AzureDevops *MeshRunnerAzureDevopsImplementation +} + +type MeshRunnerManualImplementation struct{} + +type MeshRunnerTerraformImplementation struct { + TerraformVersion string `json:"terraformVersion"` + RepositoryUrl string `json:"repositoryUrl"` + RepositoryPath *string `json:"repositoryPath,omitzero"` + RefName *string `json:"refName,omitzero"` + SshPrivateKey *string `json:"sshPrivateKey,omitzero"` + KnownHost *MeshRunnerKnownHost `json:"knownHost,omitzero"` + Async bool `json:"async"` + UseMeshHttpBackendFallback bool `json:"useMeshHttpBackendFallback"` + PreRunScript *string `json:"preRunScript,omitzero"` +} + +type MeshRunnerKnownHost struct { + Host string `json:"host"` + KeyType string `json:"keyType"` + KeyValue string `json:"keyValue"` +} + +type MeshRunnerGithubImplementation struct { + GithubBaseUrl string `json:"githubBaseUrl"` + Owner string `json:"owner"` + AppId string `json:"appId"` + AppPem string `json:"appPem"` + Repository string `json:"repository"` + Branch string `json:"branch"` + ApplyWorkflow string `json:"applyWorkflow"` + DestroyWorkflow *string `json:"destroyWorkflow,omitzero"` + Async bool `json:"async"` + OmitRunObjectInput bool `json:"omitRunObjectInput"` +} + +type MeshRunnerGitlabImplementation struct { + GitlabBaseUrl string `json:"gitlabBaseUrl"` + ProjectId string `json:"projectId"` + RefName string `json:"refName"` + PipelineTriggerToken string `json:"pipelineTriggerToken"` +} + +type MeshRunnerAzureDevopsImplementation struct { + AzureDevOpsBaseUrl string `json:"azureDevOpsBaseUrl"` + Organization string `json:"organization"` + Project string `json:"project"` + PipelineId string `json:"pipelineId"` + PersonalAccessToken string `json:"personalAccessToken"` + Async bool `json:"async"` + RefName *string `json:"refName,omitzero"` +} + +func (m MeshRunnerRunImplementation) InferType() (enum.Entry[MeshRunnerImplementationType], error) { + result, err := variant.InferType( + variant.NewCandidate(MeshRunnerImplementationTypeManual, m.Manual != nil), + variant.NewCandidate(MeshRunnerImplementationTypeTerraform, m.Terraform != nil), + variant.NewCandidate(MeshRunnerImplementationTypeGithubWorkflow, m.GithubWorkflow != nil), + variant.NewCandidate(MeshRunnerImplementationTypeGitlabCiCd, m.GitlabCiCd != nil), + variant.NewCandidate(MeshRunnerImplementationTypeAzureDevops, m.AzureDevops != nil), + ) + if err != nil { + return "", fmt.Errorf("cannot infer implementation type: %w", err) + } + return result, nil +} + +func (m MeshRunnerRunImplementation) MarshalJSON() ([]byte, error) { + implementationType := m.Type + if len(implementationType) == 0 { + var err error + if implementationType, err = m.InferType(); err != nil { + return nil, err + } + } + switch { + case m.Terraform != nil: + return marshalFlat(implementationType, m.Terraform) + case m.GithubWorkflow != nil: + return marshalFlat(implementationType, m.GithubWorkflow) + case m.GitlabCiCd != nil: + return marshalFlat(implementationType, m.GitlabCiCd) + case m.AzureDevops != nil: + return marshalFlat(implementationType, m.AzureDevops) + default: + return marshalFlat(implementationType, &MeshRunnerManualImplementation{}) + } +} + +func marshalFlat[V any](implementationType enum.Entry[MeshRunnerImplementationType], implementation *V) ([]byte, error) { + var fields map[string]jsontext.Value + if encoded, err := json.Marshal(implementation); err != nil { + return nil, err + } else if err := json.Unmarshal(encoded, &fields); err != nil { + return nil, err + } + var err error + if fields["type"], err = json.Marshal(implementationType); err != nil { + return nil, err + } + return json.Marshal(fields) +} + +func (m *MeshRunnerRunImplementation) UnmarshalJSON(data []byte) error { + var typed struct { + Type enum.Entry[MeshRunnerImplementationType] `json:"type"` + } + if err := json.Unmarshal(data, &typed); err != nil { + return err + } + *m = MeshRunnerRunImplementation{Type: typed.Type} + var err error + switch typed.Type { + case MeshRunnerImplementationTypeManual: + m.Manual = &MeshRunnerManualImplementation{} + case MeshRunnerImplementationTypeTerraform: + m.Terraform, err = unmarshalFlat[MeshRunnerTerraformImplementation](data) + case MeshRunnerImplementationTypeGithubWorkflow: + m.GithubWorkflow, err = unmarshalFlat[MeshRunnerGithubImplementation](data) + case MeshRunnerImplementationTypeGitlabCiCd: + m.GitlabCiCd, err = unmarshalFlat[MeshRunnerGitlabImplementation](data) + case MeshRunnerImplementationTypeAzureDevops: + m.AzureDevops, err = unmarshalFlat[MeshRunnerAzureDevopsImplementation](data) + } + return err +} + +func unmarshalFlat[V any](data []byte) (*V, error) { + implementation := new(V) + if err := json.Unmarshal(data, implementation); err != nil { + return nil, err + } + return implementation, nil +} + +type MeshRunnerRunSourceRegistration struct { + Source MeshRunnerRunSource `json:"source"` + Steps []MeshRunnerRunStepRegistration `json:"steps"` +} + +type MeshRunnerRunSource struct { + Id uuid.UUID `json:"id"` + ExternalId *string `json:"externalId,omitzero"` + ExternalUrl *string `json:"externalUrl,omitzero"` +} + +type MeshRunnerRunStepRegistration struct { + Id string `json:"id"` + DisplayName string `json:"displayName"` + Status *string `json:"status"` +} + +type MeshRunnerRunStatusUpdate struct { + BlockRunId uuid.UUID `json:"blockRunId,omitzero"` + Source uuid.UUID `json:"source,omitzero"` + Type enum.Entry[MeshRunnerImplementationType] `json:"type,omitempty"` + Status *string `json:"status"` + CreatedOn time.Time `json:"createdOn,omitzero"` + Summary *string `json:"summary"` + Steps []MeshRunnerRunStepStatusUpdate `json:"steps"` + ChangesDetected *bool `json:"changesDetected,omitzero"` +} + +type MeshRunnerRunStepStatusUpdate struct { + Id string `json:"id"` + DisplayName string `json:"displayName"` + Status *string `json:"status,omitzero"` + UserMessage *string `json:"userMessage,omitzero"` + SystemMessage *string `json:"systemMessage,omitzero"` + Outputs map[string]MeshRunnerRunOutput `json:"outputs,omitempty"` +} + +type MeshRunnerRunOutput struct { + Value any `json:"value"` + Type string `json:"type"` + Sensitive bool `json:"isSensitive"` +} + +type MeshRunnerRunStatusUpdateResult struct { + Abort bool `json:"runAborted"` +} + +const ( + runMediaType = "application/vnd.meshcloud.api.meshbuildingblockrun.v1.hal+json" + + maxArtifactBytes = 128 << 20 +) + +var ErrNoRunToClaim = errors.New("no run to claim") + +type MeshRunnerClient interface { + ClaimRun(ctx context.Context, runnerUuid uuid.UUID) (MeshRunnerRun, error) + // ForRun authorizes with the run's token, which every request about the run needs. + ForRun(run MeshRunnerRun) MeshRunnerRunClient + // ForRunUuid authorizes with the runner's own credentials and can only register a source and patch its status. + ForRunUuid(runUuid uuid.UUID) MeshRunnerRunClient +} + +type MeshRunnerRunClient interface { + RegisterSource(ctx context.Context, registration MeshRunnerRunSourceRegistration) error + PatchStatus(ctx context.Context, sourceId uuid.UUID, update MeshRunnerRunStatusUpdate) (MeshRunnerRunStatusUpdateResult, error) + DownloadArtifact(ctx context.Context, w io.Writer) error + UploadArtifact(ctx context.Context, path string) error +} + +func newRunnerClient(httpClient internal.HttpClient) MeshRunnerClient { + return meshRunnerClient{runnerHttpClient{httpClient.AuthorizedClient}, httpClient.EndpointUrl} +} + +type meshRunnerClient struct { + http runnerHttpClient + endpoint xurl.URL +} + +func (c meshRunnerClient) ClaimRun(ctx context.Context, runnerUuid uuid.UUID) (MeshRunnerRun, error) { + claimUrl := c.endpoint.JoinPath("api", "meshobjects", "meshbuildingblockruns", "create") + claimUrl.RawQuery = url.Values{"forRunnerUuid": {runnerUuid.String()}}.Encode() + run, err := c.http.do[MeshRunnerRun](ctx, http.MethodPost, claimUrl, http.WithHeader("Accept", runMediaType), http.WithHeader("Content-Type", runMediaType)) + if httpErr, ok := errors.AsType[http.Error](err); ok && httpErr.IsNotFound() { + return run, ErrNoRunToClaim + } + if err == nil && run.Metadata.Uuid == (uuid.UUID{}) { + err = errors.New("the claimed run has no uuid") + } + return run, err +} + +func (c meshRunnerClient) ForRun(run MeshRunnerRun) MeshRunnerRunClient { + runToken := http.BearerToken(run.Spec.RunToken) + return meshRunnerRunClient{runnerHttpClient{c.http.WithAuthorization(runToken)}, run.Links} +} + +func (c meshRunnerClient) ForRunUuid(runUuid uuid.UUID) MeshRunnerRunClient { + registerSource := c.endpoint.JoinPath("api", "meshobjects", "meshbuildingblockruns", runUuid.String(), "status", "source").String() + return meshRunnerRunClient{c.http, MeshRunnerRunLinks{ + RegisterSource: MeshRunnerRunLink{Href: registerSource}, + UpdateSource: MeshRunnerRunLink{Href: registerSource + "/{sourceId}", Templated: true}, + }} +} + +type meshRunnerRunClient struct { + http runnerHttpClient + links MeshRunnerRunLinks +} + +func (c meshRunnerRunClient) RegisterSource(ctx context.Context, registration MeshRunnerRunSourceRegistration) error { + registerUrl, err := c.links.RegisterSource.parse("registerSource") + if err != nil { + return err + } + _, err = c.http.do[[]byte](ctx, http.MethodPost, registerUrl, http.WithJsonPayload(registration, runMediaType), http.Retryable()) + if httpErr, ok := errors.AsType[http.Error](err); ok && httpErr.IsConflict() { + return nil + } + return err +} + +func (c meshRunnerRunClient) PatchStatus(ctx context.Context, sourceId uuid.UUID, update MeshRunnerRunStatusUpdate) (MeshRunnerRunStatusUpdateResult, error) { + updateSource := MeshRunnerRunLink{Href: strings.ReplaceAll(c.links.UpdateSource.Href, "{sourceId}", sourceId.String())} + updateUrl, err := updateSource.parse("updateSource") + if err != nil { + return MeshRunnerRunStatusUpdateResult{}, err + } + return c.http.do[MeshRunnerRunStatusUpdateResult](ctx, http.MethodPatch, updateUrl, http.WithJsonPayload(update, runMediaType), http.Retryable()) +} + +func (c meshRunnerRunClient) DownloadArtifact(ctx context.Context, w io.Writer) error { + downloadUrl, err := c.links.Artifact.parse("artifact") + if err != nil { + return err + } + capped := &cappedWriter{w, maxArtifactBytes} + return c.http.DoStreamingRequest(ctx, http.MethodGet, downloadUrl, capped, c.http.version(), http.WithHeader("Accept", "application/octet-stream")) +} + +func (c meshRunnerRunClient) UploadArtifact(ctx context.Context, path string) error { + uploadUrl, err := c.links.ArtifactUpload.parse("artifactUpload") + if err != nil { + return err + } + _, err = c.http.do[[]byte](ctx, http.MethodPut, uploadUrl, http.WithBodyFile("application/octet-stream", path), http.WithHeader("Accept", "*/*")) + return err +} + +type runnerHttpClient struct { + http.AuthorizedClient +} + +func (c runnerHttpClient) do[R any](ctx context.Context, method string, url *url.URL, options ...http.RequestOption) (R, error) { + return c.DoRequest[R](ctx, method, url, append(options, c.version())...) +} + +func (c runnerHttpClient) version() http.RequestOption { + return http.WithHeader("X-Meshcloud-Runner-Version", c.UserAgent.Version) +} + +// cappedWriter keeps a misbehaving server from filling the disk. +type cappedWriter struct { + io.Writer + + remaining int64 +} + +func (w *cappedWriter) Write(p []byte) (int, error) { + if int64(len(p)) > w.remaining { + return 0, fmt.Errorf("the artifact exceeds %d bytes", maxArtifactBytes) + } + w.remaining -= int64(len(p)) + return w.Writer.Write(p) +} diff --git a/client/runner_test.go b/client/runner_test.go new file mode 100644 index 00000000..33d7e89d --- /dev/null +++ b/client/runner_test.go @@ -0,0 +1,167 @@ +package client_test + +import ( + "bytes" + "encoding/json/jsontext" + "encoding/json/v2" + "io" + "math/big" + gohttp "net/http" + "net/http/httptest" + "os" + "path/filepath" + "sync/atomic" + "testing" + "uuid" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/meshcloud/meshstack-cli/client" + "github.com/meshcloud/meshstack-cli/client/http" + "github.com/meshcloud/meshstack-cli/client/types/xurl" + "github.com/meshcloud/meshstack-cli/internal/testutil/fakemeshstack" +) + +func TestRunnerRun(t *testing.T) { + data, readErr := os.ReadFile("testdata/runner_run.json") + require.NoError(t, readErr) + var run client.MeshRunnerRun + require.NoError(t, json.Unmarshal(data, &run)) + + t.Run("decodes the inputs by type and round-trips unchanged", func(t *testing.T) { + inputs := run.Spec.BuildingBlock.Spec.Inputs + count, isBig := inputs[0].Value.(*big.Int) + require.True(t, isBig) + assert.Equal(t, "12345678901234567890", count.String()) + assert.InDelta(t, 0.5, inputs[1].Value, 0) + assert.Equal(t, map[string]any{"nested": []any{1.0, 2.0}}, inputs[2].Value) + assert.Equal(t, "ZW5jcnlwdGVk", inputs[3].Value) + assert.Equal(t, "eu", inputs[4].Value) + + implementation := run.Spec.Definition.Spec.Implementation + assert.Equal(t, client.MeshRunnerImplementationTypeTerraform, implementation.Type) + require.NotNil(t, implementation.Terraform) + assert.Equal(t, "1.9.0", implementation.Terraform.TerraformVersion) + + encoded, err := json.Marshal(run) + require.NoError(t, err) + assert.JSONEq(t, string(data), string(encoded)) + }) + + t.Run("round-trips unchanged once decrypted", func(t *testing.T) { + run.Spec.BuildingBlock.Spec.Inputs[3].Value = "plaintext" + run.Spec.Definition.Spec.Implementation.Terraform.SshPrivateKey = new("-----BEGIN OPENSSH PRIVATE KEY-----") + + encoded, err := json.Marshal(run) + require.NoError(t, err) + var decoded client.MeshRunnerRun + require.NoError(t, json.Unmarshal(encoded, &decoded)) + reencoded, err := json.Marshal(decoded) + require.NoError(t, err) + assert.JSONEq(t, string(encoded), string(reencoded)) + assert.Equal(t, "plaintext", decoded.Spec.BuildingBlock.Spec.Inputs[3].Value) + }) + + t.Run("an implementation keeps its type", func(t *testing.T) { + for _, implementation := range []string{ + `{"type":"MANUAL"}`, + `{"type":"GITLAB_CICD","gitlabBaseUrl":"https://gitlab.com","projectId":"1","refName":"main","pipelineTriggerToken":"token"}`, + `{"type":"A_NEW_TYPE"}`, + } { + var decoded client.MeshRunnerRunImplementation + require.NoError(t, json.Unmarshal([]byte(implementation), &decoded)) + encoded, err := json.Marshal(decoded) + require.NoError(t, err) + assert.JSONEq(t, implementation, string(encoded)) + } + }) +} + +func TestRunnerRunClient(t *testing.T) { + t.Run("PatchStatus sends only the fields set, with the run token", func(t *testing.T) { + sourceId := uuid.MustParse("98520496-627d-43e6-82da-ce499179ff3f") + runClient := newRunClient(t, func(w gohttp.ResponseWriter, r *gohttp.Request) { + assert.Equal(t, gohttp.MethodPatch, r.Method) + assert.Equal(t, "/runs/c662eb0d/status/source/"+sourceId.String(), r.URL.Path) + assert.Equal(t, "Bearer a-run-token", r.Header.Get("Authorization")) + assert.Equal(t, "application/vnd.meshcloud.api.meshbuildingblockrun.v1.hal+json", r.Header.Get("Content-Type")) + assert.Equal(t, fakemeshstack.UserAgent.Version, r.Header.Get("X-Meshcloud-Runner-Version")) + body, err := io.ReadAll(r.Body) + assert.NoError(t, err) + assert.JSONEq(t, `{ + "status": "SUCCEEDED", + "summary": null, + "steps": [{"id": "manual", "displayName": "Manual Block Run", "status": "SUCCEEDED", "userMessage": "", + "outputs": {"count": {"value": 12345678901234567890, "type": "INTEGER", "isSensitive": false}}}] + }`, string(body)) + _, _ = w.Write([]byte(`{"runAborted": true}`)) + }) + + result, err := runClient.PatchStatus(t.Context(), sourceId, client.MeshRunnerRunStatusUpdate{ + Status: new("SUCCEEDED"), + Steps: []client.MeshRunnerRunStepStatusUpdate{{ + Id: "manual", DisplayName: "Manual Block Run", Status: new("SUCCEEDED"), UserMessage: new(""), + Outputs: map[string]client.MeshRunnerRunOutput{"count": {Value: jsontext.Value("12345678901234567890"), Type: "INTEGER"}}, + }}, + }) + require.NoError(t, err) + assert.True(t, result.Abort) + }) + + t.Run("DownloadArtifact streams up to the cap", func(t *testing.T) { + var size atomic.Int64 + size.Store(1 << 20) + runClient := newRunClient(t, func(w gohttp.ResponseWriter, r *gohttp.Request) { + assert.Equal(t, "/artifact", r.URL.Path) + assert.Equal(t, "application/octet-stream", r.Header.Get("Accept")) + _, _ = io.CopyN(w, zeros{}, size.Load()) + }) + + var plan bytes.Buffer + require.NoError(t, runClient.DownloadArtifact(t.Context(), &plan)) + assert.Equal(t, size.Load(), int64(plan.Len())) + + size.Store(128<<20 + 1) + assert.ErrorContains(t, runClient.DownloadArtifact(t.Context(), io.Discard), "the artifact exceeds 134217728 bytes") + }) + + t.Run("UploadArtifact sends the file", func(t *testing.T) { + planFile := filepath.Join(t.TempDir(), "plan.tfplan") + require.NoError(t, os.WriteFile(planFile, []byte("a plan"), 0o600)) + runClient := newRunClient(t, func(w gohttp.ResponseWriter, r *gohttp.Request) { + assert.Equal(t, gohttp.MethodPut, r.Method) + assert.Equal(t, "/upload", r.URL.Path) + assert.Equal(t, "application/octet-stream", r.Header.Get("Content-Type")) + assert.Equal(t, int64(6), r.ContentLength) + body, err := io.ReadAll(r.Body) + assert.NoError(t, err) + assert.Equal(t, "a plan", string(body)) + w.WriteHeader(gohttp.StatusNoContent) + }) + + require.NoError(t, runClient.UploadArtifact(t.Context(), planFile)) + }) +} + +type zeros struct{} + +func (zeros) Read(p []byte) (int, error) { + clear(p) + return len(p), nil +} + +func newRunClient(t *testing.T, handler gohttp.HandlerFunc) client.MeshRunnerRunClient { + t.Helper() + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + c := client.New(t.Context(), xurl.MustParsef("%s", server.URL), http.NewClient(fakemeshstack.UserAgent), http.BearerToken("runner-token")) + return c.Runner.ForRun(client.MeshRunnerRun{ + Spec: client.MeshRunnerRunSpec{RunToken: "a-run-token"}, + Links: client.MeshRunnerRunLinks{ + UpdateSource: client.MeshRunnerRunLink{Href: server.URL + "/runs/c662eb0d/status/source/{sourceId}", Templated: true}, + Artifact: client.MeshRunnerRunLink{Href: server.URL + "/artifact"}, + ArtifactUpload: client.MeshRunnerRunLink{Href: server.URL + "/upload"}, + }, + }) +} diff --git a/client/testdata/runner_run.json b/client/testdata/runner_run.json new file mode 100644 index 00000000..de31b3ff --- /dev/null +++ b/client/testdata/runner_run.json @@ -0,0 +1,56 @@ +{ + "apiVersion": "v1", + "kind": "meshBuildingBlockRun", + "metadata": { + "uuid": "c662eb0d-2ee2-4dc3-92df-d0b8bd41df44" + }, + "spec": { + "runNumber": 1, + "behavior": "APPLY", + "buildingBlock": { + "uuid": "23cf7c1b-f0ba-4139-a98c-b8cea37e83cf", + "spec": { + "displayName": "my-block", + "workspaceIdentifier": "managed-customer", + "inputs": [ + { "key": "count", "value": 12345678901234567890, "type": "INTEGER", "isSensitive": false, "isEnvironment": false }, + { "key": "ratio", "value": 0.5, "type": "NUMBER", "isSensitive": false, "isEnvironment": false }, + { "key": "config", "value": { "nested": [1, 2] }, "type": "CODE", "isSensitive": false, "isEnvironment": false }, + { "key": "password", "value": "ZW5jcnlwdGVk", "type": "STRING", "isSensitive": true, "isEnvironment": true }, + { "key": "region", "value": "eu", "type": "SINGLE_SELECT", "isSensitive": false, "isEnvironment": false } + ], + "parentBuildingBlocks": [ + { "buildingBlockUuid": "5d1b8a2e-7c1f-4a51-9d0e-6f1f4b7c2a10", "definitionUuid": "0b6d1b5e-3c0f-4a51-9d0e-6f1f4b7c2a10" } + ] + } + }, + "buildingBlockDefinition": { + "uuid": "f3228cf4-acde-4ff3-a0a5-1720b69d5bcd", + "spec": { + "workspaceIdentifier": "test-workspace", + "version": 1, + "implementation": { + "type": "TERRAFORM", + "terraformVersion": "1.9.0", + "repositoryUrl": "git@github.com:meshcloud/modules.git", + "sshPrivateKey": "ZW5jcnlwdGVkLWtleQ==", + "async": false, + "useMeshHttpBackendFallback": true + } + } + }, + "runToken": "a-run-token", + "trigger": { + "reason": "MANUAL_EXECUTION", + "author": { "identifier": "jane", "type": "User", "displayName": "Jane", "username": "jane", "email": null, "euid": null } + } + }, + "status": "IN_PROGRESS", + "_links": { + "self": { "href": "http://localhost:8080/api/meshobjects/meshbuildingblockruns/c662eb0d-2ee2-4dc3-92df-d0b8bd41df44" }, + "registerSource": { "href": "http://localhost:8080/api/meshobjects/meshbuildingblockruns/c662eb0d-2ee2-4dc3-92df-d0b8bd41df44/status/source" }, + "updateSource": { "href": "http://localhost:8080/api/meshobjects/meshbuildingblockruns/c662eb0d-2ee2-4dc3-92df-d0b8bd41df44/status/source/{sourceId}", "templated": true }, + "meshstackBaseUrl": { "href": "http://localhost:8080" }, + "artifact": { "href": "http://localhost:8080/api/meshobjects/meshbuildingblockruns/a1b2c3d4-0000-4000-8000-000000000000/artifact" } + } +} From 5975d78e6bbfb79facb6c59d0312d21b9c858ec3 Mon Sep 17 00:00:00 2001 From: Andreas Grub Date: Sun, 25 Oct 2026 20:47:32 +0100 Subject: [PATCH 3/3] docs: point to the runner's tf executor in comments Co-Authored-By: Claude Opus 5.5 --- cmd/buildingblock/tfstate/exec.go | 2 +- cmd/internal/color/color.go | 2 +- internal/tfstate/store.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/buildingblock/tfstate/exec.go b/cmd/buildingblock/tfstate/exec.go index 0a141d7e..e52333ad 100644 --- a/cmd/buildingblock/tfstate/exec.go +++ b/cmd/buildingblock/tfstate/exec.go @@ -21,7 +21,7 @@ import ( "github.com/meshcloud/meshstack-cli/internal/tfstate" ) -// runLockTimeout must match stateLockTimeout in ../building-block-runner/tf-block-runner/tfrun/tfcmd.go. +// runLockTimeout must match stateLockTimeout in ../building-block-runner/internal/executor/tf/tfcmd.go. const runLockTimeout = 5 * time.Minute type mode string diff --git a/cmd/internal/color/color.go b/cmd/internal/color/color.go index 6e32039c..a8d923ad 100644 --- a/cmd/internal/color/color.go +++ b/cmd/internal/color/color.go @@ -87,7 +87,7 @@ func (e env) isOff(name string) bool { // evidence, and a log viewer outside the list, such as Jenkins without its AnsiColor plugin, shows // the escape codes as text. So does the system log of a building block run in meshPanel. Its // runner starts a script or tofu with neither CI nor TERM set, which keeps a run plain: see -// cleanSystemEnv in tf-block-runner/tfrun of ../building-block-runner. +// cleanSystemEnv in ../building-block-runner/internal/executor/tf/tfcmd.go. func (e env) ciProfile() colorprofile.Profile { has := func(name string) bool { _, found := e[name]; return found } switch { diff --git a/internal/tfstate/store.go b/internal/tfstate/store.go index 3476b4c3..9dec1e75 100644 --- a/internal/tfstate/store.go +++ b/internal/tfstate/store.go @@ -91,8 +91,8 @@ func (s Store) wrap(action string, err error) error { return fmt.Errorf("cannot %s the state of building block %s in workspace %s: %w", action, s.BuildingBlock, s.Workspace, err) } -// path must match EP_State in ../building-block-runner/tf-block-runner/tfrun/runapi.go, the address -// that the runner's backend uses. meshfed's TfHttpStateController serves it. +// path must match stateEndpointFormat in ../building-block-runner/internal/executor/tf/tfcmd.go, +// the address that the runner's backend uses. meshfed's TfHttpStateController serves it. func (s Store) path() string { return fmt.Sprintf("/api/terraform/state/workspace/%s/buildingBlock/%s", s.Workspace, s.BuildingBlock) }