Skip to content

Commit

Permalink
image-pruner: Reenable registry-url validation
Browse files Browse the repository at this point in the history 
Signed-off-by: Michal Minář <[email protected]>
  • Loading branch information
Michal Minář committed Jul 26, 2017
1 parent 158e25b commit 47551a8
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 11 deletions.
70 changes: 59 additions & 11 deletions pkg/cmd/admin/prune/images.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ import (
const PruneImagesRecommendedName = "images"

var noTokenError = errors.New("you must use a client config with a token")
var noRegistryURLPathAllowedError = fmt.Errorf("no path after <host>[:<port>] is allowed")
var noRegistryURLQueryAllowedError = fmt.Errorf("no query arguments are allowed after <host>[:<port>]")
var registryURLHostEmptyError = fmt.Errorf("no host name specified")

var (
imagesLongDesc = templates.LongDesc(`
Expand All @@ -59,9 +62,10 @@ var (
specified:
1. --force-insecure is given
2. user's config allows for insecure connection (the user logged in to the cluster with
--insecure-skip-tls-verify or allowed for insecure connection)
3. registry url is a private or link-local address`)
2. provided registry-url is prefixed with http://
3. registry url is a private or link-local address
4. user's config allows for insecure connection (the user logged in to the cluster with
--insecure-skip-tls-verify or allowed for insecure connection)`)

imagesExample = templates.Examples(`
# See, what the prune command would delete if only images more than an hour old and obsoleted
Expand All @@ -76,7 +80,13 @@ var (
%[1]s %[2]s --prune-over-size-limit
# To actually perform the prune operation, the confirm flag must be appended
%[1]s %[2]s --prune-over-size-limit --confirm`)
%[1]s %[2]s --prune-over-size-limit --confirm
# Force the insecure http protocol with the particular registry host name
%[1]s %[2]s --registry-url=http://registry.example.org --confirm
# Force a secure connection with a custom certificate authority to the particular registry host name
%[1]s %[2]s --registry-url=registry.example.org --certificate-authority=/path/to/custom/ca.crt --confirm`)
)

var (
Expand Down Expand Up @@ -134,7 +144,7 @@ func NewCmdPruneImages(f *clientcmd.Factory, parentName, name string, out io.Wri
cmd.Flags().IntVar(opts.KeepTagRevisions, "keep-tag-revisions", *opts.KeepTagRevisions, "Specify the number of image revisions for a tag in an image stream that will be preserved.")
cmd.Flags().BoolVar(opts.PruneOverSizeLimit, "prune-over-size-limit", *opts.PruneOverSizeLimit, "Specify if images which are exceeding LimitRanges (see 'openshift.io/Image'), specified in the same namespace, should be considered for pruning. This flag cannot be combined with --keep-younger-than nor --keep-tag-revisions.")
cmd.Flags().StringVar(&opts.CABundle, "certificate-authority", opts.CABundle, "The path to a certificate authority bundle to use when communicating with the managed Docker registries. Defaults to the certificate authority data from the current user's config file. It cannot be used together with --force-insecure.")
cmd.Flags().StringVar(&opts.RegistryUrlOverride, "registry-url", opts.RegistryUrlOverride, "The address to use when contacting the registry, instead of using the default value. This is useful if you can't resolve or reach the registry (e.g.; the default is a cluster-internal URL) but you do have an alternative route that works.")
cmd.Flags().StringVar(&opts.RegistryUrlOverride, "registry-url", opts.RegistryUrlOverride, "The address to use when contacting the registry, instead of using the default value. This is useful if you can't resolve or reach the registry (e.g.; the default is a cluster-internal URL) but you do have an alternative route that works. Particular transport protocol can be enforced using '<scheme>://' prefix.")
cmd.Flags().BoolVar(&opts.ForceInsecure, "force-insecure", opts.ForceInsecure, "If true, allow an insecure connection to the docker registry that is hosted via HTTP or has an invalid HTTPS certificate. Whenever possible, use --certificate-authority instead of this dangerous option.")

return cmd
Expand Down Expand Up @@ -195,14 +205,15 @@ func (o PruneImagesOptions) Validate() error {
if o.KeepTagRevisions != nil && *o.KeepTagRevisions < 0 {
return fmt.Errorf("--keep-tag-revisions must be greater than or equal to 0")
}
// golang validation tighten and our code actually expects this to be scheme-less
// TODO figure out how to validate
// if _, err := url.Parse(o.RegistryUrlOverride); err != nil {
// return fmt.Errorf("invalid --registry-url flag: %v", err)
// }
if err := validateRegistryURL(o.RegistryUrlOverride); err != nil {
return fmt.Errorf("invalid --registry-url flag: %v", err)
}
if o.ForceInsecure && len(o.CABundle) > 0 {
return fmt.Errorf("--certificate-authority cannot be specified with --force-insecure")
}
if len(o.CABundle) > 0 && strings.HasPrefix(o.RegistryUrlOverride, "http://") {
return fmt.Errorf("--cerificate-authority cannot be specified for insecure http protocol")
}
return nil
}

Expand Down Expand Up @@ -278,7 +289,8 @@ func (o PruneImagesOptions) Run() error {

insecure := o.ForceInsecure
if !insecure && len(o.CABundle) == 0 {
insecure = o.ClientConfig.TLSClientConfig.Insecure || netutils.IsPrivateAddress(registryHost)
insecure = o.ClientConfig.TLSClientConfig.Insecure || netutils.IsPrivateAddress(registryHost) ||
strings.HasPrefix(registryHost, "http://")
}

registryClient, err = getRegistryClient(o.ClientConfig, o.CABundle, insecure)
Expand Down Expand Up @@ -594,3 +606,39 @@ func getRegistryClient(clientConfig *restclient.Config, registryCABundle string,
Transport: wrappedTransport,
}, nil
}

func validateRegistryURL(registryURL string) error {
var (
u *url.URL
err error
parts = strings.SplitN(registryURL, "://", 2)
)

switch len(parts) {
case 2:
u, err = url.Parse(registryURL)
if err != nil {
return err
}
switch u.Scheme {
case "http", "https":
default:
return fmt.Errorf("unsupported scheme: %s", u.Scheme)
}
case 1:
u, err = url.Parse("https://" + registryURL)
if err != nil {
return err
}
}
if len(u.Path) > 0 && u.Path != "/" {
return noRegistryURLPathAllowedError
}
if len(u.RawQuery) > 0 {
return noRegistryURLQueryAllowedError
}
if len(u.Host) == 0 {
return registryURLHostEmptyError
}
return nil
}
82 changes: 82 additions & 0 deletions pkg/cmd/admin/prune/images_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,85 @@ func TestImagePruneNamespaced(t *testing.T) {
}
}
}

