OSAC-3082: implement reference validation interceptor

[htayrie-rh]

OSAC-3082: Implement reference validation interceptor

Jira: OSAC-3082
Story type: [DEV]

Summary

Implements a centralized gRPC unary server interceptor that validates resource references in Create and Update requests using protoreflect. The interceptor discovers reference-typed fields by naming convention (*Reference, *LocalReference), validates referenced resources exist via registered lookup functions, auto-populates missing id/name fields, and returns aggregated InvalidArgument errors with structured BadRequest FieldViolation details.

This is the foundation for OSAC-1330 (type-safe resource references). The interceptor is registered in the gRPC chain but has no lookup functions yet — it becomes active when subsequent epics (Networking, Compute, IP Management) register their reference types.

Changes

New package: internal/references/

• ReferenceValidator interceptor with builder pattern (NewReferenceValidator().SetLogger().SetMetricsRegisterer().Build())
• Protoreflect-based message walker handling singular, repeated, nested, and oneof fields
• Three resolution modes: name-only (auto-populate id), id-only (auto-populate name), both-provided (verify agreement)
• Tenant context extraction: local refs use owning resource's tenant/project; full refs support explicit project and shared tenant
• Fail-closed design: unregistered reference types return Internal error
• Prometheus metrics: osac_reference_validation_total (CounterVec) and osac_reference_validation_duration_seconds (HistogramVec)

Interceptor chain registration

• Added after JIT provisioning interceptor, before server handlers
• Both unary (validates) and stream (pass-through) chains

Test proto definitions

• proto/tests/osac/tests/v1/reference_test_types.proto with 7 message types and 3 request types covering all reference field patterns

Testing

• Unit tests: 45 Ginkgo tests covering builder, method filtering, field discovery (simple/nested/repeated/oneof), stream pass-through, reference type detection, lookup registration, tenant context extraction, error reporting/aggregation, non-proto pass-through, and Prometheus metrics
• Integration tests: N/A (interceptor is inert until lookup functions are registered in subsequent epics)
• Coverage: 84.6% — all public API behavioral contracts tested

Acceptance Criteria

• AC-1: gRPC unary server interceptor validates reference-typed fields in Create and Update requests
• AC-2: Detects reference fields by proto message naming convention via protoreflect
• AC-3: Invalid references return InvalidArgument with BadRequest containing FieldViolation entries with field paths
• AC-4: Aggregates all reference errors in a single response (no short-circuit)
• AC-5: Local references use owning resource's tenant/project; full references use explicit tenant/project with fallback
• AC-6: Prometheus metrics emitted (osac_reference_validation_total, osac_reference_validation_duration_seconds)

Summary by CodeRabbit

• New Features
  • Added automatic gRPC validation for protobuf “reference” fields on unary Create/Update requests, including nested, repeated, one-of, and local/shared variants.
  • When possible, missing reference identifiers (ID/name) are auto-populated after successful lookup; invalid or conflicting references return field-level InvalidArgument errors.
  • Validation emits Prometheus metrics for valid/invalid/error outcomes and lookup timing.
• Tests
  • Added an extensive Ginkgo/Gomega suite plus new protobuf test types to cover success, failure, aggregation, and metric behavior.
• Chores
  • Promoted select Go dependencies from indirect to direct requirements.

[openshift-ci-robot]

@htayrie-rh: This pull request references OSAC-3082 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 epic to target the "5.0.0" version, but no target version was set.

Details

In response to this:

OSAC-3082: Implement reference validation interceptor

Jira: OSAC-3082
Story type: [DEV]

Summary

Implements a centralized gRPC unary server interceptor that validates resource references in Create and Update requests using protoreflect. The interceptor discovers reference-typed fields by naming convention (*Reference, *LocalReference), validates referenced resources exist via registered lookup functions, auto-populates missing id/name fields, and returns aggregated InvalidArgument errors with structured BadRequest FieldViolation details.

This is the foundation for OSAC-1330 (type-safe resource references). The interceptor is registered in the gRPC chain but has no lookup functions yet — it becomes active when subsequent epics (Networking, Compute, IP Management) register their reference types.

Changes

New package: internal/references/

