Merge pull request #554 from wallrj/memory-optimization

[VC-35377] Reduce memory by excluding Helm release Secrets and some standard Secret types

Author: wallrj

deploy/charts/venafi-kubernetes-agent/README.md

@@ -25,11 +25,12 @@ If you do not have one, you can sign up for a free trial now at:
 | authentication.venafiConnection.name | string | `"venafi-components"` | The name of a VenafiConnection resource which contains the configuration for authenticating to Venafi. |
 | authentication.venafiConnection.namespace | string | `"venafi"` | The namespace of a VenafiConnection resource which contains the configuration for authenticating to Venafi. |
 | command | list | `[]` | Specify the command to run overriding default binary. |
-| config | object | `{"clientId":"","clusterDescription":"","clusterName":"","configmap":{"key":null,"name":null},"period":"0h1m0s","server":"https://api.venafi.cloud/"}` | Configuration section for the Venafi Kubernetes Agent itself |
+| config | object | `{"clientId":"","clusterDescription":"","clusterName":"","configmap":{"key":null,"name":null},"ignoredSecretTypes":["kubernetes.io/service-account-token","kubernetes.io/dockercfg","kubernetes.io/dockerconfigjson","kubernetes.io/basic-auth","kubernetes.io/ssh-auth","bootstrap.kubernetes.io/token","helm.sh/release.v1"],"period":"0h1m0s","server":"https://api.venafi.cloud/"}` | Configuration section for the Venafi Kubernetes Agent itself |
 | config.clientId | string | `""` | The client-id returned from the Venafi Control Plane |
 | config.clusterDescription | string | `""` | Description for the cluster resource if it needs to be created in Venafi Control Plane |
 | config.clusterName | string | `""` | Name for the cluster resource if it needs to be created in Venafi Control Plane |
 | config.configmap | object | `{"key":null,"name":null}` | Specify ConfigMap details to load config from an existing resource. This should be blank by default unless you have you own config. |
+| config.ignoredSecretTypes | list | `["kubernetes.io/service-account-token","kubernetes.io/dockercfg","kubernetes.io/dockerconfigjson","kubernetes.io/basic-auth","kubernetes.io/ssh-auth","bootstrap.kubernetes.io/token","helm.sh/release.v1"]` | Reduce the memory usage of the agent and reduce the load on the Kubernetes API server by omitting various common Secret types when listing Secrets. These Secret types will be added to a "type!=<type>" field selector in the agent config. * https://docs.venafi.cloud/vaas/k8s-components/t-cfg-tlspk-agent/#configuration * https://kubernetes.io/docs/concepts/configuration/secret/#secret-types * https://kubernetes.io/docs/concepts/overview/working-with-objects/field-selectors/#list-of-supported-fields |
 | config.period | string | `"0h1m0s"` | Send data back to the platform every minute unless changed |
 | config.server | string | `"https://api.venafi.cloud/"` | Overrides the server if using a proxy in your environment For the EU variant use: https://api.venafi.eu/ |
 | crds.forceRemoveValidationAnnotations | bool | `false` | The 'x-kubernetes-validations' annotation is not supported in Kubernetes 1.22 and below. This annotation is used by CEL, which is a feature introduced in Kubernetes 1.25 that improves how validation is performed. This option allows to force the 'x-kubernetes-validations' annotation to be excluded, even on Kubernetes 1.25+ clusters. |

deploy/charts/venafi-kubernetes-agent/templates/configmap.yaml

@@ -89,6 +89,12 @@ data:
         resource-type:
           version: v1
           resource: secrets
+        {{- with .Values.config.ignoredSecretTypes }}
+        field-selectors:
+        {{- range . }}
+        - type!={{ . }}
+        {{- end }}
+        {{- end }}
     - kind: "k8s-dynamic"
       name: "k8s/certificates"
       config:
@@ -202,5 +208,3 @@ data:
           version: v1
           resource: issuers
 {{- end }}
-
-

deploy/charts/venafi-kubernetes-agent/values.yaml

