diff --git a/api/v2alpha1/executor_types.go b/api/v2alpha1/executor_types.go index c4a815586..309b3c5a6 100644 --- a/api/v2alpha1/executor_types.go +++ b/api/v2alpha1/executor_types.go @@ -103,6 +103,15 @@ type ExecutorStatus struct { // Truncated error message if the message is too long. // +optional BriefError string `json:"briefError,omitempty"` + + // ByPod reports the status observed by each individual Ratify pod (replica). + // It is aggregated from the per-pod ExecutorPodStatus objects and lets an + // operator tell, for example, that "2 of 5 replicas are unhealthy" instead + // of the top-level fields flapping under last-writer-wins. + // +optional + // +listType=map + // +listMapKey=id + ByPod []PodStatusEntry `json:"byPod,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/v2alpha1/executorpodstatus_types.go b/api/v2alpha1/executorpodstatus_types.go new file mode 100644 index 000000000..693b670de --- /dev/null +++ b/api/v2alpha1/executorpodstatus_types.go @@ -0,0 +1,83 @@ +/* +Copyright The Ratify Authors. + +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 v2alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// PodStatusEntry captures the health a single Ratify pod (replica) reports for +// an Executor resource. It is the unit that is written by each pod into its own +// ExecutorPodStatus object and later aggregated into Executor.status.byPod. +type PodStatusEntry struct { + // ID is the name of the pod that produced this status entry. Required. + ID string `json:"id"` + + // ObservedGeneration is the metadata.generation of the Executor that this + // entry was produced for. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // Succeeded indicates whether the reporting pod successfully built the + // executor from the Executor spec. Required. + Succeeded bool `json:"succeeded"` + + // Error is the error message if the reporting pod failed to build the + // executor. + // +optional + Error string `json:"error,omitempty"` + + // BriefError is a truncated error message when Error is too long to be + // displayed conveniently. + // +optional + BriefError string `json:"briefError,omitempty"` + + // LastTransitionTime is the time the reporting pod last updated this entry. + // +optional + LastTransitionTime *metav1.Time `json:"lastTransitionTime,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:storageversion + +// ExecutorPodStatus is the per-pod status of an Executor. Each Ratify replica +// owns exactly one ExecutorPodStatus object per Executor (the object name embeds +// the pod identity), so no two pods ever write the same object and there are no +// write conflicts. The object lives in the pod's own namespace and carries an +// owner reference to the pod, so it is garbage-collected automatically when the +// pod is deleted. (It is namespaced rather than cluster-scoped because a +// cluster-scoped object cannot be owned by a namespaced pod.) +type ExecutorPodStatus struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Status PodStatusEntry `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// ExecutorPodStatusList contains a list of ExecutorPodStatus. +type ExecutorPodStatusList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []ExecutorPodStatus `json:"items"` +} + +func init() { + SchemeBuilder.Register(&ExecutorPodStatus{}, &ExecutorPodStatusList{}) +} diff --git a/api/v2alpha1/zz_generated.deepcopy.go b/api/v2alpha1/zz_generated.deepcopy.go index 1b71ae0d2..d9f65624f 100644 --- a/api/v2alpha1/zz_generated.deepcopy.go +++ b/api/v2alpha1/zz_generated.deepcopy.go @@ -30,7 +30,7 @@ func (in *Executor) DeepCopyInto(out *Executor) { out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) in.Spec.DeepCopyInto(&out.Spec) - out.Status = in.Status + in.Status.DeepCopyInto(&out.Status) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Executor. @@ -83,6 +83,64 @@ func (in *ExecutorList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ExecutorPodStatus) DeepCopyInto(out *ExecutorPodStatus) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExecutorPodStatus. +func (in *ExecutorPodStatus) DeepCopy() *ExecutorPodStatus { + if in == nil { + return nil + } + out := new(ExecutorPodStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ExecutorPodStatus) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ExecutorPodStatusList) DeepCopyInto(out *ExecutorPodStatusList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]ExecutorPodStatus, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExecutorPodStatusList. +func (in *ExecutorPodStatusList) DeepCopy() *ExecutorPodStatusList { + if in == nil { + return nil + } + out := new(ExecutorPodStatusList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *ExecutorPodStatusList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExecutorSpec) DeepCopyInto(out *ExecutorSpec) { *out = *in @@ -133,6 +191,13 @@ func (in *ExecutorSpec) DeepCopy() *ExecutorSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ExecutorStatus) DeepCopyInto(out *ExecutorStatus) { *out = *in + if in.ByPod != nil { + in, out := &in.ByPod, &out.ByPod + *out = make([]PodStatusEntry, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ExecutorStatus. @@ -145,6 +210,25 @@ func (in *ExecutorStatus) DeepCopy() *ExecutorStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PodStatusEntry) DeepCopyInto(out *PodStatusEntry) { + *out = *in + if in.LastTransitionTime != nil { + in, out := &in.LastTransitionTime, &out.LastTransitionTime + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PodStatusEntry. +func (in *PodStatusEntry) DeepCopy() *PodStatusEntry { + if in == nil { + return nil + } + out := new(PodStatusEntry) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PolicyEnforcerOptions) DeepCopyInto(out *PolicyEnforcerOptions) { *out = *in diff --git a/config/crd/bases/config.ratify.dev_executorpodstatuses.yaml b/config/crd/bases/config.ratify.dev_executorpodstatuses.yaml new file mode 100644 index 000000000..af0dee644 --- /dev/null +++ b/config/crd/bases/config.ratify.dev_executorpodstatuses.yaml @@ -0,0 +1,90 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.18.0 + name: executorpodstatuses.config.ratify.dev +spec: + group: config.ratify.dev + names: + kind: ExecutorPodStatus + listKind: ExecutorPodStatusList + plural: executorpodstatuses + singular: executorpodstatus + scope: Namespaced + versions: + - name: v2alpha1 + schema: + openAPIV3Schema: + description: |- + ExecutorPodStatus is the per-pod status of an Executor. Each Ratify replica + owns exactly one ExecutorPodStatus object per Executor (the object name embeds + the pod identity), so no two pods ever write the same object and there are no + write conflicts. The object lives in the pod's own namespace and carries an + owner reference to the pod, so it is garbage-collected automatically when the + pod is deleted. (It is namespaced rather than cluster-scoped because a + cluster-scoped object cannot be owned by a namespaced pod.) + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + status: + description: |- + PodStatusEntry captures the health a single Ratify pod (replica) reports for + an Executor resource. It is the unit that is written by each pod into its own + ExecutorPodStatus object and later aggregated into Executor.status.byPod. + properties: + briefError: + description: |- + BriefError is a truncated error message when Error is too long to be + displayed conveniently. + type: string + error: + description: |- + Error is the error message if the reporting pod failed to build the + executor. + type: string + id: + description: ID is the name of the pod that produced this status entry. + Required. + type: string + lastTransitionTime: + description: LastTransitionTime is the time the reporting pod last + updated this entry. + format: date-time + type: string + observedGeneration: + description: |- + ObservedGeneration is the metadata.generation of the Executor that this + entry was produced for. + format: int64 + type: integer + succeeded: + description: |- + Succeeded indicates whether the reporting pod successfully built the + executor from the Executor spec. Required. + type: boolean + required: + - id + - succeeded + type: object + type: object + served: true + storage: true + subresources: + status: {} diff --git a/config/crd/bases/config.ratify.dev_executors.yaml b/config/crd/bases/config.ratify.dev_executors.yaml index 309bf5dae..4fe3778a3 100644 --- a/config/crd/bases/config.ratify.dev_executors.yaml +++ b/config/crd/bases/config.ratify.dev_executors.yaml @@ -139,6 +139,56 @@ spec: briefError: description: Truncated error message if the message is too long. type: string + byPod: + description: |- + ByPod reports the status observed by each individual Ratify pod (replica). + It is aggregated from the per-pod ExecutorPodStatus objects and lets an + operator tell, for example, that "2 of 5 replicas are unhealthy" instead + of the top-level fields flapping under last-writer-wins. + items: + description: |- + PodStatusEntry captures the health a single Ratify pod (replica) reports for + an Executor resource. It is the unit that is written by each pod into its own + ExecutorPodStatus object and later aggregated into Executor.status.byPod. + properties: + briefError: + description: |- + BriefError is a truncated error message when Error is too long to be + displayed conveniently. + type: string + error: + description: |- + Error is the error message if the reporting pod failed to build the + executor. + type: string + id: + description: ID is the name of the pod that produced this status + entry. Required. + type: string + lastTransitionTime: + description: LastTransitionTime is the time the reporting pod + last updated this entry. + format: date-time + type: string + observedGeneration: + description: |- + ObservedGeneration is the metadata.generation of the Executor that this + entry was produced for. + format: int64 + type: integer + succeeded: + description: |- + Succeeded indicates whether the reporting pod successfully built the + executor from the Executor spec. Required. + type: boolean + required: + - id + - succeeded + type: object + type: array + x-kubernetes-list-map-keys: + - id + x-kubernetes-list-type: map error: description: Error is the error message if the executor failed to start. diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 9151a7e1b..dc749e2a7 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -3,6 +3,7 @@ # It should be run by config/default resources: - bases/config.ratify.dev_executors.yaml +- bases/config.ratify.dev_executorpodstatuses.yaml # +kubebuilder:scaffold:crdkustomizeresource patches: diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 427521f60..1ca99c02f 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -65,6 +65,15 @@ spec: - --health-probe-bind-address=:8081 image: controller:latest name: manager + env: + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: RATIFY_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace ports: [] securityContext: allowPrivilegeEscalation: false diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 15bc8bca3..57b2c2ec5 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -48,9 +48,18 @@ rules: - get - patch - update +- apiGroups: + - "" + resources: + - pods + verbs: + - get + - list + - watch - apiGroups: - config.ratify.dev resources: + - executorpodstatuses - executors verbs: - create @@ -63,14 +72,15 @@ rules: - apiGroups: - config.ratify.dev resources: - - executors/finalizers + - executorpodstatuses/status + - executors/status verbs: + - get + - patch - update - apiGroups: - config.ratify.dev resources: - - executors/status + - executors/finalizers verbs: - - get - - patch - update diff --git a/internal/controller/executor_controller.go b/internal/controller/executor_controller.go index 75d34adda..8ef8dacab 100644 --- a/internal/controller/executor_controller.go +++ b/internal/controller/executor_controller.go @@ -18,32 +18,57 @@ package controller import ( "context" + "time" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/predicate" configv2alpha1 "github.com/notaryproject/ratify/v2/api/v2alpha1" + "github.com/notaryproject/ratify/v2/internal/podstatus" ) -// ExecutorReconciler reconciles a Executor object +// maxBriefErrorLength is the maximum length of the BriefError field. Longer +// error messages are truncated to keep the status compact. +const maxBriefErrorLength = 120 + +// ExecutorReconciler reconciles a Executor object. +// +// Every Ratify replica runs its own ExecutorReconciler: each pod builds its own +// in-memory executor (the data plane serves verification requests from it) and +// reports its own health into a dedicated per-pod ExecutorPodStatus object. +// Because each pod writes a distinct object (the name embeds the pod identity), +// there is no shared-status write contention between replicas. type ExecutorReconciler struct { client.Client Scheme *runtime.Scheme + + // PodName and PodNamespace identify the pod this reconciler runs in. They + // are used to name and own the per-pod ExecutorPodStatus object. When + // PodName is empty (e.g. running outside a cluster), status reporting falls + // back to writing the Executor status directly. + PodName string + PodNamespace string } // +kubebuilder:rbac:groups=config.ratify.dev,resources=executors,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=config.ratify.dev,resources=executors/status,verbs=get;update;patch // +kubebuilder:rbac:groups=config.ratify.dev,resources=executors/finalizers,verbs=update +// +kubebuilder:rbac:groups=config.ratify.dev,resources=executorpodstatuses,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=config.ratify.dev,resources=executorpodstatuses/status,verbs=get;update;patch +// +kubebuilder:rbac:groups="",resources=pods,verbs=get;list;watch // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. -// TODO(user): Modify the Reconcile function to compare the state specified by -// the Executor object against the actual cluster state, and then -// perform operations to make the cluster state reflect the state specified by -// the user. // // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.21.0/pkg/reconcile @@ -59,6 +84,12 @@ func (r *ExecutorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c if err := GlobalExecutorManager.deleteExecutor(req.Namespace, req.Name); err != nil { log.Error(err, "Failed to delete Executor from GlobalExecutorManager", "executor", req.Name) } + // The Executor is gone; remove this pod's per-pod status object so + // it does not linger (its owner reference is the pod, not the + // Executor, so it is not garbage-collected on Executor deletion). + if delErr := r.deletePodStatus(ctx, req.Name); delErr != nil { + log.Error(delErr, "Failed to delete ExecutorPodStatus", "executor", req.Name) + } } else { log.Error(err, "Failed to get Executor", "executor", req.Name) } @@ -75,22 +106,190 @@ func (r *ExecutorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } // SetupWithManager sets up the controller with the Manager. +// +// The watch is filtered with GenerationChangedPredicate so that status-only +// updates (which do not bump metadata.generation) do not re-trigger Reconcile. +// Without this predicate every status write produced by updateStatus would +// itself be an update event that re-enqueues the object, creating a feedback +// loop that repeatedly rebuilds the in-memory executor (and hammers external +// providers such as Azure Key Vault). The loop is amplified once the +// deployment is scaled to multiple replicas. func (r *ExecutorReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). - For(&configv2alpha1.Executor{}). + For(&configv2alpha1.Executor{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). Complete(r) } -func (r *ExecutorReconciler) updateStatus(ctx context.Context, executor *configv2alpha1.Executor, err error) { - if err != nil { - executor.Status.Succeeded = false - executor.Status.Error = err.Error() +// updateStatus records the outcome of the reconcile for this pod. +// +// When the pod identity is known, the outcome is written to a dedicated per-pod +// ExecutorPodStatus object (owned by the pod for automatic garbage collection), +// which a separate aggregation controller folds into Executor.status.byPod. +// This avoids all replicas writing the same Executor.status concurrently. When +// the pod identity is unknown, it falls back to writing the Executor status +// directly (single-writer, e.g. out-of-cluster usage). +func (r *ExecutorReconciler) updateStatus(ctx context.Context, executor *configv2alpha1.Executor, upsertErr error) { + if r.PodName == "" { + r.updateExecutorStatusDirectly(ctx, executor, upsertErr) + return + } + r.upsertPodStatus(ctx, executor, upsertErr) +} + +// upsertPodStatus creates or updates this pod's ExecutorPodStatus object. +func (r *ExecutorReconciler) upsertPodStatus(ctx context.Context, executor *configv2alpha1.Executor, upsertErr error) { + log := logf.FromContext(ctx) + name := podstatus.PackName(r.PodName, executor.Name) + + ps := &configv2alpha1.ExecutorPodStatus{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: r.PodNamespace}, + } + + // Ensure the object exists with the correct labels and owner reference. + if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, ps, func() error { + if ps.Labels == nil { + ps.Labels = map[string]string{} + } + ps.Labels[podstatus.LabelPodName] = sanitizeLabelValue(r.PodName) + ps.Labels[podstatus.LabelExecutorName] = sanitizeLabelValue(executor.Name) + r.setPodOwnerReference(ctx, ps) + return nil + }); err != nil { + log.Error(err, "Failed to upsert ExecutorPodStatus object", "executorPodStatus", name) + return + } + + entry := buildPodStatusEntry(r.PodName, executor.Generation, upsertErr) + statusErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + var latest configv2alpha1.ExecutorPodStatus + if getErr := r.Get(ctx, types.NamespacedName{Name: name, Namespace: r.PodNamespace}, &latest); getErr != nil { + return getErr + } + latest.Status = entry + return r.Status().Update(ctx, &latest) + }) + if statusErr != nil { + log.Error(statusErr, "Failed to update ExecutorPodStatus status", "executorPodStatus", name) + } +} + +// deletePodStatus removes this pod's ExecutorPodStatus object for the given +// executor name. It is a no-op when the pod identity is unknown or the object +// is already gone. +func (r *ExecutorReconciler) deletePodStatus(ctx context.Context, executorName string) error { + if r.PodName == "" { + return nil + } + ps := &configv2alpha1.ExecutorPodStatus{ + ObjectMeta: metav1.ObjectMeta{Name: podstatus.PackName(r.PodName, executorName), Namespace: r.PodNamespace}, + } + return client.IgnoreNotFound(r.Delete(ctx, ps)) +} + +// setPodOwnerReference best-effort sets the reporting pod as the owner of the +// ExecutorPodStatus so the object is garbage-collected when the pod is deleted. +// Failure to resolve the pod is not fatal: the object is still written, it just +// won't be auto-collected. +func (r *ExecutorReconciler) setPodOwnerReference(ctx context.Context, ps *configv2alpha1.ExecutorPodStatus) { + log := logf.FromContext(ctx) + var pod corev1.Pod + if err := r.Get(ctx, types.NamespacedName{Namespace: r.PodNamespace, Name: r.PodName}, &pod); err != nil { + log.V(1).Info("could not resolve owning pod for ExecutorPodStatus; skipping owner reference", "pod", r.PodName, "error", err.Error()) + return + } + if err := controllerutil.SetOwnerReference(&pod, ps, r.Scheme); err != nil { + log.V(1).Info("could not set owner reference on ExecutorPodStatus", "pod", r.PodName, "error", err.Error()) + } +} + +// updateExecutorStatusDirectly is the single-writer fallback used when the pod +// identity is unknown. The write is wrapped in retry.RetryOnConflict and the +// object is re-fetched on conflict so the update is not silently dropped on an +// HTTP 409. +func (r *ExecutorReconciler) updateExecutorStatusDirectly(ctx context.Context, executor *configv2alpha1.Executor, upsertErr error) { + log := logf.FromContext(ctx) + key := types.NamespacedName{Namespace: executor.Namespace, Name: executor.Name} + + retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + var latest configv2alpha1.Executor + if getErr := r.Get(ctx, key, &latest); getErr != nil { + return getErr + } + if upsertErr != nil { + latest.Status.Succeeded = false + latest.Status.Error = upsertErr.Error() + latest.Status.BriefError = briefError(upsertErr.Error()) + } else { + latest.Status.Succeeded = true + latest.Status.Error = "" + latest.Status.BriefError = "" + } + return r.Status().Update(ctx, &latest) + }) + if retryErr != nil { + log.Error(retryErr, "Failed to update Executor status", "executor", executor.Name) + } +} + +// buildPodStatusEntry builds the per-pod status entry for the given outcome. +func buildPodStatusEntry(podName string, generation int64, upsertErr error) configv2alpha1.PodStatusEntry { + now := metav1.NewTime(time.Now()) + entry := configv2alpha1.PodStatusEntry{ + ID: podName, + ObservedGeneration: generation, + LastTransitionTime: &now, + } + if upsertErr != nil { + entry.Succeeded = false + entry.Error = upsertErr.Error() + entry.BriefError = briefError(upsertErr.Error()) } else { - executor.Status.Succeeded = true - executor.Status.Error = "" + entry.Succeeded = true + } + return entry +} + +// briefError truncates an error message to maxBriefErrorLength characters. +func briefError(msg string) string { + if len(msg) <= maxBriefErrorLength { + return msg + } + return msg[:maxBriefErrorLength] + "..." +} + +// sanitizeLabelValue coerces an arbitrary string into a valid Kubernetes label +// value (<=63 chars, [a-z0-9A-Z] start/end, [a-z0-9A-Z-_.] within). Labels are +// only used for observability/filtering, so a lossy transformation is fine; the +// authoritative pod/executor names live in the object name and status.id. +func sanitizeLabelValue(v string) string { + if len(v) > 63 { + v = v[:63] + } + b := []byte(v) + for i, c := range b { + valid := (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c == '_' || c == '.' + if !valid { + b[i] = '-' + } + } + out := trimNonAlphanumericEnds(string(b)) + if out == "" { + return "unknown" + } + return out +} + +func trimNonAlphanumericEnds(s string) string { + isAlnum := func(c byte) bool { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') + } + start := 0 + for start < len(s) && !isAlnum(s[start]) { + start++ } - if statusErr := r.Status().Update(ctx, executor); statusErr != nil { - log := logf.FromContext(ctx) - log.Error(statusErr, "Failed to update Executor status", "executor", executor.Name) + end := len(s) + for end > start && !isAlnum(s[end-1]) { + end-- } + return s[start:end] } diff --git a/internal/controller/executor_controller_retry_test.go b/internal/controller/executor_controller_retry_test.go new file mode 100644 index 000000000..878c2e3ce --- /dev/null +++ b/internal/controller/executor_controller_retry_test.go @@ -0,0 +1,121 @@ +/* +Copyright The Ratify Authors. + +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 controller + +import ( + "context" + "errors" + "testing" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + configv2alpha1 "github.com/notaryproject/ratify/v2/api/v2alpha1" +) + +func newTestScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + if err := configv2alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add scheme: %v", err) + } + return scheme +} + +// TestUpdateStatusRetriesOnConflict verifies that updateStatus does not silently +// drop the status write when the first Status().Update returns an HTTP 409 +// conflict, but instead re-fetches and retries until it succeeds. +func TestUpdateStatusRetriesOnConflict(t *testing.T) { + scheme := newTestScheme(t) + executor := &configv2alpha1.Executor{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + } + + var updateAttempts int + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(executor). + WithStatusSubresource(executor). + WithInterceptorFuncs(interceptor.Funcs{ + SubResourceUpdate: func(ctx context.Context, cl client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error { + updateAttempts++ + if updateAttempts == 1 { + // Simulate another writer winning the optimistic-concurrency + // race on the first attempt. + return apierrors.NewConflict( + schema.GroupResource{Group: "config.ratify.dev", Resource: "executors"}, + obj.GetName(), + errors.New("the object has been modified"), + ) + } + return cl.Status().Update(ctx, obj, opts...) + }, + }). + Build() + + r := &ExecutorReconciler{Client: c, Scheme: scheme} + r.updateStatus(context.Background(), executor, nil) + + if updateAttempts < 2 { + t.Fatalf("expected updateStatus to retry after a conflict, got %d attempt(s)", updateAttempts) + } + + var got configv2alpha1.Executor + if err := c.Get(context.Background(), types.NamespacedName{Name: "test", Namespace: "default"}, &got); err != nil { + t.Fatalf("failed to get executor: %v", err) + } + if !got.Status.Succeeded { + t.Errorf("expected Status.Succeeded to be true after retry, got false") + } + if got.Status.Error != "" { + t.Errorf("expected empty Status.Error, got %q", got.Status.Error) + } +} + +// TestUpdateStatusRecordsError verifies that a non-nil upsert error is persisted +// to the Executor status. +func TestUpdateStatusRecordsError(t *testing.T) { + scheme := newTestScheme(t) + executor := &configv2alpha1.Executor{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"}, + } + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(executor). + WithStatusSubresource(executor). + Build() + + r := &ExecutorReconciler{Client: c, Scheme: scheme} + r.updateStatus(context.Background(), executor, errors.New("boom")) + + var got configv2alpha1.Executor + if err := c.Get(context.Background(), types.NamespacedName{Name: "test", Namespace: "default"}, &got); err != nil { + t.Fatalf("failed to get executor: %v", err) + } + if got.Status.Succeeded { + t.Errorf("expected Status.Succeeded to be false") + } + if got.Status.Error != "boom" { + t.Errorf("expected Status.Error to be %q, got %q", "boom", got.Status.Error) + } +} diff --git a/internal/controller/executorpodstatus_controller.go b/internal/controller/executorpodstatus_controller.go new file mode 100644 index 000000000..a5c238ff6 --- /dev/null +++ b/internal/controller/executorpodstatus_controller.go @@ -0,0 +1,150 @@ +/* +Copyright The Ratify Authors. + +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 controller + +import ( + "context" + "fmt" + "sort" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + configv2alpha1 "github.com/notaryproject/ratify/v2/api/v2alpha1" + "github.com/notaryproject/ratify/v2/internal/podstatus" +) + +// ExecutorPodStatusReconciler watches all ExecutorPodStatus objects and folds +// the per-pod entries back into the owning Executor's status.byPod. This is the +// aggregation half of the Gatekeeper-style per-pod status pattern: individual +// pods only ever write their own ExecutorPodStatus object (no shared-status +// contention), and a single logical aggregation reconstructs the parent status. +// +// The aggregation is a full rebuild from the currently existing per-pod objects, +// so it is idempotent: a deleted pod's entry simply disappears from byPod on the +// next reconcile (its ExecutorPodStatus is garbage-collected via the pod owner +// reference, which fires a delete event that re-triggers aggregation). +type ExecutorPodStatusReconciler struct { + client.Client + Scheme *runtime.Scheme +} + +// +kubebuilder:rbac:groups=config.ratify.dev,resources=executorpodstatuses,verbs=get;list;watch +// +kubebuilder:rbac:groups=config.ratify.dev,resources=executors,verbs=get;list;watch +// +kubebuilder:rbac:groups=config.ratify.dev,resources=executors/status,verbs=get;update;patch + +// Reconcile aggregates all ExecutorPodStatus objects belonging to the same +// Executor as the reconciled object into Executor.status.byPod. +func (r *ExecutorPodStatusReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + log := logf.FromContext(ctx) + + // The object may already be deleted; recover the executor name from the + // (reversible) object name so aggregation still runs on delete events. + _, executorName, err := podstatus.UnpackName(req.Name) + if err != nil { + log.Error(err, "Failed to decode ExecutorPodStatus name; skipping", "name", req.Name) + return ctrl.Result{}, nil + } + + var list configv2alpha1.ExecutorPodStatusList + if err := r.List(ctx, &list); err != nil { + return ctrl.Result{}, fmt.Errorf("failed to list ExecutorPodStatus objects: %w", err) + } + + byPod := make([]configv2alpha1.PodStatusEntry, 0, len(list.Items)) + for i := range list.Items { + _, itemExecutor, decErr := podstatus.UnpackName(list.Items[i].Name) + if decErr != nil { + continue + } + if itemExecutor == executorName { + byPod = append(byPod, list.Items[i].Status) + } + } + sort.Slice(byPod, func(i, j int) bool { return byPod[i].ID < byPod[j].ID }) + + retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + var executor configv2alpha1.Executor + if getErr := r.Get(ctx, types.NamespacedName{Name: executorName}, &executor); getErr != nil { + return getErr + } + applyAggregatedStatus(&executor, byPod) + return r.Status().Update(ctx, &executor) + }) + if apierrors.IsNotFound(retryErr) { + // The Executor is gone; nothing to aggregate. + return ctrl.Result{}, nil + } + if retryErr != nil { + return ctrl.Result{}, fmt.Errorf("failed to aggregate status for Executor %q: %w", executorName, retryErr) + } + return ctrl.Result{}, nil +} + +// SetupWithManager sets up the controller with the Manager. Note: no +// GenerationChangedPredicate here — per-pod status writes do not bump +// generation, yet they are exactly the events aggregation must react to. +// This watch does not create a feedback loop because it only writes the +// Executor status (never ExecutorPodStatus), and the Executor watch is itself +// filtered by generation. +func (r *ExecutorPodStatusReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&configv2alpha1.ExecutorPodStatus{}). + Complete(r) +} + +// applyAggregatedStatus recomputes the Executor's aggregate status from the +// per-pod entries. The top-level Succeeded is true only when at least one pod +// reported and every reporting pod succeeded; otherwise Error summarizes how +// many replicas are unhealthy. +func applyAggregatedStatus(executor *configv2alpha1.Executor, byPod []configv2alpha1.PodStatusEntry) { + executor.Status.ByPod = byPod + + total := len(byPod) + failing := 0 + firstErr := "" + for _, entry := range byPod { + if !entry.Succeeded { + failing++ + if firstErr == "" { + firstErr = entry.Error + } + } + } + + if total > 0 && failing == 0 { + executor.Status.Succeeded = true + executor.Status.Error = "" + executor.Status.BriefError = "" + return + } + + executor.Status.Succeeded = false + if total == 0 { + executor.Status.Error = "" + executor.Status.BriefError = "" + return + } + msg := fmt.Sprintf("%d/%d replicas unhealthy: %s", failing, total, firstErr) + executor.Status.Error = msg + executor.Status.BriefError = briefError(msg) +} diff --git a/internal/controller/executorpodstatus_controller_test.go b/internal/controller/executorpodstatus_controller_test.go new file mode 100644 index 000000000..3e41a88b8 --- /dev/null +++ b/internal/controller/executorpodstatus_controller_test.go @@ -0,0 +1,201 @@ +/* +Copyright The Ratify Authors. + +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 controller + +import ( + "context" + "errors" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + configv2alpha1 "github.com/notaryproject/ratify/v2/api/v2alpha1" + "github.com/notaryproject/ratify/v2/internal/podstatus" +) + +func typesName(name string) types.NamespacedName { + return types.NamespacedName{Name: name} +} + +func reconcileRequest(name string) reconcile.Request { + return reconcile.Request{NamespacedName: types.NamespacedName{Name: name}} +} + +func newPodStatusScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + if err := clientgoscheme.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add clientgo scheme: %v", err) + } + if err := configv2alpha1.AddToScheme(scheme); err != nil { + t.Fatalf("failed to add ratify scheme: %v", err) + } + return scheme +} + +// TestUpsertPodStatusWritesPerPodObject verifies that when the pod identity is +// known, the reconciler writes a dedicated ExecutorPodStatus object (named for +// the pod+executor) instead of touching Executor.status directly. +func TestUpsertPodStatusWritesPerPodObject(t *testing.T) { + scheme := newPodStatusScheme(t) + executor := &configv2alpha1.Executor{ + ObjectMeta: metav1.ObjectMeta{Name: "default", Generation: 3}, + } + pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "ratify-0", Namespace: "ratify-system", UID: "uid-123"}} + + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(executor, pod). + WithStatusSubresource(&configv2alpha1.Executor{}, &configv2alpha1.ExecutorPodStatus{}). + Build() + + r := &ExecutorReconciler{Client: c, Scheme: scheme, PodName: "ratify-0", PodNamespace: "ratify-system"} + r.updateStatus(context.Background(), executor, nil) + + name := podstatus.PackName("ratify-0", "default") + var ps configv2alpha1.ExecutorPodStatus + if err := c.Get(context.Background(), types.NamespacedName{Namespace: "ratify-system", Name: name}, &ps); err != nil { + t.Fatalf("expected ExecutorPodStatus %q to exist: %v", name, err) + } + if !ps.Status.Succeeded { + t.Errorf("expected per-pod status Succeeded=true") + } + if ps.Status.ID != "ratify-0" { + t.Errorf("expected status.id=ratify-0, got %q", ps.Status.ID) + } + if ps.Status.ObservedGeneration != 3 { + t.Errorf("expected observedGeneration=3, got %d", ps.Status.ObservedGeneration) + } + // Owner reference to the pod enables garbage collection. + if len(ps.OwnerReferences) != 1 || ps.OwnerReferences[0].Name != "ratify-0" { + t.Errorf("expected owner reference to pod ratify-0, got %+v", ps.OwnerReferences) + } + // Executor.status must NOT have been written directly by the pod. + var gotExec configv2alpha1.Executor + if err := c.Get(context.Background(), typesName("default"), &gotExec); err != nil { + t.Fatalf("get executor: %v", err) + } + if gotExec.Status.Succeeded { + t.Errorf("expected Executor.status.succeeded to be untouched (false) by per-pod path") + } +} + +func TestUpsertPodStatusRecordsError(t *testing.T) { + scheme := newPodStatusScheme(t) + executor := &configv2alpha1.Executor{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(executor). + WithStatusSubresource(&configv2alpha1.Executor{}, &configv2alpha1.ExecutorPodStatus{}). + Build() + + r := &ExecutorReconciler{Client: c, Scheme: scheme, PodName: "ratify-1", PodNamespace: "ratify-system"} + r.updateStatus(context.Background(), executor, errors.New("akv unreachable")) + + var ps configv2alpha1.ExecutorPodStatus + if err := c.Get(context.Background(), types.NamespacedName{Namespace: "ratify-system", Name: podstatus.PackName("ratify-1", "default")}, &ps); err != nil { + t.Fatalf("get pod status: %v", err) + } + if ps.Status.Succeeded { + t.Errorf("expected Succeeded=false") + } + if ps.Status.Error != "akv unreachable" { + t.Errorf("expected error recorded, got %q", ps.Status.Error) + } +} + +// TestApplyAggregatedStatus checks the pure aggregation logic. +func TestApplyAggregatedStatus(t *testing.T) { + t.Run("all healthy", func(t *testing.T) { + var e configv2alpha1.Executor + applyAggregatedStatus(&e, []configv2alpha1.PodStatusEntry{ + {ID: "a", Succeeded: true}, + {ID: "b", Succeeded: true}, + }) + if !e.Status.Succeeded || e.Status.Error != "" || len(e.Status.ByPod) != 2 { + t.Errorf("unexpected status: %+v", e.Status) + } + }) + t.Run("some unhealthy", func(t *testing.T) { + var e configv2alpha1.Executor + applyAggregatedStatus(&e, []configv2alpha1.PodStatusEntry{ + {ID: "a", Succeeded: true}, + {ID: "b", Succeeded: false, Error: "boom"}, + }) + if e.Status.Succeeded { + t.Errorf("expected Succeeded=false") + } + if e.Status.Error == "" { + t.Errorf("expected aggregated error message") + } + }) + t.Run("no pods", func(t *testing.T) { + var e configv2alpha1.Executor + applyAggregatedStatus(&e, nil) + if e.Status.Succeeded { + t.Errorf("expected Succeeded=false when no pods reported") + } + }) +} + +// TestAggregatorReconcile verifies the aggregation reconciler folds per-pod +// objects into Executor.status.byPod. +func TestAggregatorReconcile(t *testing.T) { + scheme := newPodStatusScheme(t) + executor := &configv2alpha1.Executor{ObjectMeta: metav1.ObjectMeta{Name: "default"}} + ps1 := &configv2alpha1.ExecutorPodStatus{ + ObjectMeta: metav1.ObjectMeta{Name: podstatus.PackName("pod-1", "default"), Namespace: "ratify-system"}, + Status: configv2alpha1.PodStatusEntry{ID: "pod-1", Succeeded: true}, + } + ps2 := &configv2alpha1.ExecutorPodStatus{ + ObjectMeta: metav1.ObjectMeta{Name: podstatus.PackName("pod-2", "default"), Namespace: "ratify-system"}, + Status: configv2alpha1.PodStatusEntry{ID: "pod-2", Succeeded: false, Error: "akv"}, + } + // A pod status belonging to a different executor must be ignored. + psOther := &configv2alpha1.ExecutorPodStatus{ + ObjectMeta: metav1.ObjectMeta{Name: podstatus.PackName("pod-1", "other"), Namespace: "ratify-system"}, + Status: configv2alpha1.PodStatusEntry{ID: "pod-1", Succeeded: true}, + } + + c := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(executor, ps1, ps2, psOther). + WithStatusSubresource(&configv2alpha1.Executor{}, &configv2alpha1.ExecutorPodStatus{}). + Build() + + r := &ExecutorPodStatusReconciler{Client: c, Scheme: scheme} + if _, err := r.Reconcile(context.Background(), reconcileRequest(ps1.Name)); err != nil { + t.Fatalf("reconcile: %v", err) + } + + var got configv2alpha1.Executor + if err := c.Get(context.Background(), typesName("default"), &got); err != nil { + t.Fatalf("get executor: %v", err) + } + if len(got.Status.ByPod) != 2 { + t.Fatalf("expected 2 byPod entries, got %d: %+v", len(got.Status.ByPod), got.Status.ByPod) + } + if got.Status.Succeeded { + t.Errorf("expected aggregate Succeeded=false because pod-2 failed") + } +} diff --git a/internal/manager/manager.go b/internal/manager/manager.go index cdbfa0fc5..18222606b 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -119,10 +119,20 @@ func setupCRDControllers(mgr ctrl.Manager, disableCRDManager bool) { setupLog.Info("setting up CRD controllers") if err := (&controller.ExecutorReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + PodName: pod.Name(), + PodNamespace: pod.Namespace(), + }).SetupWithManager(mgr); err != nil { + setupLog.Error(err, "could not set up Executor reconciler") + os.Exit(1) + } + + if err := (&controller.ExecutorPodStatusReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), }).SetupWithManager(mgr); err != nil { - setupLog.Error(err, "could not set up Executor reconciler") + setupLog.Error(err, "could not set up ExecutorPodStatus reconciler") os.Exit(1) } } diff --git a/internal/pod/info.go b/internal/pod/info.go index 309559d98..58bf39ea4 100644 --- a/internal/pod/info.go +++ b/internal/pod/info.go @@ -26,6 +26,13 @@ func Namespace() string { return ns } +// Name returns the name of the pod the process is running in. It is read from +// the POD_NAME environment variable, which is expected to be injected via the +// Kubernetes downward API. It returns an empty string when not set. +func Name() string { + return os.Getenv("POD_NAME") +} + // ServiceName returns the service name. func ServiceName() string { name, found := os.LookupEnv("RATIFY_NAME") diff --git a/internal/podstatus/name.go b/internal/podstatus/name.go new file mode 100644 index 000000000..2f5776fbf --- /dev/null +++ b/internal/podstatus/name.go @@ -0,0 +1,78 @@ +/* +Copyright The Ratify Authors. + +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 podstatus contains helpers for the per-pod ExecutorPodStatus objects +// used to report per-replica Executor health without concurrent writers racing +// on a single shared status (the Gatekeeper *PodStatus pattern). +package podstatus + +import ( + "encoding/base32" + "fmt" + "strings" +) + +const ( + // LabelPodName is the label carrying the (sanitized) reporting pod name. + LabelPodName = "internal.ratify.dev/pod-name" + // LabelExecutorName is the label carrying the (sanitized) Executor name. + LabelExecutorName = "internal.ratify.dev/executor-name" +) + +// base32 without padding, lowercased, yields only [a-z2-7], which are all valid +// characters for a DNS-1123 subdomain (Kubernetes object name). Because the +// alphabet never contains '-', a single '-' is a safe, unambiguous separator +// between the two encoded segments. +var enc = base32.StdEncoding.WithPadding(base32.NoPadding) + +// PackName returns a deterministic, DNS-1123-compliant object name that embeds +// both the pod name and the executor name. Because the name is unique per +// (pod, executor) pair, no two pods ever target the same ExecutorPodStatus +// object, which eliminates write conflicts. The name is reversible via +// UnpackName so aggregation can recover the executor name even from a delete +// event (where only the object name is available). +func PackName(podName, executorName string) string { + return encode(podName) + "-" + encode(executorName) +} + +// UnpackName reverses PackName, returning the original pod and executor names. +func UnpackName(name string) (podName, executorName string, err error) { + parts := strings.SplitN(name, "-", 2) + if len(parts) != 2 { + return "", "", fmt.Errorf("invalid ExecutorPodStatus name %q: expected two dash-separated segments", name) + } + podName, err = decode(parts[0]) + if err != nil { + return "", "", fmt.Errorf("invalid ExecutorPodStatus name %q: %w", name, err) + } + executorName, err = decode(parts[1]) + if err != nil { + return "", "", fmt.Errorf("invalid ExecutorPodStatus name %q: %w", name, err) + } + return podName, executorName, nil +} + +func encode(s string) string { + return strings.ToLower(enc.EncodeToString([]byte(s))) +} + +func decode(s string) (string, error) { + b, err := enc.DecodeString(strings.ToUpper(s)) + if err != nil { + return "", err + } + return string(b), nil +} diff --git a/internal/podstatus/name_test.go b/internal/podstatus/name_test.go new file mode 100644 index 000000000..2e8dbb09a --- /dev/null +++ b/internal/podstatus/name_test.go @@ -0,0 +1,66 @@ +/* +Copyright The Ratify Authors. + +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 podstatus + +import "testing" + +func TestPackUnpackNameRoundTrip(t *testing.T) { + cases := []struct { + pod string + executor string + }{ + {"ratify-gatekeeper-provider-7d9f8c-abcde", "default"}, + {"pod-1", "my-executor.with.dots"}, + {"UPPER-Case-Pod", "Executor-With-CAPS"}, + {"p", "e"}, + } + for _, c := range cases { + name := PackName(c.pod, c.executor) + gotPod, gotExec, err := UnpackName(name) + if err != nil { + t.Fatalf("UnpackName(%q) returned error: %v", name, err) + } + if gotPod != c.pod || gotExec != c.executor { + t.Errorf("round trip mismatch: PackName(%q,%q)=%q -> (%q,%q)", c.pod, c.executor, name, gotPod, gotExec) + } + } +} + +func TestPackNameIsDNS1123Compliant(t *testing.T) { + name := PackName("Pod_Name/With:Weird*Chars", "Executor Name!") + for _, c := range name { + valid := (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '-' + if !valid { + t.Errorf("packed name %q contains invalid DNS-1123 character %q", name, c) + } + } +} + +func TestPackNameUniquePerPair(t *testing.T) { + a := PackName("pod-a", "exec") + b := PackName("pod-b", "exec") + c := PackName("pod-a", "other") + if a == b || a == c || b == c { + t.Errorf("expected unique names, got a=%q b=%q c=%q", a, b, c) + } +} + +func TestUnpackNameInvalid(t *testing.T) { + if _, _, err := UnpackName("no-separator-but-not-base32!!"); err == nil { + t.Errorf("expected error for invalid name") + } +}