Skip to content

Commit

Permalink
Prefer secure connection during image pruning
Browse files Browse the repository at this point in the history
The default stays the same. When a CA bundle or a registry url is
specified, require secure connection with certificate verification.
Allow the user to force insecure connection using --force-insecure if he
has to.

Signed-off-by: Michal Minář <[email protected]>
  • Loading branch information
Michal Minář committed May 9, 2017
1 parent aa4f611 commit ad4a823
Showing 1 changed file with 45 additions and 17 deletions.
62 changes: 45 additions & 17 deletions pkg/cmd/admin/prune/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ var (
--confirm flag is needed for changes to be effective.
Only a user with a cluster role %s or higher who is logged-in will be able to actually
delete the images.`)
delete the images.
If the registry is secured with a certificate signed by a self-signed root certificate
authority other than the one used for the cluster and present in current user's config, you
may need to specify it using --certificate-authority flag.`)

imagesExample = templates.Examples(`
# See, what the prune command would delete if only images more than an hour old and obsoleted
Expand Down Expand Up @@ -80,6 +84,7 @@ type PruneImagesOptions struct {
CABundle string
RegistryUrlOverride string
Namespace string
ForceInsecure bool

OSClient client.Interface
KClient kclientset.Interface
Expand Down Expand Up @@ -117,8 +122,9 @@ func NewCmdPruneImages(f *clientcmd.Factory, parentName, name string, out io.Wri
cmd.Flags().DurationVar(opts.KeepYoungerThan, "keep-younger-than", *opts.KeepYoungerThan, "Specify the minimum age of an image for it to be considered a candidate for pruning.")
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.")
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().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. By default, insecure connection is allowed only in case where neither certificate-authority nor registry-url is specified. Whenever possible, use --certificate-authority instead of this dangerous option.")

return cmd
}
Expand Down Expand Up @@ -151,7 +157,8 @@ func (o *PruneImagesOptions) Complete(f *clientcmd.Factory, cmd *cobra.Command,
}
o.Out = out

osClient, kClient, registryClient, err := getClients(f, o.CABundle)
insecure := (len(o.RegistryUrlOverride) == 0 && len(o.CABundle) == 0) || o.ForceInsecure
osClient, kClient, registryClient, err := getClients(f, o.CABundle, insecure)
if err != nil {
return err
}
Expand All @@ -176,6 +183,9 @@ func (o PruneImagesOptions) Validate() error {
if _, err := url.Parse(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")
}
return nil
}

Expand Down Expand Up @@ -435,8 +445,10 @@ func (p *describingManifestDeleter) DeleteManifest(registryClient *http.Client,
return err
}

// getClients returns a Kube client, OpenShift client, and registry client.
func getClients(f *clientcmd.Factory, caBundle string) (*client.Client, kclientset.Interface, *http.Client, error) {
// getClients returns a Kube client, OpenShift client, and registry client. Note that
// registryCABundle and registryInsecure=true are mutually exclusive. If registryInsecure=true is
// specified, the ca bundle is ignored.
func getClients(f *clientcmd.Factory, registryCABundle string, registryInsecure bool) (*client.Client, kclientset.Interface, *http.Client, error) {
clientConfig, err := f.ClientConfig()
if err != nil {
return nil, nil, nil, err
Expand All @@ -461,8 +473,18 @@ func getClients(f *clientcmd.Factory, caBundle string) (*client.Client, kclients
return nil, nil, nil, err
}

cadata := []byte{}
registryCABundleIncluded := false
if len(registryCABundle) > 0 {
cadata, err = ioutil.ReadFile(registryCABundle)
if err != nil {
return nil, nil, nil, fmt.Errorf("failed to read registry ca bundle: %v", err)
}
}

// copy the config
registryClientConfig := *clientConfig
registryClientConfig.TLSClientConfig.Insecure = registryInsecure

// zero out everything we don't want to use
registryClientConfig.BearerToken = ""
Expand All @@ -471,8 +493,20 @@ func getClients(f *clientcmd.Factory, caBundle string) (*client.Client, kclients
registryClientConfig.KeyFile = ""
registryClientConfig.KeyData = []byte{}

// we have to set a username to something for the Docker login
// but it's not actually used
if registryInsecure {
// it's not allowed to specify insecure flag together with CAs
registryClientConfig.CAFile = ""
registryClientConfig.CAData = []byte{}

} else if len(cadata) > 0 && len(registryClientConfig.CAData) == 0 {
// If given, we want to append cabundle to the resulting tlsConfig.RootCAs. However, if we
// leave CAData unset, tlsConfig may not be created. We could append the caBundle to the
// CAData here directly if we were ok doing a binary magic, which is not the case.
registryClientConfig.CAData = cadata
registryCABundleIncluded = true
}

// we have to set a username to something for the Docker login but it's not actually used
registryClientConfig.Username = "unused"

// set the "password" to be the token
Expand All @@ -483,19 +517,13 @@ func getClients(f *clientcmd.Factory, caBundle string) (*client.Client, kclients
return nil, nil, nil, err
}

// if the user specified a CA on the command line, add it to the
// client config's CA roots
if tlsConfig != nil && len(caBundle) > 0 {
data, err := ioutil.ReadFile(caBundle)
if err != nil {
return nil, nil, nil, err
}

// Add the CA bundle to the client config's CA roots if provided and we haven't done that already.
// FIXME: handle registryCABundle on one place
if tlsConfig != nil && len(cadata) > 0 && !registryCABundleIncluded && !registryInsecure {
if tlsConfig.RootCAs == nil {
tlsConfig.RootCAs = x509.NewCertPool()
}

tlsConfig.RootCAs.AppendCertsFromPEM(data)
tlsConfig.RootCAs.AppendCertsFromPEM(cadata)
}

transport := knet.SetTransportDefaults(&http.Transport{
Expand Down

0 comments on commit ad4a823

Please sign in to comment.