Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions .claude/skills/development/SKILL.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -123,12 +123,14 @@ there, later, and not in this repository's CI.
`develop` but not released. The source is the controllers and meshObjects of `../meshfed-release`
(*meshcloud-internal*).

**The provider implements the client's interfaces.** Its tests plug the mocks of its
`internal/clientmock` into `client.Client`, so a method added to a `Mesh…Client` interface stops
the provider compiling at its next bump. Put a method only the CLI calls on a new `client.Client`
field instead: an interface of its own, or a concrete type such as `*client.RawClient` when nothing
mocks it. `client.New` sets the field for every caller, while the provider's mock client fills the
struct by field name and leaves a new field unset.
**The provider implements the client's interfaces.** Every kind's client is the generic
`client.MeshObjectClient[M, Id, Filter]`, with Read, List, Create, Update and Delete, under a type
alias named after the kind. The tests of the provider plug the mocks of its `internal/clientmock`
into `client.Client`, so a method added to a `Mesh…Client` interface stops the provider compiling
at its next bump. An action of a kind follows a link of the object's `_links` through
`hal.Links`, such as `run.Links.Post[struct{}](ctx, "abort", nil)`. Put anything else only the CLI
calls on a new `client.Client` field, which the provider's mock client leaves unset, or in a
function of `client`, such as `client.WithKindAndApiVersion`.

Reading the pre-import history takes both paths, since the import merge re-roots the files under
`client/`:
Expand All @@ -138,32 +140,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, because another module cannot name it.
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.
</rules>

## Settings
Expand Down
13 changes: 6 additions & 7 deletions .claude/skills/meshobject-command/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ the `help-text` rule of the `development` skill.

## Leaf

- `list` lists the kind with `flags.Run[M](cmd, filter)` of `internal.ListFlags`, which pages through
`client.Raw.List[M]`.
- `list` lists the kind with `flags.Run(cmd, meshStack.<Kind>, filter)` of `internal.ListFlags`,
which pages through the kind's `List`.
- `show` shows one object.
- A sub-resource or an action is named after its path element in the API: `buildingblockrun logs`
for `…/logs`, `buildingblock trigger-run` for `…/trigger-run`.
Expand All @@ -65,11 +65,10 @@ and a `cmd/internal/markdown` template next to the command.
- A command gets its client from `internal.ResolveClient`, or for the session itself from
`auth.ResolveSession(ctx, internal.ResolveClientOptions(...))` and `Session.Client`. A
`ResolveClientOptionsModifier` such as `internal.SkipVersionCheck` adapts the resolution.
- What the CLI writes as meshStack sent it goes through `client.Raw`: `List[M]` with the kind's
list filter struct, `Get[M]` for one object or a sub-resource below it, and `DoRequest` for any
other path. `M` is the Go type of the kind's typed client, which `client.New` registers with
`Raw`; register a kind there before a command reads it.
- A list filter struct holds every query parameter the API docs give its endpoint, a repeated one
- The CLI writes an object as meshStack sent it through `client.WithKindAndApiVersion`. A
sub-resource or an action follows a link of the typed object through its `Links`, and any other
path goes through `client.Client.HttpClient`, joined onto `client.Client.Endpoint`.
- A filter struct holds every query parameter the API docs give its endpoint, a repeated one
as a slice. A parameter the command always sends, such as `includeAllPublished`, is a field the
command sets. Any other client method only the CLI calls goes on a new `client.Client` field, so
the Terraform provider's mocks keep compiling.
Expand Down
94 changes: 60 additions & 34 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -101,10 +101,6 @@ linters:
linters:
- forbidigo
text: stdout is a command's output
- path: internal/tfstate/apiproxy/
linters:
- forbidigo
text: one HTTP client
# The fake clock is a test's; production code waits on the real one.
- path-except: _test\.go
linters:
Expand All @@ -130,17 +126,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.
Expand Down Expand Up @@ -187,7 +183,7 @@ linters:
checks:
- all
- -ST1000
# SA4023 crashes on a type with a generic method, as client.RawClient has: go1.27.0 loses
# SA4023 crashes on a type with a generic method, as hal.Link has: go1.27.0 loses
# the order of methods in the export data (golang/go#81188). Enable it again with go1.27.2.
- -SA4023
# Replaces the default list, so that ClientId, BaseUrl and Uuid stand as the meshObject
Expand Down Expand Up @@ -243,7 +239,15 @@ linters:
- $all
deny:
- pkg: encoding/json$
desc: Use encoding/json/v2 and internal/json instead
desc: Use internal/json instead

json-v2:
files:
- $all
- "!**/internal/json/*.go"
deny:
- pkg: encoding/json/v2
desc: Use internal/json instead

