From 00ff6d601e552409d6519df455d40cd11b697b49 Mon Sep 17 00:00:00 2001 From: Rui Fu Date: Mon, 18 May 2026 22:44:31 +0800 Subject: [PATCH 1/3] feat(secret): add binary data support --- api/v1alpha1/secret_types.go | 16 +- api/v1alpha1/zz_generated.deepcopy.go | 14 +- .../resource.streamnative.io_secrets.yaml | 20 ++- .../resource.streamnative.io_secrets.yaml | 20 ++- config/samples/resource_v1alpha1_secret.yaml | 2 + controllers/secret_controller.go | 78 ++++++--- controllers/secret_controller_test.go | 124 ++++++++++++++ docs/secret.md | 54 +++++- .../apis/cloud/v1alpha1/secret_types.go | 3 + .../cloud/v1alpha1/zz_generated.deepcopy.go | 7 + pkg/streamnativecloud/secret_client.go | 3 + pkg/streamnativecloud/secret_client_test.go | 156 ++++++++++++++++++ 12 files changed, 460 insertions(+), 37 deletions(-) create mode 100644 controllers/secret_controller_test.go create mode 100644 pkg/streamnativecloud/secret_client_test.go diff --git a/api/v1alpha1/secret_types.go b/api/v1alpha1/secret_types.go index df5927d6..622f7523 100644 --- a/api/v1alpha1/secret_types.go +++ b/api/v1alpha1/secret_types.go @@ -43,13 +43,17 @@ type SecretSpec struct { // +optional Location string `json:"location"` - // the value should be base64 encoded + // Data contains text secret data. // +optional Data map[string]string `json:"data,omitempty"` - // SecretRef is the reference to the kubernetes secret + // BinaryData contains binary secret data. Values must be base64-encoded raw bytes. + // +optional + BinaryData map[string]string `json:"binaryData,omitempty"` + + // SecretRef is the reference to the kubernetes secret. // When SecretRef is set, it will be used to fetch the secret data. - // Data will be ignored. + // Data and BinaryData values set directly on this resource take precedence. // +optional SecretRef *KubernetesSecretReference `json:"secretRef,omitempty"` @@ -110,6 +114,12 @@ func (r PoolMemberReference) ToNamespacedName() types.NamespacedName { type KubernetesSecretReference struct { Namespace string `json:"namespace" protobuf:"bytes,1,opt,name=namespace"` Name string `json:"name" protobuf:"bytes,2,opt,name=name"` + + // BinaryDataKeys are keys from the referenced Kubernetes Secret data that should be sent + // to StreamNative Cloud as binaryData. Other keys keep the existing text data behavior. + // +optional + // +listType=set + BinaryDataKeys []string `json:"binaryDataKeys,omitempty" protobuf:"bytes,3,rep,name=binaryDataKeys"` } func (r KubernetesSecretReference) ToNamespacedName() types.NamespacedName { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 8db33293..548a9858 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -991,6 +991,11 @@ func (in *InactiveTopicPolicies) DeepCopy() *InactiveTopicPolicies { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KubernetesSecretReference) DeepCopyInto(out *KubernetesSecretReference) { *out = *in + if in.BinaryDataKeys != nil { + in, out := &in.BinaryDataKeys, &out.BinaryDataKeys + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubernetesSecretReference. @@ -3501,10 +3506,17 @@ func (in *SecretSpec) DeepCopyInto(out *SecretSpec) { (*out)[key] = val } } + if in.BinaryData != nil { + in, out := &in.BinaryData, &out.BinaryData + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.SecretRef != nil { in, out := &in.SecretRef, &out.SecretRef *out = new(KubernetesSecretReference) - **out = **in + (*in).DeepCopyInto(*out) } if in.PoolMemberName != nil { in, out := &in.PoolMemberName, &out.PoolMemberName diff --git a/charts/pulsar-resources-operator/crds/resource.streamnative.io_secrets.yaml b/charts/pulsar-resources-operator/crds/resource.streamnative.io_secrets.yaml index 069223e1..bb0298ac 100644 --- a/charts/pulsar-resources-operator/crds/resource.streamnative.io_secrets.yaml +++ b/charts/pulsar-resources-operator/crds/resource.streamnative.io_secrets.yaml @@ -80,10 +80,16 @@ spec: type: string type: object x-kubernetes-map-type: atomic + binaryData: + additionalProperties: + type: string + description: BinaryData contains binary secret data. Values must be + base64-encoded raw bytes. + type: object data: additionalProperties: type: string - description: the value should be base64 encoded + description: Data contains text secret data. type: object instanceName: description: InstanceName is the name of the instance this secret @@ -105,10 +111,18 @@ spec: type: string secretRef: description: |- - SecretRef is the reference to the kubernetes secret + SecretRef is the reference to the kubernetes secret. When SecretRef is set, it will be used to fetch the secret data. - Data will be ignored. + Data and BinaryData values set directly on this resource take precedence. properties: + binaryDataKeys: + description: |- + BinaryDataKeys are keys from the referenced Kubernetes Secret data that should be sent + to StreamNative Cloud as binaryData. Other keys keep the existing text data behavior. + items: + type: string + type: array + x-kubernetes-list-type: set name: type: string namespace: diff --git a/config/crd/bases/resource.streamnative.io_secrets.yaml b/config/crd/bases/resource.streamnative.io_secrets.yaml index 069223e1..bb0298ac 100644 --- a/config/crd/bases/resource.streamnative.io_secrets.yaml +++ b/config/crd/bases/resource.streamnative.io_secrets.yaml @@ -80,10 +80,16 @@ spec: type: string type: object x-kubernetes-map-type: atomic + binaryData: + additionalProperties: + type: string + description: BinaryData contains binary secret data. Values must be + base64-encoded raw bytes. + type: object data: additionalProperties: type: string - description: the value should be base64 encoded + description: Data contains text secret data. type: object instanceName: description: InstanceName is the name of the instance this secret @@ -105,10 +111,18 @@ spec: type: string secretRef: description: |- - SecretRef is the reference to the kubernetes secret + SecretRef is the reference to the kubernetes secret. When SecretRef is set, it will be used to fetch the secret data. - Data will be ignored. + Data and BinaryData values set directly on this resource take precedence. properties: + binaryDataKeys: + description: |- + BinaryDataKeys are keys from the referenced Kubernetes Secret data that should be sent + to StreamNative Cloud as binaryData. Other keys keep the existing text data behavior. + items: + type: string + type: array + x-kubernetes-list-type: set name: type: string namespace: diff --git a/config/samples/resource_v1alpha1_secret.yaml b/config/samples/resource_v1alpha1_secret.yaml index 150171e2..30ea2e2e 100644 --- a/config/samples/resource_v1alpha1_secret.yaml +++ b/config/samples/resource_v1alpha1_secret.yaml @@ -23,5 +23,7 @@ spec: type: Opaque data: key: value + binaryData: + keystore.p12: AAEC/w== location: "useast1" instanceName: "test-instance" diff --git a/controllers/secret_controller.go b/controllers/secret_controller.go index e9147a4c..7fe2253b 100644 --- a/controllers/secret_controller.go +++ b/controllers/secret_controller.go @@ -16,6 +16,7 @@ package controllers import ( "context" + "encoding/base64" "fmt" "time" @@ -49,29 +50,52 @@ type SecretReconciler struct { //+kubebuilder:rbac:groups=resource.streamnative.io,resources=streamnativecloudconnections,verbs=get;list;watch //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch -// getSecretData obtains the Secret data either from direct Data field or from SecretRef -func (r *SecretReconciler) getSecretData(ctx context.Context, secretCR *resourcev1alpha1.Secret) (map[string]string, *corev1.SecretType, error) { - // If direct data is provided, use it - if len(secretCR.Spec.Data) > 0 { - return secretCR.Spec.Data, secretCR.Spec.Type, nil +// resolveSecretRefData obtains Secret data from a referenced Kubernetes Secret. +func (r *SecretReconciler) resolveSecretRefData( + ctx context.Context, + secretCR *resourcev1alpha1.Secret, +) (map[string]string, map[string]string, *corev1.SecretType, error) { + if secretCR.Spec.SecretRef == nil { + return nil, nil, nil, fmt.Errorf("SecretRef is not specified in the Secret spec") } - // If SecretRef is provided, fetch from the referenced Kubernetes Secret - if secretCR.Spec.SecretRef != nil { - nsName := secretCR.Spec.SecretRef.ToNamespacedName() - k8sSecret := &corev1.Secret{} - if err := r.Get(ctx, nsName, k8sSecret); err != nil { - return nil, nil, fmt.Errorf("failed to get referenced Secret %s/%s: %w", nsName.Namespace, nsName.Name, err) + nsName := secretCR.Spec.SecretRef.ToNamespacedName() + k8sSecret := &corev1.Secret{} + if err := r.Get(ctx, nsName, k8sSecret); err != nil { + return nil, nil, nil, fmt.Errorf("failed to get referenced Secret %s/%s: %w", nsName.Namespace, nsName.Name, err) + } + + binaryDataKeys := make(map[string]struct{}, len(secretCR.Spec.SecretRef.BinaryDataKeys)) + for _, key := range secretCR.Spec.SecretRef.BinaryDataKeys { + binaryDataKeys[key] = struct{}{} + } + + stringData := make(map[string]string) + binaryData := make(map[string]string) + for key, value := range k8sSecret.Data { + if _, ok := binaryDataKeys[key]; ok { + binaryData[key] = base64.StdEncoding.EncodeToString(value) + continue } + stringData[key] = string(value) + } - stringData := make(map[string]string) - for k, v := range k8sSecret.Data { - stringData[k] = string(v) + for key := range binaryDataKeys { + if _, ok := k8sSecret.Data[key]; !ok { + return nil, nil, nil, fmt.Errorf("binaryData key %q not found in referenced Secret %s/%s", key, nsName.Namespace, nsName.Name) } - return stringData, &k8sSecret.Type, nil } - return nil, nil, fmt.Errorf("neither Data nor SecretRef is specified in the Secret spec") + return stringData, binaryData, &k8sSecret.Type, nil +} + +func validateSecretDataKeys(secretCR *resourcev1alpha1.Secret) error { + for key := range secretCR.Spec.Data { + if _, ok := secretCR.Spec.BinaryData[key]; ok { + return fmt.Errorf("secret data key %q is set in both spec.data and spec.binaryData", key) + } + } + return nil } // Reconcile handles the reconciliation of Secret objects @@ -102,6 +126,11 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err // No requeue for permanent misconfiguration } + if err := validateSecretDataKeys(secretCR); err != nil { + r.updateSecretStatus(ctx, secretCR, err, "ValidationFailed", err.Error()) + return ctrl.Result{}, err + } + // Get StreamNativeCloudConnection connection := &resourcev1alpha1.StreamNativeCloudConnection{} connErr := r.Get(ctx, types.NamespacedName{ @@ -183,18 +212,22 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{Requeue: true}, nil } - // Resolve secret data from Spec.Data or Spec.SecretRef - // The secretCR passed to cloud client methods should have its Spec.Data and Spec.Type populated. + // Resolve secret data from Spec.Data, Spec.BinaryData, or Spec.SecretRef. + // The secretCR passed to cloud client methods should have its Spec.Data, Spec.BinaryData, and Spec.Type populated. currentSpecData := make(map[string]string) for k, v := range secretCR.Spec.Data { currentSpecData[k] = v } + currentSpecBinaryData := make(map[string]string) + for k, v := range secretCR.Spec.BinaryData { + currentSpecBinaryData[k] = v + } currentSpecType := secretCR.Spec.Type // If SecretRef is used, we resolve it and update the CR if necessary. // This ensures that the CR in etcd reflects the data being sent to the cloud API. if secretCR.Spec.SecretRef != nil { - resolvedData, resolvedType, err := r.getSecretData(ctx, secretCR) + resolvedData, resolvedBinaryData, resolvedType, err := r.resolveSecretRefData(ctx, secretCR) if err != nil { r.updateSecretStatus(ctx, secretCR, err, "GetSecretDataFailed", fmt.Sprintf("Failed to get secret data from SecretRef: %v", err)) return ctrl.Result{}, err @@ -202,10 +235,14 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Check if an update to the local CR is needed updateLocalCR := false - if len(secretCR.Spec.Data) == 0 { // Only populate from SecretRef if direct data is not set + if len(secretCR.Spec.Data) == 0 && len(resolvedData) > 0 { // Only populate from SecretRef if direct data is not set secretCR.Spec.Data = resolvedData updateLocalCR = true } + if len(secretCR.Spec.BinaryData) == 0 && len(resolvedBinaryData) > 0 { // Only populate from SecretRef if direct binaryData is not set + secretCR.Spec.BinaryData = resolvedBinaryData + updateLocalCR = true + } // Only update type if original spec type was nil or empty, and resolvedType is not nil if (secretCR.Spec.Type == nil || *secretCR.Spec.Type == "") && resolvedType != nil { secretCR.Spec.Type = resolvedType @@ -216,6 +253,7 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr if err := r.Update(ctx, secretCR); err != nil { // Restore original spec data before status update to avoid inconsistent state reporting secretCR.Spec.Data = currentSpecData + secretCR.Spec.BinaryData = currentSpecBinaryData secretCR.Spec.Type = currentSpecType r.updateSecretStatus(ctx, secretCR, err, "UpdateLocalSecretFailed", fmt.Sprintf("Failed to update local Secret CR with resolved data: %v", err)) diff --git a/controllers/secret_controller_test.go b/controllers/secret_controller_test.go new file mode 100644 index 00000000..bb685588 --- /dev/null +++ b/controllers/secret_controller_test.go @@ -0,0 +1,124 @@ +// Copyright 2025 StreamNative +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controllers + +import ( + "context" + "testing" + + resourcev1alpha1 "github.com/streamnative/pulsar-resources-operator/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestResolveSecretRefDataEncodesSelectedBinaryDataKeys(t *testing.T) { + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("add core scheme: %v", err) + } + if err := resourcev1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("add resource scheme: %v", err) + } + + k8sSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "source-secret", + }, + Type: corev1.SecretTypeOpaque, + Data: map[string][]byte{ + "username": []byte("admin"), + "keystore.p12": {0x00, 0x01, 0x02, 0xff}, + }, + } + secretCR := &resourcev1alpha1.Secret{ + Spec: resourcev1alpha1.SecretSpec{ + SecretRef: &resourcev1alpha1.KubernetesSecretReference{ + Namespace: "default", + Name: "source-secret", + BinaryDataKeys: []string{"keystore.p12"}, + }, + }, + } + + reconciler := &SecretReconciler{ + Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(k8sSecret).Build(), + } + + data, binaryData, secretType, err := reconciler.resolveSecretRefData(context.Background(), secretCR) + if err != nil { + t.Fatalf("resolve SecretRef data: %v", err) + } + if got := data["username"]; got != "admin" { + t.Fatalf("resolved data username = %q, want admin", got) + } + if _, ok := data["keystore.p12"]; ok { + t.Fatalf("keystore.p12 should not be resolved as text data") + } + if got := binaryData["keystore.p12"]; got != "AAEC/w==" { + t.Fatalf("resolved binaryData keystore.p12 = %q, want AAEC/w==", got) + } + if secretType == nil || *secretType != corev1.SecretTypeOpaque { + t.Fatalf("resolved type = %v, want Opaque", secretType) + } +} + +func TestResolveSecretRefDataErrorsForMissingBinaryDataKey(t *testing.T) { + scheme := runtime.NewScheme() + if err := corev1.AddToScheme(scheme); err != nil { + t.Fatalf("add core scheme: %v", err) + } + if err := resourcev1alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("add resource scheme: %v", err) + } + + k8sSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "source-secret"}, + Data: map[string][]byte{"username": []byte("admin")}, + } + secretCR := &resourcev1alpha1.Secret{ + Spec: resourcev1alpha1.SecretSpec{ + SecretRef: &resourcev1alpha1.KubernetesSecretReference{ + Namespace: "default", + Name: "source-secret", + BinaryDataKeys: []string{"missing.p12"}, + }, + }, + } + + reconciler := &SecretReconciler{ + Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(k8sSecret).Build(), + } + + _, _, _, err := reconciler.resolveSecretRefData(context.Background(), secretCR) + if err == nil { + t.Fatal("expected missing binaryData key error") + } +} + +func TestValidateSecretDataKeysRejectsDuplicates(t *testing.T) { + secretCR := &resourcev1alpha1.Secret{ + Spec: resourcev1alpha1.SecretSpec{ + Data: map[string]string{"shared": "text"}, + BinaryData: map[string]string{"shared": "AAEC/w=="}, + }, + } + + if err := validateSecretDataKeys(secretCR); err == nil { + t.Fatal("expected duplicate key validation error") + } +} diff --git a/docs/secret.md b/docs/secret.md index c394a4d6..8cd5280d 100644 --- a/docs/secret.md +++ b/docs/secret.md @@ -12,13 +12,14 @@ The `Secret` resource defines a secret in StreamNative Cloud. It allows you to c | `lifecyclePolicy` | Whether to delete the remote secret or keep it when the Kubernetes resource is deleted. Defaults to cleanup when omitted. | No | | `instanceName` | Name of the instance this secret is for (e.g. pulsar-instance) | No | | `location` | Location of the secret | No | -| `data` | Secret data, values should be base64 encoded | No* | -| `secretRef` | Reference to a Kubernetes secret. When secretRef is set, it will be used to fetch the secret data, and data field will be ignored | No* | +| `data` | Text secret data | No* | +| `binaryData` | Binary secret data. Values must be base64-encoded raw bytes | No* | +| `secretRef` | Reference to a Kubernetes secret. When secretRef is set, it will be used to fetch secret data. Direct `data` and `binaryData` values take precedence | No* | | `poolMemberName` | Pool member to deploy the secret | No | | `tolerations` | Tolerations for the secret | No | | `type` | Used to facilitate programmatic handling of secret data | No | -*Note: Either `data` or `secretRef` must be specified. +*Note: Specify at least one of `data`, `binaryData`, or `secretRef`. ### KubernetesSecretReference Structure @@ -26,6 +27,7 @@ The `Secret` resource defines a secret in StreamNative Cloud. It allows you to c |-------|-------------|----------| | `namespace` | Namespace of the Kubernetes secret | Yes | | `name` | Name of the Kubernetes secret | Yes | +| `binaryDataKeys` | Keys from the referenced Kubernetes Secret `.data` map that should be sent as Cloud `binaryData`. All other referenced keys keep the existing text `data` behavior | No | ### Toleration Structure @@ -62,7 +64,24 @@ spec: location: us-central1 ``` -2. Create a Secret resource with Kubernetes Secret reference: +2. Create a Secret resource with binary data. The value is base64-encoded raw bytes: + +```yaml +apiVersion: resource.streamnative.io/v1alpha1 +kind: Secret +metadata: + name: resource-operator-secret-binary + namespace: default +spec: + apiServerRef: + name: test-connection + binaryData: + keystore.p12: AAEC/w== + instanceName: wstest + location: us-central1 +``` + +3. Create a Secret resource with Kubernetes Secret reference: ```yaml apiVersion: resource.streamnative.io/v1alpha1 @@ -80,13 +99,33 @@ spec: location: us-central1 ``` -3. Apply the YAML file: +4. Create a Secret resource that sends selected referenced Kubernetes Secret bytes as binary data: + +```yaml +apiVersion: resource.streamnative.io/v1alpha1 +kind: Secret +metadata: + name: resource-operator-secret-binary-ref + namespace: default +spec: + apiServerRef: + name: test-connection + secretRef: + name: certificate-secret + namespace: default + binaryDataKeys: + - keystore.p12 + instanceName: wstest + location: us-central1 +``` + +5. Apply the YAML file: ```shell kubectl apply -f secret.yaml ``` -4. Check the secret status: +6. Check the secret status: ```shell kubectl get secret.resource.streamnative.io resource-operator-secret @@ -102,7 +141,8 @@ resource-operator-secret True 1m ## Update Secret You can update the secret by modifying the YAML file and reapplying it. Most fields can be updated, including: -- Secret data +- Secret text data +- Secret binary data - Kubernetes secret reference - Tolerations diff --git a/pkg/streamnativecloud/apis/cloud/v1alpha1/secret_types.go b/pkg/streamnativecloud/apis/cloud/v1alpha1/secret_types.go index f8a92850..0c8e81ab 100644 --- a/pkg/streamnativecloud/apis/cloud/v1alpha1/secret_types.go +++ b/pkg/streamnativecloud/apis/cloud/v1alpha1/secret_types.go @@ -44,6 +44,9 @@ type Secret struct { // the value should be base64 encoded Data map[string]string `json:"data,omitempty" protobuf:"bytes,6,opt,name=data"` + // BinaryData contains binary secret data. Values must be base64-encoded raw bytes. + BinaryData map[string]string `json:"binaryData,omitempty" protobuf:"bytes,10,opt,name=binaryData"` + // PoolMemberRef is the pool member to deploy the secret. // admission controller will infer this information automatically // +optional diff --git a/pkg/streamnativecloud/apis/cloud/v1alpha1/zz_generated.deepcopy.go b/pkg/streamnativecloud/apis/cloud/v1alpha1/zz_generated.deepcopy.go index 4c80ec5a..2044666b 100644 --- a/pkg/streamnativecloud/apis/cloud/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/streamnativecloud/apis/cloud/v1alpha1/zz_generated.deepcopy.go @@ -841,6 +841,13 @@ func (in *Secret) DeepCopyInto(out *Secret) { (*out)[key] = val } } + if in.BinaryData != nil { + in, out := &in.BinaryData, &out.BinaryData + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.PoolMemberRef != nil { in, out := &in.PoolMemberRef, &out.PoolMemberRef *out = new(PoolMemberReference) diff --git a/pkg/streamnativecloud/secret_client.go b/pkg/streamnativecloud/secret_client.go index 35a8464b..e1fde7c6 100644 --- a/pkg/streamnativecloud/secret_client.go +++ b/pkg/streamnativecloud/secret_client.go @@ -93,6 +93,9 @@ func convertToCloudSecret(secret *resourcev1alpha1.Secret) *cloudapi.Secret { if secret.Spec.Data != nil { cloudSecret.Data = secret.Spec.Data } + if secret.Spec.BinaryData != nil { + cloudSecret.BinaryData = secret.Spec.BinaryData + } if secret.Spec.InstanceName != "" { cloudSecret.InstanceName = secret.Spec.InstanceName } diff --git a/pkg/streamnativecloud/secret_client_test.go b/pkg/streamnativecloud/secret_client_test.go new file mode 100644 index 00000000..cfbcf0e8 --- /dev/null +++ b/pkg/streamnativecloud/secret_client_test.go @@ -0,0 +1,156 @@ +// Copyright 2025 StreamNative +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package streamnativecloud + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + resourcev1alpha1 "github.com/streamnative/pulsar-resources-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func newTestSecretClient(t *testing.T, handler http.Handler) *SecretClient { + t.Helper() + + server := httptest.NewServer(handler) + t.Cleanup(server.Close) + + httpClient := server.Client() + if httpClient.Transport == nil { + httpClient.Transport = http.DefaultTransport + } + apiConn := &APIConnection{ + config: &resourcev1alpha1.StreamNativeCloudConnection{ + Spec: resourcev1alpha1.StreamNativeCloudConnectionSpec{Server: server.URL}, + }, + client: httpClient, + initialized: true, + } + secretClient, err := NewSecretClient(apiConn, "test-org") + if err != nil { + t.Fatalf("create secret client: %v", err) + } + return secretClient +} + +func newSecretWithBinaryData() *resourcev1alpha1.Secret { + return &resourcev1alpha1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "test-secret"}, + Spec: resourcev1alpha1.SecretSpec{ + Data: map[string]string{ + "username": "admin", + }, + BinaryData: map[string]string{ + "keystore.p12": "AAEC/w==", + }, + }, + } +} + +func assertSecretPayloadHasBinaryData(t *testing.T, payload map[string]any) { + t.Helper() + + data, ok := payload["data"].(map[string]any) + if !ok { + t.Fatalf("payload data = %#v, want object", payload["data"]) + } + if data["username"] != "admin" { + t.Fatalf("payload data username = %#v, want admin", data["username"]) + } + + binaryData, ok := payload["binaryData"].(map[string]any) + if !ok { + t.Fatalf("payload binaryData = %#v, want object", payload["binaryData"]) + } + if binaryData["keystore.p12"] != "AAEC/w==" { + t.Fatalf("payload binaryData keystore.p12 = %#v, want AAEC/w==", binaryData["keystore.p12"]) + } +} + +func TestConvertToCloudSecretIncludesBinaryData(t *testing.T) { + cloudSecret := convertToCloudSecret(newSecretWithBinaryData()) + if got := cloudSecret.Data["username"]; got != "admin" { + t.Fatalf("cloud data username = %q, want admin", got) + } + if got := cloudSecret.BinaryData["keystore.p12"]; got != "AAEC/w==" { + t.Fatalf("cloud binaryData keystore.p12 = %q, want AAEC/w==", got) + } + + payload, err := json.Marshal(cloudSecret) + if err != nil { + t.Fatalf("marshal cloud secret: %v", err) + } + + var decoded struct { + Data map[string]string `json:"data"` + BinaryData map[string]string `json:"binaryData"` + } + if err := json.Unmarshal(payload, &decoded); err != nil { + t.Fatalf("unmarshal cloud secret payload: %v", err) + } + if decoded.Data["username"] != "admin" { + t.Fatalf("payload data username = %q, want admin", decoded.Data["username"]) + } + if decoded.BinaryData["keystore.p12"] != "AAEC/w==" { + t.Fatalf("payload binaryData keystore.p12 = %q, want AAEC/w==", decoded.BinaryData["keystore.p12"]) + } +} + +func TestCreateSecretPayloadIncludesBinaryData(t *testing.T) { + var payload map[string]any + secretClient := newTestSecretClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/apis/cloud.streamnative.io/v1alpha1/namespaces/test-org/secrets" { + t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + } + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Fatalf("decode create request body: %v", err) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"apiVersion":"cloud.streamnative.io/v1alpha1","kind":"Secret","metadata":{"name":"test-secret"}}`)) + })) + + if _, err := secretClient.CreateSecret(context.Background(), newSecretWithBinaryData()); err != nil { + t.Fatalf("create secret: %v", err) + } + assertSecretPayloadHasBinaryData(t, payload) +} + +func TestUpdateSecretPayloadIncludesBinaryData(t *testing.T) { + var payload map[string]any + secretClient := newTestSecretClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.Method == http.MethodGet && r.URL.Path == "/apis/cloud.streamnative.io/v1alpha1/namespaces/test-org/secrets/test-secret": + _, _ = w.Write([]byte(`{"apiVersion":"cloud.streamnative.io/v1alpha1","kind":"Secret","metadata":{"name":"test-secret","resourceVersion":"123"}}`)) + case r.Method == http.MethodPut && r.URL.Path == "/apis/cloud.streamnative.io/v1alpha1/namespaces/test-org/secrets/test-secret": + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Fatalf("decode update request body: %v", err) + } + _, _ = w.Write([]byte(`{"apiVersion":"cloud.streamnative.io/v1alpha1","kind":"Secret","metadata":{"name":"test-secret","resourceVersion":"124"}}`)) + default: + t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path) + } + })) + + if _, err := secretClient.UpdateSecret(context.Background(), newSecretWithBinaryData()); err != nil { + t.Fatalf("update secret: %v", err) + } + assertSecretPayloadHasBinaryData(t, payload) +} From 8677398940ee9f9bee4f19af8bfadfbef7d1e96d Mon Sep 17 00:00:00 2001 From: Rui Fu Date: Mon, 18 May 2026 22:45:13 +0800 Subject: [PATCH 2/3] chore: add PLAN.md to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index b23a510c..16594764 100644 --- a/.gitignore +++ b/.gitignore @@ -53,3 +53,4 @@ node_modules/ .cursor .envrc mise.toml +/PLAN.md From bdf450b19697106088f377a78a107a63f2f32d28 Mon Sep 17 00:00:00 2001 From: Rui Fu Date: Tue, 19 May 2026 11:13:12 +0800 Subject: [PATCH 3/3] fix(secret): address binary data review comments --- api/v1alpha1/secret_types.go | 1 + .../resource.streamnative.io_secrets.yaml | 3 + .../resource.streamnative.io_secrets.yaml | 3 + controllers/secret_controller.go | 98 ++++++++++--------- controllers/secret_controller_test.go | 67 ++++++++++++- docs/secret.md | 2 +- 6 files changed, 125 insertions(+), 49 deletions(-) diff --git a/api/v1alpha1/secret_types.go b/api/v1alpha1/secret_types.go index 622f7523..e987ca86 100644 --- a/api/v1alpha1/secret_types.go +++ b/api/v1alpha1/secret_types.go @@ -48,6 +48,7 @@ type SecretSpec struct { Data map[string]string `json:"data,omitempty"` // BinaryData contains binary secret data. Values must be base64-encoded raw bytes. + // +kubebuilder:validation:XValidation:rule="self.all(k, self[k].matches('^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$'))",message="binaryData values must be valid base64" // +optional BinaryData map[string]string `json:"binaryData,omitempty"` diff --git a/charts/pulsar-resources-operator/crds/resource.streamnative.io_secrets.yaml b/charts/pulsar-resources-operator/crds/resource.streamnative.io_secrets.yaml index bb0298ac..d1bc2905 100644 --- a/charts/pulsar-resources-operator/crds/resource.streamnative.io_secrets.yaml +++ b/charts/pulsar-resources-operator/crds/resource.streamnative.io_secrets.yaml @@ -86,6 +86,9 @@ spec: description: BinaryData contains binary secret data. Values must be base64-encoded raw bytes. type: object + x-kubernetes-validations: + - message: binaryData values must be valid base64 + rule: self.all(k, self[k].matches('^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$')) data: additionalProperties: type: string diff --git a/config/crd/bases/resource.streamnative.io_secrets.yaml b/config/crd/bases/resource.streamnative.io_secrets.yaml index bb0298ac..d1bc2905 100644 --- a/config/crd/bases/resource.streamnative.io_secrets.yaml +++ b/config/crd/bases/resource.streamnative.io_secrets.yaml @@ -86,6 +86,9 @@ spec: description: BinaryData contains binary secret data. Values must be base64-encoded raw bytes. type: object + x-kubernetes-validations: + - message: binaryData values must be valid base64 + rule: self.all(k, self[k].matches('^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$')) data: additionalProperties: type: string diff --git a/controllers/secret_controller.go b/controllers/secret_controller.go index 7fe2253b..ddfc32a3 100644 --- a/controllers/secret_controller.go +++ b/controllers/secret_controller.go @@ -89,15 +89,58 @@ func (r *SecretReconciler) resolveSecretRefData( return stringData, binaryData, &k8sSecret.Type, nil } -func validateSecretDataKeys(secretCR *resourcev1alpha1.Secret) error { +func validateSecretData(secretCR *resourcev1alpha1.Secret) error { for key := range secretCR.Spec.Data { if _, ok := secretCR.Spec.BinaryData[key]; ok { return fmt.Errorf("secret data key %q is set in both spec.data and spec.binaryData", key) } } + for key, value := range secretCR.Spec.BinaryData { + if _, err := base64.StdEncoding.DecodeString(value); err != nil { + return fmt.Errorf("secret binaryData key %q must contain valid base64: %w", key, err) + } + } return nil } +func copyStringMap(in map[string]string) map[string]string { + if len(in) == 0 { + return nil + } + out := make(map[string]string, len(in)) + for key, value := range in { + out[key] = value + } + return out +} + +func applyResolvedSecretRefData( + secretCR *resourcev1alpha1.Secret, + resolvedData map[string]string, + resolvedBinaryData map[string]string, + resolvedType *corev1.SecretType, +) { + directData := copyStringMap(secretCR.Spec.Data) + directBinaryData := copyStringMap(secretCR.Spec.BinaryData) + mergedData := copyStringMap(resolvedData) + mergedBinaryData := copyStringMap(resolvedBinaryData) + + for key, value := range directData { + mergedData[key] = value + delete(mergedBinaryData, key) + } + for key, value := range directBinaryData { + mergedBinaryData[key] = value + delete(mergedData, key) + } + + secretCR.Spec.Data = mergedData + secretCR.Spec.BinaryData = mergedBinaryData + if (secretCR.Spec.Type == nil || *secretCR.Spec.Type == "") && resolvedType != nil { + secretCR.Spec.Type = resolvedType + } +} + // Reconcile handles the reconciliation of Secret objects func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := log.FromContext(ctx) @@ -126,7 +169,7 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err // No requeue for permanent misconfiguration } - if err := validateSecretDataKeys(secretCR); err != nil { + if err := validateSecretData(secretCR); err != nil { r.updateSecretStatus(ctx, secretCR, err, "ValidationFailed", err.Error()) return ctrl.Result{}, err } @@ -212,20 +255,9 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{Requeue: true}, nil } - // Resolve secret data from Spec.Data, Spec.BinaryData, or Spec.SecretRef. - // The secretCR passed to cloud client methods should have its Spec.Data, Spec.BinaryData, and Spec.Type populated. - currentSpecData := make(map[string]string) - for k, v := range secretCR.Spec.Data { - currentSpecData[k] = v - } - currentSpecBinaryData := make(map[string]string) - for k, v := range secretCR.Spec.BinaryData { - currentSpecBinaryData[k] = v - } - currentSpecType := secretCR.Spec.Type - - // If SecretRef is used, we resolve it and update the CR if necessary. - // This ensures that the CR in etcd reflects the data being sent to the cloud API. + // Resolve data from SecretRef into an effective copy used for cloud sync. + // Do not persist resolved data back to the local CR; referenced Secret rotations must be reflected every reconcile. + effectiveSecretCR := secretCR if secretCR.Spec.SecretRef != nil { resolvedData, resolvedBinaryData, resolvedType, err := r.resolveSecretRefData(ctx, secretCR) if err != nil { @@ -233,34 +265,8 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } - // Check if an update to the local CR is needed - updateLocalCR := false - if len(secretCR.Spec.Data) == 0 && len(resolvedData) > 0 { // Only populate from SecretRef if direct data is not set - secretCR.Spec.Data = resolvedData - updateLocalCR = true - } - if len(secretCR.Spec.BinaryData) == 0 && len(resolvedBinaryData) > 0 { // Only populate from SecretRef if direct binaryData is not set - secretCR.Spec.BinaryData = resolvedBinaryData - updateLocalCR = true - } - // Only update type if original spec type was nil or empty, and resolvedType is not nil - if (secretCR.Spec.Type == nil || *secretCR.Spec.Type == "") && resolvedType != nil { - secretCR.Spec.Type = resolvedType - updateLocalCR = true - } - - if updateLocalCR { - if err := r.Update(ctx, secretCR); err != nil { - // Restore original spec data before status update to avoid inconsistent state reporting - secretCR.Spec.Data = currentSpecData - secretCR.Spec.BinaryData = currentSpecBinaryData - secretCR.Spec.Type = currentSpecType - r.updateSecretStatus(ctx, secretCR, err, "UpdateLocalSecretFailed", - fmt.Sprintf("Failed to update local Secret CR with resolved data: %v", err)) - return ctrl.Result{}, err - } - return ctrl.Result{Requeue: true}, nil // Requeue to use the updated CR - } + effectiveSecretCR = secretCR.DeepCopy() + applyResolvedSecretRefData(effectiveSecretCR, resolvedData, resolvedBinaryData, resolvedType) } existingRemoteSecret, err := secretClient.GetSecret(ctx, secretCR.Name) @@ -273,13 +279,13 @@ func (r *SecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } if existingRemoteSecret == nil { - if _, err := secretClient.CreateSecret(ctx, secretCR); err != nil { + if _, err := secretClient.CreateSecret(ctx, effectiveSecretCR); err != nil { r.updateSecretStatus(ctx, secretCR, err, "CreateRemoteSecretFailed", fmt.Sprintf("Failed to create remote Secret: %v", err)) return ctrl.Result{}, err } r.updateSecretStatus(ctx, secretCR, nil, "Ready", "Secret created and synced successfully") } else { - if _, err := secretClient.UpdateSecret(ctx, secretCR); err != nil { // Pass the K8s CR + if _, err := secretClient.UpdateSecret(ctx, effectiveSecretCR); err != nil { // Pass the effective K8s CR r.updateSecretStatus(ctx, secretCR, err, "UpdateRemoteSecretFailed", fmt.Sprintf("Failed to update remote Secret: %v", err)) return ctrl.Result{}, err } diff --git a/controllers/secret_controller_test.go b/controllers/secret_controller_test.go index bb685588..9da80212 100644 --- a/controllers/secret_controller_test.go +++ b/controllers/secret_controller_test.go @@ -110,7 +110,7 @@ func TestResolveSecretRefDataErrorsForMissingBinaryDataKey(t *testing.T) { } } -func TestValidateSecretDataKeysRejectsDuplicates(t *testing.T) { +func TestValidateSecretDataRejectsDuplicates(t *testing.T) { secretCR := &resourcev1alpha1.Secret{ Spec: resourcev1alpha1.SecretSpec{ Data: map[string]string{"shared": "text"}, @@ -118,7 +118,70 @@ func TestValidateSecretDataKeysRejectsDuplicates(t *testing.T) { }, } - if err := validateSecretDataKeys(secretCR); err == nil { + if err := validateSecretData(secretCR); err == nil { t.Fatal("expected duplicate key validation error") } } + +func TestValidateSecretDataRejectsInvalidBinaryData(t *testing.T) { + secretCR := &resourcev1alpha1.Secret{ + Spec: resourcev1alpha1.SecretSpec{ + BinaryData: map[string]string{"keystore.p12": "not base64"}, + }, + } + + if err := validateSecretData(secretCR); err == nil { + t.Fatal("expected invalid base64 validation error") + } +} + +func TestApplyResolvedSecretRefDataDirectValuesTakePrecedence(t *testing.T) { + secretType := corev1.SecretTypeOpaque + secretCR := &resourcev1alpha1.Secret{ + Spec: resourcev1alpha1.SecretSpec{ + Data: map[string]string{ + "direct-data": "direct", + "resolved-binary-conflict": "direct-text", + }, + BinaryData: map[string]string{ + "direct-binary": "AAEC/w==", + "resolved-data-conflict": "AQIDBA==", + }, + }, + } + + applyResolvedSecretRefData( + secretCR, + map[string]string{ + "resolved-data": "from-ref", + "resolved-data-conflict": "from-ref-text", + }, + map[string]string{ + "resolved-binary": "BQYHCA==", + "resolved-binary-conflict": "CQoLDA==", + }, + &secretType, + ) + + if got := secretCR.Spec.Data["resolved-data"]; got != "from-ref" { + t.Fatalf("resolved data = %q, want from-ref", got) + } + if got := secretCR.Spec.BinaryData["resolved-binary"]; got != "BQYHCA==" { + t.Fatalf("resolved binaryData = %q, want BQYHCA==", got) + } + if got := secretCR.Spec.Data["resolved-binary-conflict"]; got != "direct-text" { + t.Fatalf("direct data override = %q, want direct-text", got) + } + if _, ok := secretCR.Spec.BinaryData["resolved-binary-conflict"]; ok { + t.Fatal("direct data key should remove resolved binaryData key") + } + if got := secretCR.Spec.BinaryData["resolved-data-conflict"]; got != "AQIDBA==" { + t.Fatalf("direct binaryData override = %q, want AQIDBA==", got) + } + if _, ok := secretCR.Spec.Data["resolved-data-conflict"]; ok { + t.Fatal("direct binaryData key should remove resolved data key") + } + if secretCR.Spec.Type == nil || *secretCR.Spec.Type != corev1.SecretTypeOpaque { + t.Fatalf("type = %v, want Opaque", secretCR.Spec.Type) + } +} diff --git a/docs/secret.md b/docs/secret.md index 8cd5280d..84eb4387 100644 --- a/docs/secret.md +++ b/docs/secret.md @@ -19,7 +19,7 @@ The `Secret` resource defines a secret in StreamNative Cloud. It allows you to c | `tolerations` | Tolerations for the secret | No | | `type` | Used to facilitate programmatic handling of secret data | No | -*Note: Specify at least one of `data`, `binaryData`, or `secretRef`. +*Note: Specify at least one of `data`, `binaryData`, or `secretRef`.* ### KubernetesSecretReference Structure