diff --git a/api/v1alpha1/mustgather_types.go b/api/v1alpha1/mustgather_types.go index 55a751409..6dc15de51 100644 --- a/api/v1alpha1/mustgather_types.go +++ b/api/v1alpha1/mustgather_types.go @@ -25,7 +25,7 @@ import ( // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. // MustGatherSpec defines the desired state of MustGather -// +kubebuilder:validation:XValidation:rule="!(has(self.gatherSpec) && (size(self.gatherSpec.command) > 0 || size(self.gatherSpec.args) > 0)) || has(self.imageStreamRef)",message="command and args in gatherSpec can only be set when imageStreamRef is specified" +// +kubebuilder:validation:XValidation:rule="!(has(self.gatherSpec) && ((has(self.gatherSpec.command) && size(self.gatherSpec.command) > 0) || (has(self.gatherSpec.args) && size(self.gatherSpec.args) > 0))) || has(self.imageStreamRef)",message="command and args in gatherSpec can only be set when imageStreamRef is specified" type MustGatherSpec struct { // the service account to use to run the must gather job pod, defaults to default // +kubebuilder:validation:Optional @@ -37,7 +37,8 @@ type MustGatherSpec struct { // +kubebuilder:validation:Optional ImageStreamRef *ImageStreamTagRef `json:"imageStreamRef,omitempty"` - // GatherSpec allows overriding the command and/or arguments for the custom must-gather image. + // GatherSpec allows overriding the command and/or arguments for the custom must-gather image + // and configures time-based collection filters. // This field is ignored if ImageStreamRef is not specified. // +kubebuilder:validation:Optional GatherSpec *GatherSpec `json:"gatherSpec,omitempty"` @@ -66,7 +67,8 @@ type MustGatherSpec struct { Storage *Storage `json:"storage,omitempty"` } -// GatherSpec allows specifying the execution details for a must-gather run. +// 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" type GatherSpec struct { // +kubebuilder:validation:Optional // Audit specifies whether to collect audit logs. This is translated to a signal @@ -87,6 +89,20 @@ type GatherSpec struct { // +kubebuilder:validation:MaxItems=256 // +kubebuilder:validation:Items:MaxLength=256 Args []string `json:"args,omitempty"` + + // 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. + // 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). + // 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"` } // ImageStreamTagRef provides a structured reference to a specific tag within an ImageStream. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 7337f8fe6..28dda7f2c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -22,6 +22,15 @@ func (in *GatherSpec) DeepCopyInto(out *GatherSpec) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.Since != nil { + in, out := &in.Since, &out.Since + *out = new(v1.Duration) + **out = **in + } + if in.SinceTime != nil { + in, out := &in.SinceTime, &out.SinceTime + *out = (*in).DeepCopy() + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GatherSpec. diff --git a/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml b/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml index 09ad43239..3945afdbf 100644 --- a/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml +++ b/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.18.1-0.20250818134112-b624019bbe8d + controller-gen.kubebuilder.io/version: v0.19.0 name: mustgathers.operator.openshift.io spec: group: operator.openshift.io @@ -41,7 +41,8 @@ spec: properties: gatherSpec: description: |- - GatherSpec allows overriding the command and/or arguments for the custom must-gather image. + GatherSpec allows overriding the command and/or arguments for the custom must-gather image + and configures time-based collection filters. This field is ignored if ImageStreamRef is not specified. properties: args: @@ -66,7 +67,24 @@ spec: type: string maxItems: 256 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. + 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. + Only one of since or sinceTime may be specified. + format: date-time + type: string type: object + x-kubernetes-validations: + - message: only one of since or sinceTime may be specified + rule: '!(has(self.since) && has(self.sinceTime))' imageStreamRef: description: |- ImageStreamRef specifies a custom image from the allowlist to be used for the @@ -213,8 +231,9 @@ spec: x-kubernetes-validations: - message: command and args in gatherSpec can only be set when imageStreamRef is specified - rule: '!(has(self.gatherSpec) && (size(self.gatherSpec.command) > 0 - || size(self.gatherSpec.args) > 0)) || has(self.imageStreamRef)' + rule: '!(has(self.gatherSpec) && ((has(self.gatherSpec.command) && size(self.gatherSpec.command) + > 0) || (has(self.gatherSpec.args) && size(self.gatherSpec.args) > + 0))) || has(self.imageStreamRef)' status: description: MustGatherStatus defines the observed state of MustGather properties: diff --git a/controllers/mustgather/mustgather_controller.go b/controllers/mustgather/mustgather_controller.go index 70f7ee33b..fb3ccc364 100644 --- a/controllers/mustgather/mustgather_controller.go +++ b/controllers/mustgather/mustgather_controller.go @@ -21,8 +21,10 @@ import ( goerror "errors" "fmt" "os" + "time" "github.com/go-logr/logr" + configv1 "github.com/openshift/api/config/v1" imagev1 "github.com/openshift/api/image/v1" mustgatherv1alpha1 "github.com/openshift/must-gather-operator/api/v1alpha1" "github.com/openshift/must-gather-operator/pkg/localmetrics" @@ -411,7 +413,25 @@ func (r *MustGatherReconciler) getJobFromInstance(ctx context.Context, instance return nil, err } - return getJobTemplate(image, operatorImage, *instance, r.TrustedCAConfigMap), nil + // Best-effort fetch of cluster creation time (used to clamp since/sinceTime filters). + // Errors here must NOT block must-gather execution. + var clusterCreationTime *time.Time + if t, err := r.getClusterCreationTime(ctx); err != nil { + log.V(2).Info("unable to determine cluster creation time; since filters will not be clamped", "err", err) + } else { + clusterCreationTime = t + } + + return getJobTemplate(image, operatorImage, *instance, r.TrustedCAConfigMap, clusterCreationTime), nil +} + +func (r *MustGatherReconciler) getClusterCreationTime(ctx context.Context) (*time.Time, error) { + cv := &configv1.ClusterVersion{} + if err := r.GetClient().Get(ctx, types.NamespacedName{Name: "version"}, cv); err != nil { + return nil, err + } + t := cv.CreationTimestamp.Time + return &t, nil } func (r *MustGatherReconciler) getMustGatherImage(ctx context.Context, instance *mustgatherv1alpha1.MustGather) (string, error) { diff --git a/controllers/mustgather/template.go b/controllers/mustgather/template.go index e906892a8..669346bd2 100644 --- a/controllers/mustgather/template.go +++ b/controllers/mustgather/template.go @@ -30,6 +30,10 @@ const ( gatherCommand = "timeout %v bash -x -c -- '/usr/bin/%v' 2>&1 | tee /must-gather/must-gather.log\n\nstatus=$?\nif [[ $status -eq 124 || $status -eq 137 ]]; then\n echo \"Gather timed out.\"\n exit 0\nfi | tee -a /must-gather/must-gather.log" gatherContainerName = "gather" + // Environment variables for time-based log filtering + gatherEnvSince = "MUST_GATHER_SINCE" + gatherEnvSinceTime = "MUST_GATHER_SINCE_TIME" + backoffLimit = 3 uploadContainerName = "upload" uploadEnvUsername = "username" @@ -49,7 +53,18 @@ const ( knownHostsFile = "/tmp/must-gather-operator/.ssh/known_hosts" ) -func getJobTemplate(image string, operatorImage string, mustGather v1alpha1.MustGather, trustedCAConfigMapName string) *batchv1.Job { +// timeNow exists to allow deterministic unit testing of time-based behavior. +var timeNow = time.Now + +// GatherTimeFilter holds the time-based filtering options for log collection +type GatherTimeFilter struct { + // Since is a relative duration (e.g., "2h", "30m") + Since time.Duration + // SinceTime is an absolute timestamp + SinceTime *time.Time +} + +func getJobTemplate(image string, operatorImage string, mustGather v1alpha1.MustGather, trustedCAConfigMapName string, clusterCreationTime *time.Time) *batchv1.Job { job := initializeJobTemplate(mustGather.Name, mustGather.Namespace, mustGather.Spec.ServiceAccountName, mustGather.Spec.Storage, trustedCAConfigMapName) var httpProxy, httpsProxy, noProxy string @@ -78,15 +93,27 @@ func getJobTemplate(image string, operatorImage string, mustGather v1alpha1.Must timeout = mustGather.Spec.MustGatherTimeout.Duration } + // Build time filter from spec + var timeFilter *GatherTimeFilter var command, args []string if mustGather.Spec.GatherSpec != nil { command = mustGather.Spec.GatherSpec.Command args = mustGather.Spec.GatherSpec.Args + if mustGather.Spec.GatherSpec.Since != nil || mustGather.Spec.GatherSpec.SinceTime != nil { + timeFilter = &GatherTimeFilter{} + if mustGather.Spec.GatherSpec.Since != nil { + timeFilter.Since = mustGather.Spec.GatherSpec.Since.Duration + } + if mustGather.Spec.GatherSpec.SinceTime != nil { + t := mustGather.Spec.GatherSpec.SinceTime.Time + timeFilter.SinceTime = &t + } + } } job.Spec.Template.Spec.Containers = append( job.Spec.Template.Spec.Containers, - getGatherContainer(image, audit, timeout, mustGather.Spec.Storage, trustedCAConfigMapName, command, args), + getGatherContainer(image, audit, timeout, mustGather.Spec.Storage, trustedCAConfigMapName, timeFilter, clusterCreationTime, command, args), ) // Add the upload container only if the upload target is specified @@ -192,7 +219,7 @@ func initializeJobTemplate(name string, namespace string, serviceAccountRef stri } } -func getGatherContainer(image string, audit bool, timeout time.Duration, storage *v1alpha1.Storage, trustedCAConfigMapName string, command []string, args []string) corev1.Container { +func getGatherContainer(image string, audit bool, timeout time.Duration, storage *v1alpha1.Storage, trustedCAConfigMapName string, timeFilter *GatherTimeFilter, clusterCreationTime *time.Time, command []string, args []string) corev1.Container { var commandBinary string if audit { commandBinary = gatherCommandBinaryAudit @@ -240,6 +267,44 @@ func getGatherContainer(image string, audit bool, timeout time.Duration, storage container.Args = args } + // Add time filter environment variables if specified + if timeFilter != nil { + // Clamp the time filter so it never precedes cluster creation time. + // - For Since (duration): ensure (now - since) >= clusterCreationTime by reducing Since to clusterAge. + // - For SinceTime (absolute): ensure sinceTime >= clusterCreationTime by bumping it up. + effectiveSince := timeFilter.Since + effectiveSinceTime := timeFilter.SinceTime + if clusterCreationTime != nil && !clusterCreationTime.IsZero() { + now := timeNow() + if effectiveSince > 0 { + clusterAge := now.Sub(*clusterCreationTime) + if clusterAge < 0 { + clusterAge = 0 + } + if effectiveSince > clusterAge { + effectiveSince = clusterAge + } + } + if effectiveSinceTime != nil && effectiveSinceTime.Before(*clusterCreationTime) { + t := *clusterCreationTime + effectiveSinceTime = &t + } + } + + if effectiveSince > 0 { + container.Env = append(container.Env, corev1.EnvVar{ + Name: gatherEnvSince, + Value: effectiveSince.String(), + }) + } + if effectiveSinceTime != nil { + container.Env = append(container.Env, corev1.EnvVar{ + Name: gatherEnvSinceTime, + Value: effectiveSinceTime.Format(time.RFC3339), + }) + } + } + return container } diff --git a/controllers/mustgather/template_test.go b/controllers/mustgather/template_test.go index 511dc675a..499d5f613 100644 --- a/controllers/mustgather/template_test.go +++ b/controllers/mustgather/template_test.go @@ -2,6 +2,7 @@ package mustgather import ( "fmt" + "math" "reflect" "strconv" "strings" @@ -9,7 +10,9 @@ import ( "time" mustgatherv1alpha1 "github.com/openshift/must-gather-operator/api/v1alpha1" + batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) const ( @@ -105,6 +108,8 @@ func Test_initializeJobTemplate(t *testing.T) { } func Test_getGatherContainer(t *testing.T) { + testSinceTime := time.Date(2026, 1, 7, 10, 0, 0, 0, time.UTC) + tests := []struct { name string audit bool @@ -113,6 +118,9 @@ func Test_getGatherContainer(t *testing.T) { storage *mustgatherv1alpha1.Storage command []string args []string + caConfigMap string + timeFilter *GatherTimeFilter + clusterCreation *time.Time }{ { name: "no audit", @@ -125,6 +133,12 @@ func Test_getGatherContainer(t *testing.T) { timeout: 0 * time.Second, mustGatherImage: "quay.io/foo/bar/must-gather:latest", }, + { + name: "with trusted CA config map", + timeout: 5 * time.Second, + mustGatherImage: "quay.io/foo/bar/must-gather:latest", + caConfigMap: "trusted-ca-cert-001", + }, { name: "with PVC", timeout: 5 * time.Second, @@ -140,7 +154,7 @@ func Test_getGatherContainer(t *testing.T) { }, { name: "robust timeout", - timeout: 6*time.Hour + 5*time.Minute + 3*time.Second, // 6h5m3s + timeout: 1500 * time.Millisecond, mustGatherImage: "quay.io/foo/bar/must-gather:latest", }, { @@ -150,10 +164,58 @@ func Test_getGatherContainer(t *testing.T) { command: []string{"/usr/bin/custom-gather"}, args: []string{"--verbose", "--subsystem=network"}, }, + { + name: "with since duration", + timeout: 5 * time.Second, + mustGatherImage: "quay.io/foo/bar/must-gather:latest", + timeFilter: &GatherTimeFilter{ + Since: 2 * time.Hour, + }, + }, + { + name: "with sinceTime", + timeout: 5 * time.Second, + mustGatherImage: "quay.io/foo/bar/must-gather:latest", + timeFilter: &GatherTimeFilter{ + SinceTime: &testSinceTime, + }, + }, + { + name: "clamp since duration to cluster age", + timeout: 5 * time.Second, + mustGatherImage: "quay.io/foo/bar/must-gather:latest", + timeFilter: &GatherTimeFilter{ + Since: 24 * time.Hour, + }, + clusterCreation: ToPtr(time.Date(2026, 1, 7, 9, 0, 0, 0, time.UTC)), + }, + { + name: "cluster age negative clamps to zero and omits since", + timeout: 5 * time.Second, + mustGatherImage: "quay.io/foo/bar/must-gather:latest", + timeFilter: &GatherTimeFilter{ + Since: 2 * time.Hour, + }, + // Cluster creation time is in the future relative to timeNow() below, forcing a negative age. + clusterCreation: ToPtr(time.Date(2026, 1, 7, 11, 0, 0, 0, time.UTC)), + }, + { + name: "clamp sinceTime to cluster creation time", + timeout: 5 * time.Second, + mustGatherImage: "quay.io/foo/bar/must-gather:latest", + timeFilter: &GatherTimeFilter{ + SinceTime: ToPtr(time.Date(2026, 1, 7, 8, 0, 0, 0, time.UTC)), + }, + clusterCreation: ToPtr(time.Date(2026, 1, 7, 9, 0, 0, 0, time.UTC)), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - container := getGatherContainer(tt.mustGatherImage, tt.audit, tt.timeout, tt.storage, "", tt.command, tt.args) + // Make time deterministic for clamping tests. + origNow := timeNow + timeNow = func() time.Time { return time.Date(2026, 1, 7, 10, 0, 0, 0, time.UTC) } + t.Cleanup(func() { timeNow = origNow }) + container := getGatherContainer(tt.mustGatherImage, tt.audit, tt.timeout, tt.storage, tt.caConfigMap, tt.timeFilter, tt.clusterCreation, tt.command, tt.args) if len(tt.command) == 0 { containerCommand := container.Command[2] @@ -162,8 +224,7 @@ func Test_getGatherContainer(t *testing.T) { } else if !tt.audit && !strings.Contains(containerCommand, gatherCommandBinaryNoAudit) { t.Fatalf("gather container command expected with binary %v but it wasn't present", gatherCommandBinaryNoAudit) } - - timeoutInSeconds := int(tt.timeout.Seconds()) + timeoutInSeconds := int(math.Ceil(tt.timeout.Seconds())) if !strings.HasPrefix(containerCommand, fmt.Sprintf("timeout %d", timeoutInSeconds)) { t.Fatalf("the duration was not properly added to the container command, got %v but wanted %v", strings.Split(containerCommand, " ")[1], timeoutInSeconds) } @@ -180,6 +241,26 @@ func Test_getGatherContainer(t *testing.T) { t.Fatalf("expected container image %v but got %v", tt.mustGatherImage, container.Image) } + // Check trusted CA configmap volume mount behavior + foundTrustedCAMount := false + for _, vm := range container.VolumeMounts { + if vm.Name == trustedCAVolumeName { + foundTrustedCAMount = true + if vm.MountPath != wellKnownCADirForTest { + t.Fatalf("trusted CA volume mount path was not correctly set. got %v, wanted %v", vm.MountPath, wellKnownCADirForTest) + } + if !vm.ReadOnly { + t.Fatalf("trusted CA volume mount expected to be read-only") + } + } + } + if tt.caConfigMap != "" && !foundTrustedCAMount { + t.Fatalf("expected trusted CA volume mount to be present when caConfigMap is provided") + } + if tt.caConfigMap == "" && foundTrustedCAMount { + t.Fatalf("did not expect trusted CA volume mount when caConfigMap is empty") + } + if tt.storage != nil { if len(container.VolumeMounts) == 0 { t.Fatalf("expected at least one volume mount when storage is provided") @@ -192,6 +273,34 @@ func Test_getGatherContainer(t *testing.T) { t.Fatalf("volume mount subpath was not correctly set. got %v, wanted %v", volumeMount.SubPath, tt.storage.PersistentVolume.SubPath) } } + + // Check time filter environment variables + if tt.timeFilter != nil { + envMap := envValues(container) + if tt.name == "clamp since duration to cluster age" { + if envMap[gatherEnvSince] != "1h0m0s" { + t.Fatalf("expected %s env var to be clamped to 1h0m0s, got %v", gatherEnvSince, envMap[gatherEnvSince]) + } + } else if tt.name == "cluster age negative clamps to zero and omits since" { + if _, ok := envMap[gatherEnvSince]; ok { + t.Fatalf("did not expect %s env var when clamped to zero, got %v", gatherEnvSince, envMap[gatherEnvSince]) + } + } else if tt.timeFilter.Since > 0 { + if envMap[gatherEnvSince] != tt.timeFilter.Since.String() { + t.Fatalf("expected %s env var to be %v, got %v", gatherEnvSince, tt.timeFilter.Since.String(), envMap[gatherEnvSince]) + } + } + if tt.name == "clamp sinceTime to cluster creation time" { + if envMap[gatherEnvSinceTime] != "2026-01-07T09:00:00Z" { + t.Fatalf("expected %s env var to be clamped to %v, got %v", gatherEnvSinceTime, "2026-01-07T09:00:00Z", envMap[gatherEnvSinceTime]) + } + } else if tt.timeFilter.SinceTime != nil { + expectedTime := tt.timeFilter.SinceTime.Format(time.RFC3339) + if envMap[gatherEnvSinceTime] != expectedTime { + t.Fatalf("expected %s env var to be %v, got %v", gatherEnvSinceTime, expectedTime, envMap[gatherEnvSinceTime]) + } + } + } }) } } @@ -324,3 +433,206 @@ func Test_getUploadContainer(t *testing.T) { }) } } + +func Test_getJobTemplate_GatherSpec_BuildsTimeFilter(t *testing.T) { + t.Setenv(DefaultMustGatherImageEnv, "quay.io/foo/bar/must-gather:latest") + + sinceTime := metav1.NewTime(time.Date(2026, 1, 7, 10, 11, 12, 0, time.UTC)) + + tests := []struct { + name string + gatherSpec *mustgatherv1alpha1.GatherSpec + wantSince string + wantSinceTs string + }{ + { + name: "no gatherSpec means no time filter env vars", + }, + { + name: "gatherSpec with since builds timeFilter.Since", + gatherSpec: &mustgatherv1alpha1.GatherSpec{Since: &metav1.Duration{Duration: 2 * time.Hour}}, + wantSince: "2h0m0s", + }, + { + name: "gatherSpec with sinceTime builds timeFilter.SinceTime", + gatherSpec: &mustgatherv1alpha1.GatherSpec{SinceTime: &sinceTime}, + wantSinceTs: "2026-01-07T10:11:12Z", + }, + { + name: "gatherSpec present but with no since/sinceTime means no time filter env vars", + gatherSpec: &mustgatherv1alpha1.GatherSpec{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mg := mustgatherv1alpha1.MustGather{ + ObjectMeta: metav1.ObjectMeta{Name: "mg", Namespace: "ns"}, + Spec: mustgatherv1alpha1.MustGatherSpec{ + ServiceAccountName: "default", + GatherSpec: tt.gatherSpec, + }, + } + + job := getJobTemplate("img", "operator-image", mg, "", nil) + gather := findGatherContainerInJob(t, job) + got := envValues(gather) + + if tt.wantSince == "" { + if _, ok := got[gatherEnvSince]; ok { + t.Fatalf("did not expect %s env var, got %v", gatherEnvSince, got[gatherEnvSince]) + } + } else if got[gatherEnvSince] != tt.wantSince { + t.Fatalf("expected %s=%s, got %s", gatherEnvSince, tt.wantSince, got[gatherEnvSince]) + } + + if tt.wantSinceTs == "" { + if _, ok := got[gatherEnvSinceTime]; ok { + t.Fatalf("did not expect %s env var, got %v", gatherEnvSinceTime, got[gatherEnvSinceTime]) + } + } else if got[gatherEnvSinceTime] != tt.wantSinceTs { + t.Fatalf("expected %s=%s, got %s", gatherEnvSinceTime, tt.wantSinceTs, got[gatherEnvSinceTime]) + } + }) + } +} + +func Test_getJobTemplate_ProxyAuditTimeout(t *testing.T) { + t.Setenv(DefaultMustGatherImageEnv, "quay.io/foo/bar/must-gather:latest") + + timeout := metav1.Duration{Duration: 5 * time.Second} + + tests := []struct { + name string + audit bool + timeout *metav1.Duration + httpProxy string + httpsProxy string + noProxy string + wantAudit bool + wantTimeout string + wantProxies bool + }{ + { + name: "audit false and nil timeout default; no proxy env vars", + wantAudit: false, + wantTimeout: "timeout 0", + wantProxies: false, + }, + { + name: "audit true and timeout set; proxy env vars propagate to upload container", + audit: true, + timeout: &timeout, + httpProxy: "http://proxy.example:8080", + httpsProxy: "https://proxy.example:8443", + noProxy: "127.0.0.1,localhost,.cluster.local", + wantAudit: true, + wantTimeout: "timeout 5", + wantProxies: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Always set proxy vars per test case to avoid leakage from host env. + t.Setenv("HTTP_PROXY", tt.httpProxy) + t.Setenv("HTTPS_PROXY", tt.httpsProxy) + t.Setenv("NO_PROXY", tt.noProxy) + + mg := mustgatherv1alpha1.MustGather{ + ObjectMeta: metav1.ObjectMeta{Name: "mg", Namespace: "ns"}, + Spec: mustgatherv1alpha1.MustGatherSpec{ + ServiceAccountName: "default", + MustGatherTimeout: tt.timeout, + GatherSpec: &mustgatherv1alpha1.GatherSpec{ + Audit: tt.audit, + }, + UploadTarget: &mustgatherv1alpha1.UploadTargetSpec{ + Type: mustgatherv1alpha1.UploadTypeSFTP, + SFTP: &mustgatherv1alpha1.SFTPSpec{ + CaseID: "1234", + Host: "sftp.example.com", + CaseManagementAccountSecretRef: v1.LocalObjectReference{ + Name: "case-mgmt-secret", + }, + }, + }, + }, + } + + job := getJobTemplate("image", "operator-image", mg, "", nil) + + gather := findGatherContainerInJob(t, job) + gatherCmd := gather.Command[2] + if tt.wantAudit { + if !strings.Contains(gatherCmd, gatherCommandBinaryAudit) { + t.Fatalf("expected gather command to contain %v but got %v", gatherCommandBinaryAudit, gatherCmd) + } + } else { + if !strings.Contains(gatherCmd, gatherCommandBinaryNoAudit) { + t.Fatalf("expected gather command to contain %v but got %v", gatherCommandBinaryNoAudit, gatherCmd) + } + } + if !strings.HasPrefix(gatherCmd, tt.wantTimeout) { + t.Fatalf("expected gather command to start with %q but got %q", tt.wantTimeout, gatherCmd) + } + + upload := findUploadContainerInJob(t, job) + uploadEnv := envValues(upload) + if tt.wantProxies { + if uploadEnv[uploadEnvHttpProxy] != tt.httpProxy { + t.Fatalf("expected %s=%v, got %v", uploadEnvHttpProxy, tt.httpProxy, uploadEnv[uploadEnvHttpProxy]) + } + if uploadEnv[uploadEnvHttpsProxy] != tt.httpsProxy { + t.Fatalf("expected %s=%v, got %v", uploadEnvHttpsProxy, tt.httpsProxy, uploadEnv[uploadEnvHttpsProxy]) + } + if uploadEnv[uploadEnvNoProxy] != tt.noProxy { + t.Fatalf("expected %s=%v, got %v", uploadEnvNoProxy, tt.noProxy, uploadEnv[uploadEnvNoProxy]) + } + } else { + if _, ok := uploadEnv[uploadEnvHttpProxy]; ok { + t.Fatalf("did not expect %s env var, got %v", uploadEnvHttpProxy, uploadEnv[uploadEnvHttpProxy]) + } + if _, ok := uploadEnv[uploadEnvHttpsProxy]; ok { + t.Fatalf("did not expect %s env var, got %v", uploadEnvHttpsProxy, uploadEnv[uploadEnvHttpsProxy]) + } + if _, ok := uploadEnv[uploadEnvNoProxy]; ok { + t.Fatalf("did not expect %s env var, got %v", uploadEnvNoProxy, uploadEnv[uploadEnvNoProxy]) + } + } + }) + } +} + +// helper to find gather container in a job +func findGatherContainerInJob(t *testing.T, job *batchv1.Job) v1.Container { + t.Helper() + for _, c := range job.Spec.Template.Spec.Containers { + if c.Name == gatherContainerName { + return c + } + } + t.Fatalf("gather container not found in job") + return v1.Container{} +} + +// helper to find upload container in a job +func findUploadContainerInJob(t *testing.T, job *batchv1.Job) v1.Container { + t.Helper() + for _, c := range job.Spec.Template.Spec.Containers { + if c.Name == uploadContainerName { + return c + } + } + t.Fatalf("upload container not found in job") + return v1.Container{} +} + +// helper to map env name->value +func envValues(container v1.Container) map[string]string { + m := make(map[string]string) + for _, e := range container.Env { + m[e.Name] = e.Value + } + return m +} diff --git a/deploy/02_must-gather-operator.ClusterRole.yaml b/deploy/02_must-gather-operator.ClusterRole.yaml index f52769023..6027e1c0a 100644 --- a/deploy/02_must-gather-operator.ClusterRole.yaml +++ b/deploy/02_must-gather-operator.ClusterRole.yaml @@ -10,8 +10,9 @@ rules: - clusterversions - clusterversions/status verbs: - - list - get + - list + - watch # leader election - apiGroups: - "" diff --git a/deploy/crds/operator.openshift.io_mustgathers.yaml b/deploy/crds/operator.openshift.io_mustgathers.yaml index 09ad43239..3945afdbf 100644 --- a/deploy/crds/operator.openshift.io_mustgathers.yaml +++ b/deploy/crds/operator.openshift.io_mustgathers.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.18.1-0.20250818134112-b624019bbe8d + controller-gen.kubebuilder.io/version: v0.19.0 name: mustgathers.operator.openshift.io spec: group: operator.openshift.io @@ -41,7 +41,8 @@ spec: properties: gatherSpec: description: |- - GatherSpec allows overriding the command and/or arguments for the custom must-gather image. + GatherSpec allows overriding the command and/or arguments for the custom must-gather image + and configures time-based collection filters. This field is ignored if ImageStreamRef is not specified. properties: args: @@ -66,7 +67,24 @@ spec: type: string maxItems: 256 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. + 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. + Only one of since or sinceTime may be specified. + format: date-time + type: string type: object + x-kubernetes-validations: + - message: only one of since or sinceTime may be specified + rule: '!(has(self.since) && has(self.sinceTime))' imageStreamRef: description: |- ImageStreamRef specifies a custom image from the allowlist to be used for the @@ -213,8 +231,9 @@ spec: x-kubernetes-validations: - message: command and args in gatherSpec can only be set when imageStreamRef is specified - rule: '!(has(self.gatherSpec) && (size(self.gatherSpec.command) > 0 - || size(self.gatherSpec.args) > 0)) || has(self.imageStreamRef)' + rule: '!(has(self.gatherSpec) && ((has(self.gatherSpec.command) && size(self.gatherSpec.command) + > 0) || (has(self.gatherSpec.args) && size(self.gatherSpec.args) > + 0))) || has(self.imageStreamRef)' status: description: MustGatherStatus defines the observed state of MustGather properties: diff --git a/examples/mustgather_with_since.yaml b/examples/mustgather_with_since.yaml new file mode 100644 index 000000000..5a9df791d --- /dev/null +++ b/examples/mustgather_with_since.yaml @@ -0,0 +1,16 @@ +apiVersion: operator.openshift.io/v1alpha1 +kind: MustGather +metadata: + name: mustgather-with-since +spec: + # Only collect logs from the last 10 minutes (reduces output size significantly) + gatherSpec: + since: 10m + serviceAccountName: must-gather-admin + uploadTarget: + type: SFTP + sftp: + caseID: '00006' + caseManagementAccountSecretRef: + name: case-management-creds + internalUser: true \ No newline at end of file diff --git a/examples/mustgather_with_sincetime.yaml b/examples/mustgather_with_sincetime.yaml new file mode 100644 index 000000000..63f18eb38 --- /dev/null +++ b/examples/mustgather_with_sincetime.yaml @@ -0,0 +1,16 @@ +apiVersion: operator.openshift.io/v1alpha1 +kind: MustGather +metadata: + name: mustgather-with-sincetime +spec: + gatherSpec: + sinceTime: "2026-02-02T00:00:00Z" + serviceAccountName: must-gather-admin + uploadTarget: + type: SFTP + sftp: + caseID: '00000' + caseManagementAccountSecretRef: + name: case-management-creds + internalUser: true +