OSAC-3274: remove pvcRef and pvRef from Volume

[akshaynadkarni]

Summary

OSAC-2872: Remove pvcRef and pvRef from Volume proto and CRD.

A Volume represents storage provisioned on a backend array. It exists independently
of any PVC. The PV object in Kubernetes is the native link between a PVC and an OSAC
Volume (via volumeHandle). PVC/PV tracking belongs on a future VolumeAttachment
resource that models the ControllerPublishVolume lifecycle (OSAC-3278).

Removes: PVCReference and PVReference message types from proto, pvc_ref from
VolumeSpec, pvc_ref and pv_ref from VolumeStatus, and all corresponding CRD fields.
Regenerates all generated code and CRD manifests.

Why

Avishay Traeger flagged during EP review (enhancement-proposals #151) that embedding
PVC/PV references in the Volume resource conflates two distinct lifecycle stages:
provisioning (CreateVolume) and attachment (ControllerPublishVolume). The CSI spec
cleanly separates these, and the current controller implementation does not reference
these fields.

Testing

go build ./...   # fulfillment-service: passes
go build ./...   # osac-operator: passes
make lint        # 0 issues
make test        # all tests pass

Pre-merge notes

After this merges and a new BSR module version is tagged, osac-operator should bump
its buf.gen.yaml version and run buf generate. This is a cleanup (the generated client
will have stale GetPvcRef() methods that nothing calls), not a blocker.

Ticket

OSAC-2872

Signed-off-by: akshaynadkarni 25892229+akshaynadkarni@users.noreply.github.com
Assisted-by: Cursor/Claude

[openshift-ci-robot]

@akshaynadkarni: This pull request references OSAC-2872 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 feature to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Summary

OSAC-2872: Remove pvcRef and pvRef from Volume proto and CRD.

A Volume represents storage provisioned on a backend array. It exists independently
of any PVC. The PV object in Kubernetes is the native link between a PVC and an OSAC
Volume (via volumeHandle). PVC/PV tracking belongs on a future VolumeAttachment
resource that models the ControllerPublishVolume lifecycle (OSAC-3278).

Removes: PVCReference and PVReference message types from proto, pvc_ref from
VolumeSpec, pvc_ref and pv_ref from VolumeStatus, and all corresponding CRD fields.
Regenerates all generated code and CRD manifests.

Why

Avishay Traeger flagged during EP review (enhancement-proposals #151) that embedding
PVC/PV references in the Volume resource conflates two distinct lifecycle stages:
provisioning (CreateVolume) and attachment (ControllerPublishVolume). The CSI spec
cleanly separates these, and the current controller implementation does not reference
these fields.

Testing

go build ./...   # fulfillment-service: passes
go build ./...   # osac-operator: passes
make lint        # 0 issues
make test        # all tests pass

Pre-merge notes

After this merges and a new BSR module version is tagged, osac-operator should bump
its buf.gen.yaml version and run buf generate. This is a cleanup (the generated client
will have stale GetPvcRef() methods that nothing calls), not a blocker.

Ticket

OSAC-2872

Signed-off-by: akshaynadkarni 25892229+akshaynadkarni@users.noreply.github.com
Assisted-by: Cursor/Claude

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

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 [akshaynadkarni]
• osac-operator/OWNERS [akshaynadkarni]

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

[coderabbitai]

Warning

Review limit reached

@akshaynadkarni, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 42 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro Plus

Run ID: 6da905ed-d993-4158-95c8-e2ec8a139e55

📥 Commits

Reviewing files that changed from the base of the PR and between fa35b54 and c6aac34.

⛔ Files ignored due to path filters (2)

• fulfillment-service/internal/api/osac/private/v1/volume_type.pb.go is excluded by !**/*.pb.go
• fulfillment-service/internal/api/osac/private/v1/volume_type_protoopaque.pb.go is excluded by !**/*.pb.go

📒 Files selected for processing (7)

• fulfillment-service/internal/servers/private_volumes_server.go
• fulfillment-service/internal/servers/private_volumes_server_test.go
• fulfillment-service/proto/private/osac/private/v1/volume_type.proto
• osac-operator/api/v1alpha1/volume_types.go
• osac-operator/api/v1alpha1/zz_generated.deepcopy.go
• osac-operator/charts/operator-crds/templates/osac.openshift.io_volumes.yaml
• osac-operator/config/crd/bases/osac.openshift.io_volumes.yaml

---

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

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:39 PM UTC · Completed 4:56 PM UTC

Commit: fe1197b · View workflow run →

[openshift-ci-robot]

@akshaynadkarni: This pull request references OSAC-3274 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.1.0" version, but no target version was set.

Details

In response to this:

Summary

OSAC-2872: Remove pvcRef and pvRef from Volume proto and CRD.

A Volume represents storage provisioned on a backend array. It exists independently
of any PVC. The PV object in Kubernetes is the native link between a PVC and an OSAC
Volume (via volumeHandle). PVC/PV tracking belongs on a future VolumeAttachment
resource that models the ControllerPublishVolume lifecycle (OSAC-3278).

Removes: PVCReference and PVReference message types from proto, pvc_ref from
VolumeSpec, pvc_ref and pv_ref from VolumeStatus, and all corresponding CRD fields.
Regenerates all generated code and CRD manifests.

Why

Avishay Traeger flagged during EP review (enhancement-proposals #151) that embedding
PVC/PV references in the Volume resource conflates two distinct lifecycle stages:
provisioning (CreateVolume) and attachment (ControllerPublishVolume). The CSI spec
cleanly separates these, and the current controller implementation does not reference
these fields.

Testing

go build ./...   # fulfillment-service: passes
go build ./...   # osac-operator: passes
make lint        # 0 issues
make test        # all tests pass

Pre-merge notes

After this merges and a new BSR module version is tagged, osac-operator should bump
its buf.gen.yaml version and run buf generate. This is a cleanup (the generated client
will have stale GetPvcRef() methods that nothing calls), not a blocker.

Ticket

OSAC-2872

Signed-off-by: akshaynadkarni 25892229+akshaynadkarni@users.noreply.github.com
Assisted-by: Cursor/Claude

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.

[fullsend-ai-review]

Review

Findings

High

• [api-contract] fulfillment-service/proto/private/osac/private/v1/volume_type.proto — Removed proto field numbers (4 in VolumeSpec, 7 and 8 in VolumeStatus) are not marked as reserved. The codebase consistently reserves removed field numbers (see compute_instance_type.proto fields 5/6/12/13, virtual_network_type.proto field 5, compute_instance_template_type.proto fields 1/2, event_type.proto fields 16-18/25). Without reserved, a future contributor could reuse these field numbers with different types, causing silent data corruption for serialized messages.
  Remediation: Add reserved 4; reserved "pvc_ref"; to VolumeSpec and reserved 7, 8; reserved "pvc_ref", "pv_ref"; to VolumeStatus.

Medium

• [backward-incompatible] osac-operator/api/v1alpha1/volume_types.go — Removing pvcRef and pvRef fields from the Volume CRD schema is a backward-incompatible CRD change. Any Volume CRs already persisted in etcd with these fields will have them silently stripped by Kubernetes structural schema pruning on reads. The presence of a CEL validation rule pvcRef cannot be removed once set confirms the field was designed to be populated on some resources.
  Remediation: Confirm no deployed environment has Volume CRs with these fields populated, or document that data loss is acceptable.

Low

• [stale-reference] osac-operator/internal/api/osac/private/v1/volume_type.pb.go — The osac-operator's generated gRPC client code (from BSR v0.0.85) still contains PVCReference/PVReference types. The PR body acknowledges this as an expected follow-up. Controllers do not reference these fields.

• [stale-reference] osac-csi-driver/pkg/fulfillment/volume.go:35 — The CSI driver's CreateVolumeParams.PVCRef field references a concept the Volume API no longer supports. No immediate breakage (stubs in use). PVC/PV tracking is deferred to OSAC-3278 (VolumeAttachment).

---

Labels: PR modifies Volume resource types across the storage stack (proto, CRD, server)

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run

Review

Findings

High

• [stale-reference] fulfillment-service/internal/servers/private_volumes_server_test.go:144 — Test file still references removed proto types and fields. The tests "Creates a standalone volume without pvc_ref" (line 144, assertion at line 150) and "Creates a volume with pvc_ref" (lines 155–178) use PVCReference_builder, GetPvcRef(), and SetPvcRef(), all of which are removed from the generated code by this PR. This file is not in the PR diff. The fulfillment-service will fail to compile after this change.
  Remediation: Remove or rewrite the two test cases — delete the GetPvcRef() assertion from the standalone test and delete the "Creates a volume with pvc_ref" test entirely since the feature it tests no longer exists.

Medium

• [backward-incompatible] fulfillment-service/proto/private/osac/private/v1/volume_type.proto — Removing field numbers 4 (VolumeSpec.pvc_ref), 7 (VolumeStatus.pvc_ref), and 8 (VolumeStatus.pv_ref) without adding reserved directives risks future field-number reuse. If a new field is later assigned number 4, 7, or 8, existing serialized data in PostgreSQL could be silently misinterpreted. While this is a private API (limiting blast radius), protobuf best practice requires reserving removed field numbers.
  Remediation: Add reserved 4; to VolumeSpec, reserved 7, 8; to VolumeStatus, and reserved "pvc_ref", "pv_ref"; name reservations in both messages.

Low

• [stale-reference] osac-csi-driver/pkg/fulfillment/volume.go:35 — The CSI driver's CreateVolumeParams struct has a PVCRef field that maps to the now-removed proto concept. The controller (controller.go:86) populates it and stubs (stubs.go) use it for deduplication. No build or runtime break today (the CSI driver uses in-memory stubs), but the field's proto-level backing no longer exists.
  Remediation: Coordinate with CSI driver maintainers to remove PVCRef from CreateVolumeParams or document the planned replacement.

• [stale-reference] osac-operator/internal/api/osac/private/v1/volume_type.pb.go — The osac-operator's generated protobuf files still contain PVCReference and PVReference types, generated from pinned BSR module v0.0.85. No non-generated operator code references these types. The PR body correctly identifies this as a post-merge cleanup.
  Remediation: After merging and publishing a new BSR module version, update osac-operator/buf.gen.yaml and run buf generate.

• [backward-incompatible] osac-operator/api/v1alpha1/volume_types.go — Existing Volume CRs in clusters with pvcRef or pvRef set will have those fields pruned when the updated CRD is applied (Kubernetes structural schema pruning). No functional impact since no controller code reads these fields, but operators upgrading should be aware.
  Remediation: Document the CRD schema change in release notes to warn operators of the field removal.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Previous run (2)

Review

Findings

Critical

• [proto-backward-compatibility] fulfillment-service/proto/private/osac/private/v1/volume_type.proto — Removed proto field numbers (VolumeSpec field 4, VolumeStatus fields 7 and 8) without adding reserved declarations. This repo has an established convention of reserving removed field numbers and names (verified in compute_instance_type.proto, virtual_network_type.proto, event_type.proto, compute_instance_template_type.proto, baremetal_instance_type_type.proto, external_ip_pool_type.proto). Without reserved, a future developer could accidentally reuse field numbers 4, 7, or 8 with different types, causing silent data corruption when old serialized data is deserialized with the new schema.
  Remediation: Add reserved 4; reserved "pvc_ref"; in VolumeSpec, and reserved 7, 8; reserved "pvc_ref", "pv_ref"; in VolumeStatus.

High

• [stale-reference] fulfillment-service/internal/servers/private_volumes_server.go:220 — The update mask handler for "spec.pvc_ref" (lines 220–222) calls SetPvcRef() and GetPvcRef(), which no longer exist on VolumeSpec after the proto regeneration in this PR. This will cause a compile error.
  Remediation: Remove the case "spec.pvc_ref" block from the applyVolumeUpdate function.

• [stale-reference] fulfillment-service/internal/servers/private_volumes_server_test.go:150 — Test "Creates a standalone volume without pvc_ref" (line 150) calls GetSpec().GetPvcRef() which no longer exists. Test "Creates a volume with pvc_ref" (line 155) constructs PVCReference_builder and VolumeSpec_builder.PvcRef, both removed. These will cause compile errors.
  Remediation: Remove the GetPvcRef() assertion at line 150 and the entire test case "Creates a volume with pvc_ref" (lines 155–178).

Low

• [stale-reference] osac-operator/internal/api/osac/private/v1/volume_type.pb.go — The osac-operator's generated proto code still contains PVCReference, PVReference, and related PvcRef/PvRef fields. Acknowledged in PR body as post-merge cleanup. No compile error (separate module, proto3 handles gracefully).

• [stale-reference] osac-csi-driver/pkg/fulfillment/volume.go:35 — The CSI driver's CreateVolumeParams.PVCRef field is now dead data with no destination in the API. The code compiles but the data path is orphaned. Currently uses stubs only (real client not implemented), so no runtime impact.

• [crd-backward-compatibility] osac-operator/api/v1alpha1/volume_types.go — CRD schema removes spec.pvcRef, status.pvcRef, and status.pvRef. Existing Volume CRs with these fields populated would have data stripped on next update. Low risk for a recently-added resource under active development.

• [misleading-label] PR title references Jira key OSAC-3274 but the PR body, commit message, and branch name all reference OSAC-2872. Minor traceability concern.

---

Next steps:

• /fs-fix — agent addresses review findings automatically
• /fs-fix <your instruction> — agent fixes with your specific guidance
• Push commits directly — review re-runs automatically on push
• /fs-fix-stop — disable automatic fix runs for this PR

Auto-dismissed: only Prow labels gate merging

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:10 PM UTC · Completed 6:24 PM UTC

Commit: cb0f250 · View workflow run →

Auto-dismissed: only Prow labels gate merging

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:23 PM UTC · Completed 7:39 PM UTC

Commit: c6aac34 · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

Auto-dismissed: only Prow labels gate merging