From 4f290cc2c646c732c98f66d8254eb1a51fbea342 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 7 Sep 2023 07:23:23 +0000 Subject: [PATCH 01/10] fix: allow HTTPS for localhost Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote.go | 17 ++++++++++++++--- cmd/oras/internal/option/remote_test.go | 4 +++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index f56b90bf5..09cbcb620 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -43,7 +43,7 @@ import ( // Remote options struct. type Remote struct { CACertFilePath string - PlainHTTP bool + GetPlainHTTP func() *bool Insecure bool Configs []string Username string @@ -98,7 +98,15 @@ func (opts *Remote) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description fs.StringVarP(&opts.Username, flagPrefix+"username", shortUser, "", notePrefix+"registry username") fs.StringVarP(&opts.Password, flagPrefix+"password", shortPassword, "", notePrefix+"registry password or identity token") fs.BoolVarP(&opts.Insecure, flagPrefix+"insecure", "", false, "allow connections to "+notePrefix+"SSL registry without certs") - fs.BoolVarP(&opts.PlainHTTP, flagPrefix+"plain-http", "", false, "allow insecure connections to "+notePrefix+"registry without SSL check") + plainHTTPFlagName := flagPrefix + "plain-http" + var plainHTTP bool + fs.BoolVarP(&plainHTTP, plainHTTPFlagName, "", false, "allow insecure connections to "+notePrefix+"registry without SSL check") + opts.GetPlainHTTP = func() *bool { + if !fs.Changed(plainHTTPFlagName) { + return nil + } + return &plainHTTP + } fs.StringVarP(&opts.CACertFilePath, flagPrefix+"ca-file", "", "", "server certificate authority file for the remote "+notePrefix+"registry") fs.StringArrayVarP(&opts.resolveFlag, flagPrefix+"resolve", "", nil, "customized DNS for "+notePrefix+"registry, formatted in `host:port:address[:address_port]`") fs.StringArrayVarP(&opts.Configs, flagPrefix+"registry-config", "", nil, "`path` of the authentication file for "+notePrefix+"registry") @@ -305,9 +313,12 @@ func (opts *Remote) NewRepository(reference string, common Common, logger logrus // isPlainHttp returns the plain http flag for a given registry. func (opts *Remote) isPlainHttp(registry string) bool { + if plainHTTP := opts.GetPlainHTTP(); plainHTTP != nil { + return *plainHTTP + } host, _, _ := net.SplitHostPort(registry) if host == "localhost" || registry == "localhost" { return true } - return opts.PlainHTTP + return false } diff --git a/cmd/oras/internal/option/remote_test.go b/cmd/oras/internal/option/remote_test.go index 9c60d939b..4b8a0668d 100644 --- a/cmd/oras/internal/option/remote_test.go +++ b/cmd/oras/internal/option/remote_test.go @@ -277,7 +277,9 @@ func TestRemote_NewRepository_Retry(t *testing.T) { } func TestRemote_isPlainHttp_localhost(t *testing.T) { - opts := Remote{PlainHTTP: false} + opts := Remote{GetPlainHTTP: func() *bool { + return new(bool) + }} got := opts.isPlainHttp("localhost") if got != true { t.Fatalf("tls should be disabled when domain is localhost") From 13fac86d0cf9d21e60dfe77cd001823ca8319c11 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 7 Sep 2023 07:24:57 +0000 Subject: [PATCH 02/10] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index 09cbcb620..90fd45397 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -99,8 +99,8 @@ func (opts *Remote) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description fs.StringVarP(&opts.Password, flagPrefix+"password", shortPassword, "", notePrefix+"registry password or identity token") fs.BoolVarP(&opts.Insecure, flagPrefix+"insecure", "", false, "allow connections to "+notePrefix+"SSL registry without certs") plainHTTPFlagName := flagPrefix + "plain-http" - var plainHTTP bool - fs.BoolVarP(&plainHTTP, plainHTTPFlagName, "", false, "allow insecure connections to "+notePrefix+"registry without SSL check") + plainHTTP := false + fs.BoolVar(&plainHTTP, plainHTTPFlagName, plainHTTP, "allow insecure connections to "+notePrefix+"registry without SSL check") opts.GetPlainHTTP = func() *bool { if !fs.Changed(plainHTTPFlagName) { return nil From 2db70a849fa268fc3377ccb5944c2e32dd8c227b Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 7 Sep 2023 07:25:52 +0000 Subject: [PATCH 03/10] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote.go | 6 +++--- cmd/oras/internal/option/remote_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index 90fd45397..91349ce2a 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -43,7 +43,6 @@ import ( // Remote options struct. type Remote struct { CACertFilePath string - GetPlainHTTP func() *bool Insecure bool Configs []string Username string @@ -56,6 +55,7 @@ type Remote struct { headerFlags []string headers http.Header warned map[string]*sync.Map + getPlainHTTP func() *bool } // EnableDistributionSpecFlag set distribution specification flag as applicable. @@ -101,7 +101,7 @@ func (opts *Remote) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description plainHTTPFlagName := flagPrefix + "plain-http" plainHTTP := false fs.BoolVar(&plainHTTP, plainHTTPFlagName, plainHTTP, "allow insecure connections to "+notePrefix+"registry without SSL check") - opts.GetPlainHTTP = func() *bool { + opts.getPlainHTTP = func() *bool { if !fs.Changed(plainHTTPFlagName) { return nil } @@ -313,7 +313,7 @@ func (opts *Remote) NewRepository(reference string, common Common, logger logrus // isPlainHttp returns the plain http flag for a given registry. func (opts *Remote) isPlainHttp(registry string) bool { - if plainHTTP := opts.GetPlainHTTP(); plainHTTP != nil { + if plainHTTP := opts.getPlainHTTP(); plainHTTP != nil { return *plainHTTP } host, _, _ := net.SplitHostPort(registry) diff --git a/cmd/oras/internal/option/remote_test.go b/cmd/oras/internal/option/remote_test.go index 4b8a0668d..d4e3c7358 100644 --- a/cmd/oras/internal/option/remote_test.go +++ b/cmd/oras/internal/option/remote_test.go @@ -277,7 +277,7 @@ func TestRemote_NewRepository_Retry(t *testing.T) { } func TestRemote_isPlainHttp_localhost(t *testing.T) { - opts := Remote{GetPlainHTTP: func() *bool { + opts := Remote{getPlainHTTP: func() *bool { return new(bool) }} got := opts.isPlainHttp("localhost") From b42d3683eb7f4ed7f5bdc6c7b91c08eaf66d0830 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Thu, 7 Sep 2023 07:39:56 +0000 Subject: [PATCH 04/10] add tests Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote_test.go | 50 +++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/cmd/oras/internal/option/remote_test.go b/cmd/oras/internal/option/remote_test.go index d4e3c7358..b0079ce55 100644 --- a/cmd/oras/internal/option/remote_test.go +++ b/cmd/oras/internal/option/remote_test.go @@ -162,6 +162,17 @@ func TestRemote_authClient_resolve(t *testing.T) { } } +func plainHTTPEnabled() *bool { + t := true + return &t +} +func HTTPSEnabled() *bool { + return new(bool) +} +func plainHTTPNotSpecified() *bool { + return nil +} + func TestRemote_NewRegistry(t *testing.T) { caPath := filepath.Join(t.TempDir(), "oras-test.pem") if err := os.WriteFile(caPath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: ts.Certificate().Raw}), 0644); err != nil { @@ -174,6 +185,7 @@ func TestRemote_NewRegistry(t *testing.T) { }{ Remote{ CACertFilePath: caPath, + getPlainHTTP: plainHTTPNotSpecified, }, Common{}, } @@ -201,6 +213,7 @@ func TestRemote_NewRepository(t *testing.T) { }{ Remote{ CACertFilePath: caPath, + getPlainHTTP: plainHTTPNotSpecified, }, Common{}, } @@ -248,6 +261,7 @@ func TestRemote_NewRepository_Retry(t *testing.T) { }{ Remote{ CACertFilePath: caPath, + getPlainHTTP: plainHTTPNotSpecified, }, Common{}, } @@ -276,10 +290,8 @@ func TestRemote_NewRepository_Retry(t *testing.T) { } } -func TestRemote_isPlainHttp_localhost(t *testing.T) { - opts := Remote{getPlainHTTP: func() *bool { - return new(bool) - }} +func TestRemote_default_localhost(t *testing.T) { + opts := Remote{getPlainHTTP: plainHTTPNotSpecified} got := opts.isPlainHttp("localhost") if got != true { t.Fatalf("tls should be disabled when domain is localhost") @@ -293,6 +305,36 @@ func TestRemote_isPlainHttp_localhost(t *testing.T) { } } +func TestRemote_isPlainHTTP_localhost(t *testing.T) { + opts := Remote{getPlainHTTP: plainHTTPEnabled} + isplainHTTP := opts.isPlainHttp("localhost") + if isplainHTTP != true { + t.Fatalf("tls should be disabled when domain is localhost and --plain-http is used") + + } + + isplainHTTP = opts.isPlainHttp("localhost:9090") + if isplainHTTP != true { + t.Fatalf("tls should be disabled when domain is localhost and --plain-http is used") + + } +} + +func TestRemote_isHTTPS_localhost(t *testing.T) { + opts := Remote{getPlainHTTP: HTTPSEnabled} + got := opts.isPlainHttp("localhost") + if got != false { + t.Fatalf("tls should be enabled when domain is localhost and --plain-http=false is used") + + } + + got = opts.isPlainHttp("localhost:9090") + if got != false { + t.Fatalf("tls should be enabled when domain is localhost and --plain-http=false is used") + + } +} + func TestRemote_parseResolve_err(t *testing.T) { tests := []struct { name string From fc21017f87a3c395e018b5d7ded99c400d1bbc1f Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 11 Sep 2023 02:22:51 +0000 Subject: [PATCH 05/10] resolve comments Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote.go | 19 +++++++++---------- cmd/oras/internal/option/remote_test.go | 25 ++++++++++++------------- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index 91349ce2a..de95eda70 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -55,7 +55,7 @@ type Remote struct { headerFlags []string headers http.Header warned map[string]*sync.Map - getPlainHTTP func() *bool + plainHTTP func() (plainHTTP bool, fromFlag bool) } // EnableDistributionSpecFlag set distribution specification flag as applicable. @@ -101,11 +101,11 @@ func (opts *Remote) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description plainHTTPFlagName := flagPrefix + "plain-http" plainHTTP := false fs.BoolVar(&plainHTTP, plainHTTPFlagName, plainHTTP, "allow insecure connections to "+notePrefix+"registry without SSL check") - opts.getPlainHTTP = func() *bool { + opts.plainHTTP = func() (bool, bool) { if !fs.Changed(plainHTTPFlagName) { - return nil + return plainHTTP, false } - return &plainHTTP + return plainHTTP, true } fs.StringVarP(&opts.CACertFilePath, flagPrefix+"ca-file", "", "", "server certificate authority file for the remote "+notePrefix+"registry") fs.StringArrayVarP(&opts.resolveFlag, flagPrefix+"resolve", "", nil, "customized DNS for "+notePrefix+"registry, formatted in `host:port:address[:address_port]`") @@ -313,12 +313,11 @@ func (opts *Remote) NewRepository(reference string, common Common, logger logrus // isPlainHttp returns the plain http flag for a given registry. func (opts *Remote) isPlainHttp(registry string) bool { - if plainHTTP := opts.getPlainHTTP(); plainHTTP != nil { - return *plainHTTP + if plainHTTP, specified := opts.plainHTTP(); specified { + return plainHTTP } + + // not specified, defaults to plain http for localhost host, _, _ := net.SplitHostPort(registry) - if host == "localhost" || registry == "localhost" { - return true - } - return false + return host == "localhost" || registry == "localhost" } diff --git a/cmd/oras/internal/option/remote_test.go b/cmd/oras/internal/option/remote_test.go index b0079ce55..a55a4edc3 100644 --- a/cmd/oras/internal/option/remote_test.go +++ b/cmd/oras/internal/option/remote_test.go @@ -162,15 +162,14 @@ func TestRemote_authClient_resolve(t *testing.T) { } } -func plainHTTPEnabled() *bool { - t := true - return &t +func plainHTTPEnabled() (plainHTTP bool, fromFlag bool) { + return true, true } -func HTTPSEnabled() *bool { - return new(bool) +func HTTPSEnabled() (plainHTTP bool, fromFlag bool) { + return false, true } -func plainHTTPNotSpecified() *bool { - return nil +func plainHTTPNotSpecified() (plainHTTP bool, fromFlag bool) { + return false, false } func TestRemote_NewRegistry(t *testing.T) { @@ -185,7 +184,7 @@ func TestRemote_NewRegistry(t *testing.T) { }{ Remote{ CACertFilePath: caPath, - getPlainHTTP: plainHTTPNotSpecified, + plainHTTP: plainHTTPNotSpecified, }, Common{}, } @@ -213,7 +212,7 @@ func TestRemote_NewRepository(t *testing.T) { }{ Remote{ CACertFilePath: caPath, - getPlainHTTP: plainHTTPNotSpecified, + plainHTTP: plainHTTPNotSpecified, }, Common{}, } @@ -261,7 +260,7 @@ func TestRemote_NewRepository_Retry(t *testing.T) { }{ Remote{ CACertFilePath: caPath, - getPlainHTTP: plainHTTPNotSpecified, + plainHTTP: plainHTTPNotSpecified, }, Common{}, } @@ -291,7 +290,7 @@ func TestRemote_NewRepository_Retry(t *testing.T) { } func TestRemote_default_localhost(t *testing.T) { - opts := Remote{getPlainHTTP: plainHTTPNotSpecified} + opts := Remote{plainHTTP: plainHTTPNotSpecified} got := opts.isPlainHttp("localhost") if got != true { t.Fatalf("tls should be disabled when domain is localhost") @@ -306,7 +305,7 @@ func TestRemote_default_localhost(t *testing.T) { } func TestRemote_isPlainHTTP_localhost(t *testing.T) { - opts := Remote{getPlainHTTP: plainHTTPEnabled} + opts := Remote{plainHTTP: plainHTTPEnabled} isplainHTTP := opts.isPlainHttp("localhost") if isplainHTTP != true { t.Fatalf("tls should be disabled when domain is localhost and --plain-http is used") @@ -321,7 +320,7 @@ func TestRemote_isPlainHTTP_localhost(t *testing.T) { } func TestRemote_isHTTPS_localhost(t *testing.T) { - opts := Remote{getPlainHTTP: HTTPSEnabled} + opts := Remote{plainHTTP: HTTPSEnabled} got := opts.isPlainHttp("localhost") if got != false { t.Fatalf("tls should be enabled when domain is localhost and --plain-http=false is used") From 0f7187a005d0fe818f77ca4f145da926979cfda0 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 11 Sep 2023 02:25:33 +0000 Subject: [PATCH 06/10] apply flag via Bool Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index de95eda70..2b6c3d1cb 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -99,8 +99,7 @@ func (opts *Remote) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description fs.StringVarP(&opts.Password, flagPrefix+"password", shortPassword, "", notePrefix+"registry password or identity token") fs.BoolVarP(&opts.Insecure, flagPrefix+"insecure", "", false, "allow connections to "+notePrefix+"SSL registry without certs") plainHTTPFlagName := flagPrefix + "plain-http" - plainHTTP := false - fs.BoolVar(&plainHTTP, plainHTTPFlagName, plainHTTP, "allow insecure connections to "+notePrefix+"registry without SSL check") + plainHTTP := *fs.Bool(plainHTTPFlagName, false, "allow insecure connections to "+notePrefix+"registry without SSL check") opts.plainHTTP = func() (bool, bool) { if !fs.Changed(plainHTTPFlagName) { return plainHTTP, false From 477a7bf51e4166aafcb2626cde76f2207bbf99da Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 11 Sep 2023 02:33:50 +0000 Subject: [PATCH 07/10] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index 2b6c3d1cb..40ea6cc1e 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -101,10 +101,7 @@ func (opts *Remote) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description plainHTTPFlagName := flagPrefix + "plain-http" plainHTTP := *fs.Bool(plainHTTPFlagName, false, "allow insecure connections to "+notePrefix+"registry without SSL check") opts.plainHTTP = func() (bool, bool) { - if !fs.Changed(plainHTTPFlagName) { - return plainHTTP, false - } - return plainHTTP, true + return plainHTTP, fs.Changed(plainHTTPFlagName) } fs.StringVarP(&opts.CACertFilePath, flagPrefix+"ca-file", "", "", "server certificate authority file for the remote "+notePrefix+"registry") fs.StringArrayVarP(&opts.resolveFlag, flagPrefix+"resolve", "", nil, "customized DNS for "+notePrefix+"registry, formatted in `host:port:address[:address_port]`") From 20566d191ffdc7576c22222c7c19fcdf37bff5ff Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 11 Sep 2023 02:53:51 +0000 Subject: [PATCH 08/10] rename Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index 40ea6cc1e..f691dda90 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -55,7 +55,7 @@ type Remote struct { headerFlags []string headers http.Header warned map[string]*sync.Map - plainHTTP func() (plainHTTP bool, fromFlag bool) + plainHTTP func() (plainHTTP bool, enforced bool) } // EnableDistributionSpecFlag set distribution specification flag as applicable. @@ -309,7 +309,7 @@ func (opts *Remote) NewRepository(reference string, common Common, logger logrus // isPlainHttp returns the plain http flag for a given registry. func (opts *Remote) isPlainHttp(registry string) bool { - if plainHTTP, specified := opts.plainHTTP(); specified { + if plainHTTP, enforced := opts.plainHTTP(); enforced { return plainHTTP } From 706c468c9106bb992b0c29f91612efc3513a9b0b Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 11 Sep 2023 03:08:57 +0000 Subject: [PATCH 09/10] fix failed e2e Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index f691dda90..16dacee36 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -99,9 +99,9 @@ func (opts *Remote) ApplyFlagsWithPrefix(fs *pflag.FlagSet, prefix, description fs.StringVarP(&opts.Password, flagPrefix+"password", shortPassword, "", notePrefix+"registry password or identity token") fs.BoolVarP(&opts.Insecure, flagPrefix+"insecure", "", false, "allow connections to "+notePrefix+"SSL registry without certs") plainHTTPFlagName := flagPrefix + "plain-http" - plainHTTP := *fs.Bool(plainHTTPFlagName, false, "allow insecure connections to "+notePrefix+"registry without SSL check") + plainHTTP := fs.Bool(plainHTTPFlagName, false, "allow insecure connections to "+notePrefix+"registry without SSL check") opts.plainHTTP = func() (bool, bool) { - return plainHTTP, fs.Changed(plainHTTPFlagName) + return *plainHTTP, fs.Changed(plainHTTPFlagName) } fs.StringVarP(&opts.CACertFilePath, flagPrefix+"ca-file", "", "", "server certificate authority file for the remote "+notePrefix+"registry") fs.StringArrayVarP(&opts.resolveFlag, flagPrefix+"resolve", "", nil, "customized DNS for "+notePrefix+"registry, formatted in `host:port:address[:address_port]`") From dc3c2975641452dfb19cf6c3e21dda5f979e1ecc Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 11 Sep 2023 04:22:53 +0000 Subject: [PATCH 10/10] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/option/remote.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go index 16dacee36..c56282cc2 100644 --- a/cmd/oras/internal/option/remote.go +++ b/cmd/oras/internal/option/remote.go @@ -309,11 +309,14 @@ func (opts *Remote) NewRepository(reference string, common Common, logger logrus // isPlainHttp returns the plain http flag for a given registry. func (opts *Remote) isPlainHttp(registry string) bool { - if plainHTTP, enforced := opts.plainHTTP(); enforced { + plainHTTP, enforced := opts.plainHTTP() + if enforced { return plainHTTP } - - // not specified, defaults to plain http for localhost host, _, _ := net.SplitHostPort(registry) - return host == "localhost" || registry == "localhost" + if host == "localhost" || registry == "localhost" { + // not specified, defaults to plain http for localhost + return true + } + return plainHTTP }