fix: avoid nil-pointer-panic in RegistriesConfigController

One billion dollar mistake strikes again. Increase code coverage. Closes #9912 Signed-off-by: Dmitriy Matrenichev <dmitry.matrenichev@siderolabs.com>
siderolabs · Dec 11, 2024 · 30016a0 · 30016a0
1 parent fe04571
commit 30016a0
Show file tree

Hide file tree

Showing 3 changed files with 215 additions and 19 deletions.
diff --git a/internal/app/machined/pkg/controllers/cri/registries_config.go b/internal/app/machined/pkg/controllers/cri/registries_config.go
@@ -15,6 +15,7 @@ import (
 	"github.com/siderolabs/gen/optional"
 	"go.uber.org/zap"
 
+	"github.com/siderolabs/talos/pkg/machinery/config/types/v1alpha1"
 	"github.com/siderolabs/talos/pkg/machinery/constants"
 	"github.com/siderolabs/talos/pkg/machinery/resources/config"
 	"github.com/siderolabs/talos/pkg/machinery/resources/cri"
@@ -54,19 +55,7 @@ func NewRegistriesConfigController() *RegistriesConfigController {
 					mr := cfg.Provider().RawV1Alpha1().MachineConfig.MachineRegistries
 
 					for k, v := range mr.RegistryConfig {
-						spec.RegistryConfig[k] = &cri.RegistryConfig{
-							RegistryTLS: &cri.RegistryTLSConfig{
-								TLSClientIdentity:     v.RegistryTLS.TLSClientIdentity,
-								TLSCA:                 v.RegistryTLS.TLSCA,
-								TLSInsecureSkipVerify: v.RegistryTLS.TLSInsecureSkipVerify,
-							},
-							RegistryAuth: &cri.RegistryAuthConfig{
-								RegistryUsername:      v.RegistryAuth.RegistryUsername,
-								RegistryPassword:      v.RegistryAuth.RegistryPassword,
-								RegistryAuth:          v.RegistryAuth.RegistryAuth,
-								RegistryIdentityToken: v.RegistryAuth.RegistryIdentityToken,
-							},
-						}
+						spec.RegistryConfig[k] = makeRegistryConfig(v)
 					}
 
 					for k, v := range mr.RegistryMirrors {
@@ -116,3 +105,26 @@ func clearInit[M ~map[K]V, K comparable, V any](m M) M {
 
 	return m
 }
+
+func makeRegistryConfig(cfg *v1alpha1.RegistryConfig) *cri.RegistryConfig {
+	result := &cri.RegistryConfig{}
+
+	if rtls := cfg.RegistryTLS; rtls != nil {
+		result.RegistryTLS = &cri.RegistryTLSConfig{
+			TLSClientIdentity:     rtls.TLSClientIdentity,
+			TLSCA:                 rtls.TLSCA,
+			TLSInsecureSkipVerify: rtls.TLSInsecureSkipVerify,
+		}
+	}
+
+	if rauth := cfg.RegistryAuth; rauth != nil {
+		result.RegistryAuth = &cri.RegistryAuthConfig{
+			RegistryUsername:      rauth.RegistryUsername,
+			RegistryPassword:      rauth.RegistryPassword,
+			RegistryAuth:          rauth.RegistryAuth,
+			RegistryIdentityToken: rauth.RegistryIdentityToken,
+		}
+	}
+
+	return result
+}
diff --git a/internal/app/machined/pkg/controllers/cri/registries_config_test.go b/internal/app/machined/pkg/controllers/cri/registries_config_test.go
@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/siderolabs/go-pointer"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 
@@ -74,6 +75,173 @@ func (suite *ConfigSuite) TestRegistry() {
 	})
 }
 
+func (suite *ConfigSuite) TestRegistryAuth() {
+	cfg := config.NewMachineConfig(container.NewV1Alpha1(&v1alpha1.Config{
+		ConfigVersion: "v1alpha1",
+		MachineConfig: &v1alpha1.MachineConfig{
+			MachineType: "controlplane",
+			MachineRegistries: v1alpha1.RegistriesConfig{
+				RegistryMirrors: map[string]*v1alpha1.RegistryMirrorConfig{
+					"docker.io": {MirrorEndpoints: []string{"https://mirror.io"}},
+				},
+				RegistryConfig: map[string]*v1alpha1.RegistryConfig{
+					"docker.io": {
+						RegistryAuth: &v1alpha1.RegistryAuthConfig{
+							RegistryUsername:      "example",
+							RegistryPassword:      "pass",
+							RegistryAuth:          "someauth",
+							RegistryIdentityToken: "token",
+						},
+					},
+				},
+			},
+		},
+	}))
+
+	suite.Require().NoError(suite.State().Create(suite.Ctx(), cfg))
+
+	ctest.AssertResource(suite, crires.RegistriesConfigID, func(r *crires.RegistriesConfig, a *assert.Assertions) {
+		spec := r.TypedSpec()
+
+		a.Equal(
+			map[string]*crires.RegistryMirrorConfig{
+				"docker.io": {MirrorEndpoints: []string{"https://mirror.io"}},
+			},
+			spec.RegistryMirrors,
+		)
+
+		a.Equal(
+			map[string]*crires.RegistryConfig{
+				"docker.io": {
+					RegistryAuth: &crires.RegistryAuthConfig{
+						RegistryUsername:      "example",
+						RegistryPassword:      "pass",
+						RegistryAuth:          "someauth",
+						RegistryIdentityToken: "token",
+					},
+				},
+			},
+			spec.RegistryConfig,
+		)
+	})
+
+	ic := crires.NewImageCacheConfig()
+	ic.TypedSpec().Roots = []string{"/imagecache"}
+	ic.TypedSpec().Status = crires.ImageCacheStatusReady
+
+	suite.Require().NoError(suite.State().Create(suite.Ctx(), ic))
+
+	ctest.AssertResource(suite, crires.RegistriesConfigID, func(r *crires.RegistriesConfig, a *assert.Assertions) {
+		spec := r.TypedSpec()
+
+		a.Equal(
+			map[string]*crires.RegistryMirrorConfig{
+				"*": {MirrorEndpoints: []string{
+					"http://" + constants.RegistrydListenAddress,
+				}},
+				"docker.io": {MirrorEndpoints: []string{
+					"http://" + constants.RegistrydListenAddress,
+					"https://mirror.io",
+				}},
+			},
+			spec.RegistryMirrors,
+		)
+
+		a.Equal(
+			map[string]*crires.RegistryConfig{
+				"docker.io": {
+					RegistryAuth: &crires.RegistryAuthConfig{
+						RegistryUsername:      "example",
+						RegistryPassword:      "pass",
+						RegistryAuth:          "someauth",
+						RegistryIdentityToken: "token",
+					},
+				},
+			},
+			spec.RegistryConfig,
+		)
+	})
+}
+
+func (suite *ConfigSuite) TestRegistryTLS() {
+	cfg := config.NewMachineConfig(container.NewV1Alpha1(&v1alpha1.Config{
+		ConfigVersion: "v1alpha1",
+		MachineConfig: &v1alpha1.MachineConfig{
+			MachineType: "controlplane",
+			MachineRegistries: v1alpha1.RegistriesConfig{
+				RegistryMirrors: map[string]*v1alpha1.RegistryMirrorConfig{
+					"docker.io": {MirrorEndpoints: []string{"https://mirror.io"}},
+				},
+				RegistryConfig: map[string]*v1alpha1.RegistryConfig{
+					"docker.io": {
+						RegistryTLS: &v1alpha1.RegistryTLSConfig{
+							TLSInsecureSkipVerify: pointer.To(true),
+						},
+					},
+				},
+			},
+		},
+	}))
+
+	suite.Require().NoError(suite.State().Create(suite.Ctx(), cfg))
+
+	ctest.AssertResource(suite, crires.RegistriesConfigID, func(r *crires.RegistriesConfig, a *assert.Assertions) {
+		spec := r.TypedSpec()
+
+		a.Equal(
+			map[string]*crires.RegistryMirrorConfig{
+				"docker.io": {MirrorEndpoints: []string{"https://mirror.io"}},
+			},
+			spec.RegistryMirrors,
+		)
+
+		a.Equal(
+			map[string]*crires.RegistryConfig{
+				"docker.io": {
+					RegistryTLS: &crires.RegistryTLSConfig{
+						TLSInsecureSkipVerify: pointer.To(true),
+					},
+				},
+			},
+			spec.RegistryConfig,
+		)
+	})
+
+	ic := crires.NewImageCacheConfig()
+	ic.TypedSpec().Roots = []string{"/imagecache"}
+	ic.TypedSpec().Status = crires.ImageCacheStatusReady
+
+	suite.Require().NoError(suite.State().Create(suite.Ctx(), ic))
+
+	ctest.AssertResource(suite, crires.RegistriesConfigID, func(r *crires.RegistriesConfig, a *assert.Assertions) {
+		spec := r.TypedSpec()
+
+		a.Equal(
+			map[string]*crires.RegistryMirrorConfig{
+				"*": {MirrorEndpoints: []string{
+					"http://" + constants.RegistrydListenAddress,
+				}},
+				"docker.io": {MirrorEndpoints: []string{
+					"http://" + constants.RegistrydListenAddress,
+					"https://mirror.io",
+				}},
+			},
+			spec.RegistryMirrors,
+		)
+
+		a.Equal(
+			map[string]*crires.RegistryConfig{
+				"docker.io": {
+					RegistryTLS: &crires.RegistryTLSConfig{
+						TLSInsecureSkipVerify: pointer.To(true),
+					},
+				},
+			},
+			spec.RegistryConfig,
+		)
+	})
+}
+
 func (suite *ConfigSuite) TestRegistryNoMachineConfig() {
 	cfg := config.NewMachineConfig(container.NewV1Alpha1(nil))
 

diff --git a/pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go b/pkg/machinery/config/types/v1alpha1/v1alpha1_validation.go
@@ -215,12 +215,16 @@ func (c *Config) Validate(mode validation.RuntimeMode, options ...validation.Opt
 		}
 	}
 
-	if c.MachineConfig.MachineDisks != nil {
-		for _, disk := range c.MachineConfig.MachineDisks {
-			for i, pt := range disk.DiskPartitions {
-				if pt.DiskSize == 0 && i != len(disk.DiskPartitions)-1 {
-					result = multierror.Append(result, fmt.Errorf("partition for disk %q is set to occupy full disk, but it's not the last partition in the list", disk.Device()))
-				}
+	for i, disk := range c.MachineConfig.MachineDisks {
+		if disk == nil {
+			result = multierror.Append(result, fmt.Errorf("machine.disks[%d] is null", i))
+
+			continue
+		}
+
+		for i, pt := range disk.DiskPartitions {
+			if pt.DiskSize == 0 && i != len(disk.DiskPartitions)-1 {
+				result = multierror.Append(result, fmt.Errorf("partition for disk %q is set to occupy full disk, but it's not the last partition in the list", disk.Device()))
 			}
 		}
 	}
@@ -348,6 +352,18 @@ func (c *Config) Validate(mode validation.RuntimeMode, options ...validation.Opt
 		}
 	}
 
+	for key, val := range c.MachineConfig.MachineRegistries.RegistryConfig {
+		if val == nil {
+			result = multierror.Append(result, fmt.Errorf("registries.config[%q] is null", key))
+		}
+	}
+
+	for key, val := range c.MachineConfig.MachineRegistries.RegistryMirrors {
+		if val == nil {
+			result = multierror.Append(result, fmt.Errorf("registries.mirrors[%q] is null", key))
+		}
+	}
+
 	if opts.Strict {
 		for _, w := range warnings {
 			result = multierror.Append(result, fmt.Errorf("warning: %s", w))