fix: correctly handle multiple registries with the same Host [Backport v0.27.x] (#1064)

**What problem does this PR solve?**:
Backport of #1063


**Which issue(s) this PR fixes**:
Fixes #

**How Has This Been Tested?**:
<!--
Please describe the tests that you ran to verify your changes.
Provide output from the tests and any manual steps needed to replicate
the tests.
-->

**Special notes for your reviewer**:
<!--
Use this to provide any additional information to the reviewers.
This may include:
- Best way to review the PR.
- Where the author wants the most review attention on.
- etc.
-->

Co-authored-by: Dimitri Koshkin <[email protected]>

Author: jimmidyson

pkg/handlers/generic/mutation/mirrors/containerd_files.go

@@ -6,12 +6,14 @@ package mirrors
 import (
 	"bytes"
 	_ "embed"
+	"errors"
 	"fmt"
 	"net/url"
 	"path"
 	"strings"
 	"text/template"
 
+	"github.com/samber/lo"
 	cabpkv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1"
 
 	"github.com/nutanix-cloud-native/cluster-api-runtime-extensions-nutanix/pkg/common"
@@ -139,21 +141,17 @@ func generateRegistryCACertFiles(
 
 	var files []cabpkv1.File //nolint:prealloc // We don't know the size of the slice yet.
 
-	for _, config := range configs {
-		if config.CASecretName == "" {
-			continue
-		}
-
-		registryCACertPathOnRemote, err := config.filePathFromURL()
-		if err != nil {
-			return nil, fmt.Errorf("failed generating CA certificate file path from URL: %w", err)
-		}
+	filesToGenerate, err := registryCACertFiles(configs)
+	if err != nil {
+		return nil, err
+	}
+	for _, file := range filesToGenerate {
 		files = append(files, cabpkv1.File{
-			Path:        registryCACertPathOnRemote,
+			Path:        file.path,
 			Permissions: "0600",
 			ContentFrom: &cabpkv1.FileSource{
 				Secret: cabpkv1.SecretFileSource{
-					Name: config.CASecretName,
+					Name: file.caSecretName,
 					Key:  secretKeyForCACert,
 				},
 			},
@@ -189,3 +187,65 @@ func formatURLForContainerd(uri string) (string, error) {
 	// using path.Join on all elements incorrectly drops a "/" from "https://"
 	return fmt.Sprintf("%s/%s", mirror, mirrorPath), nil
 }
+
+type containerdConfigFile struct {
+	path         string
+	url          string
+	caSecretName string
+	caCert       string
+}
+
+var ErrConflictingRegistryCACertificates = errors.New(
+	"conflicting CA certificate specified for registry host",
+)
+
+// registryCACertFiles returns a list of CA certificate files
+// that should be generated for the given containerd configurations.
+// If any of the provided configurations share the same url.Host only a single file will be generated.
+// An error will be returned, if the CA certificate content for the same URL.Host do not match.
+func registryCACertFiles(configs []containerdConfig) ([]containerdConfigFile, error) {
+	filesToGenerate := make([]containerdConfigFile, 0, len(configs))
+
+	for _, config := range configs {
+		// Skip if CA certificate is not provided.
+		if config.CASecretName == "" {
+			continue
+		}
+		registryCACertPathOnRemote, err := config.filePathFromURL()
+		if err != nil {
+			return nil, fmt.Errorf("failed generating CA certificate file path from URL: %w", err)
+		}
+
+		existingFileToGenerate, existing := lo.Find(
+			filesToGenerate,
+			func(f containerdConfigFile) bool {
+				return registryCACertPathOnRemote == f.path
+			},
+		)
+		// File exists and needs to be checked for conflicts.
+		if existing {
+			if config.CACert != existingFileToGenerate.caCert {
+				return nil, fmt.Errorf(
+					"%w: %q (from secrets %q and %q)",
+					ErrConflictingRegistryCACertificates,
+					config.URL,
+					config.CASecretName,
+					existingFileToGenerate.caSecretName,
+				)
+			}
+
+			// File already found with matching content - skipping.
+			continue
+		}
+
+		// File not already found and needs to be generated.
+		filesToGenerate = append(filesToGenerate, containerdConfigFile{
+			path:         registryCACertPathOnRemote,
+			url:          config.URL,
+			caSecretName: config.CASecretName,
+			caCert:       config.CACert,
+		})
+	}
+
+	return filesToGenerate, nil
+}

pkg/handlers/generic/mutation/mirrors/containerd_files_test.go

@@ -40,7 +40,6 @@ func Test_generateContainerdDefaultHostsFile(t *testing.T) {
   override_path = true
 `,
 			},
-			wantErr: nil,
 		},
 		{
 			name: "ECR mirror image registry with a path and no CA certificate",
@@ -63,7 +62,6 @@ func Test_generateContainerdDefaultHostsFile(t *testing.T) {
   override_path = true
 `,
 			},
-			wantErr: nil,
 		},
 		{
 			name: "Mirror image registry with a CA and an image registry with no CA certificate",
@@ -91,7 +89,6 @@ func Test_generateContainerdDefaultHostsFile(t *testing.T) {
   override_path = true
 `,
 			},
-			wantErr: nil,
 		},
 		{
 			name: "Mirror image registry with a CA and an image registry with a CA",
@@ -124,7 +121,6 @@ func Test_generateContainerdDefaultHostsFile(t *testing.T) {
   override_path = true
 `,
 			},
-			wantErr: nil,
 		},
 		{
 			name: "Image registry with a CA",
@@ -135,8 +131,6 @@ func Test_generateContainerdDefaultHostsFile(t *testing.T) {
 				},
 			},
 			want: nil,
-
-			wantErr: nil,
 		},
 	}
 	for idx := range tests {
@@ -156,6 +150,7 @@ func Test_generateRegistryCACertFiles(t *testing.T) {
 		name    string
 		configs []containerdConfig
 		want    []cabpkv1.File
+		wantErr error
 	}{
 		{
 			name: "ECR mirror image registry with no CA certificate",
@@ -173,8 +168,41 @@ func Test_generateRegistryCACertFiles(t *testing.T) {
 				{
 					URL:          "https://registry.example.com",
 					CASecretName: "my-registry-credentials-secret",
+					CACert:       "-----BEGIN CERTIFICATE-----",
+					Mirror:       true,
+				},
+			},
+			want: []cabpkv1.File{
+				{
+					Path:        "/etc/containerd/certs.d/registry.example.com/ca.crt",
+					Owner:       "",
+					Permissions: "0600",
+					Encoding:    "",
+					Append:      false,
+					ContentFrom: &cabpkv1.FileSource{
+						Secret: cabpkv1.SecretFileSource{
+							Name: "my-registry-credentials-secret",
+							Key:  "ca.crt",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "Mirror image registry and registry with the same CA certificate",
+			configs: []containerdConfig{
+				{
+					URL:          "https://registry.example.com/mirror",
+					CASecretName: "my-registry-credentials-secret",
+					CACert:       "-----BEGIN CERTIFICATE-----",
 					Mirror:       true,
 				},
+				{
+					URL:          "https://registry.example.com/library",
+					CASecretName: "my-registry-credentials-secret",
+					CACert:       "-----BEGIN CERTIFICATE-----",
+					Mirror:       false,
+				},
 			},
 			want: []cabpkv1.File{
 				{
@@ -192,13 +220,49 @@ func Test_generateRegistryCACertFiles(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "Mirror image registry and registry with different CA certificates",
+			configs: []containerdConfig{
+				{
+					URL:          "https://registry.example.com/mirror",
+					CASecretName: "my-registry-credentials-secret",
+					CACert:       "-----BEGIN CERTIFICATE-----",
+					Mirror:       true,
+				},
+				{
+					URL:          "https://registry.example.com/library",
+					CASecretName: "my-registry-credentials-secret",
+					CACert:       "-----BEGIN CERTIFICATE----------END CERTIFICATE-----",
+					Mirror:       false,
+				},
+			},
+			wantErr: ErrConflictingRegistryCACertificates,
+		},
+		{
+			name: "Multiple image registries with different CA certificates",
+			configs: []containerdConfig{
+				{
+					URL:          "https://registry.example.com/mirror",
+					CASecretName: "my-registry-credentials-secret",
+					CACert:       "-----BEGIN CERTIFICATE-----",
+					Mirror:       false,
+				},
+				{
+					URL:          "https://registry.example.com/library",
+					CASecretName: "my-registry-credentials-secret",
+					CACert:       "-----BEGIN CERTIFICATE----------END CERTIFICATE-----",
+					Mirror:       false,
+				},
+			},
+			wantErr: ErrConflictingRegistryCACertificates,
+		},
 	}
 	for idx := range tests {
 		tt := tests[idx]
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 			file, err := generateRegistryCACertFiles(tt.configs)
-			require.NoError(t, err)
+			require.ErrorIs(t, err, tt.wantErr)
 			assert.Equal(t, tt.want, file)
 		})
 	}