From 0491d78d192ab8a4a1e1ab24661b2a77d9cf71ab Mon Sep 17 00:00:00 2001 From: Alexandr Gorshunov Date: Thu, 27 Aug 2026 16:28:14 +0200 Subject: [PATCH 1/2] IMC-50566 Fix Helm value injection via file share name --- go.mod | 2 +- .../controller/nfsprovisioner_controller.go | 66 ++++++++++---- .../nfsprovisioner_controller_test.go | 87 +++++++++++++++++++ 3 files changed, 139 insertions(+), 16 deletions(-) diff --git a/go.mod b/go.mod index 56ac2be..f4d9575 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( k8s.io/apimachinery v0.27.3 k8s.io/client-go v0.27.3 sigs.k8s.io/controller-runtime v0.15.0 + sigs.k8s.io/yaml v1.3.0 ) require ( @@ -157,7 +158,6 @@ require ( sigs.k8s.io/kustomize/api v0.13.2 // indirect sigs.k8s.io/kustomize/kyaml v0.14.1 // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect - sigs.k8s.io/yaml v1.3.0 // indirect ) replace github.com/G-Core/gcore-sfs-controller/pkg/gcoreclient => ./pkg/gcoreclient diff --git a/internal/controller/nfsprovisioner_controller.go b/internal/controller/nfsprovisioner_controller.go index c027061..24b4494 100644 --- a/internal/controller/nfsprovisioner_controller.go +++ b/internal/controller/nfsprovisioner_controller.go @@ -25,7 +25,6 @@ import ( "github.com/G-Core/gcore-sfs-controller/pkg/gcoreclient" "github.com/G-Core/gcorelabscloud-go/gcore/file_share/v1/file_shares" gohelmclient "github.com/mittwald/go-helm-client" - "github.com/mittwald/go-helm-client/values" "helm.sh/helm/v3/pkg/repo" corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" @@ -38,6 +37,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/yaml" ) const RepositoryName = "nfs-subdir-external-provisioner" @@ -218,24 +218,40 @@ func (r *NfsProvisionerReconciler) deployNfsProvisioner(ctx context.Context, pro if err != nil { return "", err } + // Values are passed as structured YAML instead of --set-style strings so that + // characters like ',' and '=' in API-provided fields (e.g. the file share name) + // cannot inject additional Helm keys. + chartValues := map[string]interface{}{ + "nfs": map[string]interface{}{ + "server": nfsServer, + "path": nfsPath, + // Options allow unmount volume when file share was deleted + "mountOptions": []string{"soft"}, + }, + "storageClass": map[string]interface{}{ + "name": fmt.Sprintf("nfs-%s", fileShare.ID), + "accessModes": "ReadWriteMany", + "defaultClass": false, + }, + "image": map[string]interface{}{ + "tag": provisioner.Spec.ImageVersion, + }, + "labels": map[string]interface{}{ + NfsProvisionerIDLabelName: string(provisioner.UID), + FileShareIDLabelName: fileShare.ID, + FileShareNameLabelName: sanitizeLabelValue(fileShare.Name), + }, + } + valuesYaml, err := yaml.Marshal(chartValues) + if err != nil { + return "", err + } release, err := r.HelmClient.InstallOrUpgradeChart(ctx, &gohelmclient.ChartSpec{ ReleaseName: r.getReleaseName(fileShare.ID), ChartName: fmt.Sprintf("%s/%s", RepositoryName, provisioner.Spec.ChartName), Namespace: provisioner.Namespace, - ValuesOptions: values.Options{ - Values: []string{ - fmt.Sprintf("nfs.server=%s", nfsServer), - fmt.Sprintf("nfs.path=%s", nfsPath), - fmt.Sprintf("storageClass.name=nfs-%s", fileShare.ID), - "storageClass.accessModes=ReadWriteMany", - "storageClass.defaultClass=false", - "nfs.mountOptions={soft}", // Options allow unmount volume when file share was deleted - fmt.Sprintf("image.tag=%s", provisioner.Spec.ImageVersion), - fmt.Sprintf("labels.%s=%s", NfsProvisionerIDLabelName, provisioner.UID), - fmt.Sprintf("labels.%s=%s", FileShareIDLabelName, fileShare.ID), - fmt.Sprintf("labels.%s=%s", FileShareNameLabelName, fileShare.Name), - }, - }}, + ValuesYaml: string(valuesYaml), + }, nil) if err != nil { return "", err @@ -243,6 +259,26 @@ func (r *NfsProvisionerReconciler) deployNfsProvisioner(ctx context.Context, pro return release.Name, nil } +// sanitizeLabelValue converts an arbitrary string into a valid Kubernetes label +// value: at most 63 characters, alphanumeric with '-', '_' and '.' allowed in +// the middle. Invalid characters are replaced with '-'. +func sanitizeLabelValue(value string) string { + const maxLabelValueLength = 63 + sanitized := []byte(strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z', r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '-', r == '_', r == '.': + return r + default: + return '-' + } + }, value)) + if len(sanitized) > maxLabelValueLength { + sanitized = sanitized[:maxLabelValueLength] + } + trimmed := strings.Trim(string(sanitized), "-_.") + return trimmed +} + func (r *NfsProvisionerReconciler) reconcileDelete(ctx context.Context, provisioner *crdv1.NfsProvisioner) (ctrl.Result, error) { currentReleaseNameSet, err := r.getCurrentReleaseNameSet(ctx, provisioner) if err != nil { diff --git a/internal/controller/nfsprovisioner_controller_test.go b/internal/controller/nfsprovisioner_controller_test.go index 85ef2fc..52e6602 100644 --- a/internal/controller/nfsprovisioner_controller_test.go +++ b/internal/controller/nfsprovisioner_controller_test.go @@ -110,4 +110,91 @@ var _ = Describe("NfsProvisioner Reconciler", func() { Expect(len(storageClassList.Items)).To(Equal(0)) }) + + It("File share name with Helm --set syntax should not inject chart values", func() { + provisioner := crdv1.NfsProvisioner{ + TypeMeta: metav1.TypeMeta{ + Kind: "NfsProvisioner", + APIVersion: crdv1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: testNfsProvisionerName + "-inject", + Namespace: DefaultNamespace, + }, + Spec: crdv1.NfsProvisionerSpec{ + APIToken: "faketoken", + APIURL: "http://127.0.0.1", + RegionID: 2, + ProjectID: 5, + HelmRepository: "https://kubernetes-sigs.github.io/nfs-subdir-external-provisioner", + ChartName: "nfs-subdir-external-provisioner", + ImageVersion: "v4.0.2", + }, + } + err := k8sClient.Create(ctx, &provisioner) + Expect(err).NotTo(HaveOccurred()) + + helmClient, err := gohelmclient.NewClientFromRestConf( + &gohelmclient.RestConfClientOptions{ + Options: &gohelmclient.Options{}, + RestConfig: cfg, + }) + Expect(err).NotTo(HaveOccurred()) + fileShare := file_shares.FileShare{ + Name: "legit,image.repository=evil/malicious,image.tag=latest", + ID: "0e2a9d4c-11a2-4f54-b67e-6c9d4e34a409", + Protocol: "nfs", + Status: "available", + Size: 2, + VolumeType: "default_share_type", + ConnectionPoint: "10.33.20.91:/shares/share-0e2a9d4c-11a2-4f54-b67e-6c9d4e34a409", + ProjectID: 1, + RegionID: 1, + } + fileShareLister := gcoreclient.MockFileShareClient{ + FileShares: []file_shares.FileShare{fileShare}, + } + + reconciler := NfsProvisionerReconciler{ + Client: k8sClient, + HelmClient: helmClient, + FileShareClient: fileShareLister, + } + _, err = reconciler.Reconcile( + ctx, + ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: DefaultNamespace, + Name: provisioner.Name, + }}) + Expect(err).NotTo(HaveOccurred()) + + // The user-supplied release values must not contain keys injected via + // commas/equals in the file share name. + releaseValues, err := helmClient.GetReleaseValues("nfsprovisioner-"+fileShare.ID, false) + Expect(err).NotTo(HaveOccurred()) + imageValues, ok := releaseValues["image"].(map[string]interface{}) + Expect(ok).To(BeTrue()) + Expect(imageValues).NotTo(HaveKey("repository")) + Expect(imageValues["tag"]).To(Equal("v4.0.2")) + + // The file share name must be sanitized into a valid label value. + storageClass := storagev1.StorageClass{} + err = k8sClient.Get(ctx, types.NamespacedName{Name: "nfs-" + fileShare.ID}, &storageClass) + Expect(err).NotTo(HaveOccurred()) + Expect(storageClass.Labels["fileShareName"]).To( + Equal("legit-image.repository-evil-malicious-image.tag-latest")) + + // Cleanup + err = k8sClient.Delete(ctx, &provisioner) + Expect(err).NotTo(HaveOccurred()) + _, err = reconciler.Reconcile( + ctx, + ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: DefaultNamespace, + Name: provisioner.Name, + }}) + Expect(err).NotTo(HaveOccurred()) + }) }) From d2a14613c86d2b480323555b7b7dcda97f26117e Mon Sep 17 00:00:00 2001 From: Alexandr Gorshunov Date: Thu, 27 Aug 2026 16:33:50 +0200 Subject: [PATCH 2/2] ICM-50567 Support API token via Secret reference, deprecate plaintext apiToken --- README.md | 20 ++++-- api/v1/nfsprovisioner_types.go | 19 +++++- api/v1/nfsprovisioner_webhook.go | 16 ++++- api/v1/nfsprovisioner_webhook_test.go | 64 +++++++++++++++++++ api/v1/zz_generated.deepcopy.go | 8 ++- ...ore-sfs-controller.io_nfsprovisioners.yaml | 28 +++++++- config/samples/crd_v1_nfsprovisioner.yaml | 11 +++- .../deploy/gcore-sfs-controller-install.yaml | 28 +++++++- example/deploy/nfsprovisioner.yaml | 12 +++- .../controller/nfsprovisioner_controller.go | 34 +++++++++- .../nfsprovisioner_controller_test.go | 22 ++++++- pkg/gcoreclient/client.go | 12 ++-- 12 files changed, 250 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index bd08ee9..6948290 100644 --- a/README.md +++ b/README.md @@ -18,12 +18,24 @@ kubectl apply -f example/deploy/gcore-sfs-controller-install.yaml ### Install CRD 1. You must fill in the next values: ```yaml + apiVersion: v1 + kind: Secret + metadata: + name: gcore-api-token + namespace: gcore-sfs-controller-system + stringData: + apiToken: + --- spec: - apiToken: - region: - project: + apiTokenSecretRef: + name: gcore-api-token + key: apiToken + region: + project: ``` - `apiToken`: Create API token in [CLOUD UI](https://gcore.com/docs/account-settings/create-use-or-delete-a-permanent-api-token). + `apiTokenSecretRef`: References a key of a Secret (in the same namespace as the NfsProvisioner) holding your API token. Create API token in [CLOUD UI](https://gcore.com/docs/account-settings/create-use-or-delete-a-permanent-api-token). + + **Note:** The `spec.apiToken` field (plaintext token stored directly in the custom resource) is deprecated because anyone able to read the resource can read the token. Use `apiTokenSecretRef` instead. `region`: You can get a region id from our [API](https://api.gcore.com/docs/cloud#tag/Regions/operation/RegionHandler.get): You will get a list of regions from the "v1/regions" handler, and then you can find the needed region by the "display_name" field. diff --git a/api/v1/nfsprovisioner_types.go b/api/v1/nfsprovisioner_types.go index 4950542..76f4a23 100644 --- a/api/v1/nfsprovisioner_types.go +++ b/api/v1/nfsprovisioner_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1 import ( + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -24,10 +25,26 @@ import ( // by its managing controller. const NfsProvisionerFinalizer = "nfsprovisioner.gcore-sfs-controller.io" +// DefaultAPITokenSecretKey is the Secret key used when +// spec.apiTokenSecretRef does not specify one. +const DefaultAPITokenSecretKey = "apiToken" + // NfsProvisionerSpec defines the desired state of NfsProvisioner type NfsProvisionerSpec struct { // APIToken is the API token used to authenticate with Gcore Cloud. - APIToken string `json:"apiToken"` + // + // Deprecated: storing the token in the custom resource exposes it to + // anyone who can read the resource (kubectl get, etcd backups, audit + // logs). Use APITokenSecretRef instead. + // +optional + APIToken string `json:"apiToken,omitempty"` + + // APITokenSecretRef references a key of a Secret in the same namespace + // as the NfsProvisioner that holds the Gcore Cloud API token. + // If the key is not specified, it defaults to "apiToken". + // Exactly one of APIToken and APITokenSecretRef must be set. + // +optional + APITokenSecretRef *corev1.SecretKeySelector `json:"apiTokenSecretRef,omitempty"` // APIURL is the URL of the Gcore Cloud API. // +optional APIURL string `json:"apiURL,omitempty"` diff --git a/api/v1/nfsprovisioner_webhook.go b/api/v1/nfsprovisioner_webhook.go index 2a74c4f..91425b1 100644 --- a/api/v1/nfsprovisioner_webhook.go +++ b/api/v1/nfsprovisioner_webhook.go @@ -66,6 +66,9 @@ func (r *NfsProvisioner) Default() { r.Spec.ImageVersion = DefaultNfsProvisionerImageVersion } nfsprovisionerlog.Info("default", "imageVersion", r.Spec.ImageVersion) + if r.Spec.APITokenSecretRef != nil && r.Spec.APITokenSecretRef.Key == "" { + r.Spec.APITokenSecretRef.Key = DefaultAPITokenSecretKey + } } //+kubebuilder:webhook:path=/validate-crd-gcore-sfs-controller-io-v1-nfsprovisioner,mutating=false,failurePolicy=fail,sideEffects=None,groups=crd.gcore-sfs-controller.io,resources=nfsprovisioners,verbs=create;update,versions=v1,name=vnfsprovisioner.kb.io,admissionReviewVersions=v1 @@ -79,9 +82,20 @@ func ValidateNfsProvisioner(r *NfsProvisioner) error { allErrs = append(allErrs, regionErr) } if r.Spec.ProjectID <= 0 { - projectErr := field.Invalid(field.NewPath("spec").Child("project"), r.Spec.RegionID, "must be positive") + projectErr := field.Invalid(field.NewPath("spec").Child("project"), r.Spec.ProjectID, "must be positive") allErrs = append(allErrs, projectErr) } + switch { + case r.Spec.APIToken == "" && r.Spec.APITokenSecretRef == nil: + allErrs = append(allErrs, field.Required(field.NewPath("spec").Child("apiTokenSecretRef"), + "one of apiToken or apiTokenSecretRef must be set")) + case r.Spec.APIToken != "" && r.Spec.APITokenSecretRef != nil: + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec").Child("apiToken"), + "apiToken and apiTokenSecretRef are mutually exclusive")) + case r.Spec.APITokenSecretRef != nil && r.Spec.APITokenSecretRef.Name == "": + allErrs = append(allErrs, field.Required(field.NewPath("spec").Child("apiTokenSecretRef").Child("name"), + "secret name must be set")) + } if len(allErrs) == 0 { return nil } diff --git a/api/v1/nfsprovisioner_webhook_test.go b/api/v1/nfsprovisioner_webhook_test.go index 5315550..7c39ef6 100644 --- a/api/v1/nfsprovisioner_webhook_test.go +++ b/api/v1/nfsprovisioner_webhook_test.go @@ -4,6 +4,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -59,6 +60,9 @@ var _ = Describe("NfsProvisioner webhooks", func() { Namespace: "default", }, Spec: NfsProvisionerSpec{ + APITokenSecretRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "gcore-api-token"}, + }, RegionID: 1, ProjectID: 1, }, @@ -69,5 +73,65 @@ var _ = Describe("NfsProvisioner webhooks", func() { Expect(provisioner.Spec.HelmRepository).To(Equal(DefaultHelmRepository)) Expect(provisioner.Spec.ChartName).To(Equal(DefaultHelmChartName)) Expect(provisioner.Spec.ImageVersion).To(Equal(DefaultNfsProvisionerImageVersion)) + Expect(provisioner.Spec.APITokenSecretRef.Key).To(Equal(DefaultAPITokenSecretKey)) + }) + It("Check NfsProvisioner webhook rejects missing API token configuration", func() { + provisioner := NfsProvisioner{ + TypeMeta: metav1.TypeMeta{ + Kind: "NfsProvisioner", + APIVersion: GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "provisioner-no-token", + Namespace: "default", + }, + Spec: NfsProvisionerSpec{ + RegionID: 1, + ProjectID: 1, + }, + } + err := k8sClient.Create(ctx, &provisioner) + Expect(err).To(MatchError(ContainSubstring("one of apiToken or apiTokenSecretRef must be set"))) + }) + It("Check NfsProvisioner webhook rejects both apiToken and apiTokenSecretRef", func() { + provisioner := NfsProvisioner{ + TypeMeta: metav1.TypeMeta{ + Kind: "NfsProvisioner", + APIVersion: GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "provisioner-both-tokens", + Namespace: "default", + }, + Spec: NfsProvisionerSpec{ + APIToken: "faketoken", + APITokenSecretRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "gcore-api-token"}, + }, + RegionID: 1, + ProjectID: 1, + }, + } + err := k8sClient.Create(ctx, &provisioner) + Expect(err).To(MatchError(ContainSubstring("mutually exclusive"))) + }) + It("Check NfsProvisioner webhook rejects apiTokenSecretRef without a name", func() { + provisioner := NfsProvisioner{ + TypeMeta: metav1.TypeMeta{ + Kind: "NfsProvisioner", + APIVersion: GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "provisioner-unnamed-secret", + Namespace: "default", + }, + Spec: NfsProvisionerSpec{ + APITokenSecretRef: &corev1.SecretKeySelector{}, + RegionID: 1, + ProjectID: 1, + }, + } + err := k8sClient.Create(ctx, &provisioner) + Expect(err).To(MatchError(ContainSubstring("secret name must be set"))) }) }) diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 1e6e223..68a9070 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -22,6 +22,7 @@ limitations under the License. package v1 import ( + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -30,7 +31,7 @@ func (in *NfsProvisioner) DeepCopyInto(out *NfsProvisioner) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) out.Status = in.Status } @@ -87,6 +88,11 @@ func (in *NfsProvisionerList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NfsProvisionerSpec) DeepCopyInto(out *NfsProvisionerSpec) { *out = *in + if in.APITokenSecretRef != nil { + in, out := &in.APITokenSecretRef, &out.APITokenSecretRef + *out = new(corev1.SecretKeySelector) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NfsProvisionerSpec. diff --git a/config/crd/bases/crd.gcore-sfs-controller.io_nfsprovisioners.yaml b/config/crd/bases/crd.gcore-sfs-controller.io_nfsprovisioners.yaml index d398641..d5f1bf2 100644 --- a/config/crd/bases/crd.gcore-sfs-controller.io_nfsprovisioners.yaml +++ b/config/crd/bases/crd.gcore-sfs-controller.io_nfsprovisioners.yaml @@ -35,9 +35,32 @@ spec: description: NfsProvisionerSpec defines the desired state of NfsProvisioner properties: apiToken: - description: APIToken is the API token used to authenticate with Gcore - Cloud. + description: "APIToken is the API token used to authenticate with + Gcore Cloud. \n Deprecated: storing the token in the custom resource + exposes it to anyone who can read the resource (kubectl get, etcd + backups, audit logs). Use APITokenSecretRef instead." type: string + apiTokenSecretRef: + description: APITokenSecretRef references a key of a Secret in the + same namespace as the NfsProvisioner that holds the Gcore Cloud + API token. If the key is not specified, it defaults to "apiToken". + Exactly one of APIToken and APITokenSecretRef must be set. + properties: + key: + description: The key of the secret to select from. Must be a + valid secret key. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the Secret or its key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic apiURL: description: APIURL is the URL of the Gcore Cloud API. type: string @@ -64,7 +87,6 @@ spec: description: File share region ID type: integer required: - - apiToken - project - region type: object diff --git a/config/samples/crd_v1_nfsprovisioner.yaml b/config/samples/crd_v1_nfsprovisioner.yaml index ec6e32c..8a140a6 100644 --- a/config/samples/crd_v1_nfsprovisioner.yaml +++ b/config/samples/crd_v1_nfsprovisioner.yaml @@ -1,3 +1,10 @@ +apiVersion: v1 +kind: Secret +metadata: + name: gcore-api-token +stringData: + apiToken: +--- apiVersion: crd.gcore-sfs-controller.io/v1 kind: NfsProvisioner metadata: @@ -9,6 +16,8 @@ metadata: app.kubernetes.io/created-by: gcore-sfs-controller name: nfsprovisioner-sample spec: - apiToken: + apiTokenSecretRef: + name: gcore-api-token + key: apiToken region: project: diff --git a/example/deploy/gcore-sfs-controller-install.yaml b/example/deploy/gcore-sfs-controller-install.yaml index 6280fc2..53c990b 100644 --- a/example/deploy/gcore-sfs-controller-install.yaml +++ b/example/deploy/gcore-sfs-controller-install.yaml @@ -58,9 +58,32 @@ spec: description: NfsProvisionerSpec defines the desired state of NfsProvisioner properties: apiToken: - description: APIToken is the API token used to authenticate with Gcore - Cloud. + description: "APIToken is the API token used to authenticate with + Gcore Cloud. \n Deprecated: storing the token in the custom resource + exposes it to anyone who can read the resource (kubectl get, etcd + backups, audit logs). Use APITokenSecretRef instead." type: string + apiTokenSecretRef: + description: APITokenSecretRef references a key of a Secret in the + same namespace as the NfsProvisioner that holds the Gcore Cloud + API token. If the key is not specified, it defaults to "apiToken". + Exactly one of APIToken and APITokenSecretRef must be set. + properties: + key: + description: The key of the secret to select from. Must be a + valid secret key. + type: string + name: + description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid?' + type: string + optional: + description: Specify whether the Secret or its key must be defined + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic apiURL: description: APIURL is the URL of the Gcore Cloud API. type: string @@ -87,7 +110,6 @@ spec: description: File share region ID type: integer required: - - apiToken - project - region type: object diff --git a/example/deploy/nfsprovisioner.yaml b/example/deploy/nfsprovisioner.yaml index ec401a4..42810a9 100644 --- a/example/deploy/nfsprovisioner.yaml +++ b/example/deploy/nfsprovisioner.yaml @@ -1,3 +1,11 @@ +apiVersion: v1 +kind: Secret +metadata: + name: gcore-api-token + namespace: gcore-sfs-controller-system +stringData: + apiToken: +--- apiVersion: crd.gcore-sfs-controller.io/v1 kind: NfsProvisioner metadata: @@ -10,6 +18,8 @@ metadata: name: nfsprovisioner namespace: gcore-sfs-controller-system spec: - apiToken: + apiTokenSecretRef: + name: gcore-api-token + key: apiToken region: project: diff --git a/internal/controller/nfsprovisioner_controller.go b/internal/controller/nfsprovisioner_controller.go index 24b4494..459f8d6 100644 --- a/internal/controller/nfsprovisioner_controller.go +++ b/internal/controller/nfsprovisioner_controller.go @@ -159,7 +159,12 @@ func (r *NfsProvisionerReconciler) updateStatus(ctx context.Context, provisioner func (r *NfsProvisionerReconciler) reconcileNormal(ctx context.Context, provisioner *crdv1.NfsProvisioner) (ctrl.Result, error) { log := log.FromContext(ctx) - allFileShares, err := r.FileShareClient.ListFileShares(provisioner) + apiToken, err := r.resolveAPIToken(ctx, provisioner) + if err != nil { + log.Error(err, "resolve Gcore Cloud API token", "namespace", provisioner.Namespace, "name", provisioner.Name) + return ctrl.Result{}, err + } + allFileShares, err := r.FileShareClient.ListFileShares(provisioner, apiToken) if err != nil { log.Error(err, "get file shares in the project", "regionID", provisioner.Spec.RegionID, "projectID", provisioner.Spec.ProjectID) return ctrl.Result{}, err @@ -192,6 +197,33 @@ func (r *NfsProvisionerReconciler) reconcileNormal(ctx context.Context, provisio return ctrl.Result{}, nil } +// resolveAPIToken returns the Gcore Cloud API token for the provisioner, +// reading it from the referenced Secret if spec.apiTokenSecretRef is set and +// falling back to the deprecated plaintext spec.apiToken otherwise. +func (r *NfsProvisionerReconciler) resolveAPIToken(ctx context.Context, provisioner *crdv1.NfsProvisioner) (string, error) { + secretRef := provisioner.Spec.APITokenSecretRef + if secretRef == nil { + if provisioner.Spec.APIToken == "" { + return "", fmt.Errorf("neither spec.apiTokenSecretRef nor spec.apiToken is set") + } + return provisioner.Spec.APIToken, nil + } + secret := corev1.Secret{} + secretName := client.ObjectKey{Namespace: provisioner.Namespace, Name: secretRef.Name} + if err := r.Client.Get(ctx, secretName, &secret); err != nil { + return "", fmt.Errorf("get API token secret %q: %w", secretName, err) + } + key := secretRef.Key + if key == "" { + key = crdv1.DefaultAPITokenSecretKey + } + 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) + } + return string(token), nil +} + func (r NfsProvisionerReconciler) getReleaseName(fileShareID string) string { return fmt.Sprintf("nfsprovisioner-%s", fileShareID) } diff --git a/internal/controller/nfsprovisioner_controller_test.go b/internal/controller/nfsprovisioner_controller_test.go index 52e6602..7291d43 100644 --- a/internal/controller/nfsprovisioner_controller_test.go +++ b/internal/controller/nfsprovisioner_controller_test.go @@ -7,6 +7,7 @@ import ( gohelmclient "github.com/mittwald/go-helm-client" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" @@ -112,6 +113,20 @@ var _ = Describe("NfsProvisioner Reconciler", func() { }) It("File share name with Helm --set syntax should not inject chart values", func() { + // The API token is referenced from a Secret instead of being stored + // in plaintext on the custom resource. + tokenSecret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gcore-api-token", + Namespace: DefaultNamespace, + }, + Data: map[string][]byte{ + crdv1.DefaultAPITokenSecretKey: []byte("faketoken"), + }, + } + err := k8sClient.Create(ctx, &tokenSecret) + Expect(err).NotTo(HaveOccurred()) + provisioner := crdv1.NfsProvisioner{ TypeMeta: metav1.TypeMeta{ Kind: "NfsProvisioner", @@ -122,7 +137,10 @@ var _ = Describe("NfsProvisioner Reconciler", func() { Namespace: DefaultNamespace, }, Spec: crdv1.NfsProvisionerSpec{ - APIToken: "faketoken", + APITokenSecretRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: tokenSecret.Name}, + Key: crdv1.DefaultAPITokenSecretKey, + }, APIURL: "http://127.0.0.1", RegionID: 2, ProjectID: 5, @@ -131,7 +149,7 @@ var _ = Describe("NfsProvisioner Reconciler", func() { ImageVersion: "v4.0.2", }, } - err := k8sClient.Create(ctx, &provisioner) + err = k8sClient.Create(ctx, &provisioner) Expect(err).NotTo(HaveOccurred()) helmClient, err := gohelmclient.NewClientFromRestConf( diff --git a/pkg/gcoreclient/client.go b/pkg/gcoreclient/client.go index e136943..955d2fe 100644 --- a/pkg/gcoreclient/client.go +++ b/pkg/gcoreclient/client.go @@ -12,15 +12,15 @@ import ( const NfsProtocolName = "nfs" type FileShareLister interface { - ListFileShares(provisioner *crdv1.NfsProvisioner) ([]file_shares.FileShare, error) + ListFileShares(provisioner *crdv1.NfsProvisioner, apiToken string) ([]file_shares.FileShare, error) } type FileShareClient struct{} -func newApiTokenClient(provisioner *crdv1.NfsProvisioner, endpoint string, version string) (*gcorecloud.ServiceClient, error) { +func newApiTokenClient(provisioner *crdv1.NfsProvisioner, apiToken string, endpoint string, version string) (*gcorecloud.ServiceClient, error) { settings := gcorecloud.APITokenAPISettings{ APIURL: provisioner.Spec.APIURL, - APIToken: provisioner.Spec.APIToken, + APIToken: apiToken, Type: "", Name: endpoint, Region: provisioner.Spec.RegionID, @@ -31,8 +31,8 @@ func newApiTokenClient(provisioner *crdv1.NfsProvisioner, endpoint string, versi return cloudclient.APITokenClientServiceWithDebug(settings.ToAPITokenOptions(), settings.ToEndpointOptions(), settings.Debug) } -func (c FileShareClient) ListFileShares(provisioner *crdv1.NfsProvisioner) ([]file_shares.FileShare, error) { - fileShareClient, err := newApiTokenClient(provisioner, "file_shares", "v1") +func (c FileShareClient) ListFileShares(provisioner *crdv1.NfsProvisioner, apiToken string) ([]file_shares.FileShare, error) { + fileShareClient, err := newApiTokenClient(provisioner, apiToken, "file_shares", "v1") if err != nil { return []file_shares.FileShare{}, err } @@ -53,6 +53,6 @@ type MockFileShareClient struct { FileShares []file_shares.FileShare } -func (m MockFileShareClient) ListFileShares(provisioner *crdv1.NfsProvisioner) ([]file_shares.FileShare, error) { +func (m MockFileShareClient) ListFileShares(provisioner *crdv1.NfsProvisioner, apiToken string) ([]file_shares.FileShare, error) { return m.FileShares, nil }