Parse registries for wildcard entries #11

umohnani8 · 2021-04-09T02:27:30Z

Insecure, blocked, and allowed registries now accept
wildcard entries so add logic to parse that from the
Image CR.
Update tests to reflect the new parsing logic for wildcards.

Signed-off-by: Urvashi Mohnani [email protected]

openshift-ci-robot · 2021-04-09T02:27:42Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: umohnani8
To complete the pull request process, please assign adambkaplan after the PR has been reviewed.
You can assign the PR to them by writing /assign @adambkaplan in a comment when ready.

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

Details

Needs approval from an approver in each of these files:

OWNERS

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

umohnani8 · 2021-04-09T02:28:22Z

Tests will fail till containers/image#1191 gets in and we vendor it in here.
@vrothberg @mtrmac @lsm5 PTAL

pkg/registries/registries.go

mtrmac

I don’t think this works correctly, and the validation approach worries me.

Ultimately it’s also a bit early, the semantics of the matching (and of the allowed configuration format WRT Prefix/Location presence) is still somewhat in flux.

pkg/registries/registries.go

umohnani8 · 2021-04-12T14:25:15Z

@mtrmac addressed comments, PTAL

openshift-ci-robot · 2021-04-12T14:36:41Z

@umohnani8: This pull request references Bugzilla bug 1948593, which is invalid:

expected the bug to target the "4.8.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

[WIP]Bug 1948593: Parse registries for wildcard entries

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.

umohnani8 · 2021-04-12T14:37:43Z

/bugzilla refresh

openshift-ci-robot · 2021-04-12T14:37:52Z

