BZ#1928021: Restricting container registries in Configuring image settings

[mburke5678]

https://bugzilla.redhat.com/show_bug.cgi?id=1928021

When I updated and improved the topics on allowed, blocked, and insecure registries recently in the Image configuration resources assembly,I didn't bring the changes to the post-install Preparing for users module well enough.

With this PR I added some of the new information that I had added to Image configuration to Preparing for users and put a link to Image configuration resources.

Direct preview link: https://deploy-preview-34585--osdocs.netlify.app/openshift-enterprise/latest/post_installation_configuration/preparing-for-users.html#images-configuration-file_post-install-preparing-for-users

[mburke5678]

NOTE TO SELF
Remove the When using the allowedRegistries, blockedRegistries, or insecureRegistries parameter, you can specify an individual repository within a registry. For example: reg1.io/myrepo/myapp:latest. note and example reg1.io/myrepo/myapp:latest for 4.6 and 4.7.

[netlify]

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: ae9a147

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/60f8375d09d7e10007073d5a

😎 Browse the preview: https://deploy-preview-34585--osdocs.netlify.app

[simonkrenger]

There is a minor typo below.

Other than that, I am missing a section that describes that the following also works:
Instead of just specifying a registry (such as quay.io), it is also possible to limit projects within that registry:

spec:
  registrySources:
    allowedRegistries:
    - quay.io/openshift-release-dev

The above configuration will only allow images being pulled from quay.io/openshift-release-dev and for example will block images being pulled frrom quay.io/simonkrenger.

That should also be reflected in the documentation I believe

[mburke5678]

@simonkrenger Simon --

In the note below the example, I have When using the allowedRegistries, blockedRegistries, or insecureRegistries parameter, you can specify an individual repository within a registry..

I added an repo with a registry to the example:

  registrySources:
    allowedRegistries:
    - example.com
    - quay.io
    - registry.redhat.io
    - reg1.io/myrepo/myapp:latest

I believe the initial request was to explicitly add the entire quay.io registry, as there are several repos that are needed.

Thanks for your comment!

[mtrmac]

@simonkrenger Simon --

In the note below the example, I have When using the allowedRegistries, blockedRegistries, or insecureRegistries parameter, you can specify an individual repository within a registry..
…

    - reg1.io/myrepo/myapp:latest

NAK.

That would be a useful feature, but right now it only happens to work but is not designed to.

The API in https://github.com/openshift/api/blob/02b64ca27d54dafe24b8740e45662f45bd3a332e/config/v1/types_image.go#L110-L114 is not documented that way, there are AFAIK no regression tests, the relevant MCO code talks about registries and not repos. Unless all of that changes, the behavior may change any time, whether due to unrelated feature work or because someone decides to add stricter validation for allowedRegistries.

[mburke5678]

@mtrmac This change came from https://issues.redhat.com/browse/OCPNODE-461 starting in 4.8. If you think it doesn't work in this way, can you please file a docs BZ against 4.8 and assign it to me. I would be happy to look into it. It is out-of-scope for this PR, as removing things from the docs requires a (sometimes) lengthy approval process.

[mtrmac]

I’m not saying it doesn’t work, I’m saying there’s nothing established to ensure it will continue to work.

*sigh* If this is considered already publicly documented enough that there might be users’ deployments, I guess we can just roll with it and deal with regressions as they come; there’s no putting the genie back into the bottle.

[simonkrenger]

@mtrmac Documenting this was the root cause for https://bugzilla.redhat.com/show_bug.cgi?id=1928021.

Customers are concerned when they need to add quay.io as an allowed registry to their cluster. That would mean that any content from quay.io can be pulled by users - which for security-concious customers is not a good option.

So we created https://bugzilla.redhat.com/show_bug.cgi?id=1928021 to document that for example quay.io/openshift-release-dev an also be specified in the allowed registries, limiting the accessible images on quay.io.

If we do not want to document this (fine by me), we should at least provide some other mechanism to make sure customers can somehow limit what can be pulled from quay.io.

[mtrmac]

I’m not objecting to the idea of the feature at all; we would not provide “some other mechanism”, we would provide exactly this mechanism, but with code documentation and testing infrastructure that doesn’t exist yet.

We put the cart before the horse.

[simonkrenger]

Understood, thanks for the clarification

[mburke5678]

@mtrmac @simonkrenger I was asked in a BZ to add a note to the Blocked Registries topic that states To prevent pod failure, do not add the registry.redhat.io and quay.io registries to the blockedRegistries list, as they are required by payload images within your environment.

To be safe, I added another note to the Allowed Registries topic to make sure When the allowedRegistries parameter is defined, all registries, including the registry.redhat.io and quay.io registries, are blocked unless explicitly listed. If you use the parameter, to prevent pod failure, you must add registry.redhat.io and quay.io to the allowedRegistries list.

My concern now, if we provide an example of a specific quay.io repo (quay.io/openshift-release-dev), that would block all other quay repos, including ones that are needed to prevent pod failure?
Is it possible or reasonable to provide a list of all the quay.io repos that are required?

[mtrmac]

That’s a good question, but not one I can answer.

[mburke5678]

@mtrmac I reviewed the QE test case and it appears you might be correct. It appears that he used a dummy repo/registry to verify that the MCO updated the search registry lists on the nodes (which I also did, not having access to live repos/registries to test), and not whether the change has an effect on the node's ability to pull just from that registry. Is this your understanding?

[mtrmac]

That looks like a test case for containerRuntimeSearchRegistries, which has no relationship at all to supporting namespaces/repositories in allowedRegistries/blockedRegistries. I don’t know why the example lists a reg4.io/myrepo/myapp:latest or mentions reading policy.json without prescribing any contents to check for, maybe it’s a copy&paste from some other test case that might be relevant.

[mburke5678]

@simonkrenger Can I move this PR forward to address your core concern that the ability to add specific repos to the allow-lists?

I can track @mtrmac concerns and my questions about specific quay.io registries in separate issues, as needed.

[simonkrenger]

@mburke5678 Sure. I also updated RFE-1011 to share my thoughts.

[vikram-redhat]

I’m not objecting to the idea of the feature at all; we would not provide “some other mechanism”, we would provide exactly this mechanism, but with code documentation and testing infrastructure that doesn’t exist yet.

@xltian - any chance that this can be tested completely for us to document it?

[mtrmac]

(To clarify, I meant API documentation and automated unit tests; there well might have been some manual end-to-end testing I’m not aware of.)

[mburke5678]

@sunilcio Can you please take a look at the changes? I moved some information that was in the individual allowedRegistries and blockedRegistries topic into the Configuring image settings topic, which is aslo used in the Post-installation Preparing for Users topic. No new text.

Feel free to weigh in on any of the other concerns in this PR:

• BZ#1928021: Restricting container registries in Configuring image settings #34585 (review)
• BZ#1928021: Restricting container registries in Configuring image settings #34585 (comment)

[kalexand-rh]

Couple of minor things, but this is looking good!

[kalexand-rh]

Suggested change

<4> `registrySources`: Contains configuration that determines whether the container runtime allows or blocks individual registries when accessing images for builds and pods. Either the `allowedRegistries` parameter or the `blockedRegistries` parameter can be set, but not both. You can also define whether or not to allow access to insecure registries or registries that allow registries that use image short names. This example uses the `allowedRegistries` parameter, which defines the registries that are allowed to be used. The insecure registry `insecure.com` is also allowed. The `registrySources` paramter does not contain configuration for the internal cluster registry.

<4> `registrySources`: Contains configuration that determines whether the container runtime allows or blocks individual registries when accessing images for builds and pods. You can set either the `allowedRegistries` parameter or the `blockedRegistries` parameter but not both. You can also define whether or not to allow access to insecure registries or registries that allow registries that use image short names. This example uses the `allowedRegistries` parameter, which defines the registries that are allowed to be used. The insecure registry `insecure.com` is also allowed. The `registrySources` parameter does not contain configuration for the internal cluster registry.

Also, is "registries that allow registries that use image short names" right? I can't tell if the repetition is intentional.

[kalexand-rh]

Suggested change

When the `allowedRegistries` parameter is defined, all registries, including the registry.redhat.io and quay.io registries and the default internal image registry, are blocked unless explicitly listed. If you use the parameter, to prevent pod failure, you must add the `registry.redhat.io` and `quay.io` registries and the `internalRegistryHostname` to the `allowedRegistries` list, as they are required by payload images within your environment. Do not add the `registry.redhat.io` and `quay.io` registries to the `blockedRegistries` list.

When the `allowedRegistries` parameter is defined, all registries, including the registry.redhat.io and quay.io registries and the default internal image registry, are blocked unless explicitly listed. If you use the parameter, to prevent pod failure, you must add the `registry.redhat.io` and `quay.io` registries and the `internalRegistryHostname` to the `allowedRegistries` list, because they are required by payload images within your environment. Do not add the `registry.redhat.io` and `quay.io` registries to the `blockedRegistries` list.

Double-check the use of backticks in this note. I'd probably put registry.redhat.io and quay.io in bacticks because they're specific registries.

[kalexand-rh]

! should verb
Whenever possible, rewrite in a more direct way. Do not use "should" to refer to an action that must be performed.

[mburke5678]

@kalexand-rh Here should does not mean must, it is a recommendation, not requirement. I rewrote, also removing the passive:
Avoid using insecure external registries to reduce possible security risks.

[kalexand-rh]

Because this assembly heavily favors includes over xrefs, I suggest considering using an include instead. (I assume that you're happy with the modules that you're removing.)

[mburke5678]

@kalexand-rh I would need to add 2 includes, for a total of 4, to get all of the topics in the linked topic. That seemed a bit excessive, as the topic is already quite long. Going from what I have here, not using the includes for the image-configuration docs to adding them, adds ~15 page downs to the file on my laptop (~55 to ~70).
That one section should give enough info for folks to get started. The insecure and short name are optional. Perhaps @ahardin-rh can weigh in?

[sunilcio]

@mburke5678 @mtrmac I updated test case with correct steps for namespaces/repositories and tested again. Also added comment in OCPNODE-461 . The feature is working as expected. For API documentation and automated unit tests part mentioned in comment , I guess @QiWang19 can help answer?

[sunilcio]

/lgtm

[QiWang19]

I will look into how MCO works with the AllowedRegistries later.

[mburke5678]

/cherrypick enterprise-4.8