Repository navigation
feat: read Assets object type attributes, and select the Jira API version on the passthrough - #76
Conversation
The passthrough was bound to /rest/api/3. Add --api-version so v2 is reachable: both versions expose the same resources and differ in how they carry rich text, so reading through v2 returns a description as wiki markup instead of ADF. The version is interpolated into the URL, which the path-level guard never sees, so it is validated against an allowlist inside the API layer rather than only at the flag.
An Assets object omits every attribute it holds no value for, so reading one object cannot show whether an attribute exists on its type at all. Add 'jira assets attributes <object-type-id>' (alias 'fields'), which reads the type definition instead, and prints the type's name above the table so a wrong id is distinguishable from a type that genuinely lacks the attribute. Assets grants reads per resource kind: the two endpoints need read:cmdb-type:jira and read:cmdb-attribute:jira, which the existing object and schema scopes do not cover. Both are added to the requested scopes and checked before the request, so a token without them fails with the scope named rather than an opaque 401 "scope does not match".
Also splits the Assets scopes onto their own line in the OAuth app setup, now that there are four of them.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for listing Jira Assets object types and their attributes, including new CLI commands and API client methods. It also introduces the ability to select Jira API versions (v2 or v3) for the api passthrough command. The reviewer suggested implementing a TypeName() helper method on AssetObjectTypeAttribute to correctly label non-default attribute types, updating the CLI output to use this method, and adding corresponding unit tests.
…g cardinality The type and its attributes are gated by different scopes, so a token holding one but not the other failed on the second request only after the first was fixed. Check both before either request. The multi-value label read every upper cardinality other than 1 as unbounded, which labels an absent or zero field as multi-valued. Assets spells unbounded as a negative number, so require that form or a stated bound above one.
The contract of the combined pre-check is that one message lists every gap; a per-call check would have satisfied the type before failing on the attributes.
A Select rejects any value outside its option list, so the options are what a caller has to match when writing the attribute. Reading the type without them answers that the attribute exists but not what may be put in it.
Only a Default attribute carries its kind in defaultType; a reference, user, group, or project attribute leaves it empty and rendered as a blank column. Fall back to the numeric type, reported verbatim because Assets does not publish that enum and a guessed label would be worse than a number. Also corrects the options doc: a Text attribute can carry a predefined value list too, so options are not confined to Select.
…on a bad version Any negative upper cardinality read as unbounded; -1 is the only negative a live workspace produces, so an unobserved negative now reads as no flag rather than as a claim about a sentinel whose meaning Assets does not publish. The version allowlist ran only inside the API layer, after the authenticated client was built, so an unsupported version surfaced behind an auth error for anyone not logged in. The URL-building guard stays; this is the message.
|
/gemini review |
…le to the API The id is interpolated into the request path and url.PathEscape leaves ";" intact, which is enough to append a path parameter and change how the server parses the request. The passthrough already blocks that class for Jira paths; the Assets reads had no equivalent guard. An id is always digits, so an allowlist closes it without chasing encodings. The scope union is now derived from the per-call requirements instead of restating them, so the up-front check cannot advertise less than the calls demand. IsMulti moves next to Required and TypeName, leaving the command with presentation only. Both reads wrap their error so a failure names which one. RawGet had no callers left and is removed rather than kept as a second entry point to the same URL construction. ObjectCount and Inherited lose omitempty, which was hiding a zero count and a false inherited from JSON consumers.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for querying Jira Assets (CMDB) object types and their attributes, including a new atl jira assets attributes command. It also enhances the read-only REST passthrough (atl jira api) command by adding an --api-version flag to select between Jira platform REST API versions 2 and 3. Feedback suggests executing the independent network requests for fetching object type metadata and attributes concurrently to reduce latency and improve responsiveness.
There was a problem hiding this comment.
Code Review
This pull request introduces support for listing Jira Assets (CMDB) object types and attributes via a new 'attributes' command, updates the read-only REST passthrough to support selecting the Jira API version (v2 or v3), and adds corresponding validation, documentation, and tests. Feedback on the changes suggests running the object type and attribute retrieval concurrently in the new command to reduce latency (while pre-resolving the workspace ID to prevent a data race), and fixing a resource leak in 'TestObjectTypeIDMustBeNumeric' where a test server is not closed.
| objectType, err := client.ObjectType(cmd.Context(), args[0]) | ||
| if err != nil { | ||
| return fmt.Errorf("read object type %s: %w", args[0], err) | ||
| } | ||
| attributes, err := client.ObjectTypeAttributes(cmd.Context(), args[0]) | ||
| if err != nil { | ||
| return fmt.Errorf("read attributes of object type %s: %w", args[0], err) | ||
| } |
There was a problem hiding this comment.
The client.ObjectType and client.ObjectTypeAttributes calls are currently executed sequentially. Since they are independent, they can be executed concurrently to significantly reduce command latency.
Additionally, because AssetsClient does not protect its workspaceID field with a mutex, calling these two methods concurrently when the workspace ID is not yet cached would introduce a data race. Pre-resolving the workspace ID sequentially first avoids this data race while still allowing the actual API requests to run in parallel.
// Pre-resolve workspace ID sequentially to avoid a data race in the client
if _, err := client.WorkspaceID(cmd.Context()); err != nil {
return err
}
type typeResult struct {
val *api.AssetObjectType
err error
}
type attrsResult struct {
val []api.AssetObjectTypeAttribute
err error
}
typeChan := make(chan typeResult, 1)
attrsChan := make(chan attrsResult, 1)
go func() {
val, err := client.ObjectType(cmd.Context(), args[0])
typeChan <- typeResult{val, err}
}()
go func() {
val, err := client.ObjectTypeAttributes(cmd.Context(), args[0])
attrsChan <- attrsResult{val, err}
}()
tRes := <-typeChan
if tRes.err != nil {
return fmt.Errorf("read object type %s: %w", args[0], tRes.err)
}
objectType := tRes.val
aRes := <-attrsChan
if aRes.err != nil {
return fmt.Errorf("read attributes of object type %s: %w", args[0], aRes.err)
}
attributes := aRes.val| func TestObjectTypeIDMustBeNumeric(t *testing.T) { | ||
| client := newTestAssetsClient(httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { | ||
| t.Error("a rejected id reached the network") | ||
| w.WriteHeader(http.StatusOK) | ||
| })), "workspace-456") |
There was a problem hiding this comment.
The test server created by httptest.NewServer is passed directly to newTestAssetsClient without being assigned to a variable, which prevents it from being closed. This leaks the test server and its background listener. Assign the server to a variable and defer server.Close() to clean up resources properly.
func TestObjectTypeIDMustBeNumeric(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
t.Error("a rejected id reached the network")
w.WriteHeader(http.StatusOK)
}))
defer server.Close()
client := newTestAssetsClient(server, "workspace-456")
|
🔏 Readiness attested — the Readiness summaryPR Readiness Check (#76, 6457687) Verification
Verified live against both Atlassian sites with a binary built from this branch: DispositionsCouncil: 3 rounds, 12 findings FIXED, 4 REFUTED, 4 DEFERRED, 3 ADVISORY. No Gemini: reviewed the current head. Its blank-TYPE-column finding was confirmed Three refutations were reviewers contradicted by the file itself: two Council Full detail in Disclosed limitationsThe Council jury ran degraded in all three rounds, returning 2 of 3 usable This change grows the OAuth scopes every login requests by |
Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
1 participant
What
Two read-only additions, both driven by questions the CLI could not answer.
atl jira assets attributes <object-type-id>(aliasfields) lists theattributes an Assets object type defines, with their numeric ids. An Assets
object omits every attribute it holds no value for, so reading one object can
never show whether an attribute exists on its type at all; this reads the type
definition instead. The object type's name is printed above the table, because
a wrong id and a type that genuinely lacks an attribute otherwise look the
same.
atl jira api --api-version <2|3>lifts the passthrough off its hardcoded/rest/api/3. Both versions expose the same resources and differ in how theycarry rich text, so reading through v2 returns a description as wiki markup
instead of ADF.
Why
Deciding whether a Jira Assets object type carries a given attribute came up
while verifying a sync that writes to Assets, and neither AQL nor a single
object read can answer it. AQL resolves attribute names workspace-wide in both
WHEREandORDER BY, so a type-scoped probe returning zero rows provesnothing, and an object without a value for an attribute is indistinguishable
from an object whose type lacks it.
New OAuth scopes
Assets grants reads per resource kind.
GET /objecttype/{id}needsread:cmdb-type:jiraandGET /objecttype/{id}/attributesneedsread:cmdb-attribute:jira; the object and schema scopes the CLI alreadyrequests cover neither. Both are added to
DefaultScopes()and checked beforethe request, so a token without them fails with the missing scope named rather
than as an opaque
401 "scope does not match"from Atlassian.This needs action outside the repo before the command works: the atl-cli
OAuth app must list the two scopes in the developer console, and each site
needs a fresh login afterwards. Existing tokens do not gain scopes
retroactively. Consent on
enthus.atlassian.netis site-admin gated.Security
The version segment is interpolated into the URL, which
validateRawPathnever inspects, so
JiraBaseURLVersionvalidates it against an allowlistinside the API layer. The flag-level check is a convenience on top of that, not
the guarantee.
TestJiraBaseURLVersioncovers3/../../..and../agile/1.0among others, and fails if the allowlist check is neutered (verified by
mutation). The passthrough stays GET-only.
Testing
New tests:
TestJiraBaseURLVersion,TestJiraBaseURLMatchesDefaultVersion,TestRawGetVersionRejectsUnsupportedVersion,TestNewCmdAPI_APIVersionFlag,TestAssetsObjectType,TestAssetsObjectTypeAttributes,TestAssetsObjectTypeReadsNeedTypeAndAttributeScopes,TestAttributeFlags.Run against both live sites with a binary built from this branch. Every
objecttype/*endpoint returns401 "scope does not match"on prod andsandbox alike, which is the scope gap above, not a defect in these commands;
with the pre-check in place the CLI now reports the missing scope by name and
points at the re-login. The endpoints the current scopes do cover
(
objectschema/list,objectschema/{id}, the workspace lookup) still return200, so the wall is specific to the object type resources.