Common flag env metadata whitelist

build/integration.sh

@@ -27,7 +27,7 @@ function start {
   set +e  # We want to handle errors if cAdvisor crashes.
   echo ">> starting cAdvisor locally"
   # This cpuset, percpu, memory, disk, diskIO, network, perf_event metrics should be enabled.
-  GORACE="halt_on_error=1" ./cadvisor --enable_metrics="cpuset,percpu,memory,disk,diskIO,network,perf_event" --docker_env_metadata_whitelist=TEST_VAR --v=6 --logtostderr $CADVISOR_ARGS &> "$log_file"
+  GORACE="halt_on_error=1" ./cadvisor --enable_metrics="cpuset,percpu,memory,disk,diskIO,network,perf_event" --env_metadata_whitelist=TEST_VAR --v=6 --logtostderr $CADVISOR_ARGS &> "$log_file"
   if [ $? != 0 ]; then
     echo "!! cAdvisor exited unexpectedly with Exit $?"
     kill $TEST_PID # cAdvisor crashed: abort testing.

cmd/cadvisor.go

@@ -65,6 +65,8 @@ var collectorKey = flag.String("collector_key", "", "Key for the collector's cer
 var storeContainerLabels = flag.Bool("store_container_labels", true, "convert container labels and environment variables into labels on prometheus metrics for each container. If flag set to false, then only metrics exported are container name, first alias, and image name")
 var whitelistedContainerLabels = flag.String("whitelisted_container_labels", "", "comma separated list of container labels to be converted to labels on prometheus metrics for each container. store_container_labels must be set to false for this to take effect.")
 
+var envMetadataWhiteList = flag.String("env_metadata_whitelist", "", "a comma-separated list of environment variable keys matched with specified prefix that needs to be collected for containers, only support containerd and docker runtime for now.")
+
 var urlBasePrefix = flag.String("url_base_prefix", "", "prefix path that will be prepended to all paths to support some reverse proxies")
 
 var rawCgroupPrefixWhiteList = flag.String("raw_cgroup_prefix_whitelist", "", "A comma-separated list of cgroup path prefix that needs to be collected even when -docker_only is specified")
@@ -129,7 +131,7 @@ func main() {
 
 	collectorHttpClient := createCollectorHttpClient(*collectorCert, *collectorKey)
 
-	resourceManager, err := manager.New(memoryStorage, sysFs, manager.HousekeepingConfigFlags, includedMetrics, &collectorHttpClient, strings.Split(*rawCgroupPrefixWhiteList, ","), *perfEvents)
+	resourceManager, err := manager.New(memoryStorage, sysFs, manager.HousekeepingConfigFlags, includedMetrics, &collectorHttpClient, strings.Split(*rawCgroupPrefixWhiteList, ","), strings.Split(*envMetadataWhiteList, ","), *perfEvents)
 	if err != nil {
 		klog.Fatalf("Failed to create a manager: %s", err)
 	}

cmd/internal/container/mesos/factory.go

@@ -59,7 +59,7 @@ func (f *mesosFactory) String() string {
 	return MesosNamespace
 }
 
-func (f *mesosFactory) NewContainerHandler(name string, inHostNamespace bool) (container.ContainerHandler, error) {
+func (f *mesosFactory) NewContainerHandler(name string, metadataEnvAllowList []string, inHostNamespace bool) (container.ContainerHandler, error) {
 	client, err := Client()
 	if err != nil {
 		return nil, err
@@ -72,6 +72,7 @@ func (f *mesosFactory) NewContainerHandler(name string, inHostNamespace bool) (c
 		f.fsInfo,
 		f.includedMetrics,
 		inHostNamespace,
+		metadataEnvAllowList,
 		client,
 	)
 }

cmd/internal/container/mesos/handler.go

@@ -57,6 +57,7 @@ func newMesosContainerHandler(
 	fsInfo fs.FsInfo,
 	includedMetrics container.MetricSet,
 	inHostNamespace bool,
+	metadataEnvAllowList []string,
 	client mesosAgentClient,
 ) (container.ContainerHandler, error) {
 	cgroupPaths := common.MakeCgroupPaths(cgroupSubsystems.MountPoints, name)

cmd/internal/container/mesos/handler_test.go

@@ -37,13 +37,14 @@ func PopulateContainer() *mContainer {
 func TestContainerReference(t *testing.T) {
 	as := assert.New(t)
 	type testCase struct {
-		client             mesosAgentClient
-		name               string
-		machineInfoFactory info.MachineInfoFactory
-		fsInfo             fs.FsInfo
-		cgroupSubsystems   *containerlibcontainer.CgroupSubsystems
-		inHostNamespace    bool
-		includedMetrics    container.MetricSet
+		client               mesosAgentClient
+		name                 string
+		machineInfoFactory   info.MachineInfoFactory
+		fsInfo               fs.FsInfo
+		cgroupSubsystems     *containerlibcontainer.CgroupSubsystems
+		inHostNamespace      bool
+		metadataEnvAllowList []string
+		includedMetrics      container.MetricSet
 
 		hasErr         bool
 		errContains    string
@@ -57,6 +58,7 @@ func TestContainerReference(t *testing.T) {
 			nil,
 			&containerlibcontainer.CgroupSubsystems{},
 			false,
+			[]string{},
 			nil,
 
 			true,
@@ -70,6 +72,7 @@ func TestContainerReference(t *testing.T) {
 			nil,
 			&containerlibcontainer.CgroupSubsystems{},
 			false,
+			[]string{},
 			nil,
 
 			true,
@@ -83,6 +86,7 @@ func TestContainerReference(t *testing.T) {
 			nil,
 			&containerlibcontainer.CgroupSubsystems{},
 			false,
+			[]string{},
 			nil,
 
 			false,
@@ -95,7 +99,7 @@ func TestContainerReference(t *testing.T) {
 			},
 		},
 	} {
-		handler, err := newMesosContainerHandler(ts.name, ts.cgroupSubsystems, ts.machineInfoFactory, ts.fsInfo, ts.includedMetrics, ts.inHostNamespace, ts.client)
+		handler, err := newMesosContainerHandler(ts.name, ts.cgroupSubsystems, ts.machineInfoFactory, ts.fsInfo, ts.includedMetrics, ts.inHostNamespace, ts.metadataEnvAllowList, ts.client)
 		if ts.hasErr {
 			as.NotNil(err)
 			if ts.errContains != "" {

container/containerd/factory.go

@@ -34,15 +34,15 @@ import (
 var ArgContainerdEndpoint = flag.String("containerd", "/run/containerd/containerd.sock", "containerd endpoint")
 var ArgContainerdNamespace = flag.String("containerd-namespace", "k8s.io", "containerd namespace")
 
+var containerdEnvMetadataWhiteList = flag.String("containerd_env_metadata_whitelist", "", "DEPRECATED: this flag will be removed, please use `env_metadata_whitelist`. A comma-separated list of environment variable keys matched with specified prefix that needs to be collected for containerd containers")
+
 // The namespace under which containerd aliases are unique.
 const k8sContainerdNamespace = "containerd"
 
 // Regexp that identifies containerd cgroups, containers started with
 // --cgroup-parent have another prefix than 'containerd'
 var containerdCgroupRegexp = regexp.MustCompile(`([a-z0-9]{64})`)
 
-var containerdEnvWhitelist = flag.String("containerd_env_metadata_whitelist", "", "a comma-separated list of environment variable keys matched with specified prefix that needs to be collected for containerd containers")
-
 type containerdFactory struct {
 	machineInfoFactory info.MachineInfoFactory
 	client             ContainerdClient
@@ -58,13 +58,18 @@ func (f *containerdFactory) String() string {
 	return k8sContainerdNamespace
 }
 
-func (f *containerdFactory) NewContainerHandler(name string, inHostNamespace bool) (handler container.ContainerHandler, err error) {
+func (f *containerdFactory) NewContainerHandler(name string, metadataEnvAllowList []string, inHostNamespace bool) (handler container.ContainerHandler, err error) {
 	client, err := Client(*ArgContainerdEndpoint, *ArgContainerdNamespace)
 	if err != nil {
 		return
 	}
 
-	metadataEnvs := strings.Split(*containerdEnvWhitelist, ",")
+	containerdMetadataEnvAllowList := strings.Split(*containerdEnvMetadataWhiteList, ",")
+
+	// prefer using the unified metadataEnvAllowList
+	if len(metadataEnvAllowList) != 0 {
+		containerdMetadataEnvAllowList = metadataEnvAllowList
+	}
 
 	return newContainerdContainerHandler(
 		client,
@@ -73,7 +78,7 @@ func (f *containerdFactory) NewContainerHandler(name string, inHostNamespace boo
 		f.fsInfo,
 		&f.cgroupSubsystems,
 		inHostNamespace,
-		metadataEnvs,
+		containerdMetadataEnvAllowList,
 		f.includedMetrics,
 	)
 }

container/containerd/handler.go

@@ -60,7 +60,7 @@ func newContainerdContainerHandler(
 	fsInfo fs.FsInfo,
 	cgroupSubsystems *containerlibcontainer.CgroupSubsystems,
 	inHostNamespace bool,
-	metadataEnvs []string,
+	metadataEnvAllowList []string,
 	includedMetrics container.MetricSet,
 ) (container.ContainerHandler, error) {
 	// Create the cgroup paths.
@@ -134,9 +134,9 @@ func newContainerdContainerHandler(
 	// Add the name and bare ID as aliases of the container.
 	handler.image = cntr.Image
 
-	for _, exposedEnv := range metadataEnvs {
+	for _, exposedEnv := range metadataEnvAllowList {
 		if exposedEnv == "" {
-			// if no containerdEnvWhitelist provided, len(metadataEnvs) == 1, metadataEnvs[0] == ""
+			// if no containerdEnvWhitelist provided, len(metadataEnvAllowList) == 1, metadataEnvAllowList[0] == ""
 			continue
 		}
 

container/containerd/handler_test.go

@@ -45,14 +45,14 @@ func (m *mockedMachineInfo) GetVersionInfo() (*info.VersionInfo, error) {
 func TestHandler(t *testing.T) {
 	as := assert.New(t)
 	type testCase struct {
-		client             ContainerdClient
-		name               string
-		machineInfoFactory info.MachineInfoFactory
-		fsInfo             fs.FsInfo
-		cgroupSubsystems   *containerlibcontainer.CgroupSubsystems
-		inHostNamespace    bool
-		metadataEnvs       []string
-		includedMetrics    container.MetricSet
+		client               ContainerdClient
+		name                 string
+		machineInfoFactory   info.MachineInfoFactory
+		fsInfo               fs.FsInfo
+		cgroupSubsystems     *containerlibcontainer.CgroupSubsystems
+		inHostNamespace      bool
+		metadataEnvAllowList []string
+		includedMetrics      container.MetricSet
 
 		hasErr         bool
 		errContains    string
@@ -121,7 +121,7 @@ func TestHandler(t *testing.T) {
 			map[string]string{"TEST_REGION": "FRA", "TEST_ZONE": "A"},
 		},
 	} {
-		handler, err := newContainerdContainerHandler(ts.client, ts.name, ts.machineInfoFactory, ts.fsInfo, ts.cgroupSubsystems, ts.inHostNamespace, ts.metadataEnvs, ts.includedMetrics)
+		handler, err := newContainerdContainerHandler(ts.client, ts.name, ts.machineInfoFactory, ts.fsInfo, ts.cgroupSubsystems, ts.inHostNamespace, ts.metadataEnvAllowList, ts.includedMetrics)
 		if ts.hasErr {
 			as.NotNil(err)
 			if ts.errContains != "" {

container/crio/factory.go

@@ -64,13 +64,11 @@ func (f *crioFactory) String() string {
 	return CrioNamespace
 }
 
-func (f *crioFactory) NewContainerHandler(name string, inHostNamespace bool) (handler container.ContainerHandler, err error) {
+func (f *crioFactory) NewContainerHandler(name string, metadataEnvAllowList []string, inHostNamespace bool) (handler container.ContainerHandler, err error) {
 	client, err := Client()
 	if err != nil {
 		return
 	}
-	// TODO are there any env vars we need to white list, if so, do it here...
-	metadataEnvs := []string{}
 	handler, err = newCrioContainerHandler(
 		client,
 		name,
@@ -80,7 +78,7 @@ func (f *crioFactory) NewContainerHandler(name string, inHostNamespace bool) (ha
 		f.storageDir,
 		&f.cgroupSubsystems,
 		inHostNamespace,
-		metadataEnvs,
+		metadataEnvAllowList,
 		f.includedMetrics,
 	)
 	return

container/crio/handler.go

@@ -85,7 +85,7 @@ func newCrioContainerHandler(
 	storageDir string,
 	cgroupSubsystems *containerlibcontainer.CgroupSubsystems,
 	inHostNamespace bool,
-	metadataEnvs []string,
+	metadataEnvAllowList []string,
 	includedMetrics container.MetricSet,
 ) (container.ContainerHandler, error) {
 	// Create the cgroup paths.
@@ -186,7 +186,7 @@ func newCrioContainerHandler(
 		handler.fsHandler = common.NewFsHandler(common.DefaultPeriod, rootfsStorageDir, storageLogDir, fsInfo)
 	}
 	// TODO for env vars we wanted to show from container.Config.Env from whitelist
-	//for _, exposedEnv := range metadataEnvs {
+	//for _, exposedEnv := range metadataEnvAllowList {
 	//klog.V(4).Infof("TODO env whitelist: %v", exposedEnv)
 	//}
 

container/crio/handler_test.go

@@ -28,16 +28,16 @@ import (
 func TestHandler(t *testing.T) {
 	as := assert.New(t)
 	type testCase struct {
-		client             CrioClient
-		name               string
-		machineInfoFactory info.MachineInfoFactory
-		fsInfo             fs.FsInfo
-		storageDriver      storageDriver
-		storageDir         string
-		cgroupSubsystems   *containerlibcontainer.CgroupSubsystems
-		inHostNamespace    bool
-		metadataEnvs       []string
-		includedMetrics    container.MetricSet
+		client               CrioClient
+		name                 string
+		machineInfoFactory   info.MachineInfoFactory
+		fsInfo               fs.FsInfo
+		storageDriver        storageDriver
+		storageDir           string
+		cgroupSubsystems     *containerlibcontainer.CgroupSubsystems
+		inHostNamespace      bool
+		metadataEnvAllowList []string
+		includedMetrics      container.MetricSet
 
 		hasErr         bool
 		errContains    string
@@ -102,7 +102,7 @@ func TestHandler(t *testing.T) {
 			},
 		},
 	} {
-		handler, err := newCrioContainerHandler(ts.client, ts.name, ts.machineInfoFactory, ts.fsInfo, ts.storageDriver, ts.storageDir, ts.cgroupSubsystems, ts.inHostNamespace, ts.metadataEnvs, ts.includedMetrics)
+		handler, err := newCrioContainerHandler(ts.client, ts.name, ts.machineInfoFactory, ts.fsInfo, ts.storageDriver, ts.storageDir, ts.cgroupSubsystems, ts.inHostNamespace, ts.metadataEnvAllowList, ts.includedMetrics)
 		if ts.hasErr {
 			as.NotNil(err)
 			if ts.errContains != "" {

container/docker/factory.go

@@ -47,21 +47,21 @@ var ArgDockerCert = flag.String("docker-tls-cert", "cert.pem", "path to client c
 var ArgDockerKey = flag.String("docker-tls-key", "key.pem", "path to private key")
 var ArgDockerCA = flag.String("docker-tls-ca", "ca.pem", "path to trusted CA")
 
+var dockerEnvMetadataWhiteList = flag.String("docker_env_metadata_whitelist", "", "DEPRECATED: this flag will be removed, please use `env_metadata_whitelist`. A comma-separated list of environment variable keys matched with specified prefix that needs to be collected for docker containers")
+
 // The namespace under which Docker aliases are unique.
 const DockerNamespace = "docker"
 
 // The retry times for getting docker root dir
 const rootDirRetries = 5
 
-//The retry period for getting docker root dir, Millisecond
+// The retry period for getting docker root dir, Millisecond
 const rootDirRetryPeriod time.Duration = 1000 * time.Millisecond
 
 // Regexp that identifies docker cgroups, containers started with
 // --cgroup-parent have another prefix than 'docker'
 var dockerCgroupRegexp = regexp.MustCompile(`([a-z0-9]{64})`)
 
-var dockerEnvWhitelist = flag.String("docker_env_metadata_whitelist", "", "a comma-separated list of environment variable keys matched with specified prefix that needs to be collected for docker containers")
-
 var (
 	// Basepath to all container specific information that libcontainer stores.
 	dockerRootDir string
@@ -136,13 +136,18 @@ func (f *dockerFactory) String() string {
 	return DockerNamespace
 }
 
-func (f *dockerFactory) NewContainerHandler(name string, inHostNamespace bool) (handler container.ContainerHandler, err error) {
+func (f *dockerFactory) NewContainerHandler(name string, metadataEnvAllowList []string, inHostNamespace bool) (handler container.ContainerHandler, err error) {
 	client, err := Client()
 	if err != nil {
 		return
 	}
 
-	metadataEnvs := strings.Split(*dockerEnvWhitelist, ",")
+	dockerMetadataEnvAllowList := strings.Split(*dockerEnvMetadataWhiteList, ",")
+
+	// prefer using the unified metadataEnvAllowList
+	if len(metadataEnvAllowList) != 0 {
+		dockerMetadataEnvAllowList = metadataEnvAllowList
+	}
 
 	handler, err = newDockerContainerHandler(
 		client,
@@ -153,7 +158,7 @@ func (f *dockerFactory) NewContainerHandler(name string, inHostNamespace bool) (
 		f.storageDir,
 		&f.cgroupSubsystems,
 		inHostNamespace,
-		metadataEnvs,
+		dockerMetadataEnvAllowList,
 		f.dockerVersion,
 		f.includedMetrics,
 		f.thinPoolName,

container/docker/handler.go

@@ -123,7 +123,7 @@ func newDockerContainerHandler(
 	storageDir string,
 	cgroupSubsystems *containerlibcontainer.CgroupSubsystems,
 	inHostNamespace bool,
-	metadataEnvs []string,
+	metadataEnvAllowList []string,
 	dockerVersion []int,
 	includedMetrics container.MetricSet,
 	thinPoolName string,
@@ -249,9 +249,9 @@ func newDockerContainerHandler(
 	}
 
 	// split env vars to get metadata map.
-	for _, exposedEnv := range metadataEnvs {
+	for _, exposedEnv := range metadataEnvAllowList {
 		if exposedEnv == "" {
-			// if no dockerEnvWhitelist provided, len(metadataEnvs) == 1, metadataEnvs[0] == ""
+			// if no dockerEnvWhitelist provided, len(metadataEnvAllowList) == 1, metadataEnvAllowList[0] == ""
 			continue
 		}
 

container/docker/handler_test.go

@@ -55,11 +55,11 @@ func TestStorageDirDetectionWithNewVersions(t *testing.T) {
 }
 
 func rawMetadataEnvMatch(dockerEnvWhiteList string, cntConfig container.Config) map[string]string {
-	metadataEnvs := strings.Split(dockerEnvWhiteList, ",")
+	metadataEnvAllowList := strings.Split(dockerEnvWhiteList, ",")
 	handlerEnvs := make(map[string]string)
 
 	// split env vars to get metadata map.
-	for _, exposedEnv := range metadataEnvs {
+	for _, exposedEnv := range metadataEnvAllowList {
 		for _, envVar := range cntConfig.Env {
 			if envVar != "" {
 				splits := strings.SplitN(envVar, "=", 2)
@@ -74,13 +74,13 @@ func rawMetadataEnvMatch(dockerEnvWhiteList string, cntConfig container.Config)
 }
 
 func newMetadataEnvMatch(dockerEnvWhiteList string, cntConfig container.Config) map[string]string {
-	metadataEnvs := strings.Split(dockerEnvWhiteList, ",")
+	metadataEnvAllowList := strings.Split(dockerEnvWhiteList, ",")
 	handlerEnvs := make(map[string]string)
 
 	// split env vars to get metadata map.
-	for _, exposedEnv := range metadataEnvs {
+	for _, exposedEnv := range metadataEnvAllowList {
 		if exposedEnv == "" {
-			// if no dockerEnvWhitelist provided, len(metadataEnvs) == 1, metadataEnvs[0] == ""
+			// if no dockerEnvWhitelist provided, len(metadataEnvAllowList) == 1, metadataEnvAllowList[0] == ""
 			continue
 		}