pkg/oc/cli/admin/release: Add --insecure #21266

wking · 2018-10-16T04:53:23Z

Matching oc image mirror --insecure. This lets callers avoid:

$ podman run -d -p 5000:5000 --name registry docker.io/library/registry
$ oc adm release mirror --from=registry.svc.ci.openshift.org/openshift/origin-release:v4.0 --to localhost:5000/openshift-v4.0
info: Mirroring 79 images to localhost:5000/openshift-v4.0 ...

error: unable to connect to localhost:5000/openshift-v4.0: Get https://localhost:5000/v2/: http: server gave HTTP response to HTTPS client
...
error: unable to connect to localhost:5000/openshift-v4.0: Get https://localhost:5000/v2/: http: server gave HTTP response to HTTPS client
error: an error occurred during planning

when the local registry only speaks HTTP.

Ideally we could turn this off only for pushes, allowing us to pull from a remote registry over HTTPS but push to the local registry over HTTP. But oc image mirror doesn't seem to support that level of granularity. And even though the coarse --insecure allows access over HTTP, we should still be using HTTPS where possible, so the risk of leaking secrets from a properly-configured registry is small. Although maybe there are increased man-in-the-middle risks? I'm not entirely clear on whether --insecure weakens our "acceptable" HTTPS quality as well (like curl --insecure), or if it's just limited to the documented "allow HTTPS too".

CC @smarterclayton.

wking · 2018-10-16T05:28:59Z

On the other hand, this may not be much use unless we can talk the cluster into pulling over HTTP. Launching a cluster based on an image mirrored to an insecure registry is giving me errors like:

[core@wking-bootstrap ~]$ journalctl -f
...
Oct 16 05:26:40 wking-bootstrap bootkube.sh[1350]: Trying to pull 192.168.122.1:5000/openshift-v4.0:release...Failed
Oct 16 05:26:40 wking-bootstrap bootkube.sh[1350]: unable to pull 192.168.122.1:5000/openshift-v4.0:release: unable to pull image: Error determining manifest MIME type for docker://192.168.122.1:5000/openshift-v4.0:release: pinging docker registry returned: Get https://192.168.122.1:5000/v2/: http: server gave HTTP response to HTTPS client
...

wking · 2018-10-16T06:48:46Z

On the other hand, this may not be much use unless we can talk the cluster into pulling over HTTP.

CRI-O's docs have:

Enabling --insecure-registry is useful when running a local registry. However, because its use creates security vulnerabilities, it should ONLY be enabled for testing purposes. For increased security, users should add their CA to their system's list of trusted CAs instead of using --insecure-registry.

So while this PR may be useful for consistency with oc image mirror, it's probably not very useful for the local-registry case. I've opened a very-WIP openshift/installer#472 as part of supporting a local registry with a self-signed cert.

smarterclayton · 2018-10-16T19:19:55Z

The intention is that all "docker registry" modifier security related flags would be on all oc image and all relevant oc adm release commands.

smarterclayton · 2018-10-16T19:20:11Z

You need to run hack/update-generated-completions.sh

wking · 2018-10-16T20:40:53Z

You need to run hack/update-generated-completions.sh

Done with 2bb9c08 -> 7524633.

wking · 2018-10-17T03:50:07Z

Unit (and basically everyone I checked) died with:

E1016 20:44:24.147738      11 event.go:209] Unable to write event: 'Post https://172.30.0.1:443/api/v1/namespaces/ci-op-n4ibmm3g/events: dial tcp 172.30.0.1:443: connect: connection refused' (may retry after sleeping)
2018/10/16 20:44:25 Ran for 3m23s
error: could not run steps: could not wait for build: could not list builds: Get https://172.30.0.1:443/apis/build.openshift.io/v1/namespaces/ci-op-n4ibmm3g/builds?fieldSelector=metadata.name%3Dsrc: dial tcp 172.30.0.1:443: connect: connection refused

Looking at the unit history, I'm just very unlucky ;).

/retest

wking · 2018-10-22T04:46:07Z

/retest

wking · 2018-10-22T17:55:06Z

/retest

flaper87 · 2018-11-09T18:41:28Z

@wking can we do this for the new subcommand too? When creating a new release, the command fails to talk to a registry that has a self-signed certificate.

