diff --git a/pkg/controller/build/imagepruner/imageinspect.go b/pkg/controller/build/imagepruner/imageinspect.go index 1f479befd5..d7a8756053 100644 --- a/pkg/controller/build/imagepruner/imageinspect.go +++ b/pkg/controller/build/imagepruner/imageinspect.go @@ -3,14 +3,13 @@ package imagepruner import ( "context" "fmt" - "strings" "github.com/containers/common/pkg/retry" - "github.com/containers/image/v5/docker" "github.com/containers/image/v5/image" "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/types" digest "github.com/opencontainers/go-digest" + "github.com/openshift/machine-config-operator/pkg/imageutils" ) const ( @@ -58,24 +57,6 @@ func (i *imageInspectorImpl) DeleteImage(ctx context.Context, sysCtx *types.Syst return deleteImage(ctx, sysCtx, image) } -// parseImageName parses the image name string into an ImageReference, -// handling various prefix formats like "docker://" and ensuring a standard format. -func parseImageName(imgName string) (types.ImageReference, error) { - if strings.Contains(imgName, "//") && !strings.HasPrefix(imgName, "docker://") { - return nil, fmt.Errorf("unknown transport for pullspec %s", imgName) - } - - if strings.HasPrefix(imgName, "docker://") { - imgName = strings.ReplaceAll(imgName, "docker://", "//") - } - - if !strings.HasPrefix(imgName, "//") { - imgName = "//" + imgName - } - - return docker.Transport.ParseReference(imgName) -} - // deleteImage attempts to delete the specified image with retries, // using the provided context and system context. // @@ -85,7 +66,7 @@ func deleteImage(ctx context.Context, sysCtx *types.SystemContext, imageName str MaxRetry: cmdRetriesCount, } - ref, err := parseImageName(imageName) + ref, err := imageutils.ParseImageName(imageName) if err != nil { return err } @@ -105,29 +86,19 @@ func deleteImage(ctx context.Context, sysCtx *types.SystemContext, imageName str // //nolint:unparam func imageInspect(ctx context.Context, sysCtx *types.SystemContext, imageName string) (*types.ImageInspectInfo, *digest.Digest, error) { - var ( - src types.ImageSource - imgInspect *types.ImageInspectInfo - err error - ) + ref, err := imageutils.ParseImageName(imageName) + if err != nil { + return nil, nil, fmt.Errorf("error parsing image name %q: %w", imageName, err) + } retryOpts := retry.RetryOptions{ MaxRetry: cmdRetriesCount, } - - ref, err := parseImageName(imageName) + src, err := imageutils.GetImageSourceFromReference(ctx, sysCtx, ref, &retryOpts) if err != nil { - return nil, nil, fmt.Errorf("error parsing image name %q: %w", imageName, err) - } - - // retry.IfNecessary takes into account whether the error is "retryable" - // so we don't keep looping on errors that will never resolve - if err := retry.RetryIfNecessary(ctx, func() error { - src, err = ref.NewImageSource(ctx, sysCtx) - return err - }, &retryOpts); err != nil { - return nil, nil, newErrImage(imageName, fmt.Errorf("error getting image source: %w", err)) + return nil, nil, fmt.Errorf("error getting image source for %s: %w", imageName, err) } + defer src.Close() var rawManifest []byte unparsedInstance := image.UnparsedInstance(src, nil) @@ -144,14 +115,13 @@ func imageInspect(ctx context.Context, sysCtx *types.SystemContext, imageName st return nil, nil, fmt.Errorf("error retrieving image digest: %q: %w", imageName, err) } - defer src.Close() - img, err := image.FromUnparsedImage(ctx, sysCtx, unparsedInstance) if err != nil { return nil, nil, newErrImage(imageName, fmt.Errorf("error parsing manifest for image: %w", err)) } - if err := retry.RetryIfNecessary(ctx, func() error { + var imgInspect *types.ImageInspectInfo + if err := retry.IfNecessary(ctx, func() error { imgInspect, err = img.Inspect(ctx) return err }, &retryOpts); err != nil { diff --git a/pkg/controller/build/imagepruner/imagepruner.go b/pkg/controller/build/imagepruner/imagepruner.go index 28c3089971..60c233c96f 100644 --- a/pkg/controller/build/imagepruner/imagepruner.go +++ b/pkg/controller/build/imagepruner/imagepruner.go @@ -3,18 +3,14 @@ package imagepruner import ( "context" "fmt" - "os" - "path/filepath" - "strings" + "github.com/openshift/machine-config-operator/pkg/imageutils" corev1 "k8s.io/api/core/v1" "k8s.io/klog/v2" "github.com/containers/image/v5/types" "github.com/opencontainers/go-digest" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" - "github.com/openshift/machine-config-operator/pkg/controller/template" - "github.com/openshift/machine-config-operator/pkg/secrets" ) // ImagePruner defines the interface for inspecting and deleting container images. @@ -42,18 +38,18 @@ func NewImagePruner() ImagePruner { // InspectImage inspects the given image using the provided secret. It also accepts a // ControllerConfig so that certificates may be placed on the filesystem for authentication. func (i *imagePrunerImpl) InspectImage(ctx context.Context, pullspec string, secret *corev1.Secret, cc *mcfgv1.ControllerConfig) (*types.ImageInspectInfo, *digest.Digest, error) { - sysCtx, err := i.prepareSystemContext(secret, cc) + sysCtx, err := imageutils.NewSysContextFromControllerConfig(secret, cc) if err != nil { return nil, nil, fmt.Errorf("could not prepare for image inspection: %w", err) } defer func() { - if err := i.cleanup(sysCtx); err != nil { + if err := sysCtx.Cleanup(); err != nil { klog.Warningf("Unable to clean up after inspection of %s: %s", pullspec, err) } }() - info, digest, err := i.images.ImageInspect(ctx, sysCtx, pullspec) + info, digest, err := i.images.ImageInspect(ctx, sysCtx.SysContext, pullspec) if err != nil { return nil, nil, fmt.Errorf("could not inspect image: %w", err) } @@ -64,158 +60,20 @@ func (i *imagePrunerImpl) InspectImage(ctx context.Context, pullspec string, sec // DeleteImage deletes the given image using the provided secret. It also accepts a // ControllerConfig so that certificates may be placed on the filesystem for authentication. func (i *imagePrunerImpl) DeleteImage(ctx context.Context, pullspec string, secret *corev1.Secret, cc *mcfgv1.ControllerConfig) error { - sysCtx, err := i.prepareSystemContext(secret, cc) + sysCtx, err := imageutils.NewSysContextFromControllerConfig(secret, cc) if err != nil { return fmt.Errorf("could not prepare for image deletion: %w", err) } defer func() { - if err := i.cleanup(sysCtx); err != nil { + if err := sysCtx.Cleanup(); err != nil { klog.Warningf("Unable to clean up after deletion of %s: %s", pullspec, err) } }() - if err := i.images.DeleteImage(ctx, sysCtx, pullspec); err != nil { + if err := i.images.DeleteImage(ctx, sysCtx.SysContext, pullspec); err != nil { return fmt.Errorf("could not delete image: %w", err) } return nil } - -// prepareSystemContext prepares to perform the requested operation by first creating the -// certificate directory and then writing the authfile to the appropriate path. -func (i *imagePrunerImpl) prepareSystemContext(secret *corev1.Secret, cc *mcfgv1.ControllerConfig) (*types.SystemContext, error) { - // Make a deep copy of the ControllerConfig because the write process mutates - // the ControllerConfig in-place. - certsDir, err := i.prepareCerts(cc.DeepCopy()) - if err != nil { - return nil, fmt.Errorf("could not prepare certs: %w", err) - } - - authfilePath, err := i.prepareAuthfile(secret) - if err != nil { - return nil, fmt.Errorf("could not get authfile path for secret %s: %w", secret.Name, err) - } - - return &types.SystemContext{ - AuthFilePath: authfilePath, - DockerPerHostCertDirPath: certsDir, - }, nil -} - -// cleanup cleans up after an operation by removing the temporary certificates directory -// and the temporary authfile. -func (i *imagePrunerImpl) cleanup(sysCtx *types.SystemContext) error { - if err := os.RemoveAll(sysCtx.DockerPerHostCertDirPath); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("could not clean up certs directory %s: %w", sysCtx.DockerPerHostCertDirPath, err) - } - - if err := os.RemoveAll(sysCtx.AuthFilePath); err != nil && !os.IsNotExist(err) { - return fmt.Errorf("could not clean up authfile directory %s: %w", sysCtx.AuthFilePath, err) - } - - return nil -} - -// prepareCerts prepares the certificates by first creating a temporary directory for them -// and then writing the certs from the ControllerConfig to that directory. -func (i *imagePrunerImpl) prepareCerts(cc *mcfgv1.ControllerConfig) (string, error) { - certsDir, err := os.MkdirTemp("", "imagepruner-certs-dir") - if err != nil { - return "", fmt.Errorf("could not create temp dir: %w", err) - } - - if err := i.writeCerts(certsDir, cc); err != nil { - return "", fmt.Errorf("could not write certs: %w", err) - } - - return certsDir, nil -} - -// writeCerts extracts the certificates from the ControllerConfig and writes them -// to the appropriate directory, which defaults to /etc/docker/certs.d. -func (i *imagePrunerImpl) writeCerts(certsDir string, cc *mcfgv1.ControllerConfig) error { - template.UpdateControllerConfigCerts(cc) - - for _, irb := range cc.Spec.ImageRegistryBundleData { - if err := i.writeCertFromImageRegistryBundle(certsDir, irb); err != nil { - return fmt.Errorf("could not write image registry bundle from ImageRegistryBundleData: %w", err) - } - } - - for _, irb := range cc.Spec.ImageRegistryBundleUserData { - if err := i.writeCertFromImageRegistryBundle(certsDir, irb); err != nil { - return fmt.Errorf("could not write image registry bundle from ImageRegistryBundleUserData: %w", err) - } - } - - return nil -} - -// writeCertFromImageRegistryBundle writes a certificate from an image registry bundle -// to the specified certificates directory, creating necessary subdirectories. -func (i *imagePrunerImpl) writeCertFromImageRegistryBundle(certsDir string, irb mcfgv1.ImageRegistryBundle) error { - caFile := strings.ReplaceAll(irb.File, "..", ":") - - certDir := filepath.Join(certsDir, caFile) - - if err := os.MkdirAll(certDir, 0o755); err != nil { - return fmt.Errorf("could not create cert dir %q: %w", certDir, err) - } - - certFile := filepath.Join(certDir, "ca.crt") - - if err := os.WriteFile(certFile, irb.Data, 0o644); err != nil { - return fmt.Errorf("could not write cert file %q: %w", certFile, err) - } - - return nil -} - -// prepareAuthfile creates a temporary directory and writes the Docker secret -// (authfile) into a file named "authfile.json" within that directory. -// It returns the path to the created authfile. -func (i *imagePrunerImpl) prepareAuthfile(secret *corev1.Secret) (string, error) { - authfileDir, err := os.MkdirTemp("", "imagepruner-authfile") - if err != nil { - return "", fmt.Errorf("could not create temp dir for authfile: %w", err) - } - - authfilePath := filepath.Join(authfileDir, "authfile.json") - - is, err := secrets.NewImageRegistrySecret(secret) - if err != nil { - return "", fmt.Errorf("could not create an ImageRegistrySecret for '%s/%s': %w", secret.Namespace, secret.Name, err) - } - - secretBytes, err := is.JSONBytes(corev1.SecretTypeDockerConfigJson) - if err != nil { - return "", fmt.Errorf("could not normalize secret '%s/%s' to %s: %w", secret.Namespace, secret.Name, corev1.SecretTypeDockerConfigJson, err) - } - - if err := os.WriteFile(authfilePath, secretBytes, 0o644); err != nil { - return "", fmt.Errorf("could not write temp authfile %q for secret %q: %w", authfilePath, secret.Name, err) - } - - return authfilePath, nil -} - -// writeAuthfile ensures that the image registry secret is in the dockerconfigjson format -// and writes it to the specified path. -func (i *imagePrunerImpl) writeAuthfile(secret *corev1.Secret, authfilePath string) error { - is, err := secrets.NewImageRegistrySecret(secret) - if err != nil { - return fmt.Errorf("could not create an ImageRegistrySecret for '%s/%s': %w", secret.Namespace, secret.Name, err) - } - - secretBytes, err := is.JSONBytes(corev1.SecretTypeDockerConfigJson) - if err != nil { - return fmt.Errorf("could not normalize secret '%s/%s' to %s: %w", secret.Namespace, secret.Name, corev1.SecretTypeDockerConfigJson, err) - } - - if err := os.WriteFile(authfilePath, secretBytes, 0o644); err != nil { - return fmt.Errorf("could not write temp authfile %q for secret %q: %w", authfilePath, secret.Name, err) - } - - return nil -} diff --git a/pkg/controller/build/imagepruner/imagepruner_test.go b/pkg/controller/build/imagepruner/imagepruner_test.go index 7352abc593..5af8e94786 100644 --- a/pkg/controller/build/imagepruner/imagepruner_test.go +++ b/pkg/controller/build/imagepruner/imagepruner_test.go @@ -2,6 +2,7 @@ package imagepruner import ( "context" + "fmt" "testing" "time" @@ -23,12 +24,16 @@ type fakeImageInspector struct { deleteImageCalled bool deleteImagePullspec string deleteImageSysCtx *types.SystemContext + imageApiError bool } // ImageInspect is a mock implementation for testing, setting flags to indicate it was called. func (f *fakeImageInspector) ImageInspect(_ context.Context, sysCtx *types.SystemContext, pullspec string) (*types.ImageInspectInfo, *digest.Digest, error) { f.imageInspectCalled = true f.imageInspectPullspec = pullspec + if f.imageApiError { + return nil, nil, fmt.Errorf("fake inspect error") + } return nil, nil, nil } @@ -36,6 +41,9 @@ func (f *fakeImageInspector) ImageInspect(_ context.Context, sysCtx *types.Syste func (f *fakeImageInspector) DeleteImage(_ context.Context, sysCtx *types.SystemContext, pullspec string) error { f.deleteImageCalled = true f.deleteImagePullspec = pullspec + if f.imageApiError { + return fmt.Errorf("fake delete error") + } return nil } @@ -50,7 +58,8 @@ func TestImagePruner(t *testing.T) { testCases := []struct { name string inputSecret *corev1.Secret - expectError bool + authError bool + imageError bool }{ { name: "DockerConfigJSON", @@ -76,6 +85,32 @@ func TestImagePruner(t *testing.T) { Type: corev1.SecretTypeDockercfg, }, }, + { + name: "ImageErrorHandling", + inputSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pull-secret", + }, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(newSecret), + }, + Type: corev1.SecretTypeDockerConfigJson, + }, + imageError: true, + }, + { + name: "AuthErrorHandling", + inputSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pull-secret", + }, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(newSecret), + }, + Type: corev1.SecretTypeOpaque, + }, + authError: true, + }, } ctx, cancel := context.WithTimeout(context.Background(), time.Second) @@ -86,7 +121,9 @@ func TestImagePruner(t *testing.T) { t.Run(testCase.name, func(t *testing.T) { t.Parallel() - fii := &fakeImageInspector{} + fii := &fakeImageInspector{ + imageApiError: testCase.imageError, + } ip := &imagePrunerImpl{ images: fii, } @@ -94,36 +131,34 @@ func TestImagePruner(t *testing.T) { pullspec := "registry.hostname.com/org/repo:latest" cc := &mcfgv1.ControllerConfig{} - - sysCtx, err := ip.prepareSystemContext(testCase.inputSecret, cc) - if testCase.expectError { + _, _, err := ip.InspectImage(ctx, pullspec, testCase.inputSecret, cc) + if testCase.imageError { assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Contains(t, sysCtx.AuthFilePath, "authfile.json") - assert.FileExists(t, sysCtx.AuthFilePath) - assert.DirExists(t, sysCtx.DockerPerHostCertDirPath) - assert.NoError(t, ip.cleanup(sysCtx)) - assert.NoFileExists(t, sysCtx.AuthFilePath) - assert.NoDirExists(t, sysCtx.DockerPerHostCertDirPath) - } - - _, _, err = ip.InspectImage(ctx, pullspec, testCase.inputSecret, cc) - if testCase.expectError { + // SysContext context created but the API call failed + assert.True(t, fii.imageInspectCalled) + } else if testCase.authError { assert.Error(t, err) - assert.False(t, fii.imageInspectCalled) + // The API called wasn't performed cause the logic couldn't reach that point + assert.False(t, fii.deleteImageCalled) } else { assert.NoError(t, err) assert.True(t, fii.imageInspectCalled) + assert.Equal(t, fii.imageInspectPullspec, pullspec) } err = ip.DeleteImage(ctx, pullspec, testCase.inputSecret, cc) - if testCase.expectError { + if testCase.imageError { + assert.Error(t, err) + // SysContext context created but the API call failed + assert.True(t, fii.deleteImageCalled) + } else if testCase.authError { assert.Error(t, err) + // The API called wasn't performed cause the logic couldn't reach that point assert.False(t, fii.deleteImageCalled) } else { assert.NoError(t, err) assert.True(t, fii.deleteImageCalled) + assert.Equal(t, fii.deleteImagePullspec, pullspec) } }) } diff --git a/pkg/imageutils/reference.go b/pkg/imageutils/reference.go new file mode 100644 index 0000000000..264b982781 --- /dev/null +++ b/pkg/imageutils/reference.go @@ -0,0 +1,41 @@ +package imageutils + +import ( + "context" + "fmt" + "strings" + + "github.com/containers/common/pkg/retry" + "github.com/containers/image/v5/docker" + "github.com/containers/image/v5/types" +) + +// GetImageSourceFromReference creates an ImageSource from an ImageReference with retry logic. +func GetImageSourceFromReference(ctx context.Context, sysCtx *types.SystemContext, ref types.ImageReference, retryOpts *retry.RetryOptions) (types.ImageSource, error) { + var src types.ImageSource + var err error + if err := retry.RetryIfNecessary(ctx, func() error { + src, err = ref.NewImageSource(ctx, sysCtx) + return err + }, retryOpts); err != nil { + return nil, fmt.Errorf("error getting image source for %s: %w", ref.StringWithinTransport(), err) + } + return src, nil +} + +// ParseImageName parses an image name into a docker transport ImageReference. +func ParseImageName(imgName string) (types.ImageReference, error) { + if strings.Contains(imgName, "//") && !strings.HasPrefix(imgName, "docker://") { + return nil, fmt.Errorf("unknown transport for pullspec %s", imgName) + } + + if strings.HasPrefix(imgName, "docker://") { + imgName = strings.ReplaceAll(imgName, "docker://", "//") + } + + if !strings.HasPrefix(imgName, "//") { + imgName = "//" + imgName + } + + return docker.Transport.ParseReference(imgName) +} diff --git a/pkg/imageutils/sys_context.go b/pkg/imageutils/sys_context.go new file mode 100644 index 0000000000..8fc2f350a1 --- /dev/null +++ b/pkg/imageutils/sys_context.go @@ -0,0 +1,112 @@ +package imageutils + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/containers/image/v5/types" + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/openshift/machine-config-operator/pkg/secrets" + corev1 "k8s.io/api/core/v1" +) + +// SysContext wraps types.SystemContext and manages cleanup of temporary files. +type SysContext struct { + SysContext *types.SystemContext + temporalDir string +} + +// NewSysContextFromControllerConfig creates a SysContext with authentication and certificates +// from the provided secret and ControllerConfig. Caller must call Cleanup() method to remove temporary files. +func NewSysContextFromControllerConfig(secret *corev1.Secret, cc *mcfgv1.ControllerConfig) (*SysContext, error) { + temporalDir, err := os.MkdirTemp("", "syscontext-temporal-dir") + if err != nil { + return nil, fmt.Errorf("could not create SysContext temp dir: %w", err) + } + sysContext := &SysContext{ + temporalDir: temporalDir, + } + + certsDir, err := sysContext.buildCertsFromControllerConfig(cc) + if err != nil { + return nil, fmt.Errorf("could not prepare certs: %w", err) + } + + authfilePath, err := sysContext.buildAuthFileFromSecret(secret) + if err != nil { + return nil, fmt.Errorf("could not get authfile path for secret %s: %w", secret.Name, err) + } + sysContext.SysContext = &types.SystemContext{ + AuthFilePath: authfilePath, + DockerPerHostCertDirPath: certsDir, + } + return sysContext, nil +} + +// Cleanup removes all temporary files and directories created by NewSysContextFromControllerConfig. +func (s *SysContext) Cleanup() error { + if err := os.RemoveAll(s.temporalDir); err != nil && !os.IsNotExist(err) { + return fmt.Errorf("could not clean up SysContext temporal directory %s: %w", s.temporalDir, err) + } + return nil +} + +// buildCertsFromControllerConfig writes image registry certificates from the ControllerConfig to the temporal directory. +func (s *SysContext) buildCertsFromControllerConfig(cc *mcfgv1.ControllerConfig) (string, error) { + certsDir := filepath.Join(s.temporalDir, "certs-dir") + for _, irb := range cc.Spec.ImageRegistryBundleData { + if err := writeCertFromImageRegistryBundle(certsDir, irb); err != nil { + return "", fmt.Errorf("could not write image registry bundle from ImageRegistryBundleData: %w", err) + } + } + + for _, irb := range cc.Spec.ImageRegistryBundleUserData { + if err := writeCertFromImageRegistryBundle(certsDir, irb); err != nil { + return "", fmt.Errorf("could not write image registry bundle from ImageRegistryBundleUserData: %w", err) + } + } + return certsDir, nil +} + +// writeCertFromImageRegistryBundle writes a certificate from an image registry bundle +// to the specified certificates directory. Path traversal attempts (..) are sanitized to colons. +func writeCertFromImageRegistryBundle(certsDir string, irb mcfgv1.ImageRegistryBundle) error { + caFile := strings.ReplaceAll(irb.File, "..", ":") + + certDir := filepath.Join(certsDir, caFile) + + if err := os.MkdirAll(certDir, 0o755); err != nil { + return fmt.Errorf("could not create cert dir %q: %w", certDir, err) + } + + certFile := filepath.Join(certDir, "ca.crt") + + if err := os.WriteFile(certFile, irb.Data, 0o644); err != nil { + return fmt.Errorf("could not write cert file %q: %w", certFile, err) + } + + return nil +} + +// buildAuthFileFromSecret writes the Docker secret as authfile.json to the temporal directory. +func (s *SysContext) buildAuthFileFromSecret(secret *corev1.Secret) (string, error) { + authfilePath := filepath.Join(s.temporalDir, "authfile.json") + + is, err := secrets.NewImageRegistrySecret(secret) + if err != nil { + return "", fmt.Errorf("could not create an ImageRegistrySecret for '%s/%s': %w", secret.Namespace, secret.Name, err) + } + + secretBytes, err := is.JSONBytes(corev1.SecretTypeDockerConfigJson) + if err != nil { + return "", fmt.Errorf("could not normalize secret '%s/%s' to %s: %w", secret.Namespace, secret.Name, corev1.SecretTypeDockerConfigJson, err) + } + + if err := os.WriteFile(authfilePath, secretBytes, 0o644); err != nil { + return "", fmt.Errorf("could not write temp authfile %q for secret %q: %w", authfilePath, secret.Name, err) + } + + return authfilePath, nil +} diff --git a/pkg/imageutils/sys_context_test.go b/pkg/imageutils/sys_context_test.go new file mode 100644 index 0000000000..4c61a63a9c --- /dev/null +++ b/pkg/imageutils/sys_context_test.go @@ -0,0 +1,99 @@ +package imageutils + +import ( + "path/filepath" + "testing" + + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestNewSysContextFromControllerConfig(t *testing.T) { + legacySecret := `{"registry.hostname.com": {"username": "user", "password": "s3kr1t", "auth": "s00pers3kr1t", "email": "user@hostname.com"}}` + newSecret := `{"auths":` + legacySecret + `}` + + testCert := []byte(`-----BEGIN CERTIFICATE----- +MIICljCCAX4CCQCKz8Vz4VR5+jANBgkqhkiG9w0BAQsFADANMQswCQYDVQQGEwJV +UzAeFw0yMDAxMDEwMDAwMDBaFw0zMDAxMDEwMDAwMDBaMA0xCzAJBgNVBAYTAlVT +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAuOSW8w== +-----END CERTIFICATE-----`) + + testCases := []struct { + name string + secret *corev1.Secret + }{ + { + name: "DockerConfigJSON", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pull-secret", + Namespace: "test-namespace", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(newSecret), + }, + }, + }, + { + name: "Dockercfg", + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pull-secret", + Namespace: "test-namespace", + }, + Type: corev1.SecretTypeDockercfg, + Data: map[string][]byte{ + corev1.DockerConfigKey: []byte(legacySecret), + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cc := &mcfgv1.ControllerConfig{ + Spec: mcfgv1.ControllerConfigSpec{ + ImageRegistryBundleData: []mcfgv1.ImageRegistryBundle{ + { + File: "registry.hostname.com", + Data: testCert, + }, + }, + }, + } + + sysCtx, err := NewSysContextFromControllerConfig(tc.secret, cc) + require.NoError(t, err, "NewSysContextFromControllerConfig should not fail") + require.NotNil(t, sysCtx, "SysContext wrapper should not be nil") + require.NotNil(t, sysCtx.SysContext, "Underlying SystemContext should not be nil") + + // Check that temporal directory was created + assert.NotEmpty(t, sysCtx.temporalDir, "Temporal directory should not be empty") + assert.DirExists(t, sysCtx.temporalDir, "Temporal directory should exist") + + // Check that AuthFilePath was set and file exists + assert.NotEmpty(t, sysCtx.SysContext.AuthFilePath, "AuthFilePath should not be empty") + assert.FileExists(t, sysCtx.SysContext.AuthFilePath, "AuthFile should exist") + assert.Contains(t, sysCtx.SysContext.AuthFilePath, "authfile.json", "AuthFile should be named authfile.json") + + // Check that DockerPerHostCertDirPath was set and directory exists + assert.NotEmpty(t, sysCtx.SysContext.DockerPerHostCertDirPath, "DockerPerHostCertDirPath should not be empty") + assert.DirExists(t, sysCtx.SysContext.DockerPerHostCertDirPath, "Certs directory should exist") + + // Check that cert file was created + certFile := filepath.Join(sysCtx.SysContext.DockerPerHostCertDirPath, "registry.hostname.com", "ca.crt") + assert.FileExists(t, certFile, "Cert file should exist") + + // Cleanup + err = sysCtx.Cleanup() + require.NoError(t, err, "Cleanup should not fail") + + // Verify cleanup removed the entire temporal directory + assert.NoDirExists(t, sysCtx.temporalDir, "Temporal directory should be removed after cleanup") + }) + } +}