func TestValidateRegistryURL(t *testing.T) {
for _, tc := range []struct {
input string
expectedError bool
expectedErrorString string
}{
{input: "172.30.30.30:5000"},
{input: ":5000"},
{input: "[fd12:3456:789a:1::1]:80/"},
{input: "[fd12:3456:789a:1::1]:80"},
{input: "http://172.30.30.30:5000"},
{input: "http://[fd12:3456:789a:1::1]:5000/"},
{input: "http://[fd12:3456:789a:1::1]:5000"},
{input: "http://registry.org:5000"},
{input: "https://172.30.30.30:5000"},
{input: "https://:80/"},
{input: "https://[fd12:3456:789a:1::1]/"},
{input: "https://[fd12:3456:789a:1::1]"},
{input: "https://[fd12:3456:789a:1::1]:5000/"},
{input: "https://[fd12:3456:789a:1::1]:5000"},
{input: "https://registry.org/"},
{input: "https://registry.org"},
{input: "localhost/"},
{input: "localhost"},
{input: "localhost:80"},
{input: "registry.org/"},
{input: "registry.org"},
{input: "registry.org:5000"},

{
input: "httpss://registry.org",
expectedErrorString: "unsupported scheme: httpss",
},
{
input: "ftp://registry.org",
expectedErrorString: "unsupported scheme: ftp",
},
{
input: "http://registry.org://",
expectedErrorString: noRegistryURLPathAllowedError.Error(),
},
{
input: "http://registry.org/path",
expectedErrorString: noRegistryURLPathAllowedError.Error(),
},
{
input: "[fd12:3456:789a:1::1",
expectedError: true,
},
{
input: "bad url",
expectedError: true,
},
{
input: "/registry.org",
expectedErrorString: noRegistryURLPathAllowedError.Error(),
},
{
input: "https:///",
expectedErrorString: registryURLHostEmptyError.Error(),
},
{
input: "http://registry.org?parm=arg",
expectedErrorString: noRegistryURLQueryAllowedError.Error(),
},
} {

err := validateRegistryURL(tc.input)
if err != nil {
if len(tc.expectedErrorString) > 0 && err.Error() != tc.expectedErrorString {
t.Errorf("[%s] unexpected error string: %q != %q", tc.input, err.Error(), tc.expectedErrorString)
} else if len(tc.expectedErrorString) == 0 && !tc.expectedError {
t.Errorf("[%s] unexpected error: %q", tc.input, err.Error())
}
} else if len(tc.expectedErrorString) > 0 {
t.Errorf("[%s] got non-error while expecting %q", tc.input, tc.expectedErrorString)
} else if tc.expectedError {
t.Errorf("[%s] got unexpected non-error", tc.input)
}
}
}

0 comments on commit 47551a8

Please sign in to comment.