Repository: spegel-org/spegel
Pull request #651 (2024-11-28 12:44:16)

Fix Containerd CRI config verification
Either the configuration format has changed or I implemented it incorrectly from the beginning. This change fixes the parsing of the CRI config JSON to actually check discard unpacked layers. It also adds a check to make sure the field is present.

Relates to #634


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5481d80..d9b6b93 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- [#651](https://github.com/spegel-org/spegel/pull/651) Fix Containerd CRI config verification.
+
 ### Security
 
 ## v0.0.27
diff --git a/pkg/oci/containerd.go b/pkg/oci/containerd.go
index 6410e63..91231de 100644
--- a/pkg/oci/containerd.go
+++ b/pkg/oci/containerd.go
@@ -132,32 +132,36 @@ func verifyStatusResponse(resp *runtimeapi.StatusResponse, configPath string) er
 	}
 	cfg := &struct {
 		Registry struct {
-			ConfigPath string `json:"configPath"`
+			ConfigPath *string `json:"configPath"`
 		} `json:"registry"`
 		Containerd struct {
-			Runtimes struct {
-				DiscardUnpackedLayers bool `json:"discardUnpackedLayers"`
-			} `json:"runtimes"`
+			DiscardUnpackedLayers *bool `json:"discardUnpackedLayers"`
 		} `json:"containerd"`
 	}{}
 	err := json.Unmarshal([]byte(str), cfg)
 	if err != nil {
 		return err
 	}
-	if cfg.Containerd.Runtimes.DiscardUnpackedLayers {
+	if cfg.Containerd.DiscardUnpackedLayers == nil {
+		return errors.New("field containerd.discardUnpackedLayers missing from config")
+	}
+	if *cfg.Containerd.DiscardUnpackedLayers {
 		return errors.New("Containerd discard unpacked layers cannot be enabled")
 	}
-	if cfg.Registry.ConfigPath == "" {
+	if cfg.Registry.ConfigPath == nil {
+		return errors.New("field registry.configPath missing from config")
+	}
+	if *cfg.Registry.ConfigPath == "" {
 		return errors.New("Containerd registry config path needs to be set for mirror configuration to take effect")
 	}
-	paths := filepath.SplitList(cfg.Registry.ConfigPath)
+	paths := filepath.SplitList(*cfg.Registry.ConfigPath)
 	for _, path := range paths {
 		if path != configPath {
 			continue
 		}
 		return nil
 	}
-	return fmt.Errorf("Containerd registry config path is %s but needs to contain path %s for mirror configuration to take effect", cfg.Registry.ConfigPath, configPath)
+	return fmt.Errorf("Containerd registry config path is %s but needs to contain path %s for mirror configuration to take effect", *cfg.Registry.ConfigPath, configPath)
 }
 
 func (c *Containerd) Subscribe(ctx context.Context) (<-chan ImageEvent, <-chan error, error) {


Test patch:
diff --git a/pkg/oci/containerd_test.go b/pkg/oci/containerd_test.go
index 9b96690..d528c2d 100644
--- a/pkg/oci/containerd_test.go
+++ b/pkg/oci/containerd_test.go
@@ -80,7 +80,7 @@ func TestVerifyStatusResponse(t *testing.T) {
 
 			resp := &runtimeapi.StatusResponse{
 				Info: map[string]string{
-					"config": fmt.Sprintf(`{"registry": {"configPath": %q}, "containerd": {"runtimes":{"discardUnpackedLayers": %v}}}`, tt.configPath, tt.discardUnpackedLayers),
+					"config": fmt.Sprintf(`{"registry": {"configPath": %q}, "containerd": {"discardUnpackedLayers": %v}}`, tt.configPath, tt.discardUnpackedLayers),
 				},
 			}
 			err := verifyStatusResponse(resp, tt.requiredConfigPath)
@@ -93,6 +93,50 @@ func TestVerifyStatusResponse(t *testing.T) {
 	}
 }
 
+func TestVerifyStatusResponseMissingRequired(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name           string
+		config         string
+		expectedErrMsg string
+	}{
+		{
+			name:           "missing discard upacked layers false",
+			config:         `{"registry": {"configPath": "foo"}, "containerd": {"runtimes":{"discardUnpackedLayers": false}}}`,
+			expectedErrMsg: "field containerd.discardUnpackedLayers missing from config",
+		},
+		{
+			name:           "missing discard upacked layers true",
+			config:         `{"registry": {"configPath": "foo"}, "containerd": {"runtimes":{"discardUnpackedLayers": true}}}`,
+			expectedErrMsg: "field containerd.discardUnpackedLayers missing from config",
+		},
+		{
+			name:           "missing containerd field",
+			config:         `{"registry": {"configPath": "foo"}}`,
+			expectedErrMsg: "field containerd.discardUnpackedLayers missing from config",
+		},
+		{
+			name:           "missing registry field",
+			config:         `{"containerd": {"discardUnpackedLayers": false}}`,
+			expectedErrMsg: "field registry.configPath missing from config",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			resp := &runtimeapi.StatusResponse{
+				Info: map[string]string{
+					"config": tt.config,
+				},
+			}
+			err := verifyStatusResponse(resp, "foo")
+			require.EqualError(t, err, tt.expectedErrMsg)
+		})
+	}
+}
+
 func TestBackupConfig(t *testing.T) {
 	t.Parallel()