wking · 2018-11-09T21:53:29Z

can we do this for the new subcommand too?

Done (and rebased onto master) with 7524633 -> 4fd4121.

flaper87 · 2018-11-10T09:12:01Z

/lgtm

flaper87 · 2018-11-10T09:12:40Z

/approve

soltysh

/approve

flaper87 · 2018-11-21T08:11:22Z

/test ci/prow/cmd

flaper87 · 2018-11-21T08:11:46Z

/retest

flaper87 · 2018-11-21T12:43:37Z

I don't know why the ci/prow/cmd job is stuck

openshift-bot · 2018-11-21T14:06:02Z

/retest

Please review the full test history for this PR and help us cut down flakes.

soltysh · 2018-11-21T15:13:34Z

I don't know why the ci/prow/cmd job is stuck

Stuff has changed in the mean time, re-testing should solve the problem 😉

openshift-bot · 2018-11-21T16:06:51Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2018-11-22T02:12:07Z

/retest

Please review the full test history for this PR and help us cut down flakes.

flaper87 · 2018-11-22T04:31:51Z

The oc adm release new --insecure works when pushing new images but it fails when I try to overwrite some of the images in the release:

This command fails with: Unable to connect to the server: x509: certificate signed by unknown authority

oc adm release new --insecure -n openshift machine-config-operator=10.1.8.88:8787/openshift/machine-config-operator:latest --from-image-stream=origin-v4.0 --to-image=10.1.8.88:8787/shiftstack/origin-release:v4.0

Whereas this one works:

oc adm release new --insecure -n openshift --from-image-stream=origin-v4.0 --to-image=10.1.8.88:8787/shiftstack/origin-release:v4.0

openshift-bot · 2018-11-22T08:18:54Z

/retest

Please review the full test history for this PR and help us cut down flakes.

flaper87

I found the issue. We should pass the Insecure flag to the extract options too. It should be enough to add opts.Insecure = o.Insecure to https://github.com/openshift/origin/pull/21266/files#diff-e7e7595e7ee6718351c88cf2e07449acR639

openshift-bot · 2018-11-22T12:16:50Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2018-11-22T14:17:38Z

/retest

Please review the full test history for this PR and help us cut down flakes.

wking · 2018-11-22T15:44:48Z

/hold

cgwalters · 2019-02-27T14:30:33Z

I didn't know this PR existed and started on it in openshift/machine-config-operator#496 (comment) too

One higher level question though; how hard/hacky would it be to use the cluster CA from the active KUBECONFIG?

openshift-ci-robot · 2019-02-28T10:48:01Z

New changes are detected. LGTM label has been removed.