@umohnani8: This pull request references Bugzilla bug 1948593, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.8.0) matches configured target release for branch (4.8.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla ([email protected]), skipping review request.

Details

In response to this:

/bugzilla refresh

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.

umohnani8 · 2021-04-12T14:38:36Z

/retest

mtrmac

Thanks for adding the tests.

pkg/registries/registries.go

go.mod

pkg/registries/registries.go

pkg/registries/registries_test.go

lsm5

LGTM as far as TestRegMatchesScope and TestIsValidRegistryOrWildcardScope are concerned and they pass. I'll defer to @mtrmac and @vrothberg for further review.

vrothberg

LGTM

pkg/registries/registries.go

umohnani8 · 2021-07-22T16:55:05Z

@mtrmac addressed comments, PTAL

mtrmac

Code LGTM, if we are in a hurry; basically only documentation concerns. Of those, the public API documentation the two functions are the only externally-visible.

pkg/registries/registries.go

pkg/registries/registries_test.go

pkg/registries/registries.go

umohnani8 · 2021-07-22T17:57:11Z

@mtrmac addressed the final comments, PTAL

mtrmac

LGTM, feel free to merge without another review round.

pkg/registries/registries.go

mtrmac · 2021-07-22T19:06:24Z

/approve

Insecure, blocked, and allowed registries now accept wildcard entries so add logic to parse that from the Image CR. Update tests to reflect the new parsing logic for wildcards. Signed-off-by: Urvashi Mohnani <[email protected]>

Needed for updates to tests to pass. Signed-off-by: Urvashi Mohnani <[email protected]>

mtrmac · 2021-07-22T19:11:34Z

/lgtm (also exposing @vrothberg and @lsm5 ’s reviews to the tooling)

mtrmac · 2021-07-22T19:14:09Z

/hold cancel

mtrmac · 2021-07-22T19:14:45Z

/lgtm

openshift-ci · 2021-07-22T19:14:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lsm5, mtrmac, umohnani8

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:

~~OWNERS~~ [mtrmac]

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

... instead of a local copy. The code is currently 100% identical, so this doesn't make a difference, but that will change with openshift/runtime-utils#11 . Signed-off-by: Miloslav Trmač <[email protected]>

umohnani8 changed the title ~~Parse registries for wildcard entries~~ [WIP] Parse registries for wildcard entries Apr 9, 2021

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 9, 2021

openshift-ci-robot requested review from adambkaplan and mtrmac April 9, 2021 02:27

vrothberg reviewed Apr 9, 2021

View reviewed changes

pkg/registries/registries.go Outdated Show resolved Hide resolved

umohnani8 force-pushed the wildcards branch from 6e89f1c to 9b0e1bd Compare April 9, 2021 12:31

mtrmac reviewed Apr 9, 2021

View reviewed changes

umohnani8 force-pushed the wildcards branch from 9b0e1bd to e1b8d35 Compare April 9, 2021 12:39

mtrmac mentioned this pull request Apr 9, 2021

Enable subdomain matching in registries.conf containers/image#1191

Merged

umohnani8 force-pushed the wildcards branch from e1b8d35 to 4b674e7 Compare April 12, 2021 14:20

umohnani8 changed the title ~~[WIP] Parse registries for wildcard entries~~ [WIP]Bug 1948593: Parse registries for wildcard entries Apr 12, 2021

openshift-ci-robot added the bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. label Apr 12, 2021

openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Apr 12, 2021

umohnani8 changed the title ~~[WIP]Bug 1948593: Parse registries for wildcard entries~~ Bug 1948593: Parse registries for wildcard entries Apr 12, 2021

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Apr 12, 2021

umohnani8 force-pushed the wildcards branch 2 times, most recently from c8b6720 to 6f0e537 Compare April 12, 2021 16:28

mtrmac reviewed Apr 12, 2021

View reviewed changes

lsm5 approved these changes Jul 19, 2021

View reviewed changes

vrothberg reviewed Jul 21, 2021

View reviewed changes

mtrmac reviewed Jul 22, 2021

View reviewed changes

pkg/registries/registries.go Outdated Show resolved Hide resolved

pkg/registries/registries.go Outdated Show resolved Hide resolved

pkg/registries/registries.go Outdated Show resolved Hide resolved

umohnani8 force-pushed the wildcards branch 2 times, most recently from bc0f719 to f4057e8 Compare July 22, 2021 15:20

mtrmac reviewed Jul 22, 2021

View reviewed changes

pkg/registries/registries.go Outdated Show resolved Hide resolved

pkg/registries/registries.go Outdated Show resolved Hide resolved

umohnani8 force-pushed the wildcards branch from f4057e8 to 4fa726f Compare July 22, 2021 16:54

mtrmac reviewed Jul 22, 2021

View reviewed changes

umohnani8 force-pushed the wildcards branch from 4fa726f to e5a770d Compare July 22, 2021 17:56

mtrmac reviewed Jul 22, 2021

View reviewed changes

pkg/registries/registries.go Show resolved Hide resolved

pkg/registries/registries.go Outdated Show resolved Hide resolved

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 22, 2021

umohnani8 added 2 commits July 22, 2021 15:10

Parse registries for wildcard entries correctly

58f0006

Insecure, blocked, and allowed registries now accept wildcard entries so add logic to parse that from the Image CR. Update tests to reflect the new parsing logic for wildcards. Signed-off-by: Urvashi Mohnani <[email protected]>

Vendor containers/image v5.11.0

802e0c4

Needed for updates to tests to pass. Signed-off-by: Urvashi Mohnani <[email protected]>

umohnani8 force-pushed the wildcards branch from e5a770d to 802e0c4 Compare July 22, 2021 19:10

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 22, 2021

openshift-ci bot assigned mtrmac Jul 22, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2021

openshift-merge-robot merged commit 8b8348d into openshift:master Jul 22, 2021

mtrmac mentioned this pull request Oct 12, 2021

[OCPNODE-747] New CRD ImageDigestMirrorSet and ImageTagMirrorSet to support AllowMirrByTags openshift/enhancements#929

Merged

Parse registries for wildcard entries #11

Parse registries for wildcard entries #11

Uh oh!

Conversation

umohnani8 commented Apr 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Apr 9, 2021

Uh oh!

umohnani8 commented Apr 9, 2021

Uh oh!

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

umohnani8 commented Apr 12, 2021

Uh oh!

openshift-ci-robot commented Apr 12, 2021

Uh oh!

umohnani8 commented Apr 12, 2021

Uh oh!

openshift-ci-robot commented Apr 12, 2021

Uh oh!

umohnani8 commented Apr 12, 2021

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lsm5 left a comment

Choose a reason for hiding this comment

Uh oh!

vrothberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

umohnani8 commented Jul 22, 2021

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

umohnani8 commented Jul 22, 2021

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mtrmac commented Jul 22, 2021

Uh oh!

umohnani8 commented Apr 9, 2021 •

edited

Loading