@@ -203,6 +203,22 @@ config:
   # -- Description for the cluster resource if it needs to be created in Venafi Control Plane
   clusterDescription: ""
 
+  # -- Reduce the memory usage of the agent and reduce the load on the Kubernetes
+  # API server by omitting various common Secret types when listing Secrets.
+  # These Secret types will be added to a "type!=<type>" field selector in the
+  # agent config.
+  # * https://docs.venafi.cloud/vaas/k8s-components/t-cfg-tlspk-agent/#configuration
+  # * https://kubernetes.io/docs/concepts/configuration/secret/#secret-types
+  # * https://kubernetes.io/docs/concepts/overview/working-with-objects/field-selectors/#list-of-supported-fields
+  ignoredSecretTypes:
+  - kubernetes.io/service-account-token
+  - kubernetes.io/dockercfg
+  - kubernetes.io/dockerconfigjson
+  - kubernetes.io/basic-auth
+  - kubernetes.io/ssh-auth
+  - bootstrap.kubernetes.io/token
+  - helm.sh/release.v1
+
   # -- Specify ConfigMap details to load config from an existing resource.
   # This should be blank by default unless you have you own config.
   configmap:

docs/datagatherers/k8s-dynamic.md

@@ -64,7 +64,7 @@ resource referenced in the `kind` for that datagatherer.
 There is an example `ClusterRole` and `ClusterRoleBinding` which can be found in
 [`./deployment/kubernetes/base/00-rbac.yaml`](./deployment/kubernetes/base/00-rbac.yaml).
 
-# Secrets
+## Secrets
 
 Secrets can be gathered using the following config:
 
@@ -79,4 +79,30 @@ Secrets can be gathered using the following config:
 
 Before Secrets are sent to the Preflight backend, they are redacted so no secret data is transmitted. See [`fieldfilter.go`](./../../pkg/datagatherer/k8s/fieldfilter.go) to see the details of which fields are filteres and which ones are redacted.
 
-> **All resource other than Kubernetes Secrets are sent in full, so make sure that you don't store secret information on arbitrary resources.**
+> **All resource other than Kubernetes Secrets are sent in full, so make sure that you don't store secret information on arbitrary resources.**
+
+
+## Field Selectors
+
+You can use [field selectors](https://kubernetes.io/docs/concepts/overview/working-with-objects/field-selectors/#list-of-supported-fields)
+to include or exclude certain resources.
+For example, you can reduce the memory usage of the agent and reduce the load on the Kubernetes
+API server by omitting various common [Secret types](https://kubernetes.io/docs/concepts/configuration/secret/#secret-types)
+when listing Secrets.
+
+```yaml
+- kind: "k8s-dynamic"
+  name: "k8s/secrets"
+  config:
+    resource-type:
+      version: v1
+      resource: secrets
+    field-selectors:
+    - type!=kubernetes.io/service-account-token
+    - type!=kubernetes.io/dockercfg
+    - type!=kubernetes.io/dockerconfigjson
+    - type!=kubernetes.io/basic-auth
+    - type!=kubernetes.io/ssh-auth,
+    - type!=bootstrap.kubernetes.io/token
+    - type!=helm.sh/release.v1
+```

examples/one-shot-secret.yaml

@@ -0,0 +1,29 @@
+# one-shot-secret.yaml
+#
+# An example configuration file which can be used for local testing.
+# It gathers only secrets and it does not attempt to upload to Venafi.
+# For example:
+#
+#  builds/preflight agent \
+#     --agent-config-file examples/one-shot-secret.yaml \
+#     --one-shot \
+#     --output-path output.json
+#
+organization_id: "my-organization"
+cluster_id: "my_cluster"
+period: 1m
+data-gatherers:
+- kind: "k8s-dynamic"
+  name: "k8s/secrets"
+  config:
+    resource-type:
+      version: v1
+      resource: secrets
+    field-selectors:
+    - type!=kubernetes.io/service-account-token
+    - type!=kubernetes.io/dockercfg
+    - type!=kubernetes.io/dockerconfigjson
+    - type!=kubernetes.io/basic-auth
+    - type!=kubernetes.io/ssh-auth,
+    - type!=bootstrap.kubernetes.io/token
+    - type!=helm.sh/release.v1

pkg/datagatherer/k8s/dynamic.go

@@ -39,6 +39,8 @@ type ConfigDynamic struct {
 	ExcludeNamespaces []string `yaml:"exclude-namespaces"`
 	// IncludeNamespaces is a list of namespaces to include.
 	IncludeNamespaces []string `yaml:"include-namespaces"`
+	// FieldSelectors is a list of field selectors to use when listing this resource
+	FieldSelectors []string `yaml:"field-selectors"`
 }
 
 // UnmarshalYAML unmarshals the ConfigDynamic resolving GroupVersionResource.
@@ -52,6 +54,7 @@ func (c *ConfigDynamic) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		} `yaml:"resource-type"`
 		ExcludeNamespaces []string `yaml:"exclude-namespaces"`
 		IncludeNamespaces []string `yaml:"include-namespaces"`
+		FieldSelectors    []string `yaml:"field-selectors"`
 	}{}
 	err := unmarshal(&aux)
 	if err != nil {
@@ -64,6 +67,7 @@ func (c *ConfigDynamic) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	c.GroupVersionResource.Resource = aux.ResourceType.Resource
 	c.ExcludeNamespaces = aux.ExcludeNamespaces
 	c.IncludeNamespaces = aux.IncludeNamespaces
+	c.FieldSelectors = aux.FieldSelectors
 
 	return nil
 }
