OSAC-3995: Public Storage Tier API — fulfillment-service backend

[wgordon17]

OSAC-3995: Public Storage Tier API — fulfillment-service backend

Jira: https://redhat.atlassian.net/browse/OSAC-3995 (child of epic OSAC-3014)

Summary

Adds a public, tenant-facing StorageTiers gRPC service (Get+List only) to
fulfillment-service, wrapping the existing private StorageTier API via the same
builder/delegate/mapper pattern already used for BareMetalInstanceTypes. Tenants can now browse
available storage tiers (protocol, bandwidth, quota, encryption) to choose one when provisioning a
ComputeInstance.

Changes

• New public.v1.StorageTier/StorageTierSpec/StorageTierStatus proto types and StorageTiers
  service (Get/List only, no Create/Update/Delete — matches the BareMetalInstanceTypes/
  ExternalIPPools precedent for read-only platform-catalog resources).
• StorageTiersServer implementation with two-layer CEL filter validation to prevent tenants from
  probing excluded private fields via the filter side-channel (a scoped fix for BUG-018; the
  broader pattern across 24 other servers is tracked separately in OSAC-3609).
• OPA allowlist entries, REST gateway registration, and CLI commands (get storagetier,
  describe storagetier switched to the public client).

Testing

• Unit tests: internal/servers/storage_tiers_server_test.go — builder validation, List/Get
  delegation and field flattening, pagination, both filter-validation layers, defensive handling
  of malformed backend-association counts, and a schema-drift regression test.
• Integration tests: it/it_public_storage_tiers_test.go — end-to-end List/Get, pagination,
  filtering, the private-API permission boundary, and documented BUG-005 pass-through behavior.
• Coverage: Comprehensive against all behavioral paths in the test strategy; full local
  validation (build, gofmt, go vet, unit suite, integration suite) passed.

Acceptance Criteria

• Public StorageTiers/Get RPC implemented in fulfillment-service
• Public StorageTiers/List RPC implemented in fulfillment-service
• Distinct public.v1.StorageTier message, mirrors private minus backend_id
• OPA policies enforce tenant-scoped access to tiers
• Proto definitions follow OSAC API conventions (spec/status, pagination)
• Unit tests for public tier endpoints
• Integration tests verify tenant can list but not modify tiers

Summary by CodeRabbit

• New Features
  • Added a public Storage Tiers API for retrieving and listing storage tiers.
  • Added filtering, sorting, pagination, and HTTP gateway support.
  • Added CLI commands for compact listings and detailed tier information.
  • Added protocol, bandwidth, quota, encryption, lifecycle state, and status details.
  • Enabled client permissions for Storage Tier retrieval and listing.
• Bug Fixes
  • Improved handling of invalid filters, unsupported fields, missing results, and backend data errors.

[openshift-ci-robot]

@wgordon17: This pull request references OSAC-3995 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

OSAC-3995: Public Storage Tier API — fulfillment-service backend

Jira: https://redhat.atlassian.net/browse/OSAC-3995 (child of epic OSAC-3014)

Summary

Adds a public, tenant-facing StorageTiers gRPC service (Get+List only) to
fulfillment-service, wrapping the existing private StorageTier API via the same
builder/delegate/mapper pattern already used for BareMetalInstanceTypes. Tenants can now browse
available storage tiers (protocol, bandwidth, quota, encryption) to choose one when provisioning a
ComputeInstance.

Changes

• New public.v1.StorageTier/StorageTierSpec/StorageTierStatus proto types and StorageTiers
  service (Get/List only, no Create/Update/Delete — matches the BareMetalInstanceTypes/
  ExternalIPPools precedent for read-only platform-catalog resources).
• StorageTiersServer implementation with two-layer CEL filter validation to prevent tenants from
  probing excluded private fields via the filter side-channel (a scoped fix for BUG-018; the
  broader pattern across 24 other servers is tracked separately in OSAC-3609).
• OPA allowlist entries, REST gateway registration, and CLI commands (get storagetier,
  describe storagetier switched to the public client).

Testing

• Unit tests: internal/servers/storage_tiers_server_test.go — builder validation, List/Get
  delegation and field flattening, pagination, both filter-validation layers, defensive handling
  of malformed backend-association counts, and a schema-drift regression test.
• Integration tests: it/it_public_storage_tiers_test.go — end-to-end List/Get, pagination,
  filtering, the private-API permission boundary, and documented BUG-005 pass-through behavior.
