Icm 50567

[alexk53]

Commit 1 — 7a176a4 (ICM-50566, Helm value injection)

• deployNfsProvisioner no longer builds --set-style value strings. Values are now a typed map marshalled to ChartSpec.ValuesYaml, so , and = in API-provided fields can't inject Helm keys like image.repository.
• Added sanitizeLabelValue() as defense-in-depth: the file share name is coerced into a valid Kubernetes label value (63 chars, safe charset) before being used as the fileShareName label.
• Added an envtest regression spec: a share named legit,image.repository=evil/malicious,image.tag=latest reconciles cleanly, the release values contain no injected image.repository, and the label comes out sanitized.

Commit 2 — c8cdf62 (ICM-50567, plaintext API token)

• New spec.apiTokenSecretRef (corev1.SecretKeySelector) pointing at a Secret in the CR's namespace; the key defaults to apiToken via the mutating webhook. The controller resolves the token from the Secret in resolveAPIToken() and passes it to the Gcore
  client (the FileShareLister interface now takes the token explicitly instead of reading it off the spec).
• spec.apiToken is kept as a deprecated optional fallback per your choice; the validating webhook now requires exactly one of the two (and a non-empty secret name). Also fixed a pre-existing webhook bug where the project validation error reported the
  region value.
• Regenerated deepcopy + CRD (apiToken removed from required), mirrored the schema change into the hand-committed example/deploy/gcore-sfs-controller-install.yaml, and updated both sample CRs and the README to the Secret pattern. No RBAC change was
  needed — the controller already has Secret read access.

[Copilot]

Pull request overview

This PR hardens the NfsProvisioner controller against Helm value injection and moves Gcore API token handling away from plaintext CR fields toward Secret references, updating the controller, webhooks/CRDs, examples, and tests accordingly.

Changes:

• Pass Helm values as structured YAML (ChartSpec.ValuesYaml) and sanitize the file share name before using it as a Kubernetes label value.
• Add spec.apiTokenSecretRef (SecretKeySelector) with webhook defaulting/validation, and resolve the API token from a Secret at reconcile time (keeping spec.apiToken as a deprecated fallback).
• Update CRD manifests, samples, README, and add envtest coverage for the injection regression + Secret-based token flow.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
README.md	Documents Secret-based API token configuration and deprecates plaintext spec.apiToken.
pkg/gcoreclient/client.go	Updates client interface to accept an explicit API token argument.
internal/controller/nfsprovisioner_controller.go	Resolves API token from Secret, switches Helm values to YAML, and sanitizes label values.
internal/controller/nfsprovisioner_controller_test.go	Adds envtest regression verifying Helm injection resistance and label sanitization; uses Secret token.
go.mod	Promotes sigs.k8s.io/yaml to a direct dependency.
example/deploy/nfsprovisioner.yaml	Updates example to create a Secret and reference it via apiTokenSecretRef.
example/deploy/gcore-sfs-controller-install.yaml	Regenerates/install manifest CRD schema to include apiTokenSecretRef and deprecate apiToken.
config/samples/crd_v1_nfsprovisioner.yaml	Updates sample manifest to use Secret + apiTokenSecretRef.
config/crd/bases/crd.gcore-sfs-controller.io_nfsprovisioners.yaml	Updates CRD base schema to include apiTokenSecretRef and deprecate apiToken.
api/v1/zz_generated.deepcopy.go	Regenerates deepcopy methods for the new pointer field in spec.
api/v1/nfsprovisioner_webhook.go	Adds defaulting for Secret key + validates mutual exclusivity and presence of token config; fixes project error reporting.
api/v1/nfsprovisioner_webhook_test.go	Extends webhook tests to cover defaulting and new validation rules.
api/v1/nfsprovisioner_types.go	Adds DefaultAPITokenSecretKey and new spec field APITokenSecretRef; marks APIToken optional/deprecated.

Files not reviewed (1)

• api/v1/zz_generated.deepcopy.go: Generated file

Suppressed comments (1)

internal/controller/nfsprovisioner_controller.go:223

• Same formatting issue here: %q is used with client.ObjectKey, which is not a string/rune/[]byte and will produce a %!q(...) formatting artifact. Prefer %s/%v for the Secret identifier.

	token, found := secret.Data[key]
	if !found || len(token) == 0 {
		return "", fmt.Errorf("secret %q does not contain a non-empty key %q", secretName, key)
	}

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.