From b81c28c3e25f165af8ef14f76d699518f4b6b5c2 Mon Sep 17 00:00:00 2001 From: dekiel Date: Thu, 10 Oct 2024 19:52:22 +0200 Subject: [PATCH 1/6] Read access token to use it in authentication. --- cmd/image-syncer/main.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/image-syncer/main.go b/cmd/image-syncer/main.go index d1c77ab07f03..1932c4d27e2b 100644 --- a/cmd/image-syncer/main.go +++ b/cmd/image-syncer/main.go @@ -253,6 +253,8 @@ func main() { if err != nil { log.WithError(err).Fatal("Could not open target auth key JSON") } + } else { + authCfg = []byte(cfg.AccessToken) } if cfg.DryRun { From 84d5141b4cb37593dffb2d9721562f2703a33585 Mon Sep 17 00:00:00 2001 From: dekiel Date: Fri, 11 Oct 2024 08:31:34 +0200 Subject: [PATCH 2/6] Extract building new authenticator to separate function. The authenticator passed to the sync function already contains key or token. --- cmd/image-syncer/image_syncer_suite_test.go | 13 +++++ cmd/image-syncer/main.go | 59 +++++++++++++-------- cmd/image-syncer/main_test.go | 55 +++++++++++++++++++ cmd/image-syncer/test-fixtures/keyfile.json | 1 + go.mod | 2 +- go.sum | 4 +- 6 files changed, 109 insertions(+), 25 deletions(-) create mode 100644 cmd/image-syncer/image_syncer_suite_test.go create mode 100644 cmd/image-syncer/main_test.go create mode 100644 cmd/image-syncer/test-fixtures/keyfile.json diff --git a/cmd/image-syncer/image_syncer_suite_test.go b/cmd/image-syncer/image_syncer_suite_test.go new file mode 100644 index 000000000000..09f386b8a85c --- /dev/null +++ b/cmd/image-syncer/image_syncer_suite_test.go @@ -0,0 +1,13 @@ +package main + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestImageSyncer(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "ImageSyncer Suite") +} diff --git a/cmd/image-syncer/main.go b/cmd/image-syncer/main.go index 1932c4d27e2b..a559c3bfc2b5 100644 --- a/cmd/image-syncer/main.go +++ b/cmd/image-syncer/main.go @@ -184,13 +184,7 @@ func SyncImage(ctx context.Context, src, dest string, dryRun bool, auth authn.Au } // SyncImages is a main syncing function that takes care of copying images. -func SyncImages(ctx context.Context, cfg *Config, images *imagesync.SyncDef, authCfg []byte) error { - var auth authn.Authenticator - if cfg.TargetKeyFile != "" { - auth = &authn.Basic{Username: "_json_key", Password: string(authCfg)} - } else { - auth = &authn.Bearer{Token: string(authCfg)} - } +func SyncImages(ctx context.Context, cfg *Config, images *imagesync.SyncDef, auth authn.Authenticator) error { for _, img := range images.Images { target, err := getTarget(img.Source, cfg.TargetRepoPrefix, img.Tag) imageType := "Index" @@ -224,6 +218,35 @@ func SyncImages(ctx context.Context, cfg *Config, images *imagesync.SyncDef, aut return nil } +// newAuthenticator creates a new authenticator based on the provided configuration +// An authenticator is used to authenticate to the target repository. +func (cfg *Config) newAuthenticator() (authn.Authenticator, error) { + log.Debug("started creating new authenticator") + var ( + auth authn.Authenticator + authCfg []byte + err error + ) + + if cfg.TargetKeyFile != "" { + log.WithField("targetKeyFile", cfg.TargetKeyFile).Debug("target key file path provided, reading the file") + authCfg, err = os.ReadFile(cfg.TargetKeyFile) + if err != nil { + return nil, fmt.Errorf("could not open target auth key JSON file, error: %w", err) + } + log.Debug("target key file read successfully, creating basic authenticator") + auth = &authn.Basic{Username: "_json_key", Password: string(authCfg)} + log.WithField("username", "_json_key").Debug("basic authenticator created successfully") + return auth, nil + } + if cfg.AccessToken != "" { + log.Debug("access token provided, creating bearer authenticator") + auth = &authn.Bearer{Token: cfg.AccessToken} + return auth, nil + } + return nil, fmt.Errorf("no target auth key file or access token provided") +} + func main() { log.Out = os.Stdout var cfg Config @@ -234,7 +257,6 @@ func main() { Long: `image-syncer copies docker images. It compares checksum between source and target and protects target images against overriding`, //nolint:revive Run: func(cmd *cobra.Command, args []string) { - var authCfg []byte logLevel := logrus.InfoLevel if cfg.Debug { logLevel = logrus.DebugLevel @@ -248,28 +270,21 @@ func main() { if err != nil { log.WithError(err).Fatal("Could not parse images file") } - if cfg.TargetKeyFile != "" { - authCfg, err = os.ReadFile(cfg.TargetKeyFile) - if err != nil { - log.WithError(err).Fatal("Could not open target auth key JSON") - } - } else { - authCfg = []byte(cfg.AccessToken) + + log.Info("Creating authenticator") + auth, err := cfg.newAuthenticator() + if err != nil { + log.WithError(err).Fatal("Failed to create authenticator") } if cfg.DryRun { log.Info("Dry-Run enabled. Program will not make any changes to the target repository.") } - // This error looks like some leftover. - if err != nil { - log.WithError(err).Fatal("Failed to create signer instance") - } - if err := SyncImages(ctx, &cfg, imagesFile, authCfg); err != nil { + if err := SyncImages(ctx, &cfg, imagesFile, auth); err != nil { log.WithError(err).Fatal("Failed to sync images") - } else { - log.Info("All images synced successfully") } + log.Info("All images synced successfully") }, } diff --git a/cmd/image-syncer/main_test.go b/cmd/image-syncer/main_test.go new file mode 100644 index 000000000000..a03d84668d22 --- /dev/null +++ b/cmd/image-syncer/main_test.go @@ -0,0 +1,55 @@ +package main + +import ( + "github.com/google/go-containerregistry/pkg/authn" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("newAuthenticator", func() { + var cfg Config + + BeforeEach(func() { + cfg = Config{} + }) + + Context("when TargetKeyFile is provided", func() { + It("should create a basic authenticator", func() { + cfg.TargetKeyFile = "./test-fixtures/keyfile.json" + auth, err := cfg.newAuthenticator() + Expect(err).NotTo(HaveOccurred()) + Expect(auth).To(Equal(&authn.Basic{ + Username: "_json_key", + Password: "test_content", + })) + }) + + It("should return an error if the key file cannot be read", func() { + cfg.TargetKeyFile = "./test-fixtures/invalid_keyfile.json" + auth, err := cfg.newAuthenticator() + Expect(err).To(HaveOccurred()) + Expect(auth).To(BeNil()) + }) + }) + + Context("when AccessToken is provided", func() { + It("should create a bearer authenticator", func() { + cfg.AccessToken = "valid-access-token" + auth, err := cfg.newAuthenticator() + Expect(err).NotTo(HaveOccurred()) + Expect(auth).To(Equal(&authn.Bearer{ + Token: "valid-access-token", + })) + }) + }) + + Context("when neither TargetKeyFile nor AccessToken is provided", func() { + It("should return an error", func() { + cfg.TargetKeyFile = "" + cfg.AccessToken = "" + auth, err := cfg.newAuthenticator() + Expect(err).To(HaveOccurred()) + Expect(auth).To(BeNil()) + }) + }) +}) diff --git a/cmd/image-syncer/test-fixtures/keyfile.json b/cmd/image-syncer/test-fixtures/keyfile.json new file mode 100644 index 000000000000..f650831fc641 --- /dev/null +++ b/cmd/image-syncer/test-fixtures/keyfile.json @@ -0,0 +1 @@ +test_content \ No newline at end of file diff --git a/go.mod b/go.mod index 944637a9653b..c90bbf50e51d 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( github.com/jamiealquiza/envy v1.1.0 github.com/jinzhu/copier v0.4.0 github.com/microsoft/azure-devops-go-api/azuredevops/v7 v7.1.0 - github.com/onsi/ginkgo/v2 v2.20.2 + github.com/onsi/ginkgo/v2 v2.20.1 github.com/onsi/gomega v1.34.2 github.com/pkg/errors v0.9.1 github.com/sirupsen/logrus v1.9.3 diff --git a/go.sum b/go.sum index ae6a136f42cf..a06f4b264162 100644 --- a/go.sum +++ b/go.sum @@ -548,8 +548,8 @@ github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+ github.com/onsi/ginkgo v0.0.0-20170829012221-11459a886d9c/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.11.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= -github.com/onsi/ginkgo/v2 v2.20.2 h1:7NVCeyIWROIAheY21RLS+3j2bb52W0W82tkberYytp4= -github.com/onsi/ginkgo/v2 v2.20.2/go.mod h1:K9gyxPIlb+aIvnZ8bd9Ak+YP18w3APlR+5coaZoE2ag= +github.com/onsi/ginkgo/v2 v2.20.1 h1:YlVIbqct+ZmnEph770q9Q7NVAz4wwIiVNahee6JyUzo= +github.com/onsi/ginkgo/v2 v2.20.1/go.mod h1:lG9ey2Z29hR41WMVthyJBGUBcBhGOtoPF2VFMvBXFCI= github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.34.2 h1:pNCwDkzrsv7MS9kpaQvVb1aVLahQXyJ/Tv5oAZMI3i8= From fc9285841aedc872621a87dbc54f63cfabb1b4b7 Mon Sep 17 00:00:00 2001 From: dekiel Date: Fri, 11 Oct 2024 08:33:31 +0200 Subject: [PATCH 3/6] formating --- cmd/image-syncer/main.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/image-syncer/main.go b/cmd/image-syncer/main.go index a559c3bfc2b5..21061ac1816b 100644 --- a/cmd/image-syncer/main.go +++ b/cmd/image-syncer/main.go @@ -230,20 +230,25 @@ func (cfg *Config) newAuthenticator() (authn.Authenticator, error) { if cfg.TargetKeyFile != "" { log.WithField("targetKeyFile", cfg.TargetKeyFile).Debug("target key file path provided, reading the file") + authCfg, err = os.ReadFile(cfg.TargetKeyFile) if err != nil { return nil, fmt.Errorf("could not open target auth key JSON file, error: %w", err) } log.Debug("target key file read successfully, creating basic authenticator") + auth = &authn.Basic{Username: "_json_key", Password: string(authCfg)} log.WithField("username", "_json_key").Debug("basic authenticator created successfully") + return auth, nil } if cfg.AccessToken != "" { log.Debug("access token provided, creating bearer authenticator") auth = &authn.Bearer{Token: cfg.AccessToken} + return auth, nil } + return nil, fmt.Errorf("no target auth key file or access token provided") } From ef22c1a31dec886cec2416d97e32ffee7eb1956a Mon Sep 17 00:00:00 2001 From: dekiel Date: Fri, 11 Oct 2024 08:40:52 +0200 Subject: [PATCH 4/6] Log finished important steps --- cmd/image-syncer/main.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/image-syncer/main.go b/cmd/image-syncer/main.go index 21061ac1816b..78abd858e6b8 100644 --- a/cmd/image-syncer/main.go +++ b/cmd/image-syncer/main.go @@ -275,12 +275,13 @@ func main() { if err != nil { log.WithError(err).Fatal("Could not parse images file") } + log.Info("Parsed images file") - log.Info("Creating authenticator") auth, err := cfg.newAuthenticator() if err != nil { log.WithError(err).Fatal("Failed to create authenticator") } + log.Info("Created authenticator for target repository") if cfg.DryRun { log.Info("Dry-Run enabled. Program will not make any changes to the target repository.") From 915eb478e9101705a44a65b54bde1ad9dd349872 Mon Sep 17 00:00:00 2001 From: dekiel Date: Fri, 11 Oct 2024 15:38:14 +0200 Subject: [PATCH 5/6] Add suite test file to have only one entry point in to tests. --- cmd/image-syncer/image_syncer_suite_test.go | 13 ------------- cmd/image-syncer/util_test.go | 6 ------ 2 files changed, 19 deletions(-) delete mode 100644 cmd/image-syncer/image_syncer_suite_test.go diff --git a/cmd/image-syncer/image_syncer_suite_test.go b/cmd/image-syncer/image_syncer_suite_test.go deleted file mode 100644 index 09f386b8a85c..000000000000 --- a/cmd/image-syncer/image_syncer_suite_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package main - -import ( - "testing" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -func TestImageSyncer(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "ImageSyncer Suite") -} diff --git a/cmd/image-syncer/util_test.go b/cmd/image-syncer/util_test.go index ee28a0c1cda4..fc8e68e9a738 100644 --- a/cmd/image-syncer/util_test.go +++ b/cmd/image-syncer/util_test.go @@ -2,17 +2,11 @@ package main import ( "os" - "testing" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) -func TestMainSuite(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "Main Suite") -} - var _ = Describe("getTarget", func() { tests := []struct { source string From 3359ead29e8cbd903c4c8d285de825032e925829 Mon Sep 17 00:00:00 2001 From: dekiel Date: Fri, 11 Oct 2024 15:40:36 +0200 Subject: [PATCH 6/6] Added missing suite test file. --- cmd/image-syncer/image_syncer_suite_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 cmd/image-syncer/image_syncer_suite_test.go diff --git a/cmd/image-syncer/image_syncer_suite_test.go b/cmd/image-syncer/image_syncer_suite_test.go new file mode 100644 index 000000000000..09f386b8a85c --- /dev/null +++ b/cmd/image-syncer/image_syncer_suite_test.go @@ -0,0 +1,13 @@ +package main + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestImageSyncer(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "ImageSyncer Suite") +}