Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPBUGS-6731: Anonymize env vars from containers: HTTP_PROXY, HTTPS_PROXY #723

3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
Please see OpenShift release notes for official changes\n<!--Latest hash: 76cc46b739c27281db0f7b2a23eb091b2a70f698-->
## 4.13

### Bugfix
- [#723](https://github.com/openshift/insights-operator/pull/723) Obfuscate HTTP_PROXY and HTTPS_PROXY env variables on containers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First time someone updated changelog with a new PR. :) This is not a new data enhancement, but rather bugfix or other (and yes it corresponds to the PR template selection)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New data enhancement is e.g new gatherer for XYZ.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, updating it right now to BugFix. Thanks.


### Others
- [#693](https://github.com/openshift/insights-operator/pull/693) Use cgroups memory usage data in the archive metadata

Expand Down
3 changes: 3 additions & 0 deletions pkg/gatherers/clusterconfig/container_images.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"k8s.io/klog/v2"

"github.com/openshift/insights-operator/pkg/record"
"github.com/openshift/insights-operator/pkg/utils/anonymize"
"github.com/openshift/insights-operator/pkg/utils/check"
"github.com/openshift/library-go/pkg/image/reference"
)
Expand Down Expand Up @@ -68,6 +69,8 @@ func gatherContainerImages(ctx context.Context, coreClient corev1client.CoreV1In
for podIndex, pod := range pods.Items { //nolint:gocritic
podPtr := &pods.Items[podIndex]
if strings.HasPrefix(pod.Namespace, "openshift-") && check.HasContainerInCrashloop(podPtr) {
anonymize.SensitiveEnvVars(podPtr.Spec.Containers)

records = append(records, record.Record{
Name: fmt.Sprintf("config/pod/%s/%s", pod.Namespace, pod.Name),
Item: record.ResourceMarshaller{Resource: podPtr},
Expand Down
3 changes: 3 additions & 0 deletions pkg/gatherers/clusterconfig/operators_pods_and_events.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/openshift/insights-operator/pkg/record"
"github.com/openshift/insights-operator/pkg/recorder"
"github.com/openshift/insights-operator/pkg/utils"
"github.com/openshift/insights-operator/pkg/utils/anonymize"
"github.com/openshift/insights-operator/pkg/utils/check"
"github.com/openshift/insights-operator/pkg/utils/marshal"
)
Expand Down Expand Up @@ -217,6 +218,8 @@ func gatherPodsAndTheirContainersLogs(ctx context.Context,
for _, pod := range pods {
// if pod is not healthy then record its definition and try to get previous log
if !check.IsHealthyPod(pod, time.Now()) {
anonymize.SensitiveEnvVars(pod.Spec.Containers)

records = append(records, record.Record{
Name: fmt.Sprintf("config/pod/%s/%s", pod.Namespace, pod.Name),
Item: record.ResourceMarshaller{Resource: pod},
Expand Down
1 change: 1 addition & 0 deletions pkg/gatherers/clusterconfig/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ func getClusterVersion(ctx context.Context,
}
for i := range pods.Items {
pod := &pods.Items[i]
anonymize.SensitiveEnvVars(pod.Spec.Containers)

// TODO: shift after IsHealthyPod
records = append(records, record.Record{
Expand Down
2 changes: 2 additions & 0 deletions pkg/gatherers/conditional/gather_pod_definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/openshift/insights-operator/pkg/gatherers"
"github.com/openshift/insights-operator/pkg/record"
"github.com/openshift/insights-operator/pkg/utils/anonymize"
)

// BuildGatherPodDefinition collects pod definition from pods that are firing one of the configured alerts.
Expand Down Expand Up @@ -76,6 +77,7 @@ func (g *Gatherer) gatherPodDefinition(
errs = append(errs, err)
continue
}
anonymize.SensitiveEnvVars(pod.Spec.Containers)

records = append(records, record.Record{
Name: fmt.Sprintf(
Expand Down
23 changes: 23 additions & 0 deletions pkg/utils/anonymize/envvars.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package anonymize

import (
"regexp"
"strings"

corev1 "k8s.io/api/core/v1"
)

// SensitiveEnvVars finds env variables within the given container list
// and, if they are a target, it will obfuscate their value
func SensitiveEnvVars(containers []corev1.Container) {
targets := []string{"HTTP_PROXY", "HTTPS_PROXY"}
search := regexp.MustCompile(strings.Join(targets, "|"))

for i := range containers {
for j := range containers[i].Env {
if search.MatchString(containers[i].Env[j].Name) {
containers[i].Env[j].Value = String(containers[i].Env[j].Value)
}
}
}
}
39 changes: 39 additions & 0 deletions pkg/utils/anonymize/envvars_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package anonymize

import (
"testing"

"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
)

func Test_EnvVar_Obfuscation(t *testing.T) {
// Given
mock := []corev1.Container{
{
Env: []corev1.EnvVar{
{Name: "NO_TARGET", Value: "original_value"},
{Name: "HTTP_PROXY", Value: "original_value"},
{Name: "HTTPS_PROXY", Value: "original_value"},
},
},
}
envOriginalValue := "original_value"

// When
SensitiveEnvVars(mock)

// Assert
t.Run("Non target env vars keep their original value", func(t *testing.T) {
test := mock[0].Env[0]
assert.Equal(t, envOriginalValue, test.Value)
})
t.Run("HTTP_PROXY is updated with obfuscated value", func(t *testing.T) {
test := mock[0].Env[1]
assert.NotEqual(t, envOriginalValue, test.Value)
})
t.Run("HTTPS_PROXY is updated with obfuscated value", func(t *testing.T) {
test := mock[0].Env[2]
assert.NotEqual(t, envOriginalValue, test.Value)
})
}