@@ -79,6 +83,16 @@ func (c *ConfigDynamic) validate() error {
 		errors = append(errors, "invalid configuration: GroupVersionResource.Resource cannot be empty")
 	}
 
+	for i, selectorString := range c.FieldSelectors {
+		if selectorString == "" {
+			errors = append(errors, fmt.Sprintf("invalid field selector %d: must not be empty", i))
+		}
+		_, err := fields.ParseSelector(selectorString)
+		if err != nil {
+			errors = append(errors, fmt.Sprintf("invalid field selector %d: %s", i, err))
+		}
+	}
+
 	if len(errors) > 0 {
 		return fmt.Errorf(strings.Join(errors, ", "))
 	}
@@ -150,7 +164,15 @@ func (c *ConfigDynamic) newDataGathererWithClient(ctx context.Context, cl dynami
 		return nil, err
 	}
 	// init shared informer for selected namespaces
-	fieldSelector := generateFieldSelector(c.ExcludeNamespaces)
+	fieldSelector := generateExcludedNamespacesFieldSelector(c.ExcludeNamespaces)
+
+	// Add any custom field selectors to the excluded namespaces selector
+	// The selectors have already been validated, so it is safe to use
+	// ParseSelectorOrDie here.
+	for _, selectorString := range c.FieldSelectors {
+		fieldSelector = fields.AndSelectors(fieldSelector, fields.ParseSelectorOrDie(selectorString))
+	}
+
 	// init cache to store gathered resources
 	dgCache := cache.New(5*time.Minute, 30*time.Second)
 
@@ -159,7 +181,7 @@ func (c *ConfigDynamic) newDataGathererWithClient(ctx context.Context, cl dynami
 		cl:                   cl,
 		k8sClientSet:         clientset,
 		groupVersionResource: c.GroupVersionResource,
-		fieldSelector:        fieldSelector,
+		fieldSelector:        fieldSelector.String(),
 		namespaces:           c.IncludeNamespaces,
 		cache:                dgCache,
 	}
@@ -177,7 +199,7 @@ func (c *ConfigDynamic) newDataGathererWithClient(ctx context.Context, cl dynami
 			60*time.Second,
 			informers.WithNamespace(metav1.NamespaceAll),
 			informers.WithTweakListOptions(func(options *metav1.ListOptions) {
-				options.FieldSelector = fieldSelector
+				options.FieldSelector = fieldSelector.String()
 			}))
 		newDataGatherer.nativeSharedInformer = factory
 		informer := informerFunc(factory)