• ReferenceValidator interceptor with builder pattern (NewReferenceValidator().SetLogger().SetMetricsRegisterer().Build())
• Protoreflect-based message walker handling singular, repeated, nested, and oneof fields
• Three resolution modes: name-only (auto-populate id), id-only (auto-populate name), both-provided (verify agreement)
• Tenant context extraction: local refs use owning resource's tenant/project; full refs support explicit project and shared tenant
• Fail-closed design: unregistered reference types return Internal error
• Prometheus metrics: osac_reference_validation_total (CounterVec) and osac_reference_validation_duration_seconds (HistogramVec)

Interceptor chain registration

• Added after JIT provisioning interceptor, before server handlers
• Both unary (validates) and stream (pass-through) chains

Test proto definitions

• proto/tests/osac/tests/v1/reference_test_types.proto with 7 message types and 3 request types covering all reference field patterns

Testing

• Unit tests: 45 Ginkgo tests covering builder, method filtering, field discovery (simple/nested/repeated/oneof), stream pass-through, reference type detection, lookup registration, tenant context extraction, error reporting/aggregation, non-proto pass-through, and Prometheus metrics
• Integration tests: N/A (interceptor is inert until lookup functions are registered in subsequent epics)
• Coverage: 84.6% — all public API behavioral contracts tested

Acceptance Criteria

• AC-1: gRPC unary server interceptor validates reference-typed fields in Create and Update requests
• AC-2: Detects reference fields by proto message naming convention via protoreflect
• AC-3: Invalid references return InvalidArgument with BadRequest containing FieldViolation entries with field paths
• AC-4: Aggregates all reference errors in a single response (no short-circuit)
• AC-5: Local references use owning resource's tenant/project; full references use explicit tenant/project with fallback
• AC-6: Prometheus metrics emitted (osac_reference_validation_total, osac_reference_validation_duration_seconds)

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.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds a reference-validation gRPC interceptor for Create and Update requests. It recursively resolves protobuf references, fills missing identifiers, reports invalid references, records metrics, wires into server startup, and adds schemas and comprehensive tests.

Changes

Reference validation

