Skip to content
Merged
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
29 changes: 29 additions & 0 deletions .claude/skills/development/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,35 @@ command. For a command over a meshObject kind, the `meshobject-command` skill ha
- A **parent command sets `RunE` as well as `Args`**.
</rules>

## Help text and messages

<rules id="help-text">
These rules cover `Short`, `Long`, `Example` and flag help, and every error and log message a user
reads.

- **Open with what the command does.** The first sentence of `Long` says it in plain words. Details,
caveats and pointers to other commands come after it.
Bad: "Show what the API docs say about a request, given in the same form that 'meshstack api'
takes, …". Good: "Show what the meshStack API docs say about a request or a meshObject kind."
- **Plain English.** Use common words, natural word order and complete sentences that name who
acts: "the command prints …", "you". Write literally, without metaphors such as "names one",
"takes", "holds" or "carries". Every sentence must make sense to a user who does not know the
CLI's code. Bad: "Without the API docs, only the Content-Type header does." Good: "If the command
cannot download the API docs, it sets only the Content-Type header."
- **No line breaks inside a paragraph.** The root command wraps help to the terminal width
(`cmd/internal/help.go`). Break lines only between paragraphs and list items, and around indented
examples.
- **Build flag names from the flag's definition**: `"--" + internal.WorkspaceFlag.Name.String()`,
and for a command's own flag, the const or variable that registers it, so that a rename leaves no
stale text. Write a flag name literally only where its package cannot import the definition, as
in `internal/`, and in an `Example`, which is a command line as the user types it.
- **Quote a command** with single quotes: 'meshstack login'. A Markdown template uses backticks.
- **Use the same phrase for the same thing** in every command: "Use --workspace to choose another
workspace", "prints" for output, "response" for what meshStack sends back.
- **An error says what went wrong and what the user can do next**: "no stored profile has the
endpoint X; run 'meshstack login --endpoint X' to create one".
</rules>

## Values, pointers and receivers

<rules id="values-and-pointers">
Expand Down
3 changes: 2 additions & 1 deletion .claude/skills/meshobject-command/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ The help says how to use a command, never what its kind is or does: the parent's
such as that a uuid parses, because meshStack answers a malformed one with an empty list or a 404.
The root help says once where the endpoint, the profile and the workspace come from, so a leaf
names them only where it deviates, as tfstate does, which takes the building block's own workspace.
The `Long` stays a few lines that get a user started; the errors say the rest.
The `Long` stays a few lines that get a user started; the errors say the rest. The wording follows
the `help-text` rule of the `development` skill.

## Leaf

