Skip to content

Commit

Permalink
OCPBUGS-6731: Anonymize env vars from containers: HTTP_PROXY, HTTPS_P…
Browse files Browse the repository at this point in the history
…ROXY (#723)

* WIP draft overwriting env vars values

* Add unit test for env var obfuscation on container images gatherer

* Refactor obfuscate env vars functionality

* Fix obfusctation functionality and tests lint issues

* Move sensitive env vars obfuscation logic to anonymize utils package

* Add env vars obfuscation to pod recording

* Use assert library

* Add PR 723 Obfuscate HTTP_PROXY and HTTPS_PROXY

* Fix PR 723 type to bugfix
  • Loading branch information
ncaak authored Feb 16, 2023
1 parent 70c099c commit d9f6cc0
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 0 deletions.
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

### 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 @@ -78,6 +79,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 @@ -235,6 +236,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 @@ -97,6 +97,7 @@ func getClusterVersion(ctx context.Context,
}
for i := range pods.Items {
pod := &pods.Items[i]
anonymize.SensitiveEnvVars(pod.Spec.Containers)

records = append(records, record.Record{
Name: fmt.Sprintf("config/pod/%s/%s", pod.Namespace, pod.Name),
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
Expand Down Expand Up @@ -93,6 +94,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)
})
}

0 comments on commit d9f6cc0

Please sign in to comment.