Skip to content

[release-5.9]LOG-6963:Map cluster-wide proxy env variable for compatibility with Vector expectations #3032

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

Open
wants to merge 1 commit into
base: release-5.9
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion internal/collector/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ func (f *Factory) NewCollectorContainer(inputs logging.InputSpecs, secretNames [
{Name: "POD_IP", ValueFrom: &v1.EnvVarSource{FieldRef: &v1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "status.podIP"}}},
{Name: "POD_IPS", ValueFrom: &v1.EnvVarSource{FieldRef: &v1.ObjectFieldSelector{APIVersion: "v1", FieldPath: "status.podIPs"}}},
}
collector.Env = append(collector.Env, utils.GetProxyEnvVars()...)

collector.Env = append(collector.Env, utils.GetProxyEnvVars(f.CollectorSpec.Type)...)

collector.VolumeMounts = []v1.VolumeMount{
{Name: f.ResourceNames.SecretMetrics, ReadOnly: true, MountPath: metricsVolumePath},
Expand Down
120 changes: 41 additions & 79 deletions internal/collector/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,39 +173,48 @@ var _ = Describe("Factory#Daemonset", func() {
Fail(fmt.Sprintf("Exp. collector to include env var: %s", name))
}

It("should add the proxy variables to the collector", func() {
_httpProxy := os.Getenv("http_proxy")
_httpsProxy := os.Getenv("https_proxy")
_noProxy := os.Getenv("no_proxy")
cleanup := func() {
_ = os.Setenv("http_proxy", _httpProxy)
_ = os.Setenv("https_proxy", _httpsProxy)
_ = os.Setenv("no_proxy", _noProxy)
}
defer cleanup()

httpproxy := "http://[email protected]/3128/"
noproxy := ".cluster.local,localhost"
_ = os.Setenv("http_proxy", httpproxy)
_ = os.Setenv("https_proxy", httpproxy)
_ = os.Setenv("no_proxy", noproxy)
caBundle := "-----BEGIN CERTIFICATE-----\n<PEM_ENCODED_CERT>\n-----END CERTIFICATE-----\n"
podSpec = *factory.NewPodSpec(&v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: "openshift-logging",
Name: constants.CollectorTrustedCAName,
},
Data: map[string]string{
constants.TrustedCABundleKey: caBundle,
},
}, logging.ClusterLogForwarderSpec{}, "1234", "", tls.GetClusterTLSProfileSpec(nil), nil, constants.OpenshiftNS)
collector = podSpec.Containers[0]
DescribeTable("should add the proxy variables to the collector",
func(collectorType logging.LogCollectionType, setHttpProxy, setHttpsProxy, setNoProxy, expectedHttpProxy, expectedHttpsProxy, expectedNoProxy string) {
_httpProxy := os.Getenv(setHttpProxy)
_httpsProxy := os.Getenv(setHttpsProxy)
_noProxy := os.Getenv(setNoProxy)
cleanup := func() {
_ = os.Setenv(setHttpProxy, _httpProxy)
_ = os.Setenv(setHttpsProxy, _httpsProxy)
_ = os.Setenv(setNoProxy, _noProxy)
}
defer cleanup()

httpproxy := "http://[email protected]/3128/"
noproxy := ".cluster.local,localhost"
_ = os.Setenv(setHttpProxy, httpproxy)
_ = os.Setenv(setHttpsProxy, httpproxy)
_ = os.Setenv(setNoProxy, noproxy)
caBundle := "-----BEGIN CERTIFICATE-----\n<PEM_ENCODED_CERT>\n-----END CERTIFICATE-----\n"
factory.CollectorSpec = logging.CollectionSpec{
Type: collectorType,
}
podSpec = *factory.NewPodSpec(&v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: "openshift-logging",
Name: constants.CollectorTrustedCAName,
},
Data: map[string]string{
constants.TrustedCABundleKey: caBundle,
},
}, logging.ClusterLogForwarderSpec{}, "1234", "", tls.GetClusterTLSProfileSpec(nil), nil, constants.OpenshiftNS)
collector = podSpec.Containers[0]

verifyEnvVar(collector, "http_proxy", httpproxy)
verifyEnvVar(collector, "https_proxy", httpproxy)
verifyEnvVar(collector, "no_proxy", "elasticsearch,"+noproxy)
verifyProxyVolumesAndVolumeMounts(collector, podSpec, constants.CollectorTrustedCAName)
})
verifyEnvVar(collector, expectedHttpProxy, httpproxy)
verifyEnvVar(collector, expectedHttpsProxy, httpproxy)
verifyEnvVar(collector, expectedNoProxy, "elasticsearch,"+noproxy)
verifyProxyVolumesAndVolumeMounts(collector, podSpec, constants.CollectorTrustedCAName)
},
Entry("Fluentd expect environment variable name in lowercase", logging.LogCollectionTypeFluentd, "http_proxy", "https_proxy", "no_proxy", "http_proxy", "https_proxy", "no_proxy"),
Entry("Fluentd expects existing uppercase environment variable values to be lowercased", logging.LogCollectionTypeFluentd, "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "no_proxy"),
Entry("Vector expect environment variable in uppercase", logging.LogCollectionTypeVector, "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"),
Entry("Vector expects existing lowercase environment variable values to be uppercased", logging.LogCollectionTypeVector, "http_proxy", "https_proxy", "no_proxy", "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"),
)
})