• Coverage: Comprehensive against all behavioral paths in the test strategy; full local
  validation (build, gofmt, go vet, unit suite, integration suite) passed.

Acceptance Criteria

• Public StorageTiers/Get RPC implemented in fulfillment-service
• Public StorageTiers/List RPC implemented in fulfillment-service
• Distinct public.v1.StorageTier message, mirrors private minus backend_id
• OPA policies enforce tenant-scoped access to tiers
• Proto definitions follow OSAC API conventions (spec/status, pagination)
• Unit tests for public tier endpoints
• Integration tests verify tenant can list but not modify tiers

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wgordon17

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• fulfillment-service/OWNERS [wgordon17]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 42178878-5e81-41c3-84c0-b60be3b8f5d7

📥 Commits

Reviewing files that changed from the base of the PR and between a8e9b65 and 7fb3b54.

📒 Files selected for processing (1)

• fulfillment-service/internal/servers/storage_tiers_server.go

🚧 Files skipped from review as they are similar to previous changes (1)

• fulfillment-service/internal/servers/storage_tiers_server.go

---

Walkthrough

Adds a public StorageTiers protobuf API with gRPC and REST access, private-to-public mapping, filter validation, authorization, CLI support, server registration, and unit and integration tests.

Changes

Public Storage Tiers API