@@ -200,7 +222,7 @@ func (c *ConfigDynamic) newDataGathererWithClient(ctx context.Context, cl dynami
 		cl,
 		60*time.Second,
 		metav1.NamespaceAll,
-		func(options *metav1.ListOptions) { options.FieldSelector = fieldSelector },
+		func(options *metav1.ListOptions) { options.FieldSelector = fieldSelector.String() },
 	)
 	resourceInformer := factory.ForResource(c.GroupVersionResource)
 	informer := resourceInformer.Informer()
@@ -420,17 +442,17 @@ func namespaceResourceInterface(iface dynamic.NamespaceableResourceInterface, na
 	return iface.Namespace(namespace)
 }
 
-// generateFieldSelector creates a field selector string from a list of
-// namespaces to exclude.
-func generateFieldSelector(excludeNamespaces []string) string {
-	fieldSelector := fields.Nothing()
+// generateExcludedNamespacesFieldSelector creates a field selector string from
+// a list of namespaces to exclude.
+func generateExcludedNamespacesFieldSelector(excludeNamespaces []string) fields.Selector {
+	var selectors []fields.Selector
 	for _, excludeNamespace := range excludeNamespaces {
 		if excludeNamespace == "" {
 			continue
 		}
-		fieldSelector = fields.AndSelectors(fields.OneTermNotEqualSelector("metadata.namespace", excludeNamespace), fieldSelector)
+		selectors = append(selectors, fields.OneTermNotEqualSelector("metadata.namespace", excludeNamespace))
 	}
-	return fieldSelector.String()
+	return fields.AndSelectors(selectors...)
 }
 
 func isIncludedNamespace(namespace string, namespaces []string) bool {

pkg/datagatherer/k8s/dynamic_test.go

@@ -105,8 +105,12 @@ func sortGatheredResources(list []*api.GatheredResource) {
 func TestNewDataGathererWithClientAndDynamicInformer(t *testing.T) {
 	ctx := context.Background()
 	config := ConfigDynamic{
-		IncludeNamespaces:    []string{"a"},
+		ExcludeNamespaces:    []string{"kube-system"},
 		GroupVersionResource: schema.GroupVersionResource{Group: "foobar", Version: "v1", Resource: "foos"},
+		FieldSelectors: []string{
+			"type!=kubernetes.io/service-account-token",
+			"type!=kubernetes.io/dockercfg",
+		},
 	}
 	cl := fake.NewSimpleDynamicClient(runtime.NewScheme())
 	dg, err := config.newDataGathererWithClient(ctx, cl, nil)
@@ -121,7 +125,8 @@ func TestNewDataGathererWithClientAndDynamicInformer(t *testing.T) {
 		groupVersionResource: config.GroupVersionResource,
 		// it's important that the namespaces are set as the IncludeNamespaces
 		// during initialization
-		namespaces: config.IncludeNamespaces,
+		namespaces:    config.IncludeNamespaces,
+		fieldSelector: "metadata.namespace!=kube-system,type!=kubernetes.io/service-account-token,type!=kubernetes.io/dockercfg",
 	}
 
 	gatherer := dg.(*DataGathererDynamic)
@@ -150,6 +155,9 @@ func TestNewDataGathererWithClientAndDynamicInformer(t *testing.T) {
 	if gatherer.nativeSharedInformer != nil {
 		t.Errorf("unexpected nativeSharedInformer value: %v. should be nil", gatherer.nativeSharedInformer)
 	}
+	if !reflect.DeepEqual(gatherer.fieldSelector, expected.fieldSelector) {
+		t.Errorf("expected %v, got %v", expected.fieldSelector, gatherer.fieldSelector)
+	}
 }
 
 func TestNewDataGathererWithClientAndSharedIndexInformer(t *testing.T) {
@@ -216,6 +224,8 @@ exclude-namespaces:
 # from the config file
 include-namespaces:
 - default
+field-selectors:
+- type!=kubernetes.io/service-account-token
 `
 
 	expectedGVR := schema.GroupVersionResource{
@@ -231,6 +241,10 @@ include-namespaces:
 
 	expectedIncludeNamespaces := []string{"default"}
 
+	expectedFieldSelectors := []string{
+		"type!=kubernetes.io/service-account-token",
+	}
+
 	cfg := ConfigDynamic{}
 	err := yaml.Unmarshal([]byte(textCfg), &cfg)
 	if err != nil {
@@ -251,6 +265,9 @@ include-namespaces:
 	if got, want := cfg.IncludeNamespaces, expectedIncludeNamespaces; !reflect.DeepEqual(got, want) {
 		t.Errorf("IncludeNamespaces does not match: got=%+v want=%+v", got, want)
 	}
+	if got, want := cfg.FieldSelectors, expectedFieldSelectors; !reflect.DeepEqual(got, want) {
+		t.Errorf("FieldSelectors does not match: got=%+v want=%+v", got, want)
+	}
 }
 
 func TestConfigDynamicValidate(t *testing.T) {
@@ -275,17 +292,42 @@ func TestConfigDynamicValidate(t *testing.T) {
 			},
 			ExpectedError: "cannot set excluded and included namespaces",
 		},
+		{
+			Config: ConfigDynamic{
+				GroupVersionResource: schema.GroupVersionResource{
+					Group:    "",
+					Version:  "v1",
+					Resource: "secrets",
+				},
+				FieldSelectors: []string{""},
+			},
+			ExpectedError: "invalid field selector 0: must not be empty",
+		},
+		{
+			Config: ConfigDynamic{
+				GroupVersionResource: schema.GroupVersionResource{
+					Group:    "",
+					Version:  "v1",
+					Resource: "secrets",
+				},
+				FieldSelectors: []string{"foo"},
+			},
+			ExpectedError: "invalid field selector 0: invalid selector: 'foo'; can't understand 'foo'",
+		},
 	}
 
 	for _, test := range tests {
 		err := test.Config.validate()
-		if !strings.Contains(err.Error(), test.ExpectedError) {
+		if err == nil && test.ExpectedError != "" {
+			t.Errorf("expected error: %q, got: nil", test.ExpectedError)
+		}
+		if err != nil && !strings.Contains(err.Error(), test.ExpectedError) {
 			t.Errorf("expected %s, got %s", test.ExpectedError, err.Error())
 		}
 	}
 }
 
-func TestGenerateFieldSelector(t *testing.T) {
+func TestGenerateExcludedNamespacesFieldSelector(t *testing.T) {
 	tests := []struct {
 		ExcludeNamespaces     []string
 		ExpectedFieldSelector string
@@ -300,19 +342,19 @@ func TestGenerateFieldSelector(t *testing.T) {
 			ExcludeNamespaces: []string{
 				"kube-system",
 			},
-			ExpectedFieldSelector: "metadata.namespace!=kube-system,",
+			ExpectedFieldSelector: "metadata.namespace!=kube-system",
 		},
 		{
 			ExcludeNamespaces: []string{
 				"kube-system",
 				"my-namespace",
 			},
-			ExpectedFieldSelector: "metadata.namespace!=my-namespace,metadata.namespace!=kube-system,",
+			ExpectedFieldSelector: "metadata.namespace!=kube-system,metadata.namespace!=my-namespace",
 		},
 	}
 
 	for _, test := range tests {
-		fieldSelector := generateFieldSelector(test.ExcludeNamespaces)
+		fieldSelector := generateExcludedNamespacesFieldSelector(test.ExcludeNamespaces).String()
 		if fieldSelector != test.ExpectedFieldSelector {
 			t.Errorf("ExpectedFieldSelector does not match: got=%+v want=%+v", fieldSelector, test.ExpectedFieldSelector)
 		}