Layer / File(s)	Summary
Reference contracts and validator construction proto/tests/osac/tests/v1/reference_test_types.proto, internal/references/reference_validator.go, go.mod	Adds reference test messages, lookup contracts, validator construction, registration, and Prometheus collector setup.
Reference discovery and resolution internal/references/reference_validator.go	Walks nested, repeated, and oneof protobuf fields; resolves references using tenant/project context, mutates missing identifiers, and returns gRPC violations or internal errors.
gRPC integration and validation coverage internal/cmd/service/start/grpcserver/start_grpc_server_cmd.go, internal/references/*_test.go	Creates the validator during startup, adds unary and stream interceptors, and tests traversal, lookup behavior, error handling, metrics, and pass-through cases.

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant GRPCServer
  participant ReferenceValidator
  participant ReferenceLookup
  Client->>GRPCServer: Create or Update request
  GRPCServer->>ReferenceValidator: Invoke interceptor
  ReferenceValidator->>ReferenceLookup: Resolve reference
  ReferenceLookup-->>ReferenceValidator: Resolved reference or error
  ReferenceValidator-->>GRPCServer: Continue or return validation error
  GRPCServer-->>Client: RPC response

Loading

Possibly related PRs

• osac-project/fulfillment-service#782: Both changes add middleware to the gRPC interceptor chain in start_grpc_server_cmd.go.

Suggested labels: ok-to-test

Suggested reviewers: tzvatot, danmanor, rccrdpccl

🚥 Pre-merge checks | ✅ 11

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: adding a reference validation interceptor.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
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-Hardcoded-Secrets	✅ Passed	No hardcoded secrets found in the changed files; only empty strings, test fixtures, and normal config/pin hashes appeared.
No-Weak-Crypto	✅ Passed	Touched files contain no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, no custom crypto, and no secret/token comparisons; new code is reflection/metrics only.
No-Injection-Vectors	✅ Passed	No flagged sinks found in the new reference validator, interceptor wiring, or tests; the only interpolation is %q-quoted into an internal CEL filter, not an execution sink.
Container-Privileges	✅ Passed	No privileged/container escalation flags were found in the PR diff; the changed YAML is a GitHub Actions workflow without such settings.
No-Sensitive-Data-In-Logs	✅ Passed	Sensitive values use !-prefixed keys, and the repo logger redacts ! fields by default; no raw secrets/PII logs found in the new tests.
Ai-Attribution	✅ Passed	PASS: The PR's own commits that mention AI assistants use Assisted-by trailers; no AI-related Co-Authored-By appears in the first-parent change series.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 13

🤖 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 `@internal/cmd/service/start/grpcserver/start_grpc_server_cmd.go`:
- Line 519: Remove referenceValidator.StreamServer from the stream interceptor
chain in the gRPC server startup configuration. Preserve the remaining
interceptors and registration behavior, since StreamServer is unconditionally
pass-through.
- Around line 434-443: Before wiring referenceValidator into the gRPC
interceptor chain, either disable its registration by default or populate it
with lookup registrations for every reference type handled by Create and Update
requests. Update the startup flow around NewReferenceValidator().Build() and the
interceptor registration so valid reference-bearing requests reach their
handlers without “no lookup registered” errors.

In `@internal/references/reference_validator_test.go`:
- Around line 987-1030: Extend the reference validator specs around the existing
id/name mismatch case to cover a lookup returning the supplied ID and name,
asserting the handler executes and increments a valid counter. Also add cases
for map-nested references, a lookup returning (nil, nil), and references with
neither ID nor name, using the expected validation outcomes for each.
- Around line 1140-1145: Update the UnaryServer calls in the metric
specifications around validator.UnaryServer to capture and assert the returned
gRPC status code, rather than discarding the error. Keep the existing counter
assertions, and verify the status matches the expected failure reason so
unrelated counter increments cannot pass the tests.
- Around line 200-427: Replace the “Field discovery” tests’ direct
discoverReferenceFields calls with end-to-end resolveAndMutate execution using
recording lookups registered for each reference type. Assert the captured
fullName and fieldPath tuples, including nested, repeated, and oneof paths with
the production “[i]” formatting, and remove reliance on the duplicate
collectRefs traversal; apply the same change to the corresponding later test
block.

In `@internal/references/reference_validator.go`:
- Around line 304-310: Update the warning logs in the reference-not-found paths,
including the flow around logger.WarnContext, to mark customer-scoped values for
redaction by prefixing the sensitive keys with “!”: identifier, tenant, and
project here, plus id and name in the corresponding log at the other referenced
location. Leave field_path and reference_type unchanged.
- Around line 291-295: Update the reference lookup flow around lookupFunc and
recordDuration to bound the number of references processed per request, execute
lookups concurrently using an errgroup with a small concurrency limit, and apply
a context.WithTimeout to each lookup. Preserve request cancellation and timeout
propagation through context.Context, while retaining duration recording for
every lookup.
- Around line 401-429: The reference lookup flow in
internal/references/reference_validator.go#L401-L429 must derive tenant and
project from the authenticated identity in ctx, not object metadata, and reject
payload metadata values that disagree; update the surrounding validation symbols
accordingly. In internal/references/reference_validator.go#L385-L388, authorize
the client-supplied shared flag before switching tenants and replace the literal
"shared" with the existing or newly defined named constant.
- Around line 285-292: Update the reference validation flow around lookupFunc to
detect when both id and name are empty before invoking the lookup. In that
present-but-blank case, short-circuit and record/return the same violation
result used for invalid references; preserve the existing lookup behavior when
either identifier is provided.
- Around line 477-488: Update registerOrReuse so the AlreadyRegisteredError path
uses a comma-ok type assertion for registered.ExistingCollector instead of
panicking; return the existing collector only when it matches C, and otherwise
return the zero C value with an appropriate error.
- Around line 292-329: Validate resolved immediately after a successful lookup
in the reference-validation flow before accessing resolved.ID or resolved.Name.
Treat a nil result as an internal lookup failure, log it through the existing
“Reference lookup failed” path with field_path and reference_type context,
record the result as "error", and return the established Internal gRPC status
instead of dereferencing it.
- Around line 199-204: Sort the collected validation violations by their Field
value before serializing the Bad Request detail and joining descriptions. Update
the validate() flow or the serialization step around walkMessage so
protoreflect.Message.Range traversal order cannot affect FieldViolations order
or the resulting error message.
- Around line 206-208: Update the map-field branch in the reference validation
method to avoid silently accepting map fields: either recursively validate each
map value via Map().Range, or explicitly return an Internal error or warning
that map references are unsupported. Preserve the validator’s fail-closed
behavior and avoid shadowing the ranged value variable.

🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro Plus

Run ID: b9eb8fa2-1fc6-4924-98ab-a91c8d7f35bf

📥 Commits

Reviewing files that changed from the base of the PR and between d869692 and 1cb7c24.

⛔ Files ignored due to path filters (2)

• internal/api/osac/tests/v1/reference_test_types.pb.go is excluded by !**/*.pb.go
• internal/api/osac/tests/v1/reference_test_types_protoopaque.pb.go is excluded by !**/*.pb.go

📒 Files selected for processing (6)

• go.mod
• internal/cmd/service/start/grpcserver/start_grpc_server_cmd.go
• internal/references/reference_validator.go
• internal/references/reference_validator_test.go
• internal/references/references_suite_test.go
• proto/tests/osac/tests/v1/reference_test_types.proto

[coderabbitai]

♻️ Duplicate comments (1)

internal/references/reference_validator.go (1)

199-217: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Map-valued reference fields are still skipped — only a warning was added, not validation.

The fix adds a log line but the underlying gap flagged previously remains: message-valued map entries (e.g., map<string, FooReference>) are never walked, so references nested in maps still bypass validation entirely and can't be auto-populated or rejected. This is inconsistent with the interceptor's fail-closed design for unregistered types elsewhere. Consider actually walking val.Map().Range(...) for message-valued maps (recursing into resolveAndMutate/walkMessage as appropriate), not just logging the gap.

🤖 Prompt for 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.

In `@internal/references/reference_validator.go` around lines 199 - 217, Update
ReferenceValidator.walkMessage to validate message-valued map entries instead of
returning after the warning. Iterate val.Map().Range, recurse through the
existing resolveAndMutate/walkMessage flow for each message value, propagate any
validation error through internalErr, and preserve the current behavior for
non-message map values.

🤖 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.

Duplicate comments:
In `@internal/references/reference_validator.go`:
- Around line 199-217: Update ReferenceValidator.walkMessage to validate
message-valued map entries instead of returning after the warning. Iterate
val.Map().Range, recurse through the existing resolveAndMutate/walkMessage flow
for each message value, propagate any validation error through internalErr, and
preserve the current behavior for non-message map values.

---

ℹ️ Review info

⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a9566091-57b3-4e14-b516-8444d7cf6d19

📥 Commits

Reviewing files that changed from the base of the PR and between 1cb7c24 and 40acd5f.

📒 Files selected for processing (2)

• internal/references/reference_validator.go
• internal/references/reference_validator_test.go

[danielerez]

Looks good, just a couple of comments.

[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 `@internal/references/reference_validator.go`:
- Around line 134-144: Harden the registry access used by
ReferenceValidator.Register and resolveAndMutate so sealing cannot race with map
reads or writes. Prefer making the registry immutable before UnaryServer seals
it, or protect publication and access with a synchronized/atomic replacement
mechanism; retain the late-Register rejection while ensuring no concurrent map
access occurs.
- Around line 294-301: Update the field validation in the reference validation
flow around idField and nameField to also require both fields to use the string
kind, not merely be present. Return the existing Internal error response with a
clear required-string-fields message when either kind is invalid, while
preserving the subsequent Get and Set behavior for valid reference types.

🪄 Autofix (Beta)

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: ASSERTIVE

Plan: Pro Plus

Run ID: 2ff4f329-97f7-4fb8-a7d4-58887e981d44

📥 Commits

Reviewing files that changed from the base of the PR and between e86714a and f94986b.

⛔ Files ignored due to path filters (2)

• internal/api/osac/tests/v1/reference_test_types.pb.go is excluded by !**/*.pb.go
• internal/api/osac/tests/v1/reference_test_types_protoopaque.pb.go is excluded by !**/*.pb.go

📒 Files selected for processing (6)

• go.mod
• internal/cmd/service/start/grpcserver/start_grpc_server_cmd.go
• internal/references/reference_validator.go
• internal/references/reference_validator_test.go
• internal/references/references_suite_test.go
• proto/tests/osac/tests/v1/reference_test_types.proto

[rccrdpccl]

/lgtm
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: htayrie-rh, rccrdpccl

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:

• OWNERS [rccrdpccl]

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