diff --git a/api/v1alpha1/mustgather_types.go b/api/v1alpha1/mustgather_types.go index c3bb90832..189d2ffa5 100644 --- a/api/v1alpha1/mustgather_types.go +++ b/api/v1alpha1/mustgather_types.go @@ -71,6 +71,9 @@ type MustGatherSpec struct { // GatherSpec allows specifying the execution details for a must-gather run and the collection behavior. // +kubebuilder:validation:XValidation:rule="!(has(self.since) && has(self.sinceTime))",message="only one of since or sinceTime may be specified" +// +kubebuilder:validation:XValidation:rule="!has(self.since) || !self.since.startsWith(\"-\")",message="since must be a non-negative duration string" +// +kubebuilder:validation:XValidation:rule="!has(self.since) || self.since.matches(r'^\\+?(([0-9]+(\\.[0-9]*)?|\\.[0-9]+)(ns|µs|us|ms|s|m|h))+$')",message="since may only use these duration suffixes: ns, us, µs, ms, s, m, h (e.g. 2h, 30m, 168h for one week)." +// +kubebuilder:validation:XValidation:rule="!has(self.sinceTime) || self.sinceTime.matches(r'^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(\\.\\d+)?(Z|[+-]\\d{2}:\\d{2})$')",message="sinceTime must be in RFC3339 format." type GatherSpec struct { // +kubebuilder:validation:Optional // Audit requests audit log collection via the default gather entrypoint. @@ -90,18 +93,18 @@ type GatherSpec struct { // +kubebuilder:validation:Items:MaxLength=256 Args []string `json:"args,omitempty"` - // Since only returns logs newer than a relative duration like "2h" or "30m". + // Since only returns logs newer than a relative duration (e.g. 2h, 30m, 168h for one week). Must be non-negative. + // Allowed units are ns, us, µs, ms, s, m, and h. // This is passed to the must-gather script to filter log collection. // Only one of since or sinceTime may be specified. // +kubebuilder:validation:Optional - // +kubebuilder:validation:Format=duration Since *metav1.Duration `json:"since,omitempty"` - // SinceTime only returns logs after a specific date/time (RFC3339 format). + // SinceTime only returns logs after a specific instant. Use RFC3339 with uppercase T and Z or a zone offset + // (e.g. 2026-02-02T00:00:00Z). Must not be after the current time. // This is passed to the must-gather script to filter log collection. // Only one of since or sinceTime may be specified. // +kubebuilder:validation:Optional - // +kubebuilder:validation:Format=date-time SinceTime *metav1.Time `json:"sinceTime,omitempty"` } diff --git a/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml b/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml index 4bf3925e4..8d395ef13 100644 --- a/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml +++ b/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml @@ -68,14 +68,15 @@ spec: type: array since: description: |- - Since only returns logs newer than a relative duration like "2h" or "30m". + Since only returns logs newer than a relative duration (e.g. 2h, 30m, 168h for one week). Must be non-negative. + Allowed units are ns, us, µs, ms, s, m, and h. This is passed to the must-gather script to filter log collection. Only one of since or sinceTime may be specified. - format: duration type: string sinceTime: description: |- - SinceTime only returns logs after a specific date/time (RFC3339 format). + SinceTime only returns logs after a specific instant. Use RFC3339 with uppercase T and Z or a zone offset + (e.g. 2026-02-02T00:00:00Z). Must not be after the current time. This is passed to the must-gather script to filter log collection. Only one of since or sinceTime may be specified. format: date-time @@ -84,6 +85,13 @@ spec: x-kubernetes-validations: - message: only one of since or sinceTime may be specified rule: '!(has(self.since) && has(self.sinceTime))' + - message: since must be a non-negative duration string + rule: '!has(self.since) || !self.since.startsWith("-")' + - message: 'since may only use these duration suffixes: ns, us, µs, + ms, s, m, h (e.g. 2h, 30m, 168h for one week).' + rule: '!has(self.since) || self.since.matches(r''^\+?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|µs|us|ms|s|m|h))+$'')' + - message: sinceTime must be in RFC3339 format. + rule: "!has(self.sinceTime) || self.sinceTime.matches(r'^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(\\.\\d+)?(Z|[+-]\\d{2}:\\d{2})$')" imageStreamRef: description: |- ImageStreamRef specifies a custom image from the allowlist to be used for the diff --git a/controllers/mustgather/constant.go b/controllers/mustgather/constant.go index 5703caee9..8c3a0171d 100644 --- a/controllers/mustgather/constant.go +++ b/controllers/mustgather/constant.go @@ -20,6 +20,9 @@ const ( // ValidationImageStream represents the validation type for ImageStream ValidationImageStream = "ImageStream" + // ValidationSinceTime represents validation of gatherSpec.sinceTime (e.g. must not be in the future) + ValidationSinceTime = "sinceTime" + // DefaultMustGatherImageEnv represents the environment variable for the default must-gather image DefaultMustGatherImageEnv = "DEFAULT_MUST_GATHER_IMAGE" diff --git a/controllers/mustgather/mustgather_controller.go b/controllers/mustgather/mustgather_controller.go index 44a052fbd..db888195f 100644 --- a/controllers/mustgather/mustgather_controller.go +++ b/controllers/mustgather/mustgather_controller.go @@ -22,6 +22,7 @@ import ( "fmt" "os" "slices" + "time" "github.com/go-logr/logr" imagev1 "github.com/openshift/api/image/v1" @@ -397,6 +398,14 @@ func (r *MustGatherReconciler) getJobFromInstance(ctx context.Context, instance return nil, err } + if instance.Spec.GatherSpec != nil && instance.Spec.GatherSpec.SinceTime != nil && instance.Spec.GatherSpec.SinceTime.After(time.Now()) { + err := fmt.Errorf("gatherSpec.sinceTime must be at or before the current date and time") + if _, validationErr := r.setValidationFailureStatus(ctx, log, instance, ValidationSinceTime, err); validationErr != nil { + return nil, fmt.Errorf("failed to set validation failure status: %w, %w", err, validationErr) + } + return nil, err + } + return getJobTemplate(image, operatorImage, *instance, r.TrustedCAConfigMap), nil } diff --git a/controllers/mustgather/mustgather_controller_test.go b/controllers/mustgather/mustgather_controller_test.go index 0191c26e8..c51637ef8 100644 --- a/controllers/mustgather/mustgather_controller_test.go +++ b/controllers/mustgather/mustgather_controller_test.go @@ -1588,6 +1588,48 @@ func generateFakeClient(objs ...runtime.Object) (client.Client, *runtime.Scheme) return cl, s } +func TestGetJobFromInstanceFutureSinceTimeSetsValidationStatus(t *testing.T) { + st := metav1.NewTime(time.Now().Add(48 * time.Hour)) + mg := &mustgatherv1alpha1.MustGather{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mg-future-sincetime", + Namespace: "openshift-must-gather-operator", + Finalizers: []string{mustGatherFinalizer}, + }, + Spec: mustgatherv1alpha1.MustGatherSpec{ + GatherSpec: &mustgatherv1alpha1.GatherSpec{ + SinceTime: &st, + }, + }, + } + t.Setenv("OPERATOR_IMAGE", "test-image") + t.Setenv("DEFAULT_MUST_GATHER_IMAGE", "test-must-gather-image") + cl, s := generateFakeClient(mg) + eventRec := record.NewFakeRecorder(10) + var cfg *rest.Config + r := MustGatherReconciler{ + ReconcilerBase: util.NewReconcilerBase(cl, s, cfg, eventRec, nil), + DefaultMustGatherImage: "test-must-gather-image", + OperatorNamespace: "openshift-must-gather-operator", + } + + _, err := r.getJobFromInstance(context.Background(), mg) + if err == nil { + t.Fatal("expected error") + } + + updated := &mustgatherv1alpha1.MustGather{} + if err := cl.Get(context.Background(), types.NamespacedName{Name: mg.Name, Namespace: mg.Namespace}, updated); err != nil { + t.Fatal(err) + } + if updated.Status.Status != "Failed" { + t.Fatalf("expected status Failed, got %q", updated.Status.Status) + } + if !strings.Contains(updated.Status.Reason, ValidationSinceTime) { + t.Fatalf("expected reason to mention sinceTime, got %q", updated.Status.Reason) + } +} + // TestSFTPCredentialValidation tests the credential validation logic added in the controller func TestSFTPCredentialValidation(t *testing.T) { // Setup scheme diff --git a/deploy/crds/operator.openshift.io_mustgathers.yaml b/deploy/crds/operator.openshift.io_mustgathers.yaml index 4bf3925e4..c30e2f4ae 100644 --- a/deploy/crds/operator.openshift.io_mustgathers.yaml +++ b/deploy/crds/operator.openshift.io_mustgathers.yaml @@ -68,15 +68,14 @@ spec: type: array since: description: |- - Since only returns logs newer than a relative duration like "2h" or "30m". - This is passed to the must-gather script to filter log collection. + Since only returns logs newer than a relative duration (e.g. 2h, 30m, 168h for one week). Must be non-negative. + Allowed units are ns, us, µs, ms, s, m, and h. Only one of since or sinceTime may be specified. - format: duration type: string sinceTime: description: |- - SinceTime only returns logs after a specific date/time (RFC3339 format). - This is passed to the must-gather script to filter log collection. + SinceTime only returns logs after a specific instant. Use RFC3339 with uppercase T and Z or a zone offset + (e.g. 2026-02-02T00:00:00Z). Must not be after the current time. Only one of since or sinceTime may be specified. format: date-time type: string @@ -84,6 +83,13 @@ spec: x-kubernetes-validations: - message: only one of since or sinceTime may be specified rule: '!(has(self.since) && has(self.sinceTime))' + - message: since must be a non-negative duration string + rule: '!has(self.since) || !self.since.startsWith("-")' + - message: 'since may only use these duration suffixes: ns, us, µs, + ms, s, m, h (e.g. 2h, 30m, 168h for one week).' + rule: '!has(self.since) || self.since.matches(r''^\+?(([0-9]+(\.[0-9]*)?|\.[0-9]+)(ns|µs|us|ms|s|m|h))+$'')' + - message: sinceTime must be in RFC3339 format. + rule: '!has(self.sinceTime) || self.sinceTime.matches(r''^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(Z|[+-]\d{2}:\d{2})$'')' imageStreamRef: description: |- ImageStreamRef specifies a custom image from the allowlist to be used for the