diff --git a/conf/agent/agent_full.conf b/conf/agent/agent_full.conf index c6c2852a7f..bed26aeacb 100644 --- a/conf/agent/agent_full.conf +++ b/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. diff --git a/doc/plugin_agent_workloadattestor_docker.md b/doc/plugin_agent_workloadattestor_docker.md index 8a283da7ac..2123ee1a13 100644 --- a/doc/plugin_agent_workloadattestor_docker.md +++ b/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: diff --git a/doc/plugin_agent_workloadattestor_k8s.md b/doc/plugin_agent_workloadattestor_k8s.md index ce8dbdc8e0..8be4b72539 100644 --- a/doc/plugin_agent_workloadattestor_k8s.md +++ b/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 diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_posix.go b/pkg/agent/plugin/workloadattestor/docker/docker_posix.go index f1dc803c82..19e24ff29b 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_posix.go +++ b/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`) diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go b/pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go index 8bdf01ff78..0b8fa6efb7 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_posix_test.go +++ 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/*/", -] -`, + cfg: ` + use_new_container_locator = false + container_id_cgroup_matchers = [ + "/docker/*/", + ] + `, }, { desc: "one miss one match", cgroups: testCgroupEntries, - cfg: `container_id_cgroup_matchers = [ -"/docker/*/", -"/docker/" -]`, + cfg: ` + use_new_container_locator = false + container_id_cgroup_matchers = [ + "/docker/*/", + "/docker/" + ] + `, hasMatch: true, }, { desc: "no container id", cgroups: "10:cpu:/docker/", - cfg: `container_id_cgroup_matchers = [ -"/docker/" -]`, + cfg: ` + use_new_container_locator = false + container_id_cgroup_matchers = [ + "/docker/" + ] + `, 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/", ]` diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_test.go b/pkg/agent/plugin/workloadattestor/docker/docker_test.go index ad3f211bb9..f8cabf6717 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_test.go +++ b/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 { diff --git a/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go b/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go index b9d0643824..3f71a27829 100644 --- a/pkg/agent/plugin/workloadattestor/docker/docker_windows_test.go +++ b/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"), diff --git a/pkg/agent/plugin/workloadattestor/k8s/k8s.go b/pkg/agent/plugin/workloadattestor/k8s/k8s.go index 24a996c3ca..8432c5d8b5 100644 --- a/pkg/agent/plugin/workloadattestor/k8s/k8s.go +++ b/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 diff --git a/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go b/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go index 1f8e5e4eea..4f6b996833 100644 --- a/pkg/agent/plugin/workloadattestor/k8s/k8s_posix.go +++ b/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 diff --git a/pkg/agent/plugin/workloadattestor/k8s/k8s_test.go b/pkg/agent/plugin/workloadattestor/k8s/k8s_test.go index 6603401363..97d269334c 100644 --- a/pkg/agent/plugin/workloadattestor/k8s/k8s_test.go +++ b/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 { } diff --git a/test/integration/suites/k8s/conf/agent/spire-agent.yaml b/test/integration/suites/k8s/conf/agent/spire-agent.yaml index 4d1c9f21ad..2cd61562d0 100644 --- a/test/integration/suites/k8s/conf/agent/spire-agent.yaml +++ b/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 } } } diff --git a/test/integration/suites/nested-rotation/intermediateA/agent/agent.conf b/test/integration/suites/nested-rotation/intermediateA/agent/agent.conf index f5aa52f008..fa17226629 100644 --- a/test/integration/suites/nested-rotation/intermediateA/agent/agent.conf +++ b/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 } } } diff --git a/test/integration/suites/nested-rotation/intermediateB/agent/agent.conf b/test/integration/suites/nested-rotation/intermediateB/agent/agent.conf index 9c98377e1d..54bcef5580 100644 --- a/test/integration/suites/nested-rotation/intermediateB/agent/agent.conf +++ b/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 } } } diff --git a/test/integration/suites/nested-rotation/root/agent/agent.conf b/test/integration/suites/nested-rotation/root/agent/agent.conf index d572f4b446..eb32fd7b80 100644 --- a/test/integration/suites/nested-rotation/root/agent/agent.conf +++ b/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 } } } diff --git a/test/integration/suites/oidc-discovery-provider/conf/agent/agent.conf b/test/integration/suites/oidc-discovery-provider/conf/agent/agent.conf index 412d8c4996..a7cfc3dc67 100644 --- a/test/integration/suites/oidc-discovery-provider/conf/agent/agent.conf +++ b/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 } } }