Default config no longer sets a whitelist #19829

smarterclayton · 2018-05-23T20:19:23Z

Add an integration test that validates the whitelist. We don't set this
by default in installer, and in the future we want it to by dynamic on a
cluster. In general, this is an optional setting.

@bparees

Enables openshift/image-registry#88

bparees · 2018-05-23T21:39:50Z

pkg/cmd/server/start/master_args.go

-		ImagePolicyConfig: configapi.ImagePolicyConfig{
-			AllowedRegistriesForImport: configapi.DefaultAllowedRegistriesForImport,
-		},
+		ImagePolicyConfig: configapi.ImagePolicyConfig{},


is the constant even used anywhere anymore then? can we get rid of it?

in the e2e tests

and in the integration tests

so should we move the constant into the test package? seems misleading/confusing to call if defaultallowedregistriesforimport in our core packages when it's not actually that.

We could call it recommended.

smarterclayton · 2018-05-23T23:58:24Z

/retest On May 23, 2018, at 5:40 PM, OpenShift CI Robot <[email protected]> wrote: @smarterclayton <https://github.com/smarterclayton>: The following test *failed*, say /retest to rerun them all: Test name Commit Details Rerun command ci/openshift-jenkins/extended_conformance_install e01d63e <e01d63e> link <https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/19829/test_pull_request_origin_extended_conformance_install/11839/> /test extended_conformance_install Full PR test history <https://openshift-gce-devel.appspot.com/pr/19829>. Your PR dashboard <https://openshift-gce-devel.appspot.com/pr/smarterclayton>. Please help us cut down on flakes by linking to <https://github.com/kubernetes/community/blob/master/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests> an open issue <https://github.com/openshift/origin/issues?q=is:issue+is:open> when you hit one in your PR. Instructions for interacting with me using PR comments are available here <https://git.k8s.io/community/contributors/devel/pull-requests.md>. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> repository. I understand the commands that are listed here <https://go.k8s.io/bot-commands>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19829 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p3lYlYXupaz_8-KL7nihEGz3teMDks5t1dc1gaJpZM4ULHiN> .

soltysh

lgtm, but I agree with Ben, if that default is not being used in the main code it should live in the tests only.

smarterclayton · 2018-05-24T17:00:33Z

Moved to a private constant in test.

bparees · 2018-05-24T17:48:59Z

/lgtm

openshift-ci-robot · 2018-05-24T17:49:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, smarterclayton

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:

~~pkg/cmd/OWNERS~~ [bparees,smarterclayton]
~~test/integration/OWNERS~~ [bparees,smarterclayton]
~~test/util/OWNERS~~ [bparees,smarterclayton]

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

openshift-bot · 2018-05-24T18:09:08Z

/retest

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

openshift-bot · 2018-05-24T20:10:35Z

/retest

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

Add an integration test that validates the whitelist. We don't set this by default in installer, and in the future we want it to by dynamic on a cluster. In general, this is an optional setting.

openshift-ci-robot · 2018-05-24T20:44:39Z

New changes are detected. LGTM label has been removed.

smarterclayton · 2018-05-24T23:44:51Z

/retest

Since openshift/origin#19829 (3.10+) Signed-off-by: Michal Minář <[email protected]>

openshift-ci-robot requested review from bparees and soltysh May 23, 2018 20:19

openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 23, 2018

bparees reviewed May 23, 2018

View reviewed changes

soltysh approved these changes May 24, 2018

View reviewed changes

smarterclayton force-pushed the change_default_whitelist branch 2 times, most recently from 39f893f to c1ee61c Compare May 24, 2018 17:00

smarterclayton mentioned this pull request May 24, 2018

Support TEST_KUBECONFIG for integration tests openshift/image-registry#88

Merged

openshift-ci-robot assigned bparees May 24, 2018

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

Default config no longer sets a whitelist

70243fd

Add an integration test that validates the whitelist. We don't set this by default in installer, and in the future we want it to by dynamic on a cluster. In general, this is an optional setting.

smarterclayton force-pushed the change_default_whitelist branch from c1ee61c to 70243fd Compare May 24, 2018 20:44

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label May 24, 2018

smarterclayton added the lgtm Indicates that a PR is ready to be merged. label May 25, 2018

openshift-merge-robot merged commit 2dcfb32 into openshift:master May 25, 2018

bparees mentioned this pull request Jun 15, 2018

How to whitelist a local image registry or disable the whitelist #19524

Closed

miminar pushed a commit to miminar/openshift-docs that referenced this pull request Jun 19, 2018

all images are whitelisted by default

14220d0

Since openshift/origin#19829 (3.10+) Signed-off-by: Michal Minář <[email protected]>

miminar mentioned this pull request Jun 19, 2018

all images are whitelisted by default openshift/openshift-docs#10205

Merged

openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/openshift-docs that referenced this pull request Jun 21, 2018

all images are whitelisted by default

8ca273c

Since openshift/origin#19829 (3.10+) Signed-off-by: Michal Minář <[email protected]>

Default config no longer sets a whitelist #19829

Default config no longer sets a whitelist #19829

Uh oh!

Conversation

smarterclayton commented May 23, 2018

Uh oh!

bparees May 23, 2018

Choose a reason for hiding this comment

Uh oh!

smarterclayton May 24, 2018

Choose a reason for hiding this comment

Uh oh!

smarterclayton May 24, 2018

Choose a reason for hiding this comment

Uh oh!

bparees May 24, 2018

Choose a reason for hiding this comment

Uh oh!

smarterclayton May 24, 2018

Choose a reason for hiding this comment

Uh oh!

smarterclayton commented May 23, 2018 via email

Uh oh!

soltysh left a comment

Choose a reason for hiding this comment

Uh oh!

smarterclayton commented May 24, 2018

Uh oh!

bparees commented May 24, 2018

Uh oh!

openshift-ci-robot commented May 24, 2018

Uh oh!

openshift-bot commented May 24, 2018

Uh oh!

openshift-bot commented May 24, 2018

Uh oh!

openshift-ci-robot commented May 24, 2018

Uh oh!

smarterclayton commented May 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants