diff --git a/CLAUDE.md b/CLAUDE.md index 55f99b3f2..2afe72f4e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,7 +68,7 @@ oc apply -f ./test/must-gather.yaml **API Types** (`api/v1alpha1/mustgather_types.go`): - `MustGather` CR defines the specification for must-gather collection jobs -- Key fields: `caseID`, `caseManagementAccountSecretRef`, `serviceAccountRef`, `audit`, `proxyConfig`, `mustGatherTimeout`, `internalUser` +- Key fields: `caseID`, `caseManagementAccountSecretRef`, `serviceAccountRef`, `audit`, `mustGatherTimeout`, `internalUser` - Status tracking with conditions and completion state **Controller** (`controllers/mustgather/mustgather_controller.go`): @@ -94,7 +94,7 @@ oc apply -f ./test/must-gather.yaml ### Reconciliation Flow 1. Fetch MustGather instance -2. Initialize defaults (ServiceAccountRef, ProxyConfig from cluster) +2. Initialize defaults (ServiceAccountRef from cluster) 3. Handle deletion via finalizer: - Delete secret from operator namespace - Delete job and associated pods @@ -114,7 +114,7 @@ oc apply -f ./test/must-gather.yaml - **Two-container approach**: Separate containers for gathering and uploading allows gather to run with cluster permissions while upload runs with limited permissions - **Process namespace sharing**: Enables upload container to detect gather completion via `pgrep` - **Infra node affinity**: Jobs prefer infra nodes (with tolerations) to avoid impacting application workloads -- **Proxy support**: Inherits cluster proxy config by default, overridable per MustGather CR +- **Proxy support**: Inherits cluster proxy config from environment variables - **FIPS mode**: Enabled by default (`FIPS_ENABLED=true` in Makefile) ### Important Files @@ -133,7 +133,7 @@ Required for operation: Optional: - `OSDK_FORCE_RUN_MODE=local`: Bypasses leader election for local development -- Proxy variables: `HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY` (can be overridden per CR) +- Proxy variables: `HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY` ### API Group Migration diff --git a/README.md b/README.md index 1e4ff0250..4ee33d904 100644 --- a/README.md +++ b/README.md @@ -41,30 +41,6 @@ spec: audit: true ``` -## Proxy Support - -The Must Gather operator supports using a proxy. The proxy setting can be specified in the MustGather object. If not specified, the cluster default proxy setting will be used. Here is an example: - -```yaml -apiVersion: operator.openshift.io/v1alpha1 -kind: MustGather -metadata: - name: example-mustgather-proxy -spec: - serviceAccountRef: - name: must-gather-admin - uploadTarget: - type: SFTP - sftp: - caseID: '02527285' - caseManagementAccountSecretRef: - name: case-management-creds - proxyConfig: - httpProxy: http://myproxy - httpsProxy: https://my_http_proxy - noProxy: master-api -``` - ## Garbage collection MustGather instances are cleaned up by the Must Gather operator about 6 hours after completion, regardless of whether they were successful. diff --git a/api/v1alpha1/mustgather_types.go b/api/v1alpha1/mustgather_types.go index 7b99fa641..23502279c 100644 --- a/api/v1alpha1/mustgather_types.go +++ b/api/v1alpha1/mustgather_types.go @@ -36,10 +36,6 @@ type MustGatherSpec struct { // +kubebuilder:default:=false Audit *bool `json:"audit,omitempty"` - // This represents the proxy configuration to be used. If left empty it will default to the cluster-level proxy configuration. - // +kubebuilder:validation:Optional - ProxyConfig *ProxySpec `json:"proxyConfig,omitempty"` - // A time limit for gather command to complete a floating point number with a suffix: // "s" for seconds, "m" for minutes, "h" for hours. // Will default to no time limit. @@ -153,21 +149,6 @@ type PersistentVolumeClaimReference struct { Name string `json:"name"` } -// +k8s:openapi-gen=true -type ProxySpec struct { - // httpProxy is the URL of the proxy for HTTP requests. - // +kubebuilder:validation:Required - HTTPProxy string `json:"httpProxy"` - - // httpsProxy is the URL of the proxy for HTTPS requests. - // +kubebuilder:validation:Required - HTTPSProxy string `json:"httpsProxy"` - - // noProxy is the list of domains for which the proxy should not be used. Empty means unset and will not result in an env var. - // +optional - NoProxy string `json:"noProxy,omitempty"` -} - // MustGatherStatus defines the observed state of MustGather type MustGatherStatus struct { Status string `json:"status,omitempty"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2c67362d9..a628f986c 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -76,11 +76,6 @@ func (in *MustGatherSpec) DeepCopyInto(out *MustGatherSpec) { *out = new(bool) **out = **in } - if in.ProxyConfig != nil { - in, out := &in.ProxyConfig, &out.ProxyConfig - *out = new(ProxySpec) - **out = **in - } if in.MustGatherTimeout != nil { in, out := &in.MustGatherTimeout, &out.MustGatherTimeout *out = new(v1.Duration) @@ -167,21 +162,6 @@ func (in *PersistentVolumeConfig) DeepCopy() *PersistentVolumeConfig { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ProxySpec) DeepCopyInto(out *ProxySpec) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProxySpec. -func (in *ProxySpec) DeepCopy() *ProxySpec { - if in == nil { - return nil - } - out := new(ProxySpec) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SFTPSpec) DeepCopyInto(out *SFTPSpec) { *out = *in diff --git a/api/v1alpha1/zz_generated.openapi.go b/api/v1alpha1/zz_generated.openapi.go index c4e4c659e..3c2db349c 100644 --- a/api/v1alpha1/zz_generated.openapi.go +++ b/api/v1alpha1/zz_generated.openapi.go @@ -3,53 +3,12 @@ // Code generated by openapi-gen. DO NOT EDIT. -// This file was autogenerated by openapi-gen. Do not edit it manually! - package v1alpha1 import ( common "k8s.io/kube-openapi/pkg/common" - spec "k8s.io/kube-openapi/pkg/validation/spec" ) func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenAPIDefinition { - return map[string]common.OpenAPIDefinition{ - "github.com/openshift/must-gather-operator/api/v1alpha1.ProxySpec": schema_openshift_must_gather_operator_api_v1alpha1_ProxySpec(ref), - } -} - -func schema_openshift_must_gather_operator_api_v1alpha1_ProxySpec(ref common.ReferenceCallback) common.OpenAPIDefinition { - return common.OpenAPIDefinition{ - Schema: spec.Schema{ - SchemaProps: spec.SchemaProps{ - Type: []string{"object"}, - Properties: map[string]spec.Schema{ - "httpProxy": { - SchemaProps: spec.SchemaProps{ - Description: "httpProxy is the URL of the proxy for HTTP requests.", - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "httpsProxy": { - SchemaProps: spec.SchemaProps{ - Description: "httpsProxy is the URL of the proxy for HTTPS requests.", - Default: "", - Type: []string{"string"}, - Format: "", - }, - }, - "noProxy": { - SchemaProps: spec.SchemaProps{ - Description: "noProxy is the list of domains for which the proxy should not be used. Empty means unset and will not result in an env var.", - Type: []string{"string"}, - Format: "", - }, - }, - }, - Required: []string{"httpProxy", "httpsProxy"}, - }, - }, - } + return map[string]common.OpenAPIDefinition{} } diff --git a/boilerplate/openshift/golang-osd-operator/standard.mk b/boilerplate/openshift/golang-osd-operator/standard.mk index a77c9e87a..980871ab4 100644 --- a/boilerplate/openshift/golang-osd-operator/standard.mk +++ b/boilerplate/openshift/golang-osd-operator/standard.mk @@ -216,13 +216,14 @@ op-generate: .PHONY: openapi-generate openapi-generate: find ./api -maxdepth 2 -mindepth 1 -type d | xargs -t -I% \ - $(OPENAPI_GEN) --logtostderr=true \ - -i % \ - -o "" \ - -O zz_generated.openapi \ - -p % \ - -h /dev/null \ - -r "-" + $(OPENAPI_GEN) \ + --logtostderr=true \ + --output-dir % \ + --output-pkg % \ + --output-file zz_generated.openapi.go \ + --go-header-file /dev/null \ + --report-filename - \ + % .PHONY: manifests manifests: diff --git a/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml b/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml index 6301bc909..8bc0858b0 100644 --- a/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml +++ b/bundle/manifests/tech-preview/operator.openshift.io_mustgathers.yaml @@ -52,25 +52,6 @@ spec: Will default to no time limit. format: duration type: string - proxyConfig: - description: This represents the proxy configuration to be used. If - left empty it will default to the cluster-level proxy configuration. - properties: - httpProxy: - description: httpProxy is the URL of the proxy for HTTP requests. - type: string - httpsProxy: - description: httpsProxy is the URL of the proxy for HTTPS requests. - type: string - noProxy: - description: noProxy is the list of domains for which the proxy - should not be used. Empty means unset and will not result in - an env var. - type: string - required: - - httpProxy - - httpsProxy - type: object retainResourcesOnCompletion: default: false description: |- diff --git a/controllers/mustgather/template.go b/controllers/mustgather/template.go index f0f2d1c48..9c5ea811f 100644 --- a/controllers/mustgather/template.go +++ b/controllers/mustgather/template.go @@ -56,29 +56,17 @@ func getJobTemplate(operatorImage string, mustGather v1alpha1.MustGather) *batch var httpProxy, httpsProxy, noProxy string - // Check if proxy configuration is provided in the CR - if mustGather.Spec.ProxyConfig != nil { - if mustGather.Spec.ProxyConfig.HTTPProxy != "" || mustGather.Spec.ProxyConfig.HTTPSProxy != "" || mustGather.Spec.ProxyConfig.NoProxy != "" { - // Use proxy configuration from CR - httpProxy = mustGather.Spec.ProxyConfig.HTTPProxy - httpsProxy = mustGather.Spec.ProxyConfig.HTTPSProxy - noProxy = mustGather.Spec.ProxyConfig.NoProxy - } - } - - // Fallback to operator's environment proxy variables only if not provided in the CR - if httpProxy == "" && httpsProxy == "" { - envVars := proxy.ReadProxyVarsFromEnv() - // the below loop should implicitly handle len(envVars) > 0 - for _, envVar := range envVars { - switch envVar.Name { - case "HTTP_PROXY": - httpProxy = envVar.Value - case "HTTPS_PROXY": - httpsProxy = envVar.Value - case "NO_PROXY": - noProxy = envVar.Value - } + // Use operator's environment proxy variables + envVars := proxy.ReadProxyVarsFromEnv() + // the below loop should implicitly handle len(envVars) > 0 + for _, envVar := range envVars { + switch envVar.Name { + case "HTTP_PROXY": + httpProxy = envVar.Value + case "HTTPS_PROXY": + httpsProxy = envVar.Value + case "NO_PROXY": + noProxy = envVar.Value } } diff --git a/controllers/mustgather/template_test.go b/controllers/mustgather/template_test.go index 867d53ede..f4bd91286 100644 --- a/controllers/mustgather/template_test.go +++ b/controllers/mustgather/template_test.go @@ -2,7 +2,6 @@ package mustgather import ( "fmt" - "os" "reflect" "strconv" "strings" @@ -10,9 +9,7 @@ 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" ) func Test_initializeJobTemplate(t *testing.T) { @@ -257,155 +254,3 @@ func Test_getUploadContainer(t *testing.T) { }) } } - -// 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 -} - -func Test_getJobTemplate_FallbackWhenOnlyNoProxyProvidedInCR(t *testing.T) { - _ = os.Setenv("HTTP_PROXY", "http://env-http:8080") - _ = os.Setenv("HTTPS_PROXY", "https://env-https:8443") - _ = os.Setenv("NO_PROXY", "env-no-proxy") - defer func() { - _ = os.Unsetenv("HTTP_PROXY") - _ = os.Unsetenv("HTTPS_PROXY") - _ = os.Unsetenv("NO_PROXY") - }() - - mg := mustgatherv1alpha1.MustGather{ - ObjectMeta: metav1.ObjectMeta{Name: "mg", Namespace: "ns"}, - Spec: mustgatherv1alpha1.MustGatherSpec{ - ServiceAccountName: "default", - MustGatherTimeout: &metav1.Duration{Duration: 1 * time.Hour}, - ProxyConfig: &mustgatherv1alpha1.ProxySpec{ - NoProxy: "cr-no-proxy", - }, - UploadTarget: &mustgatherv1alpha1.UploadTargetSpec{ - Type: mustgatherv1alpha1.UploadTypeSFTP, - SFTP: &mustgatherv1alpha1.SFTPSpec{ - CaseID: "case", - CaseManagementAccountSecretRef: v1.LocalObjectReference{Name: "sec"}, - }, - }, - }, - } - - job := getJobTemplate("img", mg) - upload := findUploadContainerInJob(t, job) - got := envValues(upload) - - if got[uploadEnvHttpProxy] != "http://env-http:8080" { - t.Fatalf("expected %s from env, got %s", uploadEnvHttpProxy, got[uploadEnvHttpProxy]) - } - if got[uploadEnvHttpsProxy] != "https://env-https:8443" { - t.Fatalf("expected %s from env, got %s", uploadEnvHttpsProxy, got[uploadEnvHttpsProxy]) - } - if got[uploadEnvNoProxy] != "env-no-proxy" { - t.Fatalf("expected %s from env, got %s", uploadEnvNoProxy, got[uploadEnvNoProxy]) - } -} - -func Test_getJobTemplate_NoFallbackWhenHttpAndHttpsProvidedInCR(t *testing.T) { - _ = os.Setenv("HTTP_PROXY", "http://env-http:8080") - _ = os.Setenv("HTTPS_PROXY", "https://env-https:8443") - _ = os.Setenv("NO_PROXY", "env-no-proxy") - defer func() { - _ = os.Unsetenv("HTTP_PROXY") - _ = os.Unsetenv("HTTPS_PROXY") - _ = os.Unsetenv("NO_PROXY") - }() - - mg := mustgatherv1alpha1.MustGather{ - ObjectMeta: metav1.ObjectMeta{Name: "mg", Namespace: "ns"}, - Spec: mustgatherv1alpha1.MustGatherSpec{ - ServiceAccountName: "default", - MustGatherTimeout: &metav1.Duration{Duration: 1 * time.Hour}, - ProxyConfig: &mustgatherv1alpha1.ProxySpec{ - HTTPProxy: "http://cr-http:8080", - HTTPSProxy: "https://cr-https:8443", - // NoProxy intentionally empty - }, - UploadTarget: &mustgatherv1alpha1.UploadTargetSpec{ - Type: mustgatherv1alpha1.UploadTypeSFTP, - SFTP: &mustgatherv1alpha1.SFTPSpec{ - CaseID: "case", - CaseManagementAccountSecretRef: v1.LocalObjectReference{Name: "sec"}, - }, - }, - }, - } - - job := getJobTemplate("img", mg) - upload := findUploadContainerInJob(t, job) - got := envValues(upload) - - if got[uploadEnvHttpProxy] != "http://cr-http:8080" { - t.Fatalf("expected %s to be CR value, got %s", uploadEnvHttpProxy, got[uploadEnvHttpProxy]) - } - if got[uploadEnvHttpsProxy] != "https://cr-https:8443" { - t.Fatalf("expected %s to be CR value, got %s", uploadEnvHttpsProxy, got[uploadEnvHttpsProxy]) - } - if _, ok := got[uploadEnvNoProxy]; ok { - t.Fatalf("did not expect %s when CR NoProxy is empty", uploadEnvNoProxy) - } -} - -func Test_getJobTemplate_NoFallbackIfHttpsProvidedButHttpMissing(t *testing.T) { - _ = os.Setenv("HTTP_PROXY", "http://env-http:8080") - _ = os.Setenv("HTTPS_PROXY", "https://env-https:8443") - _ = os.Setenv("NO_PROXY", "env-no-proxy") - defer func() { - _ = os.Unsetenv("HTTP_PROXY") - _ = os.Unsetenv("HTTPS_PROXY") - _ = os.Unsetenv("NO_PROXY") - }() - - mg := mustgatherv1alpha1.MustGather{ - ObjectMeta: metav1.ObjectMeta{Name: "mg", Namespace: "ns"}, - Spec: mustgatherv1alpha1.MustGatherSpec{ - ServiceAccountName: "default", - MustGatherTimeout: &metav1.Duration{Duration: 1 * time.Hour}, - ProxyConfig: &mustgatherv1alpha1.ProxySpec{ - HTTPSProxy: "https://cr-https:8443", - // HTTPProxy empty to ensure fallback condition is false - }, - UploadTarget: &mustgatherv1alpha1.UploadTargetSpec{ - Type: mustgatherv1alpha1.UploadTypeSFTP, - SFTP: &mustgatherv1alpha1.SFTPSpec{ - CaseID: "case", - CaseManagementAccountSecretRef: v1.LocalObjectReference{Name: "sec"}, - }, - }, - }, - } - - job := getJobTemplate("img", mg) - upload := findUploadContainerInJob(t, job) - got := envValues(upload) - - // http proxy should not be present (no fallback) - if _, ok := got[uploadEnvHttpProxy]; ok { - t.Fatalf("did not expect %s when only HTTPS proxy is provided in CR", uploadEnvHttpProxy) - } - // https proxy should be from CR - if got[uploadEnvHttpsProxy] != "https://cr-https:8443" { - t.Fatalf("expected %s to be CR value, got %s", uploadEnvHttpsProxy, got[uploadEnvHttpsProxy]) - } -} diff --git a/deploy/crds/operator.openshift.io_mustgathers.yaml b/deploy/crds/operator.openshift.io_mustgathers.yaml index 6301bc909..4664e6dda 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 @@ -52,25 +52,6 @@ spec: Will default to no time limit. format: duration type: string - proxyConfig: - description: This represents the proxy configuration to be used. If - left empty it will default to the cluster-level proxy configuration. - properties: - httpProxy: - description: httpProxy is the URL of the proxy for HTTP requests. - type: string - httpsProxy: - description: httpsProxy is the URL of the proxy for HTTPS requests. - type: string - noProxy: - description: noProxy is the list of domains for which the proxy - should not be used. Empty means unset and will not result in - an env var. - type: string - required: - - httpProxy - - httpsProxy - type: object retainResourcesOnCompletion: default: false description: |- diff --git a/examples/mustgather_proxy.yaml b/examples/mustgather_proxy.yaml deleted file mode 100644 index 5252f433b..000000000 --- a/examples/mustgather_proxy.yaml +++ /dev/null @@ -1,17 +0,0 @@ -apiVersion: operator.openshift.io/v1alpha1 -kind: MustGather -metadata: - name: example-mustgather-proxy -spec: - serviceAccountRef: - name: must-gather-admin - uploadTarget: - type: SFTP - sftp: - caseID: '02527285' - caseManagementAccountSecretRef: - name: case-management-creds - proxyConfig: - httpProxy: http://myproxy - httpsProxy: https://my_http_proxy - noProxy: master-api