Create ImageContentPolicy CRD to config/v1 and allowMirrorByTags support #874

QiWang19 · 2021-03-23T20:55:02Z

Create new CRD ImageContentPolicy to config/v1.
The schema of ImageContentPolicy contain schemas from ImageContentSource operator/v1alpha1 , and a new field allowMirrorByTags support.
A Jira card was created to record the upgrade OCPNODE-717. The repositories currently rely on operator/v1alpha1 ImageContentSourcePolicy will be migrated to config/v1 ImageContentPolicy.

API enhancement: openshift/enhancements#690
Follow-up enhancement for adding ICSP to operator/v1: openshift/enhancements#873
REF: https://issues.redhat.com/browse/OCPNODE-521, #636
client-go PR: openshift/client-go#195

Signed-off-by: Qi Wang [email protected]

openshift-ci-robot · 2021-03-23T20:55:16Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: QiWang19
To complete the pull request process, please assign eparis after the PR has been reviewed.
You can assign the PR to them by writing /assign @eparis 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

QiWang19 · 2021-03-23T21:09:05Z

/assign @eparis

55octet · 2021-05-14T18:28:54Z

Any updates on when this will be a viable option?

QiWang19 · 2021-05-26T20:33:59Z

Any updates on when this will be a viable option?

This feature will be available in 4.9.

slauger · 2021-05-27T07:46:21Z

It would be great if this "feature" can also be backported to the next EUS release (4.8).

Currently a lot of operators in the OLM are not (yet) using a digest. As a result a lot of customers cannot use the ImageContentSourcePolicy and need to configure a custom MachineConfig for the registries.conf.

QiWang19 · 2021-06-29T14:39:58Z

/test verify

QiWang19 · 2021-06-29T14:50:23Z

@mrunalp @abhinavdahiya @wking @adambkaplan PTAL
With the new repositoryMirrors, should we go v1alpha2 and drop repositoryDigestMirrors as the comment openshift/enhancements#690 (comment) suggested? How will the plan to drop it, should it be removed in this PR?

sttts · 2021-06-29T19:17:52Z

operator/v1alpha1/types_image_content_source_policy.go

+	// may impact the exact order mirrors are contacted in, or some mirrors may be contacted
+	// in parallel, so this should be considered a preference rather than a guarantee of ordering.
+	// +optional
+	Mirrors []string `json:"mirrors"`


this needs a list-type, and add omitempty as optional slice.

what are these strings? URL? These need a specification too (as type Mirror string).

This is not resolved.

operator/v1alpha1/types_image_content_source_policy.go

sttts · 2021-06-29T19:18:34Z

operator/v1alpha1/types_image_content_source_policy.go