Expand Down
22 changes: 9 additions & 13 deletions cmd/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,15 @@ func New() *cobra.Command {
cmd := &cobra.Command{
Use: "api <path or URL>",
Short: "Send an authorized request to the meshStack API",
Long: `Send an authorized request to a path of the meshStack API, and return the response (typically JSON).
The path is relative to the endpoint but can also be an absolute one, as long as a matching profile with authentication credentials can be found.
Long: fmt.Sprintf(`Send an authorized request to a path of the meshStack API, and print the response, which is typically JSON. Give the path relative to the endpoint, or give a full URL whose endpoint matches a stored profile with credentials.

This exposes everything that the other CLI commands do not cover.
It supports you by taking over versioned content negotiation of meshStack's public API unless you specify it explicitly.
Use this command for everything that the other commands do not cover. For a path of meshStack's public API, the command sets the Accept and Content-Type headers to the latest API version that the API docs list, unless you set them yourself.

--request-json sends a JSON file, or stdin for '-'. For a path
of no version in the API docs, the Content-Type and Accept headers default to application/json, and
without the API docs the Content-Type alone does.
--%[1]s sends a JSON file as the request body, or stdin for '-'. If the API docs list no version for the path, the command sets the Accept and Content-Type headers to application/json. If the command cannot download the API docs, it sets only the Content-Type header to application/json.

An answer outside 2xx still writes its body but the command then fails.
If the response status is not 2xx, the command still prints the response body, and then fails.

Replace the same command line with 'api-docs' in place of 'api' to describe the request.`,
To see what the API docs say about the request, run the same command line with 'api' replaced by 'api-docs'.`, requestJsonFlagName),
Example: ` meshstack api '/api/meshobjects/meshtenants?workspaceIdentifier=my-workspace'
meshstack api -X DELETE /api/meshobjects/meshbuildingblocks/<uuid>/purge
meshstack api -X POST /api/meshobjects/meshworkspaces --request-json workspace.json
Expand Down Expand Up @@ -78,7 +74,7 @@ Replace the same command line with 'api-docs' in place of 'api' to describe the
spec, wait, err := flags.loadApiDocs(cmd)
defer wait()
if err != nil {
slog.WarnContext(ctx, fmt.Sprintf("Sending the request as given, without the API docs: %s. "+
slog.WarnContext(ctx, fmt.Sprintf("Sending the request unchanged, because the API docs are not available: %s. "+
"Run %s to see why.", err, docsCommand(cmd, args)))
} else if negotiateErr := r.negotiate(ctx, spec, selector); negotiateErr != nil {
return negotiateErr
Expand All @@ -95,7 +91,7 @@ Replace the same command line with 'api-docs' in place of 'api' to describe the
}
// The body is on stdout already, so the error says only what it adds.
if r.docsHelpWith(httpErr) {
return withHint(fmt.Sprintf("%s. Run %s to see what the API takes",
return withHint(fmt.Sprintf("%s. Run %s to see what the API expects",
httpErr.Summary(), docsCommand(cmd, args)), httpErr)
}
// What wraps the HTTP error, such as the auth scope of a 403, stays.
Expand Down Expand Up @@ -161,7 +157,7 @@ func (r *request) negotiate(ctx context.Context, spec openapi.Spec, selector ope
r.documented = err == nil && len(selected.Operations) > 0
if err != nil {
if !openapi.ParseMediaType(r.header.Get("Accept")).ApiVersion.IsZero() {
slog.WarnContext(ctx, fmt.Sprintf("Sending the request as given: %s", err))
slog.WarnContext(ctx, fmt.Sprintf("Sending the request unchanged: %s", err))
return nil
}
return err
Expand All @@ -174,7 +170,7 @@ func (r *request) negotiate(ctx context.Context, spec openapi.Spec, selector ope
}
if mediaType.ApiVersion.IsZero() {
if !selector.ApiVersion.IsZero() && r.header.Get("Accept") == "" {
slog.WarnContext(ctx, fmt.Sprintf("The API docs list no version of %s %s, so no Accept header asks for %s",
slog.WarnContext(ctx, fmt.Sprintf("The API docs have no version of %s %s, so the request gets no Accept header for %s",
selector.Method, selector.Path, selector.ApiVersion))
} else {
slog.DebugContext(ctx, fmt.Sprintf("The API docs list no version of %s %s", selector.Method, selector.Path))
Expand Down
20 changes: 10 additions & 10 deletions cmd/api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ func TestApi(t *testing.T) {

t.Run("refuses a body of no JSON, or one that names another version than --api-version", func(t *testing.T) {
_, err := postBlock(t, `spec: {}`)
require.EqualError(t, err, "--request-json - is no JSON")
require.EqualError(t, err, "--request-json - is not valid JSON")

_, err = postBlock(t, `{"apiVersion": "v1"}`, "--api-version", "v2-preview")
require.EqualError(t, err, "the Accept and Content-Type headers, --api-version and the body's apiVersion ask for v2-preview, v1")
require.EqualError(t, err, "the Accept and Content-Type headers, --api-version and the body's apiVersion request different versions: v2-preview, v1")
})

t.Run("sends and asks for JSON on a path of no version, unless a header says otherwise", func(t *testing.T) {
Expand All @@ -128,7 +128,7 @@ func TestApi(t *testing.T) {

t.Run("writes the body of a failed answer, and fails with a hint to api-docs on a client error", func(t *testing.T) {
for status, wantError := range map[int]string{
gohttp.StatusConflict: "meshStack answered HTTP 409: still in use. Run meshstack api-docs /api/x -H 'Accept: application/json' -X DELETE to see what the API takes",
gohttp.StatusConflict: "meshStack answered HTTP 409: still in use. Run meshstack api-docs /api/x -H 'Accept: application/json' -X DELETE to see what the API expects",
gohttp.StatusBadGateway: "meshStack answered HTTP 502: still in use",
gohttp.StatusForbidden: "auth scope profile default: meshStack answered HTTP 403: still in use",
} {
Expand All @@ -143,7 +143,7 @@ func TestApi(t *testing.T) {

t.Run("points to api-docs on a 404 only for a path the API docs do not list", func(t *testing.T) {
for path, wantError := range map[string]string{
"/api/x": "meshStack answered HTTP 404: gone (errorCode NotFound). Run meshstack api-docs /api/x -X GET -H 'Accept: application/json' to see what the API takes",
"/api/x": "meshStack answered HTTP 404: gone (errorCode NotFound). Run meshstack api-docs /api/x -X GET -H 'Accept: application/json' to see what the API expects",
"/api/meshobjects/meshtenants/0c9ef6d1-5c5d-4b0c-9a4b-1e0e3c1f2a3b": "meshStack answered HTTP 404: gone (errorCode NotFound)",
} {
meshStack.answer(gohttp.StatusNotFound, `{"errorCode":"NotFound","message":"gone"}`)
Expand Down Expand Up @@ -214,7 +214,7 @@ func TestApi(t *testing.T) {

_, err = meshStack.run(t, api.NewDocs(), "", "-X", "PATCH")

require.EqualError(t, err, "the API docs list no operation of PATCH")
require.EqualError(t, err, "the API docs have no operation for PATCH")
})

t.Run("api-docs describes the request of a command line of api", func(t *testing.T) {
Expand Down Expand Up @@ -267,7 +267,7 @@ func TestApi(t *testing.T) {

_, err = meshStack.run(t, api.NewDocs(), "", "/api/meshobjects/meshunknowns")

require.EqualError(t, err, "the API docs list no operation of /api/meshobjects/meshunknowns")
require.EqualError(t, err, "the API docs have no operation for /api/meshobjects/meshunknowns")
})

t.Run("api-docs --describe writes the latest version of each operation of a kind as Markdown", func(t *testing.T) {
Expand All @@ -289,13 +289,13 @@ func TestApi(t *testing.T) {

t.Run("api-docs --describe takes a kind of the API docs, and neither a path nor a request", func(t *testing.T) {
_, err := meshStack.run(t, api.NewDocs(), "", "--describe", "workspace")
require.EqualError(t, err, `the API docs list no meshObject kind "workspace", write one of buildingblock, buildingblockrun, tenant`)
require.EqualError(t, err, `the API docs have no meshObject kind "workspace"; use one of buildingblock, buildingblockrun, tenant`)

_, err = meshStack.run(t, api.NewDocs(), "", "--describe", "tenant.list")
require.EqualError(t, err, `the API docs list no action "list" of meshTenant, write one of show, delete`)
require.EqualError(t, err, `the API docs have no action "list" for meshTenant; use one of show, delete`)

_, err = meshStack.run(t, api.NewDocs(), "", "--describe", "bb", "/api/meshobjects/meshbuildingblocks")
require.EqualError(t, err, "--describe takes no path, see --help")
require.EqualError(t, err, "--describe cannot be combined with a path; see --help")

_, err = meshStack.run(t, api.NewDocs(), "", "--describe", "bb", "--api-version", "v1")
require.ErrorContains(t, err, "[api-version describe] were all set")
Expand All @@ -315,7 +315,7 @@ func TestApi(t *testing.T) {
require.Len(t, meshStack.requests, 1)
assert.Empty(t, meshStack.requests[0].accept, "a meshObject endpoint answers application/json with a 406 that names no version")
assert.Equal(t, "application/json", meshStack.requests[0].contentType)
assert.Contains(t, captured.String(), "Sending the request as given, without the API docs")
assert.Contains(t, captured.String(), "Sending the request unchanged, because the API docs are not available")
assert.Contains(t, captured.String(), "Run meshstack api-docs /api/meshobjects/meshbuildingblocks -X POST to see why.")

_, err = meshStack.run(t, api.NewDocs(), "")
Expand Down
4 changes: 2 additions & 2 deletions cmd/api/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ func describeSelector(spec openapi.Spec, value string) (openapi.Selector, error)
continue
}
if actions := spec.Actions(kind); action != "" && !slices.Contains(actions, action) {
return openapi.Selector{}, fmt.Errorf("the API docs list no action %q of %s, write one of %s",
return openapi.Selector{}, fmt.Errorf("the API docs have no action %q for %s; use one of %s",
action, kind, strings.Join(actions, ", "))
}
return openapi.Selector{Kind: kind, Action: action}, nil
}
return openapi.Selector{}, fmt.Errorf("the API docs list no meshObject kind %q, write one of %s",
return openapi.Selector{}, fmt.Errorf("the API docs have no meshObject kind %q; use one of %s",
name, strings.Join(commands, ", "))
}

Expand Down
32 changes: 15 additions & 17 deletions cmd/api/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,17 @@ func NewDocs() *cobra.Command {
cmd := &cobra.Command{
Use: "api-docs [<path or URL>]",
Short: "Show what the meshStack API docs say about a request or a meshObject kind",
Long: `Show what the meshStack API docs say about a request, given as 'meshstack api' takes it, or about
the operations of a meshObject kind.
Long: fmt.Sprintf(`Show what the meshStack API docs say about a request or about a meshObject kind.

Without a path, it lists every operation of the API, or those of the method and version the flags
name.
Give the request as you would give it to 'meshstack api': a path or a URL, with --%[1]s, --%[2]s, --%[3]s and --%[4]s if needed. Without a path, the command lists every operation of the API. With --%[1]s or --%[4]s, it lists only the operations of that method or version.

--describe names a kind as its command does, buildingblock or the alias bb, and optionally one of
its actions after a dot, as in bb.list. It describes each operation of the kind, in the latest
version it offers. It takes no path, and no flag of the request.
--%[5]s describes every operation of a meshObject kind, in the latest version of the kind. Give the kind by its command name, such as buildingblock, or by its alias, such as bb. To describe one action of the kind, add the action after a dot, as in bb.list. --%[5]s cannot be combined with a path or with the flags of a request.

--output json writes the part of the OpenAPI document instead.
--%[6]s %[7]s prints the matching part of the OpenAPI document as JSON.

--dev uses the development API docs.`,
--%[8]s uses the API docs of meshStack's development version.`,
methodFlagName, headerFlagName, requestJsonFlagName, apiVersionFlagName, describeFlagName,
internal.OutputFlagName, internal.OutputJson, devFlagName),
Example: ` meshstack api-docs
meshstack api-docs -X DELETE
meshstack api-docs /api/meshobjects/meshtenants/<uuid>
Expand All @@ -63,15 +61,15 @@ version it offers. It takes no path, and no flag of the request.
return err
}
if describe != "" && len(args) > 0 {
return fmt.Errorf("--%s takes no path, see --help", describeFlagName)
return fmt.Errorf("--%s cannot be combined with a path; see --help", describeFlagName)
}
if r.target.IsAbs() {
endpoint, _, profileErr := profileFor(cmd.Context(), r.target)
if profileErr != nil {
return profileErr
}
if endpoint.URL == nil {
return fmt.Errorf("no stored profile holds %s; give the path relative to the endpoint", r.target.Redacted())
return fmt.Errorf("no stored profile matches %s; give the path relative to the endpoint", r.target.Redacted())
}
if err = r.cutTo(endpoint); err != nil {
return err
Expand Down Expand Up @@ -108,10 +106,10 @@ version it offers. It takes no path, and no flag of the request.

flags.register(cmd, "", "show only the operations of this HTTP method")
cmd.Flags().StringVar(&describe, describeFlagName, "", "describe the operations of a meshObject kind, such as buildingblock, or one of them, such as bb.list")
cmd.MarkFlagsMutuallyExclusive(describeFlagName, "method")
cmd.MarkFlagsMutuallyExclusive(describeFlagName, "header")
cmd.MarkFlagsMutuallyExclusive(describeFlagName, "request-json")
cmd.MarkFlagsMutuallyExclusive(describeFlagName, "api-version")
cmd.MarkFlagsMutuallyExclusive(describeFlagName, methodFlagName)
cmd.MarkFlagsMutuallyExclusive(describeFlagName, headerFlagName)
cmd.MarkFlagsMutuallyExclusive(describeFlagName, requestJsonFlagName)
cmd.MarkFlagsMutuallyExclusive(describeFlagName, apiVersionFlagName)
output.Register(cmd.Flags())

return cmd
Expand All @@ -120,12 +118,12 @@ version it offers. It takes no path, and no flag of the request.
func noOperationError(selector openapi.Selector) error {
of := strings.TrimSpace(selector.Method + " " + selector.Path)
if of != "" {
of = " of " + of
of = " for " + of
}
if !selector.ApiVersion.IsZero() {
of += " in " + selector.ApiVersion.String()
}
return errors.New("the API docs list no operation" + of)
return errors.New("the API docs have no operation" + of)
}

// methodOrder puts reading before writing. A method the list leaves out, such as PATCH, goes last.
Expand Down
8 changes: 4 additions & 4 deletions cmd/api/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,20 @@ func profileFor(ctx context.Context, requestURL *url.URL) (endpoint xurl.URL, so
return xurl.URL{}, nil, err
}
if !current {
slog.WarnContext(ctx, fmt.Sprintf("Using profile '%s' for %s, as its endpoint %s holds it, rather than the current profile '%s'",
selected, requestURL.Redacted(), selected.Endpoint, profiles.CurrentProfile))
slog.WarnContext(ctx, fmt.Sprintf("Using profile '%s' for '%s' instead of the current profile ('%s') because its endpoint matches the given URL.",
selected, requestURL.Redacted(), profiles.CurrentProfile))
}
endpoint = selected.Endpoint
}
return endpoint, setting.Sources{setting.FallbackSource{Source: setting.LookupSource{
MatchingKey: profile.NameSetting.EnvKey(),
Description: "profile whose endpoint holds " + requestURL.Redacted(),
Description: "profile whose endpoint matches " + requestURL.Redacted(),
Func: func(context.Context) (string, error) { return string(selected.Name), nil },
}}}, nil
}

func notHeldError(profiles profile.Profiles, requestURL *url.URL, endpoint xurl.URL, ofProfile, source string) error {
urlEndpoint, _ := profiles.EndpointHolding(requestURL)
return fmt.Errorf("%s is for endpoint %s, not for endpoint %s%s from %s; leave out %s, or give the path relative to %s",
return fmt.Errorf("%s is under endpoint %s, which does not match the endpoint %s%s from %s; leave out %s, or give the path relative to %s",
requestURL.Redacted(), urlEndpoint, endpoint, ofProfile, source, source, endpoint)
}
8 changes: 4 additions & 4 deletions cmd/api/profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,17 @@ func TestProfileForARequestURL(t *testing.T) {

_, _, _, err = resolve(t, "https://localhost:1337/api")

require.ErrorContains(t, err, "https://localhost:1337/api is for endpoint https://localhost:1337, "+
"not for endpoint https://meshstack.example.com/a of profile 'a' from env "+profile.NameSetting.EnvKey())
require.ErrorContains(t, err, "https://localhost:1337/api is under endpoint https://localhost:1337, "+
"which does not match the endpoint https://meshstack.example.com/a of profile 'a' from env "+profile.NameSetting.EnvKey())
})

t.Run("an explicit endpoint fails where it does not hold the URL", func(t *testing.T) {
t.Setenv(meshstack.EndpointSetting.EnvKey(), "https://localhost:1337")

_, _, _, err := resolve(t, "https://meshstack.example.com/a/api")

require.ErrorContains(t, err, "https://meshstack.example.com/a/api is for endpoint https://meshstack.example.com/a, "+
"not for endpoint https://localhost:1337 from env "+meshstack.EndpointSetting.EnvKey())
require.ErrorContains(t, err, "https://meshstack.example.com/a/api is under endpoint https://meshstack.example.com/a, "+
"which does not match the endpoint https://localhost:1337 from env "+meshstack.EndpointSetting.EnvKey())
})

t.Run("with no profile stored, an explicit endpoint that holds the URL leaves the name of the first profile to the session", func(t *testing.T) {
Expand Down
Loading
Loading