-
Notifications
You must be signed in to change notification settings - Fork 950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ctrd/daemon: support PlainHttp pull/push image #2810
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,18 @@ | ||
package ctrd | ||
|
||
import "fmt" | ||
import ( | ||
"fmt" | ||
"net" | ||
"strconv" | ||
"strings" | ||
) | ||
|
||
type clientOpts struct { | ||
rpcAddr string | ||
grpcClientPoolCapacity int | ||
maxStreamsClient int | ||
defaultns string | ||
insecureRegistries []string | ||
} | ||
|
||
// ClientOpt allows caller to set options for containerd client. | ||
|
@@ -59,3 +65,43 @@ func WithDefaultNamespace(ns string) ClientOpt { | |
return nil | ||
} | ||
} | ||
|
||
// WithInsecureRegistries sets the insecure registries to allow http request | ||
// and skip secure verify. | ||
func WithInsecureRegistries(endpoints []string) ClientOpt { | ||
return func(c *clientOpts) error { | ||
registries := make([]string, 0, len(endpoints)) | ||
|
||
for _, r := range endpoints { | ||
if strings.Contains(strings.ToLower(r), "://") { | ||
return fmt.Errorf("insecure registry %s should not contain any '://'", r) | ||
} | ||
|
||
if err := validateHostPort(r); err != nil { | ||
return err | ||
} | ||
registries = append(registries, r) | ||
} | ||
c.insecureRegistries = registries | ||
return nil | ||
} | ||
} | ||
|
||
func validateHostPort(s string) error { | ||
_, port, err := net.SplitHostPort(s) | ||
if err != nil { | ||
port = "" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not return err? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe it is domain name without any port, such as 80 |
||
} | ||
|
||
if port != "" { | ||
v, err := strconv.Atoi(port) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if v < 0 || v > 65535 { | ||
return fmt.Errorf("invalid port %q", port) | ||
} | ||
} | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package ctrd | ||
|
||
import "testing" | ||
|
||
func TestWithInsecureRegistries(t *testing.T) { | ||
testCases := []struct { | ||
endpoints []string | ||
hasError bool | ||
}{ | ||
{ | ||
endpoints: []string{"localhost:5000"}, | ||
hasError: false, | ||
}, | ||
{ | ||
endpoints: []string{"localhost:5000/v1"}, | ||
hasError: true, | ||
}, | ||
{ | ||
endpoints: []string{"myregistry.com"}, | ||
hasError: false, | ||
}, | ||
{ | ||
endpoints: []string{"myregistry.com:5000"}, | ||
hasError: false, | ||
}, | ||
{ | ||
endpoints: []string{"myregistry.com:5000/v1"}, | ||
hasError: true, | ||
}, | ||
{ | ||
endpoints: []string{"http://myregistry.com:5000"}, | ||
hasError: true, | ||
}, | ||
{ | ||
endpoints: []string{"dummy://myregistry.com:5000"}, | ||
hasError: true, | ||
}, | ||
{ | ||
endpoints: []string{"myregistry.com:65536"}, | ||
hasError: true, | ||
}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
err := WithInsecureRegistries(tc.endpoints)(&clientOpts{}) | ||
if (err != nil) != tc.hasError { | ||
t.Fatalf("expected hasError = %v, but got error = %v", tc.hasError, err) | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import ( | |
"crypto/tls" | ||
"net" | ||
"net/http" | ||
"net/url" | ||
"syscall" | ||
"time" | ||
|
||
|
@@ -17,6 +18,7 @@ import ( | |
"github.com/containerd/containerd/runtime/linux/runctypes" | ||
"github.com/opencontainers/runtime-spec/specs-go" | ||
"github.com/pkg/errors" | ||
"github.com/sirupsen/logrus" | ||
) | ||
|
||
func withExitShimV1CheckpointTaskOpts() containerd.CheckpointTaskOpts { | ||
|
@@ -28,32 +30,36 @@ func withExitShimV1CheckpointTaskOpts() containerd.CheckpointTaskOpts { | |
} | ||
} | ||
|
||
func resolver(authConfig *types.AuthConfig, resolverOpt docker.ResolverOptions) (remotes.Resolver, error) { | ||
// isInsecureDomain will return true if the domain of reference is in the | ||
// insecure registry. The insecure registry will accept HTTP or HTTPS with | ||
// certificates from unknown CAs. | ||
func (c *Client) isInsecureDomain(ref string) bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to add unit test cases for this function? |
||
u, err := url.Parse("dummy://" + ref) | ||
if err != nil { | ||
logrus.Warning("failed to parse reference(%s) into url: %v", ref, err) | ||
return false | ||
} | ||
|
||
for _, r := range c.insecureRegistries { | ||
if r == u.Host { | ||
return true | ||
} | ||
} | ||
return false | ||
} | ||
|
||
func (c *Client) getResolver(authConfig *types.AuthConfig, ref string, resolverOpt docker.ResolverOptions) (remotes.Resolver, error) { | ||
var ( | ||
// TODO | ||
username = "" | ||
secret = "" | ||
refresh = "" | ||
insecure = false | ||
insecure = c.isInsecureDomain(ref) | ||
) | ||
|
||
if authConfig != nil { | ||
username = authConfig.Username | ||
secret = authConfig.Password | ||
} | ||
|
||
// FIXME | ||
_ = refresh | ||
|
||
options := docker.ResolverOptions{ | ||
PlainHTTP: resolverOpt.PlainHTTP, | ||
Tracker: resolverOpt.Tracker, | ||
} | ||
options.Credentials = func(host string) (string, string, error) { | ||
// Only one host | ||
return username, secret, nil | ||
} | ||
|
||
tr := &http.Transport{ | ||
Proxy: proxyFromEnvironment, | ||
DialContext: (&net.Dialer{ | ||
|
@@ -70,10 +76,17 @@ func resolver(authConfig *types.AuthConfig, resolverOpt docker.ResolverOptions) | |
ExpectContinueTimeout: 5 * time.Second, | ||
} | ||
|
||
options.Client = &http.Client{ | ||
Transport: tr, | ||
options := docker.ResolverOptions{ | ||
Tracker: resolverOpt.Tracker, | ||
PlainHTTP: insecure, | ||
Credentials: func(host string) (string, string, error) { | ||
// Only one host | ||
return username, secret, nil | ||
}, | ||
Client: &http.Client{ | ||
Transport: tr, | ||
}, | ||
} | ||
|
||
return docker.NewResolver(options), nil | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -127,6 +127,10 @@ type Config struct { | |
|
||
// CgroupDriver sets cgroup driver for all containers | ||
CgroupDriver string `json:"cgroup-driver,omitempty"` | ||
|
||
// InsecureRegistries sets insecure registries to allow to pull | ||
// insecure registries. | ||
InsecureRegistries []string `json:"insecure-registries,omitempty"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add daemon test for this config? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is hard to add this case right now because we need to download the registry and push the image in there. I am not sure how to make it easier to add it into repo. |
||
} | ||
|
||
// GetCgroupDriver gets cgroup driver used in runc. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to add unit test cases for this function?