Context("and using custom named ClusterLogForwarder", func() {
Expand Down Expand Up @@ -546,53 +555,6 @@ var _ = Describe("Factory#Deployment", func() {

})

Context("and the proxy config exists", func() {

var verifyEnvVar = func(container v1.Container, name, value string) {
for _, elem := range container.Env {
if elem.Name == name {
Expect(elem.Value).To(Equal(value), "Exp. collector to have env var %s: %s:", name, value)
return
}
}
Fail(fmt.Sprintf("Exp. collector to include env var: %s", name))
}

It("should add the proxy variables to the collector", func() {
_httpProxy := os.Getenv("http_proxy")
_httpsProxy := os.Getenv("https_proxy")
_noProxy := os.Getenv("no_proxy")
cleanup := func() {
_ = os.Setenv("http_proxy", _httpProxy)
_ = os.Setenv("https_proxy", _httpsProxy)
_ = os.Setenv("no_proxy", _noProxy)
}
defer cleanup()

httpproxy := "http://[email protected]/3128/"
noproxy := ".cluster.local,localhost"
_ = os.Setenv("http_proxy", httpproxy)
_ = os.Setenv("https_proxy", httpproxy)
_ = os.Setenv("no_proxy", noproxy)
caBundle := "-----BEGIN CERTIFICATE-----\n<PEM_ENCODED_CERT>\n-----END CERTIFICATE-----\n"
podSpec = *factory.NewPodSpec(&v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: "openshift-logging",
Name: constants.CollectorTrustedCAName,
},
Data: map[string]string{
constants.TrustedCABundleKey: caBundle,
},
}, logging.ClusterLogForwarderSpec{}, "1234", "", tls.GetClusterTLSProfileSpec(nil), nil, constants.OpenshiftNS)
collector = podSpec.Containers[0]

verifyEnvVar(collector, "http_proxy", httpproxy)
verifyEnvVar(collector, "https_proxy", httpproxy)
verifyEnvVar(collector, "no_proxy", "elasticsearch,"+noproxy)
verifyProxyVolumesAndVolumeMounts(collector, podSpec, constants.CollectorTrustedCAName)
})
})

Context("and using custom named ClusterLogForwarder", func() {

It("should have custom named podSpec resources based on CLF name", func() {
Expand Down
10 changes: 8 additions & 2 deletions internal/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ func EnvVarResourceFieldSelectorEqual(resource1, resource2 v1.ResourceFieldSelec
resource1.Divisor.Cmp(resource2.Divisor) == 0
}

func GetProxyEnvVars() []v1.EnvVar {
func GetProxyEnvVars(collectionType logging.LogCollectionType) []v1.EnvVar {
envVars := []v1.EnvVar{}
for _, envvar := range []string{"HTTPS_PROXY", "https_proxy", "HTTP_PROXY", "http_proxy", "NO_PROXY", "no_proxy"} {
if value := os.Getenv(envvar); value != "" {
Expand All @@ -350,8 +350,14 @@ func GetProxyEnvVars() []v1.EnvVar {
value = strings.Join(constants.ExtraNoProxyList, ",") + "," + value
}
}
switch collectionType {
case logging.LogCollectionTypeVector:
envvar = strings.ToUpper(envvar)
case logging.LogCollectionTypeFluentd:
envvar = strings.ToLower(envvar)
}
envVars = append(envVars, v1.EnvVar{
Name: strings.ToLower(envvar),
Name: envvar,
Value: value,
})
}
Expand Down
31 changes: 23 additions & 8 deletions internal/utils/utils_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package utils

import (
logging "github.com/openshift/cluster-logging-operator/api/logging/v1"
"os"
"strings"
"testing"

v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -145,22 +147,35 @@ var _ = Describe("GetProxyEnvVars", func() {
var (
envvars = map[string]string{}
)
BeforeEach(func() {
AfterEach(func() {
for k, v := range envvars {
Expect(os.Setenv(k, v)).To(Succeed())
}
})
It("should retrieve the proxy settings from the operators ENV variables for Fluentd should be in lowercase", func() {
for _, envvar := range []string{"https_proxy", "http_proxy", "no_proxy"} {
envvars[envvar] = os.Getenv(envvar)
Expect(os.Setenv(envvar, envvar)).To(Succeed())
}
})
AfterEach(func() {
for k, v := range envvars {
Expect(os.Setenv(k, v)).To(Succeed())
envvars := GetProxyEnvVars(logging.LogCollectionTypeFluentd)
Expect(envvars).To(HaveLen(3)) //proxy,noproxy vars
for _, envvar := range envvars {
if envvar.Name == "no_proxy" {
Expect(envvar.Value).To(Equal("elasticsearch,"+envvar.Name), "Exp. the value to be set to the name for the test with elasticsearch prepended")
} else {
Expect(envvar.Name).To(Equal(envvar.Value), "Exp. the value to be set to the name for the test")
}
}
})
It("should retrieve the proxy settings from the operators ENV variables", func() {
envvars := GetProxyEnvVars()
It("should retrieve the proxy settings from the operators ENV variables for Vector should be in uppercase", func() {
for _, envvar := range []string{"https_proxy", "http_proxy", "no_proxy"} {
envvars[envvar] = os.Getenv(envvar)
Expect(os.Setenv(envvar, strings.ToUpper(envvar))).To(Succeed())
}
envvars := GetProxyEnvVars(logging.LogCollectionTypeVector)
Expect(envvars).To(HaveLen(3)) //proxy,noproxy vars
for _, envvar := range envvars {
if envvar.Name == "NO_PROXY" || envvar.Name == "no_proxy" {
if envvar.Name == "NO_PROXY" {
Expect(envvar.Value).To(Equal("elasticsearch,"+envvar.Name), "Exp. the value to be set to the name for the test with elasticsearch prepended")
} else {
Expect(envvar.Name).To(Equal(envvar.Value), "Exp. the value to be set to the name for the test")
Expand Down