Repository navigation
OSAC-3530: Merge osac-csi-driver into osac mono-repo - #75
Conversation
Scaffold the project infrastructure for the OSAC CSI meta-driver: - go.mod with Go 1.26.3 and CSI spec, gRPC, klog dependencies - Makefile with build, test, lint, image targets (podman, UBI10) - Containerfile for multi-stage container build - golangci-lint v2.12.1 configuration matching osac-operator - .gitignore and pre-commit hooks with golangci-lint Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Define the Client interface that the CSI driver uses to communicate with the OSAC fulfillment service's Volume API. The Resolve method determines which vendor backend handles a volume request for a given tenant and tier. Includes a LoggingStub implementation for development before the real gRPC client is wired to the fulfillment-service. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Migrate the CSI meta-driver code from the PoC, replacing the stub HTTP storage API with the fulfillment client interface: - Controller plugin resolves storage tiers via fulfillment.Client.Resolve instead of the storageapi HTTP stub - Removed CheckPolicy call (policy enforcement moves to fulfillment-service) - Tenant extracted from CSI request parameters instead of hardcoded - Deterministic default vendor socket selection (sorted map keys) - Removed dead proxyError() function - Version injected via ldflags instead of hardcoded constant - Node plugin and proxy manager carried over with minimal changes Not migrated (per design): cmd/osac-storage-api, pkg/storageapi, deploy/ manifests, Dockerfile.storage-api, Dockerfile.pure-node. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Replace the template boilerplate with OSAC CSI driver documentation covering architecture (controller/node plugin modes), build commands, and CLI flag reference. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
The local golangci-lint pre-commit hook uses language:system which requires the binary at bin/golangci-lint. In CI this binary is not available, so follow the osac-operator pattern: - Add .pre-commit-config-ci.yaml without the golangci-lint hook - Run golangci-lint as a separate CI job using the official action - Update checkout action to v7, add paths-ignore for docs Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
- Pin google.golang.org/protobuf to stable v1.36.11 instead of pseudo-version - Track volume-to-backend mapping in NodeServer so NodeUnstageVolume and NodeUnpublishVolume route to the correct vendor instead of blindly using the default socket - Extract isUnimplemented helper to reduce duplication Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Vendor resolution must be explicit, no surprises. Removed the defaultVendorSocket fallback that silently routed to an arbitrary vendor when the backend was unknown. Controller: DeleteVolume and ControllerUnpublishVolume now read osac.backend from the secrets map. ValidateVolumeCapabilities reads it from volume context. ListVolumes removed (cannot determine backend without volume context per-volume). Node: resolveVendorSocket fails with InvalidArgument when osac.backend is missing from volume context. lookupBackendSocket fails with FailedPrecondition when no backend was recorded for the volume. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Signed-off-by: Roy Golan <rgolan@redhat.com>
Restrict GITHUB_TOKEN to read-only contents access. SHA pinning skipped to stay consistent with other osac-project repos. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
NO-ISSUE: Migrate CSI meta-driver from PoC with fulfillment client interface
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6 to 7. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v6...v7) --- updated-dependencies: - dependency-name: actions/setup-python dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 6 to 7. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v6...v7) --- updated-dependencies: - dependency-name: actions/setup-go dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Helm chart for the OSAC CSI meta-driver deployment: - Controller Deployment with csi-provisioner and csi-attacher sidecars - Node DaemonSet with csi-node-driver-registrar - CSIDriver resource (csi.osac.openshift.io) - RBAC (ServiceAccounts, ClusterRoles, ClusterRoleBindings) - Configurable image refs, resources, vendor sockets, leader election Also excludes Helm templates from yamllint and adds a helm-lint pre-commit hook. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Deploys vendor CSI controller pods (Trident, VAST, Pure) as separate Deployments + Services in a dedicated namespace. Each vendor is conditionally enabled via values (disabled by default). Includes per-vendor: ServiceAccount, Deployment, Service, and for Pure: ClusterRole + ClusterRoleBinding. Credentials are referenced as existing Secrets, not created by the chart. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
CSI driver containers need root to bind unix sockets on hostPath volumes. Add runAsUser: 0 to securityContext on both controller and node containers (complements existing privileged: true). Also add -buildvcs=false to Containerfile to fix build when .git is not available in the container context. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Add imagePullSecrets to all vendor controller pod specs so that private registry credentials can be provided via values. Fix default Trident image tag from 25.02.0 (does not exist) to 25.10.0. Available tags: 25.10.0, 26.02.0, latest. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Document the secrets and configuration each vendor requires before chart installation: Trident TLS certs, VAST credentials, Pure config, and registry pull secrets. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Pure CSI controller needs the Kubernetes API to function. Enable automountServiceAccountToken (was disabled from PoC workaround). Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
- Add coordination.k8s.io/leases RBAC for leader election (csi-provisioner/attacher) - Remove unnecessary hostPID and hostNetwork from node DaemonSet - Harden controller containers: drop privileged, add restricted securityContext - Add securityContext to all sidecar containers (provisioner, attacher, registrar) - Change VAST verifySsl default to true for TLS verification - Change image tag default from latest to 0.0.0 with pullPolicy Always - Change backends namespace PSA from privileged to baseline (warn: restricted) - Scope Pure Secrets RBAC from ClusterRole to namespaced Role (least privilege) - Add Trident CRD RBAC (ClusterRole/Binding) for --crd_persistence Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Deploy separate DaemonSets for Trident, VAST, and Pure node plugins alongside the OSAC meta-driver. Each vendor node plugin: - Writes its CSI socket to a well-known hostPath - Runs privileged for mount/device operations - Is conditionally enabled via vendors.<name>.enabled - Reuses the csi-driver node ServiceAccount Also fixes runAsNonRoot on sidecar containers (csi-provisioner and csi-attacher run as root) and reverts backends namespace PSA to privileged since vendor controllers may need it. Tested on kind: controller 3/3 Running, all 4 node DaemonSets created. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
…_actions/actions/setup-python-7 NO-ISSUE: Bump actions/setup-python from 6 to 7
…_actions/actions/setup-go-7 NO-ISSUE: Bump actions/setup-go from 6 to 7
OSAC-3052: Add Helm charts for CSI driver and vendor backends
- Add namespace.yaml template so the chart owns its namespace with pod-security privileged labels - Switch all templates from Release.Namespace to Values.namespace for uniform deployment - Remove hardcoded securityContext from controller sidecars - Use templated privileged flag from Values for all node containers - Default image tag to 'main' (placeholder, overwritten at release time) Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Build and push container images to ghcr.io on merge to main (tagged main + sha-<commit>) and on v* tags (semver tags). Build-only on PRs to validate the Containerfile. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
OSAC-3052: add publish-image GitHub Actions workflow
The Containerfile sets USER 1001 but the CSI node plugin needs root to bind unix sockets on hostPath volumes under /var/lib/kubelet/plugins/. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
Restore allowPrivilegeEscalation: false, drop ALL capabilities, and readOnlyRootFilesystem on the controller containers (osac-csi-driver, csi-provisioner, csi-attacher) and node-driver-registrar. These containers don't need privileges — only the node driver plugin does. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Roy Golan <rgolan@redhat.com>
# Conflicts: # .github/workflows/helm-lint.yaml
|
Triaged all 33 CodeRabbit findings on this PR against the actual diff (not just file location) to separate what this merge's own new content introduced from what's pre-existing in osac-csi-driver's carried-over driver/chart/test code (this is a full-history 5 findings are caused by this merge's own new content (the wiring commit's workflow/pre-commit/README changes, not the subtree-added code) -- fixes ready to push to this branch, coordinating with the PR author first since some may already be in progress:
The remaining 28 findings are real, but in osac-csi-driver's own pre-existing code (Helm charts, Containerfile, Go driver logic, sanity tests) that this PR carries over unchanged from the standalone repo's history -- not something a migration PR should silently rewrite out from under the component's actual owners (zszabo-rh, wgordon17, avishayt, rgolangh per OWNERS). Filed for follow-up, not blocking this merge:
Neither ticket blocks this PR -- they're pre-existing conditions in code that already exists and runs today in the standalone repo, unaffected by whether this merge lands. |
…r wiring Addresses the 5 of 33 CodeRabbit findings on this PR that are caused by this merge's own new content, as opposed to pre-existing osac-csi-driver driver/chart/test code carried over verbatim by the subtree merge (those are tracked separately as OSAC-3532/OSAC-3533, not fixed here -- see PR comment). - publish-charts.yaml: the new csi-driver values.yaml tag-placeholder sed step had no verification it actually matched, unlike the sibling publish-osac-aap-chart job's replace_and_verify pattern. Applied the same verify-after-sed guard. - publish-csi-driver-image.yaml: excluded osac-csi-driver/charts/** from the image-rebuild trigger (chart-only changes are already covered by helm-lint.yaml's own chart path filter), and SHA-pinned the 4 actions this new workflow uses, reusing the exact SHA+comment pairs fulfillment-service's own publish-image.yaml already has for the identical action+version combos. - .pre-commit-config.yaml: the new osac-csi-driver-golangci-lint entry called bin/golangci-lint directly, which doesn't exist on a clean checkout. Changed to `make -C osac-csi-driver lint-fix` so the Makefile installs the pinned tool first. - README.md: the go.work workspace paragraph was missing osac-csi-driver (this PR's addition) and bare-metal-fulfillment-operator (a pre-existing gap from an earlier merge, fixed in the same pass since it's the same sentence). Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elior Erez <eerez@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/publish-csi-driver-image.yaml (2)
94-104: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift.github/workflows/publish-csi-driver-image.yaml#L94-L104, .github/workflows/publish-charts.yaml#L604-L615, .github/workflows/publish-charts.yaml#L617-L668
Sign every published OCI artifact.
These release paths push images or charts but do not create Sigstore/cosign signatures. Consumers cannot verify artifact provenance. Add keyless signing after each push, sign the immutable digest, and grant each signing job
id-token: write.
.github/workflows/publish-csi-driver-image.yaml#L94-L104: Sign the pushed image digest afterdocker/build-push-action..github/workflows/publish-charts.yaml#L604-L615: Sign the pushedcsi-driverchart digest..github/workflows/publish-charts.yaml#L617-L668: Sign the pushedcsi-backendschart digest.As per path instructions, “Sign artifacts with Sigstore/cosign.”
🤖 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 @.github/workflows/publish-csi-driver-image.yaml around lines 94 - 104, Sign every published OCI artifact with keyless Sigstore/cosign after its push, using the immutable digest and granting the relevant jobs id-token: write. Update .github/workflows/publish-csi-driver-image.yaml lines 94-104 to sign the digest output from docker/build-push-action; update .github/workflows/publish-charts.yaml lines 604-615 to sign the csi-driver chart digest; and update lines 617-668 to sign the csi-backends chart digest.Source: Path instructions
67-73: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not publish prereleases under stable aliases.
Lines 70-73 derive
v1.2andv1aliases after accepting prerelease versions. Lines 87-88 publish those aliases wheneverRELEASE_VERSIONexists. A tag such asosac-csi-driver/v1.2.3-rc.1can overwritev1.2andv1with a prerelease image.Set major and minor aliases only when
versionhas no prerelease suffix. Enable those metadata entries only when their alias variables are set.Also applies to: 85-88
🤖 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 @.github/workflows/publish-csi-driver-image.yaml around lines 67 - 73, Update the release metadata logic around RELEASE_MAJOR_MINOR and RELEASE_MAJOR so prerelease versions do not populate stable alias variables; only set these aliases when version has no prerelease suffix. In the publish steps, conditionally enable the major/minor metadata entries only when their corresponding alias variables are set, while continuing to publish RELEASE_VERSION for prereleases.
🤖 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 @.github/workflows/helm-lint.yaml:
- Around line 116-123: Add a job-level permissions block to helm-lint-installer
granting only contents: read, keeping the existing checkout and Helm validation
steps unchanged.
---
Outside diff comments:
In @.github/workflows/publish-csi-driver-image.yaml:
- Around line 94-104: Sign every published OCI artifact with keyless
Sigstore/cosign after its push, using the immutable digest and granting the
relevant jobs id-token: write. Update
.github/workflows/publish-csi-driver-image.yaml lines 94-104 to sign the digest
output from docker/build-push-action; update
.github/workflows/publish-charts.yaml lines 604-615 to sign the csi-driver chart
digest; and update lines 617-668 to sign the csi-backends chart digest.
- Around line 67-73: Update the release metadata logic around
RELEASE_MAJOR_MINOR and RELEASE_MAJOR so prerelease versions do not populate
stable alias variables; only set these aliases when version has no prerelease
suffix. In the publish steps, conditionally enable the major/minor metadata
entries only when their corresponding alias variables are set, while continuing
to publish RELEASE_VERSION for prereleases.
🪄 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: 40e75231-fc50-49f9-af6a-c334b8bc1fb3
📒 Files selected for processing (5)
.github/workflows/helm-lint.yaml.github/workflows/publish-charts.yaml.github/workflows/publish-csi-driver-image.yaml.pre-commit-config.yamlREADME.md
| helm-lint-installer: | ||
| name: Lint Helm charts (osac-installer) | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v6 | ||
| with: | ||
| submodules: recursive | ||
| persist-credentials: false |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Restrict the installer lint job token.
This job inherits repository-default GITHUB_TOKEN permissions. It only checks out repository contents and runs local Helm validation. Set permissions: contents: read.
Proposed fix
helm-lint-installer:
name: Lint Helm charts (osac-installer)
runs-on: ubuntu-latest
+ permissions:
+ contents: read
steps:As per path instructions, “Least privilege: minimize GITHUB_TOKEN permissions.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| helm-lint-installer: | |
| name: Lint Helm charts (osac-installer) | |
| runs-on: ubuntu-latest | |
| steps: | |
| - name: Checkout repository | |
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v6 | |
| with: | |
| submodules: recursive | |
| persist-credentials: false | |
| helm-lint-installer: | |
| name: Lint Helm charts (osac-installer) | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: read | |
| steps: | |
| - name: Checkout repository | |
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v6 | |
| with: | |
| persist-credentials: false |
🤖 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 @.github/workflows/helm-lint.yaml around lines 116 - 123, Add a job-level
permissions block to helm-lint-installer granting only contents: read, keeping
the existing checkout and Helm validation steps unchanged.
Source: Path instructions
…liases CodeRabbit follow-up on the previous push: RELEASE_MAJOR_MINOR/RELEASE_MAJOR were derived from the prerelease-stripped core version but gated only on RELEASE_VERSION being non-empty, so a prerelease tag (e.g. osac-csi-driver/v1.2.3-rc.1) would still publish/overwrite the v1.2 and v1 stable alias tags with a prerelease image. Confirmed real: only skip setting these two env vars when the tag has no prerelease suffix, and gate their metadata-action entries on their own value instead of RELEASE_VERSION's. Assisted-by: Claude Code <noreply@anthropic.com> Signed-off-by: Elior Erez <eerez@redhat.com>
|
Follow-up review pass on CodeRabbit's post-push findings (submitted 21:46:53, after my earlier fixes). Verified each against current code rather than applying diffs blindly, per the same discipline as before. Fixed: the prerelease/stable-alias overwrite bug in Rejected as a false positive: the Deferred, not fixed here: the missing Sigstore/cosign signing on published images/charts. Checked first, per instruction, whether this is unique to csi-driver -- it isn't: grepped every workflow in the repo for Pushed the one real fix. Will keep watching this PR for further CodeRabbit rounds. |
…mp-aap-submodule NO_ISSUE: bump osac-aap submodule to latest
…ue-solver/OSAC-3049 OSAC-3049: E2E gate jobs: bare exit 1 with no diagnostic output
…tials OSAC-1665: add download kubeconfig and view password to cluster details
Summary
Merges osac-csi-driver into the mono-repo, following the same pattern established for fulfillment-service, osac-operator, osac-aap, and bare-metal-fulfillment-operator.
git subtree add --prefix=osac-csi-driver(full history preserved, no--squash) — verified the merge commit's second parent has all 34 commits from the standalone repo, cross-checked against the live repo's commit count via the GitHub API.mainbranch locked (branch protection API + durable Terraform change ingithub-config, so it survives the next auto-apply)..pre-commit-config.yaml/.pre-commit-config-ci.yaml/.yamllint.yaml/dependabot.ymlinto root's, dropped the redundant per-component copies (and the stale "Innabox"-eraLICENSE), added anosac-csi-drivermatrix entry tohelm-lint.yaml, mergedOWNERSand.gitignoreentries, added a README bullet.publish-image.yaml's tag trigger toosac-csi-driver/v*and hardcodedIMAGE_NAMEup front, to avoid the OSAC-3467/OSAC-3490 image-tag collision that hit the earlier merges. Renamed the workflow toBuild osac-csi-driver image(its default name would've collided with fulfillment-service'sContainer imageworkflow, whichpublish-charts.yamldisambiguates on).publish-charts.yamlwiring (guard job +publish-csi-driver-chart/publish-csi-backends-chart/release jobs), matching the shared per-component pattern.osac-installer/charts/osac/Chart.yaml'scsi-driver/csi-backendsdependencies from the oldbase/osac-csi-driversubmodule path to the mono-repo-localosac-csi-driver/charts/{csi-driver,csi-backends}paths, and removed the now-unused submodule from.gitmodules. Verified with a realhelm dependency build+helm template(126 rendered manifests, including the csiDriver/csiBackends resources).osac-csi-driverto the rootgo.workworkspace — required, since Go's workspace auto-discovery otherwise breaksgo build/go vet/golangci-lintfor any module living under ago.workroot that isn't listed in it. Verifiedgo build/go vetsucceed from withinosac-csi-driverafterward.pre-commitandContainer imageCI workflows post-merge (its branch is also lock-protected).Test plan
git subtree addhistory verified (commit count + merge-commit parentage)pre-commit runclean on all changed files (yamllint, whitespace/EOF checks, osac-csi-driver golangci-lint with binary pre-downloaded)helm lint/helm templateclean for bothcharts/csi-driverandcharts/csi-backends, standalone and via the umbrellaosac-installer/charts/osacchartgo build ./.../go vet ./...clean from withinosac-csi-driver, withosac-operator/bare-metal-fulfillment-operatorbuilds unaffectedpublish-csi-driver-image.yamlandpublish-charts.yamlYAML-validated; workflow-name collision with fulfillment-service'sContainer imageavoidedosac-csi-driver/v0.0.1-style) to confirm the end-to-end image-build → chart-publish → release chain, once this merges🤖 Generated with Claude Code
Summary by CodeRabbit