feat: extend metadata-info endpoint to return all 4 TUF roles

[fghanmi]

Extend /api/v1/trust/metadata-info to return metadata for all 4 TUF roles (root, targets, snapshot, timestamp) instead of only root. Response is restructured as a dict keyed by role name.

[qodo-for-securesign]

PR Summary by Qodo

Extend trust metadata-info endpoint to return all 4 TUF roles

✨ Enhancement 📝 Documentation 🧪 Tests 🕐 40+ Minutes

AI Description

• Replace root-only metadata endpoint with /api/v1/trust/metadata-info for all TUF roles.
• Restructure response as a role-keyed map (root/targets/snapshot/timestamp) with status + expiry.
• Update OpenAPI/models and add unit tests for status calculation helpers.

Diagram

graph TD
  A[Client] --> B["/api/v1/trust/metadata-info"] --> C["API layer"] --> D["TrustService"] --> E[("TUF updater cache")] --> F{{"Remote TUF repo"}}
  D --> G["Role-keyed response"]

  subgraph Legend
    direction LR
    _svc["Service/Module"] ~~~ _db[("Cache/Store")] ~~~ _ext{{"External"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep root endpoint and add a new multi-role endpoint

• ➕ Avoids breaking clients still calling /trust/root-metadata-info
• ➕ Enables gradual migration with deprecation messaging
• ➖ More surface area to maintain (two endpoints / two schemas)
• ➖ Documentation and code paths remain duplicated longer

2. Single endpoint with optional `role` query parameter

• ➕ Supports both current multi-role view and per-role queries efficiently
• ➕ Can keep response schema stable (always return role-keyed object)
• ➖ Adds branching logic and more combinations to test/document
• ➖ May require clearer API guarantees for missing roles vs empty arrays

3. Return a list of role objects instead of `map[role][]info`

• ➕ More OpenAPI-friendly for strict clients (predictable schema)
• ➕ Easier to represent ordering and to extend with per-role fields later
• ➖ More verbose payload
• ➖ Callers must search the list rather than direct map lookup

Recommendation: The PR’s approach (one endpoint returning a role-keyed object) is a good fit for consumers that want a single call and fast role lookup. If backward compatibility matters, consider temporarily keeping /api/v1/trust/root-metadata-info as an alias (or adding a deprecation period) to avoid breaking existing clients while they migrate to the new response shape.

Files changed (7) +176 / -76

Enhancement (4) +98 / -61

handlers.goRename handler and call multi-role metadata service +2/-2

Rename handler and call multi-role metadata service

• Renames the trust handler from root-only metadata to 'GetApiV1TrustMetadataInfo'. Switches the handler to call 'GetTrustMetadataInfo' on the trust service.

internal/api/handlers.go

routes.goRewire route to new metadata-info handler +1/-1

Rewire route to new metadata-info handler

• Updates the chi route registration to serve '/api/v1/trust/metadata-info'. Removes the old '/api/v1/trust/root-metadata-info' binding.

internal/api/routes.go

models.goReplace root-only models with role-keyed metadata response models +34/-33

Replace root-only models with role-keyed metadata response models

• Introduces 'MetadataInfo' and 'MetadataInfoResponse' models (map keyed by role -> list of entries). Removes 'RootMetadataInfo'/'RootMetadataInfoList' and updates server interface and parameter types to the new endpoint name and types.

internal/models/models.go

trust.goImplement multi-role metadata info aggregation (root history + current roles) +61/-25

Implement multi-role metadata info aggregation (root history + current roles)

• Renames 'GetTrustRootMetadataInfo' to 'GetTrustMetadataInfo' and returns a role-keyed response including historical root versions plus current targets/snapshot/timestamp. Adds a shared helper to compute status (valid/expiring/expired) from version+expiry and reuses it when constructing non-root role entries.

internal/services/trust.go

Tests (1) +59 / -0

trust_test.goAdd unit tests for metadata status helper +59/-0

Add unit tests for metadata status helper

• Adds coverage for 'metadataInfoFromVersionAndExpires', validating status classification and version formatting. Includes boundary tests around the 30-day expiring threshold.

internal/services/trust_test.go

Documentation (2) +19 / -15

README.mdDocument new trust metadata-info endpoint +1/-1

Document new trust metadata-info endpoint

• Updates the endpoint list to replace '/api/v1/trust/root-metadata-info' with '/api/v1/trust/metadata-info'. Clarifies that the endpoint now returns metadata info for all four TUF roles.

README.md

rhtas-console.yamlUpdate OpenAPI path and schema for role-keyed metadata info +18/-14

Update OpenAPI path and schema for role-keyed metadata info

• Renames the path to '/api/v1/trust/metadata-info' and updates summary/description to cover all TUF roles. Replaces the old root-only list schema with 'MetadataInfoResponse' whose 'data' is an object keyed by role, each mapping to an array of 'MetadataInfo' entries.

internal/api/openapi/rhtas-console.yaml

[codecov-commenter]

Codecov Report

❌ Patch coverage is 24.56140% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 4.28%. Comparing base (1860749) to head (dfc9b5b).

Files with missing lines	Patch %	Lines
internal/services/trust.go	28.57%	35 Missing ⚠️
internal/models/models.go	0.00%	5 Missing ⚠️
internal/api/handlers.go	0.00%	2 Missing ⚠️
internal/api/routes.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##            main     #98      +/-   ##
========================================
+ Coverage   3.78%   4.28%   +0.50%     
========================================
  Files          9       9              
  Lines       2536    2566      +30     
========================================
+ Hits          96     110      +14     
- Misses      2433    2449      +16     
  Partials       7       7              

Flag	Coverage Δ
unit	4.28% <24.56%> (+0.50%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[qodo-for-securesign]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. tufRepositoryUrl parameter ignored 🐞 Bug ≡ Correctness

Description

The OpenAPI spec defines an optional tufRepositoryUrl query parameter for
/api/v1/trust/metadata-info, but the HTTP handler ignores it and always uses
h.trustService.GetTUFRepoURL(). Requests that specify tufRepositoryUrl will not query the requested
repository.

Code

internal/api/handlers.go[R107-112]

+func (h *Handler) GetApiV1TrustMetadataInfo(w http.ResponseWriter, r *http.Request) {
	tufRepoUrl := h.trustService.GetTUFRepoURL()
-	resp, statusCode, err := h.trustService.GetTrustRootMetadataInfo(r.Context(), tufRepoUrl)
+	resp, statusCode, err := h.trustService.GetTrustMetadataInfo(r.Context(), tufRepoUrl)
	if err != nil {
		writeError(w, statusCode, err.Error())
		return

Relevance

⭐⭐⭐ High

Team previously rejected removing tufRepositoryUrl query usage; likely wants handler to honor
OpenAPI param.

PR-#21
PR-#5

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

OpenAPI declares the query parameter, but the handler implementation never reads it and always
passes the configured URL to the service.

internal/api/openapi/rhtas-console.yaml[233-240]
internal/api/handlers.go[107-114]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `/api/v1/trust/metadata-info` handler does not read the documented `tufRepositoryUrl` query parameter and always uses the configured repository URL.

## Issue Context
OpenAPI documents `tufRepositoryUrl` as a supported optional query parameter for this endpoint.

## Fix Focus Areas
- internal/api/handlers.go[107-115]
- internal/api/openapi/rhtas-console.yaml[233-240]

## Implementation notes
- Parse `tufRepositoryUrl := r.URL.Query().Get("tufRepositoryUrl")`; if empty, fall back to `h.trustService.GetTUFRepoURL()`.
- Validate the URL before using it (and consider allow-listing/authorization if client-supplied repository URLs are not intended to be generally supported).
- If overriding is not supported by design, remove the parameter from OpenAPI to match actual behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. 30-day boundary inconsistent ✓ Resolved 🐞 Bug ≡ Correctness

Description

metadataInfoFromVersionAndExpires uses a strict “< 30 days” check, so an expiry exactly 30 days away
is classified as "valid" instead of "expiring". The new test case states the intended contract is
that exactly 30 days should be "expiring", but it doesn’t truly validate the equality boundary
because it uses a different time.Now() inside the helper.

Code

internal/services/trust.go[R772-782]

+func metadataInfoFromVersionAndExpires(version int64, expires time.Time) models.MetadataInfo {
+	now := time.Now().UTC()
+	var status string
+	switch {
+	case expires.Before(now):
+		status = "expired"
+	case expires.Sub(now) < 30*24*time.Hour:
+		status = "expiring"
+	default:
+		status = "valid"
+	}

Relevance

⭐⭐⭐ High

Small correctness/test-determinism tweak; repo commonly accepts time-related robustness fixes in
services.

PR-#92
PR-#90

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The helper uses a strict "< 30 days" threshold, while the new test case encodes an expectation that
exactly 30 days should be "expiring"; the mismatch indicates an unclear/incorrect boundary contract.

internal/services/trust.go[772-787]
internal/services/trust_test.go[282-320]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`metadataInfoFromVersionAndExpires` treats metadata as `expiring` only when `expires.Sub(now) < 30*24*time.Hour`. The added unit test indicates the intended contract is that exactly 30 days should be `expiring`, but the implementation currently classifies the exact boundary as `valid`.

## Issue Context
This affects role metadata status in `/api/v1/trust/metadata-info`.

## Fix Focus Areas
- internal/services/trust.go[772-787]
- internal/services/trust_test.go[282-338]

## Implementation notes
- Decide the intended boundary (inclusive vs exclusive). If inclusive, change the check to `<= 30*24*time.Hour` (and consider using the same helper for root status too).
- Make the test deterministic by avoiding dual `time.Now()` reads (e.g., pass `now` into the helper or refactor helper to accept a reference time for testing).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Roles omitted from response ✓ Resolved 🐞 Bug ≡ Correctness

Description

GetTrustMetadataInfo only includes "targets", "snapshot", and "timestamp" in the response when the
corresponding TrustedMetadataSet entries are non-nil, even though the endpoint description says it
returns info for all 4 TUF roles. This can produce a successful response missing one or more role
keys, breaking clients expecting a stable four-role payload.

Code

internal/services/trust.go[R206-227]

+	result := models.MetadataInfoResponse{
+		Data:    map[string][]models.MetadataInfo{"root": rootEntries},
+		RepoUrl: &repo.opts.RepositoryBaseURL,
+	}
+
+	if targets := trustedMeta.Targets["targets"]; targets != nil {
+		result.Data["targets"] = []models.MetadataInfo{
+			metadataInfoFromVersionAndExpires(targets.Signed.Version, targets.Signed.Expires),
		}
-		results.Data = append(results.Data, rootInfo)
	}

-	results.RepoUrl = &repo.opts.RepositoryBaseURL
-	return results, http.StatusOK, nil
+	if trustedMeta.Snapshot != nil {
+		result.Data["snapshot"] = []models.MetadataInfo{
+			metadataInfoFromVersionAndExpires(trustedMeta.Snapshot.Signed.Version, trustedMeta.Snapshot.Signed.Expires),
+		}
+	}
+
+	if trustedMeta.Timestamp != nil {
+		result.Data["timestamp"] = []models.MetadataInfo{
+			metadataInfoFromVersionAndExpires(trustedMeta.Timestamp.Signed.Version, trustedMeta.Timestamp.Signed.Expires),
+		}
+	}

Relevance

⭐⭐⭐ High

PR intent/OpenAPI promise all four roles; team tends to align spec and payload shape for clients.

PR-#34
PR-#5

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The OpenAPI description promises all four roles, but the service only adds non-root roles when
metadata is present, meaning the response can omit those role keys.

internal/api/openapi/rhtas-console.yaml[224-230]
internal/services/trust.go[206-227]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`/api/v1/trust/metadata-info` is documented to return metadata info for all 4 TUF roles, but the implementation only adds `targets`, `snapshot`, and `timestamp` keys conditionally.

## Issue Context
Clients may rely on the presence of all role keys and fail or mis-handle missing keys.

## Fix Focus Areas
- internal/services/trust.go[206-229]
- internal/api/openapi/rhtas-console.yaml[224-230]

## Implementation notes
- Initialize `result.Data` with keys `root`, `targets`, `snapshot`, `timestamp` (empty arrays by default), and populate entries when available.
- Alternatively, if any role metadata is missing, return a 503/500 with a clear error so the contract is not silently violated.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---