diff --git a/pkg/operator/staticpod/certsyncpod/certsync_controller.go b/pkg/operator/staticpod/certsyncpod/certsync_controller.go index 111776d994..6911454be0 100644 --- a/pkg/operator/staticpod/certsyncpod/certsync_controller.go +++ b/pkg/operator/staticpod/certsyncpod/certsync_controller.go @@ -2,10 +2,12 @@ package certsyncpod import ( "context" + "fmt" "os" "path/filepath" "reflect" + "github.com/openshift/library-go/pkg/operator/staticpod/internal/atomicdir/types" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -17,17 +19,19 @@ import ( "github.com/openshift/library-go/pkg/controller/factory" "github.com/openshift/library-go/pkg/operator/events" - "github.com/openshift/library-go/pkg/operator/staticpod" "github.com/openshift/library-go/pkg/operator/staticpod/controller/installer" + "github.com/openshift/library-go/pkg/operator/staticpod/internal/atomicdir" ) +const stagingDirUID = "cert-sync" + type CertSyncController struct { destinationDir string namespace string configMaps []installer.UnrevisionedResource secrets []installer.UnrevisionedResource - configmapGetter corev1interface.ConfigMapInterface + configMapGetter corev1interface.ConfigMapInterface configMapLister v1.ConfigMapLister secretGetter corev1interface.SecretInterface secretLister v1.SecretLister @@ -42,10 +46,10 @@ func NewCertSyncController(targetDir, targetNamespace string, configmaps, secret secrets: secrets, eventRecorder: eventRecorder.WithComponentSuffix("cert-sync-controller"), - configmapGetter: kubeClient.CoreV1().ConfigMaps(targetNamespace), + configMapGetter: kubeClient.CoreV1().ConfigMaps(targetNamespace), configMapLister: informers.Core().V1().ConfigMaps().Lister(), - secretLister: informers.Core().V1().Secrets().Lister(), secretGetter: kubeClient.CoreV1().Secrets(targetNamespace), + secretLister: informers.Core().V1().Secrets().Lister(), } return factory.New(). @@ -60,15 +64,12 @@ func NewCertSyncController(targetDir, targetNamespace string, configmaps, secret ) } -func getConfigMapDir(targetDir, configMapName string) string { - return filepath.Join(targetDir, "configmaps", configMapName) -} - -func getSecretDir(targetDir, secretName string) string { - return filepath.Join(targetDir, "secrets", secretName) -} - func (c *CertSyncController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + if err := os.RemoveAll(getStagingDir(c.destinationDir)); err != nil { + c.eventRecorder.Warningf("CertificateUpdateFailed", fmt.Sprintf("Failed to prune staging directory: %v", err)) + return err + } + errors := []error{} klog.Infof("Syncing configmaps: %v", c.configMaps) @@ -80,7 +81,7 @@ func (c *CertSyncController) sync(ctx context.Context, syncCtx factory.SyncConte continue case apierrors.IsNotFound(err) && cm.Optional: - configMapFile := getConfigMapDir(c.destinationDir, cm.Name) + configMapFile := getConfigMapTargetDir(c.destinationDir, cm.Name) if _, err := os.Stat(configMapFile); os.IsNotExist(err) { // if the configmap file does not exist, there is no work to do, so skip making any live check and just return. // if the configmap actually exists in the API, we'll eventually see it on the watch. @@ -88,7 +89,7 @@ func (c *CertSyncController) sync(ctx context.Context, syncCtx factory.SyncConte } // Check with the live call it is really missing - configMap, err = c.configmapGetter.Get(ctx, cm.Name, metav1.GetOptions{}) + configMap, err = c.configMapGetter.Get(ctx, cm.Name, metav1.GetOptions{}) if err == nil { klog.Infof("Caches are stale. They don't see configmap '%s/%s', yet it is present", configMap.Namespace, configMap.Name) // We will get re-queued when we observe the change @@ -113,9 +114,10 @@ func (c *CertSyncController) sync(ctx context.Context, syncCtx factory.SyncConte continue } - contentDir := getConfigMapDir(c.destinationDir, cm.Name) + contentDir := getConfigMapTargetDir(c.destinationDir, cm.Name) + stagingDir := getConfigMapStagingDir(c.destinationDir, cm.Name) - data := map[string]string{} + data := make(map[string]string, len(configMap.Data)) for filename := range configMap.Data { fullFilename := filepath.Join(contentDir, filename) @@ -138,7 +140,7 @@ func (c *CertSyncController) sync(ctx context.Context, syncCtx factory.SyncConte klog.V(2).Infof("Syncing updated configmap '%s/%s'.", configMap.Namespace, configMap.Name) // We need to do a live get here so we don't overwrite a newer file with one from a stale cache - configMap, err = c.configmapGetter.Get(ctx, configMap.Name, metav1.GetOptions{}) + configMap, err = c.configMapGetter.Get(ctx, configMap.Name, metav1.GetOptions{}) if err != nil { // Even if the error is not exists we will act on it when caches catch up c.eventRecorder.Warningf("CertificateUpdateFailed", "Failed getting configmap: %s/%s: %v", c.namespace, cm.Name, err) @@ -152,27 +154,11 @@ func (c *CertSyncController) sync(ctx context.Context, syncCtx factory.SyncConte continue } - klog.Infof("Creating directory %q ...", contentDir) - if err := os.MkdirAll(contentDir, 0755); err != nil && !os.IsExist(err) { - c.eventRecorder.Warningf("CertificateUpdateFailed", "Failed creating directory for configmap: %s/%s: %v", configMap.Namespace, configMap.Name, err) - errors = append(errors, err) - continue - } - for filename, content := range configMap.Data { - fullFilename := filepath.Join(contentDir, filename) - // if the existing is the same, do nothing - if reflect.DeepEqual(data[fullFilename], content) { - continue - } - - klog.Infof("Writing configmap manifest %q ...", fullFilename) - if err := staticpod.WriteFileAtomic([]byte(content), 0644, fullFilename); err != nil { - c.eventRecorder.Warningf("CertificateUpdateFailed", "Failed writing file for configmap: %s/%s: %v", configMap.Namespace, configMap.Name, err) - errors = append(errors, err) - continue - } + files := make(map[string][]byte, len(configMap.Data)) + for k, v := range configMap.Data { + files[k] = []byte(v) } - c.eventRecorder.Eventf("CertificateUpdated", "Wrote updated configmap: %s/%s", configMap.Namespace, configMap.Name) + errors = append(errors, syncDirectory(c.eventRecorder, "configmap", configMap.ObjectMeta, contentDir, 0755, stagingDir, files, 0644)) } klog.Infof("Syncing secrets: %v", c.secrets) @@ -184,7 +170,7 @@ func (c *CertSyncController) sync(ctx context.Context, syncCtx factory.SyncConte continue case apierrors.IsNotFound(err) && s.Optional: - secretFile := getSecretDir(c.destinationDir, s.Name) + secretFile := getSecretTargetDir(c.destinationDir, s.Name) if _, err := os.Stat(secretFile); os.IsNotExist(err) { // if the secret file does not exist, there is no work to do, so skip making any live check and just return. // if the secret actually exists in the API, we'll eventually see it on the watch. @@ -218,9 +204,10 @@ func (c *CertSyncController) sync(ctx context.Context, syncCtx factory.SyncConte continue } - contentDir := getSecretDir(c.destinationDir, s.Name) + contentDir := getSecretTargetDir(c.destinationDir, s.Name) + stagingDir := getSecretStagingDir(c.destinationDir, s.Name) - data := map[string][]byte{} + data := make(map[string][]byte, len(secret.Data)) for filename := range secret.Data { fullFilename := filepath.Join(contentDir, filename) @@ -257,29 +244,50 @@ func (c *CertSyncController) sync(ctx context.Context, syncCtx factory.SyncConte continue } - klog.Infof("Creating directory %q ...", contentDir) - if err := os.MkdirAll(contentDir, 0755); err != nil && !os.IsExist(err) { - c.eventRecorder.Warningf("CertificateUpdateFailed", "Failed creating directory for secret: %s/%s: %v", secret.Namespace, secret.Name, err) - errors = append(errors, err) - continue - } - for filename, content := range secret.Data { - // TODO fix permissions - fullFilename := filepath.Join(contentDir, filename) - // if the existing is the same, do nothing - if reflect.DeepEqual(data[fullFilename], content) { - continue - } + errors = append(errors, syncDirectory(c.eventRecorder, "secret", secret.ObjectMeta, contentDir, 0755, stagingDir, secret.Data, 0600)) + } + return utilerrors.NewAggregate(errors) +} - klog.Infof("Writing secret manifest %q ...", fullFilename) - if err := staticpod.WriteFileAtomic(content, 0600, fullFilename); err != nil { - c.eventRecorder.Warningf("CertificateUpdateFailed", "Failed writing file for secret: %s/%s: %v", secret.Namespace, secret.Name, err) - errors = append(errors, err) - continue - } +func syncDirectory( + eventRecorder events.Recorder, + typeName string, o metav1.ObjectMeta, + targetDir string, targetDirPerm os.FileMode, stagingDir string, + fileContents map[string][]byte, filePerm os.FileMode, +) error { + files := make(map[string]types.File, len(fileContents)) + for filename, content := range fileContents { + files[filename] = types.File{ + Content: content, + Perm: filePerm, } - c.eventRecorder.Eventf("CertificateUpdated", "Wrote updated secret: %s/%s", secret.Namespace, secret.Name) } - return utilerrors.NewAggregate(errors) + if err := atomicdir.Sync(targetDir, targetDirPerm, stagingDir, files); err != nil { + err = fmt.Errorf("failed to sync %s %s/%s (directory %q): %w", typeName, o.Name, o.Namespace, targetDir, err) + eventRecorder.Warning("CertificateUpdateFailed", err.Error()) + return err + } + eventRecorder.Eventf("CertificateUpdated", "Wrote updated %s: %s/%s", typeName, o.Namespace, o.Name) + return nil +} + +func getStagingDir(targetDir string) string { + return filepath.Join(targetDir, "staging", stagingDirUID) +} + +func getConfigMapTargetDir(targetDir, configMapName string) string { + return filepath.Join(targetDir, "configmaps", configMapName) +} + +func getConfigMapStagingDir(targetDir, configMapName string) string { + return filepath.Join(getStagingDir(targetDir), "configmaps", configMapName) +} + +func getSecretTargetDir(targetDir, secretName string) string { + return filepath.Join(targetDir, "secrets", secretName) +} + +func getSecretStagingDir(targetDir, secretName string) string { + return filepath.Join(getStagingDir(targetDir), "secrets", secretName) } diff --git a/pkg/operator/staticpod/certsyncpod/certsync_controller_linux_test.go b/pkg/operator/staticpod/certsyncpod/certsync_controller_linux_test.go new file mode 100644 index 0000000000..f4611d7fde --- /dev/null +++ b/pkg/operator/staticpod/certsyncpod/certsync_controller_linux_test.go @@ -0,0 +1,780 @@ +//go:build linux + +package certsyncpod + +import ( + "bytes" + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "math/big" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/apiserver/pkg/server/dynamiccertificates" + "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes/fake" + ktesting "k8s.io/client-go/testing" + clocktesting "k8s.io/utils/clock/testing" + + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/staticpod/controller/installer" + "github.com/openshift/library-go/pkg/operator/staticpod/internal/atomicdir" + adtesting "github.com/openshift/library-go/pkg/operator/staticpod/internal/atomicdir/testing" + "github.com/openshift/library-go/pkg/operator/staticpod/internal/atomicdir/types" +) + +const testingNamespace = "test-namespace" + +// TestCertSyncController_Sync tests the sync method in various scenarios. +func TestCertSyncController_Sync(t *testing.T) { + testCases := []struct { + name string + configMapsToSync []installer.UnrevisionedResource + configMapObjects []*corev1.ConfigMap + configMapGetErrors map[string]error + secretsToSync []installer.UnrevisionedResource + secretObjects []*corev1.Secret + secretGetErrors map[string]error + // Keys are paths relative to the controller destination directory. + existingDirectories map[string]adtesting.DirectoryState + expectedError bool + expectedEventTypes []string + // Keys are paths relative to the controller destination directory. + expectedDirectories map[string]adtesting.DirectoryState + }{ + { + name: "create first configmap", + configMapsToSync: []installer.UnrevisionedResource{ + {Name: "test-config"}, + }, + configMapObjects: []*corev1.ConfigMap{ + createConfigMap("test-config", map[string]string{ + "config.yaml": "test-config-content", + }), + }, + expectedEventTypes: []string{"CertificateUpdated"}, + expectedDirectories: map[string]adtesting.DirectoryState{ + "configmaps/test-config": { + "config.yaml": { + Content: []byte("test-config-content"), + Perm: 0644, + }, + }, + }, + }, + { + name: "add another configmap", + configMapsToSync: []installer.UnrevisionedResource{ + {Name: "test-config-1"}, + {Name: "test-config-2"}, + }, + configMapObjects: []*corev1.ConfigMap{ + createConfigMap("test-config-1", map[string]string{ + "config.yaml": "test-config-content-1", + }), + createConfigMap("test-config-2", map[string]string{ + "config.yaml": "test-config-content-2", + }), + }, + existingDirectories: map[string]adtesting.DirectoryState{ + "configmaps/test-config-1": { + "config.yaml": { + Content: []byte("test-config-content-1"), + Perm: 0644, + }, + }, + }, + expectedEventTypes: []string{"CertificateUpdated"}, + expectedDirectories: map[string]adtesting.DirectoryState{ + "configmaps/test-config-1": { + "config.yaml": { + Content: []byte("test-config-content-1"), + Perm: 0644, + }, + }, + "configmaps/test-config-2": { + "config.yaml": { + Content: []byte("test-config-content-2"), + Perm: 0644, + }, + }, + }, + }, + { + name: "update existing configmap", + configMapsToSync: []installer.UnrevisionedResource{ + {Name: "test-config", Optional: false}, + }, + configMapObjects: []*corev1.ConfigMap{ + createConfigMap("test-config", map[string]string{ + "config.yaml": "updated-config-content", + }), + }, + existingDirectories: map[string]adtesting.DirectoryState{ + "configmaps/test-config": { + "config.yaml": { + Content: []byte("test-config-content"), + Perm: 0644, + }, + }, + }, + expectedEventTypes: []string{"CertificateUpdated"}, + expectedDirectories: map[string]adtesting.DirectoryState{ + "configmaps/test-config": { + "config.yaml": { + Content: []byte("updated-config-content"), + Perm: 0644, + }, + }, + }, + }, + { + name: "succeed when optional configmap missing", + configMapsToSync: []installer.UnrevisionedResource{ + {Name: "optional-config", Optional: true}, + }, + }, + { + name: "fail when required configmap missing", + configMapsToSync: []installer.UnrevisionedResource{ + {Name: "required-config", Optional: false}, + }, + expectedError: true, + }, + { + name: "remove directory when optional configmap missing", + configMapsToSync: []installer.UnrevisionedResource{ + {Name: "optional-config", Optional: true}, + }, + existingDirectories: map[string]adtesting.DirectoryState{ + "configmaps/optional-config": { + "config.yaml": { + Content: []byte("optional-config-content"), + Perm: 0644, + }, + }, + }, + expectedEventTypes: []string{"CertificateRemoved"}, + }, + { + name: "configmap unchanged on get error", + configMapsToSync: []installer.UnrevisionedResource{ + {Name: "error-config"}, + }, + configMapObjects: []*corev1.ConfigMap{ + createConfigMap("error-config", map[string]string{ + "config.yaml": "updated-config-content", + }), + }, + configMapGetErrors: map[string]error{ + "error-config": apierrors.NewInternalError(fmt.Errorf("API server error")), + }, + existingDirectories: map[string]adtesting.DirectoryState{ + "configmaps/error-config": { + "config.yaml": { + Content: []byte("error-config-content"), + Perm: 0644, + }, + }, + }, + expectedError: true, + expectedEventTypes: []string{"CertificateUpdateFailed"}, + expectedDirectories: map[string]adtesting.DirectoryState{ + "configmaps/error-config": { + "config.yaml": { + Content: []byte("error-config-content"), + Perm: 0644, + }, + }, + }, + }, + + { + name: "create first secret", + secretsToSync: []installer.UnrevisionedResource{ + {Name: "test-secret"}, + }, + secretObjects: []*corev1.Secret{ + createSecret("test-secret", map[string][]byte{ + "tls.crt": []byte("test-cert-content"), + "tls.key": []byte("test-key-content"), + }), + }, + expectedEventTypes: []string{"CertificateUpdated"}, + expectedDirectories: map[string]adtesting.DirectoryState{ + "secrets/test-secret": { + "tls.crt": { + Content: []byte("test-cert-content"), + Perm: 0600, + }, + "tls.key": { + Content: []byte("test-key-content"), + Perm: 0600, + }, + }, + }, + }, + { + name: "add another secret", + secretsToSync: []installer.UnrevisionedResource{ + {Name: "test-secret-1"}, + {Name: "test-secret-2"}, + }, + secretObjects: []*corev1.Secret{ + createSecret("test-secret-1", map[string][]byte{ + "tls-1.crt": []byte("test-cert-content-1"), + "tls-1.key": []byte("test-key-content-1"), + }), + createSecret("test-secret-2", map[string][]byte{ + "tls-2.crt": []byte("test-cert-content-2"), + "tls-2.key": []byte("test-key-content-2"), + }), + }, + existingDirectories: map[string]adtesting.DirectoryState{ + "secrets/test-secret-1": { + "tls-1.crt": { + Content: []byte("test-cert-content-1"), + Perm: 0600, + }, + "tls-1.key": { + Content: []byte("test-key-content-1"), + Perm: 0600, + }, + }, + }, + expectedEventTypes: []string{"CertificateUpdated"}, + expectedDirectories: map[string]adtesting.DirectoryState{ + "secrets/test-secret-1": { + "tls-1.crt": { + Content: []byte("test-cert-content-1"), + Perm: 0600, + }, + "tls-1.key": { + Content: []byte("test-key-content-1"), + Perm: 0600, + }, + }, + "secrets/test-secret-2": { + "tls-2.crt": { + Content: []byte("test-cert-content-2"), + Perm: 0600, + }, + "tls-2.key": { + Content: []byte("test-key-content-2"), + Perm: 0600, + }, + }, + }, + }, + { + name: "update existing secret", + secretsToSync: []installer.UnrevisionedResource{ + {Name: "test-secret", Optional: true}, + }, + secretObjects: []*corev1.Secret{ + createSecret("test-secret", map[string][]byte{ + "tls.crt": []byte("updated-cert-content"), + "tls.key": []byte("updated-key-content"), + }), + }, + existingDirectories: map[string]adtesting.DirectoryState{ + "secrets/test-secret": { + "tls.crt": { + Content: []byte("test-cert-content"), + Perm: 0600, + }, + "tls.key": { + Content: []byte("test-key-content"), + Perm: 0600, + }, + }, + }, + expectedEventTypes: []string{"CertificateUpdated"}, + expectedDirectories: map[string]adtesting.DirectoryState{ + "secrets/test-secret": { + "tls.crt": { + Content: []byte("updated-cert-content"), + Perm: 0600, + }, + "tls.key": { + Content: []byte("updated-key-content"), + Perm: 0600, + }, + }, + }, + }, + { + name: "succeed when optional secret missing", + secretsToSync: []installer.UnrevisionedResource{ + {Name: "optional-secret", Optional: true}, + }, + }, + { + name: "fail when required secret missing", + secretsToSync: []installer.UnrevisionedResource{ + {Name: "required-secret", Optional: false}, + }, + expectedError: true, + }, + { + name: "remove directory when optional secret missing", + secretsToSync: []installer.UnrevisionedResource{ + {Name: "optional-secret", Optional: true}, + }, + existingDirectories: map[string]adtesting.DirectoryState{ + "secrets/optional-secret": { + "tls.crt": { + Content: []byte("test-cert-content"), + Perm: 0600, + }, + "tls.key": { + Content: []byte("test-key-content"), + Perm: 0600, + }, + }, + }, + expectedEventTypes: []string{"CertificateRemoved"}, + }, + { + name: "secret unchanged on get error", + secretsToSync: []installer.UnrevisionedResource{ + {Name: "error-secret"}, + }, + secretObjects: []*corev1.Secret{ + createSecret("error-secret", map[string][]byte{ + "token": []byte("updated-secret-content"), + }), + }, + secretGetErrors: map[string]error{ + "error-secret": apierrors.NewInternalError(fmt.Errorf("API server error")), + }, + existingDirectories: map[string]adtesting.DirectoryState{ + "secrets/error-secret": { + "token": { + Content: []byte("error-config-content"), + Perm: 0600, + }, + }, + }, + expectedError: true, + expectedEventTypes: []string{"CertificateUpdateFailed"}, + expectedDirectories: map[string]adtesting.DirectoryState{ + "secrets/error-secret": { + "token": { + Content: []byte("error-config-content"), + Perm: 0600, + }, + }, + }, + }, + + { + name: "create multiple configmaps and secrets", + configMapsToSync: []installer.UnrevisionedResource{ + {Name: "test-config-1", Optional: false}, + {Name: "test-config-2", Optional: true}, + }, + configMapObjects: []*corev1.ConfigMap{ + createConfigMap("test-config-1", map[string]string{ + "app.yaml": "test-config-content-1", + }), + createConfigMap("test-config-2", map[string]string{ + "opt.yaml": "test-config-content-2", + }), + }, + secretsToSync: []installer.UnrevisionedResource{ + {Name: "test-secret-1", Optional: false}, + {Name: "test-secret-2", Optional: true}, + }, + secretObjects: []*corev1.Secret{ + createSecret("test-secret-1", map[string][]byte{ + "token": []byte("test-secret-content-1"), + }), + createSecret("test-secret-2", map[string][]byte{ + "key": []byte("test-secret-content-2"), + }), + }, + expectedEventTypes: []string{"CertificateUpdated", "CertificateUpdated", "CertificateUpdated", "CertificateUpdated"}, + expectedDirectories: map[string]adtesting.DirectoryState{ + "configmaps/test-config-1": { + "app.yaml": { + Content: []byte("test-config-content-1"), + Perm: 0644, + }, + }, + "configmaps/test-config-2": { + "opt.yaml": { + Content: []byte("test-config-content-2"), + Perm: 0644, + }, + }, + "secrets/test-secret-1": { + "token": { + Content: []byte("test-secret-content-1"), + Perm: 0600, + }, + }, + "secrets/test-secret-2": { + "key": { + Content: []byte("test-secret-content-2"), + Perm: 0600, + }, + }, + }, + }, + { + name: "already synchronized", + configMapsToSync: []installer.UnrevisionedResource{ + {Name: "test-config"}, + }, + configMapObjects: []*corev1.ConfigMap{ + createConfigMap("test-config", map[string]string{ + "config.yaml": "test-config-content", + }), + }, + secretsToSync: []installer.UnrevisionedResource{ + {Name: "test-secret"}, + }, + secretObjects: []*corev1.Secret{ + createSecret("test-secret", map[string][]byte{ + "tls.crt": []byte("test-cert-content"), + "tls.key": []byte("test-key-content"), + }), + }, + existingDirectories: map[string]adtesting.DirectoryState{ + "configmaps/test-config": { + "config.yaml": { + Content: []byte("test-config-content"), + Perm: 0644, + }, + }, + "secrets/test-secret": { + "tls.crt": { + Content: []byte("test-cert-content"), + Perm: 0600, + }, + "tls.key": { + Content: []byte("test-key-content"), + Perm: 0600, + }, + }, + }, + expectedEventTypes: nil, // No events when no changes. + expectedDirectories: map[string]adtesting.DirectoryState{ + "configmaps/test-config": { + "config.yaml": { + Content: []byte("test-config-content"), + Perm: 0644, + }, + }, + "secrets/test-secret": { + "tls.crt": { + Content: []byte("test-cert-content"), + Perm: 0600, + }, + "tls.key": { + Content: []byte("test-key-content"), + Perm: 0600, + }, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + controller, eventRecorder, stopCh := createController(t.TempDir(), + tc.configMapsToSync, tc.configMapObjects, tc.configMapGetErrors, + tc.secretsToSync, tc.secretObjects, tc.secretGetErrors, + ) + defer close(stopCh) + + for path, state := range tc.existingDirectories { + targetPath := filepath.Join(controller.destinationDir, path) + state.Write(t, targetPath, 0755) + } + + syncCtx := factory.NewSyncContext("CertSyncController", eventRecorder) + err := controller.sync(context.Background(), syncCtx) + if err != nil { + t.Log("sync returned an error:", err) + } + + if tc.expectedError && err == nil { + t.Errorf("Expected error but got none") + } else if !tc.expectedError && err != nil { + t.Errorf("Unexpected error: %v", err) + } + + verifyEvents(t, eventRecorder, tc.expectedEventTypes) + + // Check filesystem state. We need to gather all paths to know which are extra. + extraPaths := sets.NewString() + filepath.Walk(controller.destinationDir, func(path string, info os.FileInfo, err error) error { + if !info.IsDir() || + path == controller.destinationDir || + strings.HasSuffix(path, "/staging") || + strings.HasSuffix(path, "/staging/cert-sync") || + strings.HasSuffix(path, "/staging/cert-sync/secrets") || + strings.HasSuffix(path, "/staging/cert-sync/configmaps") || + path == filepath.Join(controller.destinationDir, "configmaps") || + path == filepath.Join(controller.destinationDir, "secrets") { + + return nil + } + + extraPaths.Insert(path) + return nil + }) + for path, state := range tc.expectedDirectories { + targetPath := filepath.Join(controller.destinationDir, path) + state.CheckDirectoryMatches(t, targetPath, 0755) + extraPaths.Delete(targetPath) + } + if extraPaths.Len() > 0 { + t.Errorf("Directories that should not be there detected: %v", extraPaths.List()) + } + }) + } +} + +func createController( + destinationDir string, + configMapsToSync []installer.UnrevisionedResource, + configMapObjects []*corev1.ConfigMap, + configMapGetErrors map[string]error, + secretsToSync []installer.UnrevisionedResource, + secretObjects []*corev1.Secret, + secretGetErrors map[string]error, +) (*CertSyncController, events.Recorder, chan struct{}) { + kubeObjects := make([]runtime.Object, 0) + for _, cm := range configMapObjects { + kubeObjects = append(kubeObjects, cm) + } + for _, secret := range secretObjects { + kubeObjects = append(kubeObjects, secret) + } + kubeClient := fake.NewClientset(kubeObjects...) + + if configMapGetErrors != nil { + kubeClient.PrependReactor("get", "configmaps", func(action ktesting.Action) (handled bool, ret runtime.Object, err error) { + getAction := action.(ktesting.GetAction) + if err, exists := configMapGetErrors[getAction.GetName()]; exists { + return true, nil, err + } + return false, nil, nil + }) + } + + if secretGetErrors != nil { + kubeClient.PrependReactor("get", "secrets", func(action ktesting.Action) (handled bool, ret runtime.Object, err error) { + getAction := action.(ktesting.GetAction) + if err, exists := secretGetErrors[getAction.GetName()]; exists { + return true, nil, err + } + return false, nil, nil + }) + } + + informers := informers.NewSharedInformerFactoryWithOptions(kubeClient, 1*time.Hour) + eventRecorder := events.NewInMemoryRecorder("CertSyncController", clocktesting.NewFakeClock(time.Now())) + + controller := &CertSyncController{ + destinationDir: destinationDir, + namespace: testingNamespace, + configMaps: configMapsToSync, + secrets: secretsToSync, + configMapGetter: kubeClient.CoreV1().ConfigMaps(testingNamespace), + configMapLister: informers.Core().V1().ConfigMaps().Lister(), + secretGetter: kubeClient.CoreV1().Secrets(testingNamespace), + secretLister: informers.Core().V1().Secrets().Lister(), + eventRecorder: eventRecorder, + } + + stopCh := make(chan struct{}) + informers.Start(stopCh) + informers.WaitForCacheSync(stopCh) + + return controller, eventRecorder, stopCh +} + +func verifyEvents(t *testing.T, eventRecorder events.Recorder, expectedEventTypes []string) { + var gotEventTypes []string + for _, event := range eventRecorder.(events.InMemoryRecorder).Events() { + gotEventTypes = append(gotEventTypes, event.Reason) + } + + if !cmp.Equal(gotEventTypes, expectedEventTypes) { + t.Errorf("Unexpected event types (-want, +got): \n%v", cmp.Diff(expectedEventTypes, gotEventTypes)) + } +} + +func createConfigMap(name string, data map[string]string) *corev1.ConfigMap { + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: testingNamespace, + }, + Data: data, + } +} + +func createSecret(name string, data map[string][]byte) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: testingNamespace, + }, + Data: data, + } +} + +// TestDynamicCertificates makes sure the receiving side of certificate synchronization works as expected. +// It reads and watches the certificates being synchronized in the same way as e.g. kube-apiserver, +// the very same libraries are being used. +func TestDynamicCertificates(t *testing.T) { + const typeName = "secret" + om := metav1.ObjectMeta{ + Namespace: "openshift-kube-apiserver", + Name: "s1", + } + + // Generate all necessary keypairs. + tlsCert, tlsKey := generateKeypair(t) + tlsCertUpdated, tlsKeyUpdated := generateKeypair(t) + + // Write the keypair into a secret directory. + secretDir := filepath.Join(t.TempDir(), "secrets", om.Name) + stagingDir := filepath.Join(t.TempDir(), "staging", stagingDirUID, "secrets", om.Name) + certFile := filepath.Join(secretDir, "tls.crt") + keyFile := filepath.Join(secretDir, "tls.key") + + if err := os.MkdirAll(secretDir, 0700); err != nil { + t.Fatalf("Failed to create secret directory %q: %v", secretDir, err) + } + if err := os.WriteFile(certFile, tlsCert, 0600); err != nil { + t.Fatalf("Failed to write TLS certificate into %q: %v", certFile, err) + } + if err := os.WriteFile(keyFile, tlsKey, 0600); err != nil { + t.Fatalf("Failed to write TLS key into %q: %v", keyFile, err) + } + + // Start the watcher. + // This reads the keypair synchronously so the initial state is loaded here. + dc, err := dynamiccertificates.NewDynamicServingContentFromFiles("localhost TLS", certFile, keyFile) + if err != nil { + t.Fatalf("Failed to init dynamic certificate: %v", err) + } + + // Check the initial keypair is loaded. + cert, key := dc.CurrentCertKeyContent() + if !bytes.Equal(cert, tlsCert) || !bytes.Equal(key, tlsKey) { + t.Fatal("Unexpected initial keypair loaded") + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + dc.Run(ctx, 1) + }() + defer wg.Wait() + defer cancel() + + // Poll until update detected. + files := map[string]types.File{ + "tls.crt": {Content: tlsCertUpdated, Perm: 0600}, + "tls.key": {Content: tlsKeyUpdated, Perm: 0600}, + } + err = wait.PollUntilContextCancel(ctx, 250*time.Millisecond, true, func(ctx context.Context) (bool, error) { + // Replace the secret directory. + if err := atomicdir.Sync(secretDir, 0700, stagingDir, files); err != nil { + t.Errorf("Failed to write files: %v", err) + return false, err + } + + // Check the loaded content matches. + // This is most probably updated based on write in a previous Poll invocation. + cert, key := dc.CurrentCertKeyContent() + return bytes.Equal(cert, tlsCertUpdated) && bytes.Equal(key, tlsKeyUpdated), nil + }) + if err != nil { + t.Fatalf("Failed to wait for dynamic certificate: %v", err) + } +} + +// generateKeypair returns (cert, key). +func generateKeypair(t *testing.T) ([]byte, []byte) { + t.Helper() + + privateKey, err := ecdsa.GenerateKey(elliptic.P224(), rand.Reader) + if err != nil { + t.Fatalf("Failed to generate TLS key: %v", err) + } + + notBefore := time.Now() + notAfter := notBefore.Add(1 * time.Hour) + + serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128) + serialNumber, err := rand.Int(rand.Reader, serialNumberLimit) + if err != nil { + t.Fatalf("Failed to generate serial number for TLS keypair: %v", err) + } + + template := x509.Certificate{ + SerialNumber: serialNumber, + Subject: pkix.Name{ + Organization: []string{"Example Org"}, + }, + NotBefore: notBefore, + NotAfter: notAfter, + KeyUsage: x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + DNSNames: []string{"example.com"}, + } + + publicKeyBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &privateKey.PublicKey, privateKey) + if err != nil { + t.Fatalf("Failed to create TLS certificate: %v", err) + } + + var certOut bytes.Buffer + if err := pem.Encode(&certOut, &pem.Block{Type: "CERTIFICATE", Bytes: publicKeyBytes}); err != nil { + t.Fatalf("Failed to write certificate PEM: %v", err) + } + + privateKeyBytes, err := x509.MarshalPKCS8PrivateKey(privateKey) + if err != nil { + t.Fatalf("Unable to marshal private key: %v", err) + } + + var keyOut bytes.Buffer + if err := pem.Encode(&keyOut, &pem.Block{Type: "PRIVATE KEY", Bytes: privateKeyBytes}); err != nil { + t.Fatalf("Failed to write certificate PEM: %v", err) + } + + return certOut.Bytes(), keyOut.Bytes() +} diff --git a/pkg/operator/staticpod/installerpod/cmd.go b/pkg/operator/staticpod/installerpod/cmd.go index 31afadff86..35424a56f5 100644 --- a/pkg/operator/staticpod/installerpod/cmd.go +++ b/pkg/operator/staticpod/installerpod/cmd.go @@ -5,38 +5,40 @@ import ( "fmt" "os" "path" + "path/filepath" "sort" "strconv" "strings" "time" - "k8s.io/utils/clock" - - "k8s.io/apimachinery/pkg/util/wait" - "github.com/blang/semver/v4" "github.com/davecgh/go-spew/spew" + "github.com/openshift/library-go/pkg/operator/staticpod/internal/atomicdir/types" "github.com/spf13/cobra" "github.com/spf13/pflag" - "k8s.io/klog/v2" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/server" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" + "k8s.io/klog/v2" + "k8s.io/utils/clock" "github.com/openshift/library-go/pkg/config/client" "github.com/openshift/library-go/pkg/operator/events" "github.com/openshift/library-go/pkg/operator/resource/resourceread" "github.com/openshift/library-go/pkg/operator/resource/retry" - "github.com/openshift/library-go/pkg/operator/staticpod" "github.com/openshift/library-go/pkg/operator/staticpod/internal" + "github.com/openshift/library-go/pkg/operator/staticpod/internal/atomicdir" "github.com/openshift/library-go/pkg/operator/staticpod/internal/flock" ) +const stagingDirUID = "installer" + type InstallOptions struct { // TODO replace with genericclioptions KubeConfig string @@ -219,8 +221,10 @@ func (o *InstallOptions) kubeletVersion(ctx context.Context) (string, error) { func (o *InstallOptions) copySecretsAndConfigMaps(ctx context.Context, resourceDir string, secretNames, optionalSecretNames, configNames, optionalConfigNames sets.Set[string], prefixed bool) error { - klog.Infof("Creating target resource directory %q ...", resourceDir) - if err := os.MkdirAll(resourceDir, 0755); err != nil && !os.IsExist(err) { + + stagingDirBase := getStagingDir(resourceDir) + klog.Infof("Pruning staging directory %q ...", stagingDirBase) + if err := os.RemoveAll(stagingDirBase); err != nil { return err } @@ -258,34 +262,43 @@ func (o *InstallOptions) copySecretsAndConfigMaps(ctx context.Context, resourceD if prefixed { secretBaseName = o.prefixFor(secret.Name) } - contentDir := path.Join(resourceDir, "secrets", secretBaseName) - klog.Infof("Creating directory %q ...", contentDir) - if err := os.MkdirAll(contentDir, 0755); err != nil { - return err - } - for filename, content := range secret.Data { - if err := writeSecret(content, path.Join(contentDir, filename)); err != nil { - return err + + contentDir := getSecretTargetDir(resourceDir, secretBaseName) + stagingDir := getSecretStagingDir(resourceDir, secretBaseName) + + files := make(map[string]types.File, len(secret.Data)) + for name, content := range secret.Data { + files[name] = types.File{ + Content: content, + Perm: getFilePermissionsSecret(name), } } + + if err := atomicdir.Sync(contentDir, 0755, stagingDir, files); err != nil { + return fmt.Errorf("failed to sync secret %s/%s (directory %q): %w", secret.Namespace, secret.Name, contentDir, err) + } } for _, configmap := range configs { configMapBaseName := configmap.Name if prefixed { configMapBaseName = o.prefixFor(configmap.Name) } - contentDir := path.Join(resourceDir, "configmaps", configMapBaseName) - klog.Infof("Creating directory %q ...", contentDir) - if err := os.MkdirAll(contentDir, 0755); err != nil { - return err - } - for filename, content := range configmap.Data { - if err := writeConfig([]byte(content), path.Join(contentDir, filename)); err != nil { - return err + + contentDir := getConfigMapTargetDir(resourceDir, configMapBaseName) + stagingDir := getConfigMapStagingDir(resourceDir, configMapBaseName) + + files := make(map[string]types.File, len(configmap.Data)) + for name, content := range configmap.Data { + files[name] = types.File{ + Content: []byte(content), + Perm: getFilePermissionsConfigMap(name), } } - } + if err := atomicdir.Sync(contentDir, 0755, stagingDir, files); err != nil { + return fmt.Errorf("failed to sync configmap %s/%s (directory %q): %w", configmap.Namespace, configmap.Name, contentDir, err) + } + } return nil } @@ -626,22 +639,36 @@ func (o *InstallOptions) writePod(rawPodBytes []byte, manifestFileName, resource return nil } -func writeConfig(content []byte, fullFilename string) error { - klog.Infof("Writing config file %q ...", fullFilename) +func getStagingDir(targetDir string) string { + return filepath.Join(targetDir, "staging", stagingDirUID) +} - filePerms := os.FileMode(0600) - if strings.HasSuffix(fullFilename, ".sh") { - filePerms = 0755 - } - return staticpod.WriteFileAtomic(content, filePerms, fullFilename) +func getConfigMapTargetDir(targetDir, configMapName string) string { + return filepath.Join(targetDir, "configmaps", configMapName) } -func writeSecret(content []byte, fullFilename string) error { - klog.Infof("Writing secret manifest %q ...", fullFilename) +func getConfigMapStagingDir(targetDir, configMapName string) string { + return filepath.Join(getStagingDir(targetDir), "configmaps", configMapName) +} + +func getSecretTargetDir(targetDir, secretName string) string { + return filepath.Join(targetDir, "secrets", secretName) +} + +func getSecretStagingDir(targetDir, secretName string) string { + return filepath.Join(getStagingDir(targetDir), "secrets", secretName) +} + +func getFilePermissionsConfigMap(filename string) os.FileMode { + if strings.HasSuffix(filename, ".sh") { + return 0755 + } + return 0600 +} - filePerms := os.FileMode(0600) - if strings.HasSuffix(fullFilename, ".sh") { - filePerms = 0700 +func getFilePermissionsSecret(filename string) os.FileMode { + if strings.HasSuffix(filename, ".sh") { + return 0700 } - return staticpod.WriteFileAtomic(content, filePerms, fullFilename) + return 0600 } diff --git a/pkg/operator/staticpod/installerpod/cmd_test.go b/pkg/operator/staticpod/installerpod/cmd_test.go index 5e6201b47b..cd046920ec 100644 --- a/pkg/operator/staticpod/installerpod/cmd_test.go +++ b/pkg/operator/staticpod/installerpod/cmd_test.go @@ -4,11 +4,11 @@ import ( "context" "os" "path" - "reflect" "strings" "testing" "time" + "github.com/google/go-cmp/cmp" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -51,8 +51,8 @@ func TestCopyContent(t *testing.T) { Revision: "006", Namespace: "some-ns", PodConfigMapNamePrefix: "kube-apiserver-pod", - SecretNamePrefixes: []string{"first", "second"}, - ConfigMapNamePrefixes: []string{"alpha", "bravo"}, + SecretNamePrefixes: []string{"first", "second", "third"}, + ConfigMapNamePrefixes: []string{"alpha", "bravo", "delta"}, }, client: func() *fake.Clientset { return fake.NewSimpleClientset( @@ -70,6 +70,12 @@ func TestCopyContent(t *testing.T) { "dos-B.crt": []byte("dos"), }, }, + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Namespace: "some-ns", Name: "third-006"}, + Data: map[string][]byte{ + "run-third.sh": []byte("echo third"), + }, + }, &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{Namespace: "some-ns", Name: "alpha-006"}, Data: map[string]string{ @@ -90,23 +96,31 @@ func TestCopyContent(t *testing.T) { "pod.yaml": podYaml, }, }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Namespace: "some-ns", Name: "delta-006"}, + Data: map[string]string{ + "run-delta.sh": "echo delta", + }, + }, ) }, expected: func(t *testing.T, resourceDir, podDir string) { - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "first", "one-A.crt"), "one") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "first", "two-A.crt"), "two") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "second", "uno-B.crt"), "uno") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "second", "dos-B.crt"), "dos") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "alpha", "apple-A.crt"), "apple") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "alpha", "banana-A.crt"), "banana") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "bravo", "manzana-B.crt"), "manzana") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "bravo", "platano-B.crt"), "platano") + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "first", "one-A.crt"), "one", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "first", "two-A.crt"), "two", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "second", "uno-B.crt"), "uno", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "second", "dos-B.crt"), "dos", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "third", "run-third.sh"), "echo third", 0700) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "alpha", "apple-A.crt"), "apple", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "alpha", "banana-A.crt"), "banana", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "bravo", "manzana-B.crt"), "manzana", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "bravo", "platano-B.crt"), "platano", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "delta", "run-delta.sh"), "echo delta", 0755) checkFileContentMatchesPod(t, path.Join(resourceDir, "kube-apiserver-pod-006", "kube-apiserver-pod.yaml"), podYaml) checkFileContentMatchesPod(t, path.Join(podDir, "kube-apiserver-pod.yaml"), podYaml) }, }, { - name: "optional-secrets-confmaps", + name: "optional-secrets-configmaps", o: InstallOptions{ Revision: "006", Namespace: "some-ns", @@ -169,18 +183,18 @@ func TestCopyContent(t *testing.T) { ) }, expected: func(t *testing.T, resourceDir, podDir string) { - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "first", "one-A.crt"), "one") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "first", "two-A.crt"), "two") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "second", "uno-B.crt"), "uno") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "second", "dos-B.crt"), "dos") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "third", "tres-C.crt"), "tres") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "third", "cuatro-C.crt"), "cuatro") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "alpha", "apple-A.crt"), "apple") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "alpha", "banana-A.crt"), "banana") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "bravo", "manzana-B.crt"), "manzana") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "bravo", "platano-B.crt"), "platano") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "charlie", "apple-C.crt"), "apple") - checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "charlie", "banana-C.crt"), "banana") + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "first", "one-A.crt"), "one", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "first", "two-A.crt"), "two", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "second", "uno-B.crt"), "uno", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "second", "dos-B.crt"), "dos", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "third", "tres-C.crt"), "tres", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "secrets", "third", "cuatro-C.crt"), "cuatro", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "alpha", "apple-A.crt"), "apple", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "alpha", "banana-A.crt"), "banana", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "bravo", "manzana-B.crt"), "manzana", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "bravo", "platano-B.crt"), "platano", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "charlie", "apple-C.crt"), "apple", 0600) + checkFileContent(t, path.Join(resourceDir, "kube-apiserver-pod-006", "configmaps", "charlie", "banana-C.crt"), "banana", 0600) checkFileContentMatchesPod(t, path.Join(resourceDir, "kube-apiserver-pod-006", "kube-apiserver-pod.yaml"), podYaml) checkFileContentMatchesPod(t, path.Join(podDir, "kube-apiserver-pod.yaml"), podYaml) }, @@ -215,13 +229,7 @@ func TestCopyContent(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - testDir, err := os.MkdirTemp("", "copy-content-test") - if err != nil { - t.Fatal(err) - } - defer func() { - os.Remove(testDir) - }() + testDir := t.TempDir() o := test.o o.KubeClient = test.client() @@ -230,7 +238,7 @@ func TestCopyContent(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - err = o.copyContent(ctx) + err := o.copyContent(ctx) switch { case err == nil && len(test.expectedErr) == 0: case err != nil && len(test.expectedErr) == 0: @@ -258,15 +266,25 @@ func TestKubeletVersion(t *testing.T) { } } -func checkFileContent(t *testing.T, file, expected string) { - actual, err := os.ReadFile(file) +func checkFileContent(t *testing.T, file, expected string, expectedPerm os.FileMode) { + actualBytes, err := os.ReadFile(file) if err != nil { t.Error(err) return } + actual := string(actualBytes) + + stat, err := os.Stat(file) + if err != nil { + t.Errorf("Failed to stat %q: %v", file, err) + return + } + if gotPerm := stat.Mode().Perm(); gotPerm != expectedPerm { + t.Errorf("File permissions mismatch for %q: expected %v, got %v", file, gotPerm, expectedPerm) + } - if !reflect.DeepEqual(expected, string(actual)) { - t.Errorf("%q: expected %q, got %q", file, expected, string(actual)) + if !cmp.Equal(expected, actual) { + t.Errorf("File content mismatch for %q:\n%s", file, cmp.Diff(expected, actual)) } } diff --git a/pkg/operator/staticpod/internal/atomicdir/sync.go b/pkg/operator/staticpod/internal/atomicdir/sync.go index a56c961a33..0c2cc3dfff 100644 --- a/pkg/operator/staticpod/internal/atomicdir/sync.go +++ b/pkg/operator/staticpod/internal/atomicdir/sync.go @@ -51,7 +51,7 @@ func sync(fs *fileSystem, targetDir string, targetDirPerm os.FileMode, stagingDi return fmt.Errorf("failed creating target directory: %w", err) } - klog.Infof("Creating staging directory to swap for %q ...", targetDir) + klog.Infof("Creating staging directory %q ...", stagingDir) if err := fs.MkdirAll(stagingDir, targetDirPerm); err != nil { return fmt.Errorf("failed creating staging directory: %w", err) } @@ -80,7 +80,7 @@ func sync(fs *fileSystem, targetDir string, targetDirPerm os.FileMode, stagingDi } } - klog.Infof("Atomically swapping target directory %q with staging directory %q ...", targetDir, stagingDir) + klog.Infof("Atomically swapping staging directory %q with target directory %q ...", stagingDir, targetDir) if err := fs.SwapDirectories(targetDir, stagingDir); err != nil { return fmt.Errorf("failed swapping target directory %q with staging directory %q: %w", targetDir, stagingDir, err) }