Layer / File(s)	Summary
Public storage-tier contract fulfillment-service/proto/public/osac/public/v1/*	Defines public storage-tier resources, protocols, states, specifications, status, pagination, filtering, ordering, and HTTP-mapped List and Get RPCs.
Storage-tier server adapter fulfillment-service/internal/servers/storage_tiers_server.go, fulfillment-service/internal/servers/storage_tiers_server_test.go	Validates public filters, delegates requests to the private server, flattens backend properties into public tiers, maps protocols, and handles invalid backend associations. Tests cover construction, delegation, pagination, validation, mapping, and schema drift.
API registration and access control fulfillment-service/internal/cmd/service/start/grpcserver/start_grpc_server_cmd.go, fulfillment-service/internal/cmd/service/start/restgateway/start_rest_gateway_cmd.go, fulfillment-service/internal/auth/policies/authz.rego, fulfillment-service/docs/AUTH.md, fulfillment-service/it/it_public_storage_tiers_test.go	Registers gRPC and REST handlers and permits client-level StorageTiers/List and StorageTiers/Get access. Integration tests cover public access, filtering, pagination, authorization, and protocol handling.
Storage-tier CLI commands fulfillment-service/internal/cmd/cli/get/get_cmd.go, fulfillment-service/internal/cmd/cli/get/storagetier/get_storagetier_cmd.go, fulfillment-service/internal/cmd/cli/describe/storagetier/describe_storagetier_cmd.go	Adds get storagetier, updates describe storagetier to use the public API, and renders compact and detailed public tier fields.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant StorageTiersServer
  participant PrivateStorageTiersServer
  participant StorageBackendsDAO
  Client->>StorageTiersServer: List or Get request
  StorageTiersServer->>PrivateStorageTiersServer: Validated delegated request
  PrivateStorageTiersServer->>StorageBackendsDAO: Resolve backend association
  StorageBackendsDAO-->>StorageTiersServer: Backend properties
  StorageTiersServer-->>Client: Mapped public StorageTier response

Loading

Possibly related PRs

• osac-project/osac#201: Relates to the shared storage-tier APIs and schemas used by the public storage-tier server.

Suggested reviewers: eliorerz, vladikr, adriengentil

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
No-Hardcoded-Secrets	❌ Error	The PR adds Password: "secret" directly to backend credential fields in two *_test.go files (three occurrences); no test/fixture variable qualifies for the stated exception.	Use a named fixture/test variable for the dummy value, or remove the hardcoded credential; do not assign the literal directly to Password.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (9 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the public Storage Tier API added to the fulfillment-service backend.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto	✅ Passed	The merge-base-to-HEAD diff adds no MD5, SHA1, DES, 3DES, RC4, Blowfish, ECB, or crypto APIs; changed server and CLI code has no crypto imports or secret comparisons.
No-Injection-Vectors	✅ Passed	Changed production files contain no SQL construction or listed eval/exec, pickle, unsafe YAML, os.system, shell=True, or dangerouslySetInnerHTML sinks; filters are CEL-validated before delegation.
Container-Privileges	✅ Passed	The PR diff adds no container or Kubernetes manifests and no privilege-setting lines; existing fulfillment deployments use runAsNonRoot: true and allowPrivilegeEscalation: false.
No-Sensitive-Data-In-Logs	✅ Passed	Added application logs contain only static messages, mapping errors, backend counts, tier IDs, and enum values; no passwords, tokens, PII, customer payloads, or hostnames are logged.
Ai-Attribution	✅ Passed	All 13 PR commits include Assisted-by: Claude Code <noreply@anthropic.com>, and no AI Co-Authored-By trailer appears in the PR range.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:43 PM UTC · Completed 4:00 PM UTC

Commit: ad0f7d5 · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [API contract / pagination consistency] fulfillment-service/internal/servers/storage_tiers_server.go:232 — When List silently omits malformed tiers (those with zero or more than one backend association), Total is set from privateResponse.GetTotal() (which counts the omitted tiers) while Size reflects only the successfully-mapped items. This creates an inconsistency: a pagination client summing page sizes will never reach Total, potentially causing stuck-pagination loops. Other public servers use the same SetTotal(privateResponse.GetTotal()) pattern, but they never skip items during mapping, so the inconsistency is unique to this server.
  Remediation: Adjust Total to subtract dropped items (e.g., response.SetTotal(privateResponse.GetTotal() - int32(len(privateItems) - len(publicItems)))), or document that total is approximate when tiers with malformed backend data exist.

• [stale-convention-doc] fulfillment-service/docs/API.md:267 — API.md states "Services of the public API must always declare the following five methods" (Create, List, Get, Update, Delete). The new StorageTiers service only declares Get and List, following the read-only catalog pattern also used by HostTypes, InstanceTypes, and BareMetalInstanceTypes. The documented convention should acknowledge this established exception.
  Remediation: Add a note to the "Standard methods" section in API.md acknowledging that read-only catalog/platform services may declare only Get and List when Create/Update/Delete are managed exclusively through the private API.

Low

• [order clause forwarding] fulfillment-service/internal/servers/storage_tiers_server.go:207 — The order parameter is forwarded verbatim to the private delegate without validation. The DAO currently hardcodes ordering by id (ignoring the parameter entirely), so this is safe today. If the DAO is later enhanced to support user-supplied ordering, this code path would need the same two-layer validation applied to the filter parameter. All other public servers forward order the same way, so this is a codebase-wide future-proofing concern, not StorageTiers-specific.

• [proto filter documentation] fulfillment-service/proto/public/osac/public/v1/storage_tiers_service.proto:48 — The filter field documentation does not mention that filtering by spec.protocol, spec.max_read_bandwidth_mbs, spec.max_write_bandwidth_mbs, spec.quota_gib, and spec.encryption_enabled is explicitly unsupported and will return InvalidArgument. Users would discover this limitation only at runtime.

• [public-enum-scope] fulfillment-service/proto/public/osac/public/v1/storage_tier_type.proto:21 — The StorageProtocol enum is defined inline in storage_tier_type.proto rather than in a shared file. If a public Volume API is added later that also needs StorageProtocol, the enum would need to be moved to a shared storage_common_type.proto. This is a source reorganization, not a wire-breaking change, and is acceptable for now given there is no public Volume API.

Previous run

Review

Findings

Low

• [CEL Filter Oracle - Order Parameter] fulfillment-service/internal/servers/storage_tiers_server.go:178 — The order parameter from the public request is forwarded verbatim to the private delegate without the same two-layer validation applied to filter. Currently unexploitable (the DAO hardcodes const order = "id" and ignores user-provided order values), but creates an asymmetry worth noting for defense-in-depth. This is a pre-existing pattern across all public-to-private delegation servers, not unique to this PR.
  Remediation: Apply the same validation to the order parameter as is applied to filter, or explicitly clear/drop the order field before forwarding.

• [stale-doc] fulfillment-service/docs/AUTH.md:574 — The client permissions list in AUTH.md does not include the newly-added StorageTiers/Get and StorageTiers/List endpoints. A reader using this doc to understand client access would not know StorageTiers exists. This extends pre-existing documentation drift (several other services are also missing from this list).
  Remediation: Add Storage Tiers: Get, List to the Client Users list in AUTH.md.

• [enum-duplication] fulfillment-service/proto/public/osac/public/v1/storage_tier_type.proto:21 — The StorageProtocol enum is defined independently in both osac.public.v1 and osac.private.v1. Values match today (UNSPECIFIED=0, NFS=1, BLOCK=2), and the server uses an explicit switch with a logged fallback for unknown values, which is the correct defensive pattern. If a new protocol is added to the private enum without updating the public one, it will silently map to UNSPECIFIED.

• [comment-style] fulfillment-service/internal/cmd/cli/describe/storagetier/describe_storagetier_cmd.go — The run method omits the step-by-step inline section comments (// Get the context:, // Get the console:, etc.) present in established describe commands like describe instancetype. Both styles coexist in the codebase, so this is not a violation.

---

Labels: PR adds new public StorageTiers API to the fulfillment-service storage subsystem

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 5:32 PM UTC · Ended 5:35 PM UTC

Commit: ac29b87 · View workflow run →

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@fulfillment-service/internal/cmd/cli/get/storagetier/get_storagetier_cmd.go`:
- Around line 106-159: Update renderTierTable and renderTierDetail in
fulfillment-service/internal/cmd/cli/get/storagetier/get_storagetier_cmd.go
(lines 106-159) to return errors from every fmt.Fprint* call and
tabwriter.Writer.Flush, then propagate those errors through renderStorageTier
and the respective command at lines 66-84. Apply the same output-error
propagation to the storage-tier describe command in
fulfillment-service/internal/cmd/cli/describe/storagetier/describe_storagetier_cmd.go
(lines 88-120), ensuring broken-pipe failures are returned instead of producing
success.

In `@fulfillment-service/internal/servers/storage_tiers_server.go`:
- Around line 214-223: Update the List conversion loop around privateItems and
toPublicTier to log malformed tiers and continue instead of returning the
conversion error; build the result slice with only successfully converted items
and adjust size to match the filtered items count. Keep Get’s existing Internal
error behavior unchanged, and update the affected List test expectations
accordingly.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 8696598c-879a-4f50-b239-5a4d1b122027

📥 Commits

Reviewing files that changed from the base of the PR and between 81ecdd8 and ac29b87.

⛔ Files ignored due to path filters (6)

• fulfillment-service/internal/api/osac/public/v1/storage_tier_type.pb.go is excluded by !**/*.pb.go
• fulfillment-service/internal/api/osac/public/v1/storage_tier_type_protoopaque.pb.go is excluded by !**/*.pb.go
• fulfillment-service/internal/api/osac/public/v1/storage_tiers_service.pb.go is excluded by !**/*.pb.go
• fulfillment-service/internal/api/osac/public/v1/storage_tiers_service.pb.gw.go is excluded by !**/*.pb.gw.go
• fulfillment-service/internal/api/osac/public/v1/storage_tiers_service_grpc.pb.go is excluded by !**/*.pb.go
• fulfillment-service/internal/api/osac/public/v1/storage_tiers_service_protoopaque.pb.go is excluded by !**/*.pb.go

📒 Files selected for processing (11)

• fulfillment-service/internal/auth/policies/authz.rego
• fulfillment-service/internal/cmd/cli/describe/storagetier/describe_storagetier_cmd.go
• fulfillment-service/internal/cmd/cli/get/get_cmd.go
• fulfillment-service/internal/cmd/cli/get/storagetier/get_storagetier_cmd.go
• fulfillment-service/internal/cmd/service/start/grpcserver/start_grpc_server_cmd.go
• fulfillment-service/internal/cmd/service/start/restgateway/start_rest_gateway_cmd.go
• fulfillment-service/internal/servers/storage_tiers_server.go
• fulfillment-service/internal/servers/storage_tiers_server_test.go
• fulfillment-service/it/it_public_storage_tiers_test.go
• fulfillment-service/proto/public/osac/public/v1/storage_tier_type.proto
• fulfillment-service/proto/public/osac/public/v1/storage_tiers_service.proto

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 5:36 PM UTC · Ended 5:45 PM UTC

Commit: 528f129 · View workflow run →

Auto-dismissed: only Prow labels gate merging

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 5:46 PM UTC · Ended 6:01 PM UTC

Commit: a8e9b65 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:02 PM UTC · Completed 6:19 PM UTC

Commit: 7fb3b54 · View workflow run →