+type RepositoryMirrors struct {
+	// source is the repository that users refer to, e.g. in image pull specifications.
+	// +required
+	Source string `json:"source"`


which format does this have? Please specify regex, minlength, format, etc.

This is not resolved. It needs specification.

@QiWang19 Those specifications can be added as comments:

// +kubebuilder:validation:Pattern=`^https?:\/\/` // +kubebuilder:validation:MaxLength=2048 // +kubebuilder:validation:Required // +required

The pattern in this example is just an example... Is it URL format? or host:port format?

@rphillips The format is the prefix in the doc https://github.com/containers/image/blob/main/docs/containers-registries.conf.5.md#choosing-a-registry-toml-table, should I link this doc to the comment

the format should be specified like Ryan described above through kubebuilder:validation:Pattern

operator/v1alpha1/types_image_content_source_policy.go

nee1esh · 2021-07-02T16:21:37Z

/retest

operator/v1alpha1/types_image_content_source_policy.go

QiWang19 · 2021-09-22T18:28:08Z

Added the ImageContentPolicy to the config/v1 group since it is not used for configuring an operator, according to the discussions from the thread: https://coreos.slack.com/archives/CK1AE4ZCK/p1632319949393200

QiWang19 · 2021-09-22T19:03:58Z

@sttts @rphillips PTAL

sttts · 2021-09-24T15:11:42Z

config/v1/0000_10_config-operator_01_imagecontentpolicy.crd.yaml

+kind: CustomResourceDefinition
+metadata:
+  annotations:
+    api-approved.openshift.io: https://github.com/openshift/api/pull/470


technically you have to switch this to 874 now

This PR looks good to me. Thank you @sttts for helping us get this done. I'll lgtm once this link is fixed.

Add new CRD ImageContentPolicy to config/v1 group with allowMirrorByTags support. The schema of ImageContentPolicy contains all the schema from ImageContentSource and allowMirrorByTags support. Add validations of the fields. Request for enhancement: https://issues.redhat.com/browse/RFE-1608 Epic: https://issues.redhat.com/browse/OCPNODE-521 Registry mirror set by ImageContentSourcePolicy registryDigestMirrors only will be used if the image is referenced by digest because the mirror-by-digest of /etc/containers/registries.conf is set to true. This causes issue since there are use cases where no digests are available. In the new ImageContentPolicy CRD, add allowMirrorByTags to repositoryDigestMirrors spec, so user can easily configure it in the same spot they set post-installation mirror configuration. Signed-off-by: Qi Wang <[email protected]>

rphillips · 2021-09-24T15:27:55Z

/lgtm

sttts · 2021-09-24T15:32:07Z

/hold cancel
/approve

openshift-ci · 2021-09-24T15:33:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: QiWang19, rphillips, sttts

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~~ [sttts]

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

Bump api and client-go to include new CRD ImageContentPolicy openshift/api#874 . Bump openshift/api to openshift/api@636513e Bump openshift/client-go to openshift/client-go@067cd72 Signed-off-by: Qi Wang <[email protected]>

QiWang19 · 2021-10-10T03:41:41Z

Changes added to opeshift/api by this PR is addressed in the enhancement openshift/enhancements#929

bdurrow · 2022-04-18T15:25:45Z

As best I can tell, as of 4.10, OCPNODE-717 remains open and unimplemented. Until completed, ImageContentPolicy has no effect

QiWang19 · 2022-04-18T18:24:31Z

ImageContentPolicy won't be implemented. The design changed to use new CRDs ImageDigestMirrorSet and ImageTagMirrorSet https://issues.redhat.com/browse/OCPNODE-875 for the epic OCPNODE-521

openshift-ci-robot requested review from eparis and sttts March 23, 2021 20:55

openshift-ci-robot assigned eparis Mar 23, 2021

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2021

QiWang19 force-pushed the mirror-digest branch from aedda0e to 8706922 Compare June 29, 2021 04:42

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 29, 2021

QiWang19 force-pushed the mirror-digest branch from 8706922 to d186e67 Compare June 29, 2021 04:44

QiWang19 changed the title ~~Add MirrorByDigestOnly under RepositoryDigestMirrors~~ Add MirrorByDigestOnly to RepositoryDigestMirrors Jun 29, 2021

sttts reviewed Jun 29, 2021

View reviewed changes

operator/v1alpha1/types_image_content_source_policy.go Outdated Show resolved Hide resolved

sttts reviewed Jun 29, 2021

View reviewed changes

operator/v1alpha1/types_image_content_source_policy.go Outdated Show resolved Hide resolved

QiWang19 force-pushed the mirror-digest branch from d186e67 to e076908 Compare June 30, 2021 15:48

QiWang19 force-pushed the mirror-digest branch from e076908 to 6498ad2 Compare July 7, 2021 21:21

wking reviewed Jul 7, 2021

View reviewed changes

operator/v1alpha1/types_image_content_source_policy.go Show resolved Hide resolved

QiWang19 force-pushed the mirror-digest branch 7 times, most recently from 536d0c4 to 0f0599c Compare July 13, 2021 19:47

QiWang19 force-pushed the mirror-digest branch from 1a7a93f to b1351bf Compare September 21, 2021 19:46

QiWang19 changed the title ~~Add allowMirrorByTags and add ImageContentSourcePolicy to v1~~ Add allowMirrorByTags and create ImageContentPolicy CRD to v1 Sep 21, 2021

QiWang19 force-pushed the mirror-digest branch from b1351bf to 7826d17 Compare September 21, 2021 19:52

QiWang19 changed the title ~~Add allowMirrorByTags and create ImageContentPolicy CRD to v1~~ Create ImageContentPolicy CRD to v1 and allowMirrorByTags support Sep 21, 2021

QiWang19 force-pushed the mirror-digest branch 2 times, most recently from 55006e6 to 39a076c Compare September 22, 2021 18:21

QiWang19 changed the title ~~Create ImageContentPolicy CRD to v1 and allowMirrorByTags support~~ Create ImageContentPolicy CRD to config/v1 and allowMirrorByTags support Sep 22, 2021

sttts reviewed Sep 24, 2021

View reviewed changes

QiWang19 force-pushed the mirror-digest branch from 39a076c to cda7121 Compare September 24, 2021 15:24

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

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

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

openshift-merge-robot merged commit 71bd82e into openshift:master Sep 24, 2021

QiWang19 mentioned this pull request Sep 24, 2021

Update openshift/api version to include ImageContentPolicy openshift/client-go#195

Merged

QiWang19 mentioned this pull request Sep 28, 2021

Bump openshift/api, openshit/client-go openshift/cluster-config-operator#220

Merged

This was referenced Sep 30, 2021

[OCPNODE-553] bump(*) and config MirrorByDigestOnly using ImageContentPolicy openshift/runtime-utils#12

Closed

[OCPNODE-550] ImageContentPolicy change AllowMirrorByTags to *bool #1018

Closed

QiWang19 mentioned this pull request Oct 11, 2021

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

Merged

mtrmac mentioned this pull request Apr 5, 2022

Bug 1823143: wire ICSP lookups to oc image info openshift/oc#829

Merged

Create ImageContentPolicy CRD to config/v1 and allowMirrorByTags support #874

Create ImageContentPolicy CRD to config/v1 and allowMirrorByTags support #874

Uh oh!

Conversation

QiWang19 commented Mar 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Mar 23, 2021

Uh oh!

QiWang19 commented Mar 23, 2021

Uh oh!

55octet commented May 14, 2021

Uh oh!

QiWang19 commented May 26, 2021

Uh oh!

slauger commented May 27, 2021

Uh oh!

QiWang19 commented Jun 29, 2021

Uh oh!

QiWang19 commented Jun 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nee1esh commented Jul 2, 2021

Uh oh!

Uh oh!

QiWang19 commented Sep 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QiWang19 commented Sep 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rphillips commented Sep 24, 2021

Uh oh!

sttts commented Sep 24, 2021

Uh oh!

openshift-ci bot commented Sep 24, 2021

Uh oh!

QiWang19 commented Oct 10, 2021

Uh oh!

bdurrow commented Apr 18, 2022

Uh oh!

QiWang19 commented Apr 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

QiWang19 commented Mar 23, 2021 •

edited

Loading

QiWang19 commented Jun 29, 2021 •

edited

Loading

QiWang19 commented Sep 22, 2021 •

edited

Loading