openshift-ci-robot · 2019-02-28T10:48:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: soltysh, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~contrib/completions/OWNERS~~ [soltysh]
~~pkg/oc/OWNERS~~ [soltysh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Matching 'oc image mirror --insecure'. This lets callers avoid: $ podman run -d -p 5000:5000 --name registry docker.io/library/registry $ oc adm release mirror --from=registry.svc.ci.openshift.org/openshift/origin-release:v4.0 --to localhost:5000/openshift-v4.0 info: Mirroring 79 images to localhost:5000/openshift-v4.0 ... error: unable to connect to localhost:5000/openshift-v4.0: Get https://localhost:5000/v2/: http: server gave HTTP response to HTTPS client ... error: unable to connect to localhost:5000/openshift-v4.0: Get https://localhost:5000/v2/: http: server gave HTTP response to HTTPS client error: an error occurred during planning when the local registry only speaks HTTP. Ideally we could turn this off only for pushes, allowing us to pull from a remote registry over HTTPS but push to the local registry over HTTP. But 'oc image mirror' doesn't seem to support that level of granularity. And even though the course --insecure allows access over HTTP, we should still be using HTTPS where possible, so the risk of leaking secrets from a properly-configured registry is small. Although maybe there are increased man-in-the-middle risks? I'm not entirely clear on whether --insecure weakens our "acceptable" HTTPS quality as well, or if it's just limited to the documented "allow HTTPS too". The contrib/completions/*/oc updates were generated with: $ make all WHAT=cmd/oc $ hack/update-generated-completions.sh

wking · 2019-02-28T11:08:50Z

One higher level question though; how hard/hacky would it be to use the cluster CA from the active KUBECONFIG?

For the registry access? I'd have expected that was already happening for registries that belong to the cluster (e.g. via this). Is it not?

I've pushed 4fd4121 -> 1607ad7, rebasing onto master and addressing (I think) @flaper87's ExtractOptions.Insecure suggestion.

cgwalters · 2019-03-18T22:02:44Z

I still need this PR for openshift/machine-config-operator#496

openshift-bot · 2019-06-17T02:04:31Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot · 2019-07-17T03:40:10Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

flaper87 · 2019-07-17T07:04:22Z

/unassign @flaper87

openshift-bot · 2019-08-16T11:23:22Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci-robot · 2019-08-16T11:23:23Z

@openshift-bot: Closed this PR.

Details

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 16, 2018

openshift-ci-robot requested review from juanvallejo and soltysh October 16, 2018 04:53

wking force-pushed the oc-adm-release-mirror-insecure branch from 2bb9c08 to 7524633 Compare October 16, 2018 20:40

wking force-pushed the oc-adm-release-mirror-insecure branch from 7524633 to 4fd4121 Compare November 9, 2018 21:52

openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 9, 2018

wking changed the title ~~pkg/oc/cli/admin/release/mirror: Add --insecure~~ pkg/oc/cli/admin/release: Add --insecure Nov 9, 2018

openshift-ci-robot assigned flaper87 Nov 10, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 10, 2018

soltysh approved these changes Nov 16, 2018

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 16, 2018

flaper87 suggested changes Nov 22, 2018

View reviewed changes

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 22, 2018

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 3, 2018

wking force-pushed the oc-adm-release-mirror-insecure branch from 4fd4121 to cbd6deb Compare February 28, 2019 10:47

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 28, 2019

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 28, 2019

wking force-pushed the oc-adm-release-mirror-insecure branch from cbd6deb to 1607ad7 Compare February 28, 2019 11:02

openshift-bot unassigned flaper87 Jun 17, 2019

openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 17, 2019

openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 17, 2019

openshift-ci-robot closed this Aug 16, 2019

pkg/oc/cli/admin/release: Add --insecure #21266

pkg/oc/cli/admin/release: Add --insecure #21266

Uh oh!

Conversation

wking commented Oct 16, 2018

Uh oh!

wking commented Oct 16, 2018

Uh oh!

wking commented Oct 16, 2018

Uh oh!

smarterclayton commented Oct 16, 2018

Uh oh!

smarterclayton commented Oct 16, 2018

Uh oh!

wking commented Oct 16, 2018

Uh oh!

wking commented Oct 17, 2018

Uh oh!

wking commented Oct 22, 2018

Uh oh!

wking commented Oct 22, 2018

Uh oh!

flaper87 commented Nov 9, 2018

Uh oh!

wking commented Nov 9, 2018

Uh oh!

flaper87 commented Nov 10, 2018

Uh oh!

flaper87 commented Nov 10, 2018

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

flaper87 commented Nov 21, 2018

Uh oh!

flaper87 commented Nov 21, 2018

Uh oh!

flaper87 commented Nov 21, 2018

Uh oh!

openshift-bot commented Nov 21, 2018

Uh oh!

soltysh commented Nov 21, 2018

Uh oh!

openshift-bot commented Nov 21, 2018

Uh oh!

openshift-bot commented Nov 22, 2018

Uh oh!

flaper87 commented Nov 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Nov 22, 2018

Uh oh!

flaper87 left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-bot commented Nov 22, 2018

Uh oh!

openshift-bot commented Nov 22, 2018

Uh oh!

wking commented Nov 22, 2018

Uh oh!

cgwalters commented Feb 27, 2019

Uh oh!

openshift-ci-robot commented Feb 28, 2019

Uh oh!

openshift-ci-robot commented Feb 28, 2019

Uh oh!

wking commented Feb 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cgwalters commented Mar 18, 2019

Uh oh!

openshift-bot commented Jun 17, 2019

Uh oh!

openshift-bot commented Jul 17, 2019

Uh oh!

flaper87 commented Jul 17, 2019

Uh oh!

openshift-bot commented Aug 16, 2019

Uh oh!

flaper87 commented Nov 22, 2018 •

edited

Loading

wking commented Feb 28, 2019 •

edited

Loading