Use new container locator by default (#5454)

Custom matchers can still be used in the docker attestor but require
explicitly disabling the new container locator.

Signed-off-by: Andrew Harding <azdagron@gmail.com>

conf/agent/agent_full.conf

@@ -327,8 +327,8 @@ plugins {
 
             # use_new_container_locator: If true, enables the new container
             # locator algorithm that has support for cgroups v2. Default:
-            # false. (Linux only)
-            # use_new_container_locator = false
+            # true. (Linux only)
+            # use_new_container_locator = true
 
             # verbose_container_locator_logs: If true, enables verbose logging
             # of mountinfo and cgroup information used to locate containers.
@@ -433,8 +433,8 @@ plugins {
 
             # use_new_container_locator: If true, enables the new container
             # locator algorithm that has support for cgroups v2. Default:
-            # false. (Linux only)
-            # use_new_container_locator = false
+            # true. (Linux only)
+            # use_new_container_locator = true
 
             # verbose_container_locator_logs: If true, enables verbose logging
             # of mountinfo and cgroup information used to locate containers.

doc/plugin_agent_workloadattestor_docker.md

@@ -10,7 +10,7 @@ then querying the docker daemon for the container's labels.
 | docker_version                 | The API version of the docker daemon. If not specified                                         |                                  |
 | container_id_cgroup_matchers   | A list of patterns used to discover container IDs from cgroup entries (Unix)                   |                                  |
 | docker_host                    | The location of the Docker Engine API endpoint (Windows only)                                  | "npipe:////./pipe/docker_engine" |
-| use_new_container_locator      | If true, enables the new container locator algorithm that has support for cgroups v2           | false                            |
+| use_new_container_locator      | If true, enables the new container locator algorithm that has support for cgroups v2           | true                             |
 | verbose_container_locator_logs | If true, enables verbose logging of mountinfo and cgroup information used to locate containers | false                            |
 
 A sample configuration:

doc/plugin_agent_workloadattestor_k8s.md

@@ -60,8 +60,8 @@ since [hostprocess](https://kubernetes.io/docs/tasks/configure-pod-container/cre
 | `node_name_env`                  | The environment variable used to obtain the node name. Defaults to `MY_NODE_NAME`.                                                                                                                                                      |
 | `node_name`                      | The name of the node. Overrides the value obtained by the environment variable specified by `node_name_env`.                                                                                                                            |
 | `experimental`                   | The experimental options that are subject to change or removal.                                                                                                                                                                         |
-| `use_new_container_locator`      | If true, enables the new container locator algorithm that has support for cgroups v2. Defaults to false.                                                                                                                                |
-| `verbose_container_locator_logs` | If true, enables verbose logging of mountinfo and cgroup information used to locate containers. Defaults to false.                                                                                                                                          |
+| `use_new_container_locator`      | If true, enables the new container locator algorithm that has support for cgroups v2. Defaults to true.                                                                                                                                 |
+| `verbose_container_locator_logs` | If true, enables verbose logging of mountinfo and cgroup information used to locate containers. Defaults to false.                                                                                                                      |
 
 ## Sigstore experimental feature
 

pkg/agent/plugin/workloadattestor/docker/docker_posix.go

@@ -8,7 +8,6 @@ import (
 	"io"
 	"os"
 	"path/filepath"
-	"regexp"
 
 	"github.com/gogo/status"
 	"github.com/hashicorp/go-hclog"
@@ -40,24 +39,21 @@ type OSConfig struct {
 }
 
 func createHelper(c *dockerPluginConfig, log hclog.Logger) (*containerHelper, error) {
-	var containerIDFinder cgroup.ContainerIDFinder
+	useNewContainerLocator := c.UseNewContainerLocator == nil || *c.UseNewContainerLocator
 
-	switch {
-	case c.UseNewContainerLocator != nil && *c.UseNewContainerLocator:
-		if len(c.ContainerIDCGroupMatchers) > 0 {
-			return nil, status.Error(codes.InvalidArgument, "the new container locator and custom cgroup matchers cannot both be used")
+	var containerIDFinder cgroup.ContainerIDFinder
+	if len(c.ContainerIDCGroupMatchers) > 0 {
+		if useNewContainerLocator {
+			return nil, status.Error(codes.InvalidArgument, "the new container locator and custom cgroup matchers cannot both be used; please open an issue if the new container locator fails to locate workload containers in your environment; to continue using custom matchers set use_new_container_locator=false")
 		}
-		log.Info("Using the new container locator")
-	case len(c.ContainerIDCGroupMatchers) > 0:
-		log.Warn("Using the legacy container locator with custom cgroup matchers. The new locator will be enabled by default in a future release. Consider using it now by setting `use_new_container_locator=true`.")
+		log.Warn("Using the legacy container locator with custom cgroup matchers. This feature will be removed in a future release.")
 		var err error
 		containerIDFinder, err = cgroup.NewContainerIDFinder(c.ContainerIDCGroupMatchers)
 		if err != nil {
 			return nil, err
 		}
-	default:
-		log.Warn("Using the legacy container locator. The new locator will be enabled by default in a future release. Consider using it now by setting `use_new_container_locator=true`.")
-		containerIDFinder = &defaultContainerIDFinder{}
+	} else {
+		log.Info("Using the new container locator")
 	}
 
 	rootDir := c.rootDir
@@ -143,26 +139,3 @@ func getContainerIDFromCGroups(finder cgroup.ContainerIDFinder, cgroups []cgroup
 		return containerID, nil
 	}
 }
-
-type defaultContainerIDFinder struct{}
-
-// FindContainerID returns the container ID in the given cgroup path. The cgroup
-// path must have the whole word "docker" at some point in the path followed
-// at some point by a 64 hex-character container ID. If the cgroup path does
-// not match the above description, the method returns false.
-func (f *defaultContainerIDFinder) FindContainerID(cgroupPath string) (string, bool) {
-	m := dockerCGroupRE.FindStringSubmatch(cgroupPath)
-	if m != nil {
-		return m[1], true
-	}
-	return "", false
-}
-
-// dockerCGroupRE matches cgroup paths that have the following properties.
-// 1) `\bdocker\b` the whole word docker
-// 2) `.+` followed by one or more characters (which will start on a word boundary due to #1)
-// 3) `\b([[:xdigit:]][64])\b` followed by a 64 hex-character container id on word boundary
-//
-// The "docker" prefix and 64-hex character container id can be anywhere in the path. The only
-// requirement is that the docker prefix comes before the id.
-var dockerCGroupRE = regexp.MustCompile(`\bdocker\b.+\b([[:xdigit:]]{64})\b`)

pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go

@@ -14,8 +14,7 @@ import (
 )
 
 const (
-	testCgroupEntries   = "10:devices:/docker/6469646e742065787065637420616e796f6e6520746f20726561642074686973"
-	defaultPluginConfig = "use_new_container_locator = true"
+	testCgroupEntries = "10:devices:/docker/6469646e742065787065637420616e796f6e6520746f20726561642074686973"
 )
 
 func TestContainerExtraction(t *testing.T) {
@@ -29,26 +28,34 @@ func TestContainerExtraction(t *testing.T) {
 		{
 			desc:    "no match",
 			cgroups: testCgroupEntries,
-			cfg: `container_id_cgroup_matchers = [
-"/docker/*/<id>",
-]
-`,
+			cfg: `
+				use_new_container_locator = false
+				container_id_cgroup_matchers = [
+					"/docker/*/<id>",
+				]
+			`,
 		},
 		{
 			desc:    "one miss one match",
 			cgroups: testCgroupEntries,
-			cfg: `container_id_cgroup_matchers = [
-"/docker/*/<id>",
-"/docker/<id>"
-]`,
+			cfg: `
+				use_new_container_locator = false
+				container_id_cgroup_matchers = [
+					"/docker/*/<id>",
+					"/docker/<id>"
+				]
+			`,
 			hasMatch: true,
 		},
 		{
 			desc:    "no container id",
 			cgroups: "10:cpu:/docker/",
-			cfg: `container_id_cgroup_matchers = [
-"/docker/<id>"
-]`,
+			cfg: `
+				use_new_container_locator = false
+				container_id_cgroup_matchers = [
+					"/docker/<id>"
+				]
+			`,
 			expectErr: "a pattern matched, but no container id was found",
 		},
 		{
@@ -64,11 +71,12 @@ func TestContainerExtraction(t *testing.T) {
 		{
 			desc:      "more than one id",
 			cgroups:   testCgroupEntries + "\n" + "4:devices:/system.slice/docker-41e4ab61d2860b0e1467de0da0a9c6068012761febec402dc04a5a94f32ea867.scope",
-			expectErr: "multiple container IDs found in cgroups",
+			expectErr: "multiple container IDs found",
 		},
 		{
-			desc:    "default finder does not match cgroup missing docker prefix",
-			cgroups: "4:devices:/system.slice/41e4ab61d2860b0e1467de0da0a9c6068012761febec402dc04a5a94f32ea867.scope",
+			desc:     "default configuration matches cgroup missing docker prefix",
+			cgroups:  "4:devices:/system.slice/6469646e742065787065637420616e796f6e6520746f20726561642074686973.scope",
+			hasMatch: true,
 		},
 	}
 
@@ -125,6 +133,7 @@ func TestDockerConfigPosix(t *testing.T) {
 		require.NoError(t, err)
 
 		p := newTestPlugin(t, withConfig(t, `
+use_new_container_locator = false
 docker_socket_path = "unix:///socket_path"
 docker_version = "1.20"
 container_id_cgroup_matchers = [
@@ -139,6 +148,7 @@ container_id_cgroup_matchers = [
 	t.Run("bad matcher", func(t *testing.T) {
 		p := New()
 		cfg := `
+use_new_container_locator = false
 container_id_cgroup_matchers = [
 "/docker/",
 ]`

pkg/agent/plugin/workloadattestor/docker/docker_test.go

@@ -381,7 +381,7 @@ func withSigstoreVerifier(v sigstore.Verifier) testPluginOpt {
 
 func newTestPlugin(t *testing.T, opts ...testPluginOpt) *Plugin {
 	p := New()
-	err := doConfigure(t, p, defaultPluginConfig)
+	err := doConfigure(t, p, "")
 	require.NoError(t, err)
 
 	for _, o := range opts {

pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go

@@ -12,10 +12,6 @@ import (
 	"google.golang.org/grpc/codes"
 )
 
-const (
-	defaultPluginConfig = ""
-)
-
 func TestFailToGetContainerID(t *testing.T) {
 	h := &fakeProcessHelper{
 		err: errors.New("oh no"),

pkg/agent/plugin/workloadattestor/k8s/k8s.go

@@ -124,8 +124,8 @@ type HCLConfig struct {
 	DisableContainerSelectors bool `hcl:"disable_container_selectors"`
 
 	// UseNewContainerLocator, if true, uses the new container locator
-	// mechanism instead of the legacy cgroup matchers. Defaults to false if
-	// unset. This will default to true in a future release.
+	// mechanism instead of the legacy cgroup matchers. Defaults to true if
+	// unset. This configurable will be removed in a future release.
 	UseNewContainerLocator *bool `hcl:"use_new_container_locator"`
 
 	// VerboseContainerLocatorLogs, if true, dumps extra information to the log

pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go

@@ -45,11 +45,11 @@ type containerHelper struct {
 
 func (h *containerHelper) Configure(config *HCLConfig, log hclog.Logger) error {
 	h.verboseContainerLocatorLogs = config.VerboseContainerLocatorLogs
-	h.useNewContainerLocator = config.UseNewContainerLocator != nil && *config.UseNewContainerLocator
+	h.useNewContainerLocator = config.UseNewContainerLocator == nil || *config.UseNewContainerLocator
 	if h.useNewContainerLocator {
 		log.Info("Using the new container locator")
 	} else {
-		log.Warn("Using the legacy container locator. The new locator will be enabled by default in a future release. Consider using it now by setting `use_new_container_locator=true`.")
+		log.Warn("Using the legacy container locator. This option will removed in a future release.")
 	}
 
 	return nil

pkg/agent/plugin/workloadattestor/k8s/k8s_test.go

@@ -732,7 +732,6 @@ func (s *Suite) loadInsecurePlugin() workloadattestor.WorkloadAttestor {
 		kubelet_read_only_port = %d
 		max_poll_attempts = 5
 		poll_retry_interval = "1s"
-		use_new_container_locator = true
 `, s.kubeletPort()))
 }
 
@@ -741,7 +740,6 @@ func (s *Suite) loadInsecurePluginWithExtra(extraConfig string) workloadattestor
 		kubelet_read_only_port = %d
 		max_poll_attempts = 5
 		poll_retry_interval = "1s"
-		use_new_container_locator = true
 		%s
 `, s.kubeletPort(), extraConfig))
 }
@@ -751,7 +749,6 @@ func (s *Suite) loadInsecurePluginWithSigstore() workloadattestor.WorkloadAttest
 		kubelet_read_only_port = %d
 		max_poll_attempts = 5
 		poll_retry_interval = "1s"
-		use_new_container_locator = true
 		experimental {
 			sigstore {
 			}

test/integration/suites/k8s/conf/agent/spire-agent.yaml

@@ -69,10 +69,6 @@ data:
           # Minikube does not have a cert in the cluster CA bundle that
           # can authenticate the kubelet cert, so skip validation.
           skip_kubelet_verification = true
-
-          # Use the new container locator method. This can be removed
-          # once it is on by default.
-          use_new_container_locator = true
         }
       }
     }

test/integration/suites/nested-rotation/intermediateA/agent/agent.conf

@@ -26,9 +26,6 @@ plugins {
     }
     WorkloadAttestor "docker" {
         plugin_data {
-            # Use the new container locator method. This can be removed
-            # once it is on by default.
-            use_new_container_locator = true
         }
     }
 }

test/integration/suites/nested-rotation/intermediateB/agent/agent.conf

@@ -26,9 +26,6 @@ plugins {
     }
     WorkloadAttestor "docker" {
         plugin_data {
-            # Use the new container locator method. This can be removed
-            # once it is on by default.
-            use_new_container_locator = true
         }
     }
 }

test/integration/suites/nested-rotation/root/agent/agent.conf

@@ -22,9 +22,6 @@ plugins {
     }
     WorkloadAttestor "docker" {
         plugin_data {
-            # Use the new container locator method. This can be removed
-            # once it is on by default.
-            use_new_container_locator = true
         }
     }
 }

test/integration/suites/oidc-discovery-provider/conf/agent/agent.conf

@@ -22,9 +22,6 @@ plugins {
     }
     WorkloadAttestor "docker" {
         plugin_data {
-            # Use the new container locator method. This can be removed
-            # once it is on by default.
-            use_new_container_locator = true
         }
     }
 }