# cobra is used in cmd/ and nowhere else.
cmd:
Expand All @@ -267,8 +271,8 @@ linters:
- log/slog
# a listing yields raw items, and reads a field out of one
- encoding/json/jsontext
- encoding/json/v2
# no direct access to internal/
- github.com/meshcloud/meshstack-cli/internal/json
# no direct access to internal/, apart from the JSON codec above
- github.com/meshcloud/meshstack-cli/pkg
# a parent command calls its children's New, the same way cmd/meshstack calls its own
- github.com/meshcloud/meshstack-cli/cmd
Expand Down Expand Up @@ -305,17 +309,13 @@ linters:
- log/slog
# a request body is checked as JSON, and api-docs writes a part of the OpenAPI document
- encoding/json/jsontext
- encoding/json/v2
- github.com/meshcloud/meshstack-cli/internal/json
- 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
# or profile named by flag or environment, so api reads the stored profiles and the
# source of both settings, which pkg/ leaves out
# api-docs takes the endpoint of the profile without a login, and a full URL gives its
# origin as the endpoint setting, both of which pkg/ leaves out
- github.com/meshcloud/meshstack-cli/internal/profile
- github.com/meshcloud/meshstack-cli/internal/meshstack
- github.com/meshcloud/meshstack-cli/internal/setting
Expand All @@ -334,8 +334,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
Expand Down Expand Up @@ -412,7 +411,7 @@ linters:
- log/slog
# a listing writes raw items, and a command that shows one object marshals it
- encoding/json/jsontext
- encoding/json/v2
- github.com/meshcloud/meshstack-cli/internal/json
- github.com/meshcloud/meshstack-cli/cmd/internal/markdown
- github.com/meshcloud/meshstack-cli/cmd/internal/color
- github.com/meshcloud/meshstack-cli/pkg
Expand All @@ -432,7 +431,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
Expand All @@ -446,13 +445,10 @@ linters:
desc: Use log/slog instead
allow:
- $gostd
- encoding/json/v2
- 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:
Expand All @@ -470,9 +466,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
Expand All @@ -482,9 +478,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:
Expand All @@ -499,13 +495,13 @@ linters:
- "!**/cmd/**/*.go" # ...except cmd, whose internal packages the cmd rules cover
- "!$test"
- "!**/internal/testutil/**/*.go"
- "!**/internal/json/*.go" # client-http covers it
list-mode: strict
deny:
- pkg: log
desc: Use log/slog instead
allow:
- $gostd
- encoding/json/v2
- encoding/json/jsontext
- log/slog
- github.com/meshcloud/meshstack-cli/internal
Expand All @@ -520,7 +516,6 @@ linters:
list-mode: strict
allow:
- $gostd
- encoding/json/v2
- encoding/json/jsontext
- github.com/meshcloud/meshstack-cli
- github.com/stretchr/testify
Expand All @@ -545,17 +540,19 @@ linters:
list-mode: strict
allow:
- $gostd
- github.com/meshcloud/meshstack-cli/internal/json
- github.com/meshcloud/meshstack-cli/internal/testutil/fakemeshstack

# A test may use every direct dependency in go.mod: the rules above already confine where
# production code uses each one, and a test checks that code with the same libraries.
tests:
files:
- "$test"
- "!**/client/http/*_test.go"
- "!**/cmd/internal/testacc/*_test.go"
list-mode: strict
allow:
- $gostd
- encoding/json/v2
- encoding/json/jsontext
- github.com/meshcloud/meshstack-cli
- charm.land/bubbles/v2
Expand All @@ -570,3 +567,32 @@ linters:
- github.com/spf13/cobra
- github.com/spf13/pflag
- github.com/stretchr/testify

# The API docs scenario breaks the connection to meshStack, as the tests of client/http do.
tests-testacc:
files:
- "**/cmd/internal/testacc/*_test.go"
list-mode: strict
allow:
- $gostd
- encoding/json/jsontext
- github.com/meshcloud/meshstack-cli
- github.com/spf13/cobra
- github.com/stretchr/testify
- github.com/Shopify/toxiproxy/v2
# toxiproxy.NewServer takes a zerolog.Logger, and zerolog.Nop() is the only use
- github.com/rs/zerolog

# client/http breaks connections on purpose, which toxiproxy does in process.
tests-client-http:
files:
- "**/client/http/*_test.go"
list-mode: strict
allow:
- $gostd
- encoding/json/jsontext
- github.com/meshcloud/meshstack-cli
- github.com/stretchr/testify
- github.com/Shopify/toxiproxy/v2
# toxiproxy.NewServer takes a zerolog.Logger, and zerolog.Nop() is the only use
- github.com/rs/zerolog
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading