configure imagePolicyConfig:allowedRegistriesForImport

[miminar]

Currently the allowedRegistriesForImport are not configured by ansible scripts,
which effectively disables whitelisting. This PR sets the default registry
whitelist and allowes for simple overrides.

Related upstream PR: openshift/origin#17783
Related doc PR: openshift/openshift-docs#7788

[miminar]

Instead of setting the defaults here, is it possible to load them from set_allowed_registries()? Not sure it's the best way but the current approach is not consistent with anything around.

[miminar]

Resolved. By default, everything is allowed.

[miminar]

Could this be maybe generated by the openshift binary and fetched from the library module when needed?

(the binary already generates this on openshift start master --write, I'm just not sure, how to utilize that here)

[miminar]

Just realized that there is a lot of variables like:

#oreg_url_master=example.com/openshift3/ose-${component}:${version}
#oreg_url_node=example.com/openshift3/ose-${component}:${version}
#oreg_url=example.com/openshift3/ose-${component}:${version}
#openshift_cockpit_deployer_prefix=registry.example.com/myrepo/
#openshift_metrics_image_prefix=docker.io/openshift/origin-
#openshift_metrics_image_prefix=registry.access.redhat.com/openshift3/

If the admin overrides any of those to some custom registry without touching *_allowed_registries_for_import, he will certainly fail.

Should those prefixes be added to the defaults? Or should we run some sanity check at the beginning?

[miminar]

/assign @sdodson @jim-minter

@bparees FYI

[miminar]

Some of the extended tests are now failing:

/go/src/github.com/openshift/origin/_output/local/go/src/github.com/openshift/origin/test/extended/builds/new_app.go:49
Expected error:
    <*errors.errorString | 0xc4217d2550>: {
        s: "The build \"a234567890123456789012345678901234567890123456789012345678-1\" status is \"Failed\"",
    }
    The build "a234567890123456789012345678901234567890123456789012345678-1" status is "Failed"
not to have occurred
/go/src/github.com/openshift/origin/_output/local/go/src/github.com/openshift/origin/test/extended/builds/new_app.go:59
...
2018-02-19T14:19:20.024979251Z error: build error: Failed to push image: received unexpected HTTP status: 500 Internal Server Error

And this looks to be the reason (forbidden translated to 500):

time="2018-02-19T14:23:29.223383892Z" level=error msg="error creating ImageStreamMapping: ImageStream \"nodejsroot\" is invalid: status.tags[latest].items[0].dockerImageReference: Forbidden: registry \"172.30.73.52:5000\" not allowed by whitelist: \"docker-registry.default.svc:5000\", \"docker.io:443\", \"*.docker.io:443\", \"*.redhat.com:443\", and 5 more ..." go.version=go1.9.2 http.request.contenttype=application/vnd.docker.distribution.manifest.v2+json http.request.host="docker-registry.default.svc:5000" http.request.id=b4d7546f-73e4-438d-bbe6-46ad83c9b465 http.request.method=PUT http.request.remoteaddr="172.16.6.1:49410" http.request.uri=/v2/extended-test-s2i-build-root-fvr68-b5776/nodejsroot/manifests/latest http.request.useragent="docker/1.12.6 go/go1.8.3 kernel/3.10.0-693.17.1.el7.x86_64 os/linux arch/amd64 UpstreamClient(go-dockerclient)" instance.id=c760f887-3d08-4e75-a7b4-61522c8e0880 openshift.auth.user="system:serviceaccount:extended-test-s2i-build-root-fvr68-b5776:builder" vars.name=extended-test-s2i-build-root-fvr68-b5776/nodejsroot vars.reference=latest 
time="2018-02-19T14:23:29.223538513Z" level=error msg="response completed with error" err.code=unknown err.detail="ImageStream \"nodejsroot\" is invalid: status.tags[latest].items[0].dockerImageReference: Forbidden: registry \"172.30.73.52:5000\" not allowed by whitelist: \"docker-registry.default.svc:5000\", \"docker.io:443\", \"*.docker.io:443\", \"*.redhat.com:443\", and 5 more ..." err.message="unknown error" go.version=go1.9.2 http.request.contenttype=application/vnd.docker.distribution.manifest.v2+json http.request.host="docker-registry.default.svc:5000" http.request.id=b4d7546f-73e4-438d-bbe6-46ad83c9b465 http.request.method=PUT http.request.remoteaddr="172.16.6.1:49410" http.request.uri=/v2/extended-test-s2i-build-root-fvr68-b5776/nodejsroot/manifests/latest http.request.useragent="docker/1.12.6 go/go1.8.3 kernel/3.10.0-693.17.1.el7.x86_64 os/linux arch/amd64 UpstreamClient(go-dockerclient)" http.response.contenttype="application/json; charset=utf-8" http.response.duration=698.99085ms http.response.status=500 http.response.written=783 instance.id=c760f887-3d08-4e75-a7b4-61522c8e0880 openshift.auth.user="system:serviceaccount:extended-test-s2i-build-root-fvr68-b5776:builder" vars.name=extended-test-s2i-build-root-fvr68-b5776/nodejsroot vars.reference=latest 

The docker-registry is not configured to use the same dns name as the cluster (http.request.host="docker-registry.default.svc:5000"). I'll take a look around and try to find the jenkins job that sets up the registry. If somebody knows already, please let me know.

[miminar]

/hold

[miminar]

Tentative fix: openshift/origin#18663

[bparees]

@miminar i'm puzzled by why this change is causing problems. The existing behavior was such that the ansible install didn't set a whitelist, which should have meant we fell back to the hardcoded whitelist, right? And the hardcoded whitelist is the same as the whitelist you've introduced explicilty.

[miminar]

We don't fall-back to hardcoded whitelist. If the policy whitelist is not configured in the master config file, no whitelist will be used - meaning "allow all".

The thing I don't understand right now is why the job ci/openshift-jenkins/gcp launches registry without dns name configured. Which leads to creation of imagestreammappings with dockerImageReference having "{REGISTRY_SERVICE_IP}:5000" which is not whitelisted.

Similar job for openshift/origin repository has the same parent job https://github.com/openshift/aos-cd-jobs/blob/master/sjb/config/common/test_cases/origin_release_install_gce.yml that does the setup. The origin job properly configures the registry with REGISTRY_OPENSHIFT_SERVER_ADDR=docker-registry.default.svc:5000 that gets whitelisted and resulting images have this prefix. Not sure why it's not the case for the former job. I've added some debug statements to the ansible scripts that maybe clear some unknowns.

[miminar]

@bparees So the problem is related to your #6913. The image used for testing (origin-docker-registry:v3.9.0-alpha.4) doesn't have @legionus' fix yet.

And there is just REGISTRY_OPENSHIFT_SERVER_ADDR set to docker-registry.default.svc:5000. The registry defaults to service ip:port and thus the resulting image doesn't pass the whitelisting.

I'd prefer to set OPENSHIFT_DEFAULT_REGISTRY variable as well in the ansible scripts to deal with earlier registry versions. What do you think?

[bparees]

I'd prefer to set OPENSHIFT_DEFAULT_REGISTRY variable as well in the ansible scripts to deal with earlier registry versions. What do you think?

i can live with it.

[bparees]

@miminar I do wonder if for existing clusters that do not have an imageimport policy already, we should not be setting this as we could potentially break them if they are using registries that are not in the default whitelist.

Also I suspect we don't want to drop this on 3.9 at this point since auto-defining/defaulting the registry whitelist could have some unexpected effects.

[miminar]

/retest

[smarterclayton]

Adding a hold as well.

/hold

Probably need to have a discussion about the point of this now.

[bparees]

Probably need to have a discussion about the point of this now.

@smarterclayton what sort of discussion? Don't we still want admins to be able to set this value at install time, even if the default behavior is to allow all registries?

[miminar]

Rebased.

/retest

[miminar]

/retest

[miminar]

The gcp test succeeded finally. Don't ask me why.

[miminar]

Probably need to have a discussion about the point of this now.

@smarterclayton No registry whitelist is configured by default which matches origin's latest behaviour. This only allows for whitelist configuration if the admin desires.

[sdodson]

bot, retest this please

[miminar]

Squashed and removed debug statements.

[mtnbikenc]

Lint fixes needed

[mtnbikenc]

s/basestring/str/

[mtnbikenc]

s/basestring/str/

[mtnbikenc]

Use .replace() to remove multi character strings.

[miminar]

Good catch. Changed to .split('://',1)[-1] instead.

[bparees]

anything else needed here?

[sdodson]

/lgtm
ansible code looks fine, if you think @smarterclayton's /hold is outdated go ahead and remove it

[sdodson]

bot, retest this please

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: miminar, sdodson

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [sdodson]

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

[bparees]

/hold cancel

[sdodson]

bot, retest this please

[openshift-ci-robot]

@miminar: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/openshift-jenkins/extended_conformance_install_crio	fe83785	link	/test crio
ci/openshift-jenkins/system-containers	fba6d35	link	/test system-containers
ci/openshift-jenkins/gcp	c95dc5a	link	/test gcp

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.