STOR-2923: Rebase to upstream v1.34.4 for OCP 4.23/5.0#149
Open
rvagner78 wants to merge 76 commits into
Open
Conversation
…er-6.2.0-1.34 [release-1.34] chore: upgrade to csi-provisioner v6.2.0
…roving concurrency
…ot/cherry-pick-3526-to-release-1.34 [release-1.34] fix: trigger migration within few mins by batching the volumes or imp…
…ot/cherry-pick-3537-to-release-1.34 [release-1.34] test: fix CVE-2026-25679 in trivy action
…clude nodeName parameter Modified the DetachDisk function to pass the nodeName to waitForDiskManagedByTobeRemoved. Updated the waitForDiskManagedByTobeRemoved function to handle the nodeName, allowing for better management of disk detachment scenarios, particularly when instances are not found. Added mock expectations in the test to verify behavior when managedBy is set.
…1.34 [release-1.34] fix: CVE-2026-33186
…ot/cherry-pick-3543-to-release-1.34 [release-1.34] fix: incorrect node attached on waitingForDetached call
- Add 557 new VM size entries - Update 16 existing VM size max disk count values - No entries removed from the original list The VM size data is based on the latest list from: https://github.com/andyzhangx/demo/blob/master/linux/azuredisk/vmsizelist.final
…ot/cherry-pick-3553-to-release-1.34 [release-1.34] feat: update maxDataDiskCountMap with latest VM size list
…ot/cherry-pick-3546-to-release-1.34 [release-1.34] chore: add flag wait-for-detach-disk-complete
Updates aquasecurity/trivy-action from mutable references to SHA-pinned version to address security vulnerabilities. - Updates to v0.35.0 (57a97c7e) - Pins to specific SHA for immutability - Addresses issue: aquasecurity/trivy#10425 Signed-off-by: Priyanka Saggu <priyankasaggu11929@gmail.com>
…ot/cherry-pick-3557-to-release-1.34 [release-1.34] security: Update trivy-action to use sha for v0.35.0
…ot/cherry-pick-3565-to-release-1.34 [release-1.34] fix: increase checkDiskLun throttle threshold from 1s to 10s
…ot/cherry-pick-3563-to-release-1.34 [release-1.34] fix: skip ManagedBy polling after successful VMSS detach
- Make --http-endpoint bind to localhost for hostNetwork=true, bind to 0.0.0.0 for hostNetwork=false (kubelet probes via Pod IP) - Make livenessProbe host: localhost conditional on hostNetwork - Add comment in static deploy manifest - Update chart tgz
…ot/cherry-pick-3575-to-release-1.34 [release-1.34] test: fix trivy action failure
[release-1.34] fix: CVE-2026-39883
…tead of fmt.Sprintf - Revert deploy/csi-azuredisk-node.yaml port name change (unrelated to NVMe fix) - Replace fmt.Sprintf with plain string literals where no format params are used Signed-off-by: Andy Zhang <andyzhangx@live.com>
Signed-off-by: Andy Zhang <andyzhangx@live.com>
Extract repeated NVMe disk detection + Location construction into a single getNVMeLocation() function that returns (*Location, error). This eliminates duplicated isNVMeDisk + getNVMeLunFromPath + Location literal blocks across all three disk enumeration functions. Signed-off-by: Andy Zhang <andyzhangx@live.com>
Signed-off-by: Andy Zhang <andyzhangx@live.com>
Signed-off-by: Andy Zhang <andyzhangx@live.com>
getNVMeLocation now accepts diskNum, handles all klog internally, and returns (*Location, bool) where bool indicates whether the path was NVMe. Callers reduce to a simple if/continue pattern. Signed-off-by: Andy Zhang <andyzhangx@live.com>
…ot/cherry-pick-3632-to-release-1.34 [release-1.34] fix: wrong NVMe disk LUN mapping on Windows D4ads_v7 SKU VM
Add 141 new VM SKUs while preserving all existing entries.
…ot/cherry-pick-3637-to-release-1.34 [release-1.34] chore: update max disk count map with latest VM sizes
Bumps build-image/debian-base from bookworm-v1.0.7 to bookworm-v1.0.8.
…ot/cherry-pick-3640-to-release-1.34 [release-1.34] chore(deps): bump build-image/debian-base from bookworm-v1.0.7 to bookworm-v1.0.8 in /pkg/azurediskplugin
doc: cut v1.34.4 release
Remove .github files, add .snyk file, update OWNERS, add Dockerfile.openshift.rhel7 and .ci-operator.yaml. Additional changes: Update ose-azure-disk-csi-driver-container image to be consistent with ART for 4.22.
openshift-ci-robot
added
the
jira/valid-reference
|
@rvagner78: This pull request references STOR-2923 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 story to target only the "5.0.0" version, but multiple target versions were set. DetailsIn response to this:
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. |
WalkthroughVersion bump to v1.34.4, Helm chart and manifests updated, new controller/node flags (kube API QPS/burst, wait-for-detach), Windows NVMe disk location handling added with tests, migration batch script introduced, CRDs/RBAC/templates added, docs and go.mod refreshed. Changesv1.34.4 Release and Platform Updates
Sequence Diagram(s)(skip) Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rvagner78 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (1)
charts/v1.34.4/azuredisk-csi-driver/templates/rbac-csi-azuredisk-controller.yaml (1)
66-68: ⚡ Quick winRBAC concern: legacy
csinodeinfosis redundant (chart already grantscsinodes)This template already grants read access to
storage.k8s.io/csinodesearlier in the same controller RBAC, so the attacher shouldn’t be missing node info permissions. The remaining entry at lines 66-68 forcsi.storage.k8s.io/csinodeinfosis legacy and likely unnecessary on modern clusters; consider removing it or gating it for older compatibility.🤖 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 `@charts/v1.34.4/azuredisk-csi-driver/templates/rbac-csi-azuredisk-controller.yaml` around lines 66 - 68, The RBAC rule granting apiGroups "csi.storage.k8s.io" resources "csinodeinfos" is redundant because the chart already grants "storage.k8s.io/csinodes"; remove the "csinodeinfos" rule from the controller RBAC block (resource "csinodeinfos") or gate it for older clusters by wrapping it in a Helm conditional that checks API availability (e.g. using .Capabilities.APIVersions.Has) so the "csinodeinfos" entry is only rendered when the cluster requires that legacy API.
🤖 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 `@charts/v1.34.3/azuredisk-csi-driver/templates/csi-azuredisk-controller.yaml`:
- Around line 42-47: The template is incorrectly gating rendering of
user-provided affinity on a substring check for "nodeSelectorTerms" which can
drop valid podAffinity/podAntiAffinity settings; update the logic around
.Values.controller.affinity so that if .Values.controller.affinity is set it is
rendered directly (use tpl/with to evaluate and then emit affinity: followed by
toYaml . | indent 8) instead of using contains "nodeSelectorTerms" — remove the
contains check and the else branch that skips affinity, and ensure the template
uses the existing tpl "{{ .Values.controller.affinity }}" . and toYaml to output
any affinity object provided.
In `@charts/v1.34.3/azuredisk-csi-driver/templates/csi-azuredisk-node.yaml`:
- Around line 38-39: The DaemonSet uses two different Helm values for
hostNetwork which can produce inconsistent manifests: hostNetwork is read from
.Values.linux.hostNetwork while readiness/liveness probe and container port
conditionals check .Values.node.hostNetwork; unify them by choosing one
canonical value (e.g., .Values.node.hostNetwork or .Values.linux.hostNetwork)
and update all references so hostNetwork, the probe/port if-conditions, and any
conditional blocks (the probe/port branches around readiness/liveness and
container ports) all use that single value (search for hostNetwork,
.Values.linux.hostNetwork, and .Values.node.hostNetwork in the template and make
them consistent).
In `@charts/v1.34.3/azuredisk-csi-driver/templates/csi-snapshot-controller.yaml`:
- Around line 98-115: The template incorrectly emits
.Values.snapshot.VolumeSnapshotClass.additionalLabels as a top-level
additionalLabels key; update the VolumeSnapshotClass resource rendering (in
csi-snapshot-controller.yaml for VolumeSnapshotClass) to render those entries
under metadata.labels instead of a root-level additionalLabels: block—remove the
current additionalLabels: block and, inside the resource's metadata: section,
add a metadata.labels: and render the map via {{ toYaml
.Values.snapshot.VolumeSnapshotClass.additionalLabels | indent 2 }} (or use {{-
with .Values.snapshot.VolumeSnapshotClass.additionalLabels }} ... {{- end }} to
guard emptiness) so labels are only emitted when present and indentation matches
the YAML structure.
In `@charts/v1.34.3/azuredisk-csi-driver/templates/NOTES.txt`:
- Line 5: The kubectl selector in NOTES.txt uses the wrong label key; replace
the selector key app.kubernetes.io/name={{ .Release.Name }} with
app.kubernetes.io/instance={{ .Release.Name }} so the pod lookup uses the
release instance label (update the line containing kubectl --namespace={{
.Release.Namespace }} get pods --selector="app.kubernetes.io/name={{
.Release.Name }}" --watch to use app.kubernetes.io/instance instead).
In
`@charts/v1.34.3/azuredisk-csi-driver/templates/rbac-csi-azuredisk-controller.yaml`:
- Around line 66-68: The external-attacher Role {{ .Values.rbac.name
}}-external-attacher-role grants verbs on the wrong API and resource; update the
Role binding in rbac-csi-azuredisk-controller.yaml so that the rule uses
apiGroups: ["storage.k8s.io"] and resources: ["csinodes"] (keeping the existing
verbs ["get","list","watch"]) instead of apiGroups: ["csi.storage.k8s.io"] and
resources: ["csinodeinfos"] to match the CSI Node API used elsewhere.
In `@charts/v1.34.4/azuredisk-csi-driver/templates/csi-azuredisk-controller.yaml`:
- Around line 42-61: The template currently only renders user-provided
controller.affinity when its rendered text contains "nodeSelectorTerms", which
silently drops valid affinity configs (e.g., podAntiAffinity); change the
condition to check whether controller.affinity is provided/non-empty after
rendering instead of searching for "nodeSelectorTerms". Replace the if
expression that uses tpl ... | contains "nodeSelectorTerms" with a check like
testing the tpl output for non-empty (e.g., {{- if tpl "{{
.Values.controller.affinity }}" . | trim | ne "" }} or equivalent) so any valid
affinity block (podAffinity, podAntiAffinity, nodeAffinity, etc.) is preserved;
keep the existing fallback branch that uses runOnControlPlane/runOnMaster
unchanged.
In `@charts/v1.34.4/azuredisk-csi-driver/templates/csi-azuredisk-node.yaml`:
- Around line 74-77: The chart mixes two different hostNetwork value keys
(.Values.linux.hostNetwork vs .Values.node.hostNetwork) causing probes/ports to
be rendered for the wrong network mode; pick the single canonical key used
elsewhere (e.g., .Values.linux.hostNetwork) and replace all occurrences of
.Values.node.hostNetwork in this template (the conditional blocks that choose
between --http-endpoint vs --health-port and the other branches around the node
liveness/readiness probe flags) so the checks that render --http-endpoint,
--health-port and probe host bindings all use the same value source
(.Values.linux.hostNetwork or .Values.node.hostNetwork consistently).
In `@charts/v1.34.4/azuredisk-csi-driver/templates/csi-snapshot-controller.yaml`:
- Around line 112-115: The template is rendering
.Values.snapshot.VolumeSnapshotClass.additionalLabels under a top-level
additionalLabels key, which is invalid for VolumeSnapshotClass; update the
template so the labels are merged into the VolumeSnapshotClass metadata by
replacing the top-level block with a metadata.labels section (e.g. inside the
VolumeSnapshotClass manifest add metadata: labels: {{ toYaml
.Values.snapshot.VolumeSnapshotClass.additionalLabels | indent 4 }}), ensuring
you keep the existing {{- with
.Values.snapshot.VolumeSnapshotClass.additionalLabels }} guard and use proper
indentation so labels render under metadata.labels of the VolumeSnapshotClass
resource.
In `@deploy/v1.34.4/csi-snapshot-controller.yaml`:
- Around line 34-45: The tolerations that target control-plane taints (keys
"node-role.kubernetes.io/master", "node-role.kubernetes.io/controlplane", and
"node-role.kubernetes.io/control-plane") currently use operator: "Equal" with
value: "true" which will miss key-only NoSchedule taints; update each toleration
to use operator: "Exists" and remove the value field so the toleration matches
taints that are key-only (i.e., replace operator: "Equal"/value: "true" for the
three keys with operator: "Exists" and no value).
In `@deploy/v1.34.4/rbac-csi-azuredisk-controller.yaml`:
- Around line 186-188: The RBAC rule currently grants cluster-wide secret
listing ("resources: [\"secrets\"]" with "verbs: [\"get\", \"list\"]"); remove
the "list" verb to restrict to read-only retrieval only (leave "get") or, if
listing is required, scope the permission to a namespace by converting the
ClusterRole/ClusterRoleBinding into a namespaced Role and RoleBinding for the
specific namespace; update the entry that contains resources: ["secrets"] and
verbs: ["get","list"] accordingly so no cluster-wide "list" on secrets remains.
In `@docs/install-csi-driver-v1.34.3.md`:
- Line 7: Remove the unsafe TLS skip flag from the remote install/uninstall curl
commands: replace the curl invocations that include the "-k" or "-skSL" options
(the lines invoking "curl -skSL
https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.3/deploy/install-driver.sh
| bash -s v1.34.3 snapshot --" and the corresponding uninstall curl) by removing
"-k" so TLS certificate verification is enforced (e.g., use "curl -sSL ... |
bash -s ..."); ensure both the install and uninstall command instances are
updated.
In `@docs/install-csi-driver-v1.34.4.md`:
- Line 7: Remove the insecure TLS bypass by dropping the -k flag from the curl
commands that pipe to bash (the occurrences of "curl -skSL
https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.4/deploy/install-driver.sh
| bash -s v1.34.4 snapshot --" and the similar command at the other occurrence);
update both instances so curl validates TLS by using -sSL (or no -k) to avoid
bypassing certificate checks when fetching remote install scripts.
In `@docs/migrate-batch-wrapper.md`:
- Line 124: The fenced code block that begins with the line
"migration-batch-logs/" is missing a language tag and triggers markdownlint
MD040; update the opening fence from ``` to ```text so the block becomes a
labeled "text" code block (i.e., the block containing "migration-batch-logs/",
"wrapper.log", and "preflight.log" should start with ```text).
In `@hack/premium-to-premiumv2-migrator-batch.sh`:
- Around line 491-500: The script treats any live stored_pid as an active batch
(setting stale_pid and incrementing running) which can deadlock if that PID was
recycled; update the logic around PID_BATCH/stale_pid to verify the live process
actually belongs to our migrator before counting it: after kill -0 "$stored_pid"
succeeds, confirm the process command line matches our expected marker (e.g.,
inspect /proc/$stored_pid/cmdline or use ps -o cmd= -p "$stored_pid" and compare
to the script name or a known argument), and only then assign
PID_BATCH[$stored_pid]=$bidx and increment running; if the check fails,
remove/ignore that PID from PID_BATCH and continue. Apply the same verification
where the other loop (lines 512-515) treats stored_pid as active.
- Around line 173-180: The helper get_cached_pvc_json referenced by
get_cached_batch_label is missing, making batch-label reuse a no-op; add a small
function named get_cached_pvc_json that reads the prepopulated ALL_PVCS_JSON
(populated by populate_pvcs) and returns the JSON for a given namespace and PVC
name (e.g., filter ALL_PVCS_JSON with jq using .items[] |
select(.metadata.namespace==env_ns and .metadata.name==env_name)), or
alternatively source the helper file that defines get_cached_pvc_json before
get_cached_batch_label is invoked; ensure the new function signature is
get_cached_pvc_json "$ns" "$pvc" and that errors propagate instead of being
silently swallowed so get_cached_batch_label can correctly reuse existing batch
labels.
---
Nitpick comments:
In
`@charts/v1.34.4/azuredisk-csi-driver/templates/rbac-csi-azuredisk-controller.yaml`:
- Around line 66-68: The RBAC rule granting apiGroups "csi.storage.k8s.io"
resources "csinodeinfos" is redundant because the chart already grants
"storage.k8s.io/csinodes"; remove the "csinodeinfos" rule from the controller
RBAC block (resource "csinodeinfos") or gate it for older clusters by wrapping
it in a Helm conditional that checks API availability (e.g. using
.Capabilities.APIVersions.Has) so the "csinodeinfos" entry is only rendered when
the cluster requires that legacy API.
🪄 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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 993615b3-b163-4dd5-b2b7-e03a5fbf1d76
⛔ Files ignored due to path filters (165)
go.sumis excluded by!**/*.sumvendor/go.opentelemetry.io/otel/.golangci.ymlis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/CONTRIBUTING.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/Makefileis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/RELEASING.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/hash.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/internal/attribute.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/kv.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/type_string.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/attribute/value.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/baggage/baggage.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/dependencies.Dockerfileis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/internal/errorhandler/errorhandler.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/internal/global/handler.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/internal/global/state.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/asyncfloat64.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/asyncint64.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/meter.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/syncfloat64.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/metric/syncint64.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/propagation/baggage.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/propagation/trace_context.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/requirements.txtis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/internal/x/features.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/config.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/internal/aggregate/exponential_histogram.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/internal/aggregate/histogram.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/internal/aggregate/lastvalue.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/internal/aggregate/sum.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/internal/observ/instrumentation.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/manual_reader.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/periodic_reader.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/pipeline.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/reader.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/metric/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/builtin.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/config.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/container.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/env.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/host_id_readfile.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/os.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/process.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/resource/resource.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/batch_span_processor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/batch_span_processor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/simple_span_processor.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/internal/observ/tracer.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/provider.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/sampling.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/trace/span.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/sdk/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/MIGRATION.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.39.0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.40.0/MIGRATION.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.40.0/README.mdis excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.40.0/attribute_group.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.40.0/doc.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.40.0/error_type.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.40.0/exception.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.40.0/otelconv/metric.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/semconv/v1.40.0/schema.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/auto.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/trace.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/trace/tracestate.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/version.gois excluded by!**/vendor/**,!vendor/**vendor/go.opentelemetry.io/otel/versions.yamlis excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/html/iter.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/html/node.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/html/nodetype_string.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/client_priority_go126.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/client_priority_go127.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/frame.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/http2.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/server.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/transport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_priority_rfc7540.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/http2/writesched_random.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/net/internal/httpsfv/httpsfv.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sync/singleflight/singleflight.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/asm_darwin_arm64_gc.sis excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/cpu_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/cpu_darwin_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/cpu_darwin_arm64_other.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/cpu_gccgo_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/cpu_other_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/cpu_windows_arm64.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/cpu/syscall_darwin_arm64_gc.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/plan9/syscall_plan9.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ioctl_signed.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ioctl_unsigned.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_solaris.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/syscall_unix.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/unix/ztypes_linux.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/aliases.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/registry/key.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/syscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/types_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/sys/windows/zsyscall_windows.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/cases/tables10.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/cases/tables11.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/cases/tables12.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/cases/tables15.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/cases/tables17.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/cases/tables9.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/message/catalog/catalog.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/message/catalog/dict.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/message/catalog/go19.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/message/catalog/gopre19.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule10.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/secure/bidirule/bidirule9.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables10.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables11.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables12.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables13.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables15.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables17.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/bidi/tables9.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/forminfo.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables10.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables11.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables12.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables15.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables17.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/text/unicode/norm/tables9.0.0.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ast/inspector/cursor.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ast/inspector/inspector.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/ast/inspector/iter.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/packages/packages.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/go/types/objectpath/objectpath.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/aliases/aliases.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/aliases/aliases_go122.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/core/event.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/keys/keys.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/event/label/label.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/iexport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/iimport.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/gcimporter/ureader_yes.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/deps.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/stdlib/manifest.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typeparams/free.gois excluded by!**/vendor/**,!vendor/**vendor/golang.org/x/tools/internal/typesinternal/types.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/encoding/tag/tag.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/encoding/text/decode.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/filedesc/desc_lazy.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/genid/descriptor_gen.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/codec_map.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/decode.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/impl/validate.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/internal/version/version.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/proto/decode.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/desc.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/reflect/protodesc/editions.gois excluded by!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/descriptorpb/descriptor.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/google.golang.org/protobuf/types/known/timestamppb/timestamp.pb.gois excluded by!**/*.pb.go,!**/vendor/**,!vendor/**vendor/modules.txtis excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/cloud-provider-azure/pkg/consts/consts.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_instance_metadata.gois excluded by!**/vendor/**,!vendor/**vendor/sigs.k8s.io/cloud-provider-azure/pkg/provider/azure_vmss_cache.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (93)
MakefileREADME.mdcharts/README.mdcharts/index.yamlcharts/latest/azuredisk-csi-driver-1.34.2.tgzcharts/latest/azuredisk-csi-driver-1.34.4.tgzcharts/latest/azuredisk-csi-driver/Chart.yamlcharts/latest/azuredisk-csi-driver/templates/csi-azuredisk-controller.yamlcharts/latest/azuredisk-csi-driver/templates/csi-azuredisk-node.yamlcharts/latest/azuredisk-csi-driver/values.yamlcharts/v1.34.3/azuredisk-csi-driver-1.34.3.tgzcharts/v1.34.3/azuredisk-csi-driver/Chart.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/NOTES.txtcharts/v1.34.3/azuredisk-csi-driver/templates/_helpers.tplcharts/v1.34.3/azuredisk-csi-driver/templates/crd-csi-snapshot.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/csi-azuredisk-controller.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/csi-azuredisk-driver.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/csi-azuredisk-node-windows-hostprocess.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/csi-azuredisk-node-windows.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/csi-azuredisk-node.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/csi-snapshot-controller.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/rbac-csi-azuredisk-controller.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/rbac-csi-azuredisk-node.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/rbac-csi-snapshot-controller.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/serviceaccount-csi-azuredisk-controller.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/serviceaccount-csi-azuredisk-node.yamlcharts/v1.34.3/azuredisk-csi-driver/templates/serviceaccount-csi-snapshot-controller.yamlcharts/v1.34.3/azuredisk-csi-driver/values.yamlcharts/v1.34.4/azuredisk-csi-driver-1.34.4.tgzcharts/v1.34.4/azuredisk-csi-driver/Chart.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/NOTES.txtcharts/v1.34.4/azuredisk-csi-driver/templates/_helpers.tplcharts/v1.34.4/azuredisk-csi-driver/templates/crd-csi-snapshot.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/csi-azuredisk-controller.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/csi-azuredisk-driver.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/csi-azuredisk-node-windows-hostprocess.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/csi-azuredisk-node-windows.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/csi-azuredisk-node.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/csi-snapshot-controller.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/rbac-csi-azuredisk-controller.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/rbac-csi-azuredisk-node.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/rbac-csi-snapshot-controller.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/serviceaccount-csi-azuredisk-controller.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/serviceaccount-csi-azuredisk-node.yamlcharts/v1.34.4/azuredisk-csi-driver/templates/serviceaccount-csi-snapshot-controller.yamlcharts/v1.34.4/azuredisk-csi-driver/values.yamldeploy/csi-azuredisk-controller.yamldeploy/csi-azuredisk-node-windows-hostprocess.yamldeploy/csi-azuredisk-node-windows.yamldeploy/csi-azuredisk-node.yamldeploy/v1.34.3/crd-csi-snapshot.yamldeploy/v1.34.3/csi-azuredisk-controller.yamldeploy/v1.34.3/csi-azuredisk-driver.yamldeploy/v1.34.3/csi-azuredisk-node-windows-hostprocess.yamldeploy/v1.34.3/csi-azuredisk-node-windows.yamldeploy/v1.34.3/csi-azuredisk-node.yamldeploy/v1.34.3/csi-snapshot-controller.yamldeploy/v1.34.3/rbac-csi-azuredisk-controller.yamldeploy/v1.34.3/rbac-csi-azuredisk-node.yamldeploy/v1.34.3/rbac-csi-snapshot-controller.yamldeploy/v1.34.4/crd-csi-snapshot.yamldeploy/v1.34.4/csi-azuredisk-controller.yamldeploy/v1.34.4/csi-azuredisk-driver.yamldeploy/v1.34.4/csi-azuredisk-node-windows-hostprocess.yamldeploy/v1.34.4/csi-azuredisk-node-windows.yamldeploy/v1.34.4/csi-azuredisk-node.yamldeploy/v1.34.4/csi-snapshot-controller.yamldeploy/v1.34.4/rbac-csi-azuredisk-controller.yamldeploy/v1.34.4/rbac-csi-azuredisk-node.yamldeploy/v1.34.4/rbac-csi-snapshot-controller.yamldocs/install-azuredisk-csi-driver.mddocs/install-csi-driver-v1.34.3.mddocs/install-csi-driver-v1.34.4.mddocs/migrate-batch-wrapper.mddocs/migrate-premiumlrs-to-premiumv2lrs.mdgo.modhack/lib-premiumv2-migration-common.shhack/premium-to-premiumv2-migrator-batch.shpkg/azuredisk/azure_controller_common.gopkg/azuredisk/azure_controller_common_test.gopkg/azuredisk/azure_dd_max_disk_count.gopkg/azuredisk/azure_managedDiskController.gopkg/azuredisk/azuredisk.gopkg/azuredisk/azuredisk_option.gopkg/azuredisk/controllerserver.gopkg/azurediskplugin/Dockerfilepkg/azurediskplugin/main.gopkg/azureutils/azure_disk_utils.gopkg/azureutils/azure_disk_utils_test.gopkg/os/disk/disk.gopkg/os/disk/disk_cim.gopkg/os/disk/disk_nvme_test.gotest/external-e2e/run.sh
Comment on lines
+42
to
+47
| {{- if tpl "{{ .Values.controller.affinity }}" . | contains "nodeSelectorTerms" }} | ||
| {{- with .Values.controller.affinity }} | ||
| affinity: | ||
| {{ toYaml . | indent 8 }} | ||
| {{- end }} | ||
| {{- else if or .Values.controller.runOnControlPlane .Values.controller.runOnMaster}} |
There was a problem hiding this comment.
Do not gate custom affinity on a nodeSelectorTerms substring check.
Line 42 can drop user-provided affinity unless it happens to contain nodeSelectorTerms. That silently ignores valid podAffinity/podAntiAffinity configs.
Suggested fix
- {{- if tpl "{{ .Values.controller.affinity }}" . | contains "nodeSelectorTerms" }}
- {{- with .Values.controller.affinity }}
- affinity:
-{{ toYaml . | indent 8 }}
- {{- end }}
+ {{- if .Values.controller.affinity }}
+ affinity:
+{{ toYaml .Values.controller.affinity | indent 8 }}
{{- else if or .Values.controller.runOnControlPlane .Values.controller.runOnMaster}}
affinity:
nodeAffinity:🤖 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 `@charts/v1.34.3/azuredisk-csi-driver/templates/csi-azuredisk-controller.yaml`
around lines 42 - 47, The template is incorrectly gating rendering of
user-provided affinity on a substring check for "nodeSelectorTerms" which can
drop valid podAffinity/podAntiAffinity settings; update the logic around
.Values.controller.affinity so that if .Values.controller.affinity is set it is
rendered directly (use tpl/with to evaluate and then emit affinity: followed by
toYaml . | indent 8) instead of using contains "nodeSelectorTerms" — remove the
contains check and the else branch that skips affinity, and ensure the template
uses the existing tpl "{{ .Values.controller.affinity }}" . and toYaml to output
any affinity object provided.
Comment on lines
+38
to
+39
| hostNetwork: {{ .Values.linux.hostNetwork }} | ||
| dnsPolicy: Default |
There was a problem hiding this comment.
Use one hostNetwork source consistently across pod spec and probe/port conditionals.
hostNetwork is set from .Values.linux.hostNetwork, but probe/port branches are gated by .Values.node.hostNetwork. If these values differ, the rendered DaemonSet can be internally inconsistent.
💡 Suggested fix
-{{- if eq .Values.node.hostNetwork true }}
+{{- if eq .Values.linux.hostNetwork true }}
- --http-endpoint=localhost:{{ .Values.node.livenessProbe.healthPort }}
{{- else }}
- --health-port={{ .Values.node.livenessProbe.healthPort }}
{{- end }}
@@
-{{- if ne .Values.node.hostNetwork true }}
+{{- if ne .Values.linux.hostNetwork true }}
ports:
- containerPort: {{ .Values.node.livenessProbe.healthPort }}
name: healthz
@@
-{{- if eq .Values.node.hostNetwork true }}
+{{- if eq .Values.linux.hostNetwork true }}
host: localhost
port: {{ .Values.node.livenessProbe.healthPort }}
{{- else }}
port: healthz
{{- end }}Also applies to: 74-78, 159-177
🤖 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 `@charts/v1.34.3/azuredisk-csi-driver/templates/csi-azuredisk-node.yaml` around
lines 38 - 39, The DaemonSet uses two different Helm values for hostNetwork
which can produce inconsistent manifests: hostNetwork is read from
.Values.linux.hostNetwork while readiness/liveness probe and container port
conditionals check .Values.node.hostNetwork; unify them by choosing one
canonical value (e.g., .Values.node.hostNetwork or .Values.linux.hostNetwork)
and update all references so hostNetwork, the probe/port if-conditions, and any
conditional blocks (the probe/port branches around readiness/liveness and
container ports) all use that single value (search for hostNetwork,
.Values.linux.hostNetwork, and .Values.node.hostNetwork in the template and make
them consistent).
Comment on lines
+98
to
+115
| metadata: | ||
| name: {{ .Values.snapshot.VolumeSnapshotClass.name }} | ||
| annotations: | ||
| # This is what defines this resource as a hook. Without this line, the | ||
| # job is considered part of the release. | ||
| "helm.sh/hook": post-install | ||
| driver: {{ .Values.driver.name }} | ||
| deletionPolicy: {{ .Values.snapshot.VolumeSnapshotClass.deletionPolicy }} | ||
| parameters: | ||
| incremental: {{ .Values.snapshot.VolumeSnapshotClass.parameters.incremental }} | ||
| {{- if ne .Values.snapshot.VolumeSnapshotClass.parameters.resourceGroup "" }} | ||
| resourceGroup: {{ .Values.snapshot.VolumeSnapshotClass.parameters.resourceGroup }} | ||
| {{- end }} | ||
| tags: {{ .Values.snapshot.VolumeSnapshotClass.parameters.tags }} | ||
| {{- with .Values.snapshot.VolumeSnapshotClass.additionalLabels }} | ||
| additionalLabels: | ||
| {{ toYaml . | indent 2 }} | ||
| {{- end }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="charts/v1.34.3/azuredisk-csi-driver/templates/csi-snapshot-controller.yaml"
echo "== File: $FILE =="
ls -la "$FILE" || true
echo
echo "== Lines 70-160 (context) =="
sed -n '70,160p' "$FILE"
echo
echo "== Search for additionalLabels in charts/v1.34.3/azuredisk-csi-driver/templates =="
rg -n "additionalLabels" charts/v1.34.3/azuredisk-csi-driver/templates || true
echo
echo "== Search for VolumeSnapshotClass.additionalLabels in charts/v1.34.3/azuredisk-csi-driver =="
rg -n "VolumeSnapshotClass\.additionalLabels" charts/v1.34.3/azuredisk-csi-driver || trueRepository: openshift/azure-disk-csi-driver
Length of output: 2930
🌐 Web query:
Kubernetes snapshot.storage.k8s.io/v1 VolumeSnapshotClass fields additionalLabels metadata.labels
💡 Result:
There are no VolumeSnapshotClass fields named "additionalLabels" or "metadata.labels" in the snapshot.storage.k8s.io/v1 API spec itself. 1) What VolumeSnapshotClass v1 actually defines A VolumeSnapshotClass (apiVersion: snapshot.storage.k8s.io/v1) specifies these top-level fields: driver, deletionPolicy, and parameters (plus standard Kubernetes metadata). The Kubernetes docs describe these class fields as the attributes used for dynamically provisioning VolumeSnapshots, and show the v1 YAML structure using metadata.name and the fields driver/deletionPolicy/parameters; they do not list any spec field called additionalLabels. [1] 2) Where labels go: always under standard ObjectMeta In Kubernetes, labels are part of the standard object metadata (ObjectMeta.labels). The Kubernetes API reference describes ObjectMeta as containing annotations and labels. [2] So the correct field is VolumeSnapshotClass.metadata.labels, but that is not a VolumeSnapshotClass-specific field—it's the generic Kubernetes metadata.labels on any object. 3) VolumeSnapshotClass does not have a spec-level additionalLabels concept The external-snapshotter CSI CRD for snapshot.storage.k8s.io/v1 VolumeSnapshotClasses is consistent with the Kubernetes docs: it defines metadata, driver, deletionPolicy, and parameters, with no VolumeSnapshotClass-specific spec field named additionalLabels. [3] Practical takeaway - Use VolumeSnapshotClass.metadata.labels to attach Kubernetes labels to the VolumeSnapshotClass object itself. [2] - Do not look for VolumeSnapshotClass.spec.additionalLabels or any “additionalLabels” field in snapshot.storage.k8s.io/v1; it isn’t part of the published v1 API/CRD schema. [1][3]
Citations:
- 1: https://kubernetes.io/docs/concepts/storage/volume-snapshot-classes/
- 2: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/
- 3: https://github.com/kubernetes-csi/external-snapshotter/blob/master/client/config/crd/snapshot.storage.k8s.io_volumesnapshotclasses.yaml
Render custom labels under metadata.labels for VolumeSnapshotClass (not as a top-level additionalLabels)
The template renders .Values.snapshot.VolumeSnapshotClass.additionalLabels as additionalLabels: at the resource root, but snapshot.storage.k8s.io/v1 VolumeSnapshotClass doesn’t define such a field; labels should be set via metadata.labels.
💡 Suggested fix
metadata:
name: {{ .Values.snapshot.VolumeSnapshotClass.name }}
annotations:
@@
"helm.sh/hook": post-install
+ {{- with .Values.snapshot.VolumeSnapshotClass.additionalLabels }}
+ labels:
+{{ toYaml . | indent 4 }}
+ {{- end }}
driver: {{ .Values.driver.name }}
deletionPolicy: {{ .Values.snapshot.VolumeSnapshotClass.deletionPolicy }}
parameters:
incremental: {{ .Values.snapshot.VolumeSnapshotClass.parameters.incremental }}
{{- if ne .Values.snapshot.VolumeSnapshotClass.parameters.resourceGroup "" }}
resourceGroup: {{ .Values.snapshot.VolumeSnapshotClass.parameters.resourceGroup }}
{{- end }}
tags: {{ .Values.snapshot.VolumeSnapshotClass.parameters.tags }}
-{{- with .Values.snapshot.VolumeSnapshotClass.additionalLabels }}
-additionalLabels:
-{{ toYaml . | indent 2 }}
-{{- end }}
{{- end -}}📝 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.
Suggested change
| metadata: | |
| name: {{ .Values.snapshot.VolumeSnapshotClass.name }} | |
| annotations: | |
| # This is what defines this resource as a hook. Without this line, the | |
| # job is considered part of the release. | |
| "helm.sh/hook": post-install | |
| driver: {{ .Values.driver.name }} | |
| deletionPolicy: {{ .Values.snapshot.VolumeSnapshotClass.deletionPolicy }} | |
| parameters: | |
| incremental: {{ .Values.snapshot.VolumeSnapshotClass.parameters.incremental }} | |
| {{- if ne .Values.snapshot.VolumeSnapshotClass.parameters.resourceGroup "" }} | |
| resourceGroup: {{ .Values.snapshot.VolumeSnapshotClass.parameters.resourceGroup }} | |
| {{- end }} | |
| tags: {{ .Values.snapshot.VolumeSnapshotClass.parameters.tags }} | |
| {{- with .Values.snapshot.VolumeSnapshotClass.additionalLabels }} | |
| additionalLabels: | |
| {{ toYaml . | indent 2 }} | |
| {{- end }} | |
| metadata: | |
| name: {{ .Values.snapshot.VolumeSnapshotClass.name }} | |
| annotations: | |
| # This is what defines this resource as a hook. Without this line, the | |
| # job is considered part of the release. | |
| "helm.sh/hook": post-install | |
| {{- with .Values.snapshot.VolumeSnapshotClass.additionalLabels }} | |
| labels: | |
| {{ toYaml . | indent 4 }} | |
| {{- end }} | |
| driver: {{ .Values.driver.name }} | |
| deletionPolicy: {{ .Values.snapshot.VolumeSnapshotClass.deletionPolicy }} | |
| parameters: | |
| incremental: {{ .Values.snapshot.VolumeSnapshotClass.parameters.incremental }} | |
| {{- if ne .Values.snapshot.VolumeSnapshotClass.parameters.resourceGroup "" }} | |
| resourceGroup: {{ .Values.snapshot.VolumeSnapshotClass.parameters.resourceGroup }} | |
| {{- end }} | |
| tags: {{ .Values.snapshot.VolumeSnapshotClass.parameters.tags }} |
🤖 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 `@charts/v1.34.3/azuredisk-csi-driver/templates/csi-snapshot-controller.yaml`
around lines 98 - 115, The template incorrectly emits
.Values.snapshot.VolumeSnapshotClass.additionalLabels as a top-level
additionalLabels key; update the VolumeSnapshotClass resource rendering (in
csi-snapshot-controller.yaml for VolumeSnapshotClass) to render those entries
under metadata.labels instead of a root-level additionalLabels: block—remove the
current additionalLabels: block and, inside the resource's metadata: section,
add a metadata.labels: and render the map via {{ toYaml
.Values.snapshot.VolumeSnapshotClass.additionalLabels | indent 2 }} (or use {{-
with .Values.snapshot.VolumeSnapshotClass.additionalLabels }} ... {{- end }} to
guard emptiness) so labels are only emitted when present and indentation matches
the YAML structure.
|
|
||
| To check Azure Disk CSI Driver pods status, please run: | ||
|
|
||
| kubectl --namespace={{ .Release.Namespace }} get pods --selector="app.kubernetes.io/name={{ .Release.Name }}" --watch |
There was a problem hiding this comment.
Fix the pod selector label in the install note command.
Line 5 selects app.kubernetes.io/name={{ .Release.Name }}, but release name maps to app.kubernetes.io/instance. With custom release names, this command can return zero pods.
Suggested fix
- kubectl --namespace={{ .Release.Namespace }} get pods --selector="app.kubernetes.io/name={{ .Release.Name }}" --watch
+ kubectl --namespace={{ .Release.Namespace }} get pods --selector="app.kubernetes.io/instance={{ .Release.Name }}" --watch📝 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.
Suggested change
| kubectl --namespace={{ .Release.Namespace }} get pods --selector="app.kubernetes.io/name={{ .Release.Name }}" --watch | |
| kubectl --namespace={{ .Release.Namespace }} get pods --selector="app.kubernetes.io/instance={{ .Release.Name }}" --watch |
🤖 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 `@charts/v1.34.3/azuredisk-csi-driver/templates/NOTES.txt` at line 5, The
kubectl selector in NOTES.txt uses the wrong label key; replace the selector key
app.kubernetes.io/name={{ .Release.Name }} with app.kubernetes.io/instance={{
.Release.Name }} so the pod lookup uses the release instance label (update the
line containing kubectl --namespace={{ .Release.Namespace }} get pods
--selector="app.kubernetes.io/name={{ .Release.Name }}" --watch to use
app.kubernetes.io/instance instead).
Comment on lines
+66
to
+68
| - apiGroups: ["csi.storage.k8s.io"] | ||
| resources: ["csinodeinfos"] | ||
| verbs: ["get", "list", "watch"] |
There was a problem hiding this comment.
Fix external-attacher RBAC to use storage.k8s.io/csinodes instead of legacy csi.storage.k8s.io/csinodeinfos
{{ .Values.rbac.name }}-external-attacher-role grants get/list/watch on csi.storage.k8s.io csinodeinfos, which is inconsistent with the current CSI Node API used elsewhere in the chart (storage.k8s.io csinodes), and can break external-attacher authorization.
💡 Suggested fix
- - apiGroups: ["csi.storage.k8s.io"]
- resources: ["csinodeinfos"]
+ - apiGroups: ["storage.k8s.io"]
+ resources: ["csinodes"]
verbs: ["get", "list", "watch"]📝 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.
Suggested change
| - apiGroups: ["csi.storage.k8s.io"] | |
| resources: ["csinodeinfos"] | |
| verbs: ["get", "list", "watch"] | |
| - apiGroups: ["storage.k8s.io"] | |
| resources: ["csinodes"] | |
| verbs: ["get", "list", "watch"] |
🤖 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
`@charts/v1.34.3/azuredisk-csi-driver/templates/rbac-csi-azuredisk-controller.yaml`
around lines 66 - 68, The external-attacher Role {{ .Values.rbac.name
}}-external-attacher-role grants verbs on the wrong API and resource; update the
Role binding in rbac-csi-azuredisk-controller.yaml so that the rule uses
apiGroups: ["storage.k8s.io"] and resources: ["csinodes"] (keeping the existing
verbs ["get","list","watch"]) instead of apiGroups: ["csi.storage.k8s.io"] and
resources: ["csinodeinfos"] to match the CSI Node API used elsewhere.
| ### Install by kubectl | ||
| - Option#1. remote install | ||
| ```console | ||
| curl -skSL https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.3/deploy/install-driver.sh | bash -s v1.34.3 snapshot -- |
There was a problem hiding this comment.
Remove -k from the curl install/uninstall commands.
Line 7 and Line 39 disable TLS certificate verification, which weakens the security of a remote-script execution path.
🔧 Suggested fix
-curl -skSL https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.3/deploy/install-driver.sh | bash -s v1.34.3 snapshot --
+curl -fsSL https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.3/deploy/install-driver.sh | bash -s v1.34.3 snapshot --
-curl -skSL https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.3/deploy/uninstall-driver.sh | bash -s v1.34.3 --
+curl -fsSL https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.3/deploy/uninstall-driver.sh | bash -s v1.34.3 --Also applies to: 39-39
🤖 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 `@docs/install-csi-driver-v1.34.3.md` at line 7, Remove the unsafe TLS skip
flag from the remote install/uninstall curl commands: replace the curl
invocations that include the "-k" or "-skSL" options (the lines invoking "curl
-skSL
https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.3/deploy/install-driver.sh
| bash -s v1.34.3 snapshot --" and the corresponding uninstall curl) by removing
"-k" so TLS certificate verification is enforced (e.g., use "curl -sSL ... |
bash -s ..."); ensure both the install and uninstall command instances are
updated.
| ### Install by kubectl | ||
| - Option#1. remote install | ||
| ```console | ||
| curl -skSL https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.4/deploy/install-driver.sh | bash -s v1.34.4 snapshot -- |
There was a problem hiding this comment.
Drop -k in these remote script examples.
Line 7 and Line 39 currently bypass TLS cert validation, which is risky for commands that pipe downloaded content to bash.
🔧 Suggested fix
-curl -skSL https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.4/deploy/install-driver.sh | bash -s v1.34.4 snapshot --
+curl -fsSL https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.4/deploy/install-driver.sh | bash -s v1.34.4 snapshot --
-curl -skSL https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.4/deploy/uninstall-driver.sh | bash -s v1.34.4 --
+curl -fsSL https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.4/deploy/uninstall-driver.sh | bash -s v1.34.4 --Also applies to: 39-39
🤖 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 `@docs/install-csi-driver-v1.34.4.md` at line 7, Remove the insecure TLS bypass
by dropping the -k flag from the curl commands that pipe to bash (the
occurrences of "curl -skSL
https://raw.githubusercontent.com/kubernetes-sigs/azuredisk-csi-driver/v1.34.4/deploy/install-driver.sh
| bash -s v1.34.4 snapshot --" and the similar command at the other occurrence);
update both instances so curl validates TLS by using -sSL (or no -k) to avoid
bypassing certificate checks when fetching remote install scripts.
| ## Logs | ||
|
|
||
| Per-batch logs and artifacts are written to `BATCH_LOG_DIR` (default: `migration-batch-logs/`): | ||
| ``` |
There was a problem hiding this comment.
Add a language to the fenced code block.
Line 124 starts a fenced block without a language tag, triggering markdownlint MD040.
🔧 Suggested fix
-```
+```text
migration-batch-logs/
wrapper.log # wrapper script stdout/stderr
preflight.log # shared resource creation log🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 124-124: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/migrate-batch-wrapper.md` at line 124, The fenced code block that begins
with the line "migration-batch-logs/" is missing a language tag and triggers
markdownlint MD040; update the opening fence from ``` to ```text so the block
becomes a labeled "text" code block (i.e., the block containing
"migration-batch-logs/", "wrapper.log", and "preflight.log" should start with
```text).
Comment on lines
+173
to
+180
| # Get existing batch label for a PVC from ALL_PVCS_JSON (loaded by populate_pvcs). | ||
| get_cached_batch_label() { | ||
| local ns="$1" pvc="$2" | ||
| local pvc_json | ||
| pvc_json=$(get_cached_pvc_json "$ns" "$pvc" 2>/dev/null || echo "") | ||
| if [[ -n "$pvc_json" ]]; then | ||
| echo "$pvc_json" | jq -r --arg key "$BATCH_LABEL_KEY" '.metadata.labels[$key] // empty' | ||
| fi |
There was a problem hiding this comment.
Undefined helper makes batch-label reuse a no-op.
Line 177 calls get_cached_pvc_json, but this script never defines/sources it before use. The fallback hides the failure, so existing batch IDs are never actually reused.
🔧 Proposed fix
get_cached_batch_label() {
local ns="$1" pvc="$2"
local pvc_json
- pvc_json=$(get_cached_pvc_json "$ns" "$pvc" 2>/dev/null || echo "")
+ pvc_json=$(kcmd get pvc "$pvc" -n "$ns" -o json 2>/dev/null || echo "")
if [[ -n "$pvc_json" ]]; then
echo "$pvc_json" | jq -r --arg key "$BATCH_LABEL_KEY" '.metadata.labels[$key] // empty'
fi
}🤖 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 `@hack/premium-to-premiumv2-migrator-batch.sh` around lines 173 - 180, The
helper get_cached_pvc_json referenced by get_cached_batch_label is missing,
making batch-label reuse a no-op; add a small function named get_cached_pvc_json
that reads the prepopulated ALL_PVCS_JSON (populated by populate_pvcs) and
returns the JSON for a given namespace and PVC name (e.g., filter ALL_PVCS_JSON
with jq using .items[] | select(.metadata.namespace==env_ns and
.metadata.name==env_name)), or alternatively source the helper file that defines
get_cached_pvc_json before get_cached_batch_label is invoked; ensure the new
function signature is get_cached_pvc_json "$ns" "$pvc" and that errors propagate
instead of being silently swallowed so get_cached_batch_label can correctly
reuse existing batch labels.
Comment on lines
+491
to
+500
| if [[ -n "$stored_pid" ]] && kill -0 "$stored_pid" 2>/dev/null; then | ||
| stale_pid="$stored_pid" | ||
| break | ||
| fi | ||
| done | ||
| if [[ -n "$stale_pid" ]]; then | ||
| info "Batch ${bidx}: previous process (pid=${stale_pid}) still running — tracking it." | ||
| PID_BATCH[$stale_pid]=$bidx | ||
| running=$(( running + 1 )) | ||
| continue |
There was a problem hiding this comment.
Tracking external PIDs as active can deadlock batch completion.
Line 497 treats any live stored_pid as an active tracked job and increments running. If that PID is reused by an unrelated process, the loop at Lines 512-515 can wait forever.
🔧 Proposed fix
if [[ -n "$stale_pid" ]]; then
- info "Batch ${bidx}: previous process (pid=${stale_pid}) still running — tracking it."
- PID_BATCH[$stale_pid]=$bidx
- running=$(( running + 1 ))
+ info "Batch ${bidx}: previous process (pid=${stale_pid}) appears active — skipping dispatch in this run."
continue
fiAlso applies to: 512-515
🤖 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 `@hack/premium-to-premiumv2-migrator-batch.sh` around lines 491 - 500, The
script treats any live stored_pid as an active batch (setting stale_pid and
incrementing running) which can deadlock if that PID was recycled; update the
logic around PID_BATCH/stale_pid to verify the live process actually belongs to
our migrator before counting it: after kill -0 "$stored_pid" succeeds, confirm
the process command line matches our expected marker (e.g., inspect
/proc/$stored_pid/cmdline or use ps -o cmd= -p "$stored_pid" and compare to the
script name or a known argument), and only then assign
PID_BATCH[$stored_pid]=$bidx and increment running; if the check fails,
remove/ignore that PID from PID_BATCH and continue. Apply the same verification
where the other loop (lines 512-515) treats stored_pid as active.
Author
|
/test e2e-azure |
Author
|
/retest |
2 similar comments
Author
|
/retest |
Author
|
/retest |
|
@rvagner78: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upstream changelogs
Summary of changes
Diff to upstream: kubernetes-sigs/azuredisk-csi-driver@v1.34.4...rvagner78:azure-disk-csi-driver:rebase-1.34.4
Previous rebase: #122
cc @openshift/storage
Summary by CodeRabbit
New Features
Updates
Documentation