Skip to content

https://issues.redhat.com/browse/ACM-29840#8605

Open
dockerymick wants to merge 3 commits into2.16_stagefrom
mj-ACM-29840
Open

https://issues.redhat.com/browse/ACM-29840#8605
dockerymick wants to merge 3 commits into2.16_stagefrom
mj-ACM-29840

Conversation

@dockerymick
Copy link
Contributor

also removed callouts from the file

@openshift-ci
Copy link

openshift-ci bot commented Feb 23, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dockerymick

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

Details Needs approval from an approver in each of these files:

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

@dockerymick dockerymick requested a review from smcavey February 23, 2026 21:19
----

. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map. Your config map might resemble the following YAML file:
. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be rewritten so that we don't have the spec in parenthesis, not sure how that passed peer review the first time.


. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map. Your config map might resemble the following YAML file:
. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map.
.. Collect `services` and `pods` from all `apiGroups`, while also collecting allowing all resources from the `admission.k8s.io` and `authentication.k8s.io` `apiGroups`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here if we are coding these, we need to name what they are. Or don't code.

. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map. Your config map might resemble the following YAML file:
. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map.
.. Collect `services` and `pods` from all `apiGroups`, while also collecting allowing all resources from the `admission.k8s.io` and `authentication.k8s.io` `apiGroups`.
.. Configure your config map to prevent the central collection of `secrets` from all `apiGroups`, while preventing the collection of `policies`, `iampolicies`, and `certificatepolicies` from the `apiGroup` `admission.k8s.io`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will pick up on this in a sec.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We kinda need to do the same with all of these, in that we typically don't just use the spec without naming it but I understand that would mean a lot of unnecessary rewrite. Recommend:

Configure your config map to prevent the central collection of secrets from all apiGroups specifications, while preventing the collection of policies, iampolicies, and certificatepolicies from the apiGroup admission.k8s.io specification.

----

. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map. Your config map might resemble the following YAML file:
. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map.
Copy link
Contributor Author

@dockerymick dockerymick Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map.
. List the resources in the `data.AllowedResources` and the `data.DeniedResources` fields within the config map.


. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map. Your config map might resemble the following YAML file:
. List the resources in the allow (`data.AllowedResources`) and deny list (`data.DeniedResources`) sections within the config map.
.. Collect `services` and `pods` from all `apiGroups`, while also collecting allowing all resources from the `admission.k8s.io` and `authentication.k8s.io` `apiGroups`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. Collect `services` and `pods` from all `apiGroups`, while also collecting allowing all resources from the `admission.k8s.io` and `authentication.k8s.io` `apiGroups`.
.. Collect services and pods from all `apiGroups`, while also collecting all resources from the `admission.k8s.io` and `authentication.k8s.io` `apiGroups`.

- iampolicies
- certificatepolicies
----
*Note:* If you do not provide a config map, all resources are collected by default. When you specify `AllowedResources`, all other resources are automatically excluded. If you list resources in both `AllowedResources` and `DeniedResources`, it is excluded from the collection.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*Note:* If you do not provide a config map, all resources are collected by default. When you specify `AllowedResources`, all other resources are automatically excluded. If you list resources in both `AllowedResources` and `DeniedResources`, it is excluded from the collection.
*Note:* If you do not provide a config map, all resources are collected by default. When you specify `AllowedResources`, all other resources are automatically excluded. If you list resources in both the `AllowedResources` and `DeniedResources` specifications, it is excluded from the collection.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then what is "it" in "it is excluded" -- that should be replaced with what it is to reduce confusion.

Copy link
Contributor

@swopebe swopebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see last comment.

- iampolicies
- certificatepolicies
----
*Note:* If you do not provide a config map, all resources are collected by default. When you specify `AllowedResources`, all other resources are automatically excluded. If you list resources in both `AllowedResources` and `DeniedResources`, it is excluded from the collection.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then what is "it" in "it is excluded" -- that should be replaced with